diff --git a/docs/BABYSITTER-CATALOG-HANDOFF.md b/docs/BABYSITTER-CATALOG-HANDOFF.md index ad1381c8..599920f2 100644 --- a/docs/BABYSITTER-CATALOG-HANDOFF.md +++ b/docs/BABYSITTER-CATALOG-HANDOFF.md @@ -28,8 +28,13 @@ not wired to hosted dispatch and must not be treated as enablement. The package' `compat` requires the published 2.0.26 Surface/SDK release that routes `labeled`, `unlabeled`, and `ready_for_review`. Export it only from the reviewed release commit pinned below. The Software Factory flow's own independently -versioned header remains `2.0.22`; a regression requires the hosted identity to -equal the identity obtained from that exact reviewed source. +versioned header is now `2.0.23`; the hosted loader pins that revision's source +bytes and its assigned identity, and a regression requires the hosted identity to +equal the identity obtained from that exact reviewed source. Both pins are bytes +of `examples/software-factory/software-factory.flow.ts`: any change to that file +is also a change to `SOFTWARE_FACTORY_SHA256` and the assigned version in +`packages/sdk/src/hosted-extension-runtime.ts`, and the base is rejected until +they are updated together. The sandbox contract is deliberately narrower than #442. It re-verifies the complete lock-backed installation and every manifest, binds it to the exact diff --git a/examples/software-factory/README.md b/examples/software-factory/README.md index 63b7012a..d78e5825 100644 --- a/examples/software-factory/README.md +++ b/examples/software-factory/README.md @@ -28,7 +28,28 @@ project and labels are filled in there. Each matching ticket launches one Cloud run in a fresh `relayflow/software-factory-` branch of `--repo`; a passing review opens a -PR, a blocked one opens a draft PR carrying the findings and ends `step_failed`. +PR. Review artifacts under `.relayflow/` must contain exactly one verdict: + +- `review.passed` opens a ready PR after the hooks allow it. +- `review.blocked` opens a draft with findings and ends `step_failed` (exit 1). +- Non-empty `review.unverified` names the missing verification prerequisite. + It opens a draft headed **NOT VERIFIED**, explicitly claims no defect, and + ends `needs_human` (exit 3). Unlike pre-publication parking, this outcome + leaves a pushed branch and an opened draft PR; completion detail says so. + +Silence, contradictory verdicts, and an empty `review.unverified` stay BLOCKED. +Every draft verdict (including hook refusals) names the reviewed commit and says +it covers that commit only. A new head supersedes the verdict, but this terminating +flow does not edit old bodies/comments, re-review pushes, or mark drafts ready. + +Draft bodies introduce a new machine-readable contract: +``. +The other verdict values are `unverified`, `post-review-blocked`, and +`merge-gate-blocked`. Exactly one marker is allowed before publication. +The intended first consumer is [the resident babysitter](../babysitter/README.md), +which is not yet ready for unattended deployment; a future shepherd can compare +the marker with the current PR head before amending a superseded verdict. + The pull-request title is the ticket title (whitespace-normalized and capped at 240 Unicode code points). GitHub inputs must carry `identifier: "#"`; the flow appends exactly one `Fixes #` line and validates the final diff --git a/examples/software-factory/software-factory.flow.ts b/examples/software-factory/software-factory.flow.ts index 307a4e2b..020bc8a0 100644 --- a/examples/software-factory/software-factory.flow.ts +++ b/examples/software-factory/software-factory.flow.ts @@ -28,12 +28,14 @@ const WORK = ".relayflow"; const PREPARE_CHANGE_METADATA = [ `if [ ! -s ${WORK}/pr-body.md ]; then echo missing-body; exit 0; fi`, `if [ -n "$reference" ] && ! grep -qxF "$reference" ${WORK}/pr-body.md; then printf "\\n%s\\n" "$reference" >> ${WORK}/pr-body.md; fi`, + `if [ -n "$scope" ] && ! grep -qxF "$scope" ${WORK}/pr-body.md; then printf "\\n%s\\n" "$scope" >> ${WORK}/pr-body.md; fi`, "echo prepared", ].join("; "); // Redundant with the TypeScript checks on purpose: this runs immediately // before the first external effect and validates the final title/body bytes. const VALIDATE_CHANGE_METADATA = [ + `if [ -n "$scope" ]; then scope_count=$(grep -cE '^` : ""; + await f.run(body(reviewedHead)); + await f.run(`reference=${shellWord(changeReference)}; scope=${shellWord(scope)}; ${PREPARE_CHANGE_METADATA}`); const metadata = (await f.run( - `title=${shellWord(title)}; title_length=${titleLength}; source=${shellWord(issueSource)}; identifier=${shellWord(issueIdentifier)}; ${VALIDATE_CHANGE_METADATA}`, + `title=${shellWord(title)}; scope=${shellWord(scope)}; title_length=${titleLength}; source=${shellWord(issueSource)}; identifier=${shellWord(issueIdentifier)}; ${VALIDATE_CHANGE_METADATA}`, )).trim(); if (metadata !== "valid") { + publicationFailure = `invalid pull-request metadata (${metadata}); nothing pushed`; await f.run(`echo ${shellWord(`Stopped: invalid pull-request metadata (${metadata}). No branch was pushed and no pull request was opened.`)} >&2`); return false; } @@ -119,12 +131,18 @@ export default flow("software-factory", { return true; }; + const draftBody = (heading: string, review = false, unverified = false) => (sha: string): string => + `sha=${shellWord(sha)}; { cat ${WORK}/summary.md; printf '\\n\\n## %s at %s\\n\\n' ${shellWord(heading)} "$sha"; ` + + (unverified ? `printf '%s\\n' 'No defect claimed. Verification could not run:'; cat ${WORK}/review.unverified; printf '\\n'; ` : "") + + `printf '%s\\n\\n' 'This verdict covers this commit only; a new head supersedes it and requires a new review.'; ` + + (review ? `cat ${WORK}/review.md; ` : "") + `} > ${WORK}/pr-body.md`; + // Fresh work dir, excluded from git, no leftover verdicts. await f.run(`rm -rf ${WORK} && mkdir -p ${WORK} && { grep -qxF '${WORK}/' .git/info/exclude 2>/dev/null || echo '${WORK}/' >> .git/info/exclude; }`); if (!await f.hook("pre-implement", { title, issue })) { await f.run("echo 'Stopped: pre-implement hook refused this ticket.' >&2"); - return f.done("declined"); + return f.done("declined", { detail: "pre-implement hook refused this ticket; nothing pushed" }); } await f.agent("implementer", { @@ -141,21 +159,22 @@ export default flow("software-factory", { cli: "claude", task: `Review the diff against the base branch as an adversary: find bugs, missing tests, unsafe defaults, and scope creep. ` + `Fix what is mechanical and re-run the tests. Write ${WORK}/review.md with your findings, then write ${WORK}/review.passed ` + - `ONLY if the change is ready for a human to merge; otherwise write ${WORK}/review.blocked with the blocking findings.`, + `ONLY if the change is ready for a human to merge; write ${WORK}/review.blocked with any blocking defects. ` + + `If no defect was found but verification cannot run, write a non-empty ${WORK}/review.unverified instead: name the missing prerequisite in both ${WORK}/review.md and ${WORK}/review.unverified. Write exactly one verdict file.`, }).gate({ type: "subprocess_gate", command: `test -s ${WORK}/review.md` }); await f.run(TEST, { timeout: "15m" }); if (!await f.hook("post-review", { title })) { await f.run("git add -A && (git diff --cached --quiet || git commit -qm 'Software factory: implementation and review fixes')"); - if (!await openPullRequest(`{ cat ${WORK}/summary.md; printf '\\n\\n## post-review: blocked\\n\\n'; } > ${WORK}/pr-body.md`, true)) { - return f.done("needs_human"); + if (!await openPullRequest(draftBody("post-review: blocked"), true, "post-review-blocked")) { + return f.done("needs_human", { detail: publicationFailure }); } - return f.done("step_failed"); + return f.done("step_failed", { detail: `post-review hook blocked at ${reviewedHead}; draft PR opened` }); } - // Passed means exactly one verdict, and it is the pass marker. - const verdict = await f.run(`if [ -f ${WORK}/review.passed ] && [ ! -f ${WORK}/review.blocked ]; then echo PASSED; else echo BLOCKED; fi`); + // Exactly one verdict is required; silence, contradictions and empty unverified fail closed. + const verdict = await f.run(`count=0; for v in blocked unverified passed; do [ -f ${WORK}/review.$v ] && count=$((count+1)); done; if [ "$count" -ne 1 ]; then echo BLOCKED; elif [ -f ${WORK}/review.blocked ]; then echo BLOCKED; elif [ -s ${WORK}/review.unverified ]; then echo UNVERIFIED; elif [ -f ${WORK}/review.passed ]; then echo PASSED; else echo BLOCKED; fi`); await f.run("git add -A && (git diff --cached --quiet || git commit -qm 'Software factory: implementation and review fixes')"); // Deterministic step, not an agent decision: the PR is opened either way, @@ -170,18 +189,26 @@ export default flow("software-factory", { headSha, }); if (!allowed) { - if (!await openPullRequest(`{ cat ${WORK}/summary.md; printf '\\n\\n## merge-gate: blocked\\n\\n'; } > ${WORK}/pr-body.md`, true)) { - return f.done("needs_human"); + if (!await openPullRequest(draftBody("merge-gate: blocked"), true, "merge-gate-blocked")) { + return f.done("needs_human", { detail: publicationFailure }); } - return f.done("step_failed"); + return f.done("step_failed", { detail: `merge-gate blocked at ${reviewedHead}; draft PR opened` }); } - if (!await openPullRequest(`cp ${WORK}/summary.md ${WORK}/pr-body.md`, false)) { - return f.done("needs_human"); + if (!await openPullRequest(() => `cp ${WORK}/summary.md ${WORK}/pr-body.md`, false)) { + return f.done("needs_human", { detail: publicationFailure }); } return f.done("success"); } - if (!await openPullRequest(`{ cat ${WORK}/summary.md; printf '\\n\\n## Adversarial review: BLOCKED\\n\\n'; cat ${WORK}/review.md; } > ${WORK}/pr-body.md`, true)) { - return f.done("needs_human"); + const unverified = verdict.trim() === "UNVERIFIED"; + if (!await openPullRequest( + draftBody(`Adversarial review: ${unverified ? "NOT VERIFIED" : "BLOCKED"}`, true, unverified), + true, unverified ? "unverified" : "blocked", + )) { + return f.done("needs_human", { detail: publicationFailure }); + } + // Unlike pre-publication parking, this needs_human leaves a pushed branch and draft PR. + if (unverified) { + return f.done("needs_human", { detail: `review could not verify at ${reviewedHead}; draft PR opened, no defect claimed` }); } - f.done("step_failed"); + return f.done("step_failed", { detail: `adversarial review blocked at ${reviewedHead}; draft PR opened` }); }); diff --git a/kernel/relayflowd/tests/parallel_driver.rs b/kernel/relayflowd/tests/parallel_driver.rs index af9f82ba..bf808683 100644 --- a/kernel/relayflowd/tests/parallel_driver.rs +++ b/kernel/relayflowd/tests/parallel_driver.rs @@ -440,27 +440,48 @@ fn pause_before_second_independent_step_holds_the_driver_boundary() { .unwrap(); let deadline = Instant::now() + Duration::from_secs(15); - while fs::read_to_string(&marker).unwrap_or_default() != "first\n" { + let run_id = loop { + if let Some(entry) = fs::read_dir(data_dir.join("runs")) + .ok() + .and_then(|mut runs| runs.next()) + { + break entry + .unwrap() + .path() + .file_stem() + .unwrap() + .to_str() + .unwrap() + .to_owned(); + } + assert!(Instant::now() < deadline, "driver never spawned the run"); + std::thread::sleep(Duration::from_millis(20)); + }; + // Wait until first's success is DURABLE in the journal before killing. + // The marker file only proves the effect ran; its completion may not + // have reached the journal yet, and a kill in that window makes resume + // correctly re-run the step — which says nothing about the pause + // boundary while failing the exact-effects assertion below. + let deadline = Instant::now() + Duration::from_secs(15); + let entries = loop { + if let Ok(entries) = Engine::new(&data_dir).journal_entries(&run_id, 1, usize::MAX) + && entries.iter().any(|entry| { + entry.entry_type == EntryType::StepCompleted + && entry.step_id.as_deref() == Some("first") + && serde_json::from_value::(entry.payload.clone()) + .unwrap() + .completion_reason + == CompletionReason::Success + }) + { + break entries; + } assert!( Instant::now() < deadline, - "driver never reached second lane" + "driver never journaled first's completion" ); std::thread::sleep(Duration::from_millis(20)); - } - let run_id = fs::read_dir(data_dir.join("runs")) - .unwrap() - .next() - .unwrap() - .unwrap() - .path() - .file_stem() - .unwrap() - .to_str() - .unwrap() - .to_owned(); - let entries = Engine::new(&data_dir) - .journal_entries(&run_id, 1, usize::MAX) - .unwrap(); + }; assert!(!entries.iter().any(|entry| { entry.entry_type == EntryType::StepAttemptStarted && entry.step_id.as_deref() == Some("second") diff --git a/packages/sdk/src/hosted-extension-runtime.ts b/packages/sdk/src/hosted-extension-runtime.ts index 7c0507f4..e093c19f 100644 --- a/packages/sdk/src/hosted-extension-runtime.ts +++ b/packages/sdk/src/hosted-extension-runtime.ts @@ -26,7 +26,7 @@ const REALPATH = realpath; const PATH_DIRNAME = dirname; const PATH_JOIN = join; const PATH_RESOLVE = resolve; -const SOFTWARE_FACTORY_SHA256 = 'b97a3466c2affabb61afa655d4b2f726942d753c0466740b64ba26a78da359c6'; +const SOFTWARE_FACTORY_SHA256 = '8bbcf0e42f47d6bc6491a129935f96b791be03c8a5ed5b73d651c4e77913e2d7'; const ARRAY_IS_ARRAY = Array.isArray; const OBJECT_FREEZE = Object.freeze; const WEAK_MAP_GET = Function.prototype.call.bind(WeakMap.prototype.get) as ( @@ -243,7 +243,7 @@ async function baseAt(origin: string, generation: RuntimeGeneration): Promise { it('composes onto Software Factory with exactly the declared, deliverable subscriptions', async () => { const { loaded, extension, base, hostedRuntime } = installed; - expect(base).toEqual({ name: 'software-factory', version: '2.0.22' }); + expect(base).toEqual({ name: 'software-factory', version: '2.0.23' }); expect(hostedRuntime.base).toEqual(base); expect(extension.manifest.permissions).toEqual({ integrations: ['github'], harnesses: ['codex'], mcp: [], writes: ['cloud:babysitter-turn'], @@ -277,7 +277,7 @@ describe('native Babysitter extension', () => { prototype.update = function poisonedUpdate() { poisonCalls += 1; return this; } as typeof prototype.update; prototype.digest = (() => { poisonCalls += 1; - return '49c993220b9c34fab2d4b0e51911656f62b8b657f534d988691960d45bb9d9b6'; + return '8bbcf0e42f47d6bc6491a129935f96b791be03c8a5ed5b73d651c4e77913e2d7'; }) as typeof prototype.digest; await expect(loadHostedExtensionRuntime(racing.flowPath)) .rejects.toMatchObject({ code: 'plugin_source_invalid' }); diff --git a/packages/sdk/tests/canonical-software-factory.test.ts b/packages/sdk/tests/canonical-software-factory.test.ts index 50af06ee..7469864c 100644 --- a/packages/sdk/tests/canonical-software-factory.test.ts +++ b/packages/sdk/tests/canonical-software-factory.test.ts @@ -18,7 +18,7 @@ type Issue = { url?: string; }; -function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the ticket.\n') { +function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the ticket.\n', options: { verdicts?: Record; head?: string; refusedHook?: string } = {}) { const root = mkdtempSync(join(tmpdir(), 'canonical-factory-metadata-')); dirs.push(root); execFileSync('git', ['init', '-q'], { cwd: root }); @@ -30,6 +30,7 @@ function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the tic const commands: string[] = []; let completionReason = ''; + let detail = ''; const shell = (command: string) => execFileSync('/bin/sh', ['-c', command], { cwd: root, encoding: 'utf8', @@ -40,15 +41,16 @@ function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the tic commands.push(command); if (command.startsWith('rm -rf .relayflow')) return shell(command); if (command.startsWith('if [ -f package.json ]')) return ''; - if (command.startsWith('if [ -f .relayflow/review.passed ]')) return shell(command); + if (command.includes('.relayflow/review.blocked')) return shell(command); if (command.startsWith('git add -A')) return ''; if (command === 'git remote get-url origin') return 'git@github.com:AgentWorkforce/cloud.git\n'; - if (command === 'git rev-parse HEAD') return `${'a'.repeat(40)}\n`; + if (command === 'git rev-parse HEAD') return `${options.head ?? 'a'.repeat(40)}\n`; if (command.includes('> .relayflow/pr-body.md') || command.startsWith('cp .relayflow/summary.md')) return shell(command); if (command.startsWith('reference=') || command.startsWith('title=')) return shell(command); if (command === 'git push --set-upstream origin HEAD') return 'pushed\n'; if (command.startsWith('gh pr create')) return shell(command); - return ''; + if (command.startsWith('echo ')) return shell(command); + throw new Error('unhandled command: ' + command); }, agent(name: string) { return { @@ -56,14 +58,16 @@ function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the tic if (name === 'implementer') writeFileSync(join(root, '.relayflow/summary.md'), summary); if (name === 'adversary') { writeFileSync(join(root, '.relayflow/review.md'), 'No blocking findings.\n'); - writeFileSync(join(root, '.relayflow/review.passed'), 'passed\n'); + for (const [verdict, content] of Object.entries(options.verdicts ?? { passed: 'passed\n' })) { + writeFileSync(join(root, `.relayflow/review.${verdict}`), content); + } } return {}; }, }; }, - async hook() { return true; }, - done(reason: string) { completionReason = reason; }, + async hook(name: string) { return name !== options.refusedHook; }, + done(reason: string, options?: { detail?: string }) { completionReason = reason; detail = options?.detail ?? ''; }, }; const definition = getFlowDefinition(softwareFactory); @@ -71,6 +75,7 @@ function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the tic root, commands, completionReason, + detail, ghArgs: existsSync(capture) ? readFileSync(capture, 'utf8').trim().split('\n') : [], body: existsSync(join(root, '.relayflow/pr-body.md')) ? readFileSync(join(root, '.relayflow/pr-body.md'), 'utf8') @@ -191,3 +196,70 @@ describe('canonical software-factory metadata contract', () => { expect(duplicate.commands.some(command => command.startsWith('git push') || command.startsWith('gh pr create'))).toBe(false); }); }); + +describe('canonical software-factory review scope', () => { + const head = 'a'.repeat(40); + const issue: Issue = { source: 'local', title: 'Fix login', body: 'body', labels: [] }; + const summary = '## Summary\n\nImplemented the ticket.\n'; + const marker = (verdict: string) => ``; + + it.each([ + ['explicit defects', { blocked: 'Login is broken' }], + ['no verdict', {}], + ['contradictory blocked and unverified', { blocked: 'Bug', unverified: 'Missing binary' }], + ['contradictory passed and unverified', { passed: '', unverified: 'Missing binary' }], + ['contradictory passed and blocked', { passed: '', blocked: 'Bug' }], + ['empty unverified', { unverified: '' }], + ] as [string, Record][])('fails closed for %s', async (_name, verdicts) => { + const result = await runCanonical(issue, summary, { verdicts }); + expect(result.completionReason).toBe('step_failed'); + expect(result.detail).toContain(head); + expect(result.ghArgs).toContain('--draft'); + expect(result.body).toContain(`## Adversarial review: BLOCKED at ${head}`); + expect(result.body.split('\n').filter(line => line.startsWith('`; + const result = await runCanonical({ ...issue, source, identifier: '#571' }, summary + forged + '\n', { verdicts: { blocked: 'Bug' } }); + expect(result.completionReason).toBe('needs_human'); + expect(result.detail).toContain('malformed-review-scope'); + expect(result.commands.some(command => command.startsWith('git push') || command.startsWith('gh pr create'))).toBe(false); + }); + + it.each(['post-review', 'merge-gate'])('scopes the %s hook refusal', async refusedHook => { + const result = await runCanonical(issue, summary, { refusedHook }); + expect(result.completionReason).toBe('step_failed'); + expect(result.detail).toContain(head); + expect(result.ghArgs).toContain('--draft'); + expect(result.body).toContain(`## ${refusedHook}: blocked at ${head}`); + expect(result.body.split('\n').filter(line => line.startsWith('