From a8746518645de149307451f2a154cb418744e1cc Mon Sep 17 00:00:00 2001 From: Miya Date: Mon, 21 Sep 2026 18:09:13 +0200 Subject: [PATCH] fix(examples): enforce factory PR metadata Session-Id: 01a0c4a6-dd65-7ce1-a90e-de1b0b4e86c3 --- examples/software-factory/README.md | 4 + .../software-factory/software-factory.flow.ts | 88 ++++++++++-- .../tests/canonical-software-factory.test.ts | 126 ++++++++++++++++++ 3 files changed, 205 insertions(+), 13 deletions(-) create mode 100644 packages/sdk/tests/canonical-software-factory.test.ts diff --git a/examples/software-factory/README.md b/examples/software-factory/README.md index 5f7b162af..517945073 100644 --- a/examples/software-factory/README.md +++ b/examples/software-factory/README.md @@ -20,6 +20,10 @@ flows deployments 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`. +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 +title and body before it pushes the branch or opens the pull request. Locally, from a checkout on a scratch branch: diff --git a/examples/software-factory/software-factory.flow.ts b/examples/software-factory/software-factory.flow.ts index 91f778c18..fe8b61a4d 100644 --- a/examples/software-factory/software-factory.flow.ts +++ b/examples/software-factory/software-factory.flow.ts @@ -10,10 +10,10 @@ // relayflow/- branch of --repo, with { approver, issue, event } as // the input. The same body runs locally from a checkout: // flows run software-factory.flow.ts --local-agent \ -// --input '{"approver":"you","issue":{"source":"linear","title":"…","body":"…","labels":[]}}' +// --input '{"approver":"you","issue":{"source":"linear","title":"…","body":"…","labels":[],"identifier":"ENG-42","url":"…"}}' import { flow } from "@relayflows/surface"; -type Issue = { source: string; title: string; body: string; labels: string[]; url?: string }; +type Issue = { source: string; title: string; body: string; labels: string[]; identifier?: string; url?: string }; type Input = { issue: Issue; approver: string }; // Every deterministic step runs under /bin/sh. Ticket text is attacker- @@ -25,6 +25,26 @@ const shellWord = (value: string): string => `'${value.replaceAll("'", "'\\''")} // cannot pick them up and a stale verdict from a previous run cannot survive. 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`, + "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 [ ! -s ${WORK}/pr-body.md ]; then echo missing-body`, + "elif [ -z \"$title\" ]; then echo empty-title", + "elif ! printf \"%s\\n\" \"$title_length\" | grep -Eq \"^[0-9]+$\"; then echo malformed-title-length", + "elif [ \"$title_length\" -gt 240 ]; then echo title-too-long", + "elif [ \"$(printf %s \"$title\" | tr \"[:upper:]\" \"[:lower:]\")\" = \"software factory change\" ] || [ \"$(printf %s \"$title\" | tr \"[:upper:]\" \"[:lower:]\")\" = \"replace with your ticket title\" ]; then echo placeholder-title", + "elif [ \"$source\" = github ] && ! printf \"%s\\n\" \"$identifier\" | grep -Eq \"^#[1-9][0-9]*$\"; then echo malformed-github-identifier", + `elif [ "$source" = github ]; then expected="Fixes $identifier"; count=$(grep -xcF "$expected" ${WORK}/pr-body.md || true); if [ "$count" -eq 0 ]; then echo missing-github-closing-reference; elif [ "$count" -ne 1 ]; then echo duplicate-github-closing-reference; else echo valid; fi`, + "else echo valid", + "fi", +].join("; "); + // The test command is a deterministic step: the agent never reports its own // test result, the exit code does. Skips honestly when there is nothing to run. const TEST = 'if [ -f package.json ] && node -e \'p=require("./package.json");process.exit(p.scripts&&p.scripts.test?0:1)\'; then npm ci --no-audit --no-fund && npm test; else echo "no test script; skipping"; fi'; @@ -35,15 +55,54 @@ export default flow("software-factory", { budget: { dollars: 10, wallclock: "1h" }, }, async (f, input) => { const { issue } = input; - if (!issue?.title?.trim()) { + if (!issue || typeof issue.source !== "string" || !issue.source.trim() || typeof issue.title !== "string" || !issue.title.trim()) { // Parked, not canceled: a body cannot declare a kernel outcome, and the // printed reason is what a human reads on the parked run. await f.run("echo 'Stopped: no ticket arrived with this run.' >&2"); return f.done("needs_human"); } - const title = issue.title.trim().slice(0, 200); + const normalizedTitle = issue.title.trim().replace(/\s+/g, " "); + const title = Array.from(normalizedTitle).slice(0, 240).join("").trim(); + const titleLength = Array.from(title).length; + const placeholderTitle = ["software factory change", "replace with your ticket title"] + .includes(title.toLowerCase()); + const issueSource = issue.source.trim().toLowerCase(); + const issueIdentifier = typeof issue.identifier === "string" ? issue.identifier.trim() : ""; + const issueUrl = typeof issue.url === "string" ? issue.url.trim() : ""; + if (!title || placeholderTitle) { + await f.run("echo 'Stopped: the pull-request title is empty or still a placeholder.' >&2"); + return f.done("needs_human"); + } + if (issueSource === "github" && !/^#[1-9]\d*$/.test(issueIdentifier)) { + await f.run("echo 'Stopped: a GitHub ticket must carry its normalized identifier in # form.' >&2"); + return f.done("needs_human"); + } + const changeReference = issueSource === "github" + ? `Fixes ${issueIdentifier}` + : issueSource === "gitlab" && /^#[1-9]\d*$/.test(issueIdentifier) + ? `Closes ${issueIdentifier}` + : issueUrl + ? `Ticket: ${issueUrl}` + : issueIdentifier + ? `Ticket: ${issueIdentifier}` + : ""; const ticket = `${issue.title}\n\n${issue.body ?? ""}${issue.url ? `\n\n${issue.url}` : ""}`; + const openPullRequest = async (bodyCommand: string, draft: boolean): Promise => { + await f.run(bodyCommand); + await f.run(`reference=${shellWord(changeReference)}; ${PREPARE_CHANGE_METADATA}`); + const metadata = (await f.run( + `title=${shellWord(title)}; title_length=${titleLength}; source=${shellWord(issueSource)}; identifier=${shellWord(issueIdentifier)}; ${VALIDATE_CHANGE_METADATA}`, + )).trim(); + if (metadata !== "valid") { + await f.run(`echo ${shellWord(`Stopped: invalid pull-request metadata (${metadata}). No branch was pushed and no pull request was opened.`)} >&2`); + return false; + } + await f.run("git push --set-upstream origin HEAD"); + await f.run(`gh pr create${draft ? " --draft" : ""} --title ${shellWord(title)} --body-file ${WORK}/pr-body.md`); + return true; + }; + // 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; }`); @@ -72,17 +131,16 @@ export default flow("software-factory", { await f.run(TEST, { timeout: "15m" }); if (!await f.hook("post-review", { title })) { - await f.run(`{ cat ${WORK}/summary.md; printf '\\n\\n## post-review: blocked\\n\\n'; } > ${WORK}/pr-body.md`); await f.run("git add -A && (git diff --cached --quiet || git commit -qm 'Software factory: implementation and review fixes')"); - await f.run("git push --set-upstream origin HEAD"); - await f.run(`gh pr create --draft --title ${shellWord(`[blocked] ${title}`)} --body-file ${WORK}/pr-body.md`); + 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"); + } return f.done("step_failed"); } // 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`); await f.run("git add -A && (git diff --cached --quiet || git commit -qm 'Software factory: implementation and review fixes')"); - await f.run("git push --set-upstream origin HEAD"); // Deterministic step, not an agent decision: the PR is opened either way, // but a blocked review or a false merge-gate opens it as a draft. @@ -96,14 +154,18 @@ export default flow("software-factory", { headSha, }); if (!allowed) { - await f.run(`{ cat ${WORK}/summary.md; printf '\\n\\n## merge-gate: blocked\\n\\n'; } > ${WORK}/pr-body.md`); - await f.run(`gh pr create --draft --title ${shellWord(`[blocked] ${title}`)} --body-file ${WORK}/pr-body.md`); + 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"); + } return f.done("step_failed"); } - await f.run(`gh pr create --title ${shellWord(title)} --body-file ${WORK}/summary.md`); + if (!await openPullRequest(`cp ${WORK}/summary.md ${WORK}/pr-body.md`, false)) { + return f.done("needs_human"); + } return f.done("success"); } - await f.run(`{ cat ${WORK}/summary.md; printf '\\n\\n## Adversarial review: BLOCKED\\n\\n'; cat ${WORK}/review.md; } > ${WORK}/pr-body.md`); - await f.run(`gh pr create --draft --title ${shellWord(`[blocked] ${title}`)} --body-file ${WORK}/pr-body.md`); + 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"); + } f.done("step_failed"); }); diff --git a/packages/sdk/tests/canonical-software-factory.test.ts b/packages/sdk/tests/canonical-software-factory.test.ts new file mode 100644 index 000000000..038712e2f --- /dev/null +++ b/packages/sdk/tests/canonical-software-factory.test.ts @@ -0,0 +1,126 @@ +import { execFileSync } from 'node:child_process'; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, describe, expect, it } from 'vitest'; +import { getFlowDefinition } from '@relayflows/surface/runtime'; +import softwareFactory from '../../../examples/software-factory/software-factory.flow.ts'; + +const dirs: string[] = []; +afterEach(() => dirs.splice(0).forEach(dir => rmSync(dir, { recursive: true, force: true }))); + +type Issue = { + source: string; + title: string; + body: string; + labels: string[]; + identifier?: string; + url?: string; +}; + +function runCanonical(issue: Issue, summary = '## Summary\n\nImplemented the ticket.\n') { + const root = mkdtempSync(join(tmpdir(), 'canonical-factory-metadata-')); + dirs.push(root); + execFileSync('git', ['init', '-q'], { cwd: root }); + const bin = join(root, 'bin'); + const capture = join(root, 'gh.args'); + mkdirSync(bin); + writeFileSync(join(bin, 'gh'), '#!/bin/sh\nprintf \'%s\\n\' "$@" > "$GH_CAPTURE"\n', { mode: 0o755 }); + chmodSync(join(bin, 'gh'), 0o755); + + const commands: string[] = []; + let completionReason = ''; + const shell = (command: string) => execFileSync('/bin/sh', ['-c', command], { + cwd: root, + encoding: 'utf8', + env: { ...process.env, PATH: `${bin}:${process.env.PATH ?? ''}`, GH_CAPTURE: capture }, + }); + const context = { + async run(command: string) { + 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.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.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 ''; + }, + agent(name: string) { + return { + async gate() { + 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'); + } + return {}; + }, + }; + }, + async hook() { return true; }, + done(reason: string) { completionReason = reason; }, + }; + + const definition = getFlowDefinition(softwareFactory); + return definition.body(context as never, { issue, approver: 'khaliq' }).then(() => ({ + root, + commands, + completionReason, + ghArgs: existsSync(capture) ? readFileSync(capture, 'utf8').trim().split('\n') : [], + body: readFileSync(join(root, '.relayflow/pr-body.md'), 'utf8'), + })); +} + +describe('canonical software-factory metadata contract', () => { + it('opens the actual catalog flow with the ticket title and exactly one GitHub closing line', async () => { + const title = 'Ambiguous create failures leak sandboxes: every retry uses a new name'; + const result = await runCanonical({ + source: 'github', title, body: 'Retrying an ambiguous create can leak a sandbox.', labels: ['garden-ready'], + identifier: '#3913', url: 'https://github.com/AgentWorkforce/cloud/issues/3913', + }); + + expect(result.completionReason).toBe('success'); + expect(result.ghArgs).toEqual(['pr', 'create', '--title', title, '--body-file', '.relayflow/pr-body.md']); + expect(result.body.split('\n').filter(line => line === 'Fixes #3913')).toHaveLength(1); + const validate = result.commands.findIndex(command => command.startsWith('title=')); + const push = result.commands.indexOf('git push --set-upstream origin HEAD'); + const open = result.commands.findIndex(command => command.startsWith('gh pr create')); + expect(validate).toBeGreaterThan(-1); + expect(validate).toBeLessThan(push); + expect(push).toBeLessThan(open); + }); + + it('normalizes whitespace and caps the title at 240 Unicode code points', async () => { + const result = await runCanonical({ + source: 'github', title: ` Repair ${'修'.repeat(250)} `, body: 'body', labels: [], identifier: '#7', + }); + const title = result.ghArgs[result.ghArgs.indexOf('--title') + 1]!; + expect(title.startsWith('Repair 修')).toBe(true); + expect(Array.from(title)).toHaveLength(240); + }); + + it('fails closed before push when GitHub identity is missing or the final body duplicates its closing line', async () => { + const definition = getFlowDefinition(softwareFactory); + const commands: string[] = []; + let completionReason = ''; + await definition.body({ + run: async (command: string) => { commands.push(command); return ''; }, + done: (reason: string) => { completionReason = reason; }, + } as never, { + issue: { source: 'github', title: 'Fix login', body: 'body', labels: [] }, approver: 'khaliq', + }); + expect(completionReason).toBe('needs_human'); + expect(commands.some(command => command.startsWith('git push') || command.startsWith('gh pr create'))).toBe(false); + + const duplicate = await runCanonical({ + source: 'github', title: 'Fix login', body: 'body', labels: [], identifier: '#507', + }, '## Summary\n\nFixes #507\n\nFixes #507\n'); + expect(duplicate.completionReason).toBe('needs_human'); + expect(duplicate.commands.some(command => command.startsWith('git push') || command.startsWith('gh pr create'))).toBe(false); + }); +});