F1e: planning API for lane G and feedback from review actions

[mchwang]

Lane F, step F1, slice F1e: the planning API for lane G, and feedback from review actions. Stacked on #59 (F1d) → #57 → #56 → #53. This is the last F1 slice. Related: #22, #51.

What this does

Planning endpoints for G4 (web/server.ts, per the contract's "Planning API for lane G"):

Endpoint	Does
POST /api/plan/import	Store.importRevision with the expected revision
POST /api/plan/suggestions	E3 SuggestionCoordinator.start, bound to the expected revision and snapshot, with the user's feedback text (4,000 characters or fewer)
GET /api/plan/suggestions/:id	reads only the request row
POST /api/plan/suggestions/:id/cancel	pending: handle.cancel on the retained SuggestionHandle. Otherwise, or with no handle in this process: Store.cancelSuggestions
POST /api/plan/suggestions/:id/apply	Store.applySuggestion with the trusted plan context

• Every POST goes through Store.userAction, so it replays exactly by actionId.
• E3's settlement writes (completeSuggestions, and settleSuggestion in close()) run with the shutdown capability. The coordinator closes at shutdown step 6.
• Starting a suggestion is refused with "Planning agent not available yet." until a PlanningDeps provider is injected (live planning goes through D, D follow-ups required by the F1 runner lifecycle contract #51).
• ReviewService.planContext() builds the trusted plan context: base entries from git ls-tree at the snapshot's base, and the configured path identity.

Feedback from review actions:

• A change note, a segment accept and a segment assign now require an actionId (400 without one). The action and its feedback event (change-request, segment-accept, segment-assign) are written in one transaction, inside Store.userAction.
• A later choice for the same segment links (supersedes) to the latest earlier event for it.
• Choice keys embed segment content and have no size bound, so an event's sourceRef is choice:<sha256 of the stored choice key>: fixed-size and recomputable.
• Other actions (questions, approvals, settings) replay exactly when they carry an actionId, and stay backward-compatible without one. A replayed question does not start its agent again.
• The UI (web/public/app.js) sends crypto.randomUUID() with every review action.

Validation (head bfe30df, rebased onto F1d 5d163f2)

• npm run typecheck: passes.

• CI's unit set: 529 passed, 0 failed (includes F1d's new lock test). 10 of those are new, in test/runner-planning-feedback.test.ts, over real HTTP on the demo fixture:

  • a change note and its event, with replay;
  • 400 without an actionId;
  • a segment choice with its fixed-size source, and supersede;
  • no event for a question, and no second agent start on replay;
  • a refusal replayed as the same refusal;
  • planning unavailable;
  • import once per ID, and a stale import refused;
  • start, read and apply exactly once;
  • cancelling a pending suggestion (the provider's signal is aborted) and a ready one;
  • a pending suggestion settled at shutdown.

• npm run test:browser: 53 passed.

• Mutation check: six of seven broken versions were caught:

  • a change note recording no event;
  • a feedback action allowed without an ID;
  • a pending cancel skipping the handle;
  • suggestions not closed at shutdown;
  • supersedeLatest ignored;
  • apply bypassing the action table.

  Not caught: removing the !replayed question guard. Questions.start() already refuses a note that is being answered or answered, so the guard is a second layer with no observable effect.

CI failure on the first push

The first CI run failed F1d's cross-process lock test. The cause is fixed on #59 (5d163f2, a lock connection garbage-collected when the caller dropped it), and this branch is rebased onto it.

Test changes to existing tests

• test/browser/review.spec.ts ("can assign a large foreign change…") and three runner-shutdown tests now send the actionId that the API requires for feedback-producing actions. What they assert is unchanged.
• runner-shutdown.test.ts and the new planning/feedback test use a 20 s test timeout. Each review load reads git history, so these integration tests take 3–6 s under a full parallel run, over vitest's 5 s default. CI passed on F1c: shutdown wiring and /api/runner #57 with the default, but the margin is small.

Deferred

• Reject with feedback, accepted findings and needs-human guidance have no review actions yet (F2). Their event kinds and the Store method are ready.
• The review actions cannot reassign an already-accepted segment, so supersedes is reachable only when that action exists. It is tested at the Store level.
• Merge initiation through user_actions (contract round 16) needs the merge coordinator to update the saved response when the outcome is recorded. Deferred to F6, the merge handoff.
• A live planning provider and the issue retrieval for describe() wait for D follow-ups required by the F1 runner lifecycle contract #51 and G4.

🤖 Generated with Claude Code