fix(index): expose unwired generation execution honestly

[PeterGuy326]

Merge HOLD and review provenance (2026-09-10)

This PR is now Draft. #192 is still open. The selected base codex/fix-web-audit at 11e02e21ef2c3dbd2dae26e4376872e54e78ecb5 is not an ancestor of this head 8cde0862848c33199860eac6ef85bfd16bfda04b; the lockfile change was duplicated independently, not inherited. Its bytes match #192, which does not make the commits ancestors.

Do not merge this PR into the dependency branch. First obtain a real non-author approval and land #192 in main; then explicitly retarget this PR to main, fetch/integrate that actual main commit without rewriting published history, require git merge-base --is-ancestor origin/main HEAD to exit 0, and rerun checks and non-author review on the resulting candidate.

Author-posted automated preflight comments are supplemental evidence only. They are not an independent human review or an APPROVE vote. No self-approval, review dismissal, protection bypass or merge is performed by this status update.

Summary

Refs #174. This canonical PR supersedes the implementation in #182 without closing the Issue automatically.

The index-generation API currently advertises execution_wired: true even though no worker claims targets and search does not route generation vectors. This change makes the capability honest and fail-closed:

• report execution_wired: false on generation responses;
• return 503 execution_unavailable from create and activate instead of creating stuck or empty generations;
• align the public index-generation documentation;
• keep focused tests honest about the JSON error key and isolate fake-service state between subtests;
• record the user-visible behavior under CHANGELOG.md [Unreleased].

Scope and non-goals

This is the smallest resolution from #174. It does not implement the future worker executor, generation-aware search routing, or retention scheduler.

Validation ledger

ID	Acceptance criterion	Command	Environment	Expected / observed	Status
V1	Disabled execution is reported and mutation handlers fail closed	go test ./internal/api ./internal/indexgeneration from server/	macOS arm64, Go module under server/	Both packages passed	PASS
V2	No repository regression	go test ./... from server/	macOS arm64, Go module under server/	All packages passed; no failures	PASS
V3	Patch has no whitespace errors	git diff --check	local canonical branch tree	Exit 0	PASS
V4	Real remote E2E worker execution	Requires an index-generation worker and generation-aware search deployment	unavailable in this checkout	Not exercised; intentionally out of scope for this fail-closed fix	NOT VERIFIED

The source tree on this PR head was independently compared with the locally tested tree after the GitHub ref was created.

Risk / rollback

The API now rejects unsupported mutating operations with an actionable 503 rather than accepting work that cannot progress. Read-only status routes remain available. Reverting this PR restores the old misleading capability advertisement.

Contributor provenance (2026-09-10)

Original implementation: #182 by @sun-970 (Li Yuanyang; commits attributed to liyuanyang). This canonical PR carries that implementation with maintainer integration/test corrections; original contribution credit is retained.

This attribution update does not rewrite published commits or record a new code review. Any eventual squash commit should retain verified human contributor credit from the linked source PRs.

[PeterGuy326]

CI baseline dependency: current main Web audit is independently red on published dependency advisories. Separate #192 refreshes only the Web lockfile. This PR is now stacked on #192; merge/rebase #192 first, then retarget #189 to main. The #189 application diff remains index-generation-only.

[PeterGuy326]

Candidate: 8cde0862848c33199860eac6ef85bfd16bfda04b.

Independent automated preflight (Mendel, not the implementation worker): PASS, bounded source/CI/compile-only scope. This is not a human APPROVE.

The original blocking review on #182 was checked against current handlers/tests: assertions now use the actual error response, fake state resets between cases, and unwired create/activate return 503 before any mutation. Exact-head PostgreSQL CI confirms the index-generation integration test executes and passes, including race detection; all 20 checks are green. Linux test cross-compilation and diff/format checks passed. The reviewer did not execute temporary macOS Go binaries; implementation-worker live-service evidence is separately attributed.

Non-blocking documentation follow-up: INDEX_GENERATIONS.md currently says every response has execution_wired; events/errors do not. No generation-worker availability is claimed by this preflight.

Still stacked on #192. Land the baseline through independent human review, then retarget and recheck the final candidate; earlier PR votes are not inherited.

[Bindy-lbb]

Review summary (verified independently)

• Head SHA confirmed at 8cde0862 (matches). CI confirmed 20/20 green independently.
• Reviewed handlers_index_generations.go: execution_wired now honestly reports false, and the create/activate paths return 503 execution_unavailable before touching the underlying service. Tests genuinely assert the service methods are never invoked, plus the 503 response — this is a legitimate fix, not a stubbed test.

