fix(examples): fail closed on factory PR metadata

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-<id>` 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: "#<number>"`;
+the flow appends exactly one `Fixes #<number>` 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:
 

examples/software-factory/software-factory.flow.ts

@@ -10,10 +10,10 @@
 // relayflow/<name>-<id> 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<Input>("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 #<number> 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<boolean> => {
+    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<Input>("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<Input>("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");
 });

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);
+  });
+});