diff --git a/README.md b/README.md index c6840661..a47b53a8 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ A configuration with a `github` block may also add a `runner` block. The runner - `diagnosticsCapBytes`: the size retention trims that directory back to. It is a target, not a hard limit: the file just saved is always kept, even when it alone passes the cap. The default is 256 MiB. - `limits`: task storage limits (`workBytes`, `workInodes`, `metadataBytes`, `metadataInodes`). -The runner needs Docker and `CLAUDE_CODE_OAUTH_TOKEN`. At start, before the server opens, codeboost recovers what an earlier run left and builds the agent image; a recovery it cannot finish safely stops startup with what to do. `POST /api/runner` with `start` or `resume` runs the plan; the review screen has no buttons for them yet. When a run has completed every item, codeboost pushes the task head to a `codeboost/…` branch with your `gh` credentials and opens a ready pull request into `baseBranch`; a task that needs a person gets a draft pull request with its problems. A publish that was refused (for example, the branch holds a commit codeboost did not make) is retried with the `publish` action, and `GET /api/runner` shows the last outcome under `publish`. When a task is cancelled, codeboost closes the open pull requests it opened for it (the branch is kept). A pull request a person moved is left open and reported, and `close-pull-requests` retries a close that failed. Merge PR then merges that pull request, so `github.pullRequest` can be left out; if it is set, it must name the task's pull request, or merging is blocked. Demos never publish. +The runner needs Docker and `CLAUDE_CODE_OAUTH_TOKEN`. At start, before the server opens, codeboost recovers what an earlier run left and builds the agent image; a recovery it cannot finish safely stops startup with what to do. `POST /api/runner` with `start` or `resume` runs the plan; the review screen has no runner buttons yet. A task paused for an out-of-scope change stays paused until the plan is amended, every changed path is declared, the remaining items validate against the runner's audited tree, and a person sends `approve-continuation` with the current `expectedStateVersion`, `expectedReviewVersion`, and an action ID. Then `resume` starts at the first unfinished item. `GET /api/runner` reports the checkpoint and whether the continuation is approved. When a run has completed every item, codeboost pushes the task head to a `codeboost/…` branch with your `gh` credentials and opens a ready pull request into `baseBranch`; a task that needs a person gets a draft pull request with its problems. A publish that was refused (for example, the branch holds a commit codeboost did not make) is retried with the `publish` action, and `GET /api/runner` shows the last outcome under `publish`. When a task is cancelled, codeboost closes the open pull requests it opened for it (the branch is kept). A pull request a person moved is left open and reported, and `close-pull-requests` retries a close that failed. Merge PR then merges that pull request, so `github.pullRequest` can be left out; if it is set, it must name the task's pull request, or merging is blocked. Demos never publish. ## Library diff --git a/core/plan.ts b/core/plan.ts index 3bfec071..496036ba 100644 --- a/core/plan.ts +++ b/core/plan.ts @@ -42,6 +42,7 @@ export interface PlanContext { allowedCommands: readonly (readonly string[])[]; issue: number; } +export interface ContinuationBinding { checkpointId: string; head: string; completedItems: readonly string[] } export interface Validation { errors: Diagnostic[]; warnings: Diagnostic[] } export class PlanError extends Error { readonly diagnostics: Diagnostic[]; @@ -137,6 +138,39 @@ export function validatePlan(value: unknown, context: PlanContext): Validation { return validator(value, context); } +/** Validate only work still to run, starting with the runner-audited tree at its checkpoint. */ +export function validateContinuationPlan(plan: Plan, context: PlanContext, completedItems: readonly string[], allowEmptySuffix = false): Validation { + try { assertPlan(plan); } catch (error) { + if (error instanceof PlanError) return { errors: error.diagnostics, warnings: [] }; + throw error; + } + const prefix = plan.items.slice(0, completedItems.length).map(item => item.id); + if (!completedItems.length || prefix.length !== completedItems.length || prefix.some((id, index) => id !== completedItems[index]) || + completedItems.length > plan.items.length || (!allowEmptySuffix && completedItems.length === plan.items.length)) + return { errors: [{ code: 'continuation-prefix', message: 'The amended plan must retain the completed item prefix and have a remaining item.' }], warnings: [] }; + const ids = new Set(); + for (const item of plan.items) { + if (ids.has(item.id)) return { errors: [{ code: 'duplicate-id', message: `Duplicate item ID ${item.id}.`, item: item.id }], warnings: [] }; + if (new Set(item.depends_on).size !== item.depends_on.length) + return { errors: [{ code: 'dependency', message: 'Dependencies must be unique.', item: item.id }], warnings: [] }; + ids.add(item.id); + } + // There is no executable suffix to validate, but the plan remains bound to + // the selected issue even after every item has completed. + if (completedItems.length === plan.items.length) { + if (!Number.isSafeInteger(context.issue) || context.issue < 1) + return { errors: [{ code: 'context', message: 'A selected issue is required.' }], warnings: [] }; + if (plan.issue !== context.issue) + return { errors: [{ code: 'issue', message: 'Plan issue does not match the selected issue.' }], warnings: [] }; + return { errors: [], warnings: [] }; + } + const completed = new Set(completedItems); + const suffix: Plan = { ...plan, items: plan.items.slice(completedItems.length).map(item => ({ + ...item, depends_on: item.depends_on.filter(id => !completed.has(id)), + })) }; + return validatePlan(suffix, context); +} + function validateV1(value: Plan, context: PlanContext): Validation { try { assertPlan(value); } catch (error) { if (error instanceof PlanError) return { errors: error.diagnostics, warnings: [] }; @@ -273,15 +307,17 @@ function validateV1(value: Plan, context: PlanContext): Validation { return { errors, warnings }; } -export function importPlan(source: string | Uint8Array, format: 'json' | 'yaml', context: PlanContext, revision: number): { plan: Plan; warnings: Diagnostic[] } { +export function importPlan(source: string | Uint8Array, format: 'json' | 'yaml', context: PlanContext, revision: number, + completedItems?: readonly string[]): { plan: Plan; warnings: Diagnostic[] } { if (!Number.isSafeInteger(revision) || revision < 1) fail('revision', 'Revision must be a positive safe integer.'); let data: unknown; try { data = parseV1(source, format); } catch (error) { fail('parse', `Cannot parse plan: ${(error as Error).message}`); } assertPlan(data); // No migrations exist yet: only released v1 is accepted. - const result = validatePlan(data, context); + const plan = { ...data, revision }; + const result = completedItems === undefined ? validatePlan(data, context) : validateContinuationPlan(plan, context, completedItems, true); if (result.errors.length) throw new PlanError(result.errors); - return { plan: { ...data, revision }, warnings: result.warnings }; + return { plan, warnings: result.warnings }; } const payloads = ['field', 'value', 'file', 'check', 'check_index', 'depends_on', 'new_item'] as const; @@ -297,7 +333,8 @@ export interface SuggestionBinding { /** Pure transformation. The store must load this binding by opaque suggestion ID, * verify cancellation/consumption, and CAS revision plus consume/invalidate IDs atomically. * This function cannot provide persistence, replay prevention, or concurrency control. */ -export function applySuggestion(plan: Plan, reply: unknown, index: number, context: PlanContext, binding: SuggestionBinding): Plan { +export function applySuggestion(plan: Plan, reply: unknown, index: number, context: PlanContext, binding: SuggestionBinding, + completedItems?: readonly string[]): Plan { assertPlan(plan); assertEditReply(reply); if (!binding || identityKey(binding.identity) !== identityKey(context.identity) || binding.schemaVersion !== plan.schema_version || binding.baseRevision !== reply.base_revision || @@ -339,7 +376,7 @@ export function applySuggestion(plan: Plan, reply: unknown, index: number, conte case 'set_depends': item.depends_on = edit.depends_on!; break; } } - const result = validatePlan(next, context); + const result = completedItems === undefined ? validatePlan(next, context) : validateContinuationPlan(next, context, completedItems, true); if (result.errors.length) throw new PlanError(result.errors); if (!Number.isSafeInteger(next.revision + 1)) fail('revision', 'Revision limit reached.'); next.revision++; diff --git a/core/planning-author.ts b/core/planning-author.ts index 376e3198..3dfe81ee 100644 --- a/core/planning-author.ts +++ b/core/planning-author.ts @@ -1,7 +1,7 @@ import { readFileSync } from 'node:fs'; import { identityKey, type PlanIdentity } from './identity.ts'; import { parseV1 } from './parse-v1.ts'; -import { applySuggestion, assertEditReply, importPlan, validatePlan, PlanError, +import { applySuggestion, assertEditReply, importPlan, validateContinuationPlan, validatePlan, PlanError, type Diagnostic, type EditReply, type Plan, type PlanContext } from './plan.ts'; import registry from '../schema/versions.json' with { type: 'json' }; @@ -19,6 +19,8 @@ export interface AuthorInput { approvedLessons: readonly string[]; feedback: string; previousPlan?: Plan; + completedItems?: readonly string[]; + continuationContext?: { checkpointId: string; head: string; completedItems: readonly string[]; ownerItem: string; outOfScopePaths: readonly string[] } | null; } export interface AuthorRequest { readonly mode: 'draft' | 'suggest'; @@ -39,7 +41,7 @@ export interface AuthorProvider { } export interface PreparedAuthor { readonly request: AuthorRequest; - validate(source: string | Uint8Array): { value: T; warnings: Diagnostic[] }; + validate(source: string | Uint8Array): { value: T; warnings: Diagnostic[]; candidatePlans: Plan[] }; } function integer(value: number, label: string): void { @@ -93,8 +95,17 @@ function prepare(input: AuthorInput, mode: AuthorRequest['mode']): PreparedAutho const context: PlanContext = { ...input.context, identity: { repositoryId, taskId, planId }, baseEntries: structuredClone(input.context.baseEntries), allowedCommands: structuredClone(input.context.allowedCommands) }; const previous = input.previousPlan ? structuredClone(input.previousPlan) : undefined; + const completedItems = input.completedItems === undefined || input.completedItems.length === 0 + ? undefined : structuredClone([...input.completedItems]); + if (completedItems && (!input.continuationContext || input.continuationContext.completedItems.length !== completedItems.length || + input.continuationContext.completedItems.some((item, index) => item !== completedItems[index]))) + throw new Error('Planning continuation context does not match its completed prefix.'); if (previous) { - const result = validatePlan(previous, context); + // The existing suffix is context for the provider to repair, not a candidate. Validate its structure and the + // checkpoint-owned prefix here; require the proposed candidate to pass full suffix semantics below. + const result = completedItems === undefined ? validatePlan(previous, context) : validateContinuationPlan({ + ...previous, items: previous.items.slice(0, completedItems.length), + }, context, completedItems, true); if (result.errors.length) throw new PlanError(result.errors); if (input.revision !== previous.revision + (mode === 'draft' ? 1 : 0)) throw new Error('Previous plan revision mismatch.'); } else if (mode === 'suggest') throw new Error('Suggestions require a previous plan.'); @@ -115,6 +126,9 @@ function prepare(input: AuthorInput, mode: AuthorRequest['mode']): PreparedAutho lessons_data_json: dataJSON(input.approvedLessons, 'Lessons'), feedback_data_json: dataJSON(input.feedback, 'Feedback'), previous_plan_json: dataJSON(previous ?? null, 'Previous plan'), + continuation_context_instruction: input.continuationContext + ? `Checkpoint continuation context:\n\n${dataJSON(input.continuationContext, 'Continuation context')}\n\nWhen this context is present, preserve the completed item IDs. Only the checkpoint owner may be amended to declare the observed paths. Do not rewrite completed work or claim unfinished work already ran. The unfinished suffix is context for correction and may currently fail validation against the audited tree; the candidate you return must correct it so the entire remaining suffix is valid against that tree.\n` + : '', }; const conditional = template.replace(/\{\{#if previous_plan\}\}([\s\S]*?)\{\{\/if\}\}/gu, (_, block: string) => previous ? block : ''); // One pass over trusted template only: inserted data is never interpreted again. @@ -130,18 +144,20 @@ function prepare(input: AuthorInput, mode: AuthorRequest['mode']): PreparedAutho // importPlan replaces a revision for user imports; provider output must match it first. const data = parseV1(source, 'json'); if ((data as Plan | null)?.revision !== request.revision) throw new Error('Response revision mismatch.'); - const result = importPlan(source, 'json', context, request.revision); - return { value: result.plan, warnings: result.warnings }; + const result = importPlan(source, 'json', context, request.revision, completedItems); + return { value: result.plan, warnings: result.warnings, candidatePlans: [result.plan] }; } const reply = parseV1(source, 'json'); assertEditReply(reply); if (reply.base_revision !== request.revision) throw new Error('Response revision mismatch.'); const warnings: Diagnostic[] = []; + const candidatePlans: Plan[] = []; // Validate every independent card against the captured plan before exposing any card. for (let index = 0; index < reply.edits.length; index++) { const next = applySuggestion(previous!, reply, index, context, { identity: context.identity, - schemaVersion: previous!.schema_version, baseRevision: request.revision, issue: context.issue }); - warnings.push(...validatePlan(next, context).warnings); + schemaVersion: previous!.schema_version, baseRevision: request.revision, issue: context.issue }, completedItems); + candidatePlans.push(next); + warnings.push(...(completedItems === undefined ? validatePlan(next, context) : validateContinuationPlan(next, context, completedItems, true)).warnings); } - return { value: reply, warnings }; + return { value: reply, warnings, candidatePlans }; } }); } diff --git a/core/planning-suggestions.ts b/core/planning-suggestions.ts index 3510c11e..f2425733 100644 --- a/core/planning-suggestions.ts +++ b/core/planning-suggestions.ts @@ -1,13 +1,13 @@ import { identityKey, type PlanIdentity } from './identity.ts'; import { prepareDraft, prepareSuggestions, type AuthorInput, type AuthorProvider, type PreparedAuthor } from './planning-author.ts'; -import type { Diagnostic, EditReply, Plan } from './plan.ts'; +import type { ContinuationBinding, Diagnostic, EditReply, Plan } from './plan.ts'; /** Implemented by the existing Store. No SQL or second persistence writer in core. */ export interface SuggestionStore { getPlan(identity: PlanIdentity): Plan; getSnapshot(identity: PlanIdentity): { id: string; base: string; head: string }; - beginSuggestions(identity: PlanIdentity, expected: { revision: number; snapshotId: string }, mode: PlanningMode): string; - completeSuggestions(identity: PlanIdentity, id: string, reply: unknown): void; + beginSuggestions(identity: PlanIdentity, expected: { revision: number; snapshotId: string }, mode: PlanningMode, continuation?: ContinuationBinding | null): string; + completeSuggestions(identity: PlanIdentity, id: string, reply: unknown, candidatePlans?: Plan[]): boolean | void; settleSuggestion(identity: PlanIdentity, id: string, expected: { revision: number; snapshotId: string }, outcome: { state: 'failed' | 'cancelled' | 'invalidated'; reason: string }): boolean; getSuggestions(identity: PlanIdentity, id: string): { state: string; revision: number; snapshotId: string | null; reply: EditReply | null; reason: string | null }; } @@ -16,7 +16,7 @@ export interface SuggestionStore { * in the input is always the current revision the request is bound to. */ export type PlanningMode = 'suggest' | 'draft'; -export type SuggestionInput = Omit & { snapshotId: string }; +export type SuggestionInput = Omit & { snapshotId: string; continuationBinding?: ContinuationBinding | null }; export type SuggestionOutcome = | { state: 'completed'; id: string; warnings: Diagnostic[] } | { state: 'failed' | 'cancelled' | 'stale'; id: string; reason: string }; @@ -57,6 +57,11 @@ export class SuggestionCoordinator { if (this.#closing) throw new Error('Planning coordinator is closing.'); const identity = { ...input.context.identity }, key = identityKey(identity); if (this.#active.has(key)) throw new Error('A planning invocation is still active for this plan.'); + const completed = input.completedItems ?? []; + if ((input.continuationBinding === null || input.continuationBinding === undefined) !== (completed.length === 0) || + (input.continuationBinding && (input.continuationBinding.completedItems.length !== completed.length || + input.continuationBinding.completedItems.some((item, index) => item !== completed[index])))) + throw new Error('Planning continuation binding does not match its completed prefix.'); const snapshotId = input.snapshotId; const previousPlan = this.#store.getPlan(identity), snapshot = this.#store.getSnapshot(identity); if (previousPlan.revision !== input.revision) throw new Error('Stale plan revision.'); @@ -67,7 +72,7 @@ export class SuggestionCoordinator { : prepareSuggestions({ ...input, requestId: 'pending', previousPlan }); const expected = Object.freeze({ revision: input.revision, snapshotId }); const noun = mode === 'draft' ? 'Draft' : 'Suggestion'; - const id = this.#store.beginSuggestions(identity, expected, mode); + const id = this.#store.beginSuggestions(identity, expected, mode, input.continuationBinding ?? null); const request = Object.freeze({ ...prepared.request, requestId: id }); const controller = new AbortController(); let stopped: Omit | undefined; @@ -119,8 +124,9 @@ export class SuggestionCoordinator { outcome = { id, state: current.state === 'cancelled' ? 'cancelled' : 'stale', reason: `${noun} request is ${current.state}.` }; } else { const validated = prepared.validate(source); - this.#store.completeSuggestions(identity, id, validated.value); - outcome = { id, state: 'completed', warnings: validated.warnings }; + const saved = this.#store.completeSuggestions(identity, id, validated.value, validated.candidatePlans); + outcome = saved === false ? { id, state: 'stale', reason: 'Checkpoint continuation changed during authoring.' } + : { id, state: 'completed', warnings: validated.warnings }; } } } diff --git a/docs/architecture.md b/docs/architecture.md index 930482e1..10f156cf 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -43,7 +43,7 @@ Writing standard: plain language, ISO 24495-1:2023 - **In use:** plan and linking library, SQLite store, review screen, Ask (questions to a Claude agent), guarded merge with merge-queue support, ranked Issues screen, agent isolation boundary, and the single-runner lock. - **In use for a non-demo review with a `github` block:** planning. `/api/plan/suggestions` and `/api/plan/drafts` ask Claude, through lane D, for suggestion cards or a whole next plan revision (#117, #124). Nothing becomes a revision until you apply it. - **In use with an opt-in `runner` block in `review.json`:** the whole loop after planning. `start` and `resume` on `/api/runner` run a task's plan item by item with a Claude agent (#91). They require current plan-item approvals, bind both the task state and review version, recheck approvals before each later item, and refuse when a completed prefix no longer ends at the task's current head (#107). When a run ends, codeboost publishes the task's pull request, a draft if the task needs a person (#103). Approve & merge merges that pull request (#121). Cancelling a task closes its pull requests (#111). These are API actions; the screen has no buttons for them yet. - - **Not built:** the "trust this issue" action and its runner guard (#108), screens for planning and the runner actions, rebasing, running `cmd:` checks, review rounds, continuing after a scope pause (#88, in review as PR #134), and the Planning, Queue and Learning screens. + - **Not built:** the "trust this issue" action and its runner guard (#108), screens for planning and the runner actions, rebasing, running `cmd:` checks, review rounds, and the Planning, Queue and Learning screens. ## Terms used @@ -276,7 +276,7 @@ sequenceDiagram ### Flow 4: carry out a plan and open the pull request (in use with the `runner` block) -With the opt-in `runner` block, production runs the execution part of this flow: `POST /api/runner` with `start` (a task in review or queued) or `resume` (a task that stopped between items, or that recovery left to requeue) runs the plan from the first item the current revision has not completed (#91). Both refuse a closed task, a pending cancel, active work, a scope pause, and a plan revised after earlier runner commits; the last two wait for #88. When the run ends, `TaskPublishing` publishes by task status: a ready pull request when every item completed, a draft listing the problems when the task needs a person (#103). It also publishes on the `publish` action and once at start-up, and records the last outcome. The published head includes any commits that were on your branch before the runner's first commit; review shows them as Unplanned and the merge gate blocks until each is assigned or accepted (#113). +With the opt-in `runner` block, production runs the execution part of this flow: `POST /api/runner` with `start` (a task in review or queued) or `resume` runs the plan from its first unfinished item (#91). A scope pause requires a revised plan that declares each changed path and passes validation against the runner commit's actual tree; `approve-continuation` records a person’s approval against the current revision and snapshot. Resume then reconciles every completed item after that checkpoint and starts only the remaining suffix. A later plan revision needs another continuation approval before more work starts. Both actions refuse a closed task, a pending cancel, active work, an unapproved continuation, or unrelated commits that cannot be reconciled. When the run ends, `TaskPublishing` publishes by task status: a ready pull request when every item completed, a draft listing the problems when the task needs a person (#103). It also publishes on the `publish` action and once at start-up, and records the last outcome. The published head includes any commits that were on your branch before the runner's first commit; review shows them as Unplanned and the merge gate blocks until each is assigned or accepted (#113). ```mermaid sequenceDiagram @@ -356,11 +356,11 @@ The diagram shows the designed transitions. With the `runner` block, running, ne ### Stored data -All state is in one SQLite file, opened with WAL and full synchronization. Each write takes an immediate write lock. The schema version is `PRAGMA user_version`, and migrations run in explicit steps (version 13 today); an unknown version fails. +All state is in one SQLite file, opened with WAL and full synchronization. Each write takes an immediate write lock. The schema version is `PRAGMA user_version`, and migrations run in explicit steps (version 15 today); an unknown version fails. | Group | Tables | Notes | |---|---|---| -| Plans | `plans`, `revisions`, `requests` | SQLite allocates revision numbers. Old revisions are never changed. Suggestion and draft requests (`mode`, #124) are bound to a revision and snapshot. | +| Plans | `plans`, `revisions`, `requests` | SQLite allocates revision numbers. Old revisions are never changed. Suggestion and draft requests (`mode`, #124) are bound to a revision and snapshot; continuation requests also retain their checkpoint, audited head and completed prefix. | | Code history | `snapshots`, `ledger`, `rewrites` | Ledger entries are immutable. Rebase mappings record which old commit became which new one; foreign stays foreign. | | Review | `approvals`, `choices`, `review_notes`, `checkpoints`, `continuations` | Stored approvals are claims about a past snapshot. Freshness is recomputed every time. | | Runner | `tasks`, `attempts`, `user_actions`, `feedback_events`, `merge_attempts`, `app_settings` | User actions carry idempotency keys. Feedback events are append-only and feed the future learning feature. An attempt carries its safety finding and diagnostic reference. `app_settings` holds the runner, Ask and planning owner tokens. | diff --git a/docs/implementation/runner-lifecycle.md b/docs/implementation/runner-lifecycle.md index 2289a2ee..14a8b30b 100644 --- a/docs/implementation/runner-lifecycle.md +++ b/docs/implementation/runner-lifecycle.md @@ -330,10 +330,12 @@ This runs before the coordinator opens. - Both require an approval for every item at the current plan revision. Before an item completes, those approvals must belong to the current snapshot. After a completed item, approvals survive only from the snapshots that the completed execution prefix actually used (or the current snapshot); the completed-prefix head guard below then distinguishes that expected runner movement from an unrelated moved-and-returned head. Each attribution choice and approval is stamped with the review version at which it was saved, and approvals must be later than the latest applicable choice. The review UI marks any approval this execution gate rejects as stale so the reviewer can replace it. Schema v12 stamps legacy unversioned choices at a finite boundary, clears legacy unversioned approvals so the UI offers reapproval, and advances the review version once so fresh approvals can follow. - `start` runs a task that is `in review` or `queued`, has no execute attempt at its current plan revision, and has no pending requeue (that is `resume`'s). An `in review` task moves to `queued` in the same action. - `resume` continues a task that is `running` or `queued` and has started running (it has an execute attempt at any plan revision, or recovery left it to requeue). It starts at the first item with no completed execute attempt at the current revision, whether the last item completed, failed or was stopped, and claims `requeue_pending` in the admitting transaction. Start never consumes a recovery claim, including when safety or scope work is owed. A completed prefix must still end at the task's current head; a moved head refuses instead of silently skipping earlier items. When owed work settles without an admission, that same transaction claims and clears `requeue_pending`. This is also how a task stopped between items continues. -- Both refuse a closed task, a pending cancel, a runner that cannot run execute attempts, an active attempt, an earlier run still finishing, an active merge, a scope pause (#88), and a plan revised after an earlier revision's items made runner commits (#88: those commits must be reconciled with the new revision first; earlier attempts that committed nothing do not block). During shutdown both answer 503 before any of these, and record nothing. Before the pending-cancel, active-merge and #88 refusals, a safety finding owed from an earlier run is persisted and acted on, and nothing is admitted (`outcome: "settled"`); this prevents an in-memory finding from being lost at restart. An existing human gate is preserved: schema v13 persists the evidence and an explicit owed marker, restart restores the pending escalation, and it remains owed until the person resolves that gate. An owed scope pause is offered and acted on only when the task status, pending-cancel state and merge state allow `pauseForAmendment` to commit it; otherwise the ordinary blocker is shown. Only then is the first item admitted: admission refuses a spent task budget, which also moves an idle `running` or `queued` task to `needs human` (a task still `in review` stays there: its move to `queued` rolls back with the refused action). +- Both refuse a closed task, a pending cancel, a runner that cannot run execute attempts, an active attempt, an earlier run still finishing, and an active merge. A scope pause can resume only after an amended plan explicitly declares its out-of-scope paths, its suffix validates at the checkpoint tree, and a person approves that revision and snapshot. Commits from another revision without a scope checkpoint remain blocked because their prefix cannot yet be reconciled safely; earlier attempts that committed nothing do not block. During shutdown both answer 503 before any of these, and record nothing. Before the pending-cancel, active-merge and unreconciled-commit refusals, a safety finding owed from an earlier run is persisted and acted on, and nothing is admitted (`outcome: "settled"`); this prevents an in-memory finding from being lost at restart. An existing human gate is preserved: schema v13 persists the evidence and an explicit owed marker, restart restores the pending escalation, and it remains owed until the person resolves that gate. An owed scope pause is offered and acted on only when the task status, pending-cancel state and merge state allow `pauseForAmendment` to commit it; otherwise the ordinary blocker is shown. Only then is the first item admitted: admission refuses a spent task budget, which also moves an idle `running` or `queued` task to `needs human` (a task still `in review` stays there: its move to `queued` rolls back with the refused action). - The action owns the review version for the whole multi-item run. Before each later item is admitted, `ItemExecutor` requires that version to be unchanged and rechecks the current-revision approvals. A choice, approval edit or note saved while an earlier item runs therefore stops the run before the next item; the person must review and resume from fresh state. - `GET /api/runner` adds `startable` and `resumable`, computed by the same checks, so the view never offers an action that would be refused. The view also withholds them while the task budget has run out (the action may still act on owed work, or else lets admission refuse it with its move to `needs human`) and while the runner is shutting down or needs a restart. Retrying an execute attempt is refused: a plan item runs again only through `resume`. +**Scope continuation (#88).** A checkpoint stores the leaf entries from the immutable runner commit that recorded the scope finding. The person amends the owning item so it declares every out-of-scope path, then posts `approve-continuation` with the current state and review versions and an action ID. In the same transaction, the Store proves the completed prefix still leads to the recorded task head, validates the remaining plan against that head's tree, stores approval for that revision and snapshot, and moves `needs amendment` to `queued`. `resume` requires that approval and begins with the first unfinished item after the checkpoint prefix; it never reruns completed work. The approval remains valid as that same revision completes more items only while their stored attempt contexts connect its snapshot to the current head. A further plan revision needs new approval against the latest audited head. `GET /api/runner` exposes checkpoint identity, scope paths, completed items, next item and approval state; it does not rebuild Git history while polling. Databases migrate to schema v15: v14 binds continuation approvals to snapshots, and v15 binds planning requests to their captured checkpoint, audited head, and completed prefix. Unbound legacy continuation approvals cannot admit execution. + **User actions.** Cancel, retry and "run again" requests send `attemptId`, `expectedStateVersion` and an `actionId` idempotency key (see "Feedback-event contract"). A replayed `actionId` returns the saved outcome. The server takes the plan identity from its trusted configuration, never from the request, and every `Store` call is scoped by that identity. A mismatch returns HTTP 409 with the current state. The UI then shows that state and keeps any draft. **UI rules** (from AGENTS.md "Async review UI"): diff --git a/docs/plan-format.md b/docs/plan-format.md index bb849e9f..36d6ed32 100644 --- a/docs/plan-format.md +++ b/docs/plan-format.md @@ -136,7 +136,7 @@ The schema checks the shape. codeboost then checks the meaning. A **failure** bl **Git metadata trust.** Store Git metadata on a separate runner-owned filesystem from the writable worktree and scratch, expose it only through the read-only `/work/.git` mount, and expose no writable aliases to its inodes. The worktree backing directory contains only an empty mountpoint, never another copy of the metadata. Refuse startup unless filesystem separation and mount protections are enforced; a nested read-only bind of metadata on the writable task filesystem is insufficient. Test hard-linking `.git/config` and an object into `/work` and scratch, writing via aliases, and replacing the mountpoint; all must fail with original metadata unchanged. Mount the task's `.git` read-only for the agent, including execution/fix phases; only the runner may create commits or update refs after the invocation ends. Before any post-run Git command, inspect metadata with ordinary filesystem reads against the runner-owned baseline and reject unexpected changes. Runner Git uses a sanitized immutable configuration and an explicit environment: no inherited Git variables, user/system config, hooks, fsmonitor, external diff/textconv, clean/smudge filters, SSH commands, or credential helpers supplied by the task. Network/push endpoints and credentials come only from trusted runner settings. Build that config outside the writable task mount and do not read agent-provided config before this audit. Tests must plant each command-helper setting and prove that status/diff/add/push inspection never executes its marker. -**After each run, codeboost checks the result too.** A plan check alone cannot stop an agent from creating a new symlink and writing through it. So after each invocation, before committing, codeboost: records otherwise-safe regular-file changes outside the declared list as out of scope, commits them with the owning item, and exposes them for review; these are scope findings rather than safety violations. They deliberately pause the task in "needs amendment" before any later item or check invocation. After a person approves a revised plan and continuation, reconcile the executed prefix with the audited current head and validate remaining operations from that actual checkpoint; never silently authorize the extra file or erase its original scope finding. Before that scope policy, apply the declared-link target snapshot audit above and reject target mutations even when they would otherwise look like regular-file scope changes. Separately, the safety audit rejects new symlinks or regular-file-to-symlink conversions (a renamed pre-existing symlink is tracked by its base-tree lineage); permits changed pre-existing links only at declared paths and rejects targets that leave the repo, enter `.git`, or traverse another symlink; and rejects any agent change to `.git` itself (including config, hooks, refs, and objects). Any safety violation stops tests and commits and moves the task to needs human; preserve the rejected output separately for diagnosis. Only after this audit may the runner create its own objects and commit. Hooks are disabled (`core.hooksPath=/dev/null`) in addition to the metadata/config isolation above; disabling hooks alone is not sufficient. +**After each run, codeboost checks the result too.** A plan check alone cannot stop an agent from creating a new symlink and writing through it. So after each invocation, before committing, codeboost: records otherwise-safe regular-file changes outside the declared list as out of scope, commits them with the owning item, and exposes them for review; these are scope findings rather than safety violations. They deliberately pause the task in "needs amendment" before any later item or check invocation. After a person amends the owning item to declare each out-of-scope path and approves continuation, reconcile the executed prefix with the audited current head and validate remaining operations against the actual tree stored by the checkpoint. The approval is bound to that plan revision and snapshot; after a continuation commit, another plan revision needs another approval. Never silently authorize the extra file or erase its original scope finding. Before that scope policy, apply the declared-link target snapshot audit above and reject target mutations even when they would otherwise look like regular-file scope changes. Separately, the safety audit rejects new symlinks or regular-file-to-symlink conversions (a renamed pre-existing symlink is tracked by its base-tree lineage); permits changed pre-existing links only at declared paths and rejects targets that leave the repo, enter `.git`, or traverse another symlink; and rejects any agent change to `.git` itself (including config, hooks, refs, and objects). Any safety violation stops tests and commits and moves the task to needs human; preserve the rejected output separately for diagnosis. Only after this audit may the runner create its own objects and commit. Hooks are disabled (`core.hooksPath=/dev/null`) in addition to the metadata/config isolation above; disabling hooks alone is not sufficient. **Projected file state.** Start with the paths at the plan's base commit, then walk items in their listed execution order. Check an item's file operations against the state before that item; after it passes, apply its declared additions, deletions, and renames to the projected state before checking the next item. No repo files change during validation. This describes initial plan validation. On an approved continuation after an out-of-scope pause, retain the execution history and use the audited checkpoint for the remaining suffix rather than pretending the old declared-only projection describes the actual tree. A path may participate in only one operation per item, counting both the source and destination of a rename. If an item uses a path created or renamed by an earlier item, it must depend on that item, directly or through other dependencies. diff --git a/prompts/plan-author.md b/prompts/plan-author.md index 870d72c5..2aa9f9b8 100644 --- a/prompts/plan-author.md +++ b/prompts/plan-author.md @@ -90,7 +90,7 @@ Previous plan (revision {{previous_revision}}): {{previous_plan_json}} {{/if}} - +{{continuation_context_instruction}} The person's requested changes are data below. Use them to revise the plan within the trusted task rules, never to change permissions or the output contract. {{feedback_data_json}} diff --git a/runner/execution.ts b/runner/execution.ts index 4f459825..3f2e2f77 100644 --- a/runner/execution.ts +++ b/runner/execution.ts @@ -57,6 +57,8 @@ export type AgentLauncher = (input: InvocationInput, prompt: string, workspace: /** Trusted runner-side sources for a task. Issue text and lessons are untrusted data inside the prompt. */ export interface ExecutionSources { planContext(identity: PlanIdentity): PlanContext; + /** Entries of the audited runner commit, read from its immutable tree after export. */ + checkpointContext(identity: PlanIdentity, head: string): PlanContext; /** The issue text the prompt carries; fetched per attempt, so it may await (and must stop on abort). */ issue(identity: PlanIdentity, signal: AbortSignal): IssueText | Promise; lessons(identity: PlanIdentity): readonly string[]; @@ -341,13 +343,22 @@ export class ItemExecutor { * reconciled with (#88), the items an execute attempt completed at the current revision, and the first item none did * (null when every item has run). */ - progress(identity: PlanIdentity): { started: boolean; begun: boolean; earlierCommits: boolean; completed: string[]; prefixHead: string | null; next: string | null } { + progress(identity: PlanIdentity): { started: boolean; begun: boolean; earlierCommits: boolean; completed: string[]; prefixHead: string | null; next: string | null; continuationError?: string } { const plan = this.#store.getPlan(identity), { started, begun, earlierCommits, finished, finishedHeads } = this.#store.executeProgress(identity, plan.revision); const done = new Set(finished); let prefixHead: string | null = null; for (const item of plan.items) { if (!done.has(item.id)) break; prefixHead = finishedHeads[item.id] ?? null; } - return { started, begun, earlierCommits, completed: plan.items.filter(item => done.has(item.id)).map(item => item.id), + const ordinary = { started, begun, earlierCommits, completed: plan.items.filter(item => done.has(item.id)).map(item => item.id), prefixHead, next: plan.items.find(item => !done.has(item.id))?.id ?? null }; + const checkpoint = this.#store.latestCheckpoint(identity); + if (!checkpoint) return ordinary; + try { + const continuation = this.#store.continuationProgress(identity); + return continuation ? { ...ordinary, completed: continuation.completed, prefixHead: continuation.head, next: continuation.next } : ordinary; + } catch (error) { + // Do not let a malformed continuation look complete to publishing; runnerView can still report the reason. + return { ...ordinary, continuationError: error instanceof Error ? error.message : String(error) }; + } } /** Whether a run of this task is still in progress here (its last write may still be to come). */ busy(identity: PlanIdentity): boolean { return this.#inFlight.has(identityKey(identity)); } @@ -412,7 +423,7 @@ export class ItemExecutor { */ #begin(identity: PlanIdentity, options: { fromItem?: string; claimRequeue?: boolean; expectedReviewVersion?: number }, strict: boolean): { outcome: ExecutionOutcome } | Run { const plan = this.#store.getPlan(identity); - const start = options.fromItem ? plan.items.findIndex(item => item.id === options.fromItem) : 0; + let start = options.fromItem ? plan.items.findIndex(item => item.id === options.fromItem) : 0; if (start < 0) throw new Error('Unknown plan item.'); const run: Run = { plan, index: start, attempt: undefined!, done: [], unchanged: [], stopped: undefined!, ...(options.expectedReviewVersion === undefined ? {} : { reviewVersion: options.expectedReviewVersion }) }; @@ -448,11 +459,27 @@ export class ItemExecutor { if (options.claimRequeue) this.#store.claimRequeue(identity, this.#store.getTask(identity).stateVersion); return this.#settledOrRefused(this.#pause(identity, owed.row, owed.result, run.stopped, [], true), strict); } - // Continuing after a scope pause (plan-format.md: reconcile the executed prefix with the audited head, validate the - // remaining items from that checkpoint) is not built yet (#88), so a paused task runs no further items: fail closed. - const checkpoint = this.#store.latestCheckpoint(identity); - if (checkpoint) - return notStarted(`${checkpoint.item} changed files outside its plan item. Continuing after a scope pause is not supported yet (#88), so this task runs no further items.`); + const continuation = this.#store.latestCheckpoint(identity) ? this.#store.continuationProgress(identity) : null; + const next = continuation ? continuation.next : this.progress(identity).next; + if (!options.fromItem && next) start = plan.items.findIndex(item => item.id === next); + run.index = start; + if (continuation) { + if (!continuation.next) return notStarted('Every item of the amended plan has run.'); + if (plan.revision <= continuation.checkpoint.revision || !this.#store.continuationApproved(identity, continuation)) + return notStarted('The amended plan needs an approved continuation at the current audited head.'); + if (plan.items[start]?.id !== continuation.next) + return notStarted(`Continuation must start with ${continuation.next}, the next item after the audited prefix.`); + const completed = new Set(continuation.completed); + if (plan.items[start]!.depends_on.some(id => !completed.has(id))) + return notStarted(`Dependencies of ${continuation.next} are not complete at the checkpoint.`); + } else { + const progress = this.progress(identity); + if (!progress.next) return notStarted('Every item of this plan has run.'); + if (!progress.begun && progress.earlierCommits) + return notStarted('The revised plan has earlier runner commits that need reconciliation.'); + if (plan.items[start]?.id !== progress.next) + return notStarted(`Execution must start with ${progress.next}, the first unfinished plan item.`); + } const admitted = this.#admit(identity, run, null, options.claimRequeue === true, strict); if (!('id' in admitted)) return { outcome: admitted }; run.attempt = admitted; @@ -618,7 +645,7 @@ export class ItemExecutor { let checkpointId: string; try { checkpointId = (owed ? direct : this.#write)(() => this.#store.pauseForAmendment(identity, { revision: row.context.planRevision, snapshotId }, { - item, baseEntries: this.#sources.planContext(identity).baseEntries, + item, baseEntries: this.#sources.checkpointContext(identity, result.head).baseEntries, completedItems: items.slice(0, items.findIndex(entry => entry.id === item) + 1).map(entry => entry.id), outOfScopePaths: result.outOfScope, }, { owed })).id; } catch (error) { diff --git a/runner/production.ts b/runner/production.ts index 15d2bf6e..287d3d62 100644 --- a/runner/production.ts +++ b/runner/production.ts @@ -213,6 +213,7 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd }; const sources: ExecutionSources = { planContext: requested => { only(requested); return service.planContext(); }, + checkpointContext: (requested, head) => { only(requested); return service.planContextAt(head); }, issue: (requested, signal) => { only(requested); return issueText(service.store.getPlan(identity).issue, signal); }, // Learning (L1–L4) is not built: no lesson is approved yet. lessons: () => [], diff --git a/runner/publishing.ts b/runner/publishing.ts index fc0e5f5e..2bb3907a 100644 --- a/runner/publishing.ts +++ b/runner/publishing.ts @@ -71,7 +71,7 @@ export class TaskPublishing { * What a job would do now. Throws the refusal otherwise, with what a person can act on. Writes nothing, so the view * asks it too and never offers what an action would refuse. */ - mode(identity: PlanIdentity, progress?: { next: string | null }): PullRequestJob { + mode(identity: PlanIdentity, progress?: { next: string | null; continuationError?: string }): PullRequestJob { if (this.#closing || this.#runner.closing) throw new ShuttingDownError(); if (this.busy(identity)) throw new GuardRefusal(`A pull request is already being ${this.#running.get(identityKey(identity))!.job.kind === 'close' ? 'closed' : 'published'} for this task.`); const task = this.#store.getTask(identity); @@ -83,7 +83,9 @@ export class TaskPublishing { // Publishing reads the task head, so it waits for the run (and the attempt) to end: assertPublishable refuses an active attempt. if (this.#runner.isActive(identity) || this.#executor.busy(identity)) throw new GuardRefusal('A run of this task is still in progress; publish once it has ended.'); if (task.status === 'running') { - const { next } = progress ?? this.#executor.progress(identity); + const current = progress ?? this.#executor.progress(identity); + if (current.continuationError) throw new GuardRefusal(`The task continuation cannot be reconciled for publishing: ${current.continuationError}`); + const { next } = current; if (next !== null) throw new GuardRefusal(`${next} has not run yet; publish once every plan item has run.`); this.#assertNothingOwed(identity); this.#store.assertPublishableNow(identity, false); diff --git a/runner/review.ts b/runner/review.ts index 907fc572..7e61a67a 100644 --- a/runner/review.ts +++ b/runner/review.ts @@ -11,6 +11,7 @@ import { linkHistory } from '../core/linking.ts'; import { applyChoices, approvalStates, approveItem, choiceKeys, fingerprint, reviewedSegment, stable } from '../core/approvals.ts'; import type { PlanItem } from '../core/plan.ts'; import type { GhMergeConfig } from '../github/merge.ts'; +import { GuardRefusal } from './lifecycle.ts'; export interface ReviewConfig { database: string; repository: string; /** The runner-owned repository (#87) holding the commits codeboost makes; required once the task has any. */ @@ -26,9 +27,16 @@ export class ReviewService { * base reuses it instead of running Git again on the server's thread (each runner item asks for it, #91). */ #baseEntries: { base: string; entries: readonly BaseEntry[] } | null = null; + #pathKey: (path: string) => string; constructor(config: ReviewConfig) { if (typeof config.pathIdentity?.caseSensitive !== 'boolean' || !['none', 'NFC'].includes(config.pathIdentity.unicodeNormalization)) throw new Error('Known checkout path identity is required.'); - this.config = config; this.store = new Store(config.database); + this.config = config; + this.#pathKey = path => { + if (!config.pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.'); + const normalized = config.pathIdentity.unicodeNormalization === 'NFC' ? path.normalize('NFC') : path; + return config.pathIdentity.caseSensitive ? normalized : normalized.toLowerCase(); + }; + this.store = new Store(config.database, this.#pathKey); } close() { this.store.close(); } /** @@ -43,7 +51,7 @@ export class ReviewService { return { path: this.config.runnerRepository, runnerOwned: true }; } load() { - const { identity, pathIdentity } = this.config; + const { identity } = this.config; const reviewVersion = this.store.reviewVersion(identity); const plan = this.store.getPlan(identity); let snapshot = this.store.getSnapshot(identity); @@ -53,11 +61,7 @@ export class ReviewService { // recorded head is read as it is: observing the user's HEAD would record its older commit and roll the task back. const history = readHistory(reviewed.path, snapshot.base, reviewed.runnerOwned ? snapshot.head : 'HEAD'); if (history.head !== snapshot.head) snapshot = this.store.recordHistory(identity, { revision: plan.revision, snapshotId: snapshot.id, reviewVersion }, history.base, history.head, []); - const pathKey = (path: string) => { - if (!pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.'); - const normalized = pathIdentity.unicodeNormalization === 'NFC' ? path.normalize('NFC') : path; - return pathIdentity.caseSensitive ? normalized : normalized.toLowerCase(); - }; + const pathKey = this.#pathKey; const raw = linkHistory(plan, history, this.store.ownership(identity, plan.revision), pathKey); const saved = this.store.getReview(identity), keys = choiceKeys(raw, identity); const deltas = new Map(history.final.map(file => [JSON.stringify([file.newPath ?? file.oldPath, file.oldPath]), file])); @@ -86,7 +90,14 @@ export class ReviewService { } // The execution gate also orders approvals after attribution choices and binds them to the current or executed-prefix // snapshots. Reflect that same gate in the review UI so an approval execution would refuse is available to reapprove. - const executionUnapproved = new Set(this.store.unapprovedExecutionItems(identity, plan.revision)); + let executionUnapproved: Set; + try { executionUnapproved = new Set(this.store.unapprovedExecutionItems(identity, plan.revision)); } + catch (error) { + if (!(error instanceof GuardRefusal)) throw error; + // Keep the review available when checkpoint reconciliation refuses. Surface every saved approval as stale; + // execution remains fail-closed in Store.unapprovedExecutionItems and resume. + executionUnapproved = new Set(plan.items.map(item => item.id)); + } for (const item of plan.items) if (states[item.id] === 'approved' && executionUnapproved.has(item.id)) states[item.id] = 'stale'; for (const item of plan.items) { if (states[item.id] === 'approved' && (segments.some(segment => segment.row === 'Ambiguous' && segment.owners.includes(item.id)) || item.depends_on.some(id => states[id] === 'stale'))) states[item.id] = 'stale'; @@ -144,30 +155,51 @@ export class ReviewService { const token = createHash('sha256').update(JSON.stringify({ expected, saved, plan, segments })).digest('hex'); return { repository: basename(this.config.repository), demo: this.config.demo ?? false, plan, snapshot, expected, token, items, segments, notes, approved: items.filter(item => item.state === 'approved').length }; } - /** The trusted plan context for import and Apply: base entries from the snapshot's base tree, and the configured path identity. */ - planContext(): PlanContext { - const { identity, repository, pathIdentity } = this.config, plan = this.store.getPlan(identity), snapshot = this.store.getSnapshot(identity); - const pathKey = (path: string) => { - if (!pathIdentity.caseSensitive && /[^\x20-\x7e]/.test(path)) throw new Error('Non-ASCII case-insensitive paths require a filesystem-specific identity adapter.'); - const normalized = pathIdentity.unicodeNormalization === 'NFC' ? path.normalize('NFC') : path; - return pathIdentity.caseSensitive ? normalized : normalized.toLowerCase(); - }; + /** The trusted plan context for import and Apply, or for a continuation at an audited runner head. */ + planContextAt(head?: string): PlanContext { + const { identity, repository } = this.config, plan = this.store.getPlan(identity), snapshot = this.store.getSnapshot(identity); + const pathKey = this.#pathKey; // Hardened like every repository Git call (#82, #83): no replace objects, hooks, network or inherited environment. - const git = (args: string[], maxBuffer?: number) => execFileSync('git', [...HARDENED_GIT_OPTIONS, ...args], { cwd: repository, env: hardenedGitEnvironment(), encoding: 'utf8', maxBuffer, stdio: ['ignore', 'pipe', 'pipe'] }); - if (this.#baseEntries?.base !== snapshot.base) { - const listing = git(['ls-tree', '-rz', snapshot.base], 64 * 1024 * 1024); + const treeHead = head ?? snapshot.base; + if (head && !/^(?:[a-f0-9]{40}|[a-f0-9]{64})$/.test(head)) throw new Error('Expected a full checkpoint head.'); + const git = (args: string[], maxBuffer?: number) => execFileSync('git', [...HARDENED_GIT_OPTIONS, ...args], { + cwd: head ? this.reviewRepository().path : repository, env: hardenedGitEnvironment(), encoding: 'utf8', maxBuffer, + stdio: ['ignore', 'pipe', 'pipe'], + }); + if (this.#baseEntries?.base !== treeHead) { + const listing = git(['ls-tree', '-rz', treeHead], 64 * 1024 * 1024); const entries: BaseEntry[] = listing.split('\0').filter(Boolean).map(record => { const split = record.indexOf('\t'), [mode, , oid] = record.slice(0, split).split(' '), path = record.slice(split + 1); if (mode === '160000') return { path, kind: 'gitlink' }; - if (mode === '120000') return { path, kind: 'symlink', target: git(['cat-file', 'blob', oid!]) }; + if (mode === '120000') return { path, kind: 'symlink', target: git(['cat-file', 'blob', oid!], 64 * 1024 * 1024) }; return { path, kind: 'file' }; }); - this.#baseEntries = { base: snapshot.base, entries }; + this.#baseEntries = { base: treeHead, entries }; } // Copies: callers own what they are given, and the cached listing stays as Git reported it. const baseEntries = this.#baseEntries.entries.map(entry => ({ ...entry })); return { identity, issue: plan.issue, baseEntries, pathKey, allowedCommands: [] }; } + planContext(): PlanContext { return this.planContextAt(); } + /** When a checkpoint exists, edits are validated against the audited task head and completed work is excluded. */ + planContextForAmendment(): PlanContext { + const checkpoint = this.store.latestCheckpoint(this.config.identity); + if (!checkpoint) return this.planContext(); + const progress = this.store.continuationBasis(this.config.identity); + if (!progress) throw new Error('The scope checkpoint could not be reconciled.'); + return this.planContextAt(progress.head); + } + planningContextForAmendment(): { context: PlanContext; completedItems: string[]; continuation: { checkpointId: string; head: string; completedItems: string[] } | null; + continuationContext: { checkpointId: string; head: string; completedItems: string[]; ownerItem: string; outOfScopePaths: string[] } | null } { + const checkpoint = this.store.latestCheckpoint(this.config.identity); + if (!checkpoint) return { context: this.planContext(), completedItems: [], continuation: null, continuationContext: null }; + const progress = this.store.continuationBasis(this.config.identity); + if (!progress) throw new Error('The scope checkpoint could not be reconciled.'); + return { context: this.planContextAt(progress.head), completedItems: progress.completed, + continuation: { checkpointId: checkpoint.id, head: progress.head, completedItems: progress.completed }, + continuationContext: { checkpointId: checkpoint.id, head: progress.head, completedItems: progress.completed, ownerItem: checkpoint.item, + outOfScopePaths: checkpoint.outOfScopePaths } }; + } /** With an actionId (inside Store.userAction), feedback-producing actions record their event in the same transaction. */ act(input: unknown, actionId?: string) { if (!input || typeof input !== 'object') throw new Error('Invalid review command.'); diff --git a/runner/store.ts b/runner/store.ts index bfe43db9..8e88a500 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -2,7 +2,8 @@ import type { DatabaseSync, SQLInputValue, SQLOutputValue } from 'node:sqlite'; import { createRequire } from 'node:module'; import { randomUUID } from 'node:crypto'; import { identityKey, type PlanIdentity } from '../core/identity.ts'; -import { importPlan, applySuggestion, assertEditReply, assertPlan, type Plan, type PlanContext, type EditReply } from '../core/plan.ts'; +import { importPlan, applySuggestion, assertEditReply, assertPlan, isRepoPath, validateContinuationPlan, type ContinuationBinding, type Plan, type PlanContext, type PlanItem, type EditReply } from '../core/plan.ts'; +import { stable } from '../core/approvals.ts'; import type { PlanningMode } from '../core/planning-suggestions.ts'; import type { Approval, SegmentChoice } from '../core/approvals.ts'; import type { InvocationContext, StopReason } from '../agents/contract.ts'; @@ -109,7 +110,10 @@ export interface Checkpoint { id: string; revision: number; snapshotId: string; item: string; /** Runner-audited actual tree, retained separately from the declared plan. */ baseEntries: PlanContext['baseEntries']; completedItems: string[]; outOfScopePaths: string[]; + /** Present on new checkpoints; absent on pre-continuation rows whose saved tree was only the original base tree. */ + treeHead?: string; } +type ExecutionResultLike = { head?: unknown; unchanged?: unknown }; const encode = (value: unknown) => JSON.stringify(value); const decode = (value: unknown): T => JSON.parse(value as string) as T; function sha(value: string): void { @@ -121,21 +125,38 @@ function sha(value: string): void { */ export class Store { #db: DatabaseSync; - constructor(path: string) { + #pathKey: (path: string) => string; + #assertValidItemPaths(item: PlanItem): void { + const paths: string[] = []; + for (const file of item.files) { + if ((file.kind === 'rename') !== (file.renamed_from !== null)) + throw new GuardRefusal(`Completed checkpoint item ${item.id} has invalid or overlapping file declarations.`); + for (const path of file.kind === 'rename' ? [file.path, file.renamed_from!] : [file.path]) { + if (!isRepoPath(path)) + throw new GuardRefusal(`Completed checkpoint item ${item.id} has invalid or overlapping file declarations.`); + const key = this.#pathKey(path); + if (paths.some(other => other === key || other.startsWith(`${key}/`) || key.startsWith(`${other}/`))) + throw new GuardRefusal(`Completed checkpoint item ${item.id} has invalid or overlapping file declarations.`); + paths.push(key); + } + } + } + constructor(path: string, pathKey: (path: string) => string = value => value) { requireSupportedNode(); + this.#pathKey = pathKey; const { DatabaseSync } = createRequire(import.meta.url)('node:sqlite') as typeof import('node:sqlite'); this.#db = new DatabaseSync(path, { timeout: 5000 }); try { this.#db.exec('PRAGMA foreign_keys=ON; PRAGMA journal_mode=WAL; PRAGMA synchronous=FULL;'); this.#transaction(() => { const version = this.#get('PRAGMA user_version')!.user_version as number; - if (![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13].includes(version)) throw new Error('Unsupported store schema version.'); - if (version === 13) return; + if (![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15].includes(version)) throw new Error('Unsupported store schema version.'); + if (version === 15) return; if (version === 0) this.#db.exec(` CREATE TABLE plans (key TEXT PRIMARY KEY, issue INTEGER NOT NULL, revision INTEGER NOT NULL, snapshot_id TEXT); CREATE TABLE revisions (key TEXT NOT NULL REFERENCES plans(key), revision INTEGER NOT NULL, data TEXT NOT NULL, PRIMARY KEY(key,revision)); CREATE TABLE snapshots (key TEXT NOT NULL REFERENCES plans(key), id TEXT NOT NULL, data TEXT NOT NULL, PRIMARY KEY(key,id)); - CREATE TABLE requests (id TEXT PRIMARY KEY, key TEXT NOT NULL REFERENCES plans(key), revision INTEGER NOT NULL, state TEXT NOT NULL, reply TEXT); + CREATE TABLE requests (id TEXT PRIMARY KEY, key TEXT NOT NULL REFERENCES plans(key), revision INTEGER NOT NULL, state TEXT NOT NULL, reply TEXT, continuation TEXT); CREATE TABLE ledger (key TEXT NOT NULL REFERENCES plans(key), sha TEXT NOT NULL, data TEXT NOT NULL, PRIMARY KEY(key,sha)); CREATE TABLE rewrites (key TEXT NOT NULL REFERENCES plans(key), snapshot_id TEXT NOT NULL, old_sha TEXT NOT NULL, new_sha TEXT NOT NULL, PRIMARY KEY(key,snapshot_id,old_sha)); CREATE TABLE approvals (key TEXT NOT NULL REFERENCES plans(key), item TEXT NOT NULL, data TEXT NOT NULL, PRIMARY KEY(key,item)); @@ -219,6 +240,18 @@ export class Store { this.#db.exec('ALTER TABLE attempts ADD COLUMN safety_owed INTEGER NOT NULL DEFAULT 0 CHECK (safety_owed IN (0,1))'); this.#db.exec('PRAGMA user_version=13'); } + // Bind a continuation decision to the reviewed snapshot. Old rows had no such binding and cannot admit work. + if (version < 14) { + if (!this.#db.prepare('PRAGMA table_info(continuations)').all().some(column => column.name === 'snapshot_id')) + this.#db.exec('ALTER TABLE continuations ADD COLUMN snapshot_id TEXT'); + this.#db.exec('PRAGMA user_version=14'); + } + // Planning output is bound to the checkpoint tree/prefix used to validate it. + if (version < 15) { + if (!this.#db.prepare('PRAGMA table_info(requests)').all().some(column => column.name === 'continuation')) + this.#db.exec('ALTER TABLE requests ADD COLUMN continuation TEXT'); + this.#db.exec('PRAGMA user_version=15'); + } }); } catch (error) { this.#db.close(); throw error; } } @@ -347,7 +380,9 @@ export class Store { return this.#transaction(() => { this.#context(key, context); if (this.#current(key).revision !== expected) throw new Error('Stale plan revision.'); - const plan = importPlan(source, format, context, expected + 1).plan; + const progress = this.continuationBasis(context.identity); + const plan = importPlan(source, format, context, expected + 1, progress?.completed).plan; + if (progress) this.continuationProgress(context.identity, plan); this.#savePlan(key, plan, expected); return plan; }); } @@ -368,28 +403,45 @@ export class Store { return decode(row.data); } /** `mode` is required, so a caller cannot record a draft as a suggestion by leaving it out (#124). */ - beginSuggestions(identity: PlanIdentity, expected: ReviewState, mode: PlanningMode): string { + beginSuggestions(identity: PlanIdentity, expected: ReviewState, mode: PlanningMode, continuation: ContinuationBinding | null = null): string { if (mode !== 'suggest' && mode !== 'draft') throw new Error('Invalid planning request mode.'); const key = identityKey(identity); return this.#transaction(() => { this.#expect(key, expected); + const basis = this.continuationBasis(identity), currentBinding = basis ? { checkpointId: basis.checkpoint.id, head: basis.head, completedItems: basis.completed } : null; + if (stable(continuation) !== stable(currentBinding)) throw new Error('Stale continuation context. Reload before requesting planning.'); const id = randomUUID(); - this.#run("INSERT INTO requests (id,key,revision,state,reply,snapshot_id,reason,mode) VALUES (?,?,?,'pending',NULL,?,NULL,?)", id, key, expected.revision, expected.snapshotId, mode); return id; + this.#run("INSERT INTO requests (id,key,revision,state,reply,snapshot_id,reason,mode,continuation) VALUES (?,?,?,'pending',NULL,?,NULL,?,?)", + id, key, expected.revision, expected.snapshotId, mode, continuation === null ? null : encode(continuation)); return id; }); } /** * Publish a request's validated reply, by the request's own mode: suggestion cards against the current revision, or * a draft plan of the next revision for the plan's issue. The full plan checks run again when a draft is applied. */ - completeSuggestions(identity: PlanIdentity, id: string, reply: unknown): void { + completeSuggestions(identity: PlanIdentity, id: string, reply: unknown, candidatePlans?: Plan[]): boolean { const key = identityKey(identity); - this.#transaction(() => { + return this.#transaction(() => { const current = this.#current(key); - const row = this.#get('SELECT mode FROM requests WHERE id=? AND key=?', id, key); + const row = this.#get('SELECT mode,continuation FROM requests WHERE id=? AND key=? AND state=\'pending\'', id, key); + if (!row) throw new Error('Planning request is stale, cancelled, or complete.'); + const binding = row.continuation === null ? null : decode(row.continuation); + const basis = this.continuationBasis(identity), currentBinding = basis ? { checkpointId: basis.checkpoint.id, head: basis.head, completedItems: basis.completed } : null; + if (stable(binding) !== stable(currentBinding)) { + this.#run("UPDATE requests SET state='invalidated',reason='Checkpoint continuation changed during authoring.' WHERE id=? AND key=? AND state='pending'", id, key); + return false; + } + if (binding !== null) { + const candidates = candidatePlans ?? []; + const expectedCount = row.mode === 'draft' ? 1 : (assertEditReply(reply), reply.edits.length); + if (candidates.length !== expectedCount) throw new Error('Validated continuation candidates are required before publishing this reply.'); + for (const candidate of candidates) this.continuationProgress(identity, candidate); + } const fits = row?.mode === 'draft' ? (assertPlan(reply), reply.revision === (current.revision as number) + 1 && reply.issue === current.issue) : (assertEditReply(reply), reply.base_revision === current.revision); if (!fits || this.#run("UPDATE requests SET state='ready',reply=?,reason=NULL WHERE id=? AND key=? AND revision=? AND snapshot_id=? AND state='pending'", encode(reply), id, key, current.revision!, current.snapshot_id!).changes !== 1) throw new Error(`${row?.mode === 'draft' ? 'Draft' : 'Suggestion'} request is stale, cancelled, or complete.`); + return true; }); } settleSuggestion(identity: PlanIdentity, id: string, expected: ReviewState, outcome: { state: 'failed' | 'cancelled' | 'invalidated'; reason: string }): boolean { @@ -547,33 +599,60 @@ export class Store { */ applyDraft(identity: PlanIdentity, id: string, context: PlanContext): Plan { const key = identityKey(identity); - return this.#transaction(() => { + let applied: Plan | null; + try { applied = this.#transaction(() => { this.#context(key, context); const request = this.#get("SELECT * FROM requests WHERE id=? AND key=? AND state='ready' AND mode='draft'", id, key); if (!request) throw new Error('Draft is unavailable.'); const current = this.#current(key); if (request.revision !== current.revision || request.snapshot_id !== current.snapshot_id) throw new Error('Draft is unavailable.'); + const progress = this.continuationBasis(identity), currentBinding = progress ? { checkpointId: progress.checkpoint.id, head: progress.head, completedItems: progress.completed } : null; + const requestBinding = request.continuation === null ? null : decode(request.continuation); + if (stable(requestBinding) !== stable(currentBinding)) { + const invalidate = () => this.#run("UPDATE requests SET state='invalidated',reason='Checkpoint continuation changed before application.' WHERE id=? AND key=? AND state='ready'", id, key); + throw new RefusalWithEffect('Draft is unavailable.', invalidate); + } const revision = current.revision as number; - const plan = importPlan(request.reply as string, 'json', context, revision + 1).plan; + const plan = importPlan(request.reply as string, 'json', context, revision + 1, progress?.completed).plan; + if (progress) this.continuationProgress(identity, plan); this.#savePlan(key, plan, revision); this.#run("UPDATE requests SET state='consumed',reason=NULL WHERE id=?", id); return plan; - }); + }); } catch (error) { + if (error instanceof RefusalWithEffect && this.#depth === 0) this.#transaction(error.effect); + throw error; + } + if (!applied) throw new Error('Draft is unavailable.'); + return applied; } applySuggestion(identity: PlanIdentity, id: string, index: number, context: PlanContext): Plan { const key = identityKey(identity); - return this.#transaction(() => { + let applied: Plan | null; + try { applied = this.#transaction(() => { this.#context(key, context); const request = this.#get("SELECT * FROM requests WHERE id=? AND key=? AND state='ready' AND mode='suggest'", id, key); if (!request) throw new Error('Suggestion is unavailable.'); const current = this.#current(key); if (request.revision !== current.revision || request.snapshot_id !== current.snapshot_id) throw new Error('Suggestion is unavailable.'); const plan = this.getPlan(identity); + const progress = this.continuationBasis(identity); + const currentBinding = progress ? { checkpointId: progress.checkpoint.id, head: progress.head, completedItems: progress.completed } : null; + const requestBinding = request.continuation === null ? null : decode(request.continuation); + if (stable(requestBinding) !== stable(currentBinding)) { + const invalidate = () => this.#run("UPDATE requests SET state='invalidated',reason='Checkpoint continuation changed before application.' WHERE id=? AND key=? AND state='ready'", id, key); + throw new RefusalWithEffect('Suggestion is unavailable.', invalidate); + } const next = applySuggestion(plan, decode(request.reply), index, context, { identity, schemaVersion: plan.schema_version, baseRevision: request.revision as number, issue: plan.issue, - }); + }, progress?.completed); + if (progress) this.continuationProgress(identity, next); this.#savePlan(key, next, request.revision as number); this.#run("UPDATE requests SET state='consumed',reason=NULL WHERE id=?", id); return next; - }); + }); } catch (error) { + if (error instanceof RefusalWithEffect && this.#depth === 0) this.#transaction(error.effect); + throw error; + } + if (!applied) throw new Error('Suggestion is unavailable.'); + return applied; } #entry(key: string, entry: LedgerEntry): void { sha(entry.sha); if (entry.sourceSha !== null) sha(entry.sourceSha); @@ -720,6 +799,22 @@ export class Store { if (!completedSnapshot) break; prefixSnapshots.add(completedSnapshot); } + // A continuation approval starts a new reviewed ancestry at its audited snapshot. The checkpoint's completed + // prefix belongs to earlier plan revisions, so those attempts cannot identify the snapshots its approvals survive. + const checkpoint = this.latestCheckpoint(identity), continuation = checkpoint ? this.continuationProgress(identity) : null; + if (continuation && this.continuationApproved(identity, continuation)) { + const approvalSnapshot = this.continuationApproval(identity, checkpoint!.id, revision); + if (approvalSnapshot) { + prefixSnapshots.add(approvalSnapshot); + const afterCheckpoint = new Set(continuation.completed.slice(checkpoint!.completedItems.length)); + for (const row of this.#db.prepare(`SELECT item,context FROM attempts WHERE plan_key=? AND kind='execute' AND state='completed' + AND json_extract(context,'$.planRevision')=? ORDER BY rowid`).all(key, revision)) { + const context = decode(row.context); + if (typeof row.item === 'string' && afterCheckpoint.has(row.item) && typeof context.snapshotId === 'string') + prefixSnapshots.add(context.snapshotId); + } + } + } const reviewedExecutionSnapshot = (value: ReviewState) => value.snapshotId === snapshotId || prefixSnapshots.has(value.snapshotId); // A later attribution choice changes the material reviewed by at least one item. Without rebuilding Git history on a // status poll, conservatively require approvals recorded after the latest such choice for this execution context. @@ -739,7 +834,7 @@ export class Store { this.#expect(key, expected); const plan = this.getPlan(identity), ids = plan.items.map(item => item.id); if (!ids.includes(evidence.item) || evidence.completedItems.at(-1) !== evidence.item || new Set(evidence.completedItems).size !== evidence.completedItems.length || evidence.completedItems.some((item, i) => item !== ids[i])) throw new Error('Checkpoint must describe the executed plan prefix.'); - const checkpoint = { ...evidence, ...expected, id: randomUUID() }; + const checkpoint = { ...evidence, ...expected, treeHead: this.getSnapshot(identity, expected.snapshotId).head, id: randomUUID() }; this.#run('INSERT INTO checkpoints VALUES (?,?,?)', key, checkpoint.id, encode(checkpoint)); return checkpoint; }); } @@ -763,7 +858,8 @@ export class Store { const pausable = status === 'running' || status === 'in review' || status === 'approved but merge blocked' || (options.owed === true && status === 'queued'); if (!pausable) throw new GuardRefusal(`The task is ${status}, so it was not paused for amendment.`); this.transitionTask(identity, this.#task(key).state_version as number, 'needs amendment'); - const checkpoint = { ...evidence, revision: ranAt.revision, snapshotId: ranAt.snapshotId, id: randomUUID() }; + const checkpoint = { ...evidence, revision: ranAt.revision, snapshotId: ranAt.snapshotId, + treeHead: this.getSnapshot(identity, ranAt.snapshotId).head, id: randomUUID() }; this.#run('INSERT INTO checkpoints VALUES (?,?,?)', key, checkpoint.id, encode(checkpoint)); return checkpoint; }); @@ -794,13 +890,145 @@ export class Store { const row = this.#get('SELECT data FROM checkpoints WHERE key=? AND id=?', identityKey(identity), id); if (!row) throw new Error('Unknown checkpoint.'); return decode(row.data); } - /** Persist a person's approval only after the runner reconciles the prefix and validates the suffix. */ - approveContinuation(identity: PlanIdentity, checkpointId: string, expected: ReviewState): void { + /** Derive audited completion from the checkpoint and attempt chain without trusting the editable current plan. */ + continuationBasis(identity: PlanIdentity): { checkpoint: Checkpoint; completed: string[]; completedDefinitions: PlanItem[]; head: string } | null { + const checkpoint = this.latestCheckpoint(identity); + if (!checkpoint) return null; + const checkpointPlan = this.getPlan(identity, checkpoint.revision); + const completed = [...checkpoint.completedItems], completedDefinitions = checkpointPlan.items.slice(0, completed.length); + if (completedDefinitions.length !== completed.length || completedDefinitions.some((item, index) => item.id !== completed[index])) + throw new GuardRefusal('The checkpoint does not match its saved plan revision.'); + let head = this.getSnapshot(identity, checkpoint.snapshotId).head; + const key = identityKey(identity); + const origin = this.#get(`SELECT rowid FROM attempts WHERE plan_key=? AND kind='execute' AND state='completed' AND item=? + AND json_extract(context,'$.planRevision')=? AND json_extract(result,'$.head')=? ORDER BY rowid DESC LIMIT 1`, + key, checkpoint.item, checkpoint.revision, head)?.rowid as number | undefined; + if (origin === undefined && this.#get('SELECT 1 AS found FROM attempts WHERE plan_key=? LIMIT 1', key)) + throw new GuardRefusal('The checkpoint has no completed execute attempt at its audited head.'); + const later = origin === undefined ? [] : this.#db.prepare(`SELECT * FROM attempts WHERE plan_key=? AND rowid>? + AND state='completed' AND kind IN (${WRITABLE_KINDS.map(() => '?').join(',')}) ORDER BY rowid LIMIT 10001`) + .all(key, origin, ...WRITABLE_KINDS); + if (later.length > 10000) throw new GuardRefusal('Too many completed attempts follow the checkpoint to reconcile safely.'); + for (const stored of later) { + const row = this.#attemptRecord(stored); + const result = row.result as ExecutionResultLike | null; + const executedPlan = this.getPlan(identity, row.context.planRevision); + if (row.kind !== 'execute' || !result || typeof result.head !== 'string' || !row.item || + this.getSnapshot(identity, row.context.snapshotId).head !== head || + executedPlan.items.slice(0, completed.length).some((item, index) => item.id !== completed[index]) || + executedPlan.items[completed.length]?.id !== row.item || + !this.continuationApproval(identity, checkpoint.id, row.context.planRevision)) + throw new GuardRefusal('Work after the checkpoint cannot be reconciled with the amended plan.'); + const executedItem = executedPlan.items[completed.length]; + if (!executedItem || executedItem.id !== row.item) throw new GuardRefusal(`Completed item ${row.item} is absent from its execution revision.`); + completedDefinitions.push(executedItem); + head = result.head; + completed.push(row.item); + } + if (head !== this.getSnapshot(identity).head) + throw new GuardRefusal('The task head no longer ends at the audited completed prefix.'); + return { checkpoint, completed, completedDefinitions, head }; + } + /** Reconcile audited completion against the current or proposed plan definition. */ + continuationProgress(identity: PlanIdentity, candidate?: Plan): { checkpoint: Checkpoint; completed: string[]; next: string | null; head: string } | null { + const basis = this.continuationBasis(identity); + if (!basis) return null; + const plan = candidate ?? this.getPlan(identity), { checkpoint, completed, completedDefinitions } = basis; + const prefix = plan.items.slice(0, completed.length); + if (prefix.length !== completed.length || prefix.some((item, index) => item.id !== completed[index])) + throw new GuardRefusal('The amended plan changed the completed checkpoint prefix.'); + const checkpointPlan = this.getPlan(identity, checkpoint.revision); + for (let index = 0; index < checkpoint.completedItems.length; index++) { + const before = checkpointPlan.items[index], current = plan.items[index]; + if (!before || !current) throw new GuardRefusal(`Completed item ${checkpoint.completedItems[index]} changed before the audited checkpoint.`); + if (index < checkpoint.completedItems.length - 1) { + if (stable(before) !== stable(current)) + throw new GuardRefusal(`Completed item ${checkpoint.completedItems[index]} changed before the audited checkpoint.`); + continue; + } + const observed = new Set(checkpoint.outOfScopePaths.map(this.#pathKey)); + const existingFiles = current.files.slice(0, before.files.length); + const addedFiles = current.files.slice(before.files.length); + this.#assertValidItemPaths(current); + if (stable({ ...current, files: existingFiles }) !== stable(before) || + addedFiles.some(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])].some(path => !observed.has(this.#pathKey(path))))) + throw new GuardRefusal(`Completed checkpoint item ${checkpoint.completedItems[index]} changed beyond its scope declaration.`); + } + for (let index = checkpoint.completedItems.length; index < completed.length; index++) { + if (stable(plan.items[index]) !== stable(completedDefinitions[index])) + throw new GuardRefusal(`Completed item ${completed[index]} changed after it ran; review and rerun it before continuing.`); + } + return { checkpoint, completed, next: plan.items[completed.length]?.id ?? null, head: basis.head }; + } + continuationApproval(identity: PlanIdentity, checkpointId: string, revision: number): string | null { + const row = this.#get('SELECT snapshot_id FROM continuations WHERE key=? AND checkpoint_id=? AND revision=?', identityKey(identity), checkpointId, revision); + return typeof row?.snapshot_id === 'string' ? row.snapshot_id : null; + } + /** An approval still covers this head only when its own revision's completed attempts connect it to the current head. */ + continuationApproved(identity: PlanIdentity, progress: NonNullable>): boolean { + const revision = this.getPlan(identity).revision; + const snapshotId = this.continuationApproval(identity, progress.checkpoint.id, revision); + if (!snapshotId) return false; + if (snapshotId === this.getSnapshot(identity).id) return true; + let head = this.getSnapshot(identity, snapshotId).head, advanced = false; + const key = identityKey(identity), origin = this.#get(`SELECT rowid FROM attempts WHERE plan_key=? AND kind='execute' AND state='completed' + AND item=? AND json_extract(context,'$.planRevision')=? AND json_extract(result,'$.head')=? ORDER BY rowid DESC LIMIT 1`, + key, progress.checkpoint.item, progress.checkpoint.revision, this.getSnapshot(identity, progress.checkpoint.snapshotId).head)?.rowid as number | undefined; + if (origin === undefined) return false; + const attempts = this.#db.prepare(`SELECT * FROM attempts WHERE plan_key=? AND rowid>? AND kind='execute' AND state='completed' + AND json_extract(context,'$.planRevision')=? ORDER BY rowid LIMIT 10001`).all(key, origin, revision); + if (attempts.length > 10000) return false; + for (const stored of attempts) { + const row = this.#attemptRecord(stored); + if (this.getSnapshot(identity, row.context.snapshotId).head !== head) continue; + const result = row.result as ExecutionResultLike | null; + if (!result || typeof result.head !== 'string') return false; + head = result.head; advanced = true; + } + return advanced && head === progress.head; + } + /** Persist a person's approval after reconciling the prefix and validating the suffix at its actual tree. */ + approveContinuation(identity: PlanIdentity, checkpointId: string, expected: ReviewState, context: PlanContext): void { const key = identityKey(identity); this.#transaction(() => { this.#expect(key, expected); const checkpoint = this.getCheckpoint(identity, checkpointId); - if (!checkpoint.outOfScopePaths.length || checkpoint.snapshotId !== expected.snapshotId || expected.revision <= checkpoint.revision) throw new Error('Continuation requires an amended plan at the audited checkpoint.'); - this.#run('INSERT INTO continuations VALUES (?,?,?) ON CONFLICT(key,checkpoint_id,revision) DO NOTHING', key, checkpointId, expected.revision); + this.#context(key, context); + const progress = this.continuationProgress(identity); + if (!progress || progress.checkpoint.id !== checkpointId || !checkpoint.outOfScopePaths.length || expected.revision <= checkpoint.revision) + throw new GuardRefusal('Continuation requires an amended plan at the audited checkpoint.'); + if (this.getSnapshot(identity).head !== progress.head || + (progress.completed.length === checkpoint.completedItems.length && checkpoint.snapshotId !== expected.snapshotId)) + throw new GuardRefusal('The audited checkpoint tree or snapshot changed; review it before continuing.'); + // New checkpoint rows bind the saved entries to their immutable commit tree. Legacy rows saved the original base + // tree instead; the caller re-reads the actual tree at progress.head, so comparing it to that old evidence rejects + // valid amendments and is not an integrity check. + if (checkpoint.treeHead === progress.head && JSON.stringify(context.baseEntries) !== JSON.stringify(checkpoint.baseEntries)) + throw new GuardRefusal('The audited checkpoint tree changed; review it before continuing.'); + const amendedItem = this.getPlan(identity).items[checkpoint.completedItems.length - 1]!; + const declared = new Set(amendedItem.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]).map(context.pathKey)); + if (checkpoint.outOfScopePaths.some(path => !declared.has(context.pathKey(path)))) + throw new GuardRefusal('The amended checkpoint item must declare every out-of-scope path it changed.'); + if (progress.next) { + const validation = validateContinuationPlan(this.getPlan(identity), context, progress.completed); + if (validation.errors.length) throw new GuardRefusal(`The remaining plan is invalid at the audited head: ${validation.errors.map(error => error.message).join(' ')}`); + } + const status = this.getTask(identity).status; + if (!['needs amendment', 'queued', 'running'].includes(status)) throw new GuardRefusal(`The task is ${status}; continuation cannot be approved.`); + const task = this.#task(key); + if (task.cancel_requested !== null) throw new GuardRefusal('The task is being cancelled.'); + if (this.#activeAttempt(key)) throw new GuardRefusal('An attempt is still active for this task.'); + if (this.#activeMerge(key)) throw new GuardRefusal('A merge is in progress; wait for its outcome.'); + if (status === 'needs amendment' && progress.next) this.transitionTask(identity, this.getTask(identity).stateVersion, 'queued'); + else if (!progress.next && status !== 'running') { + this.#run("UPDATE tasks SET status='running' WHERE plan_key=?", key); + this.#touch(key); + } + if (!progress.next && task.requeue_pending === 1) { + this.#run('UPDATE tasks SET requeue_pending=0 WHERE plan_key=?', key); + this.#touch(key); + } + this.#run(`INSERT INTO continuations (key,checkpoint_id,revision,snapshot_id) VALUES (?,?,?,?) + ON CONFLICT(key,checkpoint_id,revision) DO UPDATE SET snapshot_id=excluded.snapshot_id`, key, checkpointId, expected.revision, expected.snapshotId); }); } continuationRevision(identity: PlanIdentity, checkpointId: string): number | null { @@ -1333,7 +1561,8 @@ export class Store { return row ? { id: row.id as string, snapshotId: row.snapshot_id as string, result: decode(row.result), stateVersion: row.state_version as number, reviewVersion: row.review_version as number, checkedAt: row.checked_at as string } : null; } /** Publishing runs after the task's last attempt settled, while the task is running, or in needs human for a draft PR. */ - #assertPublishable(key: string, task: Record, expectedStateVersion: number, draft: boolean): void { + #assertPublishable(identity: PlanIdentity, task: Record, expectedStateVersion: number, draft: boolean): void { + const key = identityKey(identity); if (task.state_version !== expectedStateVersion) throw new GuardRefusal('Stale task state. Reload before writing.'); if (task.status !== (draft ? 'needs human' : 'running')) throw new GuardRefusal(`A ${draft ? 'draft ' : ''}pull request cannot be opened while the task is ${task.status}.`); if (this.#activeAttempt(key)) throw new GuardRefusal('An attempt is still active for this task.'); @@ -1348,6 +1577,11 @@ export class Store { const items = decode(this.#get('SELECT data FROM revisions WHERE key=? AND revision=?', key, revision)!.data).items; const finished = new Set(this.#db.prepare(`SELECT DISTINCT item FROM attempts WHERE plan_key=? AND kind='execute' AND item IS NOT NULL AND state='completed' AND json_extract(context,'$.planRevision') = ?`).all(key, revision).map(entry => entry.item as string)); + const checkpoint = this.latestCheckpoint(identity), continuation = checkpoint ? this.continuationProgress(identity) : null; + if (continuation) { + if (!this.continuationApproved(identity, continuation)) throw new GuardRefusal('The current continuation revision is not approved for publishing.'); + for (const item of continuation.completed) finished.add(item); + } const unrun = items.find(item => !finished.has(item.id)); if (unrun) throw new GuardRefusal(`${unrun.id} has not run yet; publish once every plan item has run.`); } @@ -1361,7 +1595,7 @@ export class Store { const key = identityKey(identity); return this.#transaction(() => { const task = this.#task(key); - this.#assertPublishable(key, task, expectedStateVersion, input.draft); + this.#assertPublishable(identity, task, expectedStateVersion, input.draft); if (this.#current(key).snapshot_id !== input.snapshotId) throw new GuardRefusal('The task head changed during the check.'); if (input.result.outcome !== 'clear' && !input.draft) this.#run("UPDATE tasks SET status='possibly already fixed' WHERE plan_key=?", key); this.#touch(key); @@ -1437,7 +1671,7 @@ export class Store { this.#transaction(() => { const row = this.#get("SELECT refresh_version, refresh_review_version FROM task_pull_requests WHERE plan_key=? AND opening_id=? AND refresh_head IS NOT NULL", key, openingId); if (!row) throw new GuardRefusal('No update of this pull request is in flight.'); - this.#assertPublishable(key, this.#task(key), row.refresh_version as number, draft); + this.#assertPublishable(identity, this.#task(key), row.refresh_version as number, draft); if (this.#current(key).review_version !== row.refresh_review_version) throw new GuardRefusal('The review changed after the check. Reload before writing.'); }); } @@ -1448,7 +1682,7 @@ export class Store { #assertCheckedHead(identity: PlanIdentity, input: { checkId: string; headSha: string; draft: boolean }): void { const key = identityKey(identity), task = this.#task(key), check = this.latestAlreadyFixed(identity); if (!check || check.id !== input.checkId || check.result.outcome !== 'clear') throw new GuardRefusal('A clear already-fixed check must come right before opening a pull request.'); - this.#assertPublishable(key, task, check.stateVersion, input.draft); + this.#assertPublishable(identity, task, check.stateVersion, input.draft); if (this.#current(key).review_version !== check.reviewVersion) throw new GuardRefusal('The review changed after the check. Reload before writing.'); const snapshot = this.getSnapshot(identity); if (snapshot.id !== check.snapshotId || snapshot.head !== input.headSha) throw new GuardRefusal('The task head changed after the check.'); @@ -1554,19 +1788,19 @@ export class Store { /** Whether the task can be published in this mode right now (status, no attempt, merge, requeue or rebase). */ canPublish(identity: PlanIdentity, draft: boolean): boolean { const key = identityKey(identity), task = this.#task(key); - try { this.#assertPublishable(key, task, task.state_version as number, draft); return true; } + try { this.#assertPublishable(identity, task, task.state_version as number, draft); return true; } catch (error) { if (error instanceof GuardRefusal) return false; throw error; } } /** The full publish guard at the current state version, before any GitHub call: refuse early, with its reason. */ assertPublishableNow(identity: PlanIdentity, draft: boolean): void { const key = identityKey(identity), task = this.#task(key); - this.#assertPublishable(key, task, task.state_version as number, draft); + this.#assertPublishable(identity, task, task.state_version as number, draft); } /** The task, its review and its head are exactly as a publish read them before its last await. */ assertUnchangedSince(identity: PlanIdentity, input: { stateVersion: number; reviewVersion: number; snapshotId: string; draft: boolean }): void { const key = identityKey(identity); this.#transaction(() => { - this.#assertPublishable(key, this.#task(key), input.stateVersion, input.draft); + this.#assertPublishable(identity, this.#task(key), input.stateVersion, input.draft); const plan = this.#current(key); if (plan.review_version !== input.reviewVersion) throw new GuardRefusal('The review changed after the check. Reload before writing.'); if (plan.snapshot_id !== input.snapshotId) throw new GuardRefusal('The task head changed during the check.'); diff --git a/test/plan-v1.test.ts b/test/plan-v1.test.ts index 2777d48a..b3a78e64 100644 --- a/test/plan-v1.test.ts +++ b/test/plan-v1.test.ts @@ -1,7 +1,7 @@ import { parseV1 } from '../core/parse-v1.ts'; import { describe, expect, it } from 'vitest'; import { stringify } from 'yaml'; -import { commandAllowed, commandArgv, importPlan, validatePlan, type Plan } from '../core/plan.ts'; +import { commandAllowed, commandArgv, importPlan, validateContinuationPlan, validatePlan, type Plan } from '../core/plan.ts'; const plan = (): Plan => ({ schema_version: 1, issue: 1, revision: 1, summary: 'Example', questions: [], items: [{ id: 'P1', title: 'Change', intent: 'Improve', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }], acceptance: [{ type: 'check', text: 'Works' }], depends_on: [] }] }); const context = { identity: { repositoryId: 'repo', taskId: 'task', planId: 'plan' }, issue: 1, baseFiles: ['a'], baseEntries: [{ path: 'a', kind: 'file' as const }], pathKey: (p: string) => p, allowedCommands: [] }; describe('frozen v1 input contract', () => { @@ -19,6 +19,10 @@ describe('frozen v1 input contract', () => { const input = JSON.stringify(plan()).replace('"issue":1', '"issue":2,"iss\\u0075e":1'); expect(() => importPlan(input, 'json', context, 2)).toThrow(/duplicate/i); }); + it('rejects an amended suffix that reuses a completed item ID', () => { + const value = plan(); value.items.push({ ...structuredClone(value.items[0]!), title: 'Different P1 work', intent: 'Must not shadow completed P1', depends_on: ['P1'] }); + expect(validateContinuationPlan(value, context, ['P1'], true).errors).toContainEqual(expect.objectContaining({ code: 'duplicate-id' })); + }); it.each(['&unused Example', '!!str Example', '', '.nan', '0x10', '1_000'])('rejects prohibited YAML scalar %s before schema checks', scalar => { const input = stringify(plan()).replace('summary: Example', 'summary: '+scalar); expect(() => importPlan(input, 'yaml', context, 2)).toThrow(/parse|anchor|tag|scalar/i); diff --git a/test/plan.test.ts b/test/plan.test.ts index a9ef3b17..e8706c42 100644 --- a/test/plan.test.ts +++ b/test/plan.test.ts @@ -8,7 +8,7 @@ export const basePlan = (): Plan => ({ schema_version: 1, issue: 1, revision: 1, { id: 'P1', title: 'Change', intent: 'Improve behavior', files: [{ path: 'a.txt', kind: 'edit', renamed_from: null, change: 'Update behavior.' }], acceptance: [{ type: 'cmd', text: 'npm test' }], depends_on: [] }, ] }); const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; -const applySuggestion = (plan: Plan, reply: unknown, index: number, context: PlanContext) => applyBoundSuggestion(plan, reply, index, context, { identity: context.identity, schemaVersion: plan.schema_version, baseRevision: (reply as { base_revision: number }).base_revision, issue: plan.issue }); +const applySuggestion = (plan: Plan, reply: unknown, index: number, context: PlanContext, completedItems?: readonly string[]) => applyBoundSuggestion(plan, reply, index, context, { identity: context.identity, schemaVersion: plan.schema_version, baseRevision: (reply as { base_revision: number }).base_revision, issue: plan.issue }, completedItems); const context: PlanContext = { identity, baseEntries: [{ path: 'a.txt', kind: 'file' }], pathKey: p => p, allowedCommands: [['npm', 'test']], issue: 1 }; const reply = (op: string, payload: object = {}) => ({ schema_version: 1, base_revision: 1, reply: '', edits: [{ op, item: 'P1', summary: 'Improve plan', reason: 'Clarify it', field: null, value: null, file: null, @@ -104,6 +104,25 @@ describe('suggestions', () => { plan = applySuggestion(plan, { ...reply('add_item', { item: 'P2', new_item: newItem }), base_revision: 4 }, 0, context); expect(plan.items).toHaveLength(2); }); + it('rejects an add-item continuation that exceeds the full-plan item limit', () => { + const plan = basePlan(); + plan.items[0]!.files[0] = { path: 'done.txt', kind: 'add', renamed_from: null, change: 'Create completed output.' }; + for (let index = 2; index <= 30; index++) plan.items.push({ + ...structuredClone(plan.items[0]!), id: `P${index}`, depends_on: [`P${index - 1}`], + files: [{ path: `file-${index}.txt`, kind: 'add', renamed_from: null, change: `Create output ${index}.` }], + }); + const newItem = { ...structuredClone(plan.items[29]!), id: 'P31', depends_on: ['P30'], + files: [{ path: 'file-31.txt', kind: 'add' as const, renamed_from: null, change: 'Create output 31.' }] }; + expect(() => applySuggestion(plan, reply('add_item', { item: 'P31', new_item: newItem }), 0, + { ...context, baseEntries: [] }, ['P1'])).toThrow(/must NOT have more than 30 items/); + }); + it('rejects duplicate dependencies on a completed continuation item', () => { + const plan = basePlan(); + plan.items.push({ ...structuredClone(plan.items[0]!), id: 'P2', depends_on: ['P1'], + files: [{ path: 'b.txt', kind: 'add', renamed_from: null, change: 'Create output.' }] }); + expect(() => applySuggestion(plan, reply('set_depends', { item: 'P2', depends_on: ['P1', 'P1'] }), 0, + context, ['P1'])).toThrow(/Dependencies must be unique/); + }); }); it('rejects file/parent collisions declared within the same item', () => { const plan = basePlan(); plan.items[0]!.files = ['new', 'new/child'].map(path => ({ path, kind: 'add', renamed_from: null, change: 'Create' })); diff --git a/test/planning-author.test.ts b/test/planning-author.test.ts index f69849ef..2100c5ef 100644 --- a/test/planning-author.test.ts +++ b/test/planning-author.test.ts @@ -102,6 +102,35 @@ it('validates cards independently and rejects a batch with even one dependent in const value = reply(); value.edits.push({ ...value.edits[0]!, item: 'P2' }); expect(() => prepareSuggestions({ ...input(), previousPlan: plan() }).validate(JSON.stringify(value))).toThrow(/Target item/); }); +it('validates follow-up drafts and cards only against the audited unfinished suffix', () => { + const previous = plan(); + previous.items[0]!.files = [{ path: 'planned.ts', kind: 'add', renamed_from: null, change: 'Original planned output' }]; + previous.items.push({ id: 'P2', title: 'Finish planned output', intent: 'Create the file that P1 did not produce', + files: [{ path: 'planned.ts', kind: 'add', renamed_from: null, change: 'Create planned output' }], + acceptance: [{ type: 'check', text: 'The file exists' }], depends_on: ['P1'] }); + const value = { ...input(), context: { ...input().context, baseEntries: [] }, completedItems: ['P1'], previousPlan: previous, + continuationContext: { checkpointId: 'cp-1', head: 'a'.repeat(40), completedItems: ['P1'], ownerItem: 'P1', outOfScopePaths: ['debug.log'] } }; + const draft = { ...previous, revision: 2 }; + expect(prepareDraft({ ...value, revision: 2 }).validate(JSON.stringify(draft)).value).toEqual(draft); + const suggestion = reply(); suggestion.edits[0]!.item = 'P2'; + expect(prepareSuggestions(value).validate(JSON.stringify(suggestion)).value).toEqual(suggestion); +}); +it('gives continuation authors checkpoint constraints and permits correction of an invalid unfinished suffix', () => { + const previous = plan(); + previous.items[0]!.files = [{ path: 'planned.ts', kind: 'add', renamed_from: null, change: 'P1 planned output' }]; + previous.items.push({ id: 'P2', title: 'Edit missing output', intent: 'Update the output', + files: [{ path: 'planned.ts', kind: 'edit', renamed_from: null, change: 'Update it' }], + acceptance: [{ type: 'check', text: 'The output exists' }], depends_on: ['P1'] }); + const value = { ...input(), revision: 1, context: { ...input().context, baseEntries: [] }, completedItems: ['P1'], previousPlan: previous, + continuationContext: { checkpointId: 'cp-1', head: 'a'.repeat(40), completedItems: ['P1'], ownerItem: 'P1', outOfScopePaths: ['extra.ts'] } }; + const prepared = prepareSuggestions(value); + expect(prepared.request.prompt).toContain('"completedItems"'); + expect(prepared.request.prompt).toContain('"ownerItem":"P1"'); + expect(prepared.request.prompt).toContain('"outOfScopePaths":["extra.ts"]'); + const correction = { ...previous, revision: 2, items: structuredClone(previous.items) }; + correction.items[1]!.files[0]!.kind = 'add'; + expect(prepareDraft({ ...value, revision: 2 }).validate(JSON.stringify(correction)).value).toEqual(correction); +}); it('serializes only the four issue contract fields, excluding API metadata', () => { const value = input(); value.issue = { ...value.issue, privateMetadata: 'must not reach provider' } as typeof value.issue; diff --git a/test/planning-drafts-api.test.ts b/test/planning-drafts-api.test.ts index 4a9caa72..9dce8961 100644 --- a/test/planning-drafts-api.test.ts +++ b/test/planning-drafts-api.test.ts @@ -6,6 +6,7 @@ import { afterEach, expect, it, vi } from 'vitest'; import type { AuthorProvider, AuthorRequest } from '../core/planning-author.ts'; import type { EditReply } from '../core/plan.ts'; import { createDemo } from '../scripts/demo.ts'; +import { ReviewService } from '../runner/review.ts'; import { startServer, type PlanningDeps } from '../web/server.ts'; import { Store } from '../runner/store.ts'; @@ -19,9 +20,10 @@ afterEach(async () => { type Answer = (request: AuthorRequest, signal: AbortSignal) => Promise; type View = { plan: Record; snapshot: { id: string } }; /** `answer` is made from the review as it was when the server started. */ -async function serve(answer: (view: View) => Answer, options: { describe?: PlanningDeps['describe'] } = {}) { +async function serve(answer: (view: View) => Answer, options: { describe?: PlanningDeps['describe']; prepare?: (config: ReturnType) => void } = {}) { const root = mkdtempSync(join(tmpdir(), 'codeboost-drafts-api-')); roots.push(root); const config = createDemo(join(root, 'demo')); + options.prepare?.(config); const requests: AuthorRequest[] = []; let answering!: Answer; const provider: AuthorProvider = { invoke: (request, signal) => { requests.push(request); return answering(request, signal); } }; @@ -71,6 +73,74 @@ it('drafts the next revision, keeps it until applied, then applies it exactly on expect((await served.api('GET', '/api/review')).body.plan).toMatchObject({ revision: served.view.plan.revision + 1, summary: 'Redrafted by Claude' }); }); +it('prepares a follow-up planning request from the checkpoint tree and applies the suffix safely', async () => { + const served = await serve(view => async request => request.mode === 'draft' + ? JSON.stringify({ ...view.plan, revision: request.revision, summary: 'Continue from checkpoint' }) + : JSON.stringify({ schema_version: 1, base_revision: request.revision, reply: 'Rename suffix', edits: [{ + op: 'set_field', item: 'P2', summary: 'Rename suffix', reason: 'Clearer', field: 'title', value: 'Updated suffix title', + file: null, check: null, check_index: null, depends_on: null, new_item: null, + }] }), { prepare: config => { + const service = new ReviewService(config); + try { + const current = service.store.getPlan(config.identity), base = service.planContext(); + current.items[0]!.files = [{ path: 'planned.ts', kind: 'add', renamed_from: null, change: 'Original planned output' }]; + service.store.importRevision(JSON.stringify(current), 'json', base, current.revision); + const audited = service.planContextAt(service.store.getSnapshot(config.identity).head); + const completed = service.store.getPlan(config.identity), snapshot = service.store.getSnapshot(config.identity); + service.store.recordCheckpoint(config.identity, { revision: completed.revision, snapshotId: snapshot.id, + reviewVersion: service.store.reviewVersion(config.identity) }, { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['debug.log'], baseEntries: audited.baseEntries, + }); + completed.items[0]!.files.push({ path: 'debug.log', kind: 'edit', renamed_from: null, change: 'Declare observed output' }); + completed.items[1]!.files = [{ path: 'planned.ts', kind: 'add', renamed_from: null, change: 'Create output not present at checkpoint' }]; + service.store.importRevision(JSON.stringify(completed), 'json', service.planContextForAmendment(), completed.revision); + } finally { service.close(); } + } }); + const { api, view, requests } = served; + const start = (kind: 'drafts' | 'suggestions') => api('POST', `/api/plan/${kind}`, { + expectedRevision: view.plan.revision, snapshotId: view.snapshot.id, feedback: '', actionId: randomUUID(), + }); + const started = await start('suggestions'); + expect(started).toMatchObject({ status: 200, body: { result: { requestId: expect.any(String) } } }); + const id = started.body.result.requestId as string; + await served.settled('suggestions', id); + expect(requests[0]!.mode).toBe('suggest'); + expect(await api('POST', `/api/plan/suggestions/${id}/apply`, { index: 0, actionId: randomUUID() })) + .toMatchObject({ status: 200, body: { result: { revision: view.plan.revision + 1 } } }); + expect((await api('GET', '/api/review')).body.plan.items[1].title).toBe('Updated suffix title'); + const refreshed = (await api('GET', '/api/review')).body as View; + const draftStart = await api('POST', '/api/plan/drafts', { expectedRevision: refreshed.plan.revision, + snapshotId: refreshed.snapshot.id, feedback: '', actionId: randomUUID() }); + expect(draftStart.status).toBe(200); + const draftId = draftStart.body.result.requestId as string; + await served.settled('drafts', draftId); + expect((await api('POST', `/api/plan/drafts/${draftId}/apply`, { actionId: randomUUID() })).status).toBe(200); +}); + +it.each(['drafts', 'suggestions'] as const)('API %s apply commits invalidation when checkpoint lineage changes after the reply is ready', async kind => { + const served = await serve(view => async request => request.mode === 'draft' + ? JSON.stringify({ ...view.plan, revision: request.revision }) : JSON.stringify(cards(view.plan.revision))); + const id = (await served.start(kind)).body.result.requestId as string; + await served.settled(kind, id); + const service = new ReviewService(served.config); + try { + const store = service.store, identity = served.config.identity, plan = store.getPlan(identity), snapshot = store.getSnapshot(identity); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const attempt = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, kind: 'execute', + item: plan.items[0]!.id, expectedContext: store.currentContext(identity), deadline: Date.now() + 60_000 }); + store.markRunning(identity, attempt.id); + store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: snapshot.head, unchanged: true, inScope: [], outOfScope: [] } }); + store.recordCheckpoint(identity, { revision: plan.revision, snapshotId: snapshot.id, reviewVersion: store.reviewVersion(identity) }, { + item: plan.items[0]!.id, completedItems: [plan.items[0]!.id], outOfScopePaths: ['debug.log'], baseEntries: service.planContext().baseEntries, + }); + } finally { service.close(); } + const action = { actionId: randomUUID(), ...(kind === 'suggestions' ? { index: 0 } : {}) }; + expect(await served.api('POST', `/api/plan/${kind}/${id}/apply`, action)).toMatchObject({ status: 409, + body: { error: kind === 'drafts' ? 'Draft is unavailable.' : 'Suggestion is unavailable.' } }); + expect((await served.api('GET', `/api/plan/${kind}/${id}`)).body.state).toBe('invalidated'); +}); + it('keeps draft and suggestion IDs on their own routes', async () => { const served = await serve(view => async (request, signal) => request.mode === 'draft' ? redraft(view)(request, signal) : JSON.stringify(cards(view.plan.revision))); diff --git a/test/planning-drafts.test.ts b/test/planning-drafts.test.ts index 75e8f55e..049222d7 100644 --- a/test/planning-drafts.test.ts +++ b/test/planning-drafts.test.ts @@ -170,7 +170,20 @@ it.each([9, 10])('migrates a v%i database to the current schema, reading its exi const migrated = open(); expect(migrated.getSuggestions(identity, id)).toMatchObject({ mode: 'suggest', state: 'pending' }); const db = new DatabaseSync(path); - try { expect(db.prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); } finally { db.close(); } + try { expect(db.prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); } finally { db.close(); } +}); +it('adds checkpoint bindings to v14 planning requests without losing their state', () => { + const { store, path, open } = fixture(), id = store.beginSuggestions(identity, state(store), 'draft'); + store.close(); stores.splice(stores.indexOf(store), 1); + const legacy = new DatabaseSync(path); + legacy.exec('ALTER TABLE requests DROP COLUMN continuation; PRAGMA user_version=14;'); legacy.close(); + const migrated = open(); + expect(migrated.getDraft(identity, id).state).toBe('pending'); + const db = new DatabaseSync(path); + try { + expect(db.prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); + expect(db.prepare('PRAGMA table_info(requests)').all().some(column => column.name === 'continuation')).toBe(true); + } finally { db.close(); } }); /** E3 in draft mode, with a provider that answers from `source`. */ diff --git a/test/planning-suggestions.test.ts b/test/planning-suggestions.test.ts index 18ca12e2..0777eff1 100644 --- a/test/planning-suggestions.test.ts +++ b/test/planning-suggestions.test.ts @@ -1,4 +1,5 @@ import { mkdtempSync, rmSync } from 'node:fs'; +import { randomUUID } from 'node:crypto'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, expect, it, vi } from 'vitest'; @@ -35,6 +36,23 @@ function fixture(provider?: AuthorProvider) { const coordinator = new SuggestionCoordinator(store, provider ?? { invoke(request, signal) { calls.push({ request, signal }); return pending.promise; } }, 100); return { store, path: join(dir, 'state.sqlite'), value, identity, pending, calls, coordinator }; } +function checkpointFixture() { + const f = fixture(), next = f.store.getPlan(f.identity); + f.value.context.baseEntries = [...f.value.context.baseEntries, { path: 'b', kind: 'file' }]; + next.items.push({ id: 'P2', title: 'Second', intent: 'Edit b', files: [{ path: 'b', kind: 'edit', renamed_from: null, change: 'Change b' }], + acceptance: [{ type: 'check', text: 'Works' }], depends_on: ['P1'] }); + f.store.importRevision(JSON.stringify(next), 'json', f.value.context, 1); + f.value.revision = 2; + const snapshot = f.store.getSnapshot(f.identity), checkpoint = f.store.recordCheckpoint(f.identity, { + revision: 2, snapshotId: snapshot.id, + }, { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['extra'], baseEntries: f.value.context.baseEntries }); + const basis = f.store.continuationBasis(f.identity)!; + f.value.completedItems = basis.completed; + f.value.continuationBinding = { checkpointId: checkpoint.id, head: basis.head, completedItems: basis.completed }; + f.value.continuationContext = { checkpointId: checkpoint.id, head: basis.head, completedItems: basis.completed, + ownerItem: checkpoint.item, outOfScopePaths: checkpoint.outOfScopePaths }; + return f; +} it('binds the store request before invocation and publishes only valid replies', async () => { const f = fixture(); const handle = f.coordinator.start(f.value); expect(f.store.getSuggestions(f.identity, handle.id).state).toBe('pending'); @@ -66,6 +84,54 @@ it('rejects a changed snapshot even if the plan revision is unchanged', async () expect(f.store.getPlan(f.identity).revision).toBe(1); await f.coordinator.close(); }); +it('invalidates a planning reply when its checkpoint lineage changes while the provider is running', async () => { + const f = checkpointFixture(), handle = f.coordinator.start(f.value, 'draft'); await Promise.resolve(); + const snapshot = f.store.getSnapshot(f.identity); + f.store.recordCheckpoint(f.identity, { revision: 2, snapshotId: snapshot.id }, { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['extra'], baseEntries: f.value.context.baseEntries, + }); + f.pending.resolve(JSON.stringify({ ...f.store.getPlan(f.identity), revision: 3 })); + expect(await handle.result).toMatchObject({ state: 'stale' }); + expect(f.store.getDraft(f.identity, handle.id)).toMatchObject({ state: 'invalidated', plan: null }); + await f.coordinator.close(); +}); +it('refuses to apply a ready planning reply after its checkpoint lineage changes', async () => { + const f = checkpointFixture(), handle = f.coordinator.start(f.value, 'draft'); + f.pending.resolve(JSON.stringify({ ...f.store.getPlan(f.identity), revision: 3 })); + expect(await handle.result).toMatchObject({ state: 'completed' }); + const snapshot = f.store.getSnapshot(f.identity); + f.store.recordCheckpoint(f.identity, { revision: 2, snapshotId: snapshot.id }, { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['extra'], baseEntries: f.value.context.baseEntries, + }); + expect(() => f.store.applyDraft(f.identity, handle.id, f.value.context)).toThrow(/unavailable/i); + expect(f.store.getDraft(f.identity, handle.id).state).toBe('invalidated'); + await f.coordinator.close(); +}); +it.each(['draft', 'suggest'] as const)('commits %s invalidation and the saved refusal when a nested apply sees changed checkpoint lineage', async mode => { + const f = checkpointFixture(), handle = f.coordinator.start(f.value, mode); + const suggestion = reply(); suggestion.base_revision = 2; suggestion.edits[0]!.item = 'P2'; + f.pending.resolve(mode === 'draft' ? JSON.stringify({ ...f.store.getPlan(f.identity), revision: 3 }) : JSON.stringify(suggestion)); + expect(await handle.result).toMatchObject({ state: 'completed' }); + f.store.recordCheckpoint(f.identity, { revision: 2, snapshotId: f.store.getSnapshot(f.identity).id }, { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['extra'], baseEntries: f.value.context.baseEntries, + }); + const action = { actionId: randomUUID(), kind: mode === 'draft' ? 'draft-apply' : 'suggestion-apply', request: { id: handle.id, index: 0 } }; + expect(() => f.store.userAction(f.identity, action, () => mode === 'draft' + ? f.store.applyDraft(f.identity, handle.id, f.value.context) + : f.store.applySuggestion(f.identity, handle.id, 0, f.value.context))).toThrow(/unavailable/i); + expect(f.store.getSuggestions(f.identity, handle.id).state).toBe('invalidated'); + expect(() => f.store.savedAction(f.identity, action)).toThrow(/unavailable/i); + await f.coordinator.close(); +}); +it('does not publish a continuation reply whose candidate changes an executed item', async () => { + const f = checkpointFixture(), handle = f.coordinator.start(f.value, 'draft'); + const candidate = { ...f.store.getPlan(f.identity), revision: 3, + items: f.store.getPlan(f.identity).items.map(item => item.id === 'P1' ? { ...item, title: 'Changed after execution' } : item) }; + f.pending.resolve(JSON.stringify(candidate)); + expect(await handle.result).toMatchObject({ state: 'failed', reason: expect.stringMatching(/completed|checkpoint/i) }); + expect(f.store.getDraft(f.identity, handle.id)).toMatchObject({ state: 'failed', plan: null }); + await f.coordinator.close(); +}); it('retains ownership after timeout until provider termination, even across clock jumps', async () => { vi.useFakeTimers(); const f = fixture(), handle = f.coordinator.start(f.value); await Promise.resolve(); let settled = false; void handle.result.then(() => { settled = true; }); @@ -185,7 +251,7 @@ it.each(['cancelled', 'stale'] as const)('classifies a publication CAS race as % else f.store.importRevision(JSON.stringify({ ...plan(), summary: 'New draft before publication' }), 'json', f.value.context, 1); // Assert the disputed state at the actual publication boundary, not just the outcome. expect(f.store.getSuggestions(identity, id).state).toBe(expected === 'cancelled' ? 'cancelled' : 'invalidated'); - complete(identity, id, reply); + return complete(identity, id, reply); }); const request = f.coordinator.start(f.value); f.pending.resolve(JSON.stringify(reply())); expect(await request.result).toMatchObject({ state: expected }); diff --git a/test/publish.test.ts b/test/publish.test.ts index a71bc1aa..f7646423 100644 --- a/test/publish.test.ts +++ b/test/publish.test.ts @@ -337,7 +337,7 @@ describe('schema v7', () => { expect(upgraded.taskPullRequests(identity)).toEqual([]); expect(upgraded.latestAlreadyFixed(identity)).toBeNull(); const db = new DatabaseSync(path, { readOnly: true }); - expect(db.prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(db.prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); const names = db.prepare("SELECT name FROM sqlite_master WHERE type IN ('table','index') AND (name LIKE '%pull_requests%' OR name LIKE 'already_fixed_checks%') AND name NOT LIKE 'sqlite_autoindex%' ORDER BY name").all().map(row => row.name); expect(names).toEqual(['already_fixed_checks', 'already_fixed_checks_task', 'task_pull_requests', 'task_pull_requests_number', 'task_pull_requests_opening', 'task_pull_requests_task']); db.close(); diff --git a/test/review.test.ts b/test/review.test.ts index 5a5d4362..fb19c70c 100644 --- a/test/review.test.ts +++ b/test/review.test.ts @@ -7,6 +7,7 @@ import { createDemo } from '../scripts/demo.ts'; import { ReviewService } from '../runner/review.ts'; import { startServer } from '../web/server.ts'; import { approveItem, reviewedSegment } from '../core/approvals.ts'; +import { GuardRefusal } from '../runner/lifecycle.ts'; import { fixtureGit } from './fixtures/git.ts'; // A passthrough, so the stale-key test can count how often load() serializes a segment. vi.mock('../core/approvals.ts', async original => { const actual = await original(); return { ...actual, reviewedSegment: vi.fn(actual.reviewedSegment) }; }); @@ -39,6 +40,14 @@ it('offers reapproval when a later attribution choice invalidates execution appr expect(view.items.every(item=>item.state==='approved')).toBe(true); expect(service.store.unapprovedExecutionItems(config.identity,view.plan.revision)).toEqual([]); },30000); +it('keeps review load available when continuation reconciliation refuses', () => { + const {service,config}=fixture();let view=service.load(); + for(const item of view.items)view=service.act({action:'approve',item:item.id,confirmNoChange:item.count===0,token:view.token}); + vi.spyOn(service.store,'unapprovedExecutionItems').mockImplementation(()=>{throw new GuardRefusal('Invalid continuation prefix.');}); + expect(()=>service.load()).not.toThrow(); + expect(service.load().items.every(item=>item.state==='stale')).toBe(true); + expect(service.store.getPlan(config.identity).items.map(item=>item.id)).toEqual(view.plan.items.map(item=>item.id)); +}); it('keeps both sides of a declared rename in scope after manual reassignment',()=>{ const {service,config}=fixture(),identity=config.identity,plan=service.store.getPlan(identity); plan.items=[{...plan.items[0]!,files:[{path:'renamed.ts',kind:'rename',renamed_from:'retry.ts',change:'Rename the implementation.'}],depends_on:[]}]; diff --git a/test/runner-execution.test.ts b/test/runner-execution.test.ts index 947f4b91..950c66ed 100644 --- a/test/runner-execution.test.ts +++ b/test/runner-execution.test.ts @@ -80,7 +80,11 @@ function setup(options: { manifests?: Record { throw options.pathKeyError; } } : context; - const sources: ExecutionSources = { planContext: () => auditContext, issue: options.issue ?? (() => ({ number: 1, title: 'Issue', body: 'Please fix', comments: [] })), lessons: () => [], vendor: () => options.vendor ?? 'claude' }; + const sources: ExecutionSources = { planContext: () => auditContext, + checkpointContext: () => ({ ...auditContext, baseEntries: [...auditContext.baseEntries, + ...Object.values(options.manifests ?? {}).flatMap(value => value.changes.filter(change => change.kind === 'add' && change.path === 'extra.ts') + .map(change => ({ path: change.path, kind: 'file' as const })))] }), + issue: options.issue ?? (() => ({ number: 1, title: 'Issue', body: 'Please fix', comments: [] })), lessons: () => [], vendor: () => options.vendor ?? 'claude' }; const prompts: string[] = [], argv: (readonly (readonly string[])[])[] = [], owners: string[] = [], checks: unknown[] = []; const capability = options.capability?.(store), findings = new SafetyFindings(store, capability); if (options.settleError) store.settleAttempt = () => { throw Object.assign(new Error('disk full'), { code: 'ERR_SQLITE_ERROR' }); }; @@ -634,24 +638,58 @@ describe('item execution', () => { expect(store.getTask(identity).status).toBe('needs amendment'); expect(h.commits.map(c => c.item)).toEqual(['P1']); }); - it('runs no further items once a task has a scope checkpoint, even after an approved continuation (not supported yet)', async () => { + it('resumes only the remaining item after an audited, approved scope amendment', async () => { const h = setup({ manifests: { P1: manifest([change('a.ts'), change('extra.ts', { kind: 'add', oldType: undefined })]) } }); const paused = await h.executor.runTask(identity); expect(paused).toMatchObject({ kind: 'needs amendment', item: 'P1' }); const store = h.store; store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); - const refused = { kind: 'stopped', state: 'not started', reason: expect.stringMatching(/Continuing after a scope pause is not supported yet/) }; + const refused = { kind: 'stopped', state: 'not started', reason: expect.stringMatching(/needs an approved continuation/) }; expect(await h.executor.runTask(identity, { fromItem: 'P2' })).toMatchObject(refused); const amended = { ...plan, revision: 2, items: [{ ...plan.items[0]!, files: [...plan.items[0]!.files, { path: 'extra.ts', kind: 'add', renamed_from: null, change: 'z' }] }, plan.items[1]!] }; store.importRevision(JSON.stringify(amended), 'json', context, 1); - store.approveContinuation(identity, (paused as { checkpointId: string }).checkpointId, { revision: 2, snapshotId: store.getSnapshot(identity).id }); - expect(await h.executor.runTask(identity, { fromItem: 'P2' })).toMatchObject(refused); + store.approveContinuation(identity, (paused as { checkpointId: string }).checkpointId, { revision: 2, snapshotId: store.getSnapshot(identity).id }, + h.sources.checkpointContext(identity, store.getSnapshot(identity).head)); + expect(store.getCheckpoint(identity, (paused as { checkpointId: string }).checkpointId).baseEntries).toContainEqual({ path: 'extra.ts', kind: 'file' }); + expect(await h.executor.runTask(identity, { fromItem: 'P1' })).toMatchObject({ kind: 'stopped', state: 'not started', reason: expect.stringMatching(/must start with P2/) }); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'executed', items: ['P2'] }); + expect(h.commits.map(c => c.item)).toEqual(['P1', 'P2']); + expect((await h.executor.runTask(identity)).kind).toBe('stopped'); // A later snapshot with the same head does not make the recorded pause owed again. const snapshot = store.getSnapshot(identity); store.recordHistory(identity, { revision: 2, snapshotId: snapshot.id }, snapshot.base, snapshot.head, []); - expect(await h.executor.runTask(identity, { fromItem: 'P2' })).toMatchObject(refused); - expect(store.getTask(identity).status).toBe('queued'); - expect(h.commits.map(c => c.item)).toEqual(['P1']); + expect((await h.executor.runTask(identity)).kind).toBe('stopped'); + expect(h.commits.map(c => c.item)).toEqual(['P1', 'P2']); + }); + it('requires a new continuation approval after a later revision consumes the original checkpoint', async () => { + const three: Plan = { ...structuredClone(plan), items: [...structuredClone(plan.items), { + id: 'P3', title: 'Third', intent: 'Finish a', files: [{ path: 'a.ts', kind: 'edit', renamed_from: null, change: 'finish' }], + acceptance: [{ type: 'check', text: 'a reads well' }], depends_on: ['P2'], + }] }; + let h: ReturnType; + let revised = false; + h = setup({ plan: three, manifests: { P1: manifest([change('a.ts'), change('extra.ts', { kind: 'add', oldType: undefined })]), + P3: manifest([change('a.ts')]) }, + release: async () => { + if (revised || h.store.getAttempts(identity).at(-1)?.item !== 'P2') return; + revised = true; + const current = h.store.getPlan(identity); + h.store.importRevision(JSON.stringify(current), 'json', context, current.revision); + } }); + const pause = await h.executor.runTask(identity) as { checkpointId: string }; + const amended = h.store.getPlan(identity); + amended.items[0]!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'observed' }); + h.store.importRevision(JSON.stringify(amended), 'json', context, 1); + h.store.approveContinuation(identity, pause.checkpointId, { revision: 2, snapshotId: h.store.getSnapshot(identity).id }, + h.sources.checkpointContext(identity, h.store.getSnapshot(identity).head)); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P3', reason: expect.stringMatching(/plan changed/) }); + expect(h.store.getPlan(identity).revision).toBe(3); + expect(h.store.continuationProgress(identity)?.completed).toEqual(['P1', 'P2']); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'stopped', item: 'P3', reason: expect.stringMatching(/approved continuation/) }); + h.store.approveContinuation(identity, pause.checkpointId, { revision: 3, snapshotId: h.store.getSnapshot(identity).id }, + h.sources.checkpointContext(identity, h.store.getSnapshot(identity).head)); + expect(await h.executor.runTask(identity)).toMatchObject({ kind: 'executed', items: ['P3'] }); + expect(h.commits.map(commit => commit.item)).toEqual(['P1', 'P2', 'P3']); }); it('fails the attempt when the workspace makes no new commit for a changed item', async () => { const { store, executor } = setup({ commitHead: oid(2) }); diff --git a/test/runner-lifecycle-store.test.ts b/test/runner-lifecycle-store.test.ts index 1419f331..5c7ed89e 100644 --- a/test/runner-lifecycle-store.test.ts +++ b/test/runner-lifecycle-store.test.ts @@ -105,7 +105,7 @@ describe('schema v12 review ordering', () => { migrated.saveReview(identity, { revision: 1, snapshotId: snapshot.id, reviewVersion: migratedVersion }, [{ item: 'P1', fingerprint: 'approved-after-upgrade' }], []); expect(migrated.unapprovedExecutionItems(identity, 1)).toEqual([]); - expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); }); }); @@ -690,7 +690,7 @@ describe('durable safety findings (#87 item 3)', () => { db.exec('ALTER TABLE attempts DROP COLUMN safety_finding; PRAGMA user_version=7;'); db.close(); const reopened = open(path); expect(reopened.getAttempt(identity, attempt.id).safetyFinding).toBeNull(); - expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); }); it('adds the durable owed marker to a version 12 database', () => { @@ -700,7 +700,7 @@ describe('durable safety findings (#87 item 3)', () => { db.exec('ALTER TABLE attempts DROP COLUMN safety_owed; PRAGMA user_version=12;'); db.close(); const reopened = open(path); expect(reopened.owedSafetyFindings()).toEqual([]); - expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); }); it('restores and clears an escalation owed behind a human gate across real store reopens', () => { @@ -740,7 +740,7 @@ describe('allocation baseline (#91)', () => { db.exec('ALTER TABLE attempts DROP COLUMN metadata_baseline; ALTER TABLE attempts DROP COLUMN storage_base; PRAGMA user_version=8;'); db.close(); const reopened = open(path); expect(reopened.interruptedAttempts()).toEqual([expect.objectContaining({ id: attempt.id, metadataBaseline: null, storageBase: null })]); - expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); }); }); describe('publish outcomes (#103)', () => { @@ -756,7 +756,7 @@ describe('publish outcomes (#103)', () => { reopened.recordPublish(identity, { outcome: 'opened', draft: false, message: 'Pull request #1 is open.', number: 1, url: 'https://github.com/o/r/pull/1' }); expect(reopened.lastPublish(identity)).toMatchObject({ outcome: 'opened', number: 1 }); expect(reopened.getTask(identity)).toEqual(before); - expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 13 }); + expect(new DatabaseSync(path).prepare('PRAGMA user_version').get()).toEqual({ user_version: 15 }); }); it('stamps an outcome with the version the publish saw, and refuses one it cannot have seen (#114)', () => { const { store } = queued(), version = store.getTask(identity).stateVersion; diff --git a/test/runner-production.test.ts b/test/runner-production.test.ts index 6b2baaa6..79a4745a 100644 --- a/test/runner-production.test.ts +++ b/test/runner-production.test.ts @@ -150,7 +150,8 @@ describe('server with a runner setup', () => { const assembly = (service: ReviewService) => { const deps: RunnerDeps = { runnerOwner: OWNER, kinds: ['execute'], prepare: async () => { throw new Error('no agent here'); }, cleanupPreparation: async () => undefined, start: () => { throw new Error('no agent here'); }, validate: () => null }; - const sources: ExecutionSources = { planContext: () => service.planContext(), issue: () => ({ number: 1, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; + const sources: ExecutionSources = { planContext: () => service.planContext(), checkpointContext: (_identity, head) => service.planContextAt(head), + issue: () => ({ number: 1, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; return { deps, sources, findings: new SafetyFindings(service.store), recovery: { finalized: [], requeue: [], removedDirectories: [], unknownEntries: [], unmatchedStorage: [], repairedMerges: [] } }; }; it('closes the Store only after the plan runs in progress, and after a failing step', async () => { diff --git a/test/runner-publishing.test.ts b/test/runner-publishing.test.ts index 6e072d3c..95b982df 100644 --- a/test/runner-publishing.test.ts +++ b/test/runner-publishing.test.ts @@ -123,6 +123,59 @@ function completeAll(service: ReviewService) { } expect(s.getTask(id).status).toBe('running'); } +/** One old-revision checkpoint plus a complete amended suffix, all at the unchanged base head. */ +function completeContinuation(service: ReviewService) { + const s = service.store, id = service.config.identity, initial = s.getSnapshot(id), plan = s.getPlan(id); + s.transitionTask(id, s.getTask(id).stateVersion, 'queued'); + const first = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: plan.items[0]!.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, first.id); + s.settleAttempt(id, first.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: initial.head, unchanged: true, inScope: [], outOfScope: [] } }); + const entries = [...service.planContext().baseEntries, { path: 'extra.ts', kind: 'file' as const }]; + const checkpoint = s.recordCheckpoint(id, { revision: 1, snapshotId: s.getSnapshot(id).id, reviewVersion: s.reviewVersion(id) }, { + item: plan.items[0]!.id, completedItems: [plan.items[0]!.id], outOfScopePaths: ['extra.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + const context = { ...service.planContext(), baseEntries: entries }; + s.importRevision(JSON.stringify(amended), 'json', service.planContext(), 1); + const current = s.getPlan(id), snapshot = s.getSnapshot(id); + s.saveReview(id, { revision: current.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, + current.items.map(item => approveItem(current, [], item.id, id, true)), []); + s.approveContinuation(id, checkpoint.id, { revision: current.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, context); + for (const item of current.items.slice(1)) { + const next = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: item.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, next.id); + s.settleAttempt(id, next.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: snapshot.head, unchanged: true, inScope: [], outOfScope: [] } }); + } + expect(s.getTask(id).status).toBe('running'); +} +/** The last item found an out-of-scope file, leaving no suffix after the person amends its declaration. */ +function finalItemScopeFinding(service: ReviewService) { + const s = service.store, id = service.config.identity, items = s.getPlan(id).items, initial = s.getSnapshot(id); + s.transitionTask(id, s.getTask(id).stateVersion, 'queued'); + for (const [index, item] of items.entries()) { + const attempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: item.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, attempt.id); + const last = index === items.length - 1, head = initial.head; + s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head, unchanged: true, inScope: [], outOfScope: last ? ['extra.ts'] : [] } }); + } + const baseContext = service.planContext(), entries = [...baseContext.baseEntries, { path: 'extra.ts', kind: 'file' as const }]; + const snapshot = s.getSnapshot(id), checkpoint = s.pauseForAmendment(id, + { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: items.at(-1)!.id, completedItems: items.map(item => item.id), outOfScopePaths: ['extra.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items.at(-1)!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + s.importRevision(JSON.stringify(amended), 'json', baseContext, 1); + service.planContextAt = () => ({ ...baseContext, baseEntries: entries }); + return checkpoint; +} /** * Every plan item completed, but the last changed a file outside its plan item, and its pause for amendment was never * recorded (the write failed, or the process stopped): the next run owes that pause. @@ -191,7 +244,8 @@ async function serve(w: World, options: { before?: (service: ReviewService) => v const repository = await openRunnerRepository({ runnerRoot: join(w.root, 'runner'), runnerOwner: OWNER, repositoryId: service.config.identity.repositoryId, source: service.config.repository }); await ensureCommit(repository, service.store.getSnapshot(service.config.identity).head); const prepared: PreparedAttempt = { clone: { id: 'clone', taskId: 'task', directory: '/tmp/x', head: 'f'.repeat(40) }, vendor: 'claude', approvedArgv: [] }; - const sources: ExecutionSources = { planContext: () => service.planContext(), issue: () => ({ number: 3, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; + const sources: ExecutionSources = { planContext: () => service.planContext(), checkpointContext: () => service.planContext(), + issue: () => ({ number: 3, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; const pusher = new GitBranchPusher({ repository, repositoryId: service.config.identity.repositoryId, remote: REPO, url: w.remote, ownedCommits: identity => service.store.getLedger(identity).filter(entry => entry.origin === 'owned').map(entry => entry.sha), onProcessGroup: () => options.onPushSpawn?.(++spawns, () => app!, close) }); @@ -232,11 +286,11 @@ async function serve(w: World, options: { before?: (service: ReviewService) => v return { app, close, identity, store: app.service.store, branch: branchOf(identity), head: app.service.store.getSnapshot(identity).head, findings: findings! }; } const view = async (app: App) => (await fetch(`${new URL(app.url).origin}/api/runner`, { headers: { 'x-codeboost-token': app.token } })).json() as Promise>; -async function act(app: App, action: string, actionId = randomUUID(), expectedStateVersion?: number) { - const { stateVersion, reviewVersion } = await view(app); +async function act(app: App, action: string, actionId = randomUUID(), expectedStateVersion?: number, expectedReviewVersion?: number) { + const current = await view(app), stateVersion = expectedStateVersion ?? current.stateVersion, reviewVersion = expectedReviewVersion ?? current.reviewVersion; const response = await fetch(`${new URL(app.url).origin}/api/runner`, { method: 'POST', headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json' }, body: JSON.stringify({ action, expectedStateVersion: expectedStateVersion ?? stateVersion, - ...((action === 'start' || action === 'resume') ? { expectedReviewVersion: reviewVersion } : {}), actionId }) }); + ...((action === 'start' || action === 'resume' || action === 'approve-continuation') ? { expectedReviewVersion: reviewVersion } : {}), actionId }) }); return { status: response.status, body: await response.json() as Record }; } const publishSettled = (app: App, identity: PlanIdentity) => vi.waitFor(async () => { @@ -245,6 +299,90 @@ const publishSettled = (app: App, identity: PlanIdentity) => vi.waitFor(async () }, { timeout: 20_000, interval: 20 }); describe('publishing a finished task (#103)', () => { + it('approves a final-item scope finding and publishes the completed plan ready', async () => { + const w = world(), { app, identity, store } = await serve(w, { before: service => { finalItemScopeFinding(service); }, startup: false }); + expect(store.getTask(identity).status).toBe('needs amendment'); + expect(store.continuationProgress(identity)?.next).toBeNull(); + expect((await act(app, 'approve-continuation')).status).toBe(200); + expect(store.getTask(identity).status).toBe('running'); + await publishSettled(app, identity); + expect(w.github.calls).toContain('open ready'); + expect(store.lastPublish(identity)).toMatchObject({ outcome: 'opened', draft: false }); + }); + it('publishes after approval removes an interrupted final item from an already-running task', async () => { + const w = world(), { app, identity, store } = await serve(w, { startup: false, before: service => { + const s = service.store, id = service.config.identity, plan = s.getPlan(id), snapshot = s.getSnapshot(id); + s.transitionTask(id, s.getTask(id).stateVersion, 'queued'); + const first = plan.items[0]!; + const firstAttempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: first.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, firstAttempt.id); + s.settleAttempt(id, firstAttempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: snapshot.head, unchanged: true, inScope: [], outOfScope: ['extra.ts'] } }); + const entries = [...service.planContext().baseEntries, { path: 'extra.ts', kind: 'file' as const }]; + const checkpoint = s.recordCheckpoint(id, { revision: plan.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: first.id, completedItems: [first.id], outOfScopePaths: ['extra.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'Declare observed output' }); + s.importRevision(JSON.stringify(amended), 'json', service.planContext(), amended.revision); + let current = s.getPlan(id), currentSnapshot = s.getSnapshot(id); + s.saveReview(id, { revision: current.revision, snapshotId: currentSnapshot.id, reviewVersion: s.reviewVersion(id) }, + current.items.map(item => approveItem(current, [], item.id, id, true)), []); + const context = { ...service.planContext(), baseEntries: entries }; + service.planContextAt = () => ({ ...context, baseEntries: entries }); + s.approveContinuation(id, checkpoint.id, { revision: current.revision, snapshotId: currentSnapshot.id, reviewVersion: s.reviewVersion(id) }, context); + + const second = current.items[1]!; + const secondAttempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: second.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, secondAttempt.id); + s.settleAttempt(id, secondAttempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: currentSnapshot.head, unchanged: true, inScope: [], outOfScope: [] } }); + + const third = current.items[2]!; + const thirdAttempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: third.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, thirdAttempt.id); + s.recoverInterrupted(Date.now()); + expect(s.getTask(id)).toMatchObject({ status: 'running', requeuePending: true }); + + const checkpointNow = s.latestCheckpoint(id)!, shortened = s.getPlan(id); + shortened.items = shortened.items.slice(0, 2); + s.importRevision(JSON.stringify({ ...shortened, revision: shortened.revision + 1 }), 'json', service.planContext(), shortened.revision); + current = s.getPlan(id); currentSnapshot = s.getSnapshot(id); + s.saveReview(id, { revision: current.revision, snapshotId: currentSnapshot.id, reviewVersion: s.reviewVersion(id) }, + current.items.map(item => approveItem(current, [], item.id, id, true)), []); + service.planContextAt = () => ({ ...context, baseEntries: checkpointNow.baseEntries }); + } }); + + expect(store.getTask(identity)).toMatchObject({ status: 'running', requeuePending: true }); + expect(store.continuationProgress(identity)?.next).toBeNull(); + expect(store.continuationApproved(identity, store.continuationProgress(identity)!)).toBe(false); + const beforeApproval = await view(app); + const approvalActionId = randomUUID(); + expect((await act(app, 'approve-continuation', approvalActionId, beforeApproval.stateVersion, beforeApproval.reviewVersion)).status).toBe(200); + expect((await act(app, 'approve-continuation', approvalActionId, beforeApproval.stateVersion, beforeApproval.reviewVersion)).status).toBe(200); + await publishSettled(app, identity); + const calls = [...w.github.calls]; + expect((await act(app, 'approve-continuation')).status).toBe(409); + expect(w.github.calls).toEqual(calls); + expect(w.github.calls).toContain('open ready'); + expect(store.lastPublish(identity)).toMatchObject({ outcome: 'opened', draft: false }); + }); + it.each(['automatic', 'explicit'] as const)('publishes a completed continuation through the %s path', async path => { + const w = world(), { app, identity, store } = await serve(w, { before: completeContinuation, startup: path === 'explicit' ? false : undefined }); + expect(() => store.assertPublishableNow(identity, false)).not.toThrow(); + expect(store.canPublish(identity, false)).toBe(true); + if (path === 'explicit') { + expect(app.publishing!.mode(identity)).toMatchObject({ kind: 'publish', draft: false }); + expect((await act(app, 'publish')).status).toBe(200); + } + await publishSettled(app, identity); + expect(w.github.calls).toContain('open ready'); + expect(store.lastPublish(identity)).toMatchObject({ outcome: 'opened', draft: false }); + }); + it('opens one ready pull request when a run completes the plan, with the branch at the task head', async () => { const w = world(); const { app, close, identity, store, branch, head } = await serve(w); diff --git a/test/runner-start.test.ts b/test/runner-start.test.ts index 2092b15a..08713c93 100644 --- a/test/runner-start.test.ts +++ b/test/runner-start.test.ts @@ -33,7 +33,8 @@ async function serve(options: { kinds?: AttemptKind[]; approve?: boolean; before options.before?.(service); const deps: RunnerDeps = { runnerOwner: OWNER, kinds: options.kinds ?? ['execute'], prepare: async () => { throw new Error('no agent here'); }, cleanupPreparation: async () => undefined, start: () => { throw new Error('no agent here'); }, validate: () => null }; - const sources: ExecutionSources = { planContext: () => service.planContext(), issue: () => ({ number: 1, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; + const sources: ExecutionSources = { planContext: () => service.planContext(), checkpointContext: () => service.planContext(), + issue: () => ({ number: 1, title: '', body: '', comments: [] }), lessons: () => [], vendor: () => 'claude' }; const findings = new SafetyFindings(service.store); options.findings?.(findings, service); return { deps, sources, findings, recovery: { finalized: [], requeue: [], removedDirectories: [], unknownEntries: [], unmatchedStorage: [], repairedMerges: [] } }; @@ -81,6 +82,7 @@ function committedFirstItem(service: ReviewService, unchanged = false, outOfScop if (unchanged) s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, result: { head: snapshot.head, unchanged: true, inScope: [], outOfScope: [] } }); else s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, result: { head, unchanged: false, inScope: [], outOfScope }, history: { base: snapshot.base, head, entries: [{ sha: head, owner: item, origin: 'owned', sourceSha: null }] } }); + return attempt.id; } /** Save a new plan revision, as a person editing the plan does. */ function revise(service: ReviewService) { @@ -210,6 +212,98 @@ describe('resume (#91 part 2)', () => { }); describe('start and resume refusals and races (#91 part 2)', () => { + it('retains current-revision approvals over completed continuation items and starts the next suffix item', async () => { + const { app, identity, store, items } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id), itemIds = s.getPlan(id).items.map(item => item.id); + const entries = [...service.planContext().baseEntries, { path: 'other.ts', kind: 'file' as const }]; + const checkpoint = s.recordCheckpoint(id, { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: itemIds[0]!, completedItems: [itemIds[0]!], outOfScopePaths: ['other.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'other.ts', kind: 'add', renamed_from: null, change: 'Declare the observed file' }); + const context = { ...service.planContext(), baseEntries: entries }; + s.importRevision(JSON.stringify(amended), 'json', service.planContext(), 1); + const next = s.getPlan(id); + s.saveReview(id, { revision: next.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, + next.items.map(item => approveItem(next, [], item.id, id, true)), []); + service.planContextAt = () => context; + s.approveContinuation(id, checkpoint.id, { revision: next.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, context); + const attempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: itemIds[1]!, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, attempt.id); + const committedHead = 'd'.repeat(40); + s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: committedHead, unchanged: false, inScope: [], outOfScope: [] }, + history: { base: snapshot.base, head: committedHead, entries: [{ sha: committedHead, owner: itemIds[1]!, origin: 'owned', sourceSha: null }] } }); + } }); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); + expect(await view(app)).toMatchObject({ resumable: true, continuation: { completedItems: [items[0], items[1]], next: items[2], approved: true } }); + expect((await act(app, 'resume')).body.result).toMatchObject({ outcome: 'started', item: items[2] }); + expect(store.getAttempts(identity).at(-1)?.item).toBe(items[2]); + }); + it('does not skip a completed continuation item edited without changing its ID', async () => { + let baseContext: ReturnType | undefined; + const { identity, store } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id), itemIds = s.getPlan(id).items.map(item => item.id); + baseContext = service.planContext(); + // Keep the import context so the regression can restore the executed definition with different object-key order. + const entries = [...baseContext.baseEntries, { path: 'other.ts', kind: 'file' as const }]; + const checkpoint = s.recordCheckpoint(id, { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: itemIds[0]!, completedItems: [itemIds[0]!], outOfScopePaths: ['other.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'other.ts', kind: 'add', renamed_from: null, change: 'Declare the observed file' }); + const context = { ...service.planContext(), baseEntries: entries }; + s.importRevision(JSON.stringify(amended), 'json', baseContext, 1); + const next = s.getPlan(id); + s.saveReview(id, { revision: next.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, + next.items.map(item => approveItem(next, [], item.id, id, true)), []); + service.planContextAt = () => context; + s.approveContinuation(id, checkpoint.id, { revision: next.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, context); + const attempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: itemIds[1]!, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, attempt.id); + s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: snapshot.head, unchanged: true, inScope: [], outOfScope: [] } }); + const changed = s.getPlan(id); + changed.items[1]!.intent += ' (revised after execution)'; + expect(() => s.importRevision(JSON.stringify(changed), 'json', baseContext!, next.revision)) + .toThrow(/Completed item P2 changed after it ran/); + expect(s.getPlan(id).revision).toBe(next.revision); + } }); + const restored = store.getPlan(identity), item = restored.items[1]!; + restored.items[1] = { acceptance: item.acceptance, files: item.files, id: item.id, title: item.title, intent: item.intent, depends_on: item.depends_on }; + store.importRevision(JSON.stringify(restored), 'json', baseContext!, store.getPlan(identity).revision); + expect(store.continuationProgress(identity)).toMatchObject({ completed: ['P1', 'P2'], next: 'P3' }); + }); + it('requires explicit continuation approval and resumes at the audited suffix through the API', async () => { + const { app, identity, store, items } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id); + const entries = [...service.planContext().baseEntries, { path: 'other.ts', kind: 'file' as const }]; + s.pauseForAmendment(id, { revision: 1, snapshotId: snapshot.id }, { + item: s.getPlan(id).items[0]!.id, completedItems: [s.getPlan(id).items[0]!.id], outOfScopePaths: ['other.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'other.ts', kind: 'add', renamed_from: null, change: 'Declare the observed file' }); + s.importRevision(JSON.stringify(amended), 'json', service.planContext(), 1); + const next = s.getPlan(id); + s.saveReview(id, { revision: next.revision, snapshotId: s.getSnapshot(id).id, reviewVersion: s.reviewVersion(id) }, + next.items.map(item => approveItem(next, [], item.id, id, true)), []); + const baseContext = service.planContext(); + service.planContextAt = () => ({ ...baseContext, baseEntries: entries }); + } }); + expect(await view(app)).toMatchObject({ resumable: false, startable: false }); + expect((await act(app, 'resume')).body.error).toMatch(/Approve the amended plan continuation/); + const approved = await act(app, 'approve-continuation'); + expect(approved).toMatchObject({ status: 200, body: { result: { outcome: 'approved', next: items[1] } } }); + expect(store.getTask(identity).status).toBe('queued'); + expect(await view(app)).toMatchObject({ resumable: true, startable: false }); + expect((await act(app, 'resume')).body.result).toMatchObject({ outcome: 'started', item: items[1] }); + expect(store.getAttempts(identity).map(row => row.item)).toEqual([items[0], items[1]]); + }); it('guards start with the review version as well as the task state version', async () => { const { app, identity, store } = await serve(); const before = await view(app); @@ -256,12 +350,12 @@ describe('start and resume refusals and races (#91 part 2)', () => { it('refuses both, in the view and the action alike, once an earlier revision left commits (#88)', async () => { const { app } = await serve({ before: service => { committedFirstItem(service); revise(service); } }); expect(await view(app)).toMatchObject({ startable: false, resumable: false }); - expect((await act(app, 'resume')).body.error).toMatch(/revised after items of it were committed.*#88/); - expect((await act(app, 'start')).body.error).toMatch(/revised after items of it were committed.*#88/); + expect((await act(app, 'resume')).body.error).toMatch(/plan changed after runner commits without a scope checkpoint/); + expect((await act(app, 'start')).body.error).toMatch(/plan changed after runner commits without a scope checkpoint/); }); it('counts a fix attempt\'s commit at an earlier revision too (#88)', async () => { const { app } = await serve({ before: service => { committedFirstItem(service, false, [], 'fix'); revise(service); } }); - expect((await act(app, 'start')).body.error).toMatch(/revised after items of it were committed.*#88/); + expect((await act(app, 'start')).body.error).toMatch(/plan changed after runner commits without a scope checkpoint/); }); it('lets admission refuse a spent budget, which moves the idle task to needs human; the view offers nothing', async () => { const { app, identity, store } = await serve({ before: service => { failedFirstItem(service, 1); } }); @@ -298,6 +392,53 @@ describe('start and resume refusals and races (#91 part 2)', () => { expect((await act(app, 'start')).body.error).toMatch(/The task is running; start runs a task that is in review or queued/); expect((await act(app, 'resume')).body.error).toMatch(/Every item of this plan has run/); }); + it('clears a recovery requeue when an amended suffix removes the interrupted final item', async () => { + const { app, identity, store } = await serve({ before: service => { + const s = service.store, id = service.config.identity; + const plan = s.getPlan(id), firstTwo = plan.items.slice(0, 2).map(item => item.id), firstSnapshot = s.getSnapshot(id); + s.transitionTask(id, s.getTask(id).stateVersion, 'queued'); + for (const item of plan.items.slice(0, 2)) { + const attempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: item.id, + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, attempt.id); + const last = item.id === firstTwo[0]; + s.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head: firstSnapshot.head, unchanged: true, inScope: [], outOfScope: last ? ['extra.ts'] : [] } }); + if (last) { + const entries = [...service.planContext().baseEntries, { path: 'extra.ts', kind: 'file' as const }]; + const checkpoint = s.recordCheckpoint(id, { revision: plan.revision, snapshotId: firstSnapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: item.id, completedItems: [item.id], outOfScopePaths: ['extra.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'Declare observed output' }); + s.importRevision(JSON.stringify({ ...amended, revision: amended.revision + 1 }), 'json', service.planContext(), amended.revision); + const current = s.getPlan(id), snapshot = s.getSnapshot(id); + approvePlan(service); + const checkpointContext = { ...service.planContext(), baseEntries: checkpoint.baseEntries }; + service.planContextAt = () => ({ ...checkpointContext, baseEntries: entries }); + s.approveContinuation(id, checkpoint.id, { revision: current.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, + { ...checkpointContext, baseEntries: entries }); + } else if (item.id === firstTwo[1]) { + // The preceding checkpoint is approved; this is continuation work P2. + } + } + const p3 = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', item: 'P3', + expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, p3.id); s.recoverInterrupted(Date.now()); + const checkpoint = s.latestCheckpoint(id)!, amended = s.getPlan(id); + amended.items = amended.items.slice(0, 2); + s.importRevision(JSON.stringify({ ...amended, revision: amended.revision + 1 }), 'json', service.planContext(), amended.revision); + const current = s.getPlan(id), snapshot = s.getSnapshot(id); + approvePlan(service); + const entries = checkpoint.baseEntries; + const checkpointContext = { ...service.planContext(), baseEntries: checkpoint.baseEntries }; + service.planContextAt = () => ({ ...checkpointContext, baseEntries: entries }); + s.approveContinuation(id, checkpoint.id, { revision: current.revision, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, + { ...checkpointContext, baseEntries: entries }); + } }); + expect(store.getTask(identity).requeuePending).toBe(false); + expect((await act(app, 'resume')).body.error).toMatch(/Every item of this plan has run/); + }); it('neither offers nor admits start or resume while a run of the task is still finishing between items', async () => { const { app, identity, store } = await serve({ before: service => { failedFirstItem(service); } }); expect(await view(app)).toMatchObject({ resumable: true }); @@ -488,6 +629,50 @@ describe('start and resume refusals and races (#91 part 2)', () => { expect(store.getTask(identity)).toMatchObject({ status: 'needs human', stateVersion: before + 1 }); expect(store.getAttempts(identity)).toHaveLength(1); }); + it('settles owed safety evidence before continuation reconciliation can refuse resume', async () => { + let earlier = ''; + const { app, identity, store } = await serve({ before: service => { + earlier = committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id), plan = s.getPlan(id); + s.recordCheckpoint(id, { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: plan.items[0]!.id, completedItems: [plan.items[0]!.id], outOfScopePaths: ['other.ts'], + baseEntries: [...service.planContext().baseEntries, { path: 'other.ts', kind: 'file' }], + }); + }, findings: findings => findings.record(earlier, 'Safety violation: checkpoint reconciliation regression') }); + expect((await act(app, 'resume')).body.result).toEqual({ outcome: 'settled' }); + expect(store.getTask(identity).status).toBe('needs human'); + expect(store.getAttempts(identity)[0]!.safetyFinding).toMatch(/checkpoint reconciliation regression/); + }); + it('records an owed later scope pause before continuation reconciliation', async () => { + const { app, identity, store, items } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, firstSnapshot = s.getSnapshot(id), baseContext = service.planContext(); + const entries = [...baseContext.baseEntries, { path: 'other.ts', kind: 'file' as const }]; + const firstItem = s.getPlan(id).items[0]!.id; + const checkpoint = s.recordCheckpoint(id, { revision: 1, snapshotId: firstSnapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: firstItem, completedItems: [firstItem], outOfScopePaths: ['other.ts'], baseEntries: entries, + }); + const amended = s.getPlan(id); + amended.items[0]!.files.push({ path: 'other.ts', kind: 'add', renamed_from: null, change: 'Declare the first observed path' }); + s.importRevision(JSON.stringify(amended), 'json', baseContext, 1); + const current = s.getPlan(id), reviewSnapshot = s.getSnapshot(id); + s.saveReview(id, { revision: current.revision, snapshotId: reviewSnapshot.id, reviewVersion: s.reviewVersion(id) }, + current.items.map(item => approveItem(current, [], item.id, id, true)), []); + service.planContextAt = () => ({ ...baseContext, baseEntries: entries }); + s.approveContinuation(id, checkpoint.id, { revision: current.revision, snapshotId: reviewSnapshot.id, reviewVersion: s.reviewVersion(id) }, + { ...baseContext, baseEntries: entries }); + const before = s.getSnapshot(id), second = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind: 'execute', + item: current.items[1]!.id, expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); + s.markRunning(id, second.id); + const head = 'd'.repeat(40); + s.settleAttempt(id, second.id, { firstReason: null, exitCode: 0, valid: true, + result: { head, unchanged: false, inScope: [], outOfScope: ['later.ts'] }, + history: { base: before.base, head, entries: [{ sha: head, owner: current.items[1]!.id, origin: 'owned', sourceSha: null }] } }); + } }); + expect((await act(app, 'resume')).body.result).toEqual({ outcome: 'settled' }); + expect(store.getTask(identity).status).toBe('needs amendment'); + expect(store.latestCheckpoint(identity)).toMatchObject({ item: items[1], completedItems: items.slice(0, 2), outOfScopePaths: ['later.ts'] }); + }); it('replays an action ID, and refuses a second resume made against the old state', async () => { const { app, identity, store } = await serve({ before: service => { failedFirstItem(service); } }); const { stateVersion } = await view(app), actionId = randomUUID(); @@ -554,4 +739,80 @@ describe('start and resume refusals and races (#91 part 2)', () => { // Not recorded: the same action ID is refused afresh (503 again), never replayed as a saved refusal. expect((await act(app, 'start', { actionId })).status).toBe(503); }); + it('keeps continuation approval retryable once the runner stopped admission', async () => { + const { app, identity, store } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id); + s.recordCheckpoint(id, { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: s.getPlan(id).items[0]!.id, completedItems: [s.getPlan(id).items[0]!.id], outOfScopePaths: ['other.ts'], + baseEntries: [...service.planContext().baseEntries, { path: 'other.ts', kind: 'file' }], + }); + } }); + const { stateVersion, reviewVersion } = await view(app), actionId = randomUUID(); + app.runner!.rejectAdmission(); + expect(await act(app, 'approve-continuation', { actionId })).toMatchObject({ status: 503 }); + expect(store.savedAction(identity, { actionId, kind: 'approve-continuation', request: { + attemptId: undefined, expectedStateVersion: stateVersion, expectedReviewVersion: reviewVersion, + } })).toBeUndefined(); + }); + it('answers 503 to a stale continuation approval whose body finishes arriving after shutdown began, without saving the action refusal', async () => { + const { app, close, database, identity } = await serve({ before: service => { + committedFirstItem(service, false, ['other.ts']); + const s = service.store, id = service.config.identity, snapshot = s.getSnapshot(id); + s.recordCheckpoint(id, { revision: 1, snapshotId: snapshot.id, reviewVersion: s.reviewVersion(id) }, { + item: s.getPlan(id).items[0]!.id, completedItems: [s.getPlan(id).items[0]!.id], outOfScopePaths: ['other.ts'], + baseEntries: [...service.planContext().baseEntries, { path: 'other.ts', kind: 'file' }], + }); + } }); + const current = await view(app), stateVersion = current.stateVersion - 1, reviewVersion = current.reviewVersion - 1; + const actionId = randomUUID(), url = new URL(app.url); + const text = JSON.stringify({ action: 'approve-continuation', expectedStateVersion: stateVersion, expectedReviewVersion: reviewVersion, actionId }); + let finish!: () => void; + const response = new Promise((resolve, reject) => { + const req = httpRequest({ host: url.hostname, port: url.port, path: '/api/runner', method: 'POST', + headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json', 'content-length': Buffer.byteLength(text) } }, res => { res.resume(); res.on('end', () => resolve(res.statusCode!)); }); + req.on('error', reject); + req.write(text.slice(0, 5)); + finish = () => req.end(text.slice(5)); + }); + await new Promise(resolve => setTimeout(resolve, 20)); + const closing = close(); + await new Promise(resolve => setTimeout(resolve, 20)); + finish(); + expect(await response).toBe(503); + await closing; + const store = new Store(database); cleanups.push(() => store.close()); + expect(store.savedAction(identity, { actionId, kind: 'approve-continuation', request: { + attemptId: undefined, expectedStateVersion: stateVersion, expectedReviewVersion: reviewVersion, + } })).toBeUndefined(); + }); + it('keeps a continuation approval retryable during shutdown when no runner is configured', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-no-runner-')); roots.push(root); + const demo = createDemo(join(root, 'demo')); + const app = await startServer({ ...demo, demo: false }, 0, undefined, undefined, 2_000); + let closed = false; + const close = async () => { if (!closed) { closed = true; await app.close(); } }; + cleanups.push(close); + const { stateVersion, reviewVersion } = await view(app), actionId = randomUUID(), url = new URL(app.url); + const text = JSON.stringify({ action: 'approve-continuation', expectedStateVersion: stateVersion, + expectedReviewVersion: reviewVersion, actionId }); + let finish!: () => void; + const response = new Promise((resolve, reject) => { + const req = httpRequest({ host: url.hostname, port: url.port, path: '/api/runner', method: 'POST', + headers: { 'x-codeboost-token': app.token, 'content-type': 'application/json', 'content-length': Buffer.byteLength(text) } }, res => { res.resume(); res.on('end', () => resolve(res.statusCode!)); }); + req.on('error', reject); + req.write(text.slice(0, 5)); + finish = () => req.end(text.slice(5)); + }); + await new Promise(resolve => setTimeout(resolve, 20)); + const closing = close(); + await new Promise(resolve => setTimeout(resolve, 20)); + finish(); + expect(await response).toBe(503); + await closing; + const store = new Store(demo.database); cleanups.push(() => store.close()); + expect(store.savedAction(demo.identity, { actionId, kind: 'approve-continuation', request: { + attemptId: undefined, expectedStateVersion: stateVersion, expectedReviewVersion: reviewVersion, + } })).toBeUndefined(); + }); }); diff --git a/test/runner-workspace.test.ts b/test/runner-workspace.test.ts index acf4e107..1a898819 100644 --- a/test/runner-workspace.test.ts +++ b/test/runner-workspace.test.ts @@ -48,7 +48,11 @@ async function setup(agent: Record) { const repository = await openRunnerRepository({ runnerRoot, runnerOwner: RUNNER_OWNER, repositoryId: identity.repositoryId, source }); const workspace = createTaskWorkspace({ store, runnerRoot, runnerOwner: RUNNER_OWNER, repository, imageId, limits: LIMITS, committer: { name: 'codeboost', email: 'runner@codeboost.invalid' }, now: () => 1_700_000_000_000 }); - const sources: ExecutionSources = { planContext: () => context, issue: () => ({ number: 1, title: 'Issue', body: 'Fix', comments: [] }), + const sources: ExecutionSources = { planContext: () => context, checkpointContext: (_identity, commit) => { + const review = new ReviewService({ database: join(root, 'state.sqlite'), repository: source, runnerRepository: repository.path, + identity, pathIdentity: { caseSensitive: true, unicodeNormalization: 'none' } }); + try { return review.planContextAt(commit); } finally { review.close(); } + }, issue: () => ({ number: 1, title: 'Issue', body: 'Fix', comments: [] }), lessons: () => [], vendor: () => 'claude' }; const findings = new SafetyFindings(store); // The agent: a container that edits the work volume, mounted as an agent container mounts it (metadata read-only). @@ -125,17 +129,19 @@ describe('real task workspace (#87)', () => { const outcome = await s.executor.runTask(identity) as { kind: string; checkpointId: string }; expect(outcome).toMatchObject({ kind: 'needs amendment', item: 'P1', outOfScope: ['extra.ts'] }); const checkpoint = s.store.getCheckpoint(identity, outcome.checkpointId); + expect(checkpoint.baseEntries).toContainEqual({ path: 'extra.ts', kind: 'file' }); // The review load reads the runner commit at the recorded head: it does not replace the checkpoint's snapshot. const review = new ReviewService({ database: join(s.root, 'state.sqlite'), repository: s.source, runnerRepository: s.repository.path, identity, pathIdentity: { caseSensitive: true, unicodeNormalization: 'none' } }); - try { expect(review.load().snapshot.id).toBe(checkpoint.snapshotId); } finally { review.close(); } + expect(review.load().snapshot.id).toBe(checkpoint.snapshotId); // A person amends the plan to declare the extra file, then approves continuing from the audited checkpoint. const amended = { ...plan, items: [{ ...plan.items[0]!, files: [...plan.items[0]!.files, { path: 'extra.ts', kind: 'add' as const, renamed_from: null, change: 'z' }] }, plan.items[1]!] }; s.store.importRevision(JSON.stringify(amended), 'json', { identity, issue: 1, pathKey: p => p, allowedCommands: [], baseEntries: [{ path: 'a.ts', kind: 'file' }, { path: 'b.ts', kind: 'file' }] }, 1); const revision = s.store.getPlan(identity).revision; expect(() => s.store.approveContinuation(identity, outcome.checkpointId, { revision, snapshotId: checkpoint.snapshotId, - reviewVersion: s.store.reviewVersion(identity) })).not.toThrow(); + reviewVersion: s.store.reviewVersion(identity) }, review.planContextAt(s.store.getSnapshot(identity).head))).not.toThrow(); expect(s.store.continuationRevision(identity, outcome.checkpointId)).toBe(revision); + review.close(); }, 600_000); }); diff --git a/test/store.test.ts b/test/store.test.ts index 777d1021..9cf175e9 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -19,9 +19,12 @@ const reply = (revision = 1): EditReply => ({ schema_version: 1, base_revision: const dirs: string[] = [], stores: Store[] = []; afterEach(() => { for (const store of stores.splice(0)) store.close(); for (const dir of dirs.splice(0)) rmSync(dir, { recursive: true, force: true }); }); function directory() { const dir = mkdtempSync(join(tmpdir(), 'codeboost-store-')); dirs.push(dir); return dir; } -function open(path: string) { const store = new Store(path); stores.push(store); return store; } +function open(path: string, pathKey: (path: string) => string = p => p) { const store = new Store(path, pathKey); stores.push(store); return store; } function close(store: Store) { store.close(); stores.splice(stores.indexOf(store), 1); } -function fixture() { const path = join(directory(), 'state.sqlite'); const store = open(path); store.createPlan(JSON.stringify(plan()), 'json', context, oid(1), oid(2)); return { path, store }; } +function fixture(two = false) { const path = join(directory(), 'state.sqlite'); const store = open(path); + const initial = plan(); + if (two) initial.items.push({ ...structuredClone(initial.items[0]!), id: 'P2', depends_on: ['P1'] }); + store.createPlan(JSON.stringify(initial), 'json', context, oid(1), oid(2)); return { path, store }; } const state = (store: Store) => ({ revision: store.getPlan(identity).revision, snapshotId: store.getSnapshot(identity).id }); function ready(store: Store) { const id = store.beginSuggestions(identity, state(store), 'suggest'); store.completeSuggestions(identity, id, reply()); return id; } it('allocates revisions in SQLite, survives reopen, and keeps old revisions and snapshots immutable', () => { @@ -197,13 +200,17 @@ it('retains typed file-card approvals/choices while fingerprints detect later me expect(approvalStates(store.getPlan(identity), [segment], store.getReview(identity).approvals, identity).P1).toBe('stale'); }); it('retains checkpoint scope evidence after amended continuation and rejects a moved head', () => { - const { store, path } = fixture(); const checkpoint = store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: [...context.baseEntries, { path: 'outside', kind: 'file' }] }); - expect(() => store.approveContinuation(identity, checkpoint.id, state(store))).toThrow(/amended/); - store.importRevision(JSON.stringify(plan()), 'json', context, 1); - store.approveContinuation(identity, checkpoint.id, state(store)); + const { store, path } = fixture(true); store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const entries = [...context.baseEntries, { path: 'outside', kind: 'file' as const }]; + const checkpoint = store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: entries }); + expect(() => store.approveContinuation(identity, checkpoint.id, state(store), { ...context, baseEntries: entries })).toThrow(/amended/); + const amended = store.getPlan(identity); + amended.items[0]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'scope amendment' }); + store.importRevision(JSON.stringify(amended), 'json', context, 1); + store.approveContinuation(identity, checkpoint.id, state(store), { ...context, baseEntries: entries }); const recovered = open(path); expect(recovered.getCheckpoint(identity, checkpoint.id)).toEqual(checkpoint); expect(recovered.continuationRevision(identity, checkpoint.id)).toBe(2); store.recordHistory(identity, state(store), oid(1), oid(3), []); - expect(() => store.approveContinuation(identity, checkpoint.id, state(store))).toThrow(/checkpoint/); + expect(() => store.approveContinuation(identity, checkpoint.id, state(store), { ...context, baseEntries: entries })).toThrow(/head|checkpoint/); }); it('rejects unsupported Node versions and opens SQLite without warnings', () => { expect(() => requireSupportedNode('24.0.0')).toThrow(/Upgrade Node/); expect(() => requireSupportedNode('26.6.0')).toThrow(); @@ -267,14 +274,199 @@ it('allows historical ledger retries after the owning item is removed, but rejec expect(store.getLedger(identity).find(entry => entry.sha === oid(5))!.owner).toBe('P1'); }); it('allows a later amended revision to receive a fresh continuation approval', () => { - const { store } = fixture(); const checkpoint = store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries }); - store.importRevision(JSON.stringify(plan()), 'json', context, 1); store.approveContinuation(identity, checkpoint.id, state(store)); - expect(() => store.approveContinuation(identity, checkpoint.id, state(store))).not.toThrow(); - store.importRevision(JSON.stringify(plan()), 'json', context, 2); + const { store } = fixture(true); store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const checkpoint = store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries }); + const amended = store.getPlan(identity); + amended.items[0]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'scope amendment' }); + store.importRevision(JSON.stringify(amended), 'json', context, 1); store.approveContinuation(identity, checkpoint.id, state(store), context); + expect(() => store.approveContinuation(identity, checkpoint.id, state(store), context)).not.toThrow(); + store.importRevision(JSON.stringify(store.getPlan(identity)), 'json', context, 2); expect(store.continuationRevision(identity, checkpoint.id)).toBe(2); - expect(() => store.approveContinuation(identity, checkpoint.id, state(store))).not.toThrow(); + expect(() => store.approveContinuation(identity, checkpoint.id, state(store), context)).not.toThrow(); expect(store.continuationRevision(identity, checkpoint.id)).toBe(3); }); +it('refuses a suffix that is invalid against the audited tree despite a valid base-tree plan', () => { + const { store } = fixture(true); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const checkpoint = store.recordCheckpoint(identity, state(store), { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: [], + }); + const amended = store.getPlan(identity); + amended.items[0]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'scope amendment' }); + store.importRevision(JSON.stringify(amended), 'json', context, 1); + expect(() => store.approveContinuation(identity, checkpoint.id, state(store), { ...context, baseEntries: [] })) + .toThrow(/remaining plan is invalid.*Missing source: a/); + expect(store.continuationRevision(identity, checkpoint.id)).toBeNull(); +}); +it('re-reads the actual checkpoint tree for legacy checkpoint rows', () => { + const { store, path } = fixture(true); store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const checkpoint = store.recordCheckpoint(identity, state(store), { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries, + }); + close(store); + const db = new DatabaseSync(path); + db.prepare('UPDATE checkpoints SET data=? WHERE key=? AND id=?').run( + JSON.stringify({ ...checkpoint, treeHead: undefined }), JSON.stringify([identity.repositoryId, identity.taskId, identity.planId]), checkpoint.id); + db.close(); + const reopened = open(path), amended = reopened.getPlan(identity); + amended.items[0]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'scope amendment' }); + reopened.importRevision(JSON.stringify(amended), 'json', context, 1); + expect(() => reopened.approveContinuation(identity, checkpoint.id, state(reopened), { + ...context, baseEntries: [...context.baseEntries, { path: 'outside', kind: 'file' }], + })).not.toThrow(); +}); +it('refuses edits to completed items before the checkpoint owner', () => { + const { store } = fixture(true); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + store.recordCheckpoint(identity, state(store), { + item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries, + }); + const amended = store.getPlan(identity); + amended.items[0]!.intent += ' revised after completion'; + amended.items[1]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + expect(() => store.importRevision(JSON.stringify(amended), 'json', context, 1)).toThrow(/Completed item P1 changed before the audited checkpoint/); + expect(store.getPlan(identity).revision).toBe(1); +}); +it('refuses checkpoint-owner edits beyond declaring the observed scope finding', () => { + const { store } = fixture(true); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + store.recordCheckpoint(identity, state(store), { + item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries, + }); + const amended = store.getPlan(identity); + amended.items[1]!.intent += ' changed after it completed'; + amended.items[1]!.files.push({ path: 'outside', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + expect(() => store.importRevision(JSON.stringify(amended), 'json', context, 1)).toThrow(/changed beyond its scope declaration/); + expect(store.getPlan(identity).revision).toBe(1); +}); +it('refuses a scope declaration whose rename source was not observed', () => { + const path = join(directory(), 'state.sqlite'), store = open(path), contextWithSource = { + ...context, baseEntries: [...context.baseEntries, { path: 'b', kind: 'file' as const }], + }; + const initial = plan(); initial.items.push({ ...structuredClone(initial.items[0]!), id: 'P2', depends_on: ['P1'] }); + store.createPlan(JSON.stringify(initial), 'json', contextWithSource, oid(1), oid(2)); + store.recordCheckpoint(identity, state(store), { + item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: contextWithSource.baseEntries, + }); + const amended = store.getPlan(identity); + amended.items[1]!.files.push({ path: 'outside', kind: 'rename', renamed_from: 'b', change: 'Rename the observed path' }); + expect(() => store.importRevision(JSON.stringify(amended), 'json', contextWithSource, 1)).toThrow(/changed beyond its scope declaration/); + expect(store.getPlan(identity).revision).toBe(1); +}); +it.each([ + { + name: 'duplicate operations', outOfScopePaths: ['outside'], + files: [ + { path: 'outside', kind: 'add' as const, renamed_from: null, change: 'Declare the observed add' }, + { path: 'outside', kind: 'delete' as const, renamed_from: null, change: 'Declare the observed delete' }, + ], + }, + { + name: 'parent and child paths', outOfScopePaths: ['outside', 'outside/child'], + files: [ + { path: 'outside', kind: 'add' as const, renamed_from: null, change: 'Declare the observed parent' }, + { path: 'outside/child', kind: 'add' as const, renamed_from: null, change: 'Declare the observed child' }, + ], + }, + { + name: 'a duplicated rename source', outOfScopePaths: ['outside', 'moved'], + files: [ + { path: 'outside', kind: 'add' as const, renamed_from: null, change: 'Declare the observed add' }, + { path: 'moved', kind: 'rename' as const, renamed_from: 'outside', change: 'Declare the observed rename' }, + ], + }, + { + name: 'a rename source on a non-rename operation', outOfScopePaths: ['outside', 'source'], + files: [ + { path: 'outside', kind: 'add' as const, renamed_from: 'source', change: 'Declare the observed add' }, + ], + }, + { + name: 'an unsafe canonical path', outOfScopePaths: ['bad:name'], + files: [ + { path: 'bad:name', kind: 'add' as const, renamed_from: null, change: 'Declare the observed path' }, + ], + }, +])('refuses checkpoint scope declarations with $name', ({ outOfScopePaths, files }) => { + const { store } = fixture(true); + store.recordCheckpoint(identity, state(store), { + item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths, baseEntries: context.baseEntries, + }); + const amended = store.getPlan(identity); + amended.items[1]!.files.push(...files); + expect(() => store.importRevision(JSON.stringify(amended), 'json', context, 1)).toThrow(/file declarations/); + expect(store.getPlan(identity).revision).toBe(1); +}); +it('uses configured path identity when matching checkpoint scope declarations', () => { + const path = join(directory(), 'state.sqlite'), pathKey = (value: string) => value.toLowerCase(); + const store = open(path, pathKey), normalizedContext = { ...context, pathKey }; + store.createPlan(JSON.stringify({ ...plan(), items: [...plan().items, { ...structuredClone(plan().items[0]!), id: 'P2', depends_on: ['P1'] }] }), + 'json', normalizedContext, oid(1), oid(2)); + store.recordCheckpoint(identity, state(store), { + item: 'P2', completedItems: ['P1', 'P2'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries, + }); + const amended = store.getPlan(identity); + amended.items[1]!.files.push({ path: 'OUTSIDE', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + expect(store.importRevision(JSON.stringify(amended), 'json', normalizedContext, 1).revision).toBe(2); + expect(store.continuationProgress(identity)?.next).toBeNull(); + const identityCollision = store.getPlan(identity); + identityCollision.items[1]!.files.push({ path: 'outside', kind: 'delete', renamed_from: null, change: 'Duplicate the observed path' }); + expect(() => store.importRevision(JSON.stringify(identityCollision), 'json', normalizedContext, 2)) + .toThrow(/file declarations/); + expect(store.getPlan(identity).revision).toBe(2); + const identityDrift = store.getPlan(identity); + identityDrift.issue = 2; + expect(() => store.importRevision(JSON.stringify(identityDrift), 'json', normalizedContext, 2)).toThrow(/Plan issue does not match/); + expect(store.getPlan(identity).revision).toBe(2); +}); +it('can restore an executed definition after a transient pre-checkpoint amendment', () => { + const { store } = fixture(true), id = identity; + store.transitionTask(id, store.getTask(id).stateVersion, 'queued'); + const start = store.getSnapshot(id), attempt = store.admitAttempt(id, { expectedStateVersion: store.getTask(id).stateVersion, + kind: 'execute', item: 'P1', expectedContext: store.currentContext(id), deadline: Date.now() + 60_000 }); + store.markRunning(id, attempt.id); + const head = oid(7); + store.settleAttempt(id, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { head, unchanged: false, inScope: [], outOfScope: ['outside'] }, + history: { base: start.base, head, entries: [{ sha: head, owner: 'P1', origin: 'owned', sourceSha: null }] } }); + const transient = store.getPlan(id); transient.items[0]!.intent += ' transient edit'; + store.importRevision(JSON.stringify(transient), 'json', context, 1); + store.pauseForAmendment(id, { revision: 1, snapshotId: store.getSnapshot(id).id }, { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], + baseEntries: [...context.baseEntries, { path: 'outside', kind: 'file' }], + }, { owed: true }); + const restore = store.getPlan(id); restore.items[0]!.intent = plan().items[0]!.intent; + restore.items[0]!.files.push({ path: 'outside', kind: 'edit', renamed_from: null, change: 'Declare observed file' }); + expect(() => store.importRevision(JSON.stringify(restore), 'json', { + ...context, baseEntries: [...context.baseEntries, { path: 'outside', kind: 'file' }], + }, 2)).not.toThrow(); + expect(store.continuationProgress(id)).toMatchObject({ completed: ['P1'], next: 'P2', head }); +}); +it('refuses a suffix ID collision with the completed checkpoint prefix without saving it', () => { + const { store } = fixture(true); + store.recordCheckpoint(identity, state(store), { item: 'P1', completedItems: ['P1'], outOfScopePaths: ['outside'], baseEntries: context.baseEntries }); + const amended = store.getPlan(identity); amended.items[1]!.id = 'P1'; + expect(() => store.importRevision(JSON.stringify(amended), 'json', context, 1)).toThrow(/Duplicate item ID P1/); + expect(store.getPlan(identity).revision).toBe(1); +}); +it('validates saved amendments against the checkpoint tree and completed suffix', () => { + const path = join(directory(), 'state.sqlite'), store = open(path); + const initial: Plan = { schema_version: 1, revision: 1, issue: 1, summary: 'Example', questions: [], items: [ + { id: 'P1', title: 'Create output', intent: 'Create done.ts', files: [{ path: 'done.ts', kind: 'add', renamed_from: null, change: 'Create output' }], acceptance: [{ type: 'check', text: 'Done' }], depends_on: [] }, + { id: 'P2', title: 'Continue', intent: 'Create later.ts', files: [{ path: 'later.ts', kind: 'add', renamed_from: null, change: 'Create later output' }], acceptance: [{ type: 'check', text: 'Done' }], depends_on: ['P1'] }, + ] }; + store.createPlan(JSON.stringify(initial), 'json', { ...context, baseEntries: [] }, oid(1), oid(2)); + store.recordCheckpoint(identity, state(store), { + item: 'P1', completedItems: ['P1'], outOfScopePaths: ['extra.ts'], + baseEntries: [{ path: 'done.ts', kind: 'file' }, { path: 'extra.ts', kind: 'file' }], + }); + const amended = store.getPlan(identity); + amended.items[0]!.files.push({ path: 'extra.ts', kind: 'add', renamed_from: null, change: 'Declare the observed path' }); + expect(store.importRevision(JSON.stringify(amended), 'json', { + ...context, baseEntries: [{ path: 'done.ts', kind: 'file' }, { path: 'extra.ts', kind: 'file' }], + }, 1).revision).toBe(2); + expect(store.continuationProgress(identity)).toMatchObject({ completed: ['P1'], next: 'P2' }); +}); it('rejects duplicate source SHA mappings and rolls back every resulting ledger/snapshot write', () => { const { store } = fixture(); const before = store.getSnapshot(identity); expect(() => store.recordRebase(identity, state(store), oid(3), oid(5), [{ oldSha: oid(2), newSha: oid(4) }, { oldSha: oid(2), newSha: oid(5) }])).toThrow(); diff --git a/web/server.ts b/web/server.ts index 1628138c..07863c49 100644 --- a/web/server.ts +++ b/web/server.ts @@ -80,8 +80,8 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const store = service.store; const suggestionStore: SuggestionStore = { getPlan: identity => store.getPlan(identity), getSnapshot: identity => store.getSnapshot(identity), - beginSuggestions: (identity, expected, mode) => store.beginSuggestions(identity, expected, mode), - completeSuggestions: (identity, id, reply) => capability.run(() => store.completeSuggestions(identity, id, reply)), + beginSuggestions: (identity, expected, mode, continuation) => store.beginSuggestions(identity, expected, mode, continuation), + completeSuggestions: (identity, id, reply, candidatePlans) => capability.run(() => store.completeSuggestions(identity, id, reply, candidatePlans)), settleSuggestion: (identity, id, expected, outcome) => capability.run(() => store.settleSuggestion(identity, id, expected, outcome)), getSuggestions: (identity, id) => store.getSuggestions(identity, id), }; @@ -117,7 +117,14 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if (!executor || !progress) throw new GuardRefusal(config.demo ? RUNNER_NOT_IN_DEMO : RUNNER_NOT_CONFIGURED); // Shutdown began: answer 503 before any refusal, so nothing is recorded under the action ID and the UI may resend. if (!forView && runner!.closing) throw new ShuttingDownError(); - const task = service.store.getTask(identity), { started, begun, earlierCommits, completed, prefixHead, next } = progress; + const task = service.store.getTask(identity), { started, begun, earlierCommits } = progress; + // Safety evidence must settle before reconciliation can refuse on a stale prefix or head. + const safetyOwed = executor.owes(identity, { scope: false }); + const anyFindingOwed = executor.owes(identity); + const continuation = anyFindingOwed ? null : service.store.continuationProgress(identity); + const completed = continuation?.completed ?? progress.completed; + const prefixHead = continuation?.head ?? progress.prefixHead; + const next = continuation ? continuation.next : progress.next; // Checked in the order a person can act on: a closed task first, then what admission would refuse. if (task.status === 'merged' || task.status === 'cancelled') throw new GuardRefusal(`The task is ${task.status}.`); if (!runner!.runs('execute')) throw new GuardRefusal('The runner cannot run execute attempts yet.'); @@ -130,7 +137,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge // consume it and strand the task behind the resulting human gate. if (action === 'start' && task.requeuePending) throw new GuardRefusal('Recovery left this task to requeue; it cannot start until Resume resolves that claim.'); // A failed durable save must not let an ordinary refusal strand an in-memory safety finding until restart loses it. - if (executor.owes(identity, { scope: false })) return { fromItem: next ?? service.store.getPlan(identity).items[0]!.id, + if (safetyOwed) return { fromItem: next ?? service.store.getPlan(identity).items[0]!.id, claimRequeue: task.requeuePending, queue: false, owed: true }; const merge = service.store.getMergeAttempt(identity); const activeMerge = !!merge && (merge.state === 'submitting' || merge.state === 'queued'); @@ -145,12 +152,13 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge // Only the view stops here: the action lets admission refuse, because its refusal also moves the idle task to needs // human (the time-limit mapping), and nothing else would. if (forView && task.budgetDeadline !== null && task.budgetDeadline <= Date.now()) throw new GuardRefusal('The task time budget has run out; it needs a person.'); - if (service.store.latestCheckpoint(identity)) throw new GuardRefusal('The task paused for a scope amendment; continuing after one is not supported yet (#88).'); - // Commits an earlier revision's items made have not been reconciled with the revised plan (plan-format.md): rerunning - // the plan on top of them could redo or contradict that work, so it waits for #88 rather than guessing. Earlier - // attempts that committed nothing (failed, stopped, stale or unchanged) leave nothing to reconcile. - if (!begun && earlierCommits) throw new GuardRefusal('The plan was revised after items of it were committed; running a revised plan on top of those commits is not supported yet (#88).'); + if (continuation && !service.store.continuationApproved(identity, continuation)) + throw new GuardRefusal('Approve the amended plan continuation before resuming the task.'); + // Commits from another revision without a scope checkpoint have no audited continuation prefix to reconcile; rerunning + // the plan on top of them could redo or contradict that work. Earlier attempts that committed nothing leave none. + if (!continuation && !begun && earlierCommits) throw new GuardRefusal('The plan changed after runner commits without a scope checkpoint; those commits cannot be reconciled safely.'); if (action === 'start') { + if (continuation) throw new GuardRefusal('This task has a scope checkpoint; resume it after approving the amended continuation.'); // Point to resume wherever resume could run (a running task that ran any revision too); any other status is named as it is. const ran = begun || task.requeuePending; if (next !== null && ((ran && task.status === 'queued') || (task.status === 'running' && (started || task.requeuePending)))) @@ -194,8 +202,22 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const free = !!runner && !runner.closing && !status.unresolved && !runner.unreleased; // One progress read per poll, shared by both flags. const progress = free && executor ? executor.progress(identity) : undefined; + const checkpoint = service.store.latestCheckpoint(identity); + let continuation: Record | null = null; + if (checkpoint) { + try { + const current = service.store.continuationProgress(identity)!; + continuation = { checkpointId: checkpoint.id, item: checkpoint.item, completedItems: current.completed, + outOfScopePaths: checkpoint.outOfScopePaths, next: current.next, approved: service.store.continuationApproved(identity, current) }; + } catch (error) { + continuation = { checkpointId: checkpoint.id, item: checkpoint.item, completedItems: checkpoint.completedItems, + outOfScopePaths: checkpoint.outOfScopePaths, next: null, approved: false, + reason: error instanceof Error ? error.message : String(error) }; + } + } return { available: !!runner, task, attempts, startable: !!progress && offered('start', progress), resumable: !!progress && offered('resume', progress), - stateVersion: task.stateVersion, reviewVersion: service.store.reviewVersion(identity), retryable, stopRequested: status.stopRequested, unresolved: status.unresolved, publish: publishView(progress) }; + stateVersion: task.stateVersion, reviewVersion: service.store.reviewVersion(identity), retryable, stopRequested: status.stopRequested, + unresolved: status.unresolved, continuation, publish: publishView(progress) }; }; /** The task's publishing (#103): in progress, offered (what the publish action would run), and the last outcome. */ const publishView = (progress?: ReturnType) => { @@ -214,18 +236,18 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const runnerAction = (input: Record) => { const { action, attemptId, expectedStateVersion, expectedReviewVersion, actionId } = input; // Malformed requests are refused before userAction, so nothing is recorded under their action ID (HTTP 400). - if (!['cancel-attempt', 'retry', 'cancel-task', 'start', 'resume', 'publish', 'close-pull-requests'].includes(action as string)) throw new BadRequest('Unsupported runner action.'); + if (!['cancel-attempt', 'retry', 'cancel-task', 'start', 'resume', 'approve-continuation', 'publish', 'close-pull-requests'].includes(action as string)) throw new BadRequest('Unsupported runner action.'); if (!Number.isSafeInteger(expectedStateVersion)) throw new BadRequest('expectedStateVersion must be an integer.'); - if ((action === 'start' || action === 'resume') && !Number.isSafeInteger(expectedReviewVersion)) { + if ((action === 'start' || action === 'resume' || action === 'approve-continuation') && !Number.isSafeInteger(expectedReviewVersion)) { // Actions saved before #107 had no review version in their request hash. They remain replayable, but this shape // can never create a new action now: a miss falls through to the new-field validation below. const legacy = service.store.savedAction(identity, { actionId: actionId as string, kind: action as string, request: { attemptId, expectedStateVersion } }); if (legacy) return legacy.response; - throw new BadRequest('expectedReviewVersion must be an integer for start and resume.'); + throw new BadRequest('expectedReviewVersion must be an integer for start, resume and continuation approval.'); } if (action === 'cancel-attempt' || action === 'retry') assertUuidV4(attemptId, 'Attempt ID'); - const request = { attemptId, expectedStateVersion, ...((action === 'start' || action === 'resume') ? { expectedReviewVersion } : {}) }; + const request = { attemptId, expectedStateVersion, ...((action === 'start' || action === 'resume' || action === 'approve-continuation') ? { expectedReviewVersion } : {}) }; // A refused start or resume can still move the task to needs human (an expired budget is committed with the refusal), // and a task that moves there is owed a draft PR (#103). Only that move publishes: a person who pressed resume on a // task already in needs human asked for no publish. @@ -233,6 +255,26 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const before = service.store.getTask(identity).status; try { return act(); } finally { if (service.store.getTask(identity).status !== before) publishing?.actIfOwed(identity, { personAsked: true }); } } + // A final-item scope amendment can finish the plan without another runner attempt; its approval moves the task to + // running, after which the ordinary publishing gate can open the ready pull request. + if (action === 'approve-continuation') { + // A recovered run may already have left the task running. In that case approving removal of its interrupted last + // item does not change task status, but it does make a ready publish newly possible. Publish only for a newly + // committed action: a replay or refusal must not restart external work. + let committed = false; + try { + const result = actWithReplayState(); + committed = !result.replayed; + return result.response; + } finally { + if (committed) { + const task = service.store.getTask(identity); + const progress = task.status === 'running' ? service.store.continuationProgress(identity) : null; + if (progress?.next === null && service.store.continuationApproved(identity, progress)) + publishing?.actIfOwed(identity, { personAsked: true }); + } + } + } // A cancel that closed the task stops its publish in progress and closes its PRs (#111). A cancel that is still // stopping an attempt closes the task when the attempt settles; the run's end then closes them (afterRun). // Only this action's own cancel: a replayed or refused one (the task already closed) starts nothing. @@ -252,12 +294,34 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge catch (error) { if (!replay && error instanceof GuardRefusal && error.message === OWED_REFUSAL) publishing?.actIfOwed(identity, { personAsked: true }); throw error; } } return act(); - function act() { return service.store.userAction(identity, { actionId: actionId as string, kind: action as string, request }, () => { + function act() { return actWithReplayState().response; } + function actWithReplayState() { return service.store.userAction(identity, { actionId: actionId as string, kind: action as string, request }, () => { + // A saved replay is returned by userAction before this callback. A new continuation approval admitted before + // shutdown but parsed during the drain must remain retryable, even when no runner exists or its supplied versions + // are stale. + if (action === 'approve-continuation' && (stopping || runner?.closing)) throw new ShuttingDownError(); if (service.store.getTask(identity).stateVersion !== expectedStateVersion) throw new GuardRefusal('Stale task state. Reload before writing.'); - if ((action === 'start' || action === 'resume') && service.store.reviewVersion(identity) !== expectedReviewVersion) + if ((action === 'start' || action === 'resume' || action === 'approve-continuation') && service.store.reviewVersion(identity) !== expectedReviewVersion) throw new GuardRefusal('Stale review state. Reload before writing.'); if (action === 'cancel-task') return { outcome: runner ? runner.cancelTask(identity, expectedStateVersion as number, actionId as string) : service.store.cancelTask(identity, expectedStateVersion as number, actionId as string) }; if (!runner) throw new GuardRefusal(config.demo ? RUNNER_NOT_IN_DEMO : RUNNER_NOT_CONFIGURED); + if (action === 'approve-continuation') { + if (!executor || runner.isActive(identity) || executor.busy(identity) || publishing?.busy(identity)) + throw new GuardRefusal('The runner is busy or closing; continuation cannot be approved yet.'); + const progress = service.store.continuationProgress(identity); + if (!progress) throw new GuardRefusal('This task has no scope checkpoint.'); + if (executor.owes(identity)) throw new GuardRefusal('A scope or safety finding must settle before continuation approval.'); + const task = service.store.getTask(identity), merge = service.store.getMergeAttempt(identity); + if (!['needs amendment', 'queued', 'running'].includes(task.status) || task.cancelRequested !== null || + (merge && (merge.state === 'submitting' || merge.state === 'queued'))) + throw new GuardRefusal('The task status, cancellation or merge state prevents continuation approval.'); + const context = service.planContextAt(progress.head); + service.store.approveContinuation(identity, progress.checkpoint.id, { + revision: service.store.getPlan(identity).revision, snapshotId: service.store.getSnapshot(identity).id, + reviewVersion: expectedReviewVersion as number, + }, context); + return { outcome: 'approved', checkpointId: progress.checkpoint.id, next: progress.next }; + } if (action === 'start' || action === 'resume') { const choice = runChoice(action); // In this transaction with the admission: a refused admission rolls the move to queued back with it. @@ -297,7 +361,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge void runner.settled(identity).then(() => { if (service.store.getTask(identity).status === 'cancelled') publishing?.actIfOwed(identity); }) .catch(error => console.error(`Could not start closing pull requests: ${JSON.stringify(error instanceof Error ? error.message : String(error))}`)); return { outcome: 'started', attemptId: retry.id }; - }).response; } + }); } }; /** Handles of suggestion requests started by this process, removed once their outcome settles. */ const suggestionHandles = new Map(); @@ -344,7 +408,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge return service.store.userAction(identity, { actionId, kind, request }, () => { if (imported) { if (typeof input.source !== 'string' || !['json', 'yaml'].includes(input.format as string) || !Number.isSafeInteger(input.expectedRevision)) throw new BadRequest('source, format and expectedRevision are required.'); - return { revision: service.store.importRevision(input.source, input.format as 'json' | 'yaml', service.planContext(), input.expectedRevision as number).revision }; + return { revision: service.store.importRevision(input.source, input.format as 'json' | 'yaml', service.planContextForAmendment(), input.expectedRevision as number).revision }; } if (started) { // Like Ask and the runner, no new agent starts once shutdown began; 503 is not recorded, so the UI may resend. @@ -355,8 +419,10 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if (typeof input.feedback !== 'string' || input.feedback.length > 4000) throw new BadRequest('feedback must be text of 4000 characters or fewer.'); // Read above whenever the checks before this line pass; they cannot change across the synchronous action. if (!described) throw new GuardRefusal('Planning agent not available yet.'); - const context = service.planContext(); - const handle = suggestions.start({ context, revision: plan.revision, snapshotId: snapshot.id, issue: described.issue, approvedLessons: described.approvedLessons, feedback: input.feedback, + const amendment = service.planningContextForAmendment(), context = amendment.context; + const handle = suggestions.start({ context, completedItems: amendment.completedItems, continuationBinding: amendment.continuation, + continuationContext: amendment.continuationContext, + revision: plan.revision, snapshotId: snapshot.id, issue: described.issue, approvedLessons: described.approvedLessons, feedback: input.feedback, repo: { ...described.repo, baseSha: snapshot.base, paths: context.baseEntries.map(entry => entry.path) } }, mode); suggestionHandles.set(handle.id, handle); void handle.result.finally(() => suggestionHandles.delete(handle.id)); @@ -375,9 +441,9 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge service.store.cancelSuggestions(identity, id, 'Cancelled by the user.'); return { state: service.store.getSuggestions(identity, id).state }; } - if (mode === 'draft') return { revision: service.store.applyDraft(identity, id, service.planContext()).revision }; + if (mode === 'draft') return { revision: service.store.applyDraft(identity, id, service.planContextForAmendment()).revision }; if (!Number.isSafeInteger(input.index)) throw new BadRequest('index is required.'); - return { revision: service.store.applySuggestion(identity, id, input.index as number, service.planContext()).revision }; + return { revision: service.store.applySuggestion(identity, id, input.index as number, service.planContextForAmendment()).revision }; }).response; }; let stopping = false;