Repository navigation
F: redraft the current plan through lane D (#124) - #125
Merged
Merged
Conversation
- Store schema v11: requests.mode ('suggest' | 'draft'); earlier requests are
suggestions. completeSuggestions checks the reply by mode (a draft is a v1
plan at revision n+1 for the plan's issue); getDraft and applyDraft, which
imports a ready, still-bound draft as n+1 with every import check.
applySuggestion refuses drafts and applyDraft refuses suggestions.
- E3: SuggestionCoordinator.start(input, mode). Draft mode prepares with
prepareDraft at n+1; stale checks compare the base revision; one active
planning invocation per plan for both modes.
- Server: POST /api/plan/drafts, GET /api/plan/drafts/:id, and
/api/plan/drafts/:id/(cancel|apply), recorded like suggestions; each kind's
routes refuse the other's IDs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- beginSuggestions requires its mode, so an adapter that drops it fails to typecheck; one PlanningMode type, from core. - A recorded cancel or apply includes the request ID from the path, so one action ID cannot replay across two suggestions or drafts. - An unknown ID on a draft route is "Unknown draft request.". - Docs: drafts in the user-action and idempotency-only lists; the suggestion GET returns mode and refuses drafts; a GitHub 502 is not recorded. - Tests: apply refused during a merge, on a closed task and under another identity or issue; dismissing a ready draft; mode column already present below v11; one invocation per plan in both orders; feedback reaches the draft prompt; action ID reuse across kinds and across two drafts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # runner/store.ts # test/publish.test.ts # test/runner-lifecycle-store.test.ts
mchwang
marked this pull request as ready for review
October 5, 2026 00:05
- Startup fails every planning request an earlier process left pending (Store.settleInterruptedRequests), under the single-runner lock. - A suggestion cancel or apply recorded before #124 (body-only hash) still replays its saved outcome. - "A planning invocation is still active" names both kinds. - Docs: architecture rows for drafts; cancel, one-invocation and startup wording in runner-lifecycle.md. - Tests: startup settling, sibling invalidation on apply, the context mismatch message, draft 502 and 503, one cancel action ID on two suggestions, and the legacy replay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The startup settle of pending requests moves into the production planning setup, after the single-runner lock is verified to name the database (productionPlanning requires verifyLock). - A cancel or apply body may not carry requestId (the path names it), so an older record's hash cannot match another request. - Only a missing request reads as "Unknown ... request." (Store.requestMode); a storage error is no longer recorded as a refusal. - The draft prompt names github.baseBranch when configured (#117 follow-up). - Docs: cancel-without-handle, the requests lifecycle row, the Store rows. - Tests: snapshot change while drafting, verify-before-settle, baseBranch, body requestId refusal, legacy refusal replay, storage error retry, HTTP dismiss and shutdown cancel. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A cancel or apply looks up its replay first (AGENTS.md), then refuses a body requestId; the legacy body-only fallback never applies to a body that carries one. - A refused planning startup (lock no longer names the database) exits with its message, not a stack, without a runner block too. - Docs: startup order includes the planning settle; the cancel-without-handle note holds only while planning is on; lane D sees a draft as revision n+1. - Tests: CLI start fails a pending request left by an earlier process; the Store is closed when planning setup refuses; the settle covers every plan and no other state; replay before the body check; legacy apply replay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- productionPlanning runs inside the CLI's startup try again, so its errors release the lock as before. - E3's closing reason is "Planning coordinator is closing." (drafts too). - The requests lifecycle row says the startup settle runs while planning is on. - Test: a body naming another request never makes the old-hash fallback replay this build's record. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- An empty github.baseBranch falls back to the base commit in the prompt. - Tests: the configured base branch (or the base commit) reaches the prompt through the server; a draft against a stale revision or snapshot is refused and starts nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Docs: the planning setup runs with or without a runner block. - Tests: a replayed draft start does not read GitHub again; one apply action ID cannot act on two drafts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The draft lifecycle is consistently guarded, persisted, documented, and covered by focused regression tests.
Review effort: Balanced
Findings: None
What changed in this PR
Adds durable next-revision plan drafting through the existing planning worker and API lifecycle.
Changes:
- Adds schema v11 draft storage, validation, application, and recovery.
- Adds draft-mode coordination and recorded HTTP routes.
- Expands lifecycle documentation and regression coverage.
| File | Description |
|---|---|
runner/store.ts |
Adds schema v11 and draft persistence/application. |
core/planning-suggestions.ts |
Adds draft-mode coordination. |
web/server.ts |
Adds recorded draft API routes. |
web/planning.ts |
Settles interrupted requests and supplies base branches. |
web/cli.ts |
Verifies the database lock during planning startup. |
test/planning-drafts.test.ts |
Covers Store and coordinator draft behavior. |
test/planning-drafts-api.test.ts |
Covers draft HTTP lifecycle and replay. |
test/planning-production.test.ts |
Covers production planning startup. |
test/planning-suggestions.test.ts |
Updates shared coordinator expectations. |
test/planning-acceptance.test.ts |
Updates suggestion request setup. |
test/store.test.ts |
Updates Store tests for request modes. |
test/runner-lifecycle-store.test.ts |
Updates schema-version assertions. |
test/publish.test.ts |
Updates schema-version assertions. |
test/cli.test.ts |
Covers interrupted-request startup recovery. |
docs/implementation/runner-lifecycle.md |
Documents draft APIs and lifecycle. |
docs/architecture.md |
Reflects active draft planning support. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #124. #110 (schema v10) has merged, and main is merged into this branch: the Store migrates v10 then v11, and the four "latest version" assertions (three existing, one from #110's new test) expect 11.
What changes
POST /api/plan/draftsasks Claude, through #117's planning worker, for a whole next revision of the review's current plan. The draft is kept as a durable request and becomes revision n+1 only when the user applies it.requests.mode('suggest' | 'draft'); earlier rows read as suggestions.beginSuggestionsrequires the mode.completeSuggestionschecks the reply by mode: a draft is a v1 plan at n+1 for the plan's issue.getDraft, andapplyDraft, which imports a ready draft still bound to the current revision and snapshot, with every import check, through#savePlan(merge and closed-task guards) and then consumes it.applySuggestionandapplyDrafteach refuse the other's rows.SuggestionCoordinator.start(input, mode).prepareDraftat n+1.POST /api/plan/drafts,GET /api/plan/drafts/:id, andPOST /api/plan/drafts/:id/(cancel|apply), recorded like suggestions: replay first, then the GitHub read, then F: wire the planning provider into the server (suggestions through lane D) #117's budget and shutdown rules.runner-lifecycle.md's planning API table and user-action contract.Validation
npm run typecheck: passed.npm testcommand on the branch merged with main: 65 files, 1529 tests passed.test/planning-drafts.test.ts(21, Store and E3);test/planning-drafts-api.test.ts(5, HTTP routes).Independent review
A fresh subagent found no blocking bug. Fixed:
Not changed:
importPlancan be publishedreadyby a directcompleteSuggestionscaller. E3 validates with context first, so only a non-E3 caller could do this, andapplyDraftstill refuses it.🤖 Generated with Claude Code