From 1746a94e448ef1367b9b7691c9ed0904b7d68c85 Mon Sep 17 00:00:00 2001 From: karim Date: Fri, 16 Oct 2026 22:47:19 +0200 Subject: [PATCH 1/4] fix(executor-host): the refusals name the rule and the remedy that exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two refusal wordings in the host executor named what they should not and withheld what they should. The continueFrom refusal (harnessHandlers.ts ~581, after ownedSession returns nothing) guessed expiry or an executor restart; both guesses were wrong in the observed case and the wording sent the diagnosis there. The comment above it is right that the answer must be the one a session that never existed would get — naming the session as another run's confirms it exists. The refusal now states the rule without confirming anything: a kept session is owned by the run that made it, every commission is a new run, so a continueFrom can only name a session this same run made, and a session from any other run is refused as if it did not exist. A session this run made that expired or was lost to a restart is a different refusal. The gate's scanAgain (scanReceipt.ts ~184) and checkAgain (checkReceipt.ts ~181) told the reader to run host.commit.scan or host.commit.check and push with the receipt — operations the publication machinery holds and the Helmsman, the only agent that reads the refusal, is refused promotion of (out_of_scope). The remedy that exists is the publication: it scans and checks what it sends. Both refusals now say to publish the branch again as it stands, the way the other_range refusal in the same file already does, and name neither operation. The structured refusal codes are unchanged. Tests state each property: the continueFrom refusal for a session another run owns states the ownership rule and offers neither expiry nor restart as a guess; no gate refusal names host.commit.scan or host.commit.check. --- .../src/__tests__/commitCheck.test.ts | 20 +- .../src/__tests__/harnessContinueFrom.test.ts | 134 +++++++++++ .../src/__tests__/pushGateRefusals.test.ts | 227 ++++++++++++++++++ apps/aflow-executor-host/src/checkReceipt.ts | 12 +- .../src/handlers/harnessHandlers.ts | 9 +- apps/aflow-executor-host/src/scanReceipt.ts | 29 ++- 6 files changed, 415 insertions(+), 16 deletions(-) create mode 100644 apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts create mode 100644 apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts diff --git a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts index 533ef95a..a078ac0f 100644 --- a/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts +++ b/apps/aflow-executor-host/src/__tests__/commitCheck.test.ts @@ -431,7 +431,14 @@ describe('host.commit.check — the receipt it leaves for the push', () => { const { captured } = await check(world); const { receipt } = HostCommitCheckOutputSchema.parse(captured.output); expect(receipt).toBeDefined(); - const pushed = { bindingId: 'hb_app', sha: world.sha, base: world.base, receipt, ...sameBase }; + const pushed = { + bindingId: 'hb_app', + branch: 'aflow/x', + sha: world.sha, + base: world.base, + receipt, + ...sameBase, + }; await expect(requireCheckedPush({ ...pushed, argv: REPORTING, posture: 'open' })).resolves.toBe( 'open', ); @@ -445,6 +452,7 @@ describe('host.commit.check — the receipt it leaves for the push', () => { await expect( requireCheckedPush({ bindingId: 'hb_app', + branch: 'aflow/x', sha: world.sha, base: world.base, argv: FAILING, @@ -464,7 +472,13 @@ describe('host.commit.check — the receipt it leaves for the push', () => { expect(handed.map((input) => input.posture)).toEqual([sandbox]); expect(output.sandbox).toBe(sandbox); expect(JSON.parse(receiptBody(output.receipt ?? '')) as unknown[]).toContain(sandbox); - const pushed = { bindingId: 'hb_app', sha: world.sha, base: world.base, ...sameBase }; + const pushed = { + bindingId: 'hb_app', + branch: 'aflow/x', + sha: world.sha, + base: world.base, + ...sameBase, + }; await expect( requireCheckedPush({ ...pushed, @@ -490,6 +504,7 @@ describe('host.commit.check — the receipt it leaves for the push', () => { const { receipt } = HostCommitCheckOutputSchema.parse(captured.output); const push = { bindingId: 'hb_app', + branch: 'aflow/x', sha: world.sha, base: world.base, argv: REPORTING, @@ -542,6 +557,7 @@ describe('host.commit.check — the receipt it leaves for the push', () => { await expect( requireCheckedPush({ bindingId: 'hb_app', + branch: 'aflow/x', sha: world.sha, base: world.base, argv: reportingSkipped(10), diff --git a/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts new file mode 100644 index 00000000..e35c8d64 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts @@ -0,0 +1,134 @@ +/** + * Contract: a commission naming a session it cannot continue is told the rule + * that decides which sessions it can — a kept session is owned by the run that + * made it, and every commission is a new run — and nothing about the session it + * named. Another run's session gets the answer a session that never existed + * gets, so the refusal confirms nothing, and it guesses at no expiry or restart + * that did not happen. + */ +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('../sandboxedRun.js', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + sandboxReadiness: () => ({ ready: true, missing: [] }), + runSandboxed: () => Promise.reject(new Error('a refused commission runs nothing')), + }; +}); + +const { createHostHarnessHandler } = await import('../handlers/harnessHandlers.js'); +const { forgetSession, recordSession } = await import('../harnessSessions.js'); + +const OTHER_RUNS = 'hs_other_run'; + +let base: string; +let policyPath: string; +beforeEach(async () => { + base = await mkdtemp(join(tmpdir(), 'aflow-harness-continue-')); + policyPath = join(base, 'host-policy.json'); + await writeFile( + policyPath, + JSON.stringify({ + version: 1, + bindings: [ + { + id: 'hb', + root: base, + mode: 'readwrite', + allowsExecution: true, + singleFile: false, + spaceId: 'space-test', + }, + ], + harnesses: [ + { + id: 'converses', + executable: '/bin/sh', + args: ['-c', 'true'], + sessionArgs: ['--session', '{session}'], + resumeArgs: ['--resume', '{session}'], + }, + ], + }), + ); + // Made by another commission, on the same folder and harness, and live. + recordSession({ + id: OTHER_RUNS, + ownerRunId: 'run-earlier-commission', + logicalExecutionId: 'task:commission', + bindingId: 'hb', + harnessId: 'converses', + worktreePath: join(base, 'worktree'), + scratchDir: join(base, 'scratch'), + configDir: join(base, 'scratch', 'config'), + conversationId: 'conversation', + baseSha: 'a'.repeat(40), + createdAt: Date.now(), + lastUsedAt: Date.now(), + busy: false, + }); +}); + +afterEach(async () => { + forgetSession(OTHER_RUNS); + await rm(base, { recursive: true, force: true }); +}); + +async function commission(continueFrom: string): Promise<{ status: string; message: string }> { + const written: Record = {}; + const result = await createHostHarnessHandler(policyPath).execute({ + operationId: 'host.harness.run', + spaceId: 'space-test', + runId: 'run-this-commission', + job: { inputRef: 'inline:x' }, + signal: new AbortController().signal, + log: { error: () => undefined, warn: () => undefined, info: () => undefined }, + readPayload: () => + Promise.resolve({ + bindingId: 'hb', + harness: 'converses', + task: 'Fix what the review found.', + timeoutMs: 60_000, + continueFrom, + }), + emitLiveDelta: () => Promise.resolve(), + writePayload: (kind: string, data: unknown) => { + written[kind] = data; + return Promise.resolve(`inline:${kind}`); + }, + } as never); + const error = written['error'] as { message?: string } | undefined; + return { status: result.status, message: error?.message ?? '' }; +} + +describe('a commission that names a session another run made', () => { + it('is told the ownership rule', async () => { + const { status, message } = await commission(OTHER_RUNS); + expect(status).toBe('FAILED'); + expect(message).toContain('A kept session is owned by the run that made it'); + expect(message).toContain('every commission is a new run'); + expect(message).toContain('`continueFrom` can name only a session this same run made'); + expect(message).toContain('a session made by any other run is refused'); + }); + + it('is offered neither expiry nor a restart as the reason', async () => { + const { message } = await commission(OTHER_RUNS); + expect(message).not.toMatch(/expire/i); + expect(message).not.toMatch(/restart/i); + }); + + it('gets the answer a session that never existed gets, so nothing is confirmed', async () => { + const owned = await commission(OTHER_RUNS); + const never = await commission('hs_never_made'); + expect(never.status).toBe('FAILED'); + expect(owned.message.replace(OTHER_RUNS, '')).toBe( + never.message.replace('hs_never_made', ''), + ); + expect(owned.message).not.toContain('run-earlier-commission'); + }); +}); diff --git a/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts new file mode 100644 index 00000000..9ed33bc2 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts @@ -0,0 +1,227 @@ +/** + * Contract: every refusal the push gate gives names a remedy its reader has. + * The agent that reads a refused push is refused promotion of `host.commit.scan` + * and `host.commit.check` — the publication holds them — so a refusal that + * asks for a receipt again sends its reader to the publication, which scans and + * checks what it sends, and names neither operation. + * + * Every refusal code of both receipts is reached here, so a refusal added + * later is held to the same wording or fails the census. + */ +import { describe, expect, it } from 'vitest'; + +import type { HostSandboxPosture } from '@aflow/schemas'; + +import { type CheckOutcome, type CheckReceiptRefusal, issueCheckReceipt } from '../checkReceipt.js'; +import { RECEIPT_TTL_MS } from '../receiptSigning.js'; +import { + issueScanReceipt, + type PushHistory, + type PushUnderScan, + requireScannedPush, + type ScanOutcome, + type ScanReceiptRefusal, +} from '../scanReceipt.js'; + +const BASE = 'a'.repeat(40); +const TIP = 'b'.repeat(40); +const MOVED = 'c'.repeat(40); +const CHECKS = ['npm', 'test']; +const BRANCH = 'aflow/the-change'; +const REFSPEC = `${TIP}:refs/heads/${BRANCH}`; + +function scanReceipt(fields: { bindingId?: string; outcome?: ScanOutcome } = {}): string { + return issueScanReceipt({ + bindingId: fields.bindingId ?? 'hb', + base: BASE, + sha: TIP, + outcome: fields.outcome ?? 'clean', + }); +} + +function checkReceipt( + fields: { + bindingId?: string; + sha?: string; + base?: string; + argv?: readonly string[]; + outcome?: CheckOutcome; + sandbox?: HostSandboxPosture; + now?: number; + } = {}, +): string { + return issueCheckReceipt( + { + bindingId: fields.bindingId ?? 'hb', + sha: fields.sha ?? TIP, + base: fields.base ?? BASE, + argv: fields.argv ?? CHECKS, + outcome: fields.outcome ?? 'passed', + sandbox: fields.sandbox ?? 'open', + }, + fields.now, + ); +} + +const stillAtBase: PushHistory = { + measureBase: () => Promise.resolve(BASE), + holds: () => Promise.resolve(false), + conflicts: () => Promise.resolve([]), + scanSecrets: () => Promise.reject(new Error('a push its receipt covers scans nothing')), +}; + +/** `origin/main` moved on to `MOVED`, which holds `BASE`. */ +function movedOn(history: Partial): PushHistory { + return { + measureBase: () => Promise.resolve(MOVED), + holds: () => Promise.resolve(true), + conflicts: () => Promise.resolve([]), + scanSecrets: () => Promise.reject(new Error('not scanned in this case')), + ...history, + }; +} + +function push(overrides: Partial = {}): PushUnderScan { + return { + bindingId: 'hb', + pushApproval: 'unless-unreviewed', + refspecs: [REFSPEC], + sources: [TIP], + pushBase: 'main', + receipt: scanReceipt(), + checks: { argv: CHECKS, receipt: checkReceipt(), posture: 'open' }, + history: stillAtBase, + approvalFor: () => Promise.resolve(null), + ...overrides, + }; +} + +function withCheck(check: Partial): PushUnderScan { + return push({ checks: { argv: CHECKS, receipt: checkReceipt(), posture: 'open', ...check } }); +} + +type Refusal = ScanReceiptRefusal | CheckReceiptRefusal; + +const REFUSED: ReadonlyArray = [ + ['no_base', push({ pushBase: undefined })], + ['no_receipt', push({ receipt: undefined })], + ['not_issued_here', push({ receipt: 'not-a.receipt' })], + ['other_folder', push({ receipt: scanReceipt({ bindingId: 'hb_other' }) })], + ['other_range', push({ sources: ['main'], refspecs: [`main:refs/heads/${BRANCH}`] })], + ['other_range', push({ history: movedOn({ holds: () => Promise.resolve(false) }) })], + ['base_conflicts', push({ history: movedOn({ conflicts: () => Promise.resolve(['a.ts']) }) })], + [ + 'secret_found', + push({ + history: movedOn({ + scanSecrets: () => + Promise.resolve({ + clean: false, + findings: [], + unscanned: [], + allowed: [], + summary: 'One rule matched.', + } as never), + }), + }), + ], + ['unapproved', push({ receipt: scanReceipt({ outcome: 'allowed' }) })], + ['checks_undeclared', withCheck({ argv: undefined })], + ['no_check_receipt', withCheck({ receipt: undefined })], + ['check_not_issued_here', withCheck({ receipt: scanReceipt() })], + ['check_other_folder', withCheck({ receipt: checkReceipt({ bindingId: 'hb_other' }) })], + ['check_stale', withCheck({ receipt: checkReceipt({ now: Date.now() - RECEIPT_TTL_MS - 1 }) })], + ['check_other_commit', withCheck({ receipt: checkReceipt({ sha: 'f'.repeat(40) }) })], + ['check_other_base', withCheck({ receipt: checkReceipt({ base: 'e'.repeat(40) }) })], + ['check_other_checks', withCheck({ receipt: checkReceipt({ argv: ['npm', 'run', 'x'] }) })], + ['check_other_posture', withCheck({ posture: 'confined' })], + ['check_failed', withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })], +]; + +const EVERY_REFUSAL: readonly Refusal[] = [ + 'no_base', + 'no_receipt', + 'not_issued_here', + 'other_folder', + 'other_range', + 'base_conflicts', + 'secret_found', + 'unapproved', + 'checks_undeclared', + 'no_check_receipt', + 'check_not_issued_here', + 'check_other_folder', + 'check_stale', + 'check_other_commit', + 'check_other_base', + 'check_other_checks', + 'check_other_posture', + 'check_failed', +]; + +/** The refusals whose remedy is a receipt the push does not have, and so the publication. */ +const PUBLISH_AGAIN: readonly Refusal[] = [ + 'no_receipt', + 'not_issued_here', + 'other_folder', + 'other_range', + 'no_check_receipt', + 'check_not_issued_here', + 'check_other_folder', + 'check_stale', + 'check_other_commit', + 'check_other_base', + 'check_other_checks', + 'check_other_posture', +]; + +async function refusalOf( + pushed: PushUnderScan, +): Promise<{ refusal: Refusal; message: string } | undefined> { + try { + await requireScannedPush(pushed); + return undefined; + } catch (error) { + const { refusal, message } = error as { refusal: Refusal; message: string }; + return { refusal, message }; + } +} + +describe('what the push gate tells the agent it refuses', () => { + it('reaches every refusal it has, so none escapes what follows', async () => { + const reached = new Set(); + for (const [expected, pushed] of REFUSED) { + const refused = await refusalOf(pushed); + expect(refused?.refusal, expected).toBe(expected); + if (refused !== undefined) reached.add(refused.refusal); + } + expect([...reached].sort()).toEqual([...EVERY_REFUSAL].sort()); + }); + + it('names neither `host.commit.scan` nor `host.commit.check` in any refusal', async () => { + for (const [expected, pushed] of REFUSED) { + const message = (await refusalOf(pushed))?.message ?? ''; + expect(message, expected).not.toContain('host.commit.scan'); + expect(message, expected).not.toContain('host.commit.check'); + } + }); + + it('sends a push its receipts do not cover to publish the branch again as it stands', async () => { + for (const [expected, pushed] of REFUSED) { + if (!PUBLISH_AGAIN.includes(expected)) continue; + const message = (await refusalOf(pushed))?.message ?? ''; + expect(message, expected).toContain('Nothing was pushed.'); + expect(message, expected).toContain(`Publish \`${BRANCH}\` again as it stands`); + } + }); + + it('sends a commit whose checks failed to publish again once they pass, not as it stands', async () => { + const refused = await refusalOf(withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })); + expect(refused?.message).toContain(`Publish \`${BRANCH}\` again once they pass on it`); + expect(refused?.message).not.toContain('as it stands'); + }); + + it('clears the push every one of these departs from', async () => { + expect(await refusalOf(push())).toBeUndefined(); + }); +}); diff --git a/apps/aflow-executor-host/src/checkReceipt.ts b/apps/aflow-executor-host/src/checkReceipt.ts index 6f3c0bf9..0fb1072e 100644 --- a/apps/aflow-executor-host/src/checkReceipt.ts +++ b/apps/aflow-executor-host/src/checkReceipt.ts @@ -158,6 +158,8 @@ export interface PushUnderCheck { export async function requireCheckedPush( push: PushUnderCheck & { readonly bindingId: string; + /** The branch the push publishes, as a person names it. */ + readonly branch: string; /** The source of the push's one refspec, as a full sha. */ readonly sha: string; /** Where `origin/` is now, as a full sha. */ @@ -178,9 +180,12 @@ export async function requireCheckedPush( } const command = `\`${argv.join(' ')}\``; const short = push.sha.slice(0, SHORT_SHA_LENGTH); + // The check is the publication's to run and the agent reading this is + // refused its promotion, so the remedy is the publication, which checks what + // it sends. const checkAgain = - `Nothing was pushed. Run the folder's checks on \`${push.sha}\` against \`${push.base}\` ` + - 'with `host.commit.check`, and push with the receipt it returns once they pass.'; + `Nothing was pushed. Publish \`${push.branch}\` again as it stands, so the folder's ` + + `checks run on \`${push.sha}\` against \`${push.base}\`.`; if (push.receipt === undefined) { throw new CheckReceiptError( `\`${bindingId}\` declares checks, ${command}, and this push carries no check receipt, ` + @@ -242,7 +247,8 @@ export async function requireCheckedPush( if (receipt.outcome !== 'passed') { throw new CheckReceiptError( `The checks of \`${bindingId}\`, ${command}, failed on ${short}, and a commit is pushed ` + - `only once they pass on it. ${checkAgain}`, + `only once they pass on it. Nothing was pushed. Publish \`${push.branch}\` again once ` + + 'they pass on it: the publication runs them on what it sends.', 'check_failed', ); } diff --git a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts index 539a7cb2..ace883a0 100644 --- a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts +++ b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts @@ -574,12 +574,15 @@ async function runHarness( session = ownedSession(input.continueFrom, ctx.runId); if (session === undefined) { // The same answer a session that never existed would get: naming a - // session as another run's confirms it exists. + // session as another run's confirms it exists. So it states the rule + // and offers no reason this ref in particular was not found. return await failureWithError( ctx, validationError( - `No session \`${input.continueFrom}\` belongs to this run. It may have expired, or ` + - 'this executor may have restarted since it was made.', + `No session \`${input.continueFrom}\` is held for this run. A kept session is owned ` + + 'by the run that made it, and every commission is a new run, so `continueFrom` ' + + 'can name only a session this same run made; a session made by any other run is ' + + 'refused as one that does not exist.', ), ); } diff --git a/apps/aflow-executor-host/src/scanReceipt.ts b/apps/aflow-executor-host/src/scanReceipt.ts index da5f65ee..4e974e14 100644 --- a/apps/aflow-executor-host/src/scanReceipt.ts +++ b/apps/aflow-executor-host/src/scanReceipt.ts @@ -181,11 +181,15 @@ interface ScannedRange { readonly rescanned: boolean; } -function scanAgain(pushBase: string): string { +/** + * The remedy for a push its receipt does not cover. The scan is the + * publication's to run and the agent reading this is refused its promotion, so + * the remedy is the publication, which scans what it sends. + */ +function scanAgain(pushBase: string, refspecs: readonly string[]): string { return ( - `Nothing was pushed. Scan the range the push sends — from where \`origin/${pushBase}\` ` + - 'is now to the commit it pushes — with `host.commit.scan`, and push with the receipt it ' + - 'returns.' + `Nothing was pushed. Publish ${publishedBranch(refspecs)} again as it stands, so what it ` + + `sends is scanned from where \`origin/${pushBase}\` is now.` ); } @@ -194,6 +198,14 @@ function destinationBranch(refspec: string): string { return refspec.slice(refspec.indexOf(':') + 1).replace(/^refs\/heads\//, ''); } +/** The branch a push publishes, named where it sends one. */ +function publishedBranch(refspecs: readonly string[]): string { + const [refspec, ...more] = refspecs; + return refspec !== undefined && more.length === 0 + ? `\`${destinationBranch(refspec)}\`` + : 'the branch'; +} + /** * The range the push sends, from where `origin/` is now, and how it * was cleared of secrets. The receipt clears it where this process signed it @@ -292,7 +304,7 @@ export async function requireScannedPush( if (receipt === undefined) { throw new ScanReceiptError( 'This push carries no scan receipt, so nothing says what it sends was read for secrets. ' + - scanAgain(pushBase), + scanAgain(pushBase, push.refspecs), 'no_receipt', ); } @@ -301,7 +313,7 @@ export async function requireScannedPush( if (claimed === undefined) { throw new ScanReceiptError( 'This push carries a scan receipt that names no folder and range, so nothing says it ' + - `sends what a scan read. ${scanAgain(pushBase)}`, + `sends what a scan read. ${scanAgain(pushBase, push.refspecs)}`, 'not_issued_here', ); } @@ -309,7 +321,7 @@ export async function requireScannedPush( if (claimed.bindingId !== bindingId) { throw new ScanReceiptError( `This push carries the receipt for a scan of \`${claimed.bindingId}\`, and pushes from ` + - `\`${bindingId}\`. ${scanAgain(pushBase)}`, + `\`${bindingId}\`. ${scanAgain(pushBase, push.refspecs)}`, 'other_folder', ); } @@ -319,7 +331,7 @@ export async function requireScannedPush( throw new ScanReceiptError( `This push sends ${push.sources.map((s) => `\`${s}\``).join(', ')}, and its receipt is ` + `for a scan of ${scanned}: a push sends the one commit its scan ended at, named by ` + - `that commit's sha as the source of its one refspec. ${scanAgain(pushBase)}`, + `that commit's sha as the source of its one refspec. ${scanAgain(pushBase, push.refspecs)}`, 'other_range', ); } @@ -333,6 +345,7 @@ export async function requireScannedPush( const checkedUnder = await requireCheckedPush( { bindingId, + branch: destinationBranch(refspec), sha: claimed.sha, base: sent.base, baseHolds: (base) => push.history.holds(sent.base, base), From f57450ac75c8de262cc09dd0ccb66e115506e694 Mon Sep 17 00:00:00 2001 From: karim Date: Fri, 16 Oct 2026 22:58:58 +0200 Subject: [PATCH 2/4] fix(executor-host): refusals name the rule, not a guess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The continueFrom refusal states the ownership rule — a kept session is owned by the run that made it, every commission is a new run — and the two ways a session this run made stops being held: idle past the hour, or an executor restart. It confirms nothing about the particular ref. The gate's re-scan and re-check refusals name the remedy that exists — publish the branch again as it stands — and the check_failed refusal names commissioning the fix onto the branch and publishing again; none names host.commit.scan or host.commit.check, operations the Helmsman is refused promotion of. The refusal census in pushGateRefusals.test.ts is derived from a Record over the refusal unions so a member left out fails the type-check. Structured refusal codes unchanged. --- .../src/__tests__/fixtures/pushRefusals.ts | 33 +++++++++ .../src/__tests__/harnessContinueFrom.test.ts | 74 +++++++++++++------ .../src/__tests__/pushGateRefusals.test.ts | 40 +++------- apps/aflow-executor-host/src/checkReceipt.ts | 5 +- .../src/handlers/harnessHandlers.ts | 9 ++- 5 files changed, 103 insertions(+), 58 deletions(-) create mode 100644 apps/aflow-executor-host/src/__tests__/fixtures/pushRefusals.ts diff --git a/apps/aflow-executor-host/src/__tests__/fixtures/pushRefusals.ts b/apps/aflow-executor-host/src/__tests__/fixtures/pushRefusals.ts new file mode 100644 index 00000000..819268b6 --- /dev/null +++ b/apps/aflow-executor-host/src/__tests__/fixtures/pushRefusals.ts @@ -0,0 +1,33 @@ +/** + * Every code the push gate refuses with. This file, unlike the tests that read + * it, is compiled by `tsc`, and the census is a Record over both refusal + * unions — so a code added to either receipt and left out here fails the + * type-check. + */ +import type { CheckReceiptRefusal } from '../../checkReceipt.js'; +import type { ScanReceiptRefusal } from '../../scanReceipt.js'; + +export type Refusal = ScanReceiptRefusal | CheckReceiptRefusal; + +const CENSUS: Record = { + no_base: true, + no_receipt: true, + not_issued_here: true, + other_folder: true, + other_range: true, + base_conflicts: true, + secret_found: true, + unapproved: true, + checks_undeclared: true, + no_check_receipt: true, + check_not_issued_here: true, + check_other_folder: true, + check_stale: true, + check_other_commit: true, + check_other_base: true, + check_other_checks: true, + check_other_posture: true, + check_failed: true, +}; + +export const EVERY_REFUSAL = Object.keys(CENSUS) as readonly Refusal[]; diff --git a/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts index e35c8d64..870f39fb 100644 --- a/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts +++ b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts @@ -2,9 +2,11 @@ * Contract: a commission naming a session it cannot continue is told the rule * that decides which sessions it can — a kept session is owned by the run that * made it, and every commission is a new run — and nothing about the session it - * named. Another run's session gets the answer a session that never existed - * gets, so the refusal confirms nothing, and it guesses at no expiry or restart - * that did not happen. + * named. That rule is complete: a session this run made is no longer held once + * it has been idle for an hour or the executor has restarted since, and such a + * session reaches the same refusal as another run's. Another run's session, + * this run's idled-out one and one that never existed all get one answer, so + * the refusal confirms nothing and guesses at no reason for the ref it names. */ import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -22,9 +24,11 @@ vi.mock('../sandboxedRun.js', async (importOriginal) => { }); const { createHostHarnessHandler } = await import('../handlers/harnessHandlers.js'); -const { forgetSession, recordSession } = await import('../harnessSessions.js'); +const { forgetSession, recordSession, SESSION_IDLE_TTL_MS } = await import('../harnessSessions.js'); +const THIS_RUN = 'run-this-commission'; const OTHER_RUNS = 'hs_other_run'; +const IDLED_OUT = 'hs_this_run_idled_out'; let base: string; let policyPath: string; @@ -57,25 +61,32 @@ beforeEach(async () => { }), ); // Made by another commission, on the same folder and harness, and live. + keep(OTHER_RUNS, 'run-earlier-commission', Date.now()); + // Made by this commission, and idle past the hour since. + keep(IDLED_OUT, THIS_RUN, Date.now() - SESSION_IDLE_TTL_MS - 1); +}); + +function keep(id: string, ownerRunId: string, lastUsedAt: number): void { recordSession({ - id: OTHER_RUNS, - ownerRunId: 'run-earlier-commission', + id, + ownerRunId, logicalExecutionId: 'task:commission', bindingId: 'hb', harnessId: 'converses', - worktreePath: join(base, 'worktree'), - scratchDir: join(base, 'scratch'), - configDir: join(base, 'scratch', 'config'), + worktreePath: join(base, `worktree-${id}`), + scratchDir: join(base, `scratch-${id}`), + configDir: join(base, `scratch-${id}`, 'config'), conversationId: 'conversation', baseSha: 'a'.repeat(40), - createdAt: Date.now(), - lastUsedAt: Date.now(), + createdAt: lastUsedAt, + lastUsedAt, busy: false, }); -}); +} afterEach(async () => { forgetSession(OTHER_RUNS); + forgetSession(IDLED_OUT); await rm(base, { recursive: true, force: true }); }); @@ -84,7 +95,7 @@ async function commission(continueFrom: string): Promise<{ status: string; messa const result = await createHostHarnessHandler(policyPath).execute({ operationId: 'host.harness.run', spaceId: 'space-test', - runId: 'run-this-commission', + runId: THIS_RUN, job: { inputRef: 'inline:x' }, signal: new AbortController().signal, log: { error: () => undefined, warn: () => undefined, info: () => undefined }, @@ -106,7 +117,7 @@ async function commission(continueFrom: string): Promise<{ status: string; messa return { status: result.status, message: error?.message ?? '' }; } -describe('a commission that names a session another run made', () => { +describe('a commission that names a session it cannot continue', () => { it('is told the ownership rule', async () => { const { status, message } = await commission(OTHER_RUNS); expect(status).toBe('FAILED'); @@ -116,19 +127,34 @@ describe('a commission that names a session another run made', () => { expect(message).toContain('a session made by any other run is refused'); }); - it('is offered neither expiry nor a restart as the reason', async () => { + it("is told that this run's own session is no longer held after an idle hour", async () => { const { message } = await commission(OTHER_RUNS); - expect(message).not.toMatch(/expire/i); - expect(message).not.toMatch(/restart/i); + expect(SESSION_IDLE_TTL_MS).toBe(60 * 60 * 1000); + expect(message).toContain('A session this run made is no longer held'); + expect(message).toContain('once it has been idle for an hour'); }); - it('gets the answer a session that never existed gets, so nothing is confirmed', async () => { - const owned = await commission(OTHER_RUNS); - const never = await commission('hs_never_made'); - expect(never.status).toBe('FAILED'); - expect(owned.message.replace(OTHER_RUNS, '')).toBe( - never.message.replace('hs_never_made', ''), + it("is told that a restart of the executor since lets go of this run's own session", async () => { + const { message } = await commission(OTHER_RUNS); + expect(message).toContain('once this executor has restarted since it was made'); + }); + + it('makes no guess about why the ref it named in particular is not held', async () => { + const { message } = await commission(OTHER_RUNS); + expect(message).not.toMatch(/\b(may|might|probably|perhaps|likely)\b/i); + expect(message.startsWith(`No session \`${OTHER_RUNS}\` is held for this run. `)).toBe(true); + expect(message.split(OTHER_RUNS)).toHaveLength(2); + expect(message).not.toContain('run-earlier-commission'); + }); + + it("gets one answer for another run's session, this run's idled-out one and one never made", async () => { + const answers = await Promise.all( + [OTHER_RUNS, IDLED_OUT, 'hs_never_made'].map(async (ref) => { + const { status, message } = await commission(ref); + expect(status, ref).toBe('FAILED'); + return message.replace(ref, ''); + }), ); - expect(owned.message).not.toContain('run-earlier-commission'); + expect(new Set(answers).size).toBe(1); }); }); diff --git a/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts index 9ed33bc2..ed29e9e3 100644 --- a/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts +++ b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts @@ -5,14 +5,15 @@ * asks for a receipt again sends its reader to the publication, which scans and * checks what it sends, and names neither operation. * - * Every refusal code of both receipts is reached here, so a refusal added - * later is held to the same wording or fails the census. + * Every refusal code of both receipts is reached here, and the census it is + * measured against is a Record over both unions (`fixtures/pushRefusals.ts`), + * so a refusal added later is held to the same wording or fails the type-check. */ import { describe, expect, it } from 'vitest'; import type { HostSandboxPosture } from '@aflow/schemas'; -import { type CheckOutcome, type CheckReceiptRefusal, issueCheckReceipt } from '../checkReceipt.js'; +import { type CheckOutcome, issueCheckReceipt } from '../checkReceipt.js'; import { RECEIPT_TTL_MS } from '../receiptSigning.js'; import { issueScanReceipt, @@ -20,9 +21,10 @@ import { type PushUnderScan, requireScannedPush, type ScanOutcome, - type ScanReceiptRefusal, } from '../scanReceipt.js'; +import { EVERY_REFUSAL, type Refusal } from './fixtures/pushRefusals.js'; + const BASE = 'a'.repeat(40); const TIP = 'b'.repeat(40); const MOVED = 'c'.repeat(40); @@ -100,8 +102,6 @@ function withCheck(check: Partial): PushUnderScan { return push({ checks: { argv: CHECKS, receipt: checkReceipt(), posture: 'open', ...check } }); } -type Refusal = ScanReceiptRefusal | CheckReceiptRefusal; - const REFUSED: ReadonlyArray = [ ['no_base', push({ pushBase: undefined })], ['no_receipt', push({ receipt: undefined })], @@ -138,27 +138,6 @@ const REFUSED: ReadonlyArray = [ ['check_failed', withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })], ]; -const EVERY_REFUSAL: readonly Refusal[] = [ - 'no_base', - 'no_receipt', - 'not_issued_here', - 'other_folder', - 'other_range', - 'base_conflicts', - 'secret_found', - 'unapproved', - 'checks_undeclared', - 'no_check_receipt', - 'check_not_issued_here', - 'check_other_folder', - 'check_stale', - 'check_other_commit', - 'check_other_base', - 'check_other_checks', - 'check_other_posture', - 'check_failed', -]; - /** The refusals whose remedy is a receipt the push does not have, and so the publication. */ const PUBLISH_AGAIN: readonly Refusal[] = [ 'no_receipt', @@ -215,9 +194,12 @@ describe('what the push gate tells the agent it refuses', () => { } }); - it('sends a commit whose checks failed to publish again once they pass, not as it stands', async () => { + it('sends a commit whose checks failed to commission the fix and publish again', async () => { const refused = await refusalOf(withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })); - expect(refused?.message).toContain(`Publish \`${BRANCH}\` again once they pass on it`); + expect(refused?.refusal).toBe('check_failed'); + expect(refused?.message).toContain('Nothing was pushed'); + expect(refused?.message).toContain(`Commission the fix onto \`${BRANCH}\` and publish again`); + expect(refused?.message).not.toContain('again once they pass'); expect(refused?.message).not.toContain('as it stands'); }); diff --git a/apps/aflow-executor-host/src/checkReceipt.ts b/apps/aflow-executor-host/src/checkReceipt.ts index 0fb1072e..4c6af1b7 100644 --- a/apps/aflow-executor-host/src/checkReceipt.ts +++ b/apps/aflow-executor-host/src/checkReceipt.ts @@ -247,8 +247,9 @@ export async function requireCheckedPush( if (receipt.outcome !== 'passed') { throw new CheckReceiptError( `The checks of \`${bindingId}\`, ${command}, failed on ${short}, and a commit is pushed ` + - `only once they pass on it. Nothing was pushed. Publish \`${push.branch}\` again once ` + - 'they pass on it: the publication runs them on what it sends.', + `only once they pass on it. Nothing was pushed, and that commit will not start passing. ` + + `Commission the fix onto \`${push.branch}\` and publish again: the publication runs ` + + 'them on what it sends.', 'check_failed', ); } diff --git a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts index ace883a0..92973503 100644 --- a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts +++ b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts @@ -574,15 +574,18 @@ async function runHarness( session = ownedSession(input.continueFrom, ctx.runId); if (session === undefined) { // The same answer a session that never existed would get: naming a - // session as another run's confirms it exists. So it states the rule - // and offers no reason this ref in particular was not found. + // session as another run's confirms it exists. This run's own session + // that idled out or was lost to a restart reaches this branch too, so + // the rule states every case and guesses at none for this ref. return await failureWithError( ctx, validationError( `No session \`${input.continueFrom}\` is held for this run. A kept session is owned ` + 'by the run that made it, and every commission is a new run, so `continueFrom` ' + 'can name only a session this same run made; a session made by any other run is ' + - 'refused as one that does not exist.', + 'refused as one that does not exist. A session this run made is no longer held ' + + 'once it has been idle for an hour, or once this executor has restarted since it ' + + 'was made.', ), ); } From 441dc09bb1e94ad0f54a501c9a42788f02c420fc Mon Sep 17 00:00:00 2001 From: karim Date: Fri, 16 Oct 2026 23:06:48 +0200 Subject: [PATCH 3/4] fix(executor-host): every gate refusal carries "Nothing was pushed." whole MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check_failed refusal keeps the invariant sentence as its own and says the rest after it: the commit the checks failed on will not start passing — commission the fix onto the branch and publish again, the publication runs the checks on what it sends. --- apps/aflow-executor-host/src/checkReceipt.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/aflow-executor-host/src/checkReceipt.ts b/apps/aflow-executor-host/src/checkReceipt.ts index 4c6af1b7..8185c2b8 100644 --- a/apps/aflow-executor-host/src/checkReceipt.ts +++ b/apps/aflow-executor-host/src/checkReceipt.ts @@ -247,9 +247,9 @@ export async function requireCheckedPush( if (receipt.outcome !== 'passed') { throw new CheckReceiptError( `The checks of \`${bindingId}\`, ${command}, failed on ${short}, and a commit is pushed ` + - `only once they pass on it. Nothing was pushed, and that commit will not start passing. ` + - `Commission the fix onto \`${push.branch}\` and publish again: the publication runs ` + - 'them on what it sends.', + `only once they pass on it. Nothing was pushed. That commit will not start passing. ` + + `Commission the fix onto \`${push.branch}\` and publish again — the publication runs ` + + 'the checks on what it sends.', 'check_failed', ); } From 52cf0c8d3facd218fda005c4408fe205dedb0b96 Mon Sep 17 00:00:00 2001 From: karim Date: Fri, 16 Oct 2026 23:19:16 +0200 Subject: [PATCH 4/4] Name the withdrawal of the folder's grant among the ways a kept session ends, drop the check_failed prediction, and assert the whole sentence in the census MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix round 2 on the refusal wordings (PR #122). (a) The continueFrom refusal names the withdrawal of the folder's grant (index.ts ~446/~653, harnessHandlers.ts ~518/~1203) and the failed checkout re-preparation (~681) alongside the idle hour and the restart, and words the list as the ways a kept session ends; the test header says the same. (b) The check_failed refusal drops "That commit will not start passing" — the gate cannot know it; the rule stays: a commit is pushed only once its checks pass on it; commission the fix onto the branch and publish again. (c) The census asserts the whole sentence "Nothing was pushed." for every refusal it reaches, in one place. --- .../src/__tests__/harnessContinueFrom.test.ts | 24 ++++++++++++------- .../src/__tests__/pushGateRefusals.test.ts | 16 +++++++++++-- apps/aflow-executor-host/src/checkReceipt.ts | 2 +- .../src/handlers/harnessHandlers.ts | 11 +++++---- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts index 870f39fb..10816d93 100644 --- a/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts +++ b/apps/aflow-executor-host/src/__tests__/harnessContinueFrom.test.ts @@ -2,11 +2,13 @@ * Contract: a commission naming a session it cannot continue is told the rule * that decides which sessions it can — a kept session is owned by the run that * made it, and every commission is a new run — and nothing about the session it - * named. That rule is complete: a session this run made is no longer held once - * it has been idle for an hour or the executor has restarted since, and such a - * session reaches the same refusal as another run's. Another run's session, - * this run's idled-out one and one that never existed all get one answer, so - * the refusal confirms nothing and guesses at no reason for the ref it names. + * named. It also says the ways a kept session ends — once it has been idle for + * an hour, once the executor has restarted since it was made, or once the grant + * of its connected folder is withdrawn — because a session this run made and + * lost one of those ways reaches the same refusal as another run's. Another + * run's session, this run's idled-out one and one that never existed all get + * one answer, so the refusal confirms nothing and guesses at no reason for the + * ref it names. */ import { mkdtemp, rm, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; @@ -127,18 +129,22 @@ describe('a commission that names a session it cannot continue', () => { expect(message).toContain('a session made by any other run is refused'); }); - it("is told that this run's own session is no longer held after an idle hour", async () => { + it('is told that a kept session ends after an idle hour', async () => { const { message } = await commission(OTHER_RUNS); expect(SESSION_IDLE_TTL_MS).toBe(60 * 60 * 1000); - expect(message).toContain('A session this run made is no longer held'); - expect(message).toContain('once it has been idle for an hour'); + expect(message).toContain('A kept session ends once it has been idle for an hour'); }); - it("is told that a restart of the executor since lets go of this run's own session", async () => { + it('is told that a kept session ends when the executor restarts', async () => { const { message } = await commission(OTHER_RUNS); expect(message).toContain('once this executor has restarted since it was made'); }); + it("is told that a kept session ends when its folder's grant is withdrawn", async () => { + const { message } = await commission(OTHER_RUNS); + expect(message).toContain('or once the grant of its connected folder is withdrawn.'); + }); + it('makes no guess about why the ref it named in particular is not held', async () => { const { message } = await commission(OTHER_RUNS); expect(message).not.toMatch(/\b(may|might|probably|perhaps|likely)\b/i); diff --git a/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts index ed29e9e3..1b4987a4 100644 --- a/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts +++ b/apps/aflow-executor-host/src/__tests__/pushGateRefusals.test.ts @@ -185,11 +185,17 @@ describe('what the push gate tells the agent it refuses', () => { } }); + it('says "Nothing was pushed." as a whole sentence in every refusal', async () => { + for (const [expected, pushed] of REFUSED) { + const message = (await refusalOf(pushed))?.message ?? ''; + expect(message, expected).toMatch(/(^|[.:] )Nothing was pushed\.( |$)/); + } + }); + it('sends a push its receipts do not cover to publish the branch again as it stands', async () => { for (const [expected, pushed] of REFUSED) { if (!PUBLISH_AGAIN.includes(expected)) continue; const message = (await refusalOf(pushed))?.message ?? ''; - expect(message, expected).toContain('Nothing was pushed.'); expect(message, expected).toContain(`Publish \`${BRANCH}\` again as it stands`); } }); @@ -197,12 +203,18 @@ describe('what the push gate tells the agent it refuses', () => { it('sends a commit whose checks failed to commission the fix and publish again', async () => { const refused = await refusalOf(withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })); expect(refused?.refusal).toBe('check_failed'); - expect(refused?.message).toContain('Nothing was pushed'); + expect(refused?.message).toContain('a commit is pushed only once they pass on it.'); expect(refused?.message).toContain(`Commission the fix onto \`${BRANCH}\` and publish again`); expect(refused?.message).not.toContain('again once they pass'); expect(refused?.message).not.toContain('as it stands'); }); + it('predicts nothing about whether a commit whose checks failed will pass', async () => { + const refused = await refusalOf(withCheck({ receipt: checkReceipt({ outcome: 'failed' }) })); + expect(refused?.message).not.toMatch(/\bwill\b/); + expect(refused?.message).not.toContain('start passing'); + }); + it('clears the push every one of these departs from', async () => { expect(await refusalOf(push())).toBeUndefined(); }); diff --git a/apps/aflow-executor-host/src/checkReceipt.ts b/apps/aflow-executor-host/src/checkReceipt.ts index 8185c2b8..81eec8a2 100644 --- a/apps/aflow-executor-host/src/checkReceipt.ts +++ b/apps/aflow-executor-host/src/checkReceipt.ts @@ -247,7 +247,7 @@ export async function requireCheckedPush( if (receipt.outcome !== 'passed') { throw new CheckReceiptError( `The checks of \`${bindingId}\`, ${command}, failed on ${short}, and a commit is pushed ` + - `only once they pass on it. Nothing was pushed. That commit will not start passing. ` + + `only once they pass on it. Nothing was pushed. ` + `Commission the fix onto \`${push.branch}\` and publish again — the publication runs ` + 'the checks on what it sends.', 'check_failed', diff --git a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts index 92973503..8337193d 100644 --- a/apps/aflow-executor-host/src/handlers/harnessHandlers.ts +++ b/apps/aflow-executor-host/src/handlers/harnessHandlers.ts @@ -575,17 +575,18 @@ async function runHarness( if (session === undefined) { // The same answer a session that never existed would get: naming a // session as another run's confirms it exists. This run's own session - // that idled out or was lost to a restart reaches this branch too, so - // the rule states every case and guesses at none for this ref. + // that idled out, was lost to a restart or went with its folder's grant + // reaches this branch too, so the rule states how a kept session ends + // and guesses at none of those for this ref. return await failureWithError( ctx, validationError( `No session \`${input.continueFrom}\` is held for this run. A kept session is owned ` + 'by the run that made it, and every commission is a new run, so `continueFrom` ' + 'can name only a session this same run made; a session made by any other run is ' + - 'refused as one that does not exist. A session this run made is no longer held ' + - 'once it has been idle for an hour, or once this executor has restarted since it ' + - 'was made.', + 'refused as one that does not exist. A kept session ends once it has been idle ' + + 'for an hour, once this executor has restarted since it was made, or once the ' + + 'grant of its connected folder is withdrawn.', ), ); }