Blocking item: this PR's base is #192's branch (codex/fix-web-audit), not main, and that base is not yet a real git-ancestor relationship — please hold this until #192 lands and this PR is retargeted to main (see note on #192).

Also: the "independent preflight" comment on this PR is from the same account as the commit author — recommend a genuinely independent reviewer before merge.

[sun-970]

Code Review

Changes look clean and well-scoped. A few observations:

What's good:

• execution_wired flipped from true to false across all response paths (list, get, mutation actions) — consistent and honest.
• handleCreateIndexGeneration drains the request body (io.Copy(io.Discard, r.Body)) before returning 503, which is correct for HTTP connection reuse.
• The activate case in indexGenerationBuildAction short-circuits with 503 before touching the service layer — fail-closed as intended.
• Tests verify both the HTTP response status/error key AND that the underlying service methods are never invoked (service.lastProfile == "", service.lastAction == ""). This is the right level of assertion — it proves the guard actually prevents mutation, not just that the right status code is returned.
• TestExecutionWiredFlagMatchesCapability is a nice integration-level test that cross-checks the flag against actual behavior.
• Removed the now-unused encoding/json import — clean.

Minor note:

• In handleCreateIndexGeneration, the body drain happens before the s.IndexGenerations == nil check. This is fine functionally (draining is always safe), but the ordering is slightly unusual. No action needed — just noting it.

Base branch concern (echoing existing comments):

