diff --git a/README.md b/README.md index a47b53a8..d1bc2f96 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Review agent-made Git changes one plan item at a time. The approved plan lists each item's files and acceptance checks; the review engine shows which item produced each change and flags foreign or overlapping work. -**Status:** the plan/linking library, SQLite store, and local review screen are implemented. Run `npm run demo` and open its private local URL. Ask runs Claude Code inside the locked-down agent container for read-only answers; choose it in Settings. Codex is refused in every phase for now, including Ask, because it reads code only by running commands ([why](docs/implementation/agent-isolation.md#codex-is-refused-in-every-phase)). Ask needs Docker, plus `CLAUDE_CODE_OAUTH_TOKEN` (from `claude setup-token`). The first question builds the agent image, which can take a few minutes. A configured GitHub review can merge only after the guarded exact-head gate passes. The agent container, vendor-only network and Claude/Codex adapters are implemented ([agent isolation](docs/implementation/agent-isolation.md)); only Ask uses them so far. Automated rebasing, plan command execution, and code-writing agents are not implemented. The paired human review experiment was cancelled before results were recorded and no longer blocks roadmap work; optional future validation is tracked in [#19](https://github.com/codeabovelab/codeboost/issues/19). +**Status:** the plan/linking library, SQLite store, local review screen, opt-in code-writing runner, durable local rebase, and exact-head plan command checks are implemented. Run `npm run demo` and open its private local URL. Ask and the runner use Claude Code inside locked-down containers; Codex is refused because it reads code only by running commands ([why](docs/implementation/agent-isolation.md#codex-is-refused-in-every-phase)). They need Docker, plus `CLAUDE_CODE_OAUTH_TOKEN` (from `claude setup-token`). Startup builds the agent image, which can take a few minutes. A configured GitHub review can merge only after the guarded exact-head gate passes. The agent container, vendor-only network, credential-free command-check profile, and Claude/Codex adapters are described in [agent isolation](docs/implementation/agent-isolation.md). Pushing a rebased head, waiting for its required checks, and handing that exact pair to guarded merge remain follow-up work. The paired human review experiment was cancelled before results were recorded and no longer blocks roadmap work; optional future validation is tracked in [#19](https://github.com/codeabovelab/codeboost/issues/19). ## Development @@ -14,7 +14,7 @@ npm run typecheck npm test ``` -Tests create disposable local repositories. They do not invoke agents, access GitHub, or execute plan acceptance commands. +Tests create disposable local repositories and exercise command checks in disposable credential-free containers. They do not invoke model providers or access GitHub. ## Guarded GitHub merge @@ -31,7 +31,7 @@ An existing-store configuration may add a trusted GitHub binding: } ``` -The issue must match the stored plan. `pullRequest` is required unless the configuration has a `runner` block (see below). The authenticated `gh` account must be able to read the pull request, issue timeline, applicable rulesets, and classic branch protection, and to merge the PR. Codeboost unions required checks from both rule sources, requires strict server-enforced current-base checks, rechecks the base and head immediately before merging, and passes the reviewed head to `gh pr merge --match-head-commit`. Missing permissions and ambiguous rule responses block the merge. When the branch uses a merge queue, codeboost adds the exact reviewed head to the queue and treats the merge as done only when GitHub confirms it merged; a queued pull request is not merged. If GitHub removes it from the queue while the plan, snapshot, review and head are unchanged, Merge offers a retry after the status refresh; if the head changed, it goes back to review first. A moved base and any unexecuted `cmd:` acceptance check remain blocked until [#22](https://github.com/codeabovelab/codeboost/issues/22) adds the runner path. +The issue must match the stored plan. `pullRequest` is required unless the configuration has a `runner` block (see below). The authenticated `gh` account must be able to read the pull request, issue timeline, applicable rulesets, and classic branch protection, and to merge the PR. Codeboost unions required checks from both rule sources, requires strict server-enforced current-base checks, rechecks the base and head immediately before merging, and passes the reviewed head to `gh pr merge --match-head-commit`. Missing permissions and ambiguous rule responses block the merge. When the branch uses a merge queue, codeboost adds the exact reviewed head to the queue and treats the merge as done only when GitHub confirms it merged; a queued pull request is not merged. If GitHub removes it from the queue while the plan, snapshot, review and head are unchanged, Merge offers a retry after the status refresh; if the head changed, it goes back to review first. With the opt-in runner, `prepare-merge` handles a moved base and runs every allowed `cmd:` acceptance check on the resulting exact head; merge remains blocked until that evidence is current. ## Runner (opt-in) @@ -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 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. +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. Before merge, `prepare-merge` refreshes the published PR's exact base/head, rebases onto a moved base, refreshes attribution and approvals, and runs allowed `cmd:` checks in a credential-free read-only container. The check result counts only for that exact head and command list. 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 @@ -87,8 +87,8 @@ Inputs such as `planText` and the ledger must come from the trusted runner. `run - The caller selects and trusts the repository and its Git administrative directory. Normal Git discovery, linked-worktree gitfiles, and symlinked gitdirs are supported; object-storage links and alternates inside that selected gitdir are rejected. This adapter is not a filesystem-containment boundary for untrusted repository roots. - Git administrative metadata and object storage must remain unchanged during a read; these library checks do not isolate a concurrently hostile filesystem. - Ownership uses line diffs, not semantic inference. Within one replacement block, new lines inherit all affected owners conservatively. Function context comes from Git hunk headers, not an AST. -- The importer requires accurate typed base entries, stable plan identity, a selected issue, and a trusted checkout path-identity function. It rejects path traversal, Git metadata paths, and traversal through a listed file/symlink/submodule. Runtime symlink and write-scope enforcement belong to the future container/runner; plan validation alone is not a sandbox. -- Allowed commands restrict accidents, not hostile programs or changed scripts. Parsing returns argv and never executes it. An unlisted valid command is a warning and must not run until allowed. -- Container isolation, vendor-only network access and credential handling are implemented by the lane D boundary (`agents/`), not by this library. Ask runs in that boundary in the read-only "questions" phase: it sees a clone of the reviewed head, supplied review context, and nothing else from your computer. Safe dependency installation is not implemented. +- The importer requires accurate typed base entries, stable plan identity, a selected issue, and a trusted checkout path-identity function. It rejects path traversal, Git metadata paths, and traversal through a listed file/symlink/submodule. The opt-in runner adds runtime symlink and write-scope enforcement; plan validation alone is not a sandbox. +- Allowed commands restrict accidents, not hostile programs or changed scripts. The plan library only parses them into argv. The opt-in runner executes an exact allowed argv only in its credential-free command-check container; an unlisted command never runs. +- Container isolation, vendor-only network access and credential handling are implemented by the lane D boundary (`agents/`), not by this library. Ask, code-writing runs, conflict resolution, and credential-free command checks use that boundary with phase-specific mounts and tools. Safe dependency installation is not implemented. See [implementation decisions and evidence](docs/implementation/build-step-1.md) and the [plan format](docs/plan-format.md). diff --git a/agents/adapters/runner.ts b/agents/adapters/runner.ts new file mode 100644 index 00000000..1b1581ba --- /dev/null +++ b/agents/adapters/runner.ts @@ -0,0 +1,27 @@ +import type { InvocationHandle } from '../contract.ts'; +import { readCapturedFile } from '../container/profile.ts'; +import { createPhasePolicy, createRunnerCommand, MAX_COMMAND_SCHEMA_BYTES } from '../policy.ts'; +import { launchInvocation } from './supervisor.ts'; +import { setUpProfile } from './setup.ts'; +import { assertAdapterRequest, capAdapterInvocationBudget, createAdapterInvocationBudget, + type AgentAdapterOptions, type AgentAdapterRequest } from './types.ts'; +import { join } from 'node:path'; + +/** Run exact approved argv arrays in the read-only review container, without a provider credential or external host. */ +export function startRunnerCommandInvocation(request: AgentAdapterRequest, + options: AgentAdapterOptions = {}): InvocationHandle { + if (request.invocation.vendor !== 'runner' || request.invocation.phase !== 'review') + throw new Error('Command checks require a runner-owned review invocation.'); + const policy = createPhasePolicy(request.invocation); + const remaining = options.invocationBudget + ? capAdapterInvocationBudget(options.invocationBudget, options.timeoutMs) + : createAdapterInvocationBudget(request.invocation, options.timeoutMs); + const raw = readCapturedFile(join(request.inputDirectory, 'schema.json'), 'Runner command input', MAX_COMMAND_SCHEMA_BYTES).content; + const commands = new TextDecoder('utf-8', { fatal: true }).decode(raw); + const command = createRunnerCommand(policy, commands); + assertAdapterRequest(request); + return launchInvocation(request.invocation, remaining, (signal, start) => setUpProfile(request, remaining, signal, + network => ({ ...request, policy, network, command }), + profile => start(profile, { ...options, processLifecycle: request.processLifecycle, invocationBudget: remaining, + diagnosticOutput: true }))); +} diff --git a/agents/adapters/supervisor.ts b/agents/adapters/supervisor.ts index 96560d0f..4c03f912 100644 --- a/agents/adapters/supervisor.ts +++ b/agents/adapters/supervisor.ts @@ -41,6 +41,8 @@ export interface SupervisorOptions { readonly secrets?: Readonly>; readonly timeoutMs?: number; readonly limits?: Partial; + /** Keep bounded, lossy diagnostics without letting command output change the process exit result. */ + readonly diagnosticOutput?: boolean; /** Trusted monotonic budget carried from adapter setup. */ readonly invocationBudget?: () => number; readonly decode?: (profile: ContainerProfile, rawStdout: Buffer, maximumBytes: number, @@ -480,6 +482,9 @@ export function startProfileInvocation(profile: ContainerProfile, options: Super throw new Error('invocationBudget cannot exceed the production ten-minute ceiling.'); if (options.processLifecycle && profile.deferredOutput) throw new Error('Durably tracked invocations cannot use concurrent deferred-output controls.'); + if (options.diagnosticOutput && (invocation.vendor !== 'runner' || invocation.phase !== 'review' + || options.decode || profile.deferredOutput)) + throw new Error('Lossy diagnostic output is reserved for runner command checks.'); } catch (error) { return rejectProfile(profile, error, true, disposeContainerProfile, isDeadlineError(error) ? 'timeout' : undefined, options.processLifecycle); @@ -498,6 +503,7 @@ export function startProfileInvocation(profile: ContainerProfile, options: Super const stdoutChunks = new ByteCollector(), stderrChunks = new ByteCollector(); let stdoutBytes = 0, stderrBytes = 0, combinedBytes = 0; + let outputTruncated = false; let stopReason: StopReason | undefined, failureDetail: string | undefined; let closed = false, terminating = false, settlementComplete = false; let decodedOutput: DecodedOutput | undefined, decodePromise: Promise | undefined; @@ -595,7 +601,8 @@ export function startProfileInvocation(profile: ContainerProfile, options: Super combinedBytes += retained.length; } if (chunk.length > available) { - if (final) stopReason ??= 'output-limit'; + if (options.diagnosticOutput) outputTruncated = true; + else if (final) stopReason ??= 'output-limit'; else stop('output-limit'); } }; @@ -777,20 +784,38 @@ export function startProfileInvocation(profile: ContainerProfile, options: Super }); } } - // Publish only strictly valid UTF-8: replacement characters would grow the result past the byte ceilings. - // Only output cut at a capture limit may end in an incomplete character, which the streaming decode then drops; - // otherwise the decode flushes, so a trailing lone lead byte fails closed. + // Agent answers publish only strictly valid UTF-8: replacement characters would grow the result past the byte + // ceilings. Runner command output is diagnostic rather than an answer, so it is decoded lossily and bounded again. + // Only strict output cut at a capture limit may end in an incomplete character, which streaming decode drops; + // otherwise strict decode flushes, so a trailing lone lead byte fails closed. const truncated = stopReason === 'output-limit'; const strictText = (value: Buffer) => { try { return new TextDecoder('utf-8', { fatal: true }).decode(value, truncated ? { stream: true } : undefined); } catch { return undefined; } }; - const stdoutText = strictText(finalStdout), stderrText = strictText(finalStderr); - if (stdoutText === undefined || stderrText === undefined) { - stopReason ??= 'capture-failure'; - failureDetail ??= 'Captured output is not valid UTF-8.'; + const boundedDiagnostic = (value: Buffer, maximum: number) => { + const encoded = Buffer.from(new TextDecoder('utf-8').decode(value)); + if (encoded.length <= maximum) return encoded; + return Buffer.from(new TextDecoder('utf-8', { fatal: true }).decode(encoded.subarray(0, maximum), { stream: true })); + }; + if (options.diagnosticOutput) { + finalStdout = boundedDiagnostic(finalStdout, Math.min(limits.stdoutBytes, limits.combinedBytes)); + finalStderr = boundedDiagnostic(finalStderr, + Math.max(0, Math.min(limits.stderrBytes, limits.combinedBytes - finalStdout.length))); + if (outputTruncated) { + const notice = Buffer.from('[codeboost: command output truncated]\n'); + const maximum = Math.max(0, Math.min(limits.stderrBytes, limits.combinedBytes - finalStdout.length)); + finalStderr = maximum <= notice.length ? notice.subarray(0, maximum) + : Buffer.concat([boundedDiagnostic(finalStderr, maximum - notice.length), notice]); + } + } else { + const stdoutText = strictText(finalStdout), stderrText = strictText(finalStderr); + if (stdoutText === undefined || stderrText === undefined) { + stopReason ??= 'capture-failure'; + failureDetail ??= 'Captured output is not valid UTF-8.'; + } + finalStdout = Buffer.from(stdoutText ?? ''); finalStderr = Buffer.from(stderrText ?? ''); } - finalStdout = Buffer.from(stdoutText ?? ''); finalStderr = Buffer.from(stderrText ?? ''); if (stopReason) finalStderr = withDiagnostic(finalStderr, finalStdout.length, stopReason, limits, failureDetail); const result = Object.freeze({ attemptId: invocation.attemptId, context: invocation.context, exitCode, signal: finalSignal, ...(stopReason ? { stopReason } : {}), diff --git a/agents/container/Dockerfile b/agents/container/Dockerfile index b1d197ac..bb19bdef 100644 --- a/agents/container/Dockerfile +++ b/agents/container/Dockerfile @@ -12,6 +12,7 @@ RUN npm install --global --allow-scripts=@anthropic-ai/claude-code \ && install --directory --owner=10001 --group=10001 --mode=0755 /work /work/.git COPY --chmod=0555 container/probe.sh /usr/local/bin/codeboost-container-probe +COPY --chmod=0555 container/command-check.mjs /usr/local/bin/codeboost-command-check COPY --chmod=0444 network/proxy.mjs /usr/local/lib/codeboost-egress-proxy.mjs LABEL org.opencontainers.image.base.name="docker.io/library/node:26.7.0-bookworm@sha256:e929171d35b9df7773a3ec5b068e387fa109441dc90f91e6560af5d39b7e9bf1" \ diff --git a/agents/container/command-check.mjs b/agents/container/command-check.mjs new file mode 100644 index 00000000..c9d3d2fa --- /dev/null +++ b/agents/container/command-check.mjs @@ -0,0 +1,29 @@ +import { openSync, readFileSync, closeSync, fstatSync, constants } from 'node:fs'; +import { spawnSync } from 'node:child_process'; + +const file = process.argv[2]; +if (!file || process.argv.length !== 3) process.exit(78); +let fd; +try { + fd = openSync(file, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + const stat = fstatSync(fd); + if (!stat.isFile() || stat.nlink !== 1 || stat.size > 64 * 1024) process.exit(78); + const commands = JSON.parse(readFileSync(fd, 'utf8')); + if (!Array.isArray(commands) || commands.length === 0 || commands.length > 100) process.exit(78); + for (const argv of commands) { + if (!Array.isArray(argv) || argv.length === 0 + || argv.some(arg => typeof arg !== 'string' || !arg.isWellFormed() || arg.includes('\0'))) process.exit(78); + const result = spawnSync(argv[0], argv.slice(1), { cwd: '/work', stdio: ['ignore', 'inherit', 'inherit'], shell: false }); + if (result.error) { + process.stderr.write(`codeboost command check could not start ${JSON.stringify(argv[0])}: ${JSON.stringify(result.error.message)}\n`); + process.exit(127); + } + if (result.signal) { + process.stderr.write(`codeboost command check ${JSON.stringify(argv[0])} ended by ${result.signal}\n`); + process.exit(1); + } + if (result.status !== 0) process.exit(result.status ?? 1); + } +} finally { + if (fd !== undefined) closeSync(fd); +} diff --git a/agents/container/probe.sh b/agents/container/probe.sh index 06a42900..c1f2b1c5 100644 --- a/agents/container/probe.sh +++ b/agents/container/probe.sh @@ -27,7 +27,8 @@ require_option / ro [ "${HOME:-}" = '/home/codeboost' ] || fail 'HOME must be the isolated home directory' [ "${CODEBOOST_PHASE:-}" != '' ] || fail 'phase is required' -[ "${CODEBOOST_VENDOR:-}" = 'codex' ] || [ "${CODEBOOST_VENDOR:-}" = 'claude' ] || fail 'vendor is required' +[ "${CODEBOOST_VENDOR:-}" = 'codex' ] || [ "${CODEBOOST_VENDOR:-}" = 'claude' ] \ + || [ "${CODEBOOST_VENDOR:-}" = 'runner' ] || fail 'vendor is required' [ "$(findmnt --noheadings --output FSTYPE --target /work)" = 'tmpfs' ] || fail '/work must use a bounded tmpfs task filesystem' [ "$(findmnt --noheadings --output FSTYPE --target /work/.git)" = 'tmpfs' ] || fail 'Git metadata must use a separate tmpfs filesystem' @@ -82,6 +83,10 @@ case "$CODEBOOST_VENDOR" in [ -n "${CLAUDE_CODE_OAUTH_TOKEN:-}" ] || fail 'Claude credential is missing' [ -z "${CODEX_HOME:-}" ] || fail 'Codex credential must not accompany Claude' ;; + runner) + [ -z "${CLAUDE_CODE_OAUTH_TOKEN:-}" ] || fail 'Claude credential must not accompany runner commands' + [ -z "${CODEX_HOME:-}" ] || fail 'Codex credential must not accompany runner commands' + ;; esac [ "$(git --version)" != '' ] || fail 'Git is unavailable' diff --git a/agents/container/profile.ts b/agents/container/profile.ts index 1beb1ba5..cefdcb72 100644 --- a/agents/container/profile.ts +++ b/agents/container/profile.ts @@ -17,7 +17,7 @@ export interface ContainerProfile { readonly args: readonly string[]; readonly expectedImage: string; readonly phase: Phase; - readonly vendor: 'claude' | 'codex'; + readonly vendor: InvocationInput['vendor']; readonly filesystems: TaskFilesystems; readonly inputDirectory: string; readonly codexAuthFile?: string; @@ -283,6 +283,8 @@ export async function createContainerProfile(options: ProfileOptions): Promise>; signal?: AbortSignal; processLifecycle?: ProcessGroupLifecycle } = {}) => options.processLifecycle @@ -361,7 +363,7 @@ export async function validateContainer(container: string, profile: ContainerPro || canonicalDockerBindSource(requestedAuth.Source) !== profile.codexAuthFile || canonicalDockerBindSource(auth!.Source) !== profile.codexAuthFile || !requestedAuth.ReadOnly)) throw new Error('Codex auth mount identity changed.'); - if (profile.vendor === 'claude' && auth) throw new Error('Claude profile must not mount Codex auth.'); + if (profile.vendor !== 'codex' && auth) throw new Error('Non-Codex profiles must not mount Codex auth.'); if (inspect.Config.Env.some(value => value.indexOf('=') < 1)) throw new Error('Container environment is malformed.'); const names = inspect.Config.Env.map(value => value.slice(0, value.indexOf('='))); const environment = new Map(inspect.Config.Env.map(value => [value.slice(0, value.indexOf('=')), value.slice(value.indexOf('=') + 1)])); @@ -370,7 +372,8 @@ export async function validateContainer(container: string, profile: ContainerPro 'CODEBOOST_WORK_BYTES', 'CODEBOOST_WORK_INODES', 'CODEBOOST_METADATA_BYTES', 'CODEBOOST_METADATA_INODES', 'npm_config_cache', 'XDG_CACHE_HOME', 'HTTPS_PROXY', 'HTTP_PROXY', 'NO_PROXY', ...(profile.deferredOutput ? ['CODEBOOST_DEFERRED_OUTPUT'] : []), - ...(profile.vendor === 'codex' ? ['CODEX_HOME'] : ['CLAUDE_CODE_OAUTH_TOKEN'])]); + ...(profile.vendor === 'codex' ? ['CODEX_HOME'] + : profile.vendor === 'claude' ? ['CLAUDE_CODE_OAUTH_TOKEN'] : [])]); if (new Set(names).size !== names.length || names.some(name => !allowedEnvironment.has(name))) throw new Error('Container includes an unexpected environment variable.'); if (environment.get('PATH') !== imageEnvironment.get('PATH') @@ -393,6 +396,8 @@ export async function validateContainer(container: string, profile: ContainerPro throw new Error('Credential profiles must not be combined or redirected.'); if (profile.vendor === 'claude' && (names.includes('CODEX_HOME') || !names.includes('CLAUDE_CODE_OAUTH_TOKEN'))) throw new Error('Credential profiles must not be combined.'); + if (profile.vendor === 'runner' && (names.includes('CODEX_HOME') || names.includes('CLAUDE_CODE_OAUTH_TOKEN'))) + throw new Error('Runner profile must not receive credentials.'); await assertContainerProfile(profile, remaining(), signal, processLifecycle); remaining(); } diff --git a/agents/contract.ts b/agents/contract.ts index b3df88f6..1d343300 100644 --- a/agents/contract.ts +++ b/agents/contract.ts @@ -21,7 +21,8 @@ export interface InvocationContext { export interface InvocationInput { readonly clone: TaskClone; readonly phase: Phase; - readonly vendor: 'claude' | 'codex'; + /** `runner` executes only an exact runner-approved argv; it receives no provider credential or external egress. */ + readonly vendor: 'claude' | 'codex' | 'runner'; readonly approvedArgv: readonly (readonly string[])[]; readonly deadline: number; readonly attemptId: string; @@ -108,7 +109,7 @@ export function assertCapturedInvocation(input: InvocationInput): void { export function captureInvocation(input: InvocationInput, now = Date.now()): InvocationInput { if (!input || !input.clone || !input.context) throw new Error('Missing invocation context.'); if (!['planning', 'questions', 'review', 'execute', 'fix'].includes(input.phase) - || !['claude', 'codex'].includes(input.vendor)) throw new Error('Unsupported invocation profile.'); + || !['claude', 'codex', 'runner'].includes(input.vendor)) throw new Error('Unsupported invocation profile.'); if (!isRunnerOwner(input.runnerOwner)) throw new Error('runnerOwner must be 32 lowercase hex characters.'); if (!nonempty(input.attemptId) || !nonempty(input.clone.id) || !nonempty(input.clone.taskId) || !nonempty(input.clone.directory) || !/^(?:[a-f0-9]{40}|[a-f0-9]{64})$/.test(input.clone.head)) diff --git a/agents/network/network.ts b/agents/network/network.ts index 25000339..9a6fd45e 100644 --- a/agents/network/network.ts +++ b/agents/network/network.ts @@ -10,6 +10,7 @@ import { runTrackedDocker, runTrackedProcess, type ProcessGroupLifecycle } from export const VENDOR_HOSTS = Object.freeze({ claude: Object.freeze(['api.anthropic.com']), codex: Object.freeze(['api.openai.com', 'chatgpt.com']), + runner: Object.freeze([]), } satisfies Record); export interface VendorNetwork { diff --git a/agents/network/proxy.mjs b/agents/network/proxy.mjs index a3a89754..3c619aa1 100644 --- a/agents/network/proxy.mjs +++ b/agents/network/proxy.mjs @@ -1,7 +1,8 @@ import { createServer, connect } from 'node:net'; -const allowed = new Set((process.env.CODEBOOST_ALLOWED_HOSTS ?? '').split(',').filter(Boolean)); -if (!allowed.size) throw new Error('CODEBOOST_ALLOWED_HOSTS is required.'); +if (!Object.hasOwn(process.env, 'CODEBOOST_ALLOWED_HOSTS')) throw new Error('CODEBOOST_ALLOWED_HOSTS is required.'); +// An explicitly empty allowlist is the credential-free runner profile: keep the proxy boundary up and refuse every CONNECT. +const allowed = new Set(process.env.CODEBOOST_ALLOWED_HOSTS.split(',').filter(Boolean)); // Ports are fixed in production; the overrides exist so tests can run the proxy against a local upstream. const listenPort = Number(process.env.CODEBOOST_PROXY_PORT ?? 3128); const upstreamPort = Number(process.env.CODEBOOST_UPSTREAM_PORT ?? 443); diff --git a/agents/policy.ts b/agents/policy.ts index 13e1ce98..9d6797e1 100644 --- a/agents/policy.ts +++ b/agents/policy.ts @@ -97,6 +97,26 @@ export function createClaudeCommand(policy: PhasePolicy, prompt: string, schema? '/run/codeboost-input', '--', prompt], schema); } +/** + * The command-check adapter runs this fixed image-owned dispatcher. The approved argv travel in the captured schema + * file, not in a process argument, and the profile proves that the mounted bytes are the bytes approved here. + */ +export function createRunnerCommand(policy: PhasePolicy, commands: string): AgentCommand { + if (assertPhasePolicy(policy).vendor !== 'runner' || policy.phase !== 'review') + throw new Error('Runner commands require the read-only review policy.'); + if (!commands || commands.includes('\0') || Buffer.byteLength(commands, 'utf8') > MAX_COMMAND_SCHEMA_BYTES) + throw new Error('Runner command input must be bounded JSON without NUL.'); + let parsed: unknown; + try { parsed = JSON.parse(commands); } catch { throw new Error('Runner command input must be JSON.'); } + if (!Array.isArray(parsed) || parsed.length === 0 || parsed.length > 100 || parsed.some(argv => !Array.isArray(argv) + || argv.length === 0 || argv.some(arg => typeof arg !== 'string' || !arg.isWellFormed() || arg.includes('\0')))) + throw new Error('Runner command input must contain complete literal argv arrays.'); + const invocation = assertPhasePolicy(policy); + for (const argv of parsed as string[][]) if (!permitsCommand(invocation, argv)) + throw new Error('Runner command was not approved exactly for this invocation.'); + return command(policy, ['node', '/usr/local/bin/codeboost-command-check', '/run/codeboost-input/schema.json'], commands); +} + /** * A planning schema must describe a JSON object (Claude returns `structured_output` as an object) and fit one command * argument. Returns the schema unchanged. @@ -143,6 +163,7 @@ export function createCodexCommand(policy: PhasePolicy, prompt: string): AgentCo export type IsolationProbe = 'noop' | 'phase-worktree' | 'read-only-isolation' | 'persist-write' | 'persist-read' | 'capacity' | 'metadata' | 'must-not-run' | 'input-marker' | 'finite-output' + | 'finite-large-output' | 'finite-multibyte-output' | 'infinite-stdout' | 'infinite-stderr' | 'infinite-mixed' | 'ignore-term' | 'symlink-output' | 'oversized-output' | 'fifo-output' | 'invalid-utf8-output' | 'invalid-utf8-stderr' | 'truncated-utf8-stderr' | 'replace-output-directory' @@ -183,6 +204,8 @@ export function createIsolationProbeCommand(policy: PhasePolicy, probe: Isolatio 'input-marker': 'set -eu; grep -q codeboost-schema-marker /run/codeboost-input/schema.json; ' + 'test ! -e /run/codeboost-input/extra.json', 'finite-output': 'printf stdout-marker; printf stderr-marker >&2', + 'finite-large-output': "head -c 131072 /dev/zero | tr '\\0' x; head -c 65536 /dev/zero | tr '\\0' y >&2", + 'finite-multibyte-output': "i=0; while test \"$i\" -lt 2000; do printf '\\342\\202\\254' >&2; i=$((i+1)); done", 'invalid-utf8-stderr': "printf 'bad-\\377\\377-stderr' >&2", 'truncated-utf8-stderr': "printf 'cut-\\342' >&2", 'infinite-stdout': "while :; do head -c 4096 /dev/zero | tr '\\0' x; done", diff --git a/core/plan.ts b/core/plan.ts index 496036ba..ec202194 100644 --- a/core/plan.ts +++ b/core/plan.ts @@ -80,7 +80,7 @@ export function isRepoPath(path: string): boolean { /** Small literal-argv grammar, deliberately not a shell parser. Never executes. */ export function commandArgv(command: string): string[] { - if (/[\\\p{Cc}]/u.test(command)) + if (!command.isWellFormed() || /[\\\p{Cc}]/u.test(command)) fail('command-syntax', 'Commands must contain literal arguments, not shell syntax.'); const argv: string[] = []; let word = '', quote = '', started = false; diff --git a/docs/architecture.md b/docs/architecture.md index 0c003a60..a107e91d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -42,9 +42,9 @@ Writing standard: plain language, ISO 24495-1:2023 - **Build state:** - **In use:** plan and linking library, SQLite store, review screen, plan display/import/authoring 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). After an out-of-scope pause, an approved amended plan can reconcile the completed prefix and resume only the unfinished suffix (#88, PR #134). 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. - - **Partly built for pre-merge automation:** the trusted local rebase engine records one-to-one commit mappings, verifies the resulting checkout byte for byte, owns its subprocesses durably, and recovers interrupted work (#22, PR #136). Its production conflict path gives a sandboxed agent only the bounded conflict files and imports only audited results for both foreign and plan-owned commits (PRs #138, #140 and #144). No production request invokes the rebaser yet. - - **Not built:** screens for the runner actions, production rebase admission, post-rebase attribution and approval refresh, head-bound `cmd:` checks, remote push/check refresh, review rounds, and the Queue and Learning screens. + - **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). After an out-of-scope pause, an approved amended plan can reconcile the completed prefix and resume only the unfinished suffix (#88, PR #134). When a run ends, codeboost publishes the task's pull request, a draft if the task needs a person (#103). `prepare-merge` refreshes the published PR's exact base/head, rebases through the durable F3/F4 path, recomputes attribution and approvals, and runs allowlisted `cmd:` checks in a credential-free read-only container bound to the resulting head (#22). 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. + - **Partly built for pre-merge automation:** the trusted local rebase engine records one-to-one commit mappings, verifies the resulting checkout byte for byte, owns its subprocesses durably, and recovers interrupted work (#22, PR #136). Its production conflict path gives a sandboxed agent only the bounded conflict files and imports only audited results for both foreign and plan-owned commits (PRs #138, #140 and #144). `prepare-merge` now invokes that path after current pull-request base/head reads, then refreshes attribution and approvals and runs exact-head `cmd:` checks; required-check refresh, the already-fixed rerun, rewritten-head push, and guarded merge handoff remain for F6. + - **Not built:** screens for the runner actions, remote push/check refresh, review rounds, and the Queue and Learning screens. ## Terms used @@ -170,7 +170,7 @@ Dependencies point downward only. `core/` imports nothing that does input or out | Merge coordinator | `runner/merge.ts` | Runs the full merge gate twice, re-reads the local generation, then merges the exact reviewed head. Tracks merge-queue attempts. For a runner task it inspects and merges the task's own published pull request; a configured `github.pullRequest` that differs is refused (#121). | In use | | Planning agent | `runner/planning.ts`, `web/planning.ts`, `runner/planning-provider.ts` | Runs plan suggestions and drafts with Claude in lane D's planning phase, on a read-only copy of the current head, in its own worker with its own leftovers ledger and owner token. The request and its timer share one 10-minute budget. Issue text includes current collaborators' comments, or every comment under explicit author-bound trust. | In use for a non-demo review with a `github` block | | Runner coordinator | `runner/coordinator.ts`, `runner/lifecycle.ts` | Attempt admission, concurrency slots, compare-and-swap on results, retries, shutdown order. A stop made inside a user action takes effect only after that action's transaction commits (`Store.afterCommit`, #96). | In use with the opt-in `runner` block (#91) | -| Execution | `runner/execution.ts` | Runs plan items in order: fresh workspace, prompt, agent, post-run audit, then the runner's own commit and ledger entry. Before launch, D's tree check (`checkTaskTree`) must pass. Start and resume require current approvals, own the review version across the run, recheck approvals before later items, and verify that a completed prefix still ends at the current head (#107). Pauses in needs amendment on an out-of-scope edit. A safety violation is saved before its terminal write and remains durably owed when a human gate delays escalation; a failed run is audited too. | In use with the opt-in `runner` block; started by `start` and `resume` on `/api/runner` (#91, #107, PRs #105 and #130) | +| Execution | `runner/execution.ts`, `runner/pre-merge.ts`, `runner/checks.ts` | Runs plan items in order: fresh workspace, prompt, agent, post-run audit, then the runner's own commit and ledger entry. Before launch, D's tree check (`checkTaskTree`) must pass. Start and resume require current approvals, own the review version across the run, recheck approvals before later items, and verify that a completed prefix still ends at the current head (#107). Pre-merge preparation refreshes and rebases the exact PR pair, then records credential-free command-check evidence only for the checked head and exact argv digest. Pauses in needs amendment on an out-of-scope edit. A safety violation is saved before its terminal write and remains durably owed when a human gate delays escalation; a failed run is audited too. | In use with the opt-in `runner` block; started by `start`, `resume`, and `prepare-merge` on `/api/runner` (#22, #91, #107, PRs #105 and #130) | | Workspace | `runner/workspace.ts`, `runner/runner-repository.ts` | The real lane D workspace. A runner-owned bare repository (owner-only directories) fetches base commits by ID and takes in each verified commit bundle under a per-attempt ref. Review and Ask read a task with runner commits from there, at the head the Store recorded. | In use with the opt-in `runner` block (#91) | | Diagnostics | `runner/diagnostics.ts` | Saves the partial diff of a writable attempt that did not complete, owner-only, and keeps the folder under a size cap (256 MiB by default). | In use with the opt-in `runner` block (#91) | | Publishing | `runner/publish.ts`, `runner/publishing.ts`, `runner/branch-push.ts` | Runs the already-fixed check, pushes the task head, then opens or reuses the task's pull request (a draft when the task needs a person). Records each opening before calling GitHub, so a lost outcome can be found again by a marker. `GitBranchPusher` pushes from the runner-owned repository through `gh auth git-credential`, only to `codeboost/` branches, and overwrites only a branch head the task's ledger owns (`--force-with-lease`). `TaskPublishing` (#103) publishes when a run ends, on the `publish` runner action, and once at startup, and records the last outcome. On cancel it closes the task's PRs (#111). | In use with the opt-in `runner` block (needs `github.baseBranch`); never in a demo | @@ -410,7 +410,7 @@ These are the decisions that shape the structure. The full list, with the reason | C, K | Guarded merge, merge queue | Done | | D | Agent isolation boundary | Done. Since the isolation gate: bounded diff export (#76), task-change inspection and the runner's commit as a bounded `git bundle` (#66, PRs #85 and #90), owner-scoped recovery for Ask (#95), the plan schema for Claude (#97), gitlink mounts and the pre-launch tree check (#81, #99, PR #100), and Codex refused in every phase (#93, PR #106). The runner-contract follow-ups (#51) are closed: the last one tests export and removal through a recovery handle after a real restart (PR #109). | | E | Planning library and suggestions | Done. E4 (PR #45) added the acceptance fixtures and recorded Claude output that replays through validation and the Store. | -| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). Next in #22: F5 owns current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6 owns rewritten-head push, fresh required-check refresh, the already-fixed rerun, and exact-pair guarded-merge handoff. The integrated flow pushes a rewritten head before it recomputes review state and reruns every plan item's head-bound `cmd:` checks; fresh required GitHub checks follow. | +| F | Runner | Merged: F1 (lifecycle store, coordinator, shutdown wiring, startup recovery and the lock, planning API, feedback events, and stops that wait for their commit: #53, #74, #57, #59, #60, #96); F2a–F2b (execute prompts, audit, per-item execution, real workspace, durable safety findings: #67, #68, #92, #94, #128); F2d (already-fixed check, PR opening, branch pusher, production publishing, closing PRs on cancel: #84, #101, #110, #115, #116, #119, #123); production wiring and `start`/`resume` (#91, PRs #102, #105); planning through lane D (#117, #124, PRs #118, #120, #125); merging the task's own PR (#121, PR #126); start/resume approval, review-version, recovery and completed-prefix guards (#107, PR #130); continuation after a scope pause (#88, PR #134); the F3 durable trusted local-rebase foundation (#22, PR #136); the F4 foreign-conflict engine plus production sandbox adapter (#22, PRs #138 and #140); and sandboxed owned-commit conflict resolution (#22, PR #144). This branch adds F5: current base/head refresh, rebase admission, post-rebase attribution and approval refresh, and head-bound `cmd:` execution. F6 owns rewritten-head push, fresh required-check refresh, the already-fixed rerun, and exact-pair guarded-merge handoff. | | Hardening | Allowlisted subprocess environments | Done: every Git command (#82, #83) and every `gh` runner (#84, #86). | | Tests | Test suite reliability | Done. The full suite runs ordinary tests first and Docker-backed files serially; CI and the real-Docker gate passed on PR #132 (#129). | | G | Planning screen | Done (#145). The screen displays/imports the current plan, requests and renders a next-revision draft or suggestion cards, and applies results with revision-bound exact ambiguous replay. G4 composes the browser with the production planning setup, D-backed provider boundary, F endpoints and SQLite persistence; focused adapter, recorded-output and exact-head real-Docker planning suites cover the lower boundary. | @@ -419,10 +419,10 @@ These are the decisions that shape the structure. The full list, with the reason ## Known limits -- The trusted rebase and sandboxed conflict machinery is built, but no production request invokes it yet. Until F5 and F6 of #22 land, the current merge flow still refuses a moved base or non-linear history instead of rebasing automatically. +- `prepare-merge` invokes the trusted rebase and sandboxed conflict machinery, refreshes the review, and runs head-bound `cmd:` checks. Until F6 of #22 lands, it does not push a rewritten head, wait for fresh required checks, rerun the already-fixed check, or hand the exact pair to guarded merge. - Linking is line-based. It cannot see an unrelated edit inside a declared file; only the review agent and you can catch that. - The container's guarantees are those of Docker. The agent can always reach its own vendor account. -- `cmd:` acceptance checks do not run yet, so any plan item with a `cmd:` check blocks merging. +- `cmd:` acceptance checks run during `prepare-merge`, and their passing evidence is bound to the exact reviewed head. A plan item without current passing evidence still blocks merging. - One user, one machine, one runner per database. The lock refuses a second runner, and it refuses network filesystems, where OS locks are unreliable. - The runner is opt-in: it needs a `runner` block and a `github` block in `review.json`, and it is always off in the demo. It runs through the API only; the screen has no runner start, resume or publish controls yet. Planning authoring, suggestions and revision-bound Apply are available when the planning provider is configured. Publishing also needs `github.baseBranch`. - codeboost is Claude-only. Codex is refused in every phase (#93, PR #106), because it can read files only through its shell, which every phase turns off. `agent-isolation.md` says when to revisit this. diff --git a/docs/designs/codeboost-plan-indexed-review.md b/docs/designs/codeboost-plan-indexed-review.md index 7fa45989..5cb53ca8 100644 --- a/docs/designs/codeboost-plan-indexed-review.md +++ b/docs/designs/codeboost-plan-indexed-review.md @@ -25,9 +25,9 @@ Last checked against the code: 2026-09-26 (see "Lane status" under "Parallel bui - **What makes it different.** You review the PR one **plan item** at a time. Pick a plan item on the left and see only its code on the right. Code that belongs to no plan item is flagged in a red "Unplanned changes" row. - **Why that matters.** Other tools make you read a raw diff and guess what the agent meant. In codeboost, the plan you approved is the index to the code. - **How it stays trustworthy.** codeboost records commits in a trusted ledger with either an owning plan item or an explicit foreign/unowned classification. Rewriting a foreign commit never turns it into owned work. It also checks each change against the files the plan item said it would touch. One blind spot remains: an unrelated edit inside a file the plan item declared is caught only by the review agent and by you. -- **How it stays safe.** Agents run inside a container that holds only the task's code and the agent's own sign-in, so your other files and credentials are not there. Ask, the only agent codeboost runs today, uses this container too. codeboost needs your approval before its own dependency installation or invocation of changed scripts; containment must also cover commands the agent already ran. +- **How it stays safe.** Agents run inside a container that holds only the task's code and the credential for that phase, so your other files and credentials are not there. Ask, code-writing runs, and conflict resolution use provider-specific containers; exact-head `cmd:` checks use a credential-free read-only profile. codeboost needs your approval before its own dependency installation or invocation of changed scripts; containment must also cover commands the agent already ran. - **It learns from you.** After each task, codeboost turns your feedback into short lessons. You approve each lesson before agents use it, and a Learning screen shows whether you are repeating yourself less. -- **Where the build is.** Built: the plan and linking library, the SQLite store, the review screen with Ask and change requests, the guarded merge gate with merge-queue support, and the agent isolation boundary (containers, vendor-only network, Claude and Codex adapters). Ask already runs in that boundary. Not built yet: the runner that uses it for code-writing tasks, rebasing, `cmd:` execution, and the Planning, Queue, Lessons and Learning screens (the ranked Issues screen is built). Optional real-PR validation is tracked separately in #19 and is not a prerequisite. +- **Where the build is.** Built: the plan and linking library, SQLite store, review screen with Ask and change requests, guarded merge gate with merge-queue support, agent isolation boundary, opt-in code-writing runner and publishing, durable local rebase/conflict resolution, and pre-merge base/head refresh with exact-head `cmd:` checks. Still deferred: pushing the rewritten head, waiting for its required checks, handing that exact pair to guarded merge, and the Planning, Queue, Lessons and Learning screens (the ranked Issues screen is built). Optional real-PR validation is tracked separately in #19 and is not a prerequisite. ## Terms used diff --git a/docs/implementation/runner-lifecycle.md b/docs/implementation/runner-lifecycle.md index 01d5f7b9..9b418201 100644 --- a/docs/implementation/runner-lifecycle.md +++ b/docs/implementation/runner-lifecycle.md @@ -320,7 +320,9 @@ This runs before the coordinator opens. 7. **Unowned preparation check.** If any attempt has `preparation_started_at` set and `preparation_pgid` null (see "Launch"), stop here: do not open the coordinator, name each attempt and directory, and wait for `--release-preparation`. Step 4 does not remove those directories. 8. Open the coordinator. -**Production wiring (#91).** The runner is opt-in: the review configuration's `runner` block names its root, an optional diagnostics directory and cap, the committer and optional storage limits (`runner/production.ts`, `parseRunnerConfig`). Without the block, or in a demo, there is no coordinator and runner actions answer that the runner is not configured. With it, `web/cli.ts` takes the lock, `startServer` opens the `Store` and then, before it listens, runs the planning setup when planning is on (`web/planning.ts`: verify the lock, then fail every planning request an earlier process left `pending`, #124). That step runs with or without a runner block: every start takes the lock, and planning is on for any non-demo review with a `github` block. Then it runs `setUpRunner`: it checks for the github block (the prompt carries the issue) and `CLAUDE_CODE_OAUTH_TOKEN`, calls the lock's `verify()` (decision 1, step 4), reads the runner token for the locked file, and runs `recoverStartup` with D's own `recoverLeftovers`, `exportTaskDiff` and `removeTaskFilesystemsAsync`. The agent image is built after D's recovery has stopped every leftover agent, so no agent keeps writing to its storage during a long build, and before the first export's deadline starts (the build blocks the event loop, so a deadline armed before it would expire as soon as it returned). Any failure closes the `Store`, releases the lock and exits with its message. To export a recovered storage, F saves the storage's `metadataBaseline` and the commit it was seeded from (`attempts.metadata_baseline`, `attempts.storage_base`, schema v9) as soon as D's allocation returns; a row without them gets "Partial output could not be exported: its storage baseline was never saved". Diagnostics are kept per database, in `//diagnostics`: retention reads references only from its own `Store`, so runners of databases sharing a directory must never see each other's files. Recovered diffs are saved through `saveDiagnostic`, with the same owner-only file and retention as the live path; retention keeps every diff this recovery saved, because none is referenced until the finalization transaction. A diff D cut at its byte limit ends with a `codeboost:` line that says so, on this path and the live one. The issue text a prompt carries (title, body and collaborators' comments) is read once and reused for later items for up to five minutes; the base tree listing is read once per base commit. A first Ctrl+C during startup waits for startup to finish; a second stops the process at once, which recovery treats like a crash at the next start. The deps accept execute attempts only (`RunnerDeps.kinds`): admission refuses any other kind before it writes anything. A plan item runs only through `ItemExecutor`, so `/api/runner` refuses to retry an execute attempt on its own; a task's plan runs again through `resume` (below). +**Production wiring (#91, #22).** The runner is opt-in: the review configuration's `runner` block names its root, an optional diagnostics directory and cap, the committer and optional storage limits (`runner/production.ts`, `parseRunnerConfig`). Without the block, or in a demo, there is no coordinator and runner actions answer that the runner is not configured. With it, `web/cli.ts` takes the lock, `startServer` opens the `Store` and then, before it listens, runs the planning setup when planning is on (`web/planning.ts`: verify the lock, then fail every planning request an earlier process left `pending`, #124). That step runs with or without a runner block: every start takes the lock, and planning is on for any non-demo review with a `github` block. Then it runs `setUpRunner`: it checks for the github block (the prompt carries the issue) and `CLAUDE_CODE_OAUTH_TOKEN`, calls the lock's `verify()` (decision 1, step 4), reads the runner token for the locked file, and runs `recoverStartup` with D's own `recoverLeftovers`, `exportTaskDiff` and `removeTaskFilesystemsAsync`. The agent image is built after D's recovery has stopped every leftover agent, so no agent keeps writing to its storage during a long build, and before the first export's deadline starts (the build blocks the event loop, so a deadline armed before it would expire as soon as it returned). Any failure closes the `Store`, releases the lock and exits with its message. To export a recovered storage, F saves the storage's `metadataBaseline` and the commit it was seeded from (`attempts.metadata_baseline`, `attempts.storage_base`, schema v9) as soon as D's allocation returns; a row without them gets "Partial output could not be exported: its storage baseline was never saved". Diagnostics are kept per database, in `//diagnostics`: retention reads references only from its own `Store`, so runners of databases sharing a directory must never see each other's files. Recovered diffs are saved through `saveDiagnostic`, with the same owner-only file and retention as the live path; retention keeps every diff this recovery saved, because none is referenced until the finalization transaction. A diff D cut at its byte limit ends with a `codeboost:` line that says so, on this path and the live one. The issue text a prompt carries (title, body and collaborators' comments) is read once and reused for later items for up to five minutes; the base tree listing is read once per base commit. A first Ctrl+C during startup waits for startup to finish; a second stops the process at once, which recovery treats like a crash at the next start. The composed deps accept `execute` and credential-free `check` attempts (`RunnerDeps.kinds`); admission refuses every other kind before it writes anything. A plan item runs only through `ItemExecutor`, so `/api/runner` refuses to retry an execute attempt on its own; a task's plan runs again through `resume` (below). + +**F5 production extension (#22).** The production dependency set also accepts `check` attempts. `prepare-merge` resolves the same task-owned pull request as merge admission, fetches its exact base/head without moving a runner ref, and stops for refreshed review when the head moved. A moved base is handled through the durable F3/F4 rebase marker and mapping. Attribution and approvals are recomputed before each allowed `cmd:` list runs sequentially in a fresh read-only workspace at the exact resulting head. The fixed image dispatcher reads bounded JSON argv arrays from the input mount, invokes them without a shell, and receives no provider credential or allowed external host. Its completed result records both the head and argv digest; failure, cancellation, interruption, a changed head, or changed argv does not count. A final fresh remote read turns movement during preparation into `review-required`, never ready. The saved user-action replay is settled with the background result even during shutdown; startup marks an interrupted preparation failed after resource recovery, so it never replays `preparing` forever. `GET /api/runner` exposes the active and last in-memory pre-merge result without rebuilding history during polling. ## HTTP and UI contract diff --git a/runner/branch-push.ts b/runner/branch-push.ts index d513bcea..32fccb48 100644 --- a/runner/branch-push.ts +++ b/runner/branch-push.ts @@ -106,6 +106,23 @@ export class GitBranchPusher implements BranchPusher { this.#config = config; this.#url = url; } + /** Fetch exact remote commits and keep them reachable while durable review snapshots may still name them. */ + async fetchCommits(commits: readonly string[], signal?: AbortSignal): Promise { + if (!commits.length || commits.length > 2 || commits.some(commit => !COMMIT_ID.test(commit))) + throw new Error('One or two full commit IDs are required for refresh.'); + const unique = [...new Set(commits)]; + await this.#git(['fetch', '--no-tags', '--no-write-fetch-head', '--no-recurse-submodules', '--', this.#url, ...unique], signal, true); + const found = await this.#git(['cat-file', '--batch-check=%(objectname) %(objecttype)'], signal, false, + Buffer.from(`${unique.join('\n')}\n`)); + const records = found.split('\n'); + if (records.length !== unique.length || records.some((record, index) => record !== `${unique[index]} commit`)) + throw new Error('The refreshed remote base or head is not a commit in the runner repository.'); + // A raw object-ID fetch creates no ref. Without these runner-owned anchors, `git gc` may prune a collaborator-only + // head after its SHA has been persisted in a snapshot, leaving review and restart recovery unable to read it. + for (const commit of unique) + await this.#git(['update-ref', `refs/codeboost/remote-commits/${commit}`, commit], signal); + } + async push(identity: PlanIdentity, input: { head: string; branch: string; beforePush?: MutationBoundary }, signal?: AbortSignal): Promise { signal?.throwIfAborted(); if (identity.repositoryId !== this.#config.repositoryId) throw new Error('The task belongs to another repository.'); diff --git a/runner/checks.ts b/runner/checks.ts new file mode 100644 index 00000000..32219b24 --- /dev/null +++ b/runner/checks.ts @@ -0,0 +1,51 @@ +import { createHash } from 'node:crypto'; +import type { InvocationHandle, InvocationInput } from '../agents/contract.ts'; +import { commandAllowed, commandArgv } from '../core/plan.ts'; +import { findIdentity, type TaskWorkspace, type WorkspaceRef } from './execution.ts'; +import type { PreparedAttempt, RunnerDeps } from './coordinator.ts'; +import type { AttemptRecord, Store } from './store.ts'; + +export interface CommandCheckResult { head: string; commandsDigest: string; passed: true } +interface Private { workspace: WorkspaceRef; head: string; commands: readonly (readonly string[])[]; commandsDigest: string } +export type RunnerCommandLauncher = (input: InvocationInput, commands: string, workspace: WorkspaceRef) => InvocationHandle; + +export function commandDigest(commands: readonly (readonly string[])[]): string { + if (commands.some(argv => argv.some(argument => !argument.isWellFormed()))) + throw new Error('Command arguments must be well-formed Unicode.'); + return createHash('sha256').update(JSON.stringify(commands)).digest('hex'); +} + +/** Read-only, head-bound `cmd:` attempts. A non-zero command exit is a failed attempt and never passing evidence. */ +export function commandCheckDeps(store: Store, workspace: TaskWorkspace, + launch: RunnerCommandLauncher, allowedCommands: readonly (readonly string[])[], + runnerOwner: string): RunnerDeps { + const allowed = allowedCommands.map(argv => [...argv]); + return { + runnerOwner, kinds: ['check'], + async prepare(attempt, signal) { + if (attempt.kind !== 'check' || !attempt.item) throw new Error('Command-check deps run one plan item.'); + const identity = findIdentity(store, attempt), plan = store.getPlan(identity, attempt.context.planRevision); + const item = plan.items.find(candidate => candidate.id === attempt.item); + if (!item) throw new Error('Unknown command-check item.'); + const commands = item.acceptance.filter(check => check.type === 'cmd').map(check => commandArgv(check.text)); + if (!commands.length) throw new Error('This plan item has no command checks.'); + if (commands.some(argv => !commandAllowed(argv, allowed))) throw new Error('A command check is not approved in allowedCommands.'); + const head = store.getSnapshot(identity, attempt.context.snapshotId).head; + const materialized = await workspace.materialize(attempt, head, signal); + return { clone: materialized.clone, vendor: 'runner', approvedArgv: commands, + private: { workspace: materialized, head, commands, commandsDigest: commandDigest(commands) } satisfies Private }; + }, + async cleanupPreparation(attempt) { await workspace.cleanupPreparation?.(attempt); }, + start(input, prepared) { + const data = prepared.private as Private; + return launch(input, JSON.stringify(data.commands), data.workspace); + }, + validate() { throw new Error('Command checks publish through finish().'); }, + async finish(attempt, result, prepared) { + if (result.exitCode !== 0 || result.stopReason) throw new Error('Command checks did not pass.'); + const data = prepared.private as Private; + return { value: { head: data.head, commandsDigest: data.commandsDigest, passed: true } satisfies CommandCheckResult }; + }, + async release(_attempt, prepared) { await workspace.release?.((prepared.private as Private).workspace); }, + }; +} diff --git a/runner/coordinator.ts b/runner/coordinator.ts index 84c44b7b..858b457b 100644 --- a/runner/coordinator.ts +++ b/runner/coordinator.ts @@ -6,7 +6,7 @@ import { ATTEMPT_PHASES, GuardRefusal, ShuttingDownError, WRITABLE_KINDS, bounde /** What F's host-side preparation hands to D's start call. */ export interface PreparedAttempt { readonly clone: TaskClone; - readonly vendor: 'claude' | 'codex'; + readonly vendor: InvocationInput['vendor']; readonly approvedArgv: readonly (readonly string[])[]; /** Opaque data the deps keep for their own finish/release steps (for example the task workspace). */ readonly private?: unknown; @@ -77,16 +77,69 @@ export interface RunnerDeps { release?(attempt: AttemptRecord, prepared: PreparedAttempt): Promise; now?(): number; } + +/** Compose kind-specific runner dependencies while preserving each implementation's private prepared value. */ +export function combineRunnerDeps(...delegates: readonly RunnerDeps[]): RunnerDeps { + if (!delegates.length) throw new Error('At least one runner dependency set is required.'); + const runnerOwner = delegates[0]!.runnerOwner; + if (delegates.some(delegate => delegate.runnerOwner !== runnerOwner || !delegate.kinds?.length)) + throw new Error('Combined runner dependencies need one owner and explicit kinds.'); + const byKind = new Map(); + for (const delegate of delegates) for (const kind of delegate.kinds!) { + if (byKind.has(kind)) throw new Error(`Runner kind ${kind} has more than one implementation.`); + byKind.set(kind, delegate); + } + type Combined = { delegate: RunnerDeps; prepared: PreparedAttempt }; + const unpack = (prepared: PreparedAttempt): Combined => prepared.private as Combined; + const wrap = (delegate: RunnerDeps, prepared: PreparedAttempt): PreparedAttempt => ({ + clone: prepared.clone, vendor: prepared.vendor, approvedArgv: prepared.approvedArgv, + private: { delegate, prepared } satisfies Combined, + }); + const select = (attempt: AttemptRecord) => { + const delegate = byKind.get(attempt.kind); + if (!delegate) throw new Error(`No runner dependency can run ${attempt.kind}.`); + return delegate; + }; + return { + runnerOwner, kinds: [...byKind.keys()], + async prepare(attempt, signal) { + const delegate = select(attempt); + try { return wrap(delegate, await delegate.prepare(attempt, signal)); } + catch (error) { + // A delegate may allocate task storage before preparation fails. Preserve both its implementation identity and + // opaque cleanup handle so the combined release path can still route that allocation back to its owner. + if (error instanceof PreparationFailure) + throw new PreparationFailure(error.cause ?? error, wrap(delegate, error.allocated)); + throw error; + } + }, + async cleanupPreparation(attempt) { await select(attempt).cleanupPreparation(attempt); }, + beforeStart(attempt, prepared) { const value = unpack(prepared); value.delegate.beforeStart?.(attempt, value.prepared); }, + start(input, prepared) { const value = unpack(prepared); return value.delegate.start(input, value.prepared); }, + onStarted(attempt, prepared) { const value = unpack(prepared); value.delegate.onStarted?.(attempt, value.prepared); }, + validate(attempt, result) { return select(attempt).validate(attempt, result); }, + async finish(attempt, result, prepared, signal) { + const value = unpack(prepared); + if (!value.delegate.finish) return { value: value.delegate.validate(attempt, result) }; + return value.delegate.finish(attempt, result, value.prepared, signal); + }, + async auditFailed(attempt, prepared, signal) { const value = unpack(prepared); await value.delegate.auditFailed?.(attempt, value.prepared, signal); }, + async exportPartial(attempt, prepared) { const value = unpack(prepared); return await value.delegate.exportPartial?.(attempt, value.prepared) ?? {}; }, + async release(attempt, prepared) { const value = unpack(prepared); await value.delegate.release?.(attempt, value.prepared); }, + }; +} export interface SlotLimits { readonly writable: number; readonly readOnly: number } export interface StartRequest { expectedStateVersion: number; kind: AttemptKind; item?: string | null; deadline: number; budgetMs?: number; retryOf?: string; expectedContext: AttemptRecord['context']; /** Clears the task's requeue claim in the admitting transaction: the user's Resume of an interrupted task. */ claimRequeue?: boolean; + /** Fresh authorization plus its final launch guard. This is trusted coordinator state, never persisted input. */ + authorize?: (signal: AbortSignal) => Promise<() => void | Promise>; } export interface RunnerStatus { active: boolean; - stopRequested: { attemptId: string; reason: FirstReason; saved: boolean } | null; + stopRequested: { attemptId: string; reason: FirstReason | 'timeout'; saved: boolean } | null; unresolved: { attemptId: string; reason: UnresolvedReason } | null; } /** @@ -98,6 +151,7 @@ type Group = 'writable' | 'readOnly'; interface Job { identity: PlanIdentity; key: string; group: Group; attemptId: string; attempt?: AttemptRecord; firstReason: FirstReason | null; reasonSaved: boolean; preparationTimedOut: boolean; staleCause?: string; + timeoutRequested?: boolean; timeoutSaved?: boolean; /** * The outcome is fixed: before launch once the job starts ending (cleanup, then the terminal write), after launch once * the terminal write is done. A later stop has nothing left to change. @@ -110,6 +164,7 @@ interface Job { * later stop, but the job's reason, its abort signal and D's handle change only on commit; a rollback drops it (#79). */ pendingReason?: FirstReason; + authorize?: StartRequest['authorize']; controller: AbortController; handle?: InvocationHandle; timers: ReturnType[]; done?: Promise; } interface Marker { group: Group; attemptId: string; reason: UnresolvedReason } @@ -173,7 +228,8 @@ export class RunnerCoordinator { if (this.#deps.kinds && !this.#deps.kinds.includes(request.kind)) throw new GuardRefusal(`The runner cannot run ${request.kind} attempts yet.`); const group: Group = WRITABLE_KINDS.includes(request.kind) ? 'writable' : 'readOnly'; if (this.#used(group) >= this.#limits[group]) throw new GuardRefusal('No free runner slot. Try again when the current attempt finishes.'); - const job: Job = { identity: { ...identity }, key, group, attemptId: '', firstReason: null, reasonSaved: true, preparationTimedOut: false, controller: new AbortController(), timers: [] }; + const job: Job = { identity: { ...identity }, key, group, attemptId: '', firstReason: null, reasonSaved: true, + preparationTimedOut: false, authorize: request.authorize, controller: new AbortController(), timers: [] }; this.#jobs.set(key, job); let attempt: AttemptRecord; try { attempt = this.#store.admitAttempt(identity, { ...request, now: this.#now() }); } @@ -190,11 +246,28 @@ export class RunnerCoordinator { * User stop or detected staleness. The first reason wins; nothing is freed until settlement. * `cause` says what made the attempt stale (for example "plan revision 4 replaced 3") and is kept only if `stale` wins. */ - stop(identity: PlanIdentity, attemptId: string, reason: 'cancelled' | 'stale', cause?: string): boolean { + stop(identity: PlanIdentity, attemptId: string, reason: FirstReason, cause?: string): boolean { const job = this.#jobs.get(identityKey(identity)); if (!job || job.attemptId !== attemptId) return false; return this.#requestStop(job, reason, reason === 'stale' && cause !== undefined ? bounded(cause) : undefined); } + /** Stop one invocation because its owning operation expired, preserving timeout rather than user cancellation. */ + timeout(identity: PlanIdentity, attemptId: string): boolean { + const job = this.#jobs.get(identityKey(identity)); + if (!job || job.attemptId !== attemptId || job.decided || job.firstReason || job.pendingReason || job.timeoutRequested) + return false; + const apply = (saved: boolean) => { + job.timeoutRequested = true; job.timeoutSaved = saved; + job.controller.abort(Object.assign(new Error('Timed out.'), { code: 'ETIMEDOUT' })); + job.handle?.cancel('timeout'); + }; + try { + const changed = this.#write(() => this.#store.recordAttemptTimeout(job.identity, job.attemptId)); + if (!changed) return false; + apply(true); + } catch { apply(false); } + return true; + } /** Cancel task: the Store records the reason and the pending close; the coordinator stops the running work. */ cancelTask(identity: PlanIdentity, expectedStateVersion: number, actionId: string): 'closed' | 'stopping' { const job = this.#jobs.get(identityKey(identity)); @@ -212,9 +285,8 @@ export class RunnerCoordinator { } const outcome = this.#store.cancelTask(identity, expectedStateVersion, actionId); if (outcome === 'stopping' && job && !this.#requestStop(job, 'cancelled') && !job.firstReason && !job.pendingReason) { - // The Store already wrote `cancelled` onto the row (a pending cancel task wins, even over a preparation timeout). - // Status shows it once the write commits, but it never becomes the job's reason: the terminal write reads the - // row's own reason. + // The Store writes `cancelled` when no earlier durable invocation timeout owns the row. The task cancellation still + // wins task closure either way; this marker shows the request only when no in-memory stop already explains it. this.#store.afterCommit(() => { job.cancelShown = true; }); } return outcome; @@ -225,6 +297,7 @@ export class RunnerCoordinator { return { active: !!job, stopRequested: job?.firstReason ? { attemptId: job.attemptId, reason: job.firstReason, saved: job.reasonSaved } + : job?.timeoutRequested ? { attemptId: job.attemptId, reason: 'timeout', saved: job.timeoutSaved === true } : job?.cancelShown ? { attemptId: job.attemptId, reason: 'cancelled', saved: true } : null, unresolved: marker ? { attemptId: marker.attemptId, reason: marker.reason } : null, }; @@ -252,6 +325,7 @@ export class RunnerCoordinator { // The attempt deadline passed before launch: it ends `failed` with no first reason, so a later stop cannot claim it. if (job.decided || (job.preparationTimedOut && !job.firstReason)) return false; if (job.firstReason) { job.handle?.cancel(D_REASON[job.firstReason]); return false; } + if (job.timeoutRequested) return false; // An earlier stop in the same transaction wins; its commit applies it. if (job.pendingReason) return false; const apply = (first: FirstReason, saved: boolean) => { @@ -286,8 +360,10 @@ export class RunnerCoordinator { wait(); }; const budget = this.#store.getTask(job.identity).budgetDeadline; - if (budget !== null) at(budget, () => this.#requestStop(job, 'time-limit')); - at(attempt.deadline, () => { if (!job.handle && !job.firstReason) { job.preparationTimedOut = true; job.controller.abort(new Error(PREPARATION_TIMEOUT)); } }); + if (attempt.kind !== 'check' && budget !== null) at(budget, () => this.#requestStop(job, 'time-limit')); + at(attempt.deadline, () => { if (!job.handle && !job.firstReason && !job.timeoutRequested) { + job.preparationTimedOut = true; job.controller.abort(new Error(PREPARATION_TIMEOUT)); + } }); } async #run(job: Job, attempt: AttemptRecord): Promise { try { @@ -299,25 +375,35 @@ export class RunnerCoordinator { try { this.#arm(job, attempt); } catch (error) { return await this.#endBeforeLaunch(job, attempt, { detail: `Could not arm the task time limit: ${message(error)}` }); } // A stop can land before this point; do not start preparation for it. - if (job.firstReason) return await this.#endBeforeLaunch(job, attempt, {}); + if (job.firstReason || job.timeoutRequested) return await this.#endBeforeLaunch(job, attempt, {}); let prepared: PreparedAttempt; try { prepared = await this.#deps.prepare(attempt, job.controller.signal); } catch (error) { // Storage that preparation allocated before it failed is removed after the terminal write, like every other path. return await this.#endBeforeLaunch(job, attempt, this.#preparationDetail(job, error), error instanceof PreparationFailure ? error.allocated : undefined); } - if (job.firstReason || job.preparationTimedOut) return await this.#endBeforeLaunch(job, attempt, this.#preparationDetail(job), prepared); + if (job.firstReason || job.timeoutRequested || job.preparationTimedOut) + return await this.#endBeforeLaunch(job, attempt, this.#preparationDetail(job), prepared); + if (job.authorize) try { + const validate = await job.authorize(job.controller.signal); + await validate(); + job.controller.signal.throwIfAborted(); + } catch (error) { + return await this.#endBeforeLaunch(job, attempt, + { detail: `Authorization changed before launch: ${message(error)}` }, prepared); + } // Launch check: one synchronous turn, no await between the checks and D's start call. const now = this.#now(), row = this.#store.getAttempt(job.identity, attempt.id), task = this.#store.getTask(job.identity); if (row.firstReason && !job.firstReason) job.firstReason = row.firstReason; - if (row.state !== 'pending' || job.firstReason) return await this.#endBeforeLaunch(job, attempt, {}, prepared); + if (row.state !== 'pending' || job.firstReason || job.timeoutRequested) + return await this.#endBeforeLaunch(job, attempt, {}, prepared); // A context change comes before both time checks, as in the settlement order and startup recovery. if (!sameContext(row.context, this.#store.currentContext(job.identity))) { // Recorded like any stale stop, so the row keeps it even if a cancel task lands during cleanup. this.#requestStop(job, 'stale'); return await this.#endBeforeLaunch(job, attempt, {}, prepared); } - if (task.budgetDeadline !== null && now >= task.budgetDeadline) { this.#requestStop(job, 'time-limit'); return await this.#endBeforeLaunch(job, attempt, {}, prepared); } + if (attempt.kind !== 'check' && task.budgetDeadline !== null && now >= task.budgetDeadline) { this.#requestStop(job, 'time-limit'); return await this.#endBeforeLaunch(job, attempt, {}, prepared); } if (now >= attempt.deadline) { job.preparationTimedOut = true; return await this.#endBeforeLaunch(job, attempt, { detail: PREPARATION_TIMEOUT }, prepared); } // Fail closed: once D reported resources it could not remove, no new invocation starts, even one already admitted. if (this.#unreleased) return await this.#endBeforeLaunch(job, attempt, { detail: NOT_STARTED_UNRELEASED }, prepared); @@ -460,7 +546,8 @@ export class RunnerCoordinator { } #settle(job: Job, s: { stopReason?: StopReason; exitCode: number | null; signal: string | null; valid: boolean; result?: unknown; detail?: string; history?: HistoryRecord; diagnosticRef?: string }): Classification | undefined { try { - return this.#write(() => this.#store.settleAttempt(job.identity, job.attemptId, { ...s, firstReason: job.firstReason })); + return this.#write(() => this.#store.settleAttempt(job.identity, job.attemptId, + { ...s, ...(job.timeoutRequested ? { stopReason: 'timeout' as const } : {}), firstReason: job.firstReason })); } catch { // The row's outcome is unknown: hold the slot until startup recovery reconciles it. this.#markers.set(job.key, { group: job.group, attemptId: job.attemptId, reason: 'result-not-saved' }); diff --git a/runner/merge.ts b/runner/merge.ts index a692cf0a..e46f0e7e 100644 --- a/runner/merge.ts +++ b/runner/merge.ts @@ -36,6 +36,8 @@ export interface MergeUnavailableStatus { available: true; ready: false; action: export interface PublishedTarget { repository: string; baseBranch: string; + /** Runner-owned production PRs require a durable, exact-review preparation before irreversible admission. */ + requiresPreparation?: boolean; /** `github.pullRequest`, if the configuration still names one. It must be the task's PR. */ configured?: number; } @@ -43,6 +45,12 @@ export interface PublishedTarget { class MergeTargetUnavailable extends Error { blockers: MergeBlocker[] = []; } /** The PR one status read inspected. `openingId` is set for the task's published PR, which admission re-reads. */ interface ResolvedTarget { target?: MergeTarget; openingId: string | null; marker?: string; headBranch?: string } +interface MergeAuthorization { + /** Complete the last external authorization read before the final PR identity/mode inspection. */ + refresh(): Promise; + /** Recheck only local trust state after that inspection, without opening another external race. */ + validate(): void; +} function queueGateway(gateway: MergeGateway): gateway is QueueGateway { const queue = gateway as Partial; @@ -61,6 +69,7 @@ export class MergeCoordinator { readonly gateway: MergeGateway; readonly operationTimeoutMs: number; readonly published: PublishedTarget | null; + #authorize: ((signal: AbortSignal) => Promise) | null = null; /** Settlement of an irreversible merge keeps its writes after the Store gate closes; request-path reconciliation does not. */ #settle: (fn: () => T) => T; constructor(service: ReviewService, gateway: MergeGateway, operationTimeoutMs = MERGE_OPERATION_TIMEOUT_MS, capability?: ShutdownCapability, published?: PublishedTarget) { @@ -70,6 +79,12 @@ export class MergeCoordinator { this.#settle = settleWith(capability); } + /** Install the production issue-trust guard once the server's issue gateway is available. */ + setAuthorization(authorize: (signal: AbortSignal) => Promise): void { + if (this.#authorize) throw new Error('Merge authorization is already configured.'); + this.#authorize = authorize; + } + #attempt(): MergeAttempt | null { return this.service.store?.getMergeAttempt(this.service.config.identity) ?? null; } @@ -147,6 +162,25 @@ export class MergeCoordinator { return (await this.#status(view, fresh, signal)).status; } + /** Fresh exact base/head pair for pre-merge preparation, using the same task-PR resolution as merge admission. */ + async remotePair(signal?: AbortSignal): Promise<{ base: string; head: string }> { + const resolved = this.#resolve(this.#attempt()); + const remote = await this.gateway.inspect({ fresh: true, timeoutMs: 6_000, signal, + ...(resolved.target ? { target: resolved.target } : {}) }); + signal?.throwIfAborted(); + if (resolved.target && remote.pullRequest !== resolved.target.pullRequest) + throw new Error('GitHub returned a different pull request.'); + if (remote.pullRequestState !== 'OPEN') + throw new GuardRefusal(`Pull request #${remote.pullRequest} is ${remote.pullRequestState.toLowerCase()}; pre-merge preparation requires an open pull request.`); + if (remote.draft) + throw new GuardRefusal(`Pull request #${remote.pullRequest} is a draft; mark it ready before pre-merge preparation.`); + if (resolved.headBranch !== undefined) { + const blockers = this.#publishedBlockers(remote, resolved); + if (blockers.length) throw new GuardRefusal(blockers[0]!.message); + } + return { base: remote.base, head: remote.head }; + } + async #status(view: ReviewView, fresh: boolean, signal?: AbortSignal): Promise<{ status: MergeStatus; resolved: ResolvedTarget }> { const blockers: MergeBlocker[] = []; for (const item of view.items) { @@ -164,6 +198,12 @@ export class MergeCoordinator { if (this.service.store && this.service.config) { const task = this.service.store.getTask(this.service.config.identity); if (task.status !== 'merged' && !MERGEABLE_STATUSES.includes(task.status)) blockers.push({ code: 'task', message: `The task is ${task.status}; merge it from review.` }); + if (this.published?.requiresPreparation && view.expected.reviewVersion !== undefined + && !this.service.store.preMergeReady(this.service.config.identity, { + stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, + commandPolicyDigest: this.service.commandPolicyDigest(), + })) blockers.push({ code: 'preparation', message: 'Pre-merge preparation has not completed for the current review. Prepare the merge again.' }); } let resolved: ResolvedTarget; try { resolved = this.#resolve(this.#attempt()); } @@ -333,6 +373,9 @@ export class MergeCoordinator { const taskStateVersion = this.service.store && this.service.config ? this.service.store.getTask(this.service.config.identity).stateVersion : null; let view = this.service.load(); if (view.token !== token) throw new Error('Stale review state. Refresh before merging.'); + // Capture the externally authorized issue identity before validation. Its final external refresh precedes the + // final PR inspection; a synchronous local trust check then closes the admission window without another await. + const authorization = this.#authorize ? await this.#authorize(signal) : null; const { status, resolved } = await this.#statusForMerge(view, signal); if (!status.ready) throw new Error(status.blockers[0]?.message ?? 'Merge is blocked.'); view = this.service.load(); @@ -357,12 +400,42 @@ export class MergeCoordinator { throw new Error('Merge-queue requirements changed after queue correlation. Refresh before merging.'); if (!commandStatus.ready) throw new Error(`Merge requirements changed after queue correlation. ${commandStatus.blockers[0]!.message}`); } + if (authorization) { + await authorization.refresh(); + const authorized = await this.#statusForMerge(view, signal); + if (authorized.status.remote.base !== commandStatus.remote.base || authorized.status.remote.head !== commandStatus.remote.head + || authorized.status.remote.mergeQueue !== commandStatus.remote.mergeQueue || !samePullRequest(authorized)) + throw new Error('The pull request changed during merge validation. Refresh before merging.'); + if (!authorized.status.ready) + throw new Error(`Merge requirements changed during authorization validation. ${authorized.status.blockers[0]!.message}`); + commandStatus = authorized.status; + } + if (commandStatus.remote.mergeQueue) { + // Events produced by another actor during authorization or the preceding status checks come before this + // boundary and cannot be adopted as this attempt's lifecycle. The following fresh status read then validates + // the PR identity and queue mode that this cursor will be stored with. + queueWatermark = await (this.gateway as QueueGateway).queueWatermark(commandStatus.remote.head, + { signal, timeoutMs: 6_000, pullRequest: commandStatus.remote.pullRequest }); + const boundary = await this.#statusForMerge(view, signal); + if (boundary.status.remote.base !== commandStatus.remote.base || boundary.status.remote.head !== commandStatus.remote.head + || boundary.status.remote.mergeQueue !== commandStatus.remote.mergeQueue || !samePullRequest(boundary)) + throw new Error('The pull request changed after queue correlation. Refresh before merging.'); + if (!boundary.status.ready) + throw new Error(`Merge requirements changed after queue correlation. ${boundary.status.blockers[0]!.message}`); + commandStatus = boundary.status; + } + // The external authorization read completed before the final PR inspection and queue boundary. Re-read local + // trust synchronously after those awaits so neither remote identity nor local authorization can race admission. + authorization?.validate(); if (this.service.load().token !== token) throw new Error('Review changed during merge validation. Refresh before merging.'); if (signal.aborted) throw signal.reason; if (this.service.store && this.service.config && view.expected.reviewVersion !== undefined) { const { store, config } = this.service, reviewVersion = view.expected.reviewVersion; + const requiresPreparation = this.published?.requiresPreparation === true; const begin = () => store.beginMergeAttempt(config.identity, { ...view.expected, reviewVersion }, commandStatus.remote.head, queueWatermark, - commandStatus.remote.mergeQueue ? 'queue' : 'direct', actionId ?? null, taskStateVersion, { pullRequest: commandStatus.remote.pullRequest, openingId: resolved.openingId }); + commandStatus.remote.mergeQueue ? 'queue' : 'direct', actionId ?? null, taskStateVersion, + { pullRequest: commandStatus.remote.pullRequest, openingId: resolved.openingId }, requiresPreparation, + requiresPreparation ? this.service.commandPolicyDigest() : null); let begun: MergeAttempt | null = null; // The attempt and the click's saved response commit in one transaction, or neither does. if (action) store.userAction(config.identity, action, () => mergeActionResponse(begun = begin())); diff --git a/runner/pre-merge.ts b/runner/pre-merge.ts new file mode 100644 index 00000000..adb8bf8d --- /dev/null +++ b/runner/pre-merge.ts @@ -0,0 +1,432 @@ +import { readHistory } from '../git/history.ts'; +import { DEFAULT_PROCESS_SETTLEMENT_MS } from '../agents/process-group.ts'; +import type { PlanIdentity } from '../core/identity.ts'; +import type { RunnerCoordinator } from './coordinator.ts'; +import { GuardRefusal, MERGEABLE_STATUSES, settleWith, type ShutdownCapability } from './lifecycle.ts'; +import { MAX_REBASE_TIMEOUT_MS, MIN_REBASE_CLEANUP_TIMEOUT_MS, MIN_REBASE_TIMEOUT_MS, + RebaseResourcesUnsettled, type GitRebaser } from './rebase.ts'; +import type { ReviewService } from './review.ts'; +import type { PreMergeReadiness, RebaseMarker } from './store.ts'; + +export interface RemotePair { base: string; head: string } +export interface PreMergeRemote { + inspect(signal?: AbortSignal): Promise; + fetch(pair: RemotePair, signal?: AbortSignal): Promise; +} +export interface PreMergeAuthorization { + /** Complete the last external authorization read before a later external operation. */ + refresh(): Promise; + /** Recheck only local trust state after that operation, without opening another external race. */ + validate(): void; +} +export interface PreMergeResult { + state: 'ready' | 'review-required' | 'failed'; + base: string; head: string; checked: readonly string[]; reason: string | null; +} +type PreparedResult = PreMergeResult & { readiness?: PreMergeReadiness }; + +// D may spend 30 s on its first cleanup and 60 s retrying it; F may then spend 30 s releasing task storage. Keep all +// of that ownership settlement inside the preparation's one overall deadline. +export const COMMAND_CHECK_SETTLEMENT_RESERVE_MS = 120_000; +/** Git and GitHub subprocesses abort before the advertised operation deadline, leaving their bounded stop/drain time. */ +export const PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS = DEFAULT_PROCESS_SETTLEMENT_MS; +/** Keep a full process-settlement window between live rebase expiry and its separately budgeted durable cleanup. */ +const REBASE_CLEANUP_HANDOFF_RESERVE_MS = PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS; + +/** Production preparation before F6: refresh, rebase, re-review, then exact-head command checks. */ +export class PreMergeCoordinator { + readonly service: ReviewService; + readonly runner: RunnerCoordinator; + readonly rebaser: GitRebaser; + readonly remote: PreMergeRemote; + readonly operationTimeoutMs: number; + readonly processSettlementReserveMs: number; + readonly commandSettlementReserveMs: number; + readonly authorize?: (signal: AbortSignal) => Promise; + readonly settle: (fn: () => T) => T; + #active: Promise | null = null; + #abort: AbortController | null = null; + #closing = false; + #last: PreMergeResult | null = null; + #lastBinding: { stateVersion: number; reviewVersion: number; snapshotId: string } | null = null; + + constructor(service: ReviewService, runner: RunnerCoordinator, rebaser: GitRebaser, remote: PreMergeRemote, + operationTimeoutMs = 10 * 60_000, capability?: ShutdownCapability, + authorize?: (signal: AbortSignal) => Promise, + reserves: { processMs?: number; commandMs?: number } = {}) { + if (!Number.isSafeInteger(operationTimeoutMs) || operationTimeoutMs < 1 || operationTimeoutMs > 60 * 60_000) + throw new Error('Invalid pre-merge operation deadline.'); + this.service = service; this.runner = runner; this.rebaser = rebaser; this.remote = remote; + this.operationTimeoutMs = operationTimeoutMs; this.authorize = authorize; + this.processSettlementReserveMs = reserves.processMs ?? PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS; + this.commandSettlementReserveMs = reserves.commandMs ?? COMMAND_CHECK_SETTLEMENT_RESERVE_MS; + if (!Number.isSafeInteger(this.processSettlementReserveMs) || this.processSettlementReserveMs < 0 + || !Number.isSafeInteger(this.commandSettlementReserveMs) || this.commandSettlementReserveMs < 0) + throw new Error('Invalid pre-merge settlement reserve.'); + this.settle = settleWith(capability); + } + + get active(): boolean { return this.#active !== null; } + get last(): (PreMergeResult & { stale: boolean }) | null { + if (!this.#last) return null; + let stale = this.#lastBinding === null; + if (this.#lastBinding) try { + const task = this.service.store.getTask(this.service.config.identity); + stale = task.stateVersion !== this.#lastBinding.stateVersion + || this.service.store.reviewVersion(this.service.config.identity) !== this.#lastBinding.reviewVersion + || this.service.store.getSnapshot(this.service.config.identity).id !== this.#lastBinding.snapshotId; + } catch { stale = true; } + return { ...this.#last, stale }; + } + assertStartable(): void { + if (this.#closing) throw new GuardRefusal('The server is shutting down.'); + if (this.#active) throw new GuardRefusal('Pre-merge preparation is already running.'); + } + start(expected: { stateVersion: number; reviewVersion: number; snapshotId: string; base: string; head: string; actionId?: string }): Promise { + this.assertStartable(); + const controller = new AbortController(); this.#abort = controller; + const deadline = performance.now() + this.operationTimeoutMs; + const timer = setTimeout(() => controller.abort(Object.assign(new Error('Pre-merge preparation deadline exceeded.'), + { code: 'ETIMEDOUT' })), Math.max(0, this.operationTimeoutMs - this.processSettlementReserveMs)); + // Admission reserves the coordinator synchronously, but expensive Git/GitHub work begins after the user-action + // transaction and HTTP handler can finish. + let readiness: PreMergeReadiness | null = null; + let failureChecked: readonly string[] = []; + let failurePair = { base: expected.base, head: expected.head }; + let failureBinding = { stateVersion: expected.stateVersion, reviewVersion: expected.reviewVersion, + snapshotId: expected.snapshotId }; + const remember = (input: PreparedResult) => { + const { readiness: readyBinding, ...plain } = input; + readiness = readyBinding ?? null; + let result: PreMergeResult = plain; + this.#lastBinding = null; + if (result.state === 'ready' && readiness) this.#lastBinding = readiness; + else if (result.state === 'ready') result = { ...result, state: 'failed', + reason: 'Could not bind preparation readiness: preparation readiness was not bound to the final review.' }; + else this.#lastBinding = failureBinding; + this.#last = result; + return result; + }; + const active = Promise.resolve().then(() => this.#run(expected, controller.signal, deadline, + observation => { failurePair = observation.pair; failureBinding = observation.binding; }, + checked => { failureChecked = Object.freeze([...checked]); })).then(remember, error => { + // Failure settlement must not rebuild Git history: the original failure may itself be a repository-read error. + const result: PreMergeResult = { state: 'failed', ...failurePair, + checked: failureChecked, reason: error instanceof Error ? error.message : String(error) }; + return remember(result); + }).then(result => { + if (!expected.actionId) return result; + try { + const effective = this.settle(() => this.service.store.settlePreMergeAction( + this.service.config.identity, expected.actionId!, result, readiness)); + if (effective !== result) result = remember(effective); + } + catch (error) { + // No terminal response or readiness was committed. Remove only this action's pending placeholder, so the same + // key can start the preparation again instead of replaying work that no longer exists. + try { this.settle(() => this.service.store.makePreMergeActionResendable(this.service.config.identity, expected.actionId!)); } + catch { /* The original storage failure remains the result; startup recovery handles a placeholder we could not remove. */ } + const failed: PreMergeResult = { ...result, state: 'failed', + reason: error instanceof Error ? error.message : String(error) }; + return remember(failed); + } + return result; + }).finally(() => { clearTimeout(timer); if (this.#active === active) { this.#active = null; this.#abort = null; } }); + this.#active = active; + return active; + } + async close(): Promise { + this.#closing = true; this.#abort?.abort(new Error('Server shutdown.')); await this.#active; + } + /** Cancel the task and promptly stop whichever rebase, remote read or command check this preparation owns. */ + cancelTask(expectedStateVersion: number, actionId: string): 'closed' | 'stopping' { + const identity = this.service.config.identity; + const outcome = this.runner.isActive(identity) + ? this.runner.cancelTask(identity, expectedStateVersion, actionId) + : this.service.store.cancelTask(identity, expectedStateVersion, actionId); + this.service.store.afterCommit(() => this.#abort?.abort(new Error('Task cancelled.'))); + return outcome; + } + + #refresh(pair: RemotePair, remaining: () => number, priorHead?: string) { + const historyOptions = () => ({ maxDurationMs: Math.min(30_000, remaining()) }); + const view = this.service.load(historyOptions()); + const repository = this.service.reviewRepository().path; + let history: ReturnType; + try { history = readHistory(repository, pair.base, pair.head, historyOptions()); } + catch (error) { + // A collaborator may push on the old remote head while the base moves (or while F5 retains an unpushed local + // rebase). Review that head against the latest stored base it actually descended from; the next preparation can + // then rebase the newly attributed head. Only ancestry mismatch permits this fallback: every other read failure + // remains fail-closed. + if (!priorHead || !(error instanceof Error) + || !/linear history descended from the base|base must be an ancestor of the head/i.test(error.message)) throw error; + let recovered: ReturnType | null = null; + for (const candidate of [priorHead, ...this.service.store.rewrittenAncestors(this.service.config.identity, priorHead)]) { + const priorId = this.service.store.snapshotWithHead(this.service.config.identity, candidate); + if (!priorId) continue; + try { recovered = readHistory(repository, this.service.store.getSnapshot(this.service.config.identity, priorId).base, + pair.head, historyOptions()); break; } + catch (fallback) { + if (!(fallback instanceof Error) + || !/linear history descended from the base|base must be an ancestor of the head/i.test(fallback.message)) throw fallback; + } + } + if (!recovered) throw error; + history = recovered; + } + this.service.store.recordHistory(this.service.config.identity, view.expected, history.base, history.head, []); + return this.service.load(historyOptions()); + } + #reviewBlocker(view: ReturnType, requireChecks = false): string | null { + const invalidCommand = view.items.find(item => item.checks.tests === '✕ Invalid command'); + if (invalidCommand) return `${invalidCommand.id} has an invalid command check; amend the plan before preparing the merge.`; + const item = view.items.find(value => value.state !== 'approved' || value.outside.length); + if (item) return `${item.id} requires refreshed attribution or approval.`; + if (view.segments.some(segment => segment.row === 'Ambiguous')) return 'Ambiguous changes require attribution.'; + if (view.segments.some(segment => segment.row === 'Unplanned')) return 'Unplanned changes require a plan amendment.'; + const changes = view.notes.filter(note => note.kind === 'change' && note.revision === view.plan.revision + && note.snapshotId === view.snapshot.id).length; + if (changes) return `${changes} change request${changes === 1 ? ' remains' : 's remain'} open.`; + if (requireChecks) { + const unchecked = view.items.find(item => item.acceptance.some(check => check.type === 'cmd') && item.checks.tests !== '✓ Passed'); + if (unchecked) return `${unchecked.id} command checks have not passed on this head.`; + } + return null; + } + async #cleanupRebase(identity: PlanIdentity, marker: RebaseMarker, timeoutMs: number): Promise { + const current = this.service.store.getTask(identity).rebaseInProgress as RebaseMarker | null; + if (current?.attemptId !== marker.attemptId) throw new GuardRefusal('This rebase attempt no longer owns cleanup.'); + await this.rebaser.abort(marker.attemptId, current.resultHead ?? undefined, current.resultState, timeoutMs); + if (!this.settle(() => this.service.store.abortRebase(this.service.store.getTask(identity).planKey, marker.attemptId))) + throw new GuardRefusal('This rebase attempt no longer owns its durable marker.'); + } + async #run(expected: { stateVersion: number; reviewVersion: number; snapshotId: string; actionId?: string }, signal: AbortSignal, + deadline: number, onObserved: (value: { pair: RemotePair; binding: { stateVersion: number; reviewVersion: number; + snapshotId: string } }) => void, onChecked: (checked: readonly string[]) => void): Promise { + const identity = this.service.config.identity; + const remaining = () => { + const value = Math.ceil(deadline - performance.now()); + if (value < 1) throw Object.assign(new Error('Pre-merge preparation deadline exceeded.'), { code: 'ETIMEDOUT' }); + return value; + }; + const reviewBudget = () => { + const value = Math.ceil(deadline - performance.now() - this.processSettlementReserveMs); + if (value < 1) throw Object.assign(new Error('Pre-merge preparation deadline exceeded before process settlement could be reserved.'), + { code: 'ETIMEDOUT' }); + return value; + }; + const commandBudget = () => { + const value = Math.ceil(deadline - performance.now() - this.commandSettlementReserveMs); + if (value < 1) throw Object.assign(new Error('Pre-merge preparation deadline exceeded before command-check settlement could be reserved.'), + { code: 'ETIMEDOUT' }); + return value; + }; + const rebaseBudget = (cleanupOnly = false) => { + // A failed live run still needs a separate abort that clears its durable marker. Keep that minimum outside the + // run's scope instead of letting the run consume the whole operation budget. + const available = remaining() - (cleanupOnly ? 0 + : MIN_REBASE_CLEANUP_TIMEOUT_MS + REBASE_CLEANUP_HANDOFF_RESERVE_MS); + const value = Math.min(MAX_REBASE_TIMEOUT_MS, available); + if (value < (cleanupOnly ? MIN_REBASE_CLEANUP_TIMEOUT_MS : MIN_REBASE_TIMEOUT_MS)) + throw Object.assign(new Error(`Pre-merge preparation deadline exceeded before rebase ${cleanupOnly ? 'cleanup' : 'work and cleanup'} could be reserved.`), + { code: 'ETIMEDOUT' }); + return value; + }; + const authorize = async () => { + if (!this.authorize) return; + const authorization = await this.authorize(signal); + await authorization.refresh(); authorization.validate(); signal.throwIfAborted(); remaining(); + return authorization; + }; + const track = >(current: T): T => { + onObserved({ pair: { base: current.snapshot.base, head: current.snapshot.head }, binding: { + stateVersion: this.service.store.getTask(identity).stateVersion, + reviewVersion: this.service.store.reviewVersion(identity), snapshotId: current.snapshot.id, + } }); + return current; + }; + // Rebase lifecycle writes advance task state without changing the review that admitted them. Bind a resulting + // failure to that original review/snapshot while adopting only the task-state transition owned by the rebase. If a + // read fails, the older binding remains safely stale. + const bindRebaseFailure = (reviewed: { reviewVersion: number; snapshotId: string }, pair: RemotePair) => { + try { + onObserved({ pair, binding: { + stateVersion: this.service.store.getTask(identity).stateVersion, + reviewVersion: reviewed.reviewVersion, snapshotId: reviewed.snapshotId, + } }); + } catch { /* An unbound failure is conservatively historical. */ } + }; + const load = () => track(this.service.load({ maxDurationMs: Math.min(30_000, reviewBudget()) })); + signal.throwIfAborted(); + if (this.runner.isActive(identity)) throw new GuardRefusal('An attempt is already active for this task.'); + const runnerStatus = this.runner.status(identity); + if (runnerStatus.unresolved || this.runner.unreleased) + throw new GuardRefusal('Runner cleanup is unresolved; restart and recover owned resources before preparing a merge.'); + if (this.service.store.getTask(identity).rebaseInProgress !== null) + throw new GuardRefusal('Rebase cleanup is unresolved; restart and recover the owned rebase before preparing a merge.'); + let view = load(), task = this.service.store.getTask(identity); + if (task.stateVersion !== expected.stateVersion || view.expected.reviewVersion !== expected.reviewVersion + || view.snapshot.id !== expected.snapshotId) throw new GuardRefusal('The review changed before preparation started. Reload first.'); + if (!MERGEABLE_STATUSES.includes(task.status)) throw new GuardRefusal(`The task is ${task.status}; prepare it from review.`); + if (task.cancelRequested !== null) throw new GuardRefusal('The task is being cancelled.'); + if (!this.service.reviewRepository().runnerOwned) + throw new GuardRefusal('Pre-merge preparation requires a runner-owned head.'); + const merge = this.service.store.getMergeAttempt(identity); + if (merge && (merge.state === 'submitting' || merge.state === 'queued')) + throw new GuardRefusal('A merge is in progress; wait for its outcome.'); + const assertCurrent = (guard: { stateVersion: number; reviewVersion: number; snapshotId: string }) => { + const currentTask = this.service.store.getTask(identity); + if (currentTask.stateVersion !== guard.stateVersion || !MERGEABLE_STATUSES.includes(currentTask.status) + || currentTask.cancelRequested !== null || this.service.store.reviewVersion(identity) !== guard.reviewVersion + || this.service.store.getSnapshot(identity).id !== guard.snapshotId) + throw new GuardRefusal('The task or review changed during preparation. Reload first.'); + }; + const initial = await this.remote.inspect(signal); await this.remote.fetch(initial, signal); signal.throwIfAborted(); + assertCurrent(expected); + // F6 will push the rewritten head. Until then, a retry must recognize the durable rewrite lineage instead of + // mistaking codeboost's still-remote predecessor for a collaborator push. + const retainedRemoteHead = initial.head !== view.snapshot.head + && this.service.store.isRewrittenHead(identity, initial.head, view.snapshot.head); + if (initial.head !== view.snapshot.head && !retainedRemoteHead) { + view = track(this.#refresh(initial, reviewBudget, view.snapshot.head)); + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked: [], + reason: 'The pull request head moved; attribution and approvals were refreshed.' }; + } + if (initial.base !== view.snapshot.base) { + const repository = this.service.reviewRepository().path; + const old = readHistory(repository, view.snapshot.base, view.snapshot.head, + { maxDurationMs: Math.min(30_000, reviewBudget()) }).commits.map(commit => commit.sha); + task = this.service.store.getTask(identity); + const reviewed = { revision: view.expected.revision, snapshotId: view.expected.snapshotId, + reviewVersion: view.expected.reviewVersion! }; + const oldBase = view.snapshot.base, oldHead = view.snapshot.head; + await authorize(); + signal.throwIfAborted(); + const rebaseTimeoutMs = rebaseBudget(); + const marker = this.service.store.beginRebase(identity, reviewed, task.stateVersion, + { oldBase: view.snapshot.base, oldHead: view.snapshot.head, oldHistory: old, onto: initial.base }); + try { + const result = await this.rebaser.run({ attemptId: marker.attemptId, oldBase, + oldHead, oldHistory: old, onto: initial.base, + ledger: this.service.store.getLedger(identity), signal, timeoutMs: rebaseTimeoutMs }); + signal.throwIfAborted(); + this.service.store.finishRebase(identity, reviewed, task.stateVersion, marker.attemptId, + result.base, result.head, result.mappings); + } catch (error) { + if (error instanceof RebaseResourcesUnsettled) { + bindRebaseFailure(reviewed, { base: oldBase, head: oldHead }); + throw error; + } + try { await this.#cleanupRebase(identity, marker, rebaseBudget(true)); } + catch (cleanup) { + bindRebaseFailure(reviewed, { base: oldBase, head: oldHead }); + throw new AggregateError([error, cleanup], error instanceof Error ? error.message : 'Rebase failed.', { cause: error }); + } + bindRebaseFailure(reviewed, { base: oldBase, head: oldHead }); + throw error; + } + view = load(); + } + const blocker = this.#reviewBlocker(view); + if (blocker) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, + checked: [], reason: blocker }; + const checked: string[] = []; + for (const item of view.items.filter(candidate => candidate.acceptance.some(check => check.type === 'cmd'))) { + signal.throwIfAborted(); + task = this.service.store.getTask(identity); + const attempt = this.runner.start(identity, { expectedStateVersion: task.stateVersion, kind: 'check', item: item.id, + deadline: Date.now() + commandBudget(), expectedContext: this.service.store.currentContext(identity), + ...(this.authorize ? { authorize: async (checkSignal: AbortSignal) => { + const authorization = await this.authorize!(checkSignal); + await authorization.refresh(); + return () => authorization.validate(); + } } : {}) }); + const stop = () => { + // The invocation owns the same deadline and reports its own timeout. `time-limit` is reserved for the + // code-writing task budget: recording it here would incorrectly move an in-review task to needs human. + // The check's own deadline remains distinct from the code-writing budget, but an outer timeout still owns and + // must stop this attempt. Settlement then consumes the reserve kept inside the operation-wide deadline. + if (this.#closing) this.runner.stop(identity, attempt.id, 'shutdown'); + else if ((signal.reason as { code?: unknown } | undefined)?.code === 'ETIMEDOUT') this.runner.timeout(identity, attempt.id); + else this.runner.stop(identity, attempt.id, 'cancelled'); + }; + signal.addEventListener('abort', stop, { once: true }); + // Adding a listener does not replay an abort that won the race after admission but before registration. + if (signal.aborted) stop(); + try { await this.runner.settled(identity); } finally { signal.removeEventListener('abort', stop); } + // The check attempt itself advances task state even when it fails. Bind that owned transition without adopting a + // concurrent review edit; such an edit must leave the preparation historical and stale. + if (this.service.store.reviewVersion(identity) === view.expected.reviewVersion) { + const snapshot = this.service.store.getSnapshot(identity); + onObserved({ pair: { base: snapshot.base, head: snapshot.head }, binding: { + stateVersion: this.service.store.getTask(identity).stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: snapshot.id, + } }); + } + signal.throwIfAborted(); + const settled = this.service.store.getAttempt(identity, attempt.id); + const runnerStatus = this.runner.status(identity); + if (runnerStatus.unresolved || this.runner.unreleased) + throw new GuardRefusal('Command-check cleanup could not be confirmed; restart and recover owned resources before preparing a merge.'); + if (settled.state !== 'completed') throw new GuardRefusal(settled.exitCode === null && settled.diagnostic + ? settled.diagnostic : `${item.id} command checks did not pass.`); + checked.push(item.id); onChecked(checked); view = load(); + const changed = this.#reviewBlocker(view); + if (changed) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, reason: changed }; + } + const prepared = { base: view.snapshot.base, head: view.snapshot.head }; + const guarded = { stateVersion: this.service.store.getTask(identity).stateVersion, + // Rebase and command attempts advance task/snapshot state, but this preparation never owns a review edit. + reviewVersion: expected.reviewVersion, snapshotId: view.snapshot.id }; + const final = await this.remote.inspect(signal); signal.throwIfAborted(); + assertCurrent(guarded); + if (final.base !== initial.base || final.head !== initial.head) { + await this.remote.fetch(final, signal); signal.throwIfAborted(); assertCurrent(guarded); + view = track(this.#refresh(final, reviewBudget, initial.head)); + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: 'The pull request moved during preparation; attribution and approvals were refreshed.' }; + } + view = load(); + if (view.snapshot.base !== prepared.base || view.snapshot.head !== prepared.head) + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: 'The local review changed during preparation; reload it.' }; + const finalBlocker = this.#reviewBlocker(view, true); + if (finalBlocker) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: finalBlocker }; + const authorization = await authorize(); + // Authorization is asynchronous. A review edit during that read invalidates its result just as one during the + // final remote inspection does; nothing may persist readiness from the pre-authorization view. + assertCurrent(guarded); + view = load(); + if (view.snapshot.base !== prepared.base || view.snapshot.head !== prepared.head) + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: 'The local review changed during final authorization; reload it.' }; + const authorizedBlocker = this.#reviewBlocker(view, true); + if (authorizedBlocker) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: authorizedBlocker }; + // The authorization read above is asynchronous, so the PR may have moved after the earlier inspection. Reinspect + // it, then immediately recheck every local generation before recording readiness. + const authorizedRemote = await this.remote.inspect(signal); signal.throwIfAborted(); + assertCurrent(guarded); + authorization?.validate(); + if (authorizedRemote.base !== initial.base || authorizedRemote.head !== initial.head) { + await this.remote.fetch(authorizedRemote, signal); signal.throwIfAborted(); assertCurrent(guarded); + view = track(this.#refresh(authorizedRemote, reviewBudget, initial.head)); + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: 'The pull request moved during final authorization; attribution and approvals were refreshed.' }; + } + view = load(); + if (view.snapshot.base !== prepared.base || view.snapshot.head !== prepared.head) + return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, checked, + reason: 'The local review changed after final authorization; reload it.' }; + const postAuthorizationBlocker = this.#reviewBlocker(view, true); + if (postAuthorizationBlocker) return { state: 'review-required', base: view.snapshot.base, head: view.snapshot.head, + checked, reason: postAuthorizationBlocker }; + return { state: 'ready', base: view.snapshot.base, head: view.snapshot.head, checked, reason: null, + readiness: { stateVersion: this.service.store.getTask(identity).stateVersion, + reviewVersion: this.service.store.reviewVersion(identity), snapshotId: view.snapshot.id, + base: view.snapshot.base, head: view.snapshot.head, + commandPolicyDigest: this.service.commandPolicyDigest() } }; + } +} diff --git a/runner/production.ts b/runner/production.ts index 090d6a36..6280032d 100644 --- a/runner/production.ts +++ b/runner/production.ts @@ -5,12 +5,13 @@ import { buildAgentImage } from '../agents/container/image.ts'; import { exportTaskDiff, removeTaskFilesystemsAsync, type TaskStorageLimits } from '../agents/container/storage.ts'; import { recoverLeftovers } from '../agents/recovery.ts'; import { startClaudeInvocation } from '../agents/adapters/claude.ts'; +import { startRunnerCommandInvocation } from '../agents/adapters/runner.ts'; import { GhAlreadyFixedGateway } from '../github/already-fixed.ts'; import { GhIssueGateway, withIssueReadDeadline, type IssueAccess, type IssueText } from '../github/issues.ts'; import { GhPullRequestGateway } from '../github/pull-requests.ts'; import { baseBranch } from '../github/validate.ts'; import { identityKey } from '../core/identity.ts'; -import type { RunnerDeps } from './coordinator.ts'; +import { combineRunnerDeps, type RunnerDeps } from './coordinator.ts'; import { DEFAULT_DIAGNOSTICS_CAP_BYTES } from './diagnostics.ts'; import { executionDeps, SafetyFindings, type AgentLauncher, type ExecutionSources, type GuardedIssueText } from './execution.ts'; import { GuardRefusal, isUuidV4, quoteForTerminal, type ShutdownCapability } from './lifecycle.ts'; @@ -18,10 +19,13 @@ import { recoverStartup, removalCommand, type RecoveryDeps, type RecoveryReport, import { GitBranchPusher, pushUrl } from './branch-push.ts'; import { PullRequestPublisher } from './publish.ts'; import type { ReviewService } from './review.ts'; -import { openRunnerRepository, ownerOnlyDirectory, type RunnerRepository } from './runner-repository.ts'; +import { openRunnerRepository, ownerOnlyDirectory, retainSnapshotCommit, type RunnerRepository } from './runner-repository.ts'; import { GitRebaser } from './rebase.ts'; import { createForeignConflictResolver } from './rebase-conflict.ts'; import { createTaskWorkspace, workspaceFilesystems } from './workspace.ts'; +import { commandCheckDeps } from './checks.ts'; +import { PreMergeCoordinator, type PreMergeAuthorization } from './pre-merge.ts'; +import type { RunnerCoordinator } from './coordinator.ts'; /** * The production runner (#91): D's real workspace, launcher, recovery, export and removal, under the database's runner @@ -142,6 +146,23 @@ export function claudeLauncher(o: { imageId: string; runnerRoot: string; runnerO }; } +/** D's credential-free read-only container for one item's exact `cmd:` argv arrays. */ +export function runnerCommandLauncher(o: { imageId: string; runnerRoot: string; runnerOwner: string; + start?: typeof startRunnerCommandInvocation }) { + return (input: Parameters[0]['invocation'], commands: string, + workspace: Parameters[0]) => { + if (!isUuidV4(input.attemptId)) throw new Error('Attempt ID must be a UUID v4.'); + const directory = join(o.runnerRoot, o.runnerOwner, 'attempts', input.attemptId, 'input'); + mkdirSync(directory, { mode: 0o755 }); + chmodSync(directory, 0o755); + const schema = join(directory, 'schema.json'); + writeFileSync(schema, commands, { mode: 0o444, flag: 'wx' }); + chmodSync(schema, 0o444); + return (o.start ?? startRunnerCommandInvocation)({ invocation: input, filesystems: workspaceFilesystems(workspace), + inputDirectory: directory, imageId: o.imageId, prompt: '', networkAllocationId: randomUUID() }); + }; +} + /** * What startup recovery left for a person (runner-lifecycle.md: never removed automatically), one line each. Docker * labels and file names are not codeboost's, so each is quoted: a newline or control character in one cannot forge a line. @@ -165,6 +186,9 @@ export interface RunnerAssembly { readonly env?: NodeJS.ProcessEnv; /** Tests only: the delay of the publisher's short retry (SHORT_RETRY_MS, 30 s). */ readonly shortRetryMs?: number; + /** F5 production refresh/rebase/check coordinator, built against the configured task PR. */ + readonly preMerge?: (runner: RunnerCoordinator, inspect: (signal?: AbortSignal) => Promise<{ base: string; head: string }>, + authorize: (signal: AbortSignal) => Promise) => PreMergeCoordinator; readonly sources: ExecutionSources; readonly findings: SafetyFindings; readonly recovery: RecoveryReport; @@ -200,34 +224,46 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd const identity = review.identity; const rebasePlanKey = identityKey(identity); let repositoryPromise: Promise | undefined, rebaserPromise: Promise | undefined; + let authorizePreMerge: ((signal: AbortSignal) => Promise) | undefined; const getRepository = () => repositoryPromise ??= openRunnerRepository({ runnerRoot: config.root, runnerOwner, - repositoryId: identity.repositoryId, source: review.repository }).then(repository => { + repositoryId: identity.repositoryId, source: review.repository }).then(async repository => { // The review and rebase recovery read the same runner-owned repository that execution writes. if (review.runnerRepository !== undefined && review.runnerRepository !== repository.path) throw new Error(`runnerRepository (${review.runnerRepository}) is not the runner's repository (${repository.path}); remove it from the configuration.`); + // A fresh bare repository has no objects yet. Seed the recorded snapshot before selecting it for review reads; + // otherwise the server cannot render the initial review, and no execution attempt can get far enough to import it. + const snapshot = service.store.getSnapshot(identity); + for (const commit of new Set([snapshot.base, snapshot.head])) await retainSnapshotCommit(repository, commit); review.runnerRepository = repository.path; return repository; }); - // F3-F4 assemble the trusted local rewrite/conflict engine here so startup can recover its durable markers. No - // production action starts it yet: F5-F6 must first own base/head refresh, admission, checks, push and merge handoff. - // Exposing the raw rebaser before that coordinator exists would let a caller bypass those required guards. + // Assemble the trusted local rewrite/conflict engine here so startup can recover its durable markers. The F5 + // coordinator below is its only live entry point; exposing the raw rebaser would bypass refresh, admission and checks. const getRebaser = () => rebaserPromise ??= getRepository().then(repository => new GitRebaser({ repository, runnerRoot: config.root, runnerOwner, committer: config.committer, - onProcessStarting: attemptId => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, null, 'spawning'), - onProcessGroup: (attemptId, group) => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, 'spawning', group), - onProcessGroupSettled: (attemptId, group) => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, null), - onProcessUnsettled: (attemptId, group) => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, 'unsettled'), - onResultPrepared: (attemptId, head, history, conflicts) => service.store.prepareRebaseResult(rebasePlanKey, attemptId, head, history, conflicts), + onProcessStarting: attemptId => o.capability.run(() => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, null, 'spawning')), + onProcessGroup: (attemptId, group) => o.capability.run(() => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, 'spawning', group)), + onProcessGroupSettled: (attemptId, group) => o.capability.run(() => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, null)), + onProcessUnsettled: (attemptId, group) => o.capability.run(() => service.store.setRebaseProcessGroup(rebasePlanKey, attemptId, group, 'unsettled')), + onResultPrepared: (attemptId, head, history, conflicts) => o.capability.run(() => service.store.prepareRebaseResult(rebasePlanKey, attemptId, head, history, conflicts)), onResultState: (attemptId, state) => state === 'ready' - ? service.store.completeRebaseResult(rebasePlanKey, attemptId) - : service.store.setRebaseResultState(rebasePlanKey, attemptId, state), + ? o.capability.run(() => service.store.completeRebaseResult(rebasePlanKey, attemptId)) + : o.capability.run(() => service.store.setRebaseResultState(rebasePlanKey, attemptId, state)), resolveForeignConflict: createForeignConflictResolver({ store: service.store, identity, planKey: rebasePlanKey, repository, runnerOwner, image, token, + authorize: signal => { + if (!authorizePreMerge) throw new GuardRefusal('Issue trust admission is not configured.'); + return authorizePreMerge(signal).then(async authorization => { + await authorization.refresh(); + return () => authorization.validate(); + }); + }, limits: { ...EXECUTE_STORAGE, ...config.limits } }) })); const recovery = await recoverStartup({ store: service.store, runnerOwner, runnerRoot: config.root, diagnosticsDir, diagnosticsCapBytes: config.diagnosticsCapBytes, deps: o.recovery ? o.recovery(image) : dRecoveryDeps(image, { abort: async (attemptId, resultHead, state) => (await getRebaser()).abort(attemptId, resultHead, state) }, rebasePlanKey) }); + service.store.settleInterruptedPreMergeActions(identity); const repository = await getRepository(); const imageId = image(); const workspace = createTaskWorkspace({ store: service.store, runnerRoot: config.root, runnerOwner, repository, imageId, @@ -274,8 +310,11 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd vendor: () => 'claude', }; const findings = new SafetyFindings(service.store, o.capability); - const deps = executionDeps(service.store, workspace, claudeLauncher({ imageId, runnerRoot: config.root, runnerOwner, token }), sources, runnerOwner, findings, - { diagnostics: { directory: diagnosticsDir, capBytes: config.diagnosticsCapBytes ?? DEFAULT_DIAGNOSTICS_CAP_BYTES } }); + const deps = combineRunnerDeps( + executionDeps(service.store, workspace, claudeLauncher({ imageId, runnerRoot: config.root, runnerOwner, token }), sources, runnerOwner, findings, + { diagnostics: { directory: diagnosticsDir, capBytes: config.diagnosticsCapBytes ?? DEFAULT_DIAGNOSTICS_CAP_BYTES } }), + commandCheckDeps(service.store, workspace, + runnerCommandLauncher({ imageId, runnerRoot: config.root, runnerOwner }), review.allowedCommands ?? [], runnerOwner)); const github = review.github; // Never a `url` here (#101 review, finding 6): the push goes to the configured repository on GH_HOST, from the runner's // own repository. Only commits the ledger records as codeboost's may be overwritten. The F3 rebase foundation can @@ -286,5 +325,15 @@ export async function setUpRunner(o: { service: ReviewService; capability: Shutd const env = o.env as NodeJS.ProcessEnv; const publisher = (closing: () => boolean) => new PullRequestPublisher(service.store, { checks: new GhAlreadyFixedGateway({ repository: github.repository, env }), pulls: new GhPullRequestGateway({ repository: github.repository, env }), pusher, closing }, { repository: github.repository, baseBranch: base }); - return { deps, sources, findings, recovery, publisher, env }; + const rebaser = await getRebaser(); + const preMerge = (runner: RunnerCoordinator, + inspect: (signal?: AbortSignal) => Promise<{ base: string; head: string }>, + authorize: (signal: AbortSignal) => Promise) => { + authorizePreMerge = authorize; + return new PreMergeCoordinator(service, runner, rebaser, { + inspect, + fetch: (pair, signal) => pusher.fetchCommits([pair.base, pair.head], signal), + }, undefined, o.capability, authorize); + }; + return { deps, sources, findings, recovery, publisher, env, preMerge }; } diff --git a/runner/rebase-conflict.ts b/runner/rebase-conflict.ts index ffe9894f..707da042 100644 --- a/runner/rebase-conflict.ts +++ b/runner/rebase-conflict.ts @@ -63,6 +63,8 @@ export interface ConflictResolverOptions { readonly image: () => string; readonly token: string; readonly limits: TaskStorageLimits; + /** Fresh authorization re-read immediately before the credentialed conflict agent launches. */ + readonly authorize?: (signal: AbortSignal) => Promise<() => void | Promise>; readonly deps?: Partial; } export type ForeignConflictResolverOptions = ConflictResolverOptions; @@ -260,11 +262,11 @@ export function createConflictResolver(options: ConflictResolverOptions): (input if (remaining < 1) throw deadlineError(); return remaining; }; - const bounded = async (operation: Promise): Promise => { + // Takes a thunk so no operation (including caller-supplied hooks) starts before the deadline check and timer exist. + const bounded = async (begin: () => Promise): Promise => { const remaining = Math.floor(deadline - performance.now()); if (remaining < 1) { retainOwnership = true; - void operation.catch(() => { /* durable recovery owns any late result */ }); throw new RebaseResourcesUnsettled('Conflict child settlement exceeded the rebase work deadline.'); } let timer: ReturnType | undefined; @@ -275,6 +277,7 @@ export function createConflictResolver(options: ConflictResolverOptions): (input reject(new RebaseResourcesUnsettled('Conflict child settlement exceeded the rebase work deadline.')); }, remaining); }); + const operation = new Promise(resolve => resolve(begin())); try { return await Promise.race([operation, expired]); } finally { if (timer) clearTimeout(timer); @@ -288,22 +291,22 @@ export function createConflictResolver(options: ConflictResolverOptions): (input // Profile creation accepts only a canonical cleanup root. Preserve the UUID leaf while resolving any configured // root aliases (for example macOS /var -> /private/var) before handing the path across subsystem boundaries. staging = realpathSync(staging); - const clone = await bounded(deps.clone({ source: options.repository.path, parent: staging, + const clone = await bounded(() => deps.clone({ source: options.repository.path, parent: staging, taskId: options.planKey, head: input.baseHead, timeoutMs: operationBudget(), signal: input.signal, processLifecycle })); const conflictSnapshot = snapshotConflictPaths(input.repository, input.files); try { - filesystems = await bounded(deps.allocate(clone, options.limits, imageId, + filesystems = await bounded(() => deps.allocate(clone, options.limits, imageId, { runnerOwner: options.runnerOwner, attemptId: childAttemptId, allocationId }, { signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); } catch (error) { if (error instanceof AggregateError) retainOwnership = true; throw error; } - await bounded(deps.importPaths(filesystems, conflictSnapshot, MAX_CONFLICT_SNAPSHOT_BYTES, + await bounded(() => deps.importPaths(filesystems!, conflictSnapshot, MAX_CONFLICT_SNAPSHOT_BYTES, { imageId, signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); - const links = await bounded(deps.snapshotLinks(filesystems!, input.files, + const links = await bounded(() => deps.snapshotLinks(filesystems!, input.files, { imageId, signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); - const treeCheck: TaskTreeCheck = await bounded(deps.checkTree(filesystems!, + const treeCheck: TaskTreeCheck = await bounded(() => deps.checkTree(filesystems!, { base: input.baseHead, paths: input.files, imageId, signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); const inputDirectory = join(staging, 'input'); @@ -319,13 +322,21 @@ export function createConflictResolver(options: ConflictResolverOptions): (input const prompt = `Resolve source commit ${JSON.stringify(input.commit)} while it is replayed onto base commit ${JSON.stringify(input.baseHead)}. ` + ownership + `Resolve the in-progress conflict in exactly these paths: ${JSON.stringify(input.files)}. ` + 'Edit only those paths. Do not create commits or change repository metadata. Preserve the intent of both sides and leave each path in its final resolved form.'; + if (options.authorize) { + const authorizationSignal = input.signal ?? new AbortController().signal; + const validate = await bounded(() => options.authorize!(authorizationSignal)); + await bounded(async () => validate()); + input.signal?.throwIfAborted(); + } assertCurrentContext(); handle = deps.start({ invocation, filesystems, inputDirectory, imageId, prompt, networkAllocationId, treeCheck, cleanupRoot: staging, processLifecycle }, options.token, { invocationBudget: operationBudget }); + // The child is already running; if bounded() refuses before awaiting it, durable recovery owns any late result. + void handle.settled.catch(() => { /* durable recovery owns any late result */ }); const cancel = () => handle!.cancel(stopReason(input.signal!)); if (input.signal?.aborted) cancel(); else input.signal?.addEventListener('abort', cancel, { once: true }); - const result = await bounded(handle.settled).finally(() => input.signal?.removeEventListener('abort', cancel)); + const result = await bounded(() => handle!.settled).finally(() => input.signal?.removeEventListener('abort', cancel)); if (processGroup !== null) { retainOwnership = true; throw new Error('The conflict resolver could not confirm settlement of its Docker client.'); @@ -338,10 +349,10 @@ export function createConflictResolver(options: ConflictResolverOptions): (input throw new Error('The conflict resolver returned a result for another invocation.'); if (result.exitCode !== 0 || result.stopReason) throw new Error(`The conflict resolver failed${result.stderr ? `: ${JSON.stringify(result.stderr.slice(0, 2048))}` : '.'}`); input.signal?.throwIfAborted(); - const manifest = await bounded(deps.inspect(filesystems!, { base: input.baseHead, linkSnapshot: links, + const manifest = await bounded(() => deps.inspect(filesystems!, { base: input.baseHead, linkSnapshot: links, imageId, signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); assertResolvedManifest(manifest, input.files); - const entries = await bounded(deps.exportPaths(filesystems!, input.files, MAX_CONFLICT_SNAPSHOT_BYTES, + const entries = await bounded(() => deps.exportPaths(filesystems!, input.files, MAX_CONFLICT_SNAPSHOT_BYTES, { imageId, signal: input.signal, processLifecycle, timeoutMs: operationBudget() })); input.signal?.throwIfAborted(); if (entries.length !== input.files.length || entries.some((entry, index) => entry.path !== input.files[index])) @@ -372,10 +383,10 @@ export function createConflictResolver(options: ConflictResolverOptions): (input const cleanup: unknown[] = []; if (handle && input.signal?.aborted) handle.cancel(stopReason(input.signal)); if (filesystems && !retainOwnership) try { - await bounded(deps.remove(filesystems!, { processLifecycle, timeoutMs: operationBudget() })); + await bounded(() => deps.remove(filesystems!, { processLifecycle, timeoutMs: operationBudget() })); } catch (error) { cleanup.push(error); retainOwnership = true; } if (!retainOwnership) try { - await bounded(deps.removeStaging(staging, operationBudget(), processLifecycle)); + await bounded(() => deps.removeStaging(staging, operationBudget(), processLifecycle)); } catch (error) { cleanup.push(error); retainOwnership = true; } if (!cleanup.length && !retainOwnership) { try { options.store.clearRebaseConflict(options.planKey, input.attemptId, childAttemptId); } diff --git a/runner/rebase.ts b/runner/rebase.ts index b914d5ed..a567148f 100644 --- a/runner/rebase.ts +++ b/runner/rebase.ts @@ -14,7 +14,9 @@ const MAX_REBASE_COMMITS = 500; const PROCESS_SETTLEMENT_RESERVE_MS = 13_000; // Cleanup receives an operation-wide 30 s: 13 s to settle a cleanup process group and 17 s for Git and follow-up calls. const CLEANUP_RESERVE_MS = 30_000; -const MIN_REBASE_TIMEOUT_MS = CLEANUP_RESERVE_MS + PROCESS_SETTLEMENT_RESERVE_MS + 1; +export const MIN_REBASE_CLEANUP_TIMEOUT_MS = CLEANUP_RESERVE_MS; +export const MIN_REBASE_TIMEOUT_MS = CLEANUP_RESERVE_MS + PROCESS_SETTLEMENT_RESERVE_MS + 1; +export const MAX_REBASE_TIMEOUT_MS = 120_000; export interface RebaseMapping { oldSha: string; newSha: string } export interface RebaseConflictInput { @@ -101,7 +103,7 @@ export class GitRebaser { if (!options.committer.name || !options.committer.email || /[<>\n\r\0]/.test(options.committer.name + options.committer.email)) throw new Error('A valid rebase committer is required.'); if (options.timeoutMs !== undefined && (!Number.isSafeInteger(options.timeoutMs) || - options.timeoutMs < MIN_REBASE_TIMEOUT_MS || options.timeoutMs > 120_000)) + options.timeoutMs < MIN_REBASE_TIMEOUT_MS || options.timeoutMs > MAX_REBASE_TIMEOUT_MS)) throw new Error('Invalid rebase deadline.'); const git = configuredGit(); this.#options = options; @@ -111,7 +113,8 @@ export class GitRebaser { } async run(input: { attemptId: string; oldBase: string; oldHead: string; oldHistory: readonly string[]; onto: string; - ledger?: readonly { sha: string; owner: string | null; origin: 'owned' | 'foreign' }[]; signal?: AbortSignal }): Promise { + ledger?: readonly { sha: string; owner: string | null; origin: 'owned' | 'foreign' }[]; signal?: AbortSignal; + timeoutMs?: number }): Promise { const { attemptId, oldBase, oldHead, oldHistory, onto, signal } = input; rebaseRef(attemptId); for (const value of [oldBase, oldHead, onto]) if (!COMMIT_ID.test(value)) throw new Error('A full commit ID is required for rebasing.'); @@ -123,7 +126,7 @@ export class GitRebaser { ledger.set(entry.sha, { owner: entry.owner, origin: entry.origin }); } signal?.throwIfAborted(); - const scope = this.#scope(attemptId, signal); + const scope = this.#scope(attemptId, signal, input.timeoutMs); const ref = rebaseRef(attemptId); const existing = await this.#call(this.#options.repository.path, ['show-ref', '--verify', '--quiet', ref], scope); this.#throwIfCancelled(existing, scope); @@ -260,10 +263,10 @@ export class GitRebaser { /** Startup/live recovery for an attempt whose Store marker still exists. */ async abort(attemptId: string, resultHead?: string, - resultState: 'none' | 'prepared' | 'uncertain' | 'refused' | 'ready' = 'ready'): Promise { + resultState: 'none' | 'prepared' | 'uncertain' | 'refused' | 'ready' = 'ready', timeoutMs?: number): Promise { const ref = rebaseRef(attemptId), path = this.#path(attemptId), stat = lstatSync(path, { throwIfNoEntry: false }); if (resultHead !== undefined && !COMMIT_ID.test(resultHead)) throw new Error('A full retained result ID is required.'); - const scope = this.#scope(attemptId); + const scope = this.#scope(attemptId, undefined, timeoutMs, true); if (stat && (!stat.isDirectory() || stat.isSymbolicLink())) throw new Error('The rebase workspace is not a plain directory.'); await this.#remove(path, scope); if (resultState === 'refused') return; @@ -548,8 +551,13 @@ export class GitRebaser { return outcome; } - #scope(attemptId: string, signal?: AbortSignal): CallScope { - const timeout = this.#options.timeoutMs ?? 120_000, deadline = performance.now() + timeout; + #scope(attemptId: string, signal?: AbortSignal, requestedTimeoutMs?: number, cleanupOnly = false): CallScope { + const configured = this.#options.timeoutMs ?? MAX_REBASE_TIMEOUT_MS; + const timeout = requestedTimeoutMs === undefined ? configured : Math.min(requestedTimeoutMs, configured); + const minimum = cleanupOnly ? MIN_REBASE_CLEANUP_TIMEOUT_MS : MIN_REBASE_TIMEOUT_MS; + if (!Number.isSafeInteger(timeout) || timeout < minimum || timeout > MAX_REBASE_TIMEOUT_MS) + throw new Error('Invalid rebase deadline.'); + const deadline = performance.now() + timeout; return { attemptId, signal, deadline, workDeadline: deadline - CLEANUP_RESERVE_MS }; } diff --git a/runner/review.ts b/runner/review.ts index 25dd7d99..039ea92e 100644 --- a/runner/review.ts +++ b/runner/review.ts @@ -6,17 +6,20 @@ import type { PlanIdentity } from '../core/identity.ts'; import { readHistory } from '../git/history.ts'; import { execFileSync } from 'node:child_process'; import { HARDENED_GIT_OPTIONS, hardenedGitEnvironment } from '../scripts/git-environment.ts'; -import type { BaseEntry, PlanContext } from '../core/plan.ts'; +import { commandArgv, PlanError, type BaseEntry, type PlanContext } from '../core/plan.ts'; 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'; +import { commandDigest } from './checks.ts'; export interface ReviewConfig { database: string; repository: string; - /** The runner-owned repository (#87) holding the commits codeboost makes; required once the task has any. */ + /** The runner-owned repository (#87) holding the production task branch, including its original imported head. */ runnerRepository?: string; identity: PlanIdentity; pathIdentity: { caseSensitive: boolean; unicodeNormalization: 'none' | 'NFC' }; demo?: boolean; github?: GhMergeConfig; + /** Exact complete argv arrays a stored plan may execute as `cmd:` acceptance checks. */ + allowedCommands?: readonly (readonly string[])[]; /** The runner block, parsed by `parseRunnerConfig`. Its presence also makes the merge target the task's published PR (#121). */ runner?: unknown } export class ReviewService { @@ -31,6 +34,10 @@ export class ReviewService { 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; + if (config.allowedCommands !== undefined && (!Array.isArray(config.allowedCommands) + || config.allowedCommands.some(argv => !Array.isArray(argv) || argv.length === 0 + || argv.some(arg => typeof arg !== 'string' || !arg.isWellFormed() || arg.includes('\0'))))) + throw new Error('allowedCommands must contain complete literal argv arrays.'); 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; @@ -38,19 +45,30 @@ export class ReviewService { }; this.store = new Store(config.database, this.#pathKey); } + /** Stable binding for preparation evidence; a restart with a different command policy invalidates old readiness. */ + commandPolicyDigest(): string { return commandDigest(this.config.allowedCommands ?? []); } close() { this.store.close(); } /** - * Where the task's reviewed commits are. Once the runner has committed for the task (a completed writable attempt that - * made a commit, #87), its branch is the runner's: the head is the one the Store recorded with that commit, in the - * runner-owned repository. Before that, the user's repository and its HEAD. Owned ledger entries alone do not decide it: - * a reviewed branch in the user's repository (the demo, a planted experiment) carries them too. + * Where the task's reviewed commits are. Production configures the runner-owned repository after importing the task's + * original head, so it remains authoritative even when every execution attempt is unchanged. Without one, the review + * observes the user's repository and HEAD (demo and planted-review behavior). */ reviewRepository(): { path: string; runnerOwned: boolean } { - if (!this.store.hasRunnerCommit(this.config.identity)) return { path: this.config.repository, runnerOwned: false }; - if (!this.config.runnerRepository) throw new Error('This task has runner commits, so its review needs the runner-owned repository, which is not configured.'); - return { path: this.config.runnerRepository, runnerOwned: true }; + if (this.config.runnerRepository) return { path: this.config.runnerRepository, runnerOwned: true }; + if (this.store.hasRunnerCommit(this.config.identity)) + throw new Error('This task has runner commits, so its review needs the runner-owned repository, which is not configured.'); + return { path: this.config.repository, runnerOwned: false }; } - load() { + load(options: { maxDurationMs?: number } = {}) { + const maxDurationMs = options.maxDurationMs ?? 30_000; + if (!Number.isSafeInteger(maxDurationMs) || maxDurationMs < 1 || maxDurationMs > 30_000) + throw new Error('Review duration budget must be a positive integer no larger than 30000 ms.'); + const deadline = performance.now() + maxDurationMs; + const remaining = () => { + const ms = deadline - performance.now(); + if (ms <= 0) throw new Error('Review load exceeded its overall deadline.'); + return Math.max(1, Math.ceil(ms)); + }; const { identity } = this.config; const reviewVersion = this.store.reviewVersion(identity); const plan = this.store.getPlan(identity); @@ -59,11 +77,12 @@ export class ReviewService { // HEAD changes in the user's repository are observed; no Git mutation is performed by the review service. Runner // commits move the head only through the Store, in the same transaction as their ledger entries, so there the // 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'); + const history = readHistory(reviewed.path, snapshot.base, reviewed.runnerOwned ? snapshot.head : 'HEAD', + { maxDurationMs: remaining() }); if (history.head !== snapshot.head) snapshot = this.store.recordHistory(identity, { revision: plan.revision, snapshotId: snapshot.id, reviewVersion }, history.base, history.head, []); const pathKey = this.#pathKey; const ledger = this.store.getLedger(identity); - const raw = linkHistory(plan, history, this.store.ownership(identity, plan.revision), pathKey, {}, + const raw = linkHistory(plan, history, this.store.ownership(identity, plan.revision), pathKey, { maxDurationMs: remaining() }, new Set(ledger.filter(entry => entry.conflictResolved).map(entry => entry.sha))); 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])); @@ -150,22 +169,36 @@ export class ReviewService { if (executionUnapproved.has(item.id)) reasons.push('Execution approval is out of date'); if (!reasons.length) reasons.push('Code or plan definition changed'); } + let tests: string; + try { + const commands = item.acceptance.filter(check => check.type === 'cmd').map(check => commandArgv(check.text)); + tests = commands.length + ? this.store.commandChecksPassed(identity, item.id, snapshot.head, commandDigest(commands)) ? '✓ Passed' : '– Not run' + : '– No tests defined'; + } catch (error) { + // Plans imported before exact command spawning rejected malformed Unicode may contain an escaped lone surrogate. + // Keep the review repairable while making the legacy command visibly and durably non-passing. + if (!(error instanceof PlanError)) throw error; + tests = '✕ Invalid command'; + } return { ...item, state: states[item.id], count: owned.length, ambiguousCount: ambiguous, reasons, before, staleKey: staleKey(item), - checks: { attributed: ambiguous ? `! ${ambiguous} ambiguous` : owned.length ? '✓ Attributed' : '– No changes', scope: outside.length ? `✕ ${new Set(outside).size} out of scope` : owned.length ? '✓ In scope' : '– No changes', tests: item.acceptance.some(check => check.type === 'cmd') ? '– Not run' : '– No tests defined', ai: '– Not run' }, outside: [...new Set(outside)], + checks: { attributed: ambiguous ? `! ${ambiguous} ambiguous` : owned.length ? '✓ Attributed' : '– No changes', scope: outside.length ? `✕ ${new Set(outside).size} out of scope` : owned.length ? '✓ In scope' : '– No changes', tests, ai: '– Not run' }, outside: [...new Set(outside)], }; }); const token = createHash('sha256').update(JSON.stringify({ expected, saved, plan, segments })).digest('hex'); + remaining(); 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, 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 { identity } = this.config, plan = this.store.getPlan(identity), snapshot = this.store.getSnapshot(identity); + const repository = this.reviewRepository().path; const pathKey = this.#pathKey; // Hardened like every repository Git call (#82, #83): no replace objects, hooks, network or inherited environment. 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, + cwd: repository, env: hardenedGitEnvironment(), encoding: 'utf8', maxBuffer, stdio: ['ignore', 'pipe', 'pipe'], }); if (this.#baseEntries?.base !== treeHead) { @@ -180,7 +213,8 @@ export class ReviewService { } // 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: [] }; + return { identity, issue: plan.issue, baseEntries, pathKey, + allowedCommands: (this.config.allowedCommands ?? []).map(argv => [...argv]) }; } planContext(): PlanContext { return this.planContextAt(); } /** When a checkpoint exists, edits are validated against the audited task head and completed work is excluded. */ diff --git a/runner/runner-repository.ts b/runner/runner-repository.ts index d1bb86f1..7a59511d 100644 --- a/runner/runner-repository.ts +++ b/runner/runner-repository.ts @@ -103,6 +103,12 @@ export async function ensureCommit(repository: RunnerRepository, head: string, o if (!(await exists(repository.path, `${head}^{commit}`, options))) throw new Error(`The source repository has no commit ${head}.`); } +/** Import a persisted snapshot commit and anchor it so repository maintenance cannot prune it. */ +export async function retainSnapshotCommit(repository: RunnerRepository, head: string, options: GitCallOptions = {}): Promise { + await ensureCommit(repository, head, options); + await git(repository.path, ['update-ref', `refs/codeboost/remote-commits/${head}`, head], options); +} + /** * Take a runner commit in from the bundle `commitTaskChanges` returned, under the attempt's ref, and check that it is * the commit the runner made: the bundle's one ref is `head`, and `head` is a single commit on top of `base`. diff --git a/runner/store.ts b/runner/store.ts index bd847585..3e359b50 100644 --- a/runner/store.ts +++ b/runner/store.ts @@ -78,6 +78,18 @@ export function publishActionResponse(record: PublishRecord) { return { outcome: record.outcome, draft: record.draft, message: record.message, ...(record.action ? { action: record.action } : {}), ...(record.reconcile ? { reconcile: true } : {}), ...(record.number === undefined ? {} : { number: record.number }), ...(record.url === undefined ? {} : { url: record.url }) }; } +export interface PreMergeActionResult { + state: 'ready' | 'review-required' | 'failed'; + base: string; head: string; checked: readonly string[]; reason: string | null; +} +export interface PreMergeReadiness { + stateVersion: number; reviewVersion: number; snapshotId: string; base: string; head: string; commandPolicyDigest: string; +} +export const MAX_REWRITE_LINEAGE_ROWS = 1_000; +/** What a completed `prepare-merge` action replays after its background coordinator settles. */ +export function preMergeActionResponse(result: PreMergeActionResult) { + return { outcome: result.state, base: result.base, head: result.head, checked: result.checked, reason: result.reason }; +} export interface TaskRecord { planKey: string; status: TaskStatus; stateVersion: number; contextGeneration: number; assignmentId: string; referencedCodeHash: string; currentAttemptId: string | null; requeuePending: boolean; cancelRequested: string | null; rebaseInProgress: unknown; budgetDeadline: number | null; @@ -503,7 +515,8 @@ export class Store { * task's PRs may be in flight. */ beginMergeAttempt(identity: PlanIdentity, expected: ReviewState & { reviewVersion: number }, reviewedHead: string, queueWatermark: string | null = null, kind: MergeAttempt['kind'] = 'queue', actionId: string | null = null, expectedTaskStateVersion: number | null = null, - target: { pullRequest: number; openingId: string | null } | null = null): MergeAttempt { + target: { pullRequest: number; openingId: string | null } | null = null, requirePreparation = false, + commandPolicyDigest: string | null = null): MergeAttempt { sha(reviewedHead); if (target !== null && (!Number.isSafeInteger(target.pullRequest) || target.pullRequest < 1 || (target.openingId !== null && typeof target.openingId !== 'string'))) throw new Error('Invalid merge target.'); @@ -524,6 +537,11 @@ export class Store { if (!MERGEABLE_STATUSES.includes(task.status as TaskStatus)) throw new GuardRefusal(`The task is ${task.status}; merge it from review.`); if (this.#activeAttempt(key)) throw new GuardRefusal('An attempt is still active for this task; it cannot be merged.'); if (expectedTaskStateVersion !== null && task.state_version !== expectedTaskStateVersion) throw new GuardRefusal('Stale task state. Reload before writing.'); + if (requirePreparation && !this.preMergeReady(identity, { + stateVersion: task.state_version as number, reviewVersion: expected.reviewVersion, + snapshotId: expected.snapshotId, base: this.getSnapshot(identity).base, head: reviewedHead, + commandPolicyDigest: commandPolicyDigest ?? '', + })) throw new GuardRefusal('Pre-merge preparation has not completed for the current review. Prepare the merge again.'); const current = this.getMergeAttempt(identity); if (current?.state === 'submitting' || current?.state === 'queued') throw new Error('A merge-queue attempt is already active.'); if (current?.state === 'merged') throw new Error('The reviewed pull request is already merged.'); @@ -989,6 +1007,43 @@ export class Store { this.getSnapshot(identity, snapshotId); return this.#db.prepare('SELECT old_sha,new_sha FROM rewrites WHERE key=? AND snapshot_id=? ORDER BY old_sha').all(identityKey(identity), snapshotId).map(row => ({ oldSha: row.old_sha as string, newSha: row.new_sha as string })); } + /** Whether `descendant` was produced by one or more durable rebase mappings from `ancestor`. */ + isRewrittenHead(identity: PlanIdentity, ancestor: string, descendant: string): boolean { + sha(ancestor); sha(descendant); + const reverse = new Map>(); + for (const row of this.#rewriteLineage(identity)) { + const prior = reverse.get(row.new_sha as string) ?? new Set(); + prior.add(row.old_sha as string); reverse.set(row.new_sha as string, prior); + } + const pending = [descendant], seen = new Set(pending); + while (pending.length) for (const prior of reverse.get(pending.pop()!) ?? []) { + if (prior === ancestor) return true; + if (!seen.has(prior)) { seen.add(prior); pending.push(prior); } + } + return false; + } + /** Durable predecessors of a rewritten commit, nearest first, across every retained rebase mapping. */ + rewrittenAncestors(identity: PlanIdentity, descendant: string): string[] { + sha(descendant); + const reverse = new Map>(); + for (const row of this.#rewriteLineage(identity)) { + const prior = reverse.get(row.new_sha as string) ?? new Set(); + prior.add(row.old_sha as string); reverse.set(row.new_sha as string, prior); + } + const answer: string[] = [], pending = [descendant], seen = new Set(pending); + for (let index = 0; index < pending.length; index++) for (const prior of reverse.get(pending[index]!) ?? []) if (!seen.has(prior)) { + seen.add(prior); answer.push(prior); pending.push(prior); + } + return answer; + } + /** A bounded safety scan: truncating rewrite evidence could misclassify a remote head as safe. */ + #rewriteLineage(identity: PlanIdentity): { old_sha: string; new_sha: string }[] { + const rows = this.#db.prepare('SELECT old_sha,new_sha FROM rewrites WHERE key=? ORDER BY rowid DESC LIMIT ?') + .all(identityKey(identity), MAX_REWRITE_LINEAGE_ROWS + 1); + if (rows.length > MAX_REWRITE_LINEAGE_ROWS) + throw new GuardRefusal(`Rewrite lineage exceeds the ${MAX_REWRITE_LINEAGE_ROWS}-row safety limit; start a fresh review before preparing a merge.`); + return rows as { old_sha: string; new_sha: string }[]; + } /** Values must be computed by the runner from this exact revision/snapshot, never supplied by a browser. */ saveReview(identity: PlanIdentity, expected: ReviewState, approvals: readonly Approval[], choices: readonly SegmentChoice[]): void { const key = identityKey(identity); @@ -1058,7 +1113,7 @@ export class Store { /** Before execution, approvals belong to the current snapshot; after a completed item, this revision's approvals survive its own commits. */ unapprovedExecutionItems(identity: PlanIdentity, revision: number): string[] { const key = identityKey(identity), plan = this.getPlan(identity, revision); - const snapshotId = this.getSnapshot(identity).id, review = this.getReview(identity); + const currentSnapshot = this.getSnapshot(identity), snapshotId = currentSnapshot.id, review = this.getReview(identity); // An approval inherited across runner commits must come from the context of the completed prefix, not merely from // this revision. Otherwise A -> unrelated B approval -> A could reuse B's approval when the prefix-head guard passes. const completedAt: Record = Object.create(null); @@ -1089,11 +1144,20 @@ export class Store { } } } - const reviewedExecutionSnapshot = (value: ReviewState) => value.snapshotId === snapshotId || prefixSnapshots.has(value.snapshotId); + const reviewedExecutionSnapshot = (value: ReviewState) => { + if (value.snapshotId === snapshotId || prefixSnapshots.has(value.snapshotId)) return true; + // Head-based evidence carries approvals only across a completed prefix's own output and its validated rewrites. + // Before execution this gate does not compare fingerprints, so approvals stay bound to the current snapshot. + if (prefixSnapshots.size === 0) return false; + const evidenceHead = this.getSnapshot(identity, value.snapshotId).head; + return evidenceHead === currentSnapshot.head || this.isRewrittenHead(identity, evidenceHead, currentSnapshot.head); + }; // 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. + // Every choice for this revision counts, whatever its snapshot: approvals can survive head moves (equal or rewritten + // heads, completed prefixes), so a choice made on a snapshot that a later head replaced must still invalidate them. let latestChoiceVersion = -1; - for (const choice of review.choices) if (choice.revision === revision && (prefixSnapshots.size > 0 || choice.snapshotId === snapshotId)) { + for (const choice of review.choices) if (choice.revision === revision) { if (choice.reviewVersion === undefined) latestChoiceVersion = Number.MAX_SAFE_INTEGER; else if (choice.reviewVersion > latestChoiceVersion) latestChoiceVersion = choice.reviewVersion; } @@ -1199,8 +1263,12 @@ export class Store { 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.'); + const currentHead = this.getSnapshot(identity).head; + if (head !== currentHead) { + if (!this.isRewrittenHead(identity, head, currentHead)) + throw new GuardRefusal('The task head no longer ends at the audited completed prefix.'); + head = currentHead; + } return { checkpoint, completed, completedDefinitions, head }; } /** Reconcile audited completion against the current or proposed plan definition. */ @@ -1243,7 +1311,10 @@ export class Store { 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; + const currentSnapshot = this.getSnapshot(identity); + const approvedHead = this.getSnapshot(identity, snapshotId).head; + if (snapshotId === currentSnapshot.id || approvedHead === currentSnapshot.head + || this.isRewrittenHead(identity, approvedHead, currentSnapshot.head)) 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`, @@ -1656,7 +1727,8 @@ export class Store { try { return this.#transaction((): AttemptRecord => { const task = this.#task(key); - if (task.status !== 'running' && task.status !== 'queued') throw new GuardRefusal(`The task is ${task.status}; it cannot start work.`); + const reviewCheck = input.kind === 'check' && MERGEABLE_STATUSES.includes(task.status as TaskStatus); + if (!reviewCheck && task.status !== 'running' && task.status !== 'queued') throw new GuardRefusal(`The task is ${task.status}; it cannot start work.`); if (task.state_version !== input.expectedStateVersion) throw new GuardRefusal('Stale task state. Reload before writing.'); // Requeue claim: exactly one path (I3's requeue, or the user's Resume) clears it, by CAS in this admitting transaction. if (task.requeue_pending === 1 && !input.claimRequeue) throw new GuardRefusal('Recovery is requeueing this task.'); @@ -1665,8 +1737,9 @@ export class Store { if (this.#activeAttempt(key)) throw new GuardRefusal('An attempt is already active for this task.'); if (this.#activeMerge(key)) throw new GuardRefusal('A merge is in progress; wait for its outcome.'); if (task.rebase_in_progress !== null) throw new GuardRefusal('A rebase is in progress for this task.'); - // The whole-task budget (null until it starts) ends admission: the task waits for a person (time-limit mapping). - if (task.budget_deadline !== null && (task.budget_deadline as number) <= now) + // The code-writing task budget (null until execution starts) ends execution admission. Review checks have their + // own exact-head deadline and must remain runnable after implementation ends. + if (!reviewCheck && task.budget_deadline !== null && (task.budget_deadline as number) <= now) throw new RefusalWithEffect('The task time budget has run out; it needs a person.', expire); const current = this.#contextOf(key); if (!sameContext(input.expectedContext, current)) throw new GuardRefusal('The plan, snapshot, assignment or referenced code changed. Reload before starting.'); @@ -1680,7 +1753,8 @@ export class Store { const id = randomUUID(), created = new Date(now).toISOString(); this.#run(`INSERT INTO attempts (id,plan_key,kind,phase,item,state,context,deadline,created_at) VALUES (?,?,?,?,?,'pending',?,?,?)`, id, key, input.kind, ATTEMPT_PHASES[input.kind], input.item ?? null, encode(current), input.deadline, created); - this.#run(`UPDATE tasks SET current_attempt_id=?, status='running', requeue_pending=0, budget_deadline=COALESCE(budget_deadline, ?) WHERE plan_key=?`, id, now + budgetMs, key); + if (reviewCheck) this.#run('UPDATE tasks SET current_attempt_id=? WHERE plan_key=?', id, key); + else this.#run(`UPDATE tasks SET current_attempt_id=?, status='running', requeue_pending=0, budget_deadline=COALESCE(budget_deadline, ?) WHERE plan_key=?`, id, now + budgetMs, key); this.#touch(key); return this.getAttempt(identity, id); }); @@ -1695,7 +1769,18 @@ export class Store { if (!FIRST_REASONS.includes(reason)) throw new GuardRefusal('Unknown stop reason.'); const key = identityKey(identity); return this.#transaction(() => { - const changed = this.#run(`UPDATE attempts SET first_reason=? WHERE plan_key=? AND id=? AND state IN ('pending','running') AND first_reason IS NULL`, reason, key, id).changes === 1; + const changed = this.#run(`UPDATE attempts SET first_reason=? WHERE plan_key=? AND id=? + AND state IN ('pending','running') AND first_reason IS NULL AND stop_reason IS NULL`, reason, key, id).changes === 1; + if (changed) this.#touch(key); + return changed; + }); + } + /** Persist an invocation-owned timeout without recasting it as a user, shutdown, stale, or task-budget stop. */ + recordAttemptTimeout(identity: PlanIdentity, id: string): boolean { + const key = identityKey(identity); + return this.#transaction(() => { + const changed = this.#run(`UPDATE attempts SET stop_reason='timeout' WHERE plan_key=? AND id=? + AND state IN ('pending','running') AND first_reason IS NULL AND stop_reason IS NULL`, key, id).changes === 1; if (changed) this.#touch(key); return changed; }); @@ -1704,7 +1789,8 @@ export class Store { markRunning(identity: PlanIdentity, id: string): boolean { const key = identityKey(identity); return this.#transaction(() => { - const changed = this.#run(`UPDATE attempts SET state='running', started_at=? WHERE plan_key=? AND id=? AND state='pending' AND first_reason IS NULL + const changed = this.#run(`UPDATE attempts SET state='running', started_at=? WHERE plan_key=? AND id=? AND state='pending' + AND first_reason IS NULL AND stop_reason IS NULL AND id=(SELECT current_attempt_id FROM tasks WHERE plan_key=?)`, new Date().toISOString(), key, id, key).changes === 1; if (changed) this.#touch(key); return changed; @@ -1729,7 +1815,8 @@ export class Store { let outcome = classifySettlement({ ...settlement, firstReason, contextCurrent }); if (outcome.state === 'completed' && row.state !== 'running') throw new GuardRefusal('Only a running attempt can complete.'); // The task must still be running; a task never leaves running while an attempt is active, so this is a second safeguard. - if (outcome.state === 'completed' && (task.status !== 'running' || task.cancel_requested !== null)) + const reviewCheck = row.kind === 'check' && MERGEABLE_STATUSES.includes(task.status as TaskStatus); + if (outcome.state === 'completed' && ((!reviewCheck && task.status !== 'running') || task.cancel_requested !== null)) outcome = { state: 'cancelled', reason: this.#closed(task.status) || task.cancel_requested !== null ? 'The task was closed before the result was saved.' : 'The task left the running state before the result was saved.', timeLimit: false }; let result: string | null = null; @@ -1757,6 +1844,20 @@ export class Store { return outcome; }); } + /** Passing evidence for exactly this item, head and command list. Interrupted, failed, stale and older-head rows do not count. */ + commandChecksPassed(identity: PlanIdentity, item: string, head: string, commandsDigest: string): boolean { + sha(head); + if (!/^[a-f0-9]{64}$/.test(commandsDigest)) throw new Error('Invalid command-check digest.'); + // The latest attempt for this item and materialized head is authoritative. A later failure, cancellation or + // interruption must invalidate an older pass even though terminal failures deliberately carry no result payload. + const row = this.#get(`SELECT a.state, a.result FROM attempts a JOIN snapshots s + ON s.key=a.plan_key AND s.id=json_extract(a.context,'$.snapshotId') + WHERE a.plan_key=? AND a.kind='check' AND a.item=? AND json_extract(s.data,'$.head')=? + ORDER BY a.rowid DESC LIMIT 1`, identityKey(identity), item, head); + if (!row || row.state !== 'completed' || typeof row.result !== 'string') return false; + const result = decode>(row.result); + return result.passed === true && result.head === head && result.commandsDigest === commandsDigest; + } /** Cancel task: closes now, or, with an active attempt, stops it first and closes when it settles. */ cancelTask(identity: PlanIdentity, expectedStateVersion: number, actionId: string): 'closed' | 'stopping' { assertUuidV4(actionId, 'Action ID'); @@ -1777,7 +1878,7 @@ export class Store { } if (!active) { this.#closeTask(key, 'cancelled', actionId); return 'closed'; } if (task.cancel_requested !== null) throw new GuardRefusal('The task is already being cancelled.'); - this.#run(`UPDATE attempts SET first_reason='cancelled' WHERE id=? AND first_reason IS NULL`, active.id!); + this.#run(`UPDATE attempts SET first_reason='cancelled' WHERE id=? AND first_reason IS NULL AND stop_reason IS NULL`, active.id!); this.#run('UPDATE tasks SET cancel_requested=? WHERE plan_key=?', actionId, key); this.#touch(key); return 'stopping'; @@ -1798,9 +1899,15 @@ export class Store { this.#run('INSERT INTO user_actions VALUES (?,?,?,?,?,?)', key, action.actionId, action.kind, hash, response, new Date().toISOString()); }; let replaying = false; + let restarting = false; try { return this.#transaction(() => { const prior = saved(); if (prior) { replaying = true; return prior; } + // A background storage failure leaves a tombstone: it blocks older preparation readiness while validation is + // retried, and is replaced atomically by this same request rather than exposed as a replay. + restarting = this.#run(`DELETE FROM user_actions WHERE plan_key=? AND action_id=? AND kind='prepare-merge' + AND request_hash=? AND json_extract(response,'$.ok')=1 + AND json_extract(response,'$.value.outcome')='resendable'`, key, action.actionId, hash).changes === 1; const outer = this.#action; this.#action = { key, actionId: action.actionId }; let value: T; @@ -1813,6 +1920,9 @@ export class Store { if (!replaying && !storage && !(error instanceof ActionIdReused) && !(error instanceof BadRequest) && this.#depth === 0) { const message = error instanceof Error ? bounded(error.message) : 'Refused.'; this.#transaction(() => { + if (restarting) this.#run(`DELETE FROM user_actions WHERE plan_key=? AND action_id=? AND kind='prepare-merge' + AND request_hash=? AND json_extract(response,'$.ok')=1 + AND json_extract(response,'$.value.outcome')='resendable'`, key, action.actionId, hash); if (!this.#get('SELECT 1 FROM user_actions WHERE plan_key=? AND action_id=?', key, action.actionId)) record({ ok: false, error: message, ...(error instanceof UpstreamFailure ? { kind: 'upstream' } : {}) }); if (error instanceof RefusalWithEffect) error.effect(); @@ -1830,10 +1940,66 @@ export class Store { const row = this.#get('SELECT * FROM user_actions WHERE plan_key=? AND action_id=?', identityKey(identity), action.actionId); if (!row) return undefined; if (row.request_hash !== requestHash(action.kind, action.request)) throw new ActionIdReused('Action ID already used for a different request.'); - const outcome = decode<{ ok: boolean; value?: T; error?: string; kind?: string }>(row.response); + const outcome = decode<{ ok: boolean; value?: T & { outcome?: unknown }; error?: string; kind?: string }>(row.response); + if (row.kind === 'prepare-merge' && outcome.ok && outcome.value?.outcome === 'resendable') return undefined; if (!outcome.ok) throw outcome.kind === 'upstream' ? new UpstreamFailure(outcome.error!) : new GuardRefusal(outcome.error!); return { response: outcome.value as T, replayed: true }; } + /** Settle the exact admitted pre-merge action so retry/restart replay never remains at `preparing`. */ + settlePreMergeAction(identity: PlanIdentity, actionId: string, result: PreMergeActionResult, + readiness: PreMergeReadiness | null = null): PreMergeActionResult { + assertUuidV4(actionId, 'Action ID'); + const key = identityKey(identity); + return this.#transaction(() => { + let effective = result; + if (result.state === 'ready') { + const task = this.#task(key), current = this.#current(key), snapshot = this.getSnapshot(identity); + if (!readiness || task.state_version !== readiness.stateVersion || current.review_version !== readiness.reviewVersion + || current.snapshot_id !== readiness.snapshotId || snapshot.base !== readiness.base || snapshot.head !== readiness.head + || result.base !== readiness.base || result.head !== readiness.head + || !/^[a-f0-9]{64}$/.test(readiness.commandPolicyDigest)) { + effective = { ...result, state: 'review-required', reason: 'The task or review changed before preparation readiness was recorded. Prepare the merge again.' }; + readiness = null; + } + } + const response = encode({ ok: true, value: preMergeActionResponse(effective), ...(readiness ? { preMergeReadiness: readiness } : {}) }); + if (response.length > 65536) throw new Error('Action response is too large to record.'); + const changed = this.#run(`UPDATE user_actions SET response=? WHERE plan_key=? AND action_id=? AND kind='prepare-merge' + AND json_extract(response,'$.ok')=1 AND json_extract(response,'$.value.outcome')='preparing'`, + response, key, actionId).changes; + if (changed !== 1) throw new Error('The pre-merge action no longer owns settlement.'); + return effective; + }); + } + /** + * A background preparation whose terminal write failed applied no durable readiness. Replace only its still-pending + * placeholder with a tombstone so the same key can be resent without revealing an older ready preparation. + */ + makePreMergeActionResendable(identity: PlanIdentity, actionId: string): boolean { + assertUuidV4(actionId, 'Action ID'); + return this.#run(`UPDATE user_actions SET response=? WHERE plan_key=? AND action_id=? AND kind='prepare-merge' + AND json_extract(response,'$.ok')=1 AND json_extract(response,'$.value.outcome')='preparing'`, + encode({ ok: true, value: { outcome: 'resendable' } }), identityKey(identity), actionId).changes === 1; + } + /** The latest preparation action is authoritative and must match every current local generation and the exact pair. */ + preMergeReady(identity: PlanIdentity, readiness: PreMergeReadiness): boolean { + if (!/^[a-f0-9]{64}$/.test(readiness.commandPolicyDigest)) return false; + const row = this.#get(`SELECT response FROM user_actions WHERE plan_key=? AND kind='prepare-merge' + AND json_extract(response,'$.ok')=1 ORDER BY rowid DESC LIMIT 1`, + identityKey(identity)); + if (!row) return false; + const saved = decode<{ ok?: unknown; value?: { outcome?: unknown }; preMergeReadiness?: PreMergeReadiness }>(row.response); + return saved.ok === true && saved.value?.outcome === 'ready' && stable(saved.preMergeReadiness) === stable(readiness); + } + /** Startup recovery makes an interrupted preparation definite and replayable before new work is admitted. */ + settleInterruptedPreMergeActions(identity: PlanIdentity): void { + const snapshot = this.getSnapshot(identity); + const result: PreMergeActionResult = { state: 'failed', base: snapshot.base, head: snapshot.head, checked: [], + reason: 'The server restarted before pre-merge preparation completed. Start a new preparation.' }; + this.#run(`UPDATE user_actions SET response=? WHERE plan_key=? AND kind='prepare-merge' + AND json_extract(response,'$.ok')=1 AND json_extract(response,'$.value.outcome')='preparing'`, + encode({ ok: true, value: preMergeActionResponse(result) }), identityKey(identity)); + } /** Append one feedback event. Call inside userAction so the event and its action share one transaction. */ recordFeedback(identity: PlanIdentity, actionId: string, event: { kind: Exclude; item?: string | null; text?: string | null; sourceRef: string; supersedes?: string | null; supersedeLatest?: boolean }): FeedbackEvent { assertUuidV4(actionId, 'Action ID'); @@ -2303,7 +2469,7 @@ export class Store { const key = row.plan_key as string, task = this.#task(key); const contextCurrent = sameContext(decode(row.context), this.#contextOf(key)); let firstReason = row.first_reason as FirstReason | null; - if (firstReason === null && contextCurrent && task.budget_deadline !== null && now >= (task.budget_deadline as number)) firstReason = 'time-limit'; + if (row.kind !== 'check' && firstReason === null && contextCurrent && task.budget_deadline !== null && now >= (task.budget_deadline as number)) firstReason = 'time-limit'; const deadlinePassed = firstReason === null && now >= (row.deadline as number); const outcome = classifySettlement({ firstReason, contextCurrent, exitCode: null, valid: false, @@ -2324,7 +2490,7 @@ export class Store { const status = this.#task(key).status as string; const interrupted = outcome.state === 'failed' && (outcome.reason ?? '').startsWith('Interrupted'); const shutdown = outcome.state === 'cancelled' && firstReason === 'shutdown'; - if (!this.#closed(status) && !gated.includes(status) && (interrupted || shutdown)) { + if (row.kind !== 'check' && !this.#closed(status) && !gated.includes(status) && (interrupted || shutdown)) { this.#run('UPDATE tasks SET requeue_pending=1 WHERE plan_key=?', key); requeued = true; } this.#touch(key); diff --git a/scripts/plant.ts b/scripts/plant.ts index 89d8efa8..4c2e076a 100644 --- a/scripts/plant.ts +++ b/scripts/plant.ts @@ -73,7 +73,10 @@ export function plant(config: ReviewConfig, destination: string, input: PlantInp mappings.push({oldSha:commit.sha,newSha:git(repository,'rev-parse','HEAD')}); } const identity={repositoryId:config.identity.repositoryId,taskId:randomUUID(),planId:randomUUID()}; - const output:ReviewConfig={...config,repository,database:join(root,'review.sqlite'),identity,demo:false,github:undefined}; + // A plant is a new identity over deliberately altered code. External bindings and command authorization from the + // source review are not authority for it; both must be established again for this derived target. + const output:ReviewConfig={...config,repository,database:join(root,'review.sqlite'),identity,demo:false, + github:undefined,runnerRepository:undefined,allowedCommands:undefined}; const store=new Store(output.database); try{ store.createPlan(JSON.stringify(plan),'json',{identity,issue:plan.issue,baseEntries,pathKey,allowedCommands:[]},snapshot.base,snapshot.head); diff --git a/test/agent-container.test.ts b/test/agent-container.test.ts index 6a948dcb..48d809ed 100644 --- a/test/agent-container.test.ts +++ b/test/agent-container.test.ts @@ -24,7 +24,7 @@ import { recoverLeftovers } from '../agents/recovery.ts'; import { createVendorNetwork, removeVendorNetwork, VendorNetworkCreationCleanupError, type VendorNetwork } from '../agents/network/network.ts'; import { createClaudeCommand, createIsolationProbeCommand, createPhasePolicy, - assertPhasePolicy, type AgentCommand, type IsolationProbe } from '../agents/policy.ts'; + createRunnerCommand, assertPhasePolicy, type AgentCommand, type IsolationProbe } from '../agents/policy.ts'; const stagingFault = vi.hoisted(() => ({ chmodPathPrefix: '', realpathPathPrefix: '' })); vi.mock('node:fs', async importOriginal => { const actual = await importOriginal(); @@ -81,7 +81,7 @@ function fixture(options: { limits?: Parameters[1 return { root, source, input, clone, filesystems, fakeAuth }; } -function invocation(clone: ReturnType, phase: Phase, vendor: 'codex' | 'claude' = 'codex', +function invocation(clone: ReturnType, phase: Phase, vendor: 'codex' | 'claude' | 'runner' = 'codex', deadlineMs = 60_000): InvocationInput { return captureInvocation({ runnerOwner: TEST_RUNNER_OWNER, clone, phase, vendor, approvedArgv: phase === 'planning' || phase === 'questions' ? [] : [['git', 'status']], deadline: Date.now() + deadlineMs, attemptId: `${vendor}-${phase}-${Math.random().toString(16).slice(2)}`, @@ -96,7 +96,7 @@ const governed = async (captured: InvocationInput, probe: IsolationProbe = 'noop async function profile(data: ReturnType, phase: Phase, command: IsolationProbe | ((policy: ReturnType) => AgentCommand), options: { - vendor?: 'codex' | 'claude'; authProbe?: boolean; codexAuthFile?: string; claudeToken?: string; deadlineMs?: number; + vendor?: 'codex' | 'claude' | 'runner'; authProbe?: boolean; codexAuthFile?: string; claudeToken?: string; deadlineMs?: number; treeCheck?: TaskTreeCheck; cleanupRoot?: string; invocationBudget?: () => number; } = {}) { const vendor = options.vendor ?? 'codex'; @@ -223,6 +223,25 @@ describe('real Docker agent isolation', () => { expect(await runContainer(claude, 60_000, { CLAUDE_CODE_OAUTH_TOKEN: placeholder })).toBe('scratch-bounded'); }, 120_000); + it('runs the review runner profile read-only without provider credentials', async () => { + const data = fixture(), argv = ['node', '-e', "process.stdout.write('runner-ok')"]; + const commands = JSON.stringify([argv]); + chmodSync(data.input, 0o755); chmodSync(join(data.input, 'schema.json'), 0o644); + writeFileSync(join(data.input, 'schema.json'), commands); chmodSync(join(data.input, 'schema.json'), 0o444); chmodSync(data.input, 0o555); + const captured = captureInvocation({ runnerOwner: TEST_RUNNER_OWNER, clone: data.clone, phase: 'review', vendor: 'runner', + approvedArgv: [argv], deadline: Date.now() + 60_000, attemptId: `runner-review-${randomUUID()}`, + context: { snapshotId: 'snapshot-1', planId: 'plan-1', planRevision: 1, assignmentId: 'assignment-1', + referencedCodeHash: 'code-1', stateVersion: 1 } }); + const policy = createPhasePolicy(captured), network = await createVendorNetwork(captured, imageId, randomUUID()); + vendorNetworks.push(network); + const runner = await createContainerProfile({ invocation: captured, policy, network, filesystems: data.filesystems, + inputDirectory: data.input, command: createRunnerCommand(policy, commands), imageId }); + profiles.push(runner); + expect(runner.args).not.toContain('CODEX_HOME=/run/codeboost-auth/codex'); + expect(runner.args).not.toContain('CLAUDE_CODE_OAUTH_TOKEN'); + expect(await runContainer(runner)).toBe('runner-ok'); + }, 60_000); + it.each([ ['an absolute link to a host file', (source: string, root: string) => { writeFileSync(join(root, 'host-only.txt'), 'codeboost-host-secret\n'); diff --git a/test/agent-network.test.ts b/test/agent-network.test.ts index 1d425588..521c903d 100644 --- a/test/agent-network.test.ts +++ b/test/agent-network.test.ts @@ -222,9 +222,10 @@ describe('vendor-only egress', () => { }, 60_000); it('pins the host list with each vendor profile', async () => { - expect(VENDOR_HOSTS).toEqual({ claude: ['api.anthropic.com'], codex: ['api.openai.com', 'chatgpt.com'] }); + expect(VENDOR_HOSTS).toEqual({ claude: ['api.anthropic.com'], codex: ['api.openai.com', 'chatgpt.com'], runner: [] }); expect(Object.isFrozen(VENDOR_HOSTS.claude)).toBe(true); expect(Object.isFrozen(VENDOR_HOSTS.codex)).toBe(true); + expect(Object.isFrozen(VENDOR_HOSTS.runner)).toBe(true); }); it('reaches the vendor through the proxy while blocking other and direct hosts', async () => { diff --git a/test/agent-policy.test.ts b/test/agent-policy.test.ts index 68ab078d..75f4a5c9 100644 --- a/test/agent-policy.test.ts +++ b/test/agent-policy.test.ts @@ -1,14 +1,14 @@ import { describe, expect, it, vi } from 'vitest'; import { captureInvocation, type InvocationInput, type Phase } from '../agents/contract.ts'; import { assertAgentCommand, assertAgentTool, assertCommandSchema, codexBaseArguments, createClaudeCommand, - createCodexCommand, createPhasePolicy, dispatchApprovedCommand, MAX_COMMAND_SCHEMA_BYTES } from '../agents/policy.ts'; + createCodexCommand, createPhasePolicy, createRunnerCommand, dispatchApprovedCommand, MAX_COMMAND_SCHEMA_BYTES } from '../agents/policy.ts'; import planSchema from '../schema/versions/1/plan.schema.json' with { type: 'json' }; import editSchema from '../schema/versions/1/plan-edit.schema.json' with { type: 'json' }; const TEST_RUNNER_OWNER = '0123456789abcdef0123456789abcdef'; const SCHEMA = '{"type":"object","properties":{"title":{"type":"string"}},"required":["title"]}\n'; let attempt = 0; -const request = (phase: Phase, vendor: 'claude' | 'codex' = 'claude'): InvocationInput => captureInvocation({ runnerOwner: TEST_RUNNER_OWNER, +const request = (phase: Phase, vendor: 'claude' | 'codex' | 'runner' = 'claude'): InvocationInput => captureInvocation({ runnerOwner: TEST_RUNNER_OWNER, clone: { id: 'clone-1', taskId: 'task-1', directory: '/tmp/task', head: 'a'.repeat(40) }, vendor, phase, approvedArgv: ['planning', 'questions'].includes(phase) ? [] : [['npm', 'test']], deadline: 2000, attemptId: `attempt-${phase}-${++attempt}`, @@ -117,6 +117,17 @@ describe('agent phase policy', () => { expect(() => assertCommandSchema(review, Buffer.from('anything'))).not.toThrow(); }); + it('runs only complete approved argv arrays in the credential-free review profile', () => { + const policy = createPhasePolicy(request('review', 'runner')); + const schema = JSON.stringify([['npm', 'test']]); + const command = createRunnerCommand(policy, schema); + expect(command.argv).toEqual(['node', '/usr/local/bin/codeboost-command-check', '/run/codeboost-input/schema.json']); + expect(() => assertCommandSchema(command, Buffer.from(schema))).not.toThrow(); + expect(() => createRunnerCommand(policy, JSON.stringify([['npm', 'test', '--changed']]))).toThrow(/not approved exactly/); + expect(() => createRunnerCommand(policy, '[["npm","\\ud800"]]')).toThrow(/literal argv/); + expect(() => createRunnerCommand(createPhasePolicy(request('execute', 'runner')), schema)).toThrow(/review policy/); + }); + it.each(['planning', 'questions', 'review', 'execute', 'fix'] as const)( 'refuses Codex in %s, where its shell is off and it could not read the code (#93)', phase => { expect(() => createCodexCommand(createPhasePolicy(request(phase, 'codex')), 'Work.')) diff --git a/test/agent-supervisor.test.ts b/test/agent-supervisor.test.ts index 1244c342..7c5cc5c4 100644 --- a/test/agent-supervisor.test.ts +++ b/test/agent-supervisor.test.ts @@ -42,7 +42,7 @@ function fixture(schema = '{"probe":"codeboost-adapter-schema-marker"}\n') { return { root, input, clone, filesystems, auth }; } function invocation(data: ReturnType, attemptId: string, deadlineMs = 2 * 60_000, - vendor: 'codex' | 'claude' = 'codex', phase: Phase = 'planning'): InvocationInput { + vendor: InvocationInput['vendor'] = 'codex', phase: Phase = 'planning'): InvocationInput { return captureInvocation({ runnerOwner: TEST_RUNNER_OWNER, clone: data.clone, phase, vendor, approvedArgv: [], deadline: Date.now() + deadlineMs, attemptId, context: { snapshotId: 'snapshot', planId: 'plan', planRevision: 1, assignmentId: 'assignment', @@ -55,7 +55,8 @@ async function profile(data: ReturnType, probe: IsolationProbe, const policy = createPhasePolicy(captured); const network = await createVendorNetwork(captured, imageId, randomUUID()); const value = await createContainerProfile({ invocation: captured, policy, network, filesystems: data.filesystems, - inputDirectory: data.input, command: createIsolationProbeCommand(policy, probe), imageId, codexAuthFile: data.auth, + inputDirectory: data.input, command: createIsolationProbeCommand(policy, probe), imageId, + ...(captured.vendor === 'codex' ? { codexAuthFile: data.auth } : {}), deferredOutput }); profiles.push(value); return value; } @@ -135,6 +136,43 @@ describe('container invocation supervisor', () => { expect(isInvocationActive(attemptId)).toBe(false); }, 60_000); + it('drains finite command diagnostics past their capture limits without changing exit-0 success', async () => { + const data = fixture(); + const result = await startProfileInvocation( + await profile(data, 'finite-large-output', invocation(data, 'bounded-command-diagnostics', 2 * 60_000, 'runner', 'review')), + { limits: { stdoutBytes: 64 * 1024, stderrBytes: 32 * 1024, combinedBytes: 96 * 1024 }, + diagnosticOutput: true }).settled; + expect(result).toMatchObject({ exitCode: 0, signal: null }); + expect(result.stopReason).toBeUndefined(); + expect(Buffer.byteLength(result.stdout)).toBeLessThanOrEqual(64 * 1024); + expect(Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(32 * 1024); + expect(Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(96 * 1024); + expect(result.stderr).toContain('[codeboost: command output truncated]'); + }, 60_000); + + it('keeps multibyte truncated command diagnostics inside the byte limit', async () => { + const data = fixture(), limits = { stdoutBytes: 1024, stderrBytes: 1024, combinedBytes: 1024 }; + const result = await startProfileInvocation(await profile(data, 'finite-multibyte-output', + invocation(data, 'bounded-multibyte-command-diagnostics', 2 * 60_000, 'runner', 'review')), + { limits, diagnosticOutput: true }).settled; + expect(result).toMatchObject({ exitCode: 0, signal: null }); + expect(result.stopReason).toBeUndefined(); + expect(result.stderr).not.toContain('\uFFFD'); + expect(Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(limits.stderrBytes); + expect(Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(limits.combinedBytes); + expect(result.stderr).toContain('[codeboost: command output truncated]'); + }, 60_000); + + it('keeps invalid command diagnostics from changing exit-0 success', async () => { + const data = fixture(); + const result = await startProfileInvocation(await profile(data, 'invalid-utf8-stderr', + invocation(data, 'lossy-command-diagnostics', 2 * 60_000, 'runner', 'review')), + { diagnosticOutput: true }).settled; + expect(result).toMatchObject({ exitCode: 0, signal: null }); + expect(result.stopReason).toBeUndefined(); + expect(result.stderr).toContain('bad-'); + }, 60_000); + it('stops buffering deferred newline-free stderr after the limit is reached', async () => { const attemptId = 'deferred-stderr-limit'; const handle = startProfileInvocation(await profile(fixture(), 'infinite-stderr', attemptId, 2 * 60_000, true), { diff --git a/test/merge-task-pr.test.ts b/test/merge-task-pr.test.ts index 8282a5c0..89f44a90 100644 --- a/test/merge-task-pr.test.ts +++ b/test/merge-task-pr.test.ts @@ -13,6 +13,7 @@ import type { AlreadyFixedGateway, AlreadyFixedInput } from '../github/already-f import type { Plan, PlanContext } from '../core/plan.ts'; import { createDemo } from '../scripts/demo.ts'; import { startServer } from '../web/server.ts'; +import { commandDigest } from '../runner/checks.ts'; // #121: with a runner block, the merge gate targets the task's published PR, not github.pullRequest. const oid = (n: number) => n.toString(16).padStart(40, '0'); @@ -22,6 +23,7 @@ const plan: Plan = { schema_version: 1, issue: 12, revision: 1, summary: 'Stop t const context: PlanContext = { identity, issue: 12, baseEntries: [{ path: 'a.ts', kind: 'file' }], pathKey: p => p, allowedCommands: [] }; const BRANCH = 'codeboost/issue-12-task-42-0123456789abcdef'; const PUBLISHED: PublishedTarget = { repository: 'owner/repo', baseBranch: 'main' }; +const PREPARED_PUBLISHED: PublishedTarget = { ...PUBLISHED, requiresPreparation: true }; const url = (n: number) => `https://github.com/owner/repo/pull/${n}`; const stores: Store[] = [], roots: string[] = []; @@ -51,6 +53,17 @@ function published() { expect(opened(store, opening.openingId, 7)).toBe('in review'); return { store, opening }; } +function prepare(store: Store) { + const actionId = randomUUID(), request = { expectedStateVersion: store.getTask(identity).stateVersion, + expectedReviewVersion: store.reviewVersion(identity) }; + store.userAction(identity, { actionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + const snapshot = store.getSnapshot(identity); + const readiness = { stateVersion: store.getTask(identity).stateVersion, reviewVersion: store.reviewVersion(identity), + snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, commandPolicyDigest: commandDigest([]) }; + store.settlePreMergeAction(identity, actionId, + { state: 'ready', base: snapshot.base, head: snapshot.head, checked: [], reason: null }, readiness); + return readiness; +} /** * PR 7 is opened and running; a later opening was abandoned, so a test can adopt it as PR 8 (newer than 7) at any point. * The task is then in review. @@ -71,7 +84,7 @@ function service(store: Store) { plan: { revision: 1 }, segments: [], notes: [], snapshot: { id: snapshot.id, base: oid(1), head: oid(2) }, token: 'review-token', expected: { revision: 1, snapshotId: snapshot.id, reviewVersion: store.reviewVersion(identity) }, }; - return { store, config: { identity }, load: vi.fn(() => view) } as unknown as ReviewService; + return { store, config: { identity }, load: vi.fn(() => view), commandPolicyDigest: () => commandDigest([]) } as unknown as ReviewService; } /** @@ -100,6 +113,139 @@ function github(store: Store, change: (number: number) => Partial { + it('invalidates preparation when the current command policy changes', async () => { + const { store } = published(), gh = github(store); + prepare(store); + const review = service(store); + review.commandPolicyDigest = () => commandDigest([['npm', 'test']]); + const merges = new MergeCoordinator(review, gh.client, undefined, undefined, PREPARED_PUBLISHED); + expect((await merges.status()).blockers).toContainEqual({ code: 'preparation', + message: 'Pre-merge preparation has not completed for the current review. Prepare the merge again.' }); + }); + + it('requires durable current preparation through the irreversible admission transaction', async () => { + const { store } = published(), gh = github(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + expect((await merges.status()).blockers).toContainEqual({ code: 'preparation', + message: 'Pre-merge preparation has not completed for the current review. Prepare the merge again.' }); + prepare(store); + expect((await merges.status()).ready).toBe(true); + + let reads = 0; + gh.client.before = () => { + if (++reads !== 2) return; + const task = store.getTask(identity); + store.userAction(identity, { actionId: randomUUID(), kind: 'prepare-merge', + request: { expectedStateVersion: task.stateVersion, expectedReviewVersion: store.reviewVersion(identity) } }, + () => ({ outcome: 'preparing' })); + }; + await expect(merges.merge('review-token')).rejects.toThrow(/pre-merge preparation has not completed/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('reauthorizes issue trust immediately before irreversible admission', async () => { + const { store } = published(), gh = github(store); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + const calls: string[] = []; + merges.setAuthorization(async () => { + calls.push('capture'); + return { refresh: async () => { calls.push('revalidate'); throw new GuardRefusal('Issue trust was revoked.'); }, + validate: () => { calls.push('local'); } }; + }); + await expect(merges.merge('review-token')).rejects.toThrow('Issue trust was revoked.'); + expect(calls).toEqual(['capture', 'revalidate']); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('revalidates the pull request after the final authorization refresh', async () => { + const { store } = published(); + let remoteBase = oid(1); + const gh = github(store, () => ({ base: remoteBase })); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + merges.setAuthorization(async () => ({ refresh: async () => { remoteBase = oid(3); }, validate: () => undefined })); + await expect(merges.merge('review-token')).rejects.toThrow(/pull request changed during merge validation/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('revalidates authorization after the final pull request refresh', async () => { + const { store } = published(), gh = github(store); + let reads = 0, revoked = false; + gh.client.before = () => { if (++reads === 3) revoked = true; }; + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + merges.setAuthorization(async () => ({ refresh: async () => undefined, validate: () => { + if (revoked) throw new GuardRefusal('Issue trust was revoked during the final pull request refresh.'); + } })); + await expect(merges.merge('review-token')).rejects.toThrow(/trust was revoked during the final pull request refresh/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('captures the queue event boundary after the final authorization and pull request reads', async () => { + const { store } = published(), gh = github(store, () => ({ mergeQueue: true })); + let authorized = false; + gh.client.queueWatermark = vi.fn(async () => authorized ? 'CURSOR_after_authorization' : 'CURSOR_before_authorization'); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + merges.setAuthorization(async () => ({ refresh: async () => { authorized = true; }, validate: () => undefined })); + + await merges.merge('review-token'); + + expect(store.getMergeAttempt(identity)).toMatchObject({ queueWatermark: 'CURSOR_after_authorization' }); + }); + + it('revalidates the pull request after capturing the queue event boundary', async () => { + const { store } = published(); + let remoteBase = oid(1), watermarks = 0; + const gh = github(store, () => ({ base: remoteBase, mergeQueue: true })); + gh.client.queueWatermark = vi.fn(async () => { + if (++watermarks === 2) remoteBase = oid(3); + return 'CURSOR'; + }); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + + await expect(merges.merge('review-token')).rejects.toThrow(/pull request|requirements changed/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('revalidates local authorization after the final queue event boundary', async () => { + const { store } = published(), gh = github(store, () => ({ mergeQueue: true })); + let authorized = false, revoked = false; + gh.client.queueWatermark = vi.fn(async () => { + if (authorized) revoked = true; + return 'CURSOR'; + }); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + merges.setAuthorization(async () => ({ refresh: async () => { authorized = true; }, validate: () => { + if (revoked) throw new GuardRefusal('Issue trust was revoked during the final queue read.'); + } })); + + await expect(merges.merge('review-token')).rejects.toThrow(/trust was revoked during the final queue read/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + + it('completes the final external authorization read before validating merge-queue mode', async () => { + const { store } = published(); + let mergeQueue = false, authorizationReads = 0; + const gh = github(store, () => ({ mergeQueue })); + prepare(store); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PREPARED_PUBLISHED); + merges.setAuthorization(async () => ({ refresh: async () => { if (++authorizationReads === 1) mergeQueue = true; }, + validate: () => undefined })); + await expect(merges.merge('review-token')).rejects.toThrow(/merge-queue|pull request changed/i); + expect(gh.merged).toEqual([]); + expect(store.getMergeAttempt(identity)).toBeNull(); + }); + it('inspects and merges the task\'s published PR and pins it on the attempt', async () => { const { store } = published(); const gh = github(store); @@ -162,10 +308,18 @@ describe('the merge gate with a runner block (#121)', () => { const status = await merges.status(); expect(status.ready).toBe(false); expect(status.blockers.map(blocker => blocker.message)).toEqual([expect.stringMatching(message)]); + await expect(merges.remotePair()).rejects.toThrow(message); await expect(merges.merge('review-token')).rejects.toThrow(message); expect(gh.merged).toEqual([]); }); + it('refuses pre-merge preparation for a pull request that is no longer open', async () => { + const { store } = published(); + const gh = github(store, () => ({ pullRequestState: 'CLOSED' })); + const merges = new MergeCoordinator(service(store), gh.client, undefined, undefined, PUBLISHED); + await expect(merges.remotePair()).rejects.toThrow(/is closed; pre-merge preparation requires an open pull request/); + }); + it('does not ask a merged PR to still be open on its branch', async () => { const { store, opening } = published(); const gh = github(store, () => ({ pullRequestState: 'MERGED', published: { headBranch: BRANCH, baseBranch: 'main', crossRepository: false, marker: openingMarker(opening.openingId), branchOpen: [] } })); diff --git a/test/plan-v1.test.ts b/test/plan-v1.test.ts index b3a78e64..bc79be14 100644 --- a/test/plan-v1.test.ts +++ b/test/plan-v1.test.ts @@ -10,6 +10,8 @@ describe('frozen v1 input contract', () => { expect(commandAllowed(['go', 'test'], [['go', 'test']])).toBe(true); }); it.each(['test (x)', 'test #x', 'test !x', "test 'a\\b'"])('rejects forbidden tokenizer spelling %s', text => expect(() => commandArgv(text)).toThrow()); + it('rejects a command argument that the process boundary would normalize', () => + expect(() => commandArgv(`test "${String.fromCharCode(0xd800)}"`)).toThrow()); it('preserves empty args, adjacent quotes, and quoted punctuation', () => expect(commandArgv(`test '' ab" cd" '#!()'`)).toEqual(['test', '', 'ab cd', '#!()'])); it('requires selected issue and known path identity', () => { expect(validatePlan(plan(), { ...context, issue: undefined } as any).errors.length).toBeGreaterThan(0); diff --git a/test/plant.test.ts b/test/plant.test.ts index 463c9281..b090e6bc 100644 --- a/test/plant.test.ts +++ b/test/plant.test.ts @@ -12,9 +12,11 @@ const roots:string[]=[];afterEach(()=>roots.splice(0).forEach(root=>rmSync(root, it('plants in an isolated clone, retains ledger attribution, and leaves source history unchanged',()=>{ const root=mkdtempSync(join(tmpdir(),'codeboost-plant-'));roots.push(root);const config=createDemo(join(root,'source')); config.github={repository:'owner/source',pullRequest:7,issue:3}; + config.allowedCommands=[['npm','test']]; + config.runnerRepository=config.repository; const head=()=>fixtureGit(config.repository,'rev-parse','HEAD');const before=head(); const path=plant(config,join(root,'experiment'),{declaredText:'// planted extra behavior',undeclaredText:'unrelated diagnostic',undeclaredPath:'extra.txt'}); - expect(head()).toBe(before);const output=JSON.parse(readFileSync(path,'utf8'));expect(output.github).toBeUndefined();const service=new ReviewService(output); + expect(head()).toBe(before);const output=JSON.parse(readFileSync(path,'utf8'));expect(output.github).toBeUndefined();expect(output.allowedCommands).toBeUndefined();expect(output.runnerRepository).toBeUndefined();const service=new ReviewService(output); try {const view=service.load();expect(view.segments.some(s=>s.path==='extra.txt'&&s.scope==='out-of-scope')).toBe(true);expect(view.segments.some(s=>s.content.includes('// planted extra behavior')&&s.row.startsWith('P'))).toBe(true);}finally{service.close();} expect(JSON.parse(readFileSync(join(root,'experiment','sealed.json'),'utf8')).mappings).toHaveLength(3); },15000); diff --git a/test/pre-merge.test.ts b/test/pre-merge.test.ts new file mode 100644 index 00000000..e38b4a74 --- /dev/null +++ b/test/pre-merge.test.ts @@ -0,0 +1,826 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { randomUUID } from 'node:crypto'; +import { afterEach, expect, it, vi } from 'vitest'; +import { COMMAND_CHECK_SETTLEMENT_RESERVE_MS, PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS, PreMergeCoordinator, + type PreMergeAuthorization } from '../runner/pre-merge.ts'; +import { ReviewService } from '../runner/review.ts'; +import { createDemo } from '../scripts/demo.ts'; +import { fixtureGit } from './fixtures/git.ts'; +import type { Plan, PlanContext } from '../core/plan.ts'; +import { commandCheckDeps } from '../runner/checks.ts'; +import { RunnerCoordinator } from '../runner/coordinator.ts'; +import type { InvocationHandle, InvocationResult } from '../agents/contract.ts'; +import type { TaskWorkspace, WorkspaceRef } from '../runner/execution.ts'; +import { MIN_REBASE_CLEANUP_TIMEOUT_MS, MIN_REBASE_TIMEOUT_MS, RebaseResourcesUnsettled } from '../runner/rebase.ts'; + +const roots: string[] = [], services: ReviewService[] = []; +vi.setConfig({ testTimeout: 15_000 }); +afterEach(() => { for (const service of services.splice(0)) service.close(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); + +const markRunnerOwned = (service: ReviewService) => { + const identity = service.config.identity; + service.store.transitionTask(identity, service.store.getTask(identity).stateVersion, 'queued'); + const attempt = service.store.admitAttempt(identity, { expectedStateVersion: service.store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: Date.now() + 60_000, expectedContext: service.store.currentContext(identity) }); + service.store.markRunning(identity, attempt.id); + service.store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { unchanged: false, head: service.store.getSnapshot(identity).head } }); + service.store.transitionTask(identity, service.store.getTask(identity).stateVersion, 'in review'); + service.config.runnerRepository = service.config.repository; +}; +const markRunnerUnchanged = (service: ReviewService) => { + const identity = service.config.identity; + service.store.transitionTask(identity, service.store.getTask(identity).stateVersion, 'queued'); + const attempt = service.store.admitAttempt(identity, { expectedStateVersion: service.store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: Date.now() + 60_000, expectedContext: service.store.currentContext(identity) }); + service.store.markRunning(identity, attempt.id); + service.store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { unchanged: true, head: service.store.getSnapshot(identity).head } }); + service.store.transitionTask(identity, service.store.getTask(identity).stateVersion, 'in review'); +}; + +async function rebaseFixture(rebasedText: string, commandExit?: number, options: { + duringRebase?: (service: ReviewService, coordinator: PreMergeCoordinator, timeoutMs?: number) => void; + duringFinalInspect?: (service: ReviewService, coordinator: PreMergeCoordinator, read: number) => void; + duringAuthorize?: (service: ReviewService, call: number) => { base: string; head: string } | void; + closeDuringCommand?: boolean; + closeAfterCommandAdmission?: boolean; + commandWaitsForDeadline?: boolean; + commandWaitsForCancellation?: boolean; + releaseFails?: boolean; + unsettledRebase?: boolean; + rebaseFailure?: Error; + rebaseCleanupFailure?: Error; + runnerCommitted?: boolean; + operationTimeoutMs?: number; + authorize?: (signal: AbortSignal) => Promise; + reserves?: { processMs?: number; commandMs?: number }; +} = {}) { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-base-')); roots.push(root); + const repository = join(root, 'repo'); fixtureGit(root, 'init', '-q', '-b', 'main', repository); + fixtureGit(repository, 'config', 'user.name', 'Test'); fixtureGit(repository, 'config', 'user.email', 'test@example.com'); + writeFileSync(join(repository, 'a.txt'), 'base\n'); fixtureGit(repository, 'add', '.'); fixtureGit(repository, 'commit', '-qm', 'base'); + const base = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-qc', 'feature'); + writeFileSync(join(repository, 'a.txt'), 'feature\n'); fixtureGit(repository, 'commit', '-am', 'feature', '-q'); + const head = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-q', '-C', 'main', base); + writeFileSync(join(repository, 'b.txt'), 'new base\n'); fixtureGit(repository, 'add', '.'); fixtureGit(repository, 'commit', '-qm', 'move base'); + const onto = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-qc', 'rebased'); + writeFileSync(join(repository, 'a.txt'), rebasedText); fixtureGit(repository, 'commit', '-am', 'rebased feature', '-q'); + const rebased = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-q', 'feature'); + const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; + const allowedCommands = commandExit === undefined ? [] : [['npm', 'test']]; + const config = { database: join(root, 'review.sqlite'), repository, runnerRepository: repository, identity, + pathIdentity: { caseSensitive: true, unicodeNormalization: 'none' as const }, allowedCommands }; + const service = new ReviewService(config); services.push(service); + const plan: Plan = { schema_version: 1, revision: 1, issue: 1, summary: 'One change', questions: [], items: [{ id: 'P1', + title: 'Change a', intent: 'Change a', files: [{ path: 'a.txt', kind: 'edit', renamed_from: null, change: 'Change it' }], + acceptance: commandExit === undefined ? [{ type: 'check', text: 'a changed' }] : [{ type: 'cmd', text: 'npm test' }], depends_on: [] }] }; + const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a.txt', kind: 'file' }], pathKey: path => path, allowedCommands }; + service.store.createPlan(JSON.stringify(plan), 'json', context, base, head); + let view = service.load(); + service.store.recordHistory(identity, view.expected, base, head, [{ sha: head, owner: 'P1', origin: 'owned', sourceSha: null }]); + if (options.runnerCommitted !== false) markRunnerOwned(service); else markRunnerUnchanged(service); + view = service.load(); view = service.act({ action: 'approve', item: 'P1', token: view.token }); + let coordinator!: PreMergeCoordinator, rebaseRuns = 0; + let rebaseAborts = 0; + const rebaseBudgets: Array = [], abortBudgets: Array = []; + const rebaser = { run: async (input: { attemptId: string; signal: AbortSignal; timeoutMs?: number }) => { + rebaseRuns++; + rebaseBudgets.push(input.timeoutMs); + options.duringRebase?.(service, coordinator, input.timeoutMs); + input.signal.throwIfAborted(); + if (options.unsettledRebase) throw new RebaseResourcesUnsettled('Conflict resources remain owned.'); + if (options.rebaseFailure) throw options.rebaseFailure; + const planKey = service.store.getTask(identity).planKey; + service.store.prepareRebaseResult(planKey, input.attemptId, rebased, [rebased]); + service.store.completeRebaseResult(planKey, input.attemptId); + return { oldHead: head, base: onto, head: rebased, + mappings: [{ oldSha: head, newSha: rebased }], resolvedConflicts: [] }; + }, abort: async (_attemptId: string, _head?: string, _state?: string, timeoutMs?: number) => { + rebaseAborts++; abortBudgets.push(timeoutMs); + if (options.rebaseCleanupFailure) throw options.rebaseCleanupFailure; + } } as never; + const checkedHeads: string[] = []; + const workspace: TaskWorkspace = { + async materialize(attempt, checkedHead) { + checkedHeads.push(checkedHead); + return { clone: { id: attempt.id, taskId: 'task', directory: repository, head: checkedHead }, storage: {} } as WorkspaceRef; + }, + async snapshotDeclaredLinks() { throw new Error('not used'); }, async checkTree() { throw new Error('not used'); }, + async inspectChanges() { throw new Error('not used'); }, async commit() { throw new Error('not used'); }, + async release() { if (options.releaseFails) throw new Error('cleanup failed'); }, + }; + const cancelReasons: string[] = []; + const runner = new RunnerCoordinator(service.store, commandCheckDeps(service.store, workspace, input => { + if (options.closeDuringCommand) { + let settle!: (result: InvocationResult) => void; + const settled = new Promise(resolve => { settle = resolve; }); + queueMicrotask(() => { void coordinator.close(); }); + return { attemptId: input.attemptId, settled, cancel(reason) { + cancelReasons.push(reason); + settle({ attemptId: input.attemptId, context: input.context, exitCode: null, + signal: 'SIGTERM', stopReason: reason, stdout: '', stderr: '' }); + } } satisfies InvocationHandle; + } + if (options.commandWaitsForDeadline) { + const settled = new Promise(resolve => { + setTimeout(() => resolve({ attemptId: input.attemptId, context: input.context, exitCode: null, + signal: 'SIGTERM', stopReason: 'timeout', stdout: '', stderr: '' }), Math.max(0, input.deadline - Date.now())); + }); + return { attemptId: input.attemptId, settled, cancel() {} } satisfies InvocationHandle; + } + if (options.commandWaitsForCancellation) { + let settle!: (result: InvocationResult) => void; + const settled = new Promise(resolve => { settle = resolve; }); + return { attemptId: input.attemptId, settled, cancel(reason) { + cancelReasons.push(reason); + settle({ attemptId: input.attemptId, context: input.context, exitCode: null, + signal: 'SIGTERM', stopReason: reason, stdout: '', stderr: '' }); + } } satisfies InvocationHandle; + } + if (options.closeAfterCommandAdmission) { + let settle!: (result: InvocationResult) => void; + const settled = new Promise(resolve => { settle = resolve; }); + setTimeout(() => settle({ attemptId: input.attemptId, context: input.context, exitCode: 0, + signal: null, stdout: '', stderr: '' }), 50); + return { attemptId: input.attemptId, settled, cancel(reason) { + cancelReasons.push(reason); + settle({ attemptId: input.attemptId, context: input.context, exitCode: null, + signal: 'SIGTERM', stopReason: reason, stdout: '', stderr: '' }); + } } satisfies InvocationHandle; + } + return { attemptId: input.attemptId, + settled: Promise.resolve({ attemptId: input.attemptId, context: input.context, exitCode: commandExit ?? 0, + signal: null, stdout: '', stderr: commandExit ? 'failed' : '' }), cancel() {} } satisfies InvocationHandle; + }, context.allowedCommands, 'a'.repeat(32))); + let remoteReads = 0, remotePair = { base: onto, head }; + let authorizationChecks = 0; + const authorize = options.authorize ?? (options.duringAuthorize ? async () => ({ + refresh: async () => undefined, + validate: () => { remotePair = options.duringAuthorize!(service, ++authorizationChecks) ?? remotePair; }, + }) : undefined); + coordinator = new PreMergeCoordinator(service, + runner, + rebaser, { inspect: async () => { + remoteReads++; + if (remoteReads > 1) options.duringFinalInspect?.(service, coordinator, remoteReads); + return remotePair; + }, fetch: async () => undefined }, options.operationTimeoutMs, undefined, authorize, options.reserves); + if (options.closeAfterCommandAdmission) { + const start = runner.start.bind(runner); + runner.start = ((...args: Parameters) => { + const attempt = start(...args); + void coordinator.close(); + return attempt; + }) as RunnerCoordinator['start']; + } + const task = service.store.getTask(identity); + const result = await coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + return { service, coordinator, runner, result, rebased, checkedHeads, cancelReasons, + rebaseRuns: () => rebaseRuns, rebaseAborts: () => rebaseAborts, rebaseBudgets, abortBudgets }; +} + +it('preserves an approval when a moved base leaves its attributed fingerprint unchanged', async () => { + const { service, coordinator, runner, result, rebased } = await rebaseFixture('feature\n'); + expect(result).toMatchObject({ state: 'ready', head: rebased, checked: [], reason: null }); + expect(service.load().items.find(item => item.id === 'P1')?.state).toBe('approved'); + await coordinator.close(); await runner.close(); +}); +it('prepares a published original head when every runner item was unchanged', async () => { + const { coordinator, runner, result, rebased } = await rebaseFixture('feature\n', undefined, { runnerCommitted: false }); + expect(result).toMatchObject({ state: 'ready', head: rebased, checked: [], reason: null }); + await coordinator.close(); await runner.close(); +}); + +it('returns to review when a moved-base rewrite changes an approved fingerprint', async () => { + const { service, coordinator, runner, result, rebased } = await rebaseFixture('changed by rebase\n'); + expect(result).toMatchObject({ state: 'review-required', head: rebased, checked: [] }); + expect(result.reason).toMatch(/requires refreshed attribution or approval/); + expect(service.load().items.find(item => item.id === 'P1')?.state).toBe('stale'); + await coordinator.close(); await runner.close(); +}); + +it('marks a failed preparation stale when its final remote inspection was invalidated by a review edit', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { duringFinalInspect(service) { + const current = service.load(); + service.store.addReviewNote(service.config.identity, current.expected, 'P1', 'change', 'Changed during inspection.'); + } }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: expect.stringMatching(/changed during preparation/i) }); + expect(fixture.coordinator.last?.stale).toBe(true); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('leaves an unsettled conflict and its rebase marker for startup recovery', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { unsettledRebase: true }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Conflict resources remain owned.' }); + expect(fixture.coordinator.last).toMatchObject({ state: 'failed', stale: false }); + expect(fixture.rebaseAborts()).toBe(0); + expect(fixture.service.store.getTask(fixture.service.config.identity).rebaseInProgress).not.toBeNull(); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('keeps a failed live cleanup current while its rebase marker awaits recovery', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { + rebaseFailure: new Error('rebase failed'), rebaseCleanupFailure: new Error('cleanup failed'), + }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'rebase failed' }); + expect(fixture.coordinator.last).toMatchObject({ state: 'failed', stale: false }); + expect(fixture.rebaseAborts()).toBe(1); + expect(fixture.service.store.getTask(fixture.service.config.identity).rebaseInProgress).not.toBeNull(); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('keeps a failed rebase bound to the review that started it when that review changes in flight', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { + duringRebase(service) { + const current = service.load(); + service.store.addReviewNote(service.config.identity, current.expected, 'P1', 'change', 'Changed during rebase.'); + }, + rebaseFailure: new Error('rebase failed'), rebaseCleanupFailure: new Error('cleanup failed'), + }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'rebase failed' }); + expect(fixture.coordinator.last).toMatchObject({ state: 'failed', stale: true }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('revalidates authorization before starting the delayed local rebase', async () => { + const calls: string[] = []; + const fixture = await rebaseFixture('feature\n', undefined, { authorize: async () => { + calls.push('read'); return { refresh: async () => undefined, + validate: () => { calls.push('validate'); throw new Error('Issue trust was revoked.'); } }; + } }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Issue trust was revoked.' }); + expect(calls).toEqual(['read', 'validate']); + expect(fixture.rebaseRuns()).toBe(0); + // Refused authorization precedes the first durable rebase claim, so there is no cleanup lifecycle to start. + expect(fixture.rebaseAborts()).toBe(0); + expect(fixture.service.store.getTask(fixture.service.config.identity).rebaseInProgress).toBeNull(); + expect(fixture.coordinator.last).toMatchObject({ state: 'failed', stale: false }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('refuses readiness when local issue trust is revoked during the final remote inspection', async () => { + let trusted = true; + const fixture = await rebaseFixture('feature\n', undefined, { + authorize: async () => ({ refresh: async () => undefined, + validate: () => { if (!trusted) throw new Error('Issue trust was revoked.'); } }), + duringFinalInspect: (_service, _coordinator, read) => { if (read === 3) trusted = false; }, + }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Issue trust was revoked.' }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('does not report readiness while a current change request remains open', async () => { + const fixture = await rebaseFixture('feature\n'); + const current = fixture.service.load(); + fixture.service.store.addReviewNote(fixture.service.config.identity, current.expected, 'P1', 'change', 'Please revise this.'); + const changed = fixture.service.load(), task = fixture.service.store.getTask(fixture.service.config.identity); + const result = await fixture.coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: changed.expected.reviewVersion!, snapshotId: changed.snapshot.id, base: changed.snapshot.base, head: changed.snapshot.head }); + expect(result).toMatchObject({ state: 'review-required', checked: [] }); + expect(result.reason).toMatch(/1 change request remains open/); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('preserves an unpushed rebased head across review and preparation retries', async () => { + const first = await rebaseFixture('changed by rebase\n'); + let view = first.service.load(); + view = first.service.act({ action: 'approve', item: 'P1', token: view.token }); + const task = first.service.store.getTask(first.service.config.identity); + const result = await first.coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'ready', head: first.rebased, reason: null }); + expect(first.rebaseRuns()).toBe(1); + await first.coordinator.close(); await first.runner.close(); +}); + +it('blocks when a command check fails on the rewritten head', async () => { + const { coordinator, runner, result, rebased, checkedHeads } = await rebaseFixture('feature\n', 1); + expect(result).toMatchObject({ state: 'failed', head: rebased, checked: [] }); + expect(result.reason).toMatch(/command checks did not pass/); + expect(checkedHeads).toEqual([rebased]); + await coordinator.close(); await runner.close(); +}); + +it('preserves completed command-check IDs when a later preparation step fails', async () => { + const fixture = await rebaseFixture('feature\n', 0, { duringFinalInspect() { + throw new Error('final remote inspection failed'); + } }); + expect(fixture.result).toMatchObject({ state: 'failed', checked: ['P1'], reason: 'final remote inspection failed' }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('does not report readiness when command-check cleanup is unresolved', async () => { + const fixture = await rebaseFixture('feature\n', 0, { releaseFails: true }); + expect(fixture.result).toMatchObject({ state: 'failed', checked: [] }); + expect(fixture.result.reason).toMatch(/cleanup could not be confirmed/); + expect(fixture.runner.status(fixture.service.config.identity).unresolved).not.toBeNull(); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('revalidates task and review versions after the final remote read', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { + duringFinalInspect(service) { + const view = service.load(); + service.store.addReviewNote(service.config.identity, view.expected, 'P1', 'change', 'Changed while preparing.'); + }, + }); + expect(fixture.result).toMatchObject({ state: 'failed' }); + expect(fixture.result.reason).toMatch(/task or review changed/); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('revalidates task and review versions after final authorization', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { duringAuthorize(service, call) { + if (call !== 2) return; + const view = service.load(); + service.store.addReviewNote(service.config.identity, view.expected, 'P1', 'change', 'Changed during authorization.'); + } }); + expect(fixture.result).toMatchObject({ state: 'failed' }); + expect(fixture.result.reason).toMatch(/task or review changed/); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('refreshes a pull request head that moves during final authorization', async () => { + const fixture = await rebaseFixture('feature\n', undefined, { duringAuthorize(service, call) { + if (call !== 2) return; + writeFileSync(join(service.config.repository, 'late.txt'), 'late collaborator push\n'); + fixtureGit(service.config.repository, 'add', 'late.txt'); fixtureGit(service.config.repository, 'commit', '-qm', 'late push'); + return { base: service.load().snapshot.base, head: fixtureGit(service.config.repository, 'rev-parse', 'HEAD') }; + } }); + expect(fixture.result).toMatchObject({ state: 'review-required' }); + expect(fixture.result.reason).toMatch(/moved during final authorization/); + expect(fixture.service.load().snapshot.head).toBe(fixture.result.head); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('marks historical readiness stale after the review changes', async () => { + const fixture = await rebaseFixture('feature\n'); + expect(fixture.coordinator.last).toMatchObject({ state: 'ready', stale: false }); + const view = fixture.service.load(); + fixture.service.store.addReviewNote(fixture.service.config.identity, view.expected, 'P1', 'change', 'Review changed later.'); + expect(fixture.coordinator.last).toMatchObject({ state: 'ready', stale: true }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it.each([ + ['review-required', 'changed by rebase\n', undefined], + ['failed', 'feature\n', 1], +] as const)('marks a historical %s preparation stale after the review changes', async (state, text, exitCode) => { + const fixture = await rebaseFixture(text, exitCode); + expect(fixture.result.state).toBe(state); + expect(fixture.coordinator.last).toMatchObject({ state, stale: false }); + const view = fixture.service.load(); + fixture.service.store.addReviewNote(fixture.service.config.identity, view.expected, 'P1', 'change', 'Review changed later.'); + expect(fixture.coordinator.last).toMatchObject({ state, stale: true }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('keeps a command-check deadline separate from the code-writing task budget', async () => { + const fixture = await rebaseFixture('feature\n', 0, + { commandWaitsForDeadline: true, operationTimeoutMs: COMMAND_CHECK_SETTLEMENT_RESERVE_MS + 5_000 }); + expect(fixture.result).toMatchObject({ state: 'failed' }); + expect(fixture.result.reason).toMatch(/Timed out|deadline exceeded/); + const check = fixture.service.store.getAttempts(fixture.service.config.identity).find(attempt => attempt.kind === 'check'); + expect(check).toMatchObject({ state: 'failed', firstReason: null, stopReason: 'timeout' }); + expect(fixture.service.store.getTask(fixture.service.config.identity).status).toBe('in review'); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('keeps the complete command settlement window inside the overall preparation deadline', async () => { + const fixture = await rebaseFixture('feature\n', 0, + { operationTimeoutMs: COMMAND_CHECK_SETTLEMENT_RESERVE_MS + 5_000 }); + const attempt = fixture.service.store.getAttempts(fixture.service.config.identity).find(value => value.kind === 'check'); + expect(attempt).toBeDefined(); + expect(attempt!.deadline - Date.parse(attempt!.createdAt)).toBeLessThanOrEqual(5_000); + expect(fixture.result).toMatchObject({ state: 'ready' }); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('aborts an owned rebase promptly when the task is cancelled', async () => { + const actionId = randomUUID(); + const fixture = await rebaseFixture('feature\n', undefined, { + duringRebase(service, coordinator) { + const task = service.store.getTask(service.config.identity); + expect(coordinator.cancelTask(task.stateVersion, actionId)).toBe('stopping'); + }, + }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Task cancelled.' }); + expect(fixture.service.store.getTask(fixture.service.config.identity).status).toBe('cancelled'); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('preserves shutdown as the reason that stops an active command check', async () => { + const fixture = await rebaseFixture('feature\n', 0, { closeDuringCommand: true }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Server shutdown.' }); + expect(fixture.cancelReasons).toEqual(['shutdown']); + const check = fixture.service.store.getAttempts(fixture.service.config.identity).find(attempt => attempt.kind === 'check'); + expect(check).toMatchObject({ state: 'cancelled', firstReason: 'shutdown' }); + await fixture.runner.close(); +}); + +it('stops a command check when shutdown races between admission and abort-listener registration', async () => { + const fixture = await rebaseFixture('feature\n', 0, { closeAfterCommandAdmission: true }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Server shutdown.' }); + const check = fixture.service.store.getAttempts(fixture.service.config.identity).find(attempt => attempt.kind === 'check'); + expect(check).toMatchObject({ state: 'cancelled', firstReason: 'shutdown' }); + await fixture.runner.close(); +}); + +it('stops an active command check when the operation-wide deadline expires', async () => { + const fixture = await rebaseFixture('feature\n', 0, + { commandWaitsForCancellation: true, + operationTimeoutMs: MIN_REBASE_TIMEOUT_MS + MIN_REBASE_CLEANUP_TIMEOUT_MS + PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS + 3_000, + reserves: { processMs: MIN_REBASE_TIMEOUT_MS + MIN_REBASE_CLEANUP_TIMEOUT_MS + + PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS + 1_000, commandMs: 200 } }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'Pre-merge preparation deadline exceeded.' }); + const check = fixture.service.store.getAttempts(fixture.service.config.identity).find(attempt => attempt.kind === 'check'); + expect(check).toMatchObject({ state: 'failed', firstReason: null, stopReason: 'timeout', diagnostic: 'Timed out.' }); + expect(fixture.cancelReasons).toEqual(['timeout']); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('refreshes the moved head against its prior base when the PR base and head advance together', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(); + const initial = { base: view.snapshot.base, head: view.snapshot.head }; + const branch = 'collaborator-refresh', work = join(root, 'collaborator'); + fixtureGit(config.repository, 'branch', branch, initial.head); + fixtureGit(config.repository, 'worktree', 'add', '-q', work, branch); + writeFileSync(join(work, 'README.md'), 'A collaborator changed this after checks started.\n'); + fixtureGit(work, 'add', 'README.md'); fixtureGit(work, 'commit', '-qm', 'collaborator push'); + const baseWork = join(root, 'advanced-base'); + fixtureGit(config.repository, 'branch', 'advanced-base', initial.base); + fixtureGit(config.repository, 'worktree', 'add', '-q', baseWork, 'advanced-base'); + writeFileSync(join(baseWork, 'base-moved.txt'), 'new base\n'); fixtureGit(baseWork, 'add', '.'); + fixtureGit(baseWork, 'commit', '-qm', 'advance base'); + const moved = { base: fixtureGit(baseWork, 'rev-parse', 'HEAD'), head: fixtureGit(work, 'rev-parse', 'HEAD') }; + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => moved, fetch: async () => undefined }); + const task = service.store.getTask(config.identity); + const result = await coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result.reason).toMatch(/head moved/); + expect(result).toMatchObject({ state: 'review-required', base: initial.base, head: moved.head, checked: [] }); + const refreshed = service.load(); + expect(refreshed.snapshot).toMatchObject({ base: initial.base, head: moved.head }); + expect(refreshed.segments.some(segment => segment.row === 'Unplanned')).toBe(true); + expect(refreshed.items.some(item => item.state !== 'approved')).toBe(true); + await coordinator.close(); +}); + +it('traces an unpushed rewrite back to the base of a collaborator head', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-lineage-')); roots.push(root); + const repository = join(root, 'repo'); fixtureGit(root, 'init', '-q', '-b', 'main', repository); + fixtureGit(repository, 'config', 'user.name', 'Test'); fixtureGit(repository, 'config', 'user.email', 'test@example.com'); + writeFileSync(join(repository, 'a.txt'), 'base\n'); fixtureGit(repository, 'add', '.'); fixtureGit(repository, 'commit', '-qm', 'base'); + const oldBase = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-qc', 'feature'); + writeFileSync(join(repository, 'a.txt'), 'feature\n'); fixtureGit(repository, 'commit', '-am', 'feature', '-q'); + const oldHead = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-q', 'main'); + writeFileSync(join(repository, 'base.txt'), 'advanced\n'); fixtureGit(repository, 'add', '.'); fixtureGit(repository, 'commit', '-qm', 'advance base'); + const newBase = fixtureGit(repository, 'rev-parse', 'HEAD'); + fixtureGit(repository, 'switch', '-qc', 'local-rewrite'); + writeFileSync(join(repository, 'a.txt'), 'feature\n'); fixtureGit(repository, 'commit', '-am', 'rebased feature', '-q'); + const rewrittenHead = fixtureGit(repository, 'rev-parse', 'HEAD'); + const collaborator = join(root, 'collaborator'); fixtureGit(repository, 'branch', 'collaborator-lineage', oldHead); + fixtureGit(repository, 'worktree', 'add', '-q', collaborator, 'collaborator-lineage'); + writeFileSync(join(collaborator, 'late.txt'), 'late\n'); fixtureGit(collaborator, 'add', '.'); fixtureGit(collaborator, 'commit', '-qm', 'late push'); + const collaboratorHead = fixtureGit(collaborator, 'rev-parse', 'HEAD'); + + const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; + const config = { database: join(root, 'review.sqlite'), repository, runnerRepository: repository, identity, + pathIdentity: { caseSensitive: true, unicodeNormalization: 'none' as const } }; + const service = new ReviewService(config); services.push(service); + const plan: Plan = { schema_version: 1, revision: 1, issue: 1, summary: 'One change', questions: [], items: [{ id: 'P1', + title: 'Change a', intent: 'Change a', files: [{ path: 'a.txt', kind: 'edit', renamed_from: null, change: 'Change it' }], + acceptance: [{ type: 'check', text: 'a changed' }], depends_on: [] }] }; + const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a.txt', kind: 'file' }], pathKey: path => path, allowedCommands: [] }; + service.store.createPlan(JSON.stringify(plan), 'json', context, oldBase, oldHead); + let view = service.load(); + service.store.recordHistory(identity, view.expected, oldBase, oldHead, + [{ sha: oldHead, owner: 'P1', origin: 'owned', sourceSha: null }]); + markRunnerOwned(service); view = service.load(); + service.store.recordRebase(identity, view.expected, newBase, rewrittenHead, + [{ oldSha: oldHead, newSha: rewrittenHead }]); + view = service.load(); view = service.act({ action: 'approve', item: 'P1', token: view.token }); + let reads = 0; + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => ++reads === 1 ? { base: newBase, head: rewrittenHead } : { base: newBase, head: collaboratorHead }, + fetch: async () => undefined }); + const task = service.store.getTask(identity); + const result = await coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'review-required', base: oldBase, head: collaboratorHead }); + expect(result.reason).toMatch(/moved during preparation/); + expect(service.load().snapshot).toMatchObject({ base: oldBase, head: collaboratorHead }); + await coordinator.close(); +}); + +it('refuses preparation before review or remote work while runner cleanup is unresolved', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-unresolved-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity); + for (const kind of ['marker', 'resources'] as const) { + let inspected = false; + const load = vi.spyOn(service, 'load'); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, + status: () => ({ active: false, stopRequested: null, + unresolved: kind === 'marker' ? { attemptId: randomUUID(), reason: 'storage-not-removed' } : null }), + get unreleased() { return kind === 'resources' ? [{ kind: 'container', name: 'agent-x' }] : null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => { inspected = true; return { base: view.snapshot.base, head: view.snapshot.head }; }, + fetch: async () => undefined }); + const result = await coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'failed', reason: expect.stringMatching(/restart|cleanup/i) }); + expect(inspected).toBe(false); + expect(load).not.toHaveBeenCalled(); + load.mockRestore(); await coordinator.close(); + } +}); + +it('refuses preparation before review or remote work while a durable rebase marker awaits recovery', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-rebase-marker-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(); + service.store.beginRebase(config.identity, { revision: view.expected.revision, snapshotId: view.expected.snapshotId, + reviewVersion: view.expected.reviewVersion! }, + service.store.getTask(config.identity).stateVersion, + { oldBase: view.snapshot.base, oldHead: view.snapshot.head, oldHistory: [view.snapshot.head], onto: view.snapshot.base }); + const task = service.store.getTask(config.identity); + let inspected = false; + const load = vi.spyOn(service, 'load'); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => { inspected = true; return { base: view.snapshot.base, head: view.snapshot.head }; }, + fetch: async () => undefined }); + const result = await coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'failed', reason: expect.stringMatching(/rebase|recovery|cleanup/i) }); + expect(inspected).toBe(false); + expect(load).not.toHaveBeenCalled(); + await coordinator.close(); +}); + +it('preserves an already-requested shutdown without loading the review', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-pre-aborted-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity); + const load = vi.spyOn(service, 'load'); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => { throw new Error('No remote inspection expected.'); }, fetch: async () => undefined }); + const active = coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + await coordinator.close(); + await expect(active).resolves.toMatchObject({ state: 'failed', reason: 'Server shutdown.' }); + expect(load).not.toHaveBeenCalled(); +}); + +it('settles an admitted action after shutdown aborts its remote refresh', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-shutdown-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity), actionId = randomUUID(); + const request = { attemptId: undefined, expectedStateVersion: task.stateVersion, expectedReviewVersion: view.expected.reviewVersion }; + service.store.userAction(config.identity, { actionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + const capability = service.store.shutdownCapability(); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: signal => new Promise((_resolve, reject) => signal?.addEventListener('abort', () => reject(signal.reason), { once: true })), + fetch: async () => undefined }, 60_000, capability); + const active = coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, actionId }); + await Promise.resolve(); + service.store.closeWrites(); await coordinator.close(); + await expect(active).resolves.toMatchObject({ state: 'failed', reason: 'Server shutdown.' }); + expect(service.store.savedAction(config.identity, { actionId, kind: 'prepare-merge', request })?.response) + .toMatchObject({ outcome: 'failed', reason: 'Server shutdown.' }); +}); + +it('applies one operation-wide deadline to remote work and later checks', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-deadline-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: signal => new Promise((_resolve, reject) => signal?.addEventListener('abort', () => reject(signal.reason), { once: true })), + fetch: async () => undefined }, 20); + await expect(coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head })).resolves.toMatchObject({ state: 'failed' }); + await coordinator.close(); +}); + +it('aborts remote work early enough to reserve bounded subprocess settlement', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-remote-settlement-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity); + let abortedAt = -1; + const processReserve = 1_000, operationTimeout = 2_000; + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: signal => new Promise((_resolve, reject) => signal?.addEventListener('abort', () => { + abortedAt = performance.now(); + setTimeout(() => reject(signal.reason), processReserve); + }, { once: true })), fetch: async () => undefined }, operationTimeout, undefined, undefined, + { processMs: processReserve }); + const startedAt = performance.now(); + const result = await coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'failed', reason: 'Pre-merge preparation deadline exceeded.' }); + expect(abortedAt - startedAt).toBeLessThanOrEqual(operationTimeout - processReserve + 500); + expect(performance.now() - startedAt).toBeLessThanOrEqual(operationTimeout + 500); + await coordinator.close(); +}); + +it('budgets live rebase work and its follow-up cleanup inside the preparation deadline', async () => { + const operationTimeoutMs = MIN_REBASE_TIMEOUT_MS + MIN_REBASE_CLEANUP_TIMEOUT_MS + + PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS + 10_000; + const fixture = await rebaseFixture('feature\n', undefined, + { operationTimeoutMs, rebaseFailure: new Error('rebase failed') }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'rebase failed' }); + expect(fixture.rebaseBudgets).toHaveLength(1); + expect(fixture.abortBudgets).toHaveLength(1); + expect(fixture.rebaseBudgets[0]).toBeLessThanOrEqual(operationTimeoutMs - MIN_REBASE_CLEANUP_TIMEOUT_MS + - PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS); + for (const budget of [...fixture.rebaseBudgets, ...fixture.abortBudgets]) { + expect(budget).toBeTypeOf('number'); + expect(budget).toBeGreaterThan(0); + expect(budget).toBeLessThanOrEqual(operationTimeoutMs); + } +}); + +it('leaves handoff slack before starting follow-up rebase cleanup', async () => { + let elapsed = 0; + vi.spyOn(performance, 'now').mockImplementation(() => elapsed); + const operationTimeoutMs = MIN_REBASE_TIMEOUT_MS + MIN_REBASE_CLEANUP_TIMEOUT_MS + + PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS + 10_000; + const fixture = await rebaseFixture('feature\n', undefined, { + operationTimeoutMs, rebaseFailure: new Error('rebase failed'), + duringRebase: (_service, _coordinator, timeoutMs) => { elapsed += timeoutMs! + 1; }, + }); + expect(fixture.result).toMatchObject({ state: 'failed', reason: 'rebase failed' }); + expect(fixture.rebaseAborts()).toBe(1); + expect(fixture.service.store.getTask(fixture.service.config.identity).rebaseInProgress).toBeNull(); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('makes a preparation action resendable when its terminal storage write fails', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-settlement-failure-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity), actionId = randomUUID(); + const request = { attemptId: undefined, expectedStateVersion: task.stateVersion, expectedReviewVersion: view.expected.reviewVersion }; + const readiness = { stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, + commandPolicyDigest: service.commandPolicyDigest() }; + const priorActionId = randomUUID(); + service.store.userAction(config.identity, { actionId: priorActionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + service.store.settlePreMergeAction(config.identity, priorActionId, + { state: 'ready', base: view.snapshot.base, head: view.snapshot.head, checked: [], reason: null }, readiness); + expect(service.store.preMergeReady(config.identity, readiness)).toBe(true); + service.store.userAction(config.identity, { actionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + const original = service.store.settlePreMergeAction.bind(service.store); + vi.spyOn(service.store, 'settlePreMergeAction').mockImplementationOnce(() => { + throw Object.assign(new Error('transient storage failure'), { code: 'ERR_SQLITE_ERROR' }); + }).mockImplementation(original); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => ({ base: view.snapshot.base, head: view.snapshot.head }), fetch: async () => undefined }); + const result = await coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, actionId }); + expect(result).toMatchObject({ state: 'failed', reason: 'transient storage failure' }); + expect(service.store.preMergeReady(config.identity, readiness)).toBe(false); + expect(service.store.savedAction(config.identity, { actionId, kind: 'prepare-merge', request })).toBeUndefined(); + const replay = service.store.userAction(config.identity, { actionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + expect(replay).toMatchObject({ replayed: false, response: { outcome: 'preparing' } }); + await coordinator.close(); +}); + +it('settles a failed preparation even when the fallback snapshot read would fail', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-fallback-read-')); roots.push(root); + const config = createDemo(join(root, 'demo')), service = new ReviewService(config); services.push(service); + markRunnerOwned(service); + const view = service.load(), task = service.store.getTask(config.identity), actionId = randomUUID(); + const request = { attemptId: undefined, expectedStateVersion: task.stateVersion, expectedReviewVersion: view.expected.reviewVersion }; + service.store.userAction(config.identity, { actionId, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + const original = service.store.getSnapshot.bind(service.store); let reads = 0; + vi.spyOn(service.store, 'getSnapshot').mockImplementation((...args) => { + if (++reads >= 2) throw Object.assign(new Error('transient snapshot read failure'), { code: 'ERR_SQLITE_ERROR' }); + return original(...args); + }); + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => { throw new Error('remote inspection failed'); }, fetch: async () => undefined }); + await expect(coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head, actionId })) + .resolves.toMatchObject({ state: 'failed', reason: 'transient snapshot read failure' }); + expect(service.store.savedAction(config.identity, { actionId, kind: 'prepare-merge', request })?.response) + .toMatchObject({ outcome: 'failed', reason: 'transient snapshot read failure' }); + await coordinator.close(); +}); + +it('threads the remaining operation budget through every synchronous review reload', async () => { + const operationTimeoutMs = MIN_REBASE_TIMEOUT_MS + MIN_REBASE_CLEANUP_TIMEOUT_MS + + PRE_MERGE_PROCESS_SETTLEMENT_RESERVE_MS + 10_000, processMs = 10_000; + const fixture = await rebaseFixture('feature\n', undefined, { operationTimeoutMs, reserves: { processMs, commandMs: 0 } }); + const view = fixture.service.load(), task = fixture.service.store.getTask(fixture.service.config.identity); + const original = fixture.service.load.bind(fixture.service); const budgets: number[] = []; + vi.spyOn(fixture.service, 'load').mockImplementation(options => { + budgets.push(options?.maxDurationMs ?? -1); return original(options); + }); + const result = await fixture.coordinator.start({ stateVersion: task.stateVersion, reviewVersion: view.expected.reviewVersion!, + snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result.state).toBe('ready'); + expect(budgets.length).toBeGreaterThan(1); + expect(budgets.every(value => value > 0 && value <= operationTimeoutMs - processMs)).toBe(true); + await fixture.coordinator.close(); await fixture.runner.close(); +}); + +it('rechecks the remote after local preparation and refreshes a head that moved in flight', async () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-pre-merge-race-')); roots.push(root); + const repository = join(root, 'repo'); fixtureGit(root, 'init', '-q', repository); + fixtureGit(repository, 'config', 'user.name', 'Test'); fixtureGit(repository, 'config', 'user.email', 'test@example.com'); + writeFileSync(join(repository, 'a.txt'), 'base\n'); fixtureGit(repository, 'add', '.'); fixtureGit(repository, 'commit', '-qm', 'base'); + const base = fixtureGit(repository, 'rev-parse', 'HEAD'); + writeFileSync(join(repository, 'a.txt'), 'feature\n'); fixtureGit(repository, 'commit', '-am', 'feature', '-q'); + const head = fixtureGit(repository, 'rev-parse', 'HEAD'); + const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; + const config = { database: join(root, 'review.sqlite'), repository, runnerRepository: repository, identity, + pathIdentity: { caseSensitive: true, unicodeNormalization: 'none' as const } }; + const service = new ReviewService(config); services.push(service); + const plan: Plan = { schema_version: 1, revision: 1, issue: 1, summary: 'One change', questions: [], items: [{ id: 'P1', + title: 'Change a', intent: 'Change a', files: [{ path: 'a.txt', kind: 'edit', renamed_from: null, change: 'Change it' }], + acceptance: [{ type: 'check', text: 'a changed' }], depends_on: [] }] }; + const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a.txt', kind: 'file' }], pathKey: path => path, allowedCommands: [] }; + service.store.createPlan(JSON.stringify(plan), 'json', context, base, head); + let view = service.load(); + service.store.recordHistory(identity, view.expected, base, head, [{ sha: head, owner: 'P1', origin: 'owned', sourceSha: null }]); + markRunnerOwned(service); view = service.load(); + view = service.act({ action: 'approve', item: 'P1', token: view.token }); + const work = join(root, 'collaborator'); fixtureGit(repository, 'branch', 'collaborator-race', head); + fixtureGit(repository, 'worktree', 'add', '-q', work, 'collaborator-race'); + writeFileSync(join(work, 'a.txt'), 'collaborator\n'); fixtureGit(work, 'commit', '-am', 'collaborator', '-q'); + const moved = { base, head: fixtureGit(work, 'rev-parse', 'HEAD') }; + let reads = 0; + const coordinator = new PreMergeCoordinator(service, + { start() { throw new Error('No command checks expected.'); }, settled: async () => undefined, + stop: () => false, isActive: () => false, status: () => ({ active: false, stopRequested: null, unresolved: null }), + get unreleased() { return null; } } as never, + { run: async () => { throw new Error('No rebase expected.'); }, abort: async () => undefined } as never, + { inspect: async () => ++reads === 1 ? { base, head } : moved, fetch: async () => undefined }); + const task = service.store.getTask(identity); + const result = await coordinator.start({ stateVersion: task.stateVersion, + reviewVersion: view.expected.reviewVersion!, snapshotId: view.snapshot.id, base: view.snapshot.base, head: view.snapshot.head }); + expect(result).toMatchObject({ state: 'review-required', base, head: moved.head, checked: [] }); + expect(result.reason).toMatch(/moved during preparation/); + expect(service.load().segments.some(segment => segment.row === 'Unplanned')).toBe(true); + await coordinator.close(); +}); diff --git a/test/review.test.ts b/test/review.test.ts index fb19c70c..8cbff940 100644 --- a/test/review.test.ts +++ b/test/review.test.ts @@ -8,6 +8,7 @@ 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 { identityKey } from '../core/identity.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) }; }); @@ -16,6 +17,11 @@ vi.setConfig({ testTimeout: 15000 }); const roots:string[]=[];const services:ReviewService[]=[]; afterEach(()=>{services.splice(0).forEach(service=>service.close());roots.splice(0).forEach(root=>rmSync(root,{recursive:true,force:true}));}); function fixture(){const root=mkdtempSync(join(tmpdir(),'codeboost-review-'));roots.push(root);const config=createDemo(join(root,'demo'));const service=new ReviewService(config);services.push(service);return {service,config};} +it('shares one deadline across history reading and linking',()=>{ + const {service}=fixture();let elapsed=0;const clock=vi.spyOn(performance,'now').mockImplementation(()=>elapsed++); + try{expect(()=>service.load({maxDurationMs:200})).toThrow(/deadline/i);} + finally{clock.mockRestore();} +}); it('closes the review service when merge gateway construction fails', async()=>{ const root=mkdtempSync(join(tmpdir(),'codeboost-review-'));roots.push(root);const demo=createDemo(join(root,'demo')); const close=vi.spyOn(ReviewService.prototype,'close'); @@ -196,6 +202,27 @@ it('keeps observing the user\'s HEAD for a task the runner has not committed to, fixtureGit(config.repository,'commit','--allow-empty','-m','User work'); expect(service.load().snapshot.head).toBe(fixtureGit(config.repository,'rev-parse','HEAD')); }); +it('uses a configured runner repository before the first changed runner commit', () => { + const {service,config}=fixture();service.config.runnerRepository=config.repository; + expect(service.reviewRepository()).toEqual({path:config.repository,runnerOwned:true}); +}); +it('reads the base tree from the authoritative runner repository', () => { + const {service,config}=fixture(); + service.config={...config,repository:join(config.repository,'missing'),runnerRepository:config.repository}; + expect(service.planContext().baseEntries.length).toBeGreaterThan(0); +}); +it('keeps a legacy malformed command visible and blocked so the plan can be amended', () => { + const {service,config}=fixture();service.close();services.splice(services.indexOf(service),1); + const db=new DatabaseSync(config.database),key=identityKey(config.identity); + const row=db.prepare('SELECT data FROM revisions WHERE key=? AND revision=1').get(key)!; + const plan=JSON.parse(row.data as string);plan.items[0].acceptance=[{type:'cmd',text:`node "${String.fromCharCode(0xd800)}"`}]; + db.prepare('UPDATE revisions SET data=? WHERE key=? AND revision=1').run(JSON.stringify(plan),key);db.close(); + const reopened=new ReviewService(config);services.push(reopened);const view=reopened.load(); + expect(view.items[0]!.checks.tests).toBe('✕ Invalid command'); + expect(view.items[0]!.state).not.toBe('approved'); + const repaired=structuredClone(view.plan);repaired.items[0]!.acceptance=[{type:'check',text:'Review manually.'}]; + expect(reopened.store.importRevision(JSON.stringify(repaired),'json',reopened.planContext(),view.plan.revision).revision).toBe(2); +}); it('reads a base commit\'s tree once and hands each caller its own copy (#91)',()=>{ const {service,config}=fixture(); const first=service.planContext(); diff --git a/test/runner-branch-push.test.ts b/test/runner-branch-push.test.ts index bdaeabec..f2b6e9be 100644 --- a/test/runner-branch-push.test.ts +++ b/test/runner-branch-push.test.ts @@ -61,6 +61,17 @@ const personPushes = (s: Setup, ref: string) => git(s.source, '-c', 'protocol.fi const remoteRefs = (s: Setup) => git(s.remote, 'for-each-ref', '--format=%(objectname) %(refname)'); describe('GitBranchPusher', () => { + it('keeps exact remote commits reachable across runner-repository maintenance', async () => { + const s = await setup(); + writeFileSync(join(s.source, 'remote.txt'), 'remote\n'); git(s.source, 'add', '.'); git(s.source, 'commit', '-qm', 'remote'); + const head = git(s.source, 'rev-parse', 'HEAD'); + git(s.source, '-c', 'protocol.file.allow=always', 'push', '-q', s.remote, `HEAD:refs/heads/main`); + await pusher(s).instance.fetchCommits([s.base, head]); + expect(git(s.repository.path, 'cat-file', '-t', head)).toBe('commit'); + git(s.repository.path, 'reflog', 'expire', '--expire=now', '--all'); + git(s.repository.path, 'gc', '--prune=now', '-q'); + expect(git(s.repository.path, 'cat-file', '-t', head)).toBe('commit'); + }); it('creates the branch at the head, sending only that ref, and never writes the user\'s repository', async () => { const s = await setup(), head = await runnerCommit(s, 'one'), sourceRefs = git(s.source, 'for-each-ref'); await pusher(s).instance.push(IDENTITY, { head, branch: BRANCH }); diff --git a/test/runner-checks.test.ts b/test/runner-checks.test.ts new file mode 100644 index 00000000..573ca480 --- /dev/null +++ b/test/runner-checks.test.ts @@ -0,0 +1,88 @@ +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, expect, it } from 'vitest'; +import type { InvocationHandle, InvocationInput, InvocationResult } from '../agents/contract.ts'; +import type { Plan, PlanContext } from '../core/plan.ts'; +import { commandCheckDeps, commandDigest } from '../runner/checks.ts'; +import { RunnerCoordinator } from '../runner/coordinator.ts'; +import type { TaskWorkspace, WorkspaceRef } from '../runner/execution.ts'; +import { Store } from '../runner/store.ts'; + +const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; +const oid = (n: number) => n.toString(16).padStart(40, '0'); +const commands = [['npm', 'test']] as const; +const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a', kind: 'file' }], pathKey: path => path, + allowedCommands: commands }; +const plan: Plan = { schema_version: 1, revision: 1, issue: 1, summary: 'Check', questions: [], items: [{ id: 'P1', + title: 'Change', intent: 'Change a', files: [{ path: 'a', kind: 'edit', renamed_from: null, change: 'Change' }], + acceptance: [{ type: 'cmd', text: 'npm test' }], depends_on: [] }] }; +const roots: string[] = [], stores: Store[] = []; +afterEach(() => { for (const store of stores.splice(0)) store.close(); for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); }); + +it('refuses to hash command arguments that a process spawn would normalize', () => { + expect(() => commandDigest([['node', '\ud800']])).toThrow(/well-formed Unicode/); +}); + +function fixture(result: (input: InvocationInput) => Promise) { + const root = mkdtempSync(join(tmpdir(), 'codeboost-checks-')); roots.push(root); + const store = new Store(join(root, 'state.sqlite')); stores.push(store); + store.createPlan(JSON.stringify(plan), 'json', context, oid(1), oid(2)); + const released: string[] = []; + const workspace: TaskWorkspace = { + async materialize(attempt, head) { return { clone: { id: attempt.id, taskId: 'task', directory: root, head }, storage: {} } as WorkspaceRef; }, + async snapshotDeclaredLinks() { throw new Error('not used'); }, async checkTree() { throw new Error('not used'); }, + async inspectChanges() { throw new Error('not used'); }, async commit() { throw new Error('not used'); }, + async release(value) { released.push(value.clone.head); }, + }; + const deps = commandCheckDeps(store, workspace, (input, encoded) => { + expect(encoded).toBe(JSON.stringify(commands)); + const handle: InvocationHandle = { attemptId: input.attemptId, settled: result(input), cancel() {} }; + return handle; + }, commands, 'a'.repeat(32)); + return { store, runner: new RunnerCoordinator(store, deps), released }; +} + +it('records passing evidence only for the exact checked head and command digest', async () => { + const f = fixture(async input => ({ attemptId: input.attemptId, context: input.context, exitCode: 0, signal: null, + stdout: '', stderr: '' })); + const attempt = f.runner.start(identity, { expectedStateVersion: f.store.getTask(identity).stateVersion, kind: 'check', item: 'P1', + deadline: Date.now() + 60_000, expectedContext: f.store.currentContext(identity) }); + await f.runner.settled(identity); + const settled = f.store.getAttempt(identity, attempt.id); + expect({ state: settled.state, diagnostic: settled.diagnostic }).toEqual({ state: 'completed', diagnostic: null }); + expect(f.store.commandChecksPassed(identity, 'P1', oid(2), commandDigest(commands))).toBe(true); + const snapshot = f.store.getSnapshot(identity); + f.store.recordHistory(identity, { revision: 1, snapshotId: snapshot.id, reviewVersion: f.store.reviewVersion(identity) }, oid(1), oid(3), []); + expect(f.store.commandChecksPassed(identity, 'P1', oid(3), commandDigest(commands))).toBe(false); + expect(f.released).toEqual([oid(2)]); + await f.runner.close(); +}); + +it('does not turn a nonzero or stopped command into passing evidence', async () => { + for (const outcome of [{ exitCode: 1, stopReason: undefined }, { exitCode: 0, stopReason: 'cancelled' as const }]) { + const f = fixture(async input => ({ attemptId: input.attemptId, context: input.context, exitCode: outcome.exitCode, + signal: null, ...(outcome.stopReason ? { stopReason: outcome.stopReason } : {}), stdout: '', stderr: 'failed' })); + const attempt = f.runner.start(identity, { expectedStateVersion: f.store.getTask(identity).stateVersion, kind: 'check', item: 'P1', + deadline: Date.now() + 60_000, expectedContext: f.store.currentContext(identity) }); + await f.runner.settled(identity); + expect(f.store.getAttempt(identity, attempt.id).state).toBe('failed'); + expect(f.store.commandChecksPassed(identity, 'P1', oid(2), commandDigest(commands))).toBe(false); + await f.runner.close(); + } +}); + +it('invalidates an older pass when the latest check on the same head fails', async () => { + let exitCode = 0; + const f = fixture(async input => ({ attemptId: input.attemptId, context: input.context, exitCode, signal: null, + stdout: '', stderr: exitCode ? 'failed' : '' })); + for (const expected of [true, false]) { + const attempt = f.runner.start(identity, { expectedStateVersion: f.store.getTask(identity).stateVersion, kind: 'check', item: 'P1', + deadline: Date.now() + 60_000, expectedContext: f.store.currentContext(identity) }); + await f.runner.settled(identity); + expect(f.store.getAttempt(identity, attempt.id).state).toBe(expected ? 'completed' : 'failed'); + expect(f.store.commandChecksPassed(identity, 'P1', oid(2), commandDigest(commands))).toBe(expected); + exitCode = 1; + } + await f.runner.close(); +}); diff --git a/test/runner-command-script.test.ts b/test/runner-command-script.test.ts new file mode 100644 index 00000000..0da431a4 --- /dev/null +++ b/test/runner-command-script.test.ts @@ -0,0 +1,19 @@ +import { spawnSync } from 'node:child_process'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, it } from 'vitest'; + +it('refuses malformed Unicode before spawning an approved command', () => { + const root = mkdtempSync(join(tmpdir(), 'codeboost-command-check-')); + try { + const input = join(root, 'commands.json'); + writeFileSync(input, '[["node","\\ud800"]]'); + const script = fileURLToPath(new URL('../agents/container/command-check.mjs', import.meta.url)); + const result = spawnSync(process.execPath, [script, input]); + expect({ status: result.status, signal: result.signal }).toEqual({ status: 78, signal: null }); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/test/runner-coordinator.test.ts b/test/runner-coordinator.test.ts index 4aabd653..0b7fec6f 100644 --- a/test/runner-coordinator.test.ts +++ b/test/runner-coordinator.test.ts @@ -5,7 +5,7 @@ import { randomUUID } from 'node:crypto'; import { DatabaseSync } from 'node:sqlite'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { Store } from '../runner/store.ts'; -import { RunnerCoordinator, type PreparedAttempt, type RunnerDeps, type StartRequest } from '../runner/coordinator.ts'; +import { PreparationFailure, RunnerCoordinator, combineRunnerDeps, type PreparedAttempt, type RunnerDeps, type StartRequest } from '../runner/coordinator.ts'; import type { InvocationHandle, InvocationInput, InvocationResult, StopReason } from '../agents/contract.ts'; import type { PlanIdentity } from '../core/identity.ts'; import type { Plan, PlanContext } from '../core/plan.ts'; @@ -72,6 +72,22 @@ async function until(check: () => boolean, label: string) { } describe('admission and slots', () => { + it('routes a partial allocation back to the delegate that created it when combined preparation fails', async () => { + const { store } = setup(); + const allocated: PreparedAttempt = { clone: { id: 'partial', taskId: 'task', directory: '/tmp/partial', head: oid(2) }, + vendor: 'runner', approvedArgv: [], private: { storage: 'owned' } }; + const release = vi.fn(async (_attempt, prepared: PreparedAttempt) => { expect(prepared).toBe(allocated); }); + const failing: RunnerDeps = { runnerOwner: RUNNER_OWNER, kinds: ['execute'], + prepare: async () => { throw new PreparationFailure(new Error('materialization failed'), allocated); }, + cleanupPreparation: async () => undefined, start: () => { throw new Error('must not launch'); }, validate: () => null, release }; + const other: RunnerDeps = { ...fakeD().deps, kinds: ['review'] }; + const combined = new RunnerCoordinator(store, combineRunnerDeps(failing, other)); coordinators.push(combined); + const attempt = combined.start(A, request(store, A)); + await combined.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'failed', diagnostic: 'Preparation failed: "materialization failed"' }); + expect(release).toHaveBeenCalledOnce(); + expect(combined.status(A).unresolved).toBeNull(); + }); it('refuses a kind its deps cannot run before writing anything (#91)', () => { const { store } = setup(); const limited = new RunnerCoordinator(store, { ...fakeD().deps, kinds: ['execute'] }); coordinators.push(limited); @@ -132,6 +148,31 @@ describe('admission and slots', () => { }); describe('stops and settlement', () => { + it('persists an owning-operation timeout and refuses to recast it as cancellation', async () => { + const { store, runner, launches, preparations } = setup(); + const attempt = runner.start(A, request(store, A)); + await until(() => preparations.length === 1, 'preparation'); preparations[0]!.resolve(); + await until(() => launches.length === 1, 'launch'); + expect(runner.timeout(A, attempt.id)).toBe(true); + expect(runner.stop(A, attempt.id, 'cancelled')).toBe(false); + expect(runner.status(A).stopRequested).toEqual({ attemptId: attempt.id, reason: 'timeout', saved: true }); + expect(launches[0]!.cancels).toEqual(['timeout']); + launches[0]!.settle({ exitCode: null, signal: 'SIGTERM', stopReason: 'cancelled' }); + await runner.settled(A); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'failed', firstReason: null, + stopReason: 'timeout', diagnostic: 'Timed out.' }); + }); + it('does not launch when an owning-operation timeout lands during preparation', async () => { + const { store, runner, launches, preparations } = setup({ prepareIgnoresAbort: true }); + const attempt = runner.start(A, request(store, A)); + await until(() => preparations.length === 1, 'preparation'); + expect(runner.timeout(A, attempt.id)).toBe(true); + preparations[0]!.resolve(); + await runner.settled(A); + expect(launches).toEqual([]); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'failed', firstReason: null, + stopReason: 'timeout', diagnostic: 'Timed out.' }); + }); it('keeps the slot after cancel until D settles, and keeps the first reason', async () => { const { store, runner, launches, preparations } = setup(); const attempt = runner.start(A, request(store, A)); @@ -366,6 +407,37 @@ describe('review regressions', () => { expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'stale', firstReason: 'stale' }); expect(store.getTask(A).status).not.toBe('needs human'); }); + it('runs a review command check after the code-writing task budget has expired', async () => { + let clock = Date.now(); + const { store, runner, launches, preparations, deps } = setup(); + deps.now = () => clock; + runner.start(A, request(store, A, { budgetMs: 100, deadline: clock + 60_000 })); + await until(() => preparations.length === 1, 'execution preparation'); preparations[0]!.resolve(); + await until(() => launches.length === 1, 'execution launch'); launches[0]!.settle(); + await runner.settled(A); + store.transitionTask(A, store.getTask(A).stateVersion, 'in review'); + clock += 1_000; + const check = runner.start(A, request(store, A, { kind: 'check', deadline: clock + 60_000 })); + await until(() => preparations.length === 2, 'check preparation'); preparations[1]!.resolve(); + await until(() => launches.length === 2, 'check launch'); launches[1]!.settle(); + await runner.settled(A); + expect(store.getAttempt(A, check.id)).toMatchObject({ state: 'completed', firstReason: null }); + expect(store.getTask(A).status).toBe('in review'); + }); + it('revalidates authorization after preparation and refuses a revoked launch', async () => { + const { store, runner, launches, preparations } = setup(); + const calls: string[] = []; + const attempt = runner.start(A, request(store, A, { authorize: async () => { + calls.push('read'); + return () => { calls.push('validate'); throw new Error('Issue trust was revoked.'); }; + } })); + await until(() => preparations.length === 1, 'preparation'); preparations[0]!.resolve(); + await runner.settled(A); + expect(calls).toEqual(['read', 'validate']); + expect(launches).toEqual([]); + expect(store.getAttempt(A, attempt.id)).toMatchObject({ state: 'failed', firstReason: null, + diagnostic: expect.stringMatching(/Authorization changed before launch.*trust was revoked/) }); + }); it('shows the cancel task stop after a preparation timeout, and closes the task', async () => { const { store, runner, preparations } = setup({ prepareIgnoresAbort: true }); const attempt = runner.start(A, request(store, A, { deadline: Date.now() + 30 })); diff --git a/test/runner-production.test.ts b/test/runner-production.test.ts index b5dfde1d..a15f6da4 100644 --- a/test/runner-production.test.ts +++ b/test/runner-production.test.ts @@ -103,12 +103,18 @@ describe('runner startup', () => { buildImage: () => { calls.push('image'); return 'sha256:x'; }, recovery: () => { calls.push('deps'); return recovery(calls); } }); // D's recovery stops every leftover agent before the (possibly long) image build. expect(calls).toEqual(['verify', 'deps', 'recover', 'image']); - expect(assembly.deps.kinds).toEqual(['execute']); + expect(assembly.deps.kinds).toEqual(['execute', 'check']); + expect(assembly.preMerge).toBeTypeOf('function'); // The token is the database's own, kept for this file identity. expect(assembly.deps.runnerOwner).toBe(service.store.runnerOwnerToken({ dev: 1n, ino: 2n })); // The review now reads runner commits from the repository the runner writes. const repositories = join(root, 'runner', assembly.deps.runnerOwner, 'repositories'); expect(service.config.runnerRepository).toBe(join(repositories, readdirSync(repositories)[0]!)); + const snapshot = service.store.getSnapshot(service.config.identity); + for (const commit of new Set([snapshot.base, snapshot.head])) + expect(existsSync(join(service.config.runnerRepository!, 'refs', 'codeboost', 'remote-commits', commit))).toBe(true); + // Selecting a freshly-created runner repository must not make the initial review unreadable before any attempt runs. + expect(service.load().snapshot).toMatchObject({ base: snapshot.base, head: snapshot.head }); // Per database, under its runner token: another database sharing the root keeps its own folder. expect(statSync(join(root, 'runner', assembly.deps.runnerOwner, 'diagnostics')).mode & 0o777).toBe(0o700); expect(existsSync(join(root, 'runner', 'diagnostics'))).toBe(false); @@ -290,6 +296,25 @@ describe('server with a runner setup', () => { expect(await response.json()).toEqual({ error: expect.stringMatching(/run again by resuming the task/) }); expect(app.service.store.getAttempts(demo.identity)).toHaveLength(1); }); + it('refuses to retry an exact-head command check outside pre-merge preparation', async () => { + const { demo } = fixture(); + const app = await startServer({ ...demo }, 0, undefined, undefined, 2_000, undefined, undefined, undefined, async service => { + const store = service.store, identity = demo.identity; + store.transitionTask(identity, store.getTask(identity).stateVersion, 'in review'); + const attempt = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, kind: 'check', item: store.getPlan(identity).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: 1, valid: false }); + return assembly(service); + }); + cleanups.push(() => app.close()); + const origin = new URL(app.url).origin, headers = { 'x-codeboost-token': app.token }; + const view = await (await fetch(`${origin}/api/runner`, { headers })).json() as { stateVersion: number; attempts: { id: string }[] }; + const response = await fetch(`${origin}/api/runner`, { method: 'POST', headers: { ...headers, 'content-type': 'application/json' }, + body: JSON.stringify({ action: 'retry', attemptId: view.attempts[0]!.id, expectedStateVersion: view.stateVersion, actionId: randomUUID() }) }); + expect(await response.json()).toEqual({ error: expect.stringMatching(/preparing the merge/) }); + expect(app.service.store.getAttempts(demo.identity)).toHaveLength(1); + }); it('tells a review without a runner block to add one, and a demo that it never runs the runner', async () => { for (const [demoMode, message] of [[false, RUNNER_NOT_CONFIGURED], [true, RUNNER_NOT_IN_DEMO]] as const) { const { demo } = fixture(); diff --git a/test/runner-publishing.test.ts b/test/runner-publishing.test.ts index d18ae8b2..6e0bdc0b 100644 --- a/test/runner-publishing.test.ts +++ b/test/runner-publishing.test.ts @@ -23,6 +23,7 @@ import type { AlreadyFixedGateway } from '../github/already-fixed.ts'; import type { PlanIdentity } from '../core/identity.ts'; import { approveItem } from '../core/approvals.ts'; import { GhIssueGateway, type IssueTrustGateway } from '../github/issues.ts'; +import type { PreMergeCoordinator } from '../runner/pre-merge.ts'; vi.setConfig({ testTimeout: 30_000 }); const roots: string[] = [], cleanups: (() => Promise | void)[] = []; @@ -231,7 +232,7 @@ function world(): World { * earlier process would have left it. */ async function serve(w: World, options: { before?: (service: ReviewService) => void; onPushSpawn?: (n: number, app: () => App, close: () => Promise) => void; demo?: boolean; startup?: boolean; settleMs?: number; env?: NodeJS.ProcessEnv; hold?: Promise; shortRetryMs?: number; findingSource?: FindingSource; - issueGateway?: IssueTrustGateway } = {}) { + issueGateway?: IssueTrustGateway; preMerge?: (service: ReviewService) => PreMergeCoordinator } = {}) { let app: App | undefined, spawns = 0, closing: Promise | undefined; const close = () => closing ??= app!.close(); let branchOf: (identity: PlanIdentity) => string = () => ''; @@ -280,7 +281,8 @@ async function serve(w: World, options: { before?: (service: ReviewService) => v // `hold`: the attempt keeps running until the test releases it. settled: (options.hold ?? Promise.resolve()).then(() => ({ attemptId: input.attemptId, context: input.context, exitCode: 0, signal: null, stdout: '', stderr: '' })) }), validate: () => ({ head: service.store.getSnapshot(service.config.identity).head, unchanged: true, inScope: [], outOfScope: [] }) }; - return { deps, sources, findings, publisher, ...(options.env ? { env: options.env } : {}), ...(options.shortRetryMs ? { shortRetryMs: options.shortRetryMs } : {}), + return { deps, sources, findings, publisher, ...(options.preMerge ? { preMerge: () => options.preMerge!(service) } : {}), + ...(options.env ? { env: options.env } : {}), ...(options.shortRetryMs ? { shortRetryMs: options.shortRetryMs } : {}), recovery: { finalized: [], requeue: [], removedDirectories: [], unknownEntries: [], unmatchedStorage: [], repairedMerges: [] } }; }); cleanups.push(close); @@ -294,7 +296,7 @@ async function act(app: App, action: string, actionId = randomUUID(), expectedSt 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' || action === 'approve-continuation') ? { expectedReviewVersion: reviewVersion } : {}), actionId }) }); + ...((action === 'start' || action === 'resume' || action === 'approve-continuation' || action === 'prepare-merge') ? { expectedReviewVersion: reviewVersion } : {}), actionId }) }); return { status: response.status, body: await response.json() as Record }; } const publishSettled = (app: App, identity: PlanIdentity) => vi.waitFor(async () => { @@ -1357,6 +1359,56 @@ describe('github.baseBranch', () => { describe('closing a cancelled task\'s pull requests (#111)', () => { const closedRecord = { outcome: 'closed', action: 'close' }; + it('settles pre-merge before shutdown closes Store writes', async () => { + const w = world(); + const issueGateway: IssueTrustGateway = { + repository: REPO, + async fetch() { return { repository: REPO, retrievedAt: new Date().toISOString(), issues: [] }; }, + async issueAccess(number) { return { number, authorLogin: 'member', collaborator: true }; }, + async issueText(number) { return { number, title: '', body: '', comments: [] }; }, + }; + let writesClosed: boolean | undefined; + const { close } = await serve(w, { issueGateway, preMerge: service => ({ + get active() { return false; }, get last() { return null; }, assertStartable() {}, + start: async () => { throw new Error('not used'); }, cancelTask: () => 'closed' as const, + close: async () => { writesClosed = service.store.writesClosed; }, + } as unknown as PreMergeCoordinator) }); + await close(); + expect(writesClosed).toBe(false); + }); + + it('closes a ready PR after an asynchronously cancelled pre-merge preparation settles', async () => { + const w = world(), settle = Promise.withResolvers(); + const issueGateway: IssueTrustGateway = { + repository: REPO, + async fetch() { return { repository: REPO, retrievedAt: new Date().toISOString(), issues: [] }; }, + async issueAccess(number) { return { number, authorLogin: 'member', collaborator: true }; }, + async issueText(number) { return { number, title: '', body: '', comments: [] }; }, + }; + let active = false, cancelId: string | undefined; + const { app, identity, store } = await serve(w, { before: completeAll, issueGateway, preMerge: service => ({ + get active() { return active; }, get last() { return null; }, assertStartable() {}, + start: () => { + active = true; + return settle.promise.then(() => { + service.store.cancelTask(service.config.identity, service.store.getTask(service.config.identity).stateVersion, cancelId!); + active = false; + const snapshot = service.store.getSnapshot(service.config.identity); + return { state: 'failed' as const, base: snapshot.base, head: snapshot.head, checked: [], reason: 'Task cancelled.' }; + }); + }, + cancelTask: (_version: number, actionId: string) => { cancelId = actionId; settle.resolve(); return 'stopping' as const; }, + close: async () => undefined, + } as unknown as PreMergeCoordinator) }); + await publishSettled(app, identity); + expect(w.github.prs).toEqual([expect.objectContaining({ open: true, draft: false })]); + expect(await act(app, 'prepare-merge')).toMatchObject({ status: 200, body: { result: { outcome: 'preparing' } } }); + expect((await act(app, 'cancel-task')).body.result).toEqual({ outcome: 'stopping' }); + await vi.waitFor(() => expect(store.lastPublish(identity)).toMatchObject(closedRecord), { timeout: 20_000, interval: 20 }); + expect(store.getTask(identity).status).toBe('cancelled'); + expect(w.github.prs[0]!.open).toBe(false); + }); + it('closes a ready PR when a task in review is cancelled, and keeps the branch', async () => { const w = world(); const { app, identity, store, branch, head } = await serve(w, { before: completeAll }); diff --git a/test/runner-rebase-conflict.test.ts b/test/runner-rebase-conflict.test.ts index 6f7a945b..609bc12f 100644 --- a/test/runner-rebase-conflict.test.ts +++ b/test/runner-rebase-conflict.test.ts @@ -165,6 +165,68 @@ describe('production rebase conflict resolver', () => { expect(f.store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: null, processGroup: null }); }); + it('revalidates authorization immediately before the credentialed conflict agent launch', async () => { + const f = fixture(), x = deps(f), calls: string[] = []; + const resolve = createForeignConflictResolver({ store: f.store, identity, planKey: f.planKey, + repository: { path: join(f.root, 'bare.git') } as RunnerRepository, runnerOwner: 'a'.repeat(32), + image: () => 'sha256:' + 'b'.repeat(64), token: 'secret', + authorize: async () => { calls.push('read'); return () => { calls.push('validate'); throw new Error('Issue trust was revoked.'); }; }, + limits: { workBytes: 1, workInodes: 1, metadataBytes: 1, metadataInodes: 1 }, deps: x.d as never }); + await expect(resolve(input(f))).rejects.toThrow(/Issue trust was revoked/); + expect(calls).toEqual(['read', 'validate']); + expect(x.events).not.toContain('start'); + expect(f.store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: null, processGroup: null }); + }); + + it('does not invoke the authorization validator after the read consumes the work deadline', async () => { + const f = fixture(), x = deps(f), calls: string[] = [], realNow = performance.now.bind(performance); + let expired = false; + vi.spyOn(performance, 'now').mockImplementation(() => expired ? Number.MAX_SAFE_INTEGER : realNow()); + const resolve = createForeignConflictResolver({ store: f.store, identity, planKey: f.planKey, + repository: { path: join(f.root, 'bare.git') } as RunnerRepository, runnerOwner: 'a'.repeat(32), + image: () => 'sha256:' + 'b'.repeat(64), token: 'secret', + authorize: async () => { calls.push('read'); await Promise.resolve(); expired = true; return () => { calls.push('validate'); }; }, + limits: { workBytes: 1, workInodes: 1, metadataBytes: 1, metadataInodes: 1 }, deps: x.d as never }); + await expect(resolve(input(f))).rejects.toThrow(/deadline/); + expect(calls).toEqual(['read']); + expect(x.events).not.toContain('start'); + }); + + it('refuses launch and releases resources when the authorization hook throws synchronously', async () => { + const f = fixture(), x = deps(f); + const resolve = createForeignConflictResolver({ store: f.store, identity, planKey: f.planKey, + repository: { path: join(f.root, 'bare.git') } as RunnerRepository, runnerOwner: 'a'.repeat(32), + image: () => 'sha256:' + 'b'.repeat(64), token: 'secret', + authorize: (() => { throw new Error('Issue trust read failed.'); }) as never, + limits: { workBytes: 1, workInodes: 1, metadataBytes: 1, metadataInodes: 1 }, deps: x.d as never }); + await expect(resolve(input(f))).rejects.toThrow(/Issue trust read failed/); + expect(x.events).not.toContain('start'); + expect(f.store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: null, processGroup: null }); + }); + + it('retains a running child without leaking its rejection when the deadline passes before settlement is awaited', async () => { + const f = fixture(), realNow = performance.now.bind(performance), failed = Promise.withResolvers(); + let expired = false; + vi.spyOn(performance, 'now').mockImplementation(() => expired ? Number.MAX_SAFE_INTEGER : realNow()); + const unhandled: unknown[] = [], onUnhandled = (reason: unknown) => { unhandled.push(reason); }; + process.on('unhandledRejection', onUnhandled); + try { + const x = deps(f, { start: (value: AgentAdapterRequest) => { + expired = true; + return { attemptId: value.invocation.attemptId, cancel: vi.fn(), settled: failed.promise } satisfies InvocationHandle; + } }); + const resolve = createForeignConflictResolver({ store: f.store, identity, planKey: f.planKey, + repository: { path: join(f.root, 'bare.git') } as RunnerRepository, runnerOwner: 'a'.repeat(32), + image: () => 'sha256:' + 'b'.repeat(64), token: 'secret', + limits: { workBytes: 1, workInodes: 1, metadataBytes: 1, metadataInodes: 1 }, deps: x.d as never }); + await expect(resolve(input(f))).rejects.toThrow(/retained resources/); + expect(f.store.getTask(identity).rebaseInProgress).toMatchObject({ conflict: { source: oid(2) } }); + failed.reject(new Error('late child failure')); + await new Promise(resolve => setTimeout(resolve, 0)); + expect(unhandled).toEqual([]); + } finally { process.off('unhandledRejection', onUnhandled); } + }); + it('bounds the complete host snapshot before changing the destination', () => { const root = mkdtempSync(join(tmpdir(), 'codeboost-conflict-copy-')); roots.push(root); const source = join(root, 'source'), destination = join(root, 'destination'); @@ -348,7 +410,7 @@ describe('production rebase conflict resolver', () => { const settlementDeadline = Date.now() + CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS + 1_000; const running = resolve({ ...input(f), deadline: settlementDeadline }); const rejected = expect(running).rejects.toThrow(/retained resources/); - for (let turn = 0; turn < 20 && !request; turn++) await Promise.resolve(); + for (let turn = 0; turn < 200 && !request; turn++) await Promise.resolve(); expect(request?.invocation.deadline).toBe(settlementDeadline - CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS); await vi.advanceTimersByTimeAsync(CONFLICT_PROCESS_SETTLEMENT_RESERVE_MS + 1_000); await rejected; diff --git a/test/runner-rebase.test.ts b/test/runner-rebase.test.ts index 7727ae1b..cc47ca69 100644 --- a/test/runner-rebase.test.ts +++ b/test/runner-rebase.test.ts @@ -5,7 +5,8 @@ import { dirname, join } from 'node:path'; import { randomUUID } from 'node:crypto'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { fixtureGit as git } from './fixtures/git.ts'; -import { GitRebaser, RebaseConflict, RebaseResourcesUnsettled, rebaseRef, type GitRebaserOptions } from '../runner/rebase.ts'; +import { GitRebaser, MIN_REBASE_CLEANUP_TIMEOUT_MS, RebaseConflict, RebaseResourcesUnsettled, rebaseRef, + type GitRebaserOptions } from '../runner/rebase.ts'; import { ensureCommit, openRunnerRepository } from '../runner/runner-repository.ts'; import { splitBoundedIndexRecords, verifyCheckout } from '../runner/verify-checkout.ts'; import { readBoundedRebaseStateNames } from '../runner/hash-rebase-state.ts'; @@ -65,6 +66,10 @@ function createRebaser(s: Awaited>, options: Partial { + it('reserves Git work as well as process settlement for cleanup-only calls', () => { + expect(MIN_REBASE_CLEANUP_TIMEOUT_MS).toBe(30_000); + }); + it('bounds rebase-state directory names while they are enumerated', () => { const names = [Buffer.from('one'), Buffer.from('two'), Buffer.from('three')]; let index = 0; diff --git a/test/runner-start.test.ts b/test/runner-start.test.ts index 7796ddb0..7123e19b 100644 --- a/test/runner-start.test.ts +++ b/test/runner-start.test.ts @@ -13,6 +13,7 @@ import { SafetyFindings, type ExecutionSources } from '../runner/execution.ts'; import type { AttemptKind } from '../runner/lifecycle.ts'; import { approveItem } from '../core/approvals.ts'; import type { IssueTrustGateway } from '../github/issues.ts'; +import { fixtureGit } from './fixtures/git.ts'; vi.setConfig({ testTimeout: 20_000 }); const roots: string[] = [], cleanups: (() => Promise | void)[] = []; @@ -85,7 +86,13 @@ function failedFirstItem(service: ReviewService, budgetMs?: number) { function committedFirstItem(service: ReviewService, unchanged = false, outOfScope: string[] = [], kind: AttemptKind = 'execute') { const s = service.store, id = service.config.identity; s.transitionTask(id, s.getTask(id).stateVersion, 'queued'); - const item = s.getPlan(id).items[0]!.id, snapshot = s.getSnapshot(id), head = 'f'.repeat(40); + const item = s.getPlan(id).items[0]!.id, snapshot = s.getSnapshot(id); + // Production selects the runner-owned repository before settlement. This fixture uses the demo repository as that + // authority and, for a changed result, creates a real commit object without moving its checkout. + service.config.runnerRepository = service.config.repository; + const tree = unchanged ? null : fixtureGit(service.config.repository, 'rev-parse', `${snapshot.head}^{tree}`); + const head = tree === null ? snapshot.head + : fixtureGit(service.config.repository, 'commit-tree', tree, '-p', snapshot.head, '-m', `Runner fixture ${randomUUID()}`); const attempt = s.admitAttempt(id, { expectedStateVersion: s.getTask(id).stateVersion, kind, item, expectedContext: s.currentContext(id), deadline: Date.now() + 60_000 }); s.markRunning(id, attempt.id); diff --git a/test/store.test.ts b/test/store.test.ts index 67355f21..fcf945fb 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -2,16 +2,19 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { execFileSync, spawn } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; import { fixtureGit } from './fixtures/git.ts'; import { once } from 'node:events'; import { DatabaseSync } from 'node:sqlite'; import { afterEach, expect, it, vi } from 'vitest'; -import { Store, requireSupportedNode } from '../runner/store.ts'; +import { MAX_REWRITE_LINEAGE_ROWS, Store, requireSupportedNode } from '../runner/store.ts'; import type { Plan, PlanContext, EditReply } from '../core/plan.ts'; import { approveItem, approvalStates, choiceKeys, applyChoices, stable } from '../core/approvals.ts'; import { linkHistory, type Segment } from '../core/linking.ts'; import { readHistory } from '../git/history.ts'; import { identityKey } from '../core/identity.ts'; +import { GuardRefusal } from '../runner/lifecycle.ts'; +import { commandDigest } from '../runner/checks.ts'; const identity = { repositoryId: 'repo', taskId: 'task', planId: 'plan' }; const context: PlanContext = { identity, issue: 1, baseEntries: [{ path: 'a', kind: 'file' }], pathKey: p => p, allowedCommands: [] }; const plan = (): Plan => ({ schema_version: 1, revision: 99, issue: 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: [] }] }); @@ -28,6 +31,69 @@ function fixture(two = false) { const path = join(directory(), 'state.sqlite'); 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('settles prepare-merge replays through shutdown and fails interrupted preparations at startup recovery', () => { + const { store } = fixture(), request = { expectedStateVersion: 0, expectedReviewVersion: 0 }; + const completed = randomUUID(), interrupted = randomUUID(); + store.userAction(identity, { actionId: completed, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + store.userAction(identity, { actionId: interrupted, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + store.settleInterruptedPreMergeActions(identity); + expect(store.savedAction(identity, { actionId: completed, kind: 'prepare-merge', request })?.response) + .toMatchObject({ outcome: 'failed', reason: expect.stringMatching(/restarted/) }); + const active = randomUUID(); + store.userAction(identity, { actionId: active, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + const snapshot = store.getSnapshot(identity); + const readiness = { stateVersion: store.getTask(identity).stateVersion, reviewVersion: store.reviewVersion(identity), + snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, commandPolicyDigest: commandDigest([]) }; + const capability = store.shutdownCapability(); store.closeWrites(); + capability.run(() => store.settlePreMergeAction(identity, active, + { state: 'ready', base: oid(1), head: oid(2), checked: ['P1'], reason: null }, readiness)); + expect(store.savedAction(identity, { actionId: active, kind: 'prepare-merge', request })?.response) + .toEqual({ outcome: 'ready', base: oid(1), head: oid(2), checked: ['P1'], reason: null }); + expect(store.preMergeReady(identity, readiness)).toBe(true); +}); +it('does not let a refused preparation shadow an admitted preparation that later becomes ready', () => { + const { store } = fixture(), request = { expectedStateVersion: 0, expectedReviewVersion: 0 }; + const admitted = randomUUID(); + store.userAction(identity, { actionId: admitted, kind: 'prepare-merge', request }, () => ({ outcome: 'preparing' })); + expect(() => store.userAction(identity, { actionId: randomUUID(), kind: 'prepare-merge', request }, () => { + throw new GuardRefusal('Pre-merge preparation is already running.'); + })).toThrow('Pre-merge preparation is already running.'); + const snapshot = store.getSnapshot(identity); + const readiness = { stateVersion: store.getTask(identity).stateVersion, reviewVersion: store.reviewVersion(identity), + snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, commandPolicyDigest: commandDigest([]) }; + store.settlePreMergeAction(identity, admitted, + { state: 'ready', base: snapshot.base, head: snapshot.head, checked: [], reason: null }, readiness); + expect(store.preMergeReady(identity, readiness)).toBe(true); +}); +it('fails closed when durable rewrite lineage exceeds its safety bound', () => { + const { store } = fixture(); + let head = oid(2); + for (let index = 0; index <= MAX_REWRITE_LINEAGE_ROWS; index++) { + const next = oid(index + 3); + store.recordRebase(identity, state(store), oid(1), next, [{ oldSha: head, newSha: next }]); + head = next; + } + expect(() => store.isRewrittenHead(identity, oid(2), head)).toThrow(/exceeds the 1000-row safety limit/); + expect(() => store.rewrittenAncestors(identity, head)).toThrow(/exceeds the 1000-row safety limit/); +}); +it('recovers an interrupted review check without applying the expired code-writing budget', () => { + const { store } = fixture(); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const now = Date.now(); + const execute = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: now + 60_000, budgetMs: 10, + expectedContext: store.currentContext(identity), now }); + store.markRunning(identity, execute.id); + store.settleAttempt(identity, execute.id, { firstReason: null, exitCode: 0, valid: true }); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'in review'); + const check = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, + kind: 'check', item: 'P1', deadline: now + 60_000, + expectedContext: store.currentContext(identity), now: now + 1_000 }); + const [recovered] = store.recoverInterrupted(now + 2_000); + expect(recovered).toMatchObject({ attemptId: check.id, state: 'failed', requeued: false }); + expect(store.getAttempt(identity, check.id)).toMatchObject({ state: 'failed', firstReason: null }); + expect(store.getTask(identity).status).toBe('in review'); +}); it('allocates revisions in SQLite, survives reopen, and keeps old revisions and snapshots immutable', () => { const { store, path } = fixture(); const first = store.getSnapshot(identity); expect(store.getPlan(identity).revision).toBe(1); @@ -206,6 +272,93 @@ it('preserves owned, foreign, and conflict-resolution provenance through repeate expect(recovered.getLedger(identity).find(e => e.sha === oid(24))).toEqual({ sha: oid(24), owner: null, origin: 'foreign', sourceSha: oid(14) }); expect(recovered.getRewrites(identity, snapshot.id)).toHaveLength(3); }); +it('preserves execution approval recorded at the rewritten final output snapshot', () => { + const { store } = fixture(); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const attempt = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: Date.now() + 60_000, expectedContext: store.currentContext(identity) }); + store.markRunning(identity, attempt.id); + store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { unchanged: false, head: oid(3) } }); + store.recordHistory(identity, state(store), oid(1), oid(3), [ + { sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }, + { sha: oid(3), owner: 'P1', origin: 'owned', sourceSha: null }, + ]); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'in review'); + store.saveReview(identity, state(store), [approveItem(store.getPlan(identity), [], 'P1', identity, true)], []); + store.recordRebase(identity, state(store), oid(10), oid(13), [ + { oldSha: oid(2), newSha: oid(12) }, { oldSha: oid(3), newSha: oid(13) }, + ]); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); +}); +it('binds pre-execution approvals to the current snapshot across a rewrite of the reviewed head', () => { + const { store } = fixture(); + store.recordHistory(identity, state(store), oid(1), oid(3), [ + { sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }, + { sha: oid(3), owner: 'P1', origin: 'owned', sourceSha: null }, + ]); + store.saveReview(identity, state(store), [approveItem(store.getPlan(identity), [], 'P1', identity, true)], []); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); + store.recordRebase(identity, state(store), oid(10), oid(13), [ + { oldSha: oid(2), newSha: oid(12) }, { oldSha: oid(3), newSha: oid(13) }, + ]); + expect(store.isRewrittenHead(identity, oid(3), oid(13))).toBe(true); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); + store.recordHistory(identity, state(store), oid(1), oid(3), []); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); +}); +it('keeps a later attribution choice binding before execution when a collaborator head returns', () => { + const { store } = fixture(); + store.saveReview(identity, state(store), [approveItem(store.getPlan(identity), [], 'P1', identity, true)], []); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); + store.recordHistory(identity, state(store), oid(1), oid(5), []); + store.saveReview(identity, state(store), [], [{ key: 'later-choice', action: 'accept', item: null }]); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); + store.recordHistory(identity, state(store), oid(1), oid(2), []); + expect(store.getSnapshot(identity).head).toBe(oid(2)); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); + store.saveReview(identity, state(store), [approveItem(store.getPlan(identity), [], 'P1', identity, true)], []); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); +}); +it('keeps a later attribution choice binding after a collaborator head replaces its snapshot', () => { + const { store } = fixture(); + store.saveReview(identity, state(store), [approveItem(store.getPlan(identity), [], 'P1', identity, true)], []); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const attempt = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: Date.now() + 60_000, expectedContext: store.currentContext(identity) }); + store.markRunning(identity, attempt.id); + store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { unchanged: false, head: oid(3) } }); + store.recordHistory(identity, state(store), oid(1), oid(3), [ + { sha: oid(2), owner: 'P1', origin: 'owned', sourceSha: null }, + { sha: oid(3), owner: 'P1', origin: 'owned', sourceSha: null }, + ]); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'in review'); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual([]); + store.saveReview(identity, state(store), [], [{ key: 'later-choice', action: 'accept', item: null }]); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); + store.recordHistory(identity, state(store), oid(1), oid(5), []); + expect(store.unapprovedExecutionItems(identity, store.getPlan(identity).revision)).toEqual(['P1']); +}); +it('preserves an approved checkpoint continuation across its validated rewrite lineage', () => { + const { store } = fixture(true); + store.transitionTask(identity, store.getTask(identity).stateVersion, 'queued'); + const attempt = store.admitAttempt(identity, { expectedStateVersion: store.getTask(identity).stateVersion, + kind: 'execute', item: 'P1', deadline: Date.now() + 60_000, expectedContext: store.currentContext(identity) }); + store.markRunning(identity, attempt.id); + store.settleAttempt(identity, attempt.id, { firstReason: null, exitCode: 0, valid: true, + result: { unchanged: false, head: oid(2) } }); + 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); + store.recordRebase(identity, state(store), oid(10), oid(12), [{ oldSha: oid(2), newSha: oid(12) }]); + const progress = store.continuationProgress(identity); + expect(progress).toMatchObject({ completed: ['P1'], head: oid(12), next: 'P2' }); + expect(store.continuationApproved(identity, progress!)).toBe(true); +}); it('feeds persisted remapped ownership into the linking engine on real Git history', () => { const dir = directory(); const git = (...args: string[]) => fixtureGit(dir, ...args); git('init', '-b', 'main'); git('config', 'user.name', 'Test'); git('config', 'user.email', 'test@example.invalid'); git('config', 'commit.gpgsign', 'false'); diff --git a/web/server.ts b/web/server.ts index 44127bf6..43f3c397 100644 --- a/web/server.ts +++ b/web/server.ts @@ -20,6 +20,7 @@ import { IssueBoard } from './issues.ts'; import { SuggestionCoordinator, type PlanningMode, type SuggestionHandle, type SuggestionInput, type SuggestionStore } from '../core/planning-suggestions.ts'; import type { AuthorProvider } from '../core/planning-author.ts'; import { PLANNING_BUDGET_MS } from '../runner/planning-provider.ts'; +import type { PreMergeCoordinator } from '../runner/pre-merge.ts'; export type PlanningDescription = Pick & { repo: { name: string; baseRef: string }; /** Revalidates any mutable authority carried by this description at the synchronous prompt-construction boundary. */ @@ -61,6 +62,7 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if (!Number.isSafeInteger(shutdownDrainMs) || shutdownDrainMs < 1 || shutdownDrainMs > MAX_SHUTDOWN_DRAIN_MS) throw new Error('Invalid shutdown drain deadline.'); const service = new ReviewService(config), token = randomBytes(32).toString('hex'); let questions: Questions, merges: MergeCoordinator | null, issues: IssueBoard, runner: RunnerCoordinator | null, suggestions: SuggestionCoordinator | null; + let preMerge: PreMergeCoordinator | null = null; let planning: PlanningDeps | undefined, issueSource: IssueGateway | null; /** Runs a task's plan items; one per Store, like the coordinator. Only the production runner has one. */ let executor: ItemExecutor | null = null; @@ -80,7 +82,8 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge (repository, issue) => service.store.issueTrust(repository, issue)); // With a runner block the merge targets the task's published PR (#121); without one, github.pullRequest is required. const published = !config.demo && config.runner !== undefined && config.github - ? { repository: config.github.repository, baseBranch: baseBranch(config.github), ...(config.github.pullRequest !== undefined ? { configured: config.github.pullRequest } : {}) } : undefined; + ? { repository: config.github.repository, baseBranch: baseBranch(config.github), requiresPreparation: true, + ...(config.github.pullRequest !== undefined ? { configured: config.github.pullRequest } : {}) } : undefined; if (!config.demo && config.github && !published && !mergeGateway && config.github.pullRequest === undefined) throw new Error('Add github.pullRequest, the pull request to merge, to the review configuration, or add a runner block so codeboost publishes its own.'); merges = !config.demo && (mergeGateway || config.github) ? new MergeCoordinator(service, mergeGateway ?? new GhMergeGateway(config.github!), MERGE_OPERATION_TIMEOUT_MS, capability, published) : null; @@ -105,6 +108,14 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge try { const assembly = await runnerSetup(service, capability); runner = new RunnerCoordinator(service.store, assembly.deps, undefined, capability); + if (assembly.preMerge && merges) preMerge = assembly.preMerge(runner, signal => merges!.remotePair(signal), async signal => { + let access = await readIssueAccess(signal); + requireTrustedIssue(access); + return { + refresh: async () => { access = await readIssueAccess(signal); requireTrustedIssue(access); }, + validate: () => requireTrustedIssue(access), + }; + }); executor = new ItemExecutor(service.store, runner, assembly.sources, assembly.findings, { capability }); // A demo never publishes, whatever its github block or an injected setup provides (setUpRunner refuses demos too). if (assembly.publisher && !config.demo) { const coordinator = runner; publishing = new TaskPublishing(service.store, assembly.publisher(() => coordinator.closing), runner, executor, capability, assembly.env, { @@ -143,6 +154,14 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge requireTrustedIssue(access); return async () => requireTrustedIssue(await readIssueAccess(signal)); }; + if (config.github) merges?.setAuthorization(async signal => { + let access = await readIssueAccess(signal); + requireTrustedIssue(access); + return { + refresh: async () => { access = await readIssueAccess(signal); requireTrustedIssue(access); }, + validate: () => requireTrustedIssue(access), + }; + }); /** * What `start` or `resume` would run (#91 part 2), or the local refusal. It writes nothing, so the view asks it too. * The view also suppresses controls when the latest complete issue board says trust is blocked; every action still @@ -258,7 +277,8 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const trustBlocked = !!config.github && issues.trustStatus(config.github.issue) === 'blocked'; return { available: !!runner, task, attempts, startable: !trustBlocked && !!progress && offered('start', progress), resumable: !trustBlocked && !!progress && offered('resume', progress), stateVersion: task.stateVersion, reviewVersion: service.store.reviewVersion(identity), retryable, stopRequested: status.stopRequested, - unresolved: status.unresolved, continuation, publish: publishView(progress, trustBlocked) }; + unresolved: status.unresolved, continuation, publish: publishView(progress, trustBlocked), + preMerge: { available: !!preMerge, active: preMerge?.active ?? false, last: preMerge?.last ?? null } }; }; /** The task's publishing (#103): in progress, offered (what the publish action would run), and the last outcome. */ const publishView = (progress: ReturnType | undefined, trustBlocked: boolean) => { @@ -277,25 +297,32 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if (!publishing || stopping) return; publishing.actIfOwed(identity); }); + /** A preparation can finish the delayed half of task cancellation; close its PRs only after that settlement. */ + const afterPreMerge = (outcome: Promise) => void outcome + .catch(error => console.error(`Pre-merge preparation failed: ${JSON.stringify(error instanceof Error ? error.message : String(error))}`)) + .finally(() => { + if (!publishing || stopping || service.store.getTask(identity).status !== 'cancelled') return; + publishing.taskCancelled(identity); + }); const runnerAction = async (input: Record, signal: AbortSignal) => { 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', 'approve-continuation', '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', 'prepare-merge'].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' || action === 'approve-continuation') && !Number.isSafeInteger(expectedReviewVersion)) { + if ((action === 'start' || action === 'resume' || action === 'approve-continuation' || action === 'prepare-merge') && !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, resume and continuation approval.'); + throw new BadRequest('expectedReviewVersion must be an integer for start, resume, preparation and continuation approval.'); } if (action === 'cancel-attempt' || action === 'retry') assertUuidV4(attemptId, 'Attempt ID'); - const request = { attemptId, expectedStateVersion, ...((action === 'start' || action === 'resume' || action === 'approve-continuation') ? { expectedReviewVersion } : {}) }; + const request = { attemptId, expectedStateVersion, ...((action === 'start' || action === 'resume' || action === 'approve-continuation' || action === 'prepare-merge') ? { expectedReviewVersion } : {}) }; let access: IssueAccess | undefined; const trustGated = !!config.github && (((action === 'start' || action === 'resume') && !!executor) || - (action === 'approve-continuation' && !!executor) || (action === 'publish' && !!publishing)); + (action === 'approve-continuation' && !!executor) || (action === 'publish' && !!publishing) || (action === 'prepare-merge' && !!preMerge)); if (trustGated) { const replay = service.store.savedAction(identity, { actionId: actionId as string, kind: action as string, request }); if (replay) return replay.response; @@ -334,7 +361,8 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge } } // 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). + // stopping an attempt closes the task when the attempt settles; the execution or pre-merge run's end then closes + // them (afterRun). // Only this action's own cancel: a replayed or refused one (the task already closed) starts nothing. if (action === 'cancel-task') { const before = service.store.getTask(identity).status; @@ -357,12 +385,27 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge // 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 ((action === 'approve-continuation' || action === 'prepare-merge') && (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' || action === 'approve-continuation') && service.store.reviewVersion(identity) !== expectedReviewVersion) + if ((action === 'start' || action === 'resume' || action === 'approve-continuation' || action === 'prepare-merge') && 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 (action === 'cancel-task') return { outcome: preMerge?.active + ? preMerge.cancelTask(expectedStateVersion as number, actionId as string) + : 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 === 'prepare-merge') { + if (access) requireTrustedIssue(access); + if (!preMerge) throw new GuardRefusal('Pre-merge preparation is not configured.'); + if (runner.isActive(identity) || executor?.busy(identity) || publishing?.busy(identity)) + throw new GuardRefusal('The runner or publisher is busy; pre-merge preparation cannot start yet.'); + preMerge.assertStartable(); + const snapshot = service.store.getSnapshot(identity); + service.store.afterCommit(() => { afterPreMerge(preMerge!.start({ stateVersion: expectedStateVersion as number, + reviewVersion: expectedReviewVersion as number, snapshotId: snapshot.id, base: snapshot.base, head: snapshot.head, + actionId: actionId as string })); }); + return { outcome: 'preparing' }; + } if (action === 'approve-continuation') { if (access) requireTrustedIssue(access); if (!executor || runner.isActive(identity) || executor.busy(identity) || publishing?.busy(identity)) @@ -415,6 +458,9 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const last = service.store.getAttempt(identity, attemptId as string); // A plan item runs only through the executor, which pauses or escalates after it; a bare retry would skip both. if (executor && last.kind === 'execute') throw new GuardRefusal('A plan item is run again by resuming the task, not by retrying its attempt.'); + // Exact-head command checks belong to pre-merge preparation, which refreshes the remote pair, review state and + // issue authorization before every launch. A bare retry would bypass all of those guards. + if (last.kind === 'check') throw new GuardRefusal('Command checks are run again by preparing the merge, not by retrying their attempt.'); const retry = runner.retry(identity, attemptId as string, { expectedStateVersion: expectedStateVersion as number, kind: last.kind, item: last.item, expectedContext: service.store.currentContext(identity), deadline: Date.now() + 10 * 60_000 }); // A cancel that stops this attempt closes the task when it settles; its PRs are closed then (#111), as after a run. @@ -693,6 +739,11 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge if(active.readingBody)active.request.destroy(reason); } const publishingFailure = await publishingClosed; + // A conflict resolver records and clears child process ownership directly through the Store. Abort and await the + // whole pre-merge lifecycle before the write gate closes, so those settlement writes cannot be refused. + const failures: unknown[] = publishingFailure === undefined ? [] : [publishingFailure]; + const step = async (run: () => Promise | unknown) => { try { await run(); } catch (error) { failures.push(error); } }; + await step(() => preMerge?.close()); // Step 3: after the drain, close the Store write gate. A request-path write still pending after the abort // (for example merge reconciliation after a GitHub await) now fails with 503; settling coordinators keep the capability. service.store.closeWrites(); @@ -701,8 +752,6 @@ export async function startServer(config: ReviewConfig, port = 4318, questionAge const issuesClosed=issues.close(); // Every step runs even when an earlier one fails: agents are still stopped, plan runs awaited and the Store closed // last. The first failure is reported; a later one never hides it. - const failures: unknown[] = publishingFailure === undefined ? [] : [publishingFailure]; - const step = async (run: () => Promise | unknown) => { try { await run(); } catch (error) { failures.push(error); } }; await step(() => merges?.close()); await step(() => issuesClosed); // Step 4: stop runner jobs (shutdown reason only where none is set) and await settlement; no timer abandons a job.