From fecdd6b5d94596d72efc3eef65bbb99a7443c468 Mon Sep 17 00:00:00 2001 From: Relayflow Date: Thu, 24 Sep 2026 05:46:17 +0000 Subject: [PATCH 1/6] Scope Software Factory draft verdicts to reviewed commits --- docs/BABYSITTER-CATALOG-HANDOFF.md | 7 +- examples/software-factory/README.md | 24 +- .../software-factory/software-factory.flow.ts | 73 +++-- .../tests/babysitter-native-extension.test.ts | 2 +- .../tests/canonical-software-factory.test.ts | 88 +++++- summary.md | 260 ++++++++++++++++++ 6 files changed, 419 insertions(+), 35 deletions(-) create mode 100644 summary.md diff --git a/docs/BABYSITTER-CATALOG-HANDOFF.md b/docs/BABYSITTER-CATALOG-HANDOFF.md index ad1381c8c..f30c83146 100644 --- a/docs/BABYSITTER-CATALOG-HANDOFF.md +++ b/docs/BABYSITTER-CATALOG-HANDOFF.md @@ -28,8 +28,11 @@ 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 still pins the previously +reviewed `2.0.22` source bytes and rejects this revised base. Independent review +must authorize updating that source digest and the loader's assigned version +before this base can be used with the native extension. The identity regression +expects `2.0.23` and remains blocked by that source check until then. 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 517945073..bafe63304 100644 --- a/examples/software-factory/README.md +++ b/examples/software-factory/README.md @@ -19,7 +19,29 @@ flows deployments `--on` also takes `github:labels=agent`, `jira:project=OPS`, `shortcut:workspace=…` or `slack:channel=#eng`. 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 fe8b61a4d..4685301fb 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; } @@ -103,12 +115,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", { @@ -125,21 +143,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, @@ -154,18 +173,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/packages/sdk/tests/babysitter-native-extension.test.ts b/packages/sdk/tests/babysitter-native-extension.test.ts index b99a5d2b3..e9afb3a3e 100644 --- a/packages/sdk/tests/babysitter-native-extension.test.ts +++ b/packages/sdk/tests/babysitter-native-extension.test.ts @@ -108,7 +108,7 @@ async function handle(eventType: string, input: unknown, f: Ctx) { describe('native Babysitter extension', () => { 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'], diff --git a/packages/sdk/tests/canonical-software-factory.test.ts b/packages/sdk/tests/canonical-software-factory.test.ts index 038712e2f..5290797c3 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,8 +75,9 @@ function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the tic root, commands, completionReason, + detail, ghArgs: existsSync(capture) ? readFileSync(capture, 'utf8').trim().split('\n') : [], - body: readFileSync(join(root, '.relayflow/pr-body.md'), 'utf8'), + body: existsSync(join(root, '.relayflow/pr-body.md')) ? readFileSync(join(root, '.relayflow/pr-body.md'), 'utf8') : '', })); } @@ -124,3 +129,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('