From 08921f13544e7c44255449af911170d6686fbbdb Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 20 Sep 2026 12:56:16 -0700 Subject: [PATCH 1/2] fix(flows): preserve ticket metadata in garden PRs --- web/lib/flow-local.ts | 73 +++++++++++++++--------- web/lib/flow-sources.ts | 4 ++ web/lib/flow-workflows.ts | 72 ++++++++++++++++++++++- web/lib/test/flow-agent-settings.test.ts | 5 +- web/lib/test/flow-local.test.ts | 25 +++++--- web/lib/test/flow-onboarding.test.ts | 25 +++++++- web/lib/test/flow-sources.test.ts | 4 ++ web/lib/test/flow-workflows.test.ts | 46 +++++++++++++-- 8 files changed, 206 insertions(+), 48 deletions(-) diff --git a/web/lib/flow-local.ts b/web/lib/flow-local.ts index 6f265be..ef1f841 100644 --- a/web/lib/flow-local.ts +++ b/web/lib/flow-local.ts @@ -84,6 +84,9 @@ export function localInput(draft: FactoryDraft) { source, title: contains?.trim() || PLACEHOLDER_TITLE, body: PLACEHOLDER_BODY, labels: labels?.split(',').map(label => label.trim()).filter(Boolean) ?? [], + // Cloud supplies this from provider metadata. A local GitHub run must ask + // for the real number rather than manufacture one on the user's behalf. + ...(source === 'github' ? { identifier: '' } : {}), ...fields, } }; } @@ -386,43 +389,57 @@ if (existsSync(INPUT)) { // body is the sentinel, not title: a "contains" filter overwrites title // when the kit is built, so only body is reliably the placeholder. const untouched = issue.body === PLACEHOLDER_BODY; - if (untouched && !process.stdin.isTTY) { - fail(INPUT + " still holds the placeholder ticket.", - "Set issue.title and issue.body to the real ticket, then run again.", + const missingGithubIdentifier = issue.source === "github" && !/^#[1-9]\\d*$/.test((issue.identifier || "").trim()); + if ((untouched || missingGithubIdentifier) && !process.stdin.isTTY) { + fail(INPUT + " does not contain complete ticket metadata.", + "Set issue.title and issue.body to the real ticket and, for GitHub, set issue.identifier to #.", "Nothing here can be asked without a terminal, so the run stops rather", "than sending a coding agent after " + JSON.stringify(PLACEHOLDER_TITLE) + "."); } - if (untouched) { + if (untouched || missingGithubIdentifier) { const { rl, ask } = prompter(); try { - console.log(INPUT + " still holds the placeholder ticket. Fill it in now."); + console.log(INPUT + " needs complete ticket metadata. Fill it in now."); console.log(""); - let title = ""; - while (!title) { - const answer = await ask("Ticket title: "); - // Ctrl+D or a closed pipe at the prompt ends with the same advice as - // the no-terminal path, not an unhandled rejection. - if (answer === null) { - fail("the ticket was not entered.", - "Set issue.title and issue.body in " + INPUT + ", then run again."); + if (untouched) { + let title = ""; + while (!title) { + const answer = await ask("Ticket title: "); + // Ctrl+D or a closed pipe at the prompt ends with the same advice as + // the no-terminal path, not an unhandled rejection. + if (answer === null) { + fail("the ticket was not entered.", + "Set issue.title and issue.body in " + INPUT + ", then run again."); + } + title = answer.trim(); + if (!title) console.log(" A title is required."); } - title = answer.trim(); - if (!title) console.log(" A title is required."); - } - console.log("Description and acceptance criteria. Finish with an empty line."); - const lines = []; - for (;;) { - const line = await ask("> "); - // Ctrl+D ends the description, exactly as the empty line does. - if (line === null || !line.trim()) break; - lines.push(line); + console.log("Description and acceptance criteria. Finish with an empty line."); + const lines = []; + for (;;) { + const line = await ask("> "); + // Ctrl+D ends the description, exactly as the empty line does. + if (line === null || !line.trim()) break; + lines.push(line); + } + const body = lines.join("\\n").trim(); + if (!body) { + fail("no description was entered.", + "Run again and describe the work, or edit " + INPUT + " by hand."); + } + input.issue = { ...issue, title, body }; } - const body = lines.join("\\n").trim(); - if (!body) { - fail("no description was entered.", - "Run again and describe the work, or edit " + INPUT + " by hand."); + if (missingGithubIdentifier) { + let identifier = ""; + while (!/^#[1-9]\\d*$/.test(identifier)) { + const answer = await ask("GitHub issue number (for example #507): "); + if (answer === null) fail("the GitHub issue number was not entered.", + "Set issue.identifier to # in " + INPUT + ", then run again."); + identifier = answer.trim(); + if (!/^#[1-9]\\d*$/.test(identifier)) console.log(" Use # followed by the issue number."); + } + input.issue = { ...input.issue, identifier }; } - input.issue = { ...issue, title, body }; writeFileSync(INPUT, JSON.stringify(input, null, 2) + "\\n"); console.log(""); console.log("Saved to " + INPUT + "."); diff --git a/web/lib/flow-sources.ts b/web/lib/flow-sources.ts index 1f5edaa..2a62332 100644 --- a/web/lib/flow-sources.ts +++ b/web/lib/flow-sources.ts @@ -106,6 +106,10 @@ export type Issue = { title: string; body: string; labels: string[];${optional} + // Normalized by Cloud for every connected ticket provider. A local input + // may leave either field blank when that provider has no such metadata. + identifier?: string; + url?: string; };`; } diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index 76b9ad3..90abae7 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -325,6 +325,35 @@ export const FLOW_DROP_WORKING_FILES_COMMAND = [ export const FLOW_OPEN_CHANGE_COMMAND = 'open_change() { if command -v relayflow-open-change >/dev/null 2>&1; then relayflow-open-change "$@"; else gh pr create "$@"; fi; }; open_change'; +/** + * Adds the deterministic provider reference after the generated check report. + * The reference is supplied through a shell-quoted variable by the generated + * flow; grep matches a complete, fixed line so an agent-written matching line + * is retained rather than duplicated. + */ +export const FLOW_PREPARE_CHANGE_METADATA_COMMAND = [ + 'if [ ! -s .relayflow/pr-body.md ]; then echo missing-body; exit 0; fi', + 'if [ -n "$reference" ] && ! grep -qxF "$reference" .relayflow/pr-body.md; then printf "\\n%s\\n" "$reference" >> .relayflow/pr-body.md; fi', + 'echo prepared', +].join('; '); + +/** + * Final fail-closed contract immediately before a branch is pushed or a + * change request is opened. It deliberately exits zero with one verdict: an + * invalid verdict is handled by the flow instead of being retried as a flaky + * command. GitHub inputs must have exactly one normalized closing line. + */ +export const FLOW_VALIDATE_CHANGE_METADATA_COMMAND = [ + 'if [ ! -s .relayflow/pr-body.md ]; then echo missing-body', + 'elif [ -z "$title" ]; then echo empty-title', + 'elif [ "${#title}" -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" .relayflow/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('; '); + /** * Decides whether there is anything to publish, before the branch is pushed and * before `gh pr create` runs. @@ -411,7 +440,35 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType form. No branch was pushed and no pull request was opened."); + return f.done("needs_human"); + } + // GitHub receives its exact closing keyword. Other providers get a stable + // native reference when one is available; Markdown invents no identifier. + const changeReference = issueSource === "github" + ? "Fixes " + issueIdentifier + : issueSource === "gitlab" && /^#[1-9]\\d*$/.test(issueIdentifier) + ? "Closes " + issueIdentifier + : issueUrl + ? "Ticket: " + issueUrl + : issueIdentifier + ? "Ticket: " + issueIdentifier + : ""; + const task = issue.title + "\\n" + issue.body + "\\n" + ${JSON.stringify(instructions.trim() || 'Follow existing patterns. Keep changes focused and add regression tests.')}; // Files the agents write for each other (summary.md, plans, reviews, and // .relayflow/) are never part of the change; keep them out of every commit. @@ -523,8 +580,8 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType command.startsWith('base=') ? 'publish' : command.startsWith('mktemp') ? '/tmp/prototypes' : command.includes('review.clean &&') ? 'yes' : 'base', done: () => {} }, - { issue: { source: 'github', title: 'Ticket title', body: 'Ticket body', labels: [] } }); + run: async (command: string) => command.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND) ? 'valid' : command.startsWith('base=') ? 'publish' : command.startsWith('mktemp') ? '/tmp/prototypes' : command.includes('review.clean &&') ? 'yes' : 'base', done: () => {} }, + { issue: { source: 'github', title: 'Ticket title', body: 'Ticket body', labels: [], identifier: '#507' } }); return calls; } diff --git a/web/lib/test/flow-local.test.ts b/web/lib/test/flow-local.test.ts index 6acda68..e7670ff 100644 --- a/web/lib/test/flow-local.test.ts +++ b/web/lib/test/flow-local.test.ts @@ -8,7 +8,7 @@ import { pathToFileURL } from 'node:url'; import ts from 'typescript'; import { DEFAULT_FACTORY, factorySource, type FactoryDraft } from '../flow-onboarding'; import { LOCAL_INSTALL, LOCAL_PREFLIGHT, LOCAL_RUN, PLACEHOLDER_BODY, PLACEHOLDER_TITLE, RELAYFLOWS_VERSION, localInput, localKitArchive, localKitFiles } from '../flow-local'; -import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_BLOCKED_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PUBLISH_CHECK_COMMAND } from '../flow-workflows'; +import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_BLOCKED_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_VALIDATE_CHANGE_METADATA_COMMAND } from '../flow-workflows'; /** * What each deterministic step reports, keyed by the command itself: three @@ -19,10 +19,17 @@ function answer(command: string, { publish = 'publish', clean = 'yes', check = ' if (command === FLOW_CHECK_RUN_COMMAND) return typeof check === 'function' ? check() : check; if (command.endsWith(FLOW_BASE_CHECK_COMMAND)) return baseline; if (command.endsWith(FLOW_PUBLISH_CHECK_COMMAND)) return publish; + if (command.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND)) return 'valid'; return command.startsWith('test -f') ? clean : ''; } const draft: FactoryDraft = { ...DEFAULT_FACTORY, sources: ['github'], sourceSettings: { github: { repository: 'acme/app', labels: 'bug, ready' } }, agents: ['claude', 'codex'], workflow: 'traditional', step: 3 }; +const localRunInput = (selected = draft) => { + const input = localInput(selected); + return 'issue' in input ? { ...input, issue: { + ...input.issue, title: 'Fix login', body: 'Users cannot sign in.', identifier: '#507', + } } : input; +}; /** * The generated flow's comments name the completion reasons it deliberately @@ -119,6 +126,7 @@ describe('local flow starter kit', () => { // matching what localInput prefills, an unedited ticket reaches an agent. expect(issue.body).toBe(PLACEHOLDER_BODY); expect(issue.title).toBe(PLACEHOLDER_TITLE); + expect((issue as { identifier?: string }).identifier).toBe(''); expect(script).toContain(JSON.stringify(PLACEHOLDER_BODY)); // body, not title: a `contains` filter overwrites title at build time. const filtered = { ...draft, sourceSettings: { github: { repository: 'acme/app', contains: 'Please fix' } } }; @@ -131,9 +139,10 @@ describe('local flow starter kit', () => { it('prompts on a terminal and fails fast without one instead of hanging', () => { const script = localKitFiles(draft)[LOCAL_PREFLIGHT]; expect(script).toContain('createInterface'); - expect(script).toContain('untouched && !process.stdin.isTTY'); + expect(script).toContain('(untouched || missingGithubIdentifier) && !process.stdin.isTTY'); // The non-interactive refusal has to name the file and both fields. expect(script).toContain('Set issue.title and issue.body to the real ticket'); + expect(script).toContain('issue.identifier to #'); expect(script).toContain('writeFileSync(INPUT, JSON.stringify(input, null, 2)'); }); @@ -242,7 +251,7 @@ describe('local flow starter kit', () => { agent: async (name: string) => { calls.push(name); }, run: async (command: string) => answer(command), done: (reason: string) => { finish = reason; }, - }, localInput(draft)); + }, localRunInput()); expect(calls).toEqual(['planner', 'plan-reviewer', 'check-discovery', 'implementer', 'adversary-1', 'adversary-2']); expect(finish).toBe('needs_human'); expect(factorySource(draft)).toContain('return f.done("needs_human")'); @@ -265,7 +274,7 @@ describe('local flow starter kit', () => { agent: async () => {}, run: async (command: string) => { commands.push(command); return answer(command, { publish: 'no-commits' }); }, done: (reason: string) => { finish = reason; }, - }, localInput(draft)); + }, localRunInput()); } finally { console.error = original; } expect(commands.some(command => command.startsWith(FLOW_OPEN_CHANGE_COMMAND))).toBe(false); expect(commands.some(command => command.startsWith('git push'))).toBe(false); @@ -282,7 +291,7 @@ describe('local flow starter kit', () => { agent: async () => {}, run: async (command: string) => { commands.push(command); return answer(command); }, done: (reason: string) => { finish = reason; }, - }, localInput(selected)); + }, localRunInput(selected)); expect(finish).toBe('needs_human'); const testIndex = commands.indexOf(FLOW_CHECK_RUN_COMMAND); const createIndex = commands.findIndex(command => command.startsWith(FLOW_OPEN_CHANGE_COMMAND)); @@ -304,7 +313,7 @@ describe('local flow starter kit', () => { agent: async () => {}, run: async (command: string) => { commands.push(command); return answer(command, { check: 'fail', baseline: 'pass' }); }, done: (reason: string) => { finish = reason; }, - }, localInput(selected)); + }, localRunInput(selected)); const create = commands.find(command => command.startsWith(FLOW_OPEN_CHANGE_COMMAND)) ?? ''; expect(create).toContain('--draft'); expect(commands).toContain('git push --set-upstream origin HEAD'); @@ -326,7 +335,7 @@ describe('local flow starter kit', () => { return answer(command, { clean: 'no', check: () => (++checks >= 2 && fail ? 'fail' : 'pass') }); }, done: (reason: string) => { finish = reason; }, - }, localInput(draft)); + }, localRunInput()); const fixer = calls.indexOf('fixer'); expect(fixer).toBeGreaterThan(0); expect(calls[fixer + 1]).toBe(FLOW_CHECK_RUN_COMMAND); @@ -531,7 +540,7 @@ describe('relocating a kit that was extracted outside a repository', () => { } if (options.ticket) { writeFileSync(join(target, 'flow-input.json'), JSON.stringify({ approver: 'local', - issue: { source: 'github', title: 'Fix login', body: 'Users cannot sign in.', labels: ['bug', 'ready'], repository: 'acme/app' } }, null, 2) + '\n'); + issue: { source: 'github', title: 'Fix login', body: 'Users cannot sign in.', labels: ['bug', 'ready'], repository: 'acme/app', identifier: '#507' } }, null, 2) + '\n'); } return target; } diff --git a/web/lib/test/flow-onboarding.test.ts b/web/lib/test/flow-onboarding.test.ts index 6a8be58..ed98ef5 100644 --- a/web/lib/test/flow-onboarding.test.ts +++ b/web/lib/test/flow-onboarding.test.ts @@ -1,10 +1,10 @@ import { describe, expect, it } from 'vitest'; import ts from 'typescript'; -import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_BLOCKED_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_DROP_WORKING_FILES_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND } from '../flow-workflows'; +import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_BLOCKED_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_DROP_WORKING_FILES_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PREPARE_CHANGE_METADATA_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND, FLOW_VALIDATE_CHANGE_METADATA_COMMAND } from '../flow-workflows'; import { cloudBlockedReason, cloudConnectionsHref, DEFAULT_FACTORY, factorySource, isMarkdownOnly, MARKDOWN_ONLY_CLOUD_NOTE, readFactoryDraft, canContinue, primaryAgent, onboardingPath, accessibleOnboardingStep, type FactoryDraft } from '../flow-onboarding'; import { localInput } from '../flow-local'; -const matchingIssue = { source: 'github', title: 'Fix login', body: 'Login fails', labels: ['ready', 'bug'], repository: 'acme/app' }; +const matchingIssue = { source: 'github', title: ' Fix login ', body: 'Login fails', labels: ['ready', 'bug'], repository: 'acme/app', identifier: '#507', url: 'https://github.com/acme/app/issues/507' }; const completed: FactoryDraft = { version: 4, sources: ['github'], sourceSettings: { github: { repository: 'acme/app', labels: 'ready, bug' } }, agents: ['claude', 'codex'], otherAgent: '', task: 'Add a test', workflow: 'traditional', step: 3 }; @@ -41,6 +41,7 @@ async function runFactory(clean: boolean[], _approved = true, issue = matchingIs if (command === FLOW_CHECK_RUN_COMMAND) return checks[checkIndex++] ?? 'pass'; if (command.endsWith(FLOW_BASE_CHECK_COMMAND)) return baseline; if (command.endsWith(FLOW_PUBLISH_CHECK_COMMAND)) return publish; + if (command.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND)) return 'valid'; return command.startsWith('test -f') ? (clean[index++] ? 'yes' : 'no') : command.startsWith('mktemp') ? '/tmp/relay-prototypes.test' : command === 'git rev-parse HEAD' ? 'abc123' : ''; }, human: async () => { throw new Error('Interactive human approval is unsupported'); }, @@ -222,6 +223,19 @@ describe('software factory onboarding', () => { expect(withoutComments(source)).not.toContain('f.done("canceled")'); }); + it('refuses placeholder titles and missing GitHub identifiers before agents, push, or PR creation', async () => { + for (const issue of [ + { ...matchingIssue, title: 'Software factory change' }, + { ...matchingIssue, identifier: '' }, + { ...matchingIssue, identifier: '507' }, + ]) { + const { calls, finish, errors } = await runFactory([true], true, issue); + expect(calls).toEqual([]); + expect(finish).toBe('needs_human'); + expect(errors.join('\n')).toContain('No branch was pushed and no pull request was opened'); + } + }); + it('gives Cloud flows a wall-clock budget so unpriced agents are never refused', () => { for (const agents of [['claude', 'codex'], ['codex'], ['claude']] as FactoryDraft['agents'][]) { const source = factorySource({ ...completed, agents }); @@ -448,7 +462,12 @@ describe('software factory onboarding', () => { expect(calls.filter(call => call.startsWith('check-repair'))).toEqual([]); expect(calls.some(call => call.endsWith(FLOW_BASE_CHECK_COMMAND))).toBe(false); expect(reportCall(calls)).toMatch(/^check=pass; baseline=; /); - expect(createCall(calls)).toBe(FLOW_OPEN_CHANGE_COMMAND + ' --title "Software factory change" --body-file .relayflow/pr-body.md'); + const prepare = calls.find(call => call.endsWith(FLOW_PREPARE_CHANGE_METADATA_COMMAND)) ?? ''; + const validate = calls.find(call => call.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND)) ?? ''; + expect(prepare).toContain("reference='Fixes #507'"); + expect(validate).toContain("identifier='#507'"); + expect(calls.indexOf(validate)).toBeLessThan(calls.indexOf('git push --set-upstream origin HEAD')); + expect(createCall(calls)).toBe(FLOW_OPEN_CHANGE_COMMAND + " --title 'Fix login' --body-file .relayflow/pr-body.md"); expect(finish).toBe('needs_human'); }); diff --git a/web/lib/test/flow-sources.test.ts b/web/lib/test/flow-sources.test.ts index 8cc74e3..968c84d 100644 --- a/web/lib/test/flow-sources.test.ts +++ b/web/lib/test/flow-sources.test.ts @@ -104,10 +104,14 @@ describe('generated issue source filters', () => { it('declares only the fields the chosen sources can deliver', () => { const github = issueSourceCode(['github'], {}, 'cloud'); + expect(github).toContain('identifier?: string;'); + expect(github).toContain('url?: string;'); expect(github).toContain('repository?: string;'); expect(github).not.toContain('channel?'); expect(github).not.toContain('mentioned?'); const slack = issueSourceCode(['slack'], {}, 'cloud'); + expect(slack).toContain('identifier?: string;'); + expect(slack).toContain('url?: string;'); expect(slack).toContain('channel?: string;'); expect(slack).toContain('mentioned?: boolean;'); expect(slack).not.toContain('repository?'); diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index ad28cb4..ed67c8c 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -5,7 +5,7 @@ import { tmpdir } from 'node:os'; import path from 'node:path'; import { FLOW_BASE_CHECK_COMMAND, FLOW_CHECK_REPORT_COMMAND, FLOW_CHECK_RESOLVE_COMMAND, FLOW_CHECK_RUN_COMMAND, FLOW_CHECK_SCRIPT, - FLOW_DROP_WORKING_FILES_COMMAND, FLOW_EXCLUDE_WORKING_FILES_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND, + FLOW_DROP_WORKING_FILES_COMMAND, FLOW_EXCLUDE_WORKING_FILES_COMMAND, FLOW_OPEN_CHANGE_COMMAND, FLOW_PREPARE_CHANGE_METADATA_COMMAND, FLOW_PUBLISH_CHECK_COMMAND, FLOW_REVIEW_BLOCKED_COMMAND, FLOW_VALIDATE_CHANGE_METADATA_COMMAND, } from '../flow-workflows'; /** @@ -620,13 +620,13 @@ describe('FLOW_OPEN_CHANGE_COMMAND', () => { } return { root, env: { PATH: `${bin}:/usr/bin:/bin` } }; } - const args = ' --title "Software factory change" --body-file .relayflow/pr-body.md --draft'; + const args = ' --title "Fix login" --body-file .relayflow/pr-body.md --draft'; it('uses the hosted helper when Cloud put it on PATH (GitHub or GitLab alike)', () => { const { root, env } = fakes(['relayflow-open-change', 'gh']); expect(sh(FLOW_OPEN_CHANGE_COMMAND + args, root, env).code).toBe(0); expect(read(root, 'relayflow-open-change.args').trim().split('\n')) - .toEqual(['relayflow-open-change', '--title', 'Software factory change', '--body-file', '.relayflow/pr-body.md', '--draft']); + .toEqual(['relayflow-open-change', '--title', 'Fix login', '--body-file', '.relayflow/pr-body.md', '--draft']); expect(read(root, 'gh.args')).toBe(''); }); @@ -634,7 +634,7 @@ describe('FLOW_OPEN_CHANGE_COMMAND', () => { const { root, env } = fakes(['gh']); expect(sh(FLOW_OPEN_CHANGE_COMMAND + args, root, env).code).toBe(0); expect(read(root, 'gh.args').trim().split('\n')) - .toEqual(['gh', 'pr', 'create', '--title', 'Software factory change', '--body-file', '.relayflow/pr-body.md', '--draft']); + .toEqual(['gh', 'pr', 'create', '--title', 'Fix login', '--body-file', '.relayflow/pr-body.md', '--draft']); }); it('keeps the exit status, so a failed create still fails the step', () => { @@ -642,3 +642,41 @@ describe('FLOW_OPEN_CHANGE_COMMAND', () => { expect(sh(FLOW_OPEN_CHANGE_COMMAND + args, root, env).code).toBe(3); }); }); + +describe('change metadata contract', () => { + const prepare = (root: string, reference: string) => + sh(`reference='${reference}'; ${FLOW_PREPARE_CHANGE_METADATA_COMMAND}`, root); + const validate = (root: string, title: string, source: string, identifier: string) => + sh(`title='${title}'; source='${source}'; identifier='${identifier}'; ${FLOW_VALIDATE_CHANGE_METADATA_COMMAND}`, root); + + it('adds exactly one normalized GitHub closing line and accepts the final artifacts', () => { + const root = fixture({ '.relayflow/pr-body.md': '## Summary\n\nImplemented login recovery.\n' }); + expect(prepare(root, 'Fixes #507').token).toBe('prepared'); + expect(prepare(root, 'Fixes #507').token).toBe('prepared'); + const lines = read(root, '.relayflow/pr-body.md').split('\n'); + expect(lines.filter(line => line === 'Fixes #507')).toHaveLength(1); + expect(validate(root, 'Fix login', 'github', '#507').token).toBe('valid'); + }); + + it('refuses placeholder titles and a missing or malformed GitHub closing contract', () => { + const missing = fixture({ '.relayflow/pr-body.md': '## Summary\n' }); + expect(validate(missing, 'Software factory change', 'github', '#507').token).toBe('placeholder-title'); + expect(validate(missing, 'Fix login', 'github', '507').token).toBe('malformed-github-identifier'); + expect(validate(missing, 'Fix login', 'github', '#507').token).toBe('missing-github-closing-reference'); + + const duplicate = fixture({ '.relayflow/pr-body.md': 'Fixes #507\n\nFixes #507\n' }); + expect(validate(duplicate, 'Fix login', 'github', '#507').token).toBe('duplicate-github-closing-reference'); + }); + + it('keeps deterministic non-GitHub references without inventing an issue number', () => { + const linked = fixture({ '.relayflow/pr-body.md': '## Summary\n' }); + prepare(linked, 'Ticket: https://linear.app/acme/issue/ENG-42'); + expect(read(linked, '.relayflow/pr-body.md')).toContain('Ticket: https://linear.app/acme/issue/ENG-42'); + expect(validate(linked, 'Fix login', 'linear', 'ENG-42').token).toBe('valid'); + + const markdown = fixture({ '.relayflow/pr-body.md': '## Summary\n' }); + prepare(markdown, ''); + expect(read(markdown, '.relayflow/pr-body.md')).toBe('## Summary\n'); + expect(validate(markdown, 'tasks.md', 'markdown', '').token).toBe('valid'); + }); +}); From 0e7ccbfb7433885eab6f3d53b0f6471e425cdb2c Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sun, 20 Sep 2026 15:03:36 -0700 Subject: [PATCH 2/2] fix(flows): validate Unicode PR title length --- web/lib/flow-workflows.ts | 10 ++++++---- web/lib/test/flow-onboarding.test.ts | 1 + web/lib/test/flow-workflows.test.ts | 13 +++++++++++-- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/web/lib/flow-workflows.ts b/web/lib/flow-workflows.ts index 90abae7..aa9171b 100644 --- a/web/lib/flow-workflows.ts +++ b/web/lib/flow-workflows.ts @@ -346,7 +346,8 @@ export const FLOW_PREPARE_CHANGE_METADATA_COMMAND = [ export const FLOW_VALIDATE_CHANGE_METADATA_COMMAND = [ 'if [ ! -s .relayflow/pr-body.md ]; then echo missing-body', 'elif [ -z "$title" ]; then echo empty-title', - 'elif [ "${#title}" -gt 240 ]; then echo title-too-long', + '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" .relayflow/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', @@ -441,9 +442,10 @@ export function workflowCode(workflow: WorkflowId, agents: ReturnType { const prepare = calls.find(call => call.endsWith(FLOW_PREPARE_CHANGE_METADATA_COMMAND)) ?? ''; const validate = calls.find(call => call.endsWith(FLOW_VALIDATE_CHANGE_METADATA_COMMAND)) ?? ''; expect(prepare).toContain("reference='Fixes #507'"); + expect(validate).toContain('title_length=9'); expect(validate).toContain("identifier='#507'"); expect(calls.indexOf(validate)).toBeLessThan(calls.indexOf('git push --set-upstream origin HEAD')); expect(createCall(calls)).toBe(FLOW_OPEN_CHANGE_COMMAND + " --title 'Fix login' --body-file .relayflow/pr-body.md"); diff --git a/web/lib/test/flow-workflows.test.ts b/web/lib/test/flow-workflows.test.ts index ed67c8c..df580ee 100644 --- a/web/lib/test/flow-workflows.test.ts +++ b/web/lib/test/flow-workflows.test.ts @@ -646,8 +646,8 @@ describe('FLOW_OPEN_CHANGE_COMMAND', () => { describe('change metadata contract', () => { const prepare = (root: string, reference: string) => sh(`reference='${reference}'; ${FLOW_PREPARE_CHANGE_METADATA_COMMAND}`, root); - const validate = (root: string, title: string, source: string, identifier: string) => - sh(`title='${title}'; source='${source}'; identifier='${identifier}'; ${FLOW_VALIDATE_CHANGE_METADATA_COMMAND}`, root); + const validate = (root: string, title: string, source: string, identifier: string, titleLength = Array.from(title).length) => + sh(`title='${title}'; title_length=${titleLength}; source='${source}'; identifier='${identifier}'; ${FLOW_VALIDATE_CHANGE_METADATA_COMMAND}`, root); it('adds exactly one normalized GitHub closing line and accepts the final artifacts', () => { const root = fixture({ '.relayflow/pr-body.md': '## Summary\n\nImplemented login recovery.\n' }); @@ -668,6 +668,15 @@ describe('change metadata contract', () => { expect(validate(duplicate, 'Fix login', 'github', '#507').token).toBe('duplicate-github-closing-reference'); }); + it('enforces the title cap in Unicode code points rather than UTF-8 bytes', () => { + const root = fixture({ '.relayflow/pr-body.md': 'Fixes #507\n' }); + const atLimit = '修'.repeat(240); + expect(Buffer.byteLength(atLimit, 'utf8')).toBeGreaterThan(240); + expect(validate(root, atLimit, 'github', '#507')).toMatchObject({ code: 0, token: 'valid' }); + expect(validate(root, 'x'.repeat(241), 'github', '#507').token).toBe('title-too-long'); + expect(validate(root, 'Fix login', 'github', '#507', Number.NaN).token).toBe('malformed-title-length'); + }); + it('keeps deterministic non-GitHub references without inventing an issue number', () => { const linked = fixture({ '.relayflow/pr-body.md': '## Summary\n' }); prepare(linked, 'Ticket: https://linear.app/acme/issue/ENG-42');