This PR is stacked on codex/fix-web-audit (#192), not main. Should be retargeted to main after #192 lands.

Overall: solid fail-closed fix. The API now honestly refuses work it cannot complete rather than silently creating stuck state.

[waterbro-8]

REQUEST-CHANGES — a PR whose whole point is honesty ships a false statement about its own responses

The core of this PR is right and I want it on record: flipping execution_wired: true → false
on the three success paths is the correct fix for the reported bug, activate returning 503
instead of quietly succeeding is right, and TestExecutionWiredFlagMatchesCapability (:285) is
the kind of test that earns its keep — it asserts the flag and that create/activate cannot
succeed while it's false. The three problems below are all in the margins around that core.

S2-1 — "All responses include execution_wired: false" is false on this head

docs/INDEX_GENERATIONS.md:137. Two classes of counterexample, both in this same handler file:

• handleListIndexGenerationEvents writes map[string]any{"items": events} and nothing else
  (handlers_index_generations.go:83). No flag.
• every error response: writeError (util.go:23) writes exactly {"error", "hint"}. So the
  503 execution_unavailable you are introducing does not carry the flag — which is the one
  response a client most plausibly needs it on, since it is the response whose meaning is
  "this is not wired" rather than "try again later".

Only three success paths carry it (:39, :62, :160). Say "list/status/action success
responses", or add the flag to the events payload and to the 503 body. This is the follow-up
Peter left non-blocking in 8cde0862; it is still unfixed, and a PR whose subject is the
honesty of this flag is the wrong place to leave it approximate.

S2-2 — create loses its input validation, and the test that covered it now asserts something else

At head, handleCreateIndexGeneration drains the body unread (io.Copy(io.Discard, r.Body),
:87) and unconditionally 503s. Main's bad_json (with DisallowUnknownFields) and
bad_profile_id 400s are gone. The consequence in the test file is the one I'd like addressed:

• create_rejects_empty_profile (handlers_index_generations_test.go:213) now asserts
  StatusServiceUnavailable where it asserted StatusBadRequest. The handler ignores the body
  entirely, so this subtest can no longer fail for any profile value — including a body of
  {{{{. It is a second copy of the create case (:192) wearing a name that describes a
  check that no longer exists.

Rename it to what it now proves (create_is_unavailable), and decide deliberately whether a
malformed request should still get a 400 in front of the 503. Returning 503 for syntactically
invalid input invites clients to retry-loop a permanent condition; if you intend that tradeoff,
say so in the doc rather than leaving it implicit in a deleted validation block.

S2-3 — the guarantee is enforced one layer above the only writer

indexGenerationBuildAction gates create and activate in the handler, but rollback still
runs the full activation write path:
Service.Rollback (service.go:513) → activate(…, rollback=true) (store.go:419) →
UPDATE index_generation_builds SET state = 'active', activated_at = … (store.go:523) →
UPDATE index_generations SET state = 'active' (store.go:530) → saveActiveProfile(...), and
the route answers 200 with execution_wired: false. Service.Activate is likewise still
live, just unreachable over HTTP now.

So the exact false state your own new doc paragraph warns about — metadata saying "active" while
queries use the released legacy embedding tables — remains reachable through one of the four
mutation routes this PR deliberately leaves open.

Two honest caveats, because I did not resolve this and I don't want to assert more than I know:

1. I could not determine whether a build can actually satisfy the quality gate with no executor
   (the stateAllowed / FailedTargets == 0 / Succeeded+Skipped == Required / vectors-present
   conjunction in activate). RequiredTargets == 0 passes that arithmetic vacuously, and
   rollback additionally requires a pre-existing active build. If that combination is
   unreachable today, the fix is to say so — currently docs/INDEX_GENERATIONS.md:112 ("Cancel,
   resume, rollback and discard … cannot produce a searchable corpus") asks the reader to take
   that on faith, and it is the one claim in the new text I could not confirm from the code.
2. Either way this belongs in activate(), not the handler, or it needs a test on the rollback
   route's outcome. There is none: Rollback exists in the fake service (:96) but no
   subtest calls it.

Related and cheap: TestIndexGenerationPublicRoutesAreReadOnly (:325) does not test
read-only-ness at all — it walks chi routes and asserts six POST routes are registered. After
this PR the name is actively misleading, and it is the test a future reader will trust as the
guardrail. Worth renaming alongside S2-2's subtest.

Provenance

Static read of head 442b81bb (handlers_index_generations.go, its test file,
docs/INDEX_GENERATIONS.md) plus indexgeneration/service.go, store.go and api/util.go on
main 3c13f04. I ran no Go tests and made no live API calls, so S2-3 is a reachability
question I am handing you, not a demonstrated exploit. Draft status, and
mergeable_state=dirty — it conflicts with main as it stands.

[waterbro-8]

Review: the code change is right, the sentence describing it is false

The behavioural change here is the correct one. Forcing execution_wired: false, returning 503 execution_unavailable from create and activate, and refusing to call s.IndexGenerations.Create are exactly right — the previous state let metadata claim active while search still used the released legacy embedding tables. TestExecutionWiredFlagMatchesCapability is a good test to have. I am not asking you to change the logic.

But the doc line this PR adds is falsifiable with one curl

docs/INDEX_GENERATIONS.md:137 on this head:

All responses include execution_wired: false. The flag is honest: no code path executes a build or routes search queries through generation vectors.

"All responses" is not true, and this PR is the reason it is not true. writeError on main (server/internal/api/util.go:23) writes:

func writeError(w http.ResponseWriter, status int, code, hint string) {
	writeJSON(w, status, map[string]any{
		"error": code,
		"hint":  hint,
	})
}

There is no execution_wired key in that body. So the actual surfaces on this head are:

endpoint	response	has execution_wired?
GET .../index-generations	200 {items, execution_wired:false}	yes
GET .../index-generations/{id}	200 {generation, execution_wired:false}	yes
POST .../index-generations	503 {error:"execution_unavailable", hint:...}	no
POST .../{id}/activate	503 {error:"execution_unavailable", hint:...}	no
POST .../{id}/cancel|resume|rollback|discard	200 {generation, execution_wired:false}	yes

Two of the five write paths do not carry the field. A reader who follows the document and calls create will get a body with no such key.

I want to be precise about why I am blocking on a doc line rather than waving it through as cosmetic. The entire premise of this PR is "stop claiming a capability we do not have". Shipping an absolute claim that the next curl disproves re-breaks exactly the trust this PR exists to repair — and it does so in the file a reader consults to decide whether they can trust the flag. The cost of the bug is low; the cost of the class of bug, in this specific PR, is high.

Pick one, but align them

1. Narrow the sentence. Something like: read responses and status-changing actions include execution_wired: false; create and activate return 503 execution_unavailable and do not carry the field.
2. Or make it true — have the 503 body also carry execution_wired: false, so a client can branch on the field alone instead of on the status code.

I mildly prefer (2) on API-design grounds: a capability flag that is present on every response of a surface is easier to consume than one that is present on some responses and implied absent by a status code on others. But either is fine. What is not fine is the current state.

One contract that quietly lost its test

TestIndexGenerationMutationHandlers previously asserted http.StatusBadRequest for a malformed body; that subtest now asserts http.StatusServiceUnavailable. That is the correct new behaviour, since create no longer parses the body — but it means the old negative case for body validation was replaced rather than preserved, and nothing now pins the fact that malformed JSON must yield 503 rather than 400. Please add that as an explicit case. It is a one-liner and it stops a future refactor from reintroducing body parsing without anyone noticing.

[waterbro-8]

Draft + conflicting, CHANGES_REQUESTED still standing. Not merging. Rebase and resolve the requested changes, or close in favor of a current-main successor.

[xiaocui-big]

平台组 Review（崔泽生，assigned by @冯浩然）

一、总体评价

核心改动方向正确：将 execution_wired 从 true 改为 false，并在 create/activate/rollback 路径上返回 503 execution_unavailable 而非静默创建无法完成的构建。这是"诚实失败"（fail-closed）修复，解决了 #174 描述的"API 宣称有能力但实际没有"的信任问题。

代码变更范围小且聚焦（3 个文件），测试覆盖了关键路径。但存在若干需要关注的问题。

二、Handler 逻辑——基本正确，一处防御深度不足

• execution_wired: false 在所有三个成功响应路径一致设置。✅
• handleCreateIndexGeneration 在返回 503 前用 io.Copy(io.Discard, r.Body) 排空请求体，对 HTTP 连接复用正确。✅
• activate 和 rollback 在 handler 层被短路为 503，service 方法未被调用。测试验证了 service.lastAction 为空。✅

需要关注：Service.Rollback 仍然调用 activate(…, rollback=true) 执行完整写入路径。当前 handler 层的 503 短路阻止了 HTTP 到达该路径，但如果未来有人直接调用 service 层或添加新路由入口，保护就不存在了。建议至少在 Service.Rollback 中添加注释说明当前依赖 handler 层阻止调用，或在 service 层也加 guard（defense in depth）。

三、测试——关键路径覆盖，但有遗漏

• create 子测试验证 503 + execution_unavailable + service.Create 未被调用 ✅
• activate_blocked 和 rollback_blocked 验证 503 + service 方法未被调用 ✅
• TestExecutionWiredFlagMatchesCapability 交叉验证 flag 值与实际行为 ✅

遗漏：

1. 缺少 resume 和 discard 成功响应中 execution_wired: false 的断言——既然 PR 核心是让 flag 诚实，建议覆盖所有成功路径
2. events 端点不包含 flag，没有测试固定这个行为

四、文档——一处不一致

docs/INDEX_GENERATIONS.md 的 HTTP 路由列表中，rollback 没有标注 → 503 execution_unavailable，但代码中 case "rollback" 确实返回 503。CLI 部分标注了 rejected until execution is wired（正确），但 HTTP 路由列表遗漏了。

五、合并阻塞项

项	状态
waterbro-8 的 CHANGES_REQUESTED	❌ 需确认修改是否满足
文档 rollback 路由 503 标注	❌ 遗漏
Rebase 到 main（behind）	❌ 需 rebase
非 author APPROVE	❌ 尚无
#192 依赖	✅ 已合并

六、核心判断

行为变更正确——API 不再假装拥有它没有的能力。测试覆盖了 fail-closed 关键路径。主要问题在文档准确性和合并流程上，而非逻辑正确性。建议修复上述遗漏后尽快合并，因为每多一天 main 上就多一天"API 撒谎"的状态。

[waterbro-8]

崔泽生指出的 HTTP 路由列表遗漏已补：rollback 与 create/activate 一样标注 → 503 execution_unavailable。

execution_wired 的表述在当前 head 已是「Successful list/status/cancel/resume/discard… events 只有 items；create/activate/rollback 返回 503」，不再写 All responses。我这边的 CHANGES_REQUESTED 那句文档不实，这条已覆盖。

分支仍 behind main，rebase 需要作者 @PeterGuy326。

[waterbro-8]

Addressed CHANGES_REQUESTED on a rebase onto current main (825998b).

• S2-1: events payload and 503 execution_unavailable now include execution_wired: false.
• S2-2: create validates JSON / empty profile_id as 400 before the 503. Restored create_rejects_empty_profile and added create_rejects_malformed_body.
• S2-3: HTTP create/activate/rollback still never call Service.Create/Activate/Rollback (tested). Those service methods stay in-process for tests; docs now say that cancel/resume/discard cannot produce a searchable corpus because the only HTTP writers of active are activate/rollback and both are 503. Renamed the route-walk test earlier to TestIndexGenerationPublicRoutesAreRegistered.

请再审。本地没有 Go 1.25，CI 会跑 handler 测试。