From 26751dc6ec67258759204d2bf7e577c30ec8aa2a Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Sun, 11 Oct 2026 15:35:37 -0700 Subject: [PATCH 1/2] fix(gh-aw): align lifecycle examples with command-only contract Closes #2201 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9cb2dad2-b638-45e5-840b-6ccc12e94499 --- docs/demo-agentic-sdlc-walkthrough.md | 4 +- docs/src/content/docs/guide/gh-aw.md | 16 ++- test/gh-aw-command-authorization.test.ts | 152 ++++++++++++++++++-- workflows/package/squad-deps-worker.md | 2 +- workflows/package/squad-implement-worker.md | 2 +- workflows/package/squad-retro.md | 2 +- workflows/package/squad.md | 9 +- workflows/shared/squad-planning-ontology.md | 7 +- workflows/shared/squad.md | 2 +- workflows/squad-workflows.manifest.json | 44 +++--- 10 files changed, 194 insertions(+), 46 deletions(-) diff --git a/docs/demo-agentic-sdlc-walkthrough.md b/docs/demo-agentic-sdlc-walkthrough.md index 08310a1f4..815c8f944 100644 --- a/docs/demo-agentic-sdlc-walkthrough.md +++ b/docs/demo-agentic-sdlc-walkthrough.md @@ -439,7 +439,9 @@ with full hierarchy (Root → Epics → Tasks) and assigned to their respective | Activated | ✅ Done | [comment link] | 2026-08-10 | **Current state:** Activated -**Last command:** `/squad plan activate` by @bradygaster at 2026-08-10T00:39:00Z +**Last command:** `/squad plan activate` +**Updated by:** @user at 2026-08-10T00:39:00Z +**Next action:** Track execution in the linked issues. ``` **Key point:** This comment is a living progress bar. It updates itself at every transition. Any agent — human or AI — can look at this one comment and know exactly where in the SDLC this issue stands. No context lost, no ambiguity. diff --git a/docs/src/content/docs/guide/gh-aw.md b/docs/src/content/docs/guide/gh-aw.md index 73323cc2f..19d53403c 100644 --- a/docs/src/content/docs/guide/gh-aw.md +++ b/docs/src/content/docs/guide/gh-aw.md @@ -1377,13 +1377,23 @@ Each step is user-initiated — Squad proposes, you review and approve. After each command, Squad posts (or updates) a **lifecycle state comment** on the issue. This comment shows where you are, what just happened, and what to do next: -``` +```markdown +## Planning Lifecycle + **Current state:** Triaged -**Last command:** `/squad triage` by @user at 2026-08-10 -**Next action:** `/squad plan program` — create a program plan from triage dispositions +**Last command:** `/squad triage` +**Updated by:** @user at 2026-08-10 +**Next action:** `/squad plan program` +**Guidance:** Create a program plan from triage dispositions. **Also available:** `/squad triage revise ` — adjust triage before planning ``` +Use exactly one `Last command` field with only the requested backticked command. +Keep actor/time attribution on the separate `Updated by` line. Open research and +plan commands report `Researched` and `Planned`, respectively, never acceptance +or activation. For nonterminal states, `Next action` is also command-only; use +`Guidance` for explanatory prose. + The `Next action` field tells you (or any agent reading the issue) the primary next command. `Also available` shows alternative valid commands at this point in the lifecycle. This means you never have to remember the state machine — the diff --git a/test/gh-aw-command-authorization.test.ts b/test/gh-aw-command-authorization.test.ts index 97a5d44fe..0386b9b98 100644 --- a/test/gh-aw-command-authorization.test.ts +++ b/test/gh-aw-command-authorization.test.ts @@ -41,11 +41,13 @@ const payloadFor = (command: string, eventName = 'issue_comment') => ({ }); let dispatcher: Lock; let router: Lock; +let installedDispatcher: string; let sanitizeContent: (body: string, options?: { allowedAliases: string[] }) => string; let diagnosticSources: Map; beforeAll(async () => { const fixture = createFirstInstallFixture('a'.repeat(40), 'package'); + installedDispatcher = fixture.consumerFiles.get('.github/workflows/squad.md')!.toString(); dispatcher = parse(fixture.consumerFiles.get('.github/workflows/squad.lock.yml')!.toString()); router = parse(fixture.consumerFiles.get('.github/workflows/squad-command-router.lock.yml')!.toString()); // Execute the actual sanitizer dependency closure at the compiled action pin, @@ -151,6 +153,43 @@ async function interpretLifecycleComment(body?: string) { return mutations; } +function lifecycleFromTemplate(source: string, mode: 'research' | 'plan') { + const template = source.replace(/\r\n/g, '\n') + .match(/```(?:markdown)?\n(## Planning Lifecycle\n[\s\S]*?)\n```/)?.[1]; + expect(template).toBeTruthy(); + return template! + .replace('**Current state:** Triaged', `**Current state:** ${mode === 'research' ? 'Researched' : 'Planned'}`) + .replace('**Last command:** `/squad triage`', `**Last command:** \`/squad ${mode}\``) + .replace('**Next action:** `/squad plan program`', + `**Next action:** \`/squad ${mode === 'research' ? 'triage' : 'plan validate'}\``) + .replaceAll('@user', '@contributor') + .replaceAll('', '2026-01-01T00:00:00Z') + .replaceAll('', '2026-01-01') + .replaceAll('', 'Artifact comment'); +} + +async function writeLifecycle(body: string) { + const job = dispatcher.jobs.upsert_lifecycle_state; + expect(job.if).toContain("needs.safe_outputs.result == 'success'"); + const step = job.steps.find(candidate => candidate.name === 'Upsert Squad lifecycle state')!; + const writes: Record[] = []; + const failures: string[] = []; + writeFileSync(outputPath, JSON.stringify({ items: [{ type: 'upsert_lifecycle_state', body }] })); + await compileFunction(`return (async () => { ${step.with!.script} })();`, + ['github', 'context', 'core', 'process'], + { importModuleDynamically: constants.USE_MAIN_CONTEXT_DEFAULT_LOADER })({ + paginate: async () => [], + rest: { issues: { + listComments: vi.fn(), + createComment: async (params: Record) => writes.push(params), + updateComment: async (params: Record) => writes.push(params), + } }, + }, { repo: { owner: 'owner', repo: 'repo' } }, + { setFailed: (message: string) => failures.push(message) }, + { env: { GH_AW_AGENT_OUTPUT: outputPath, ISSUE_NUMBER: '42' } }); + return { writes, failures }; +} + describe('compiled Squad command authorization boundary (#2185)', () => { function assertDiagnosticIsolation(lock: Lock) { const steps = lock.jobs.conclusion.steps; @@ -380,22 +419,109 @@ describe('compiled Squad command authorization boundary (#2185)', () => { } }); - it('permits research/plan artifacts and only their nonterminal lifecycle states', async () => { - for (const mode of ['research', 'plan']) { - const item = mode === 'research' - ? { type: 'upsert_research_artifact', body: '## Squad Research\nResearch content' } - : { type: 'add_comment', body: '## Plan', data: { - squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [], - } }; - const lifecycle = { type: 'upsert_lifecycle_state', - body: `## Squad Lifecycle\n- **State:** ${mode === 'research' ? 'Researched' : 'Planned'}\n- **Last command:** \`/squad ${mode}\`\n- **Next action:** \`/squad triage\`` }; - expect((await runGuard({ payload: payloadFor(mode), permission: 'read', items: [item, lifecycle] })).failures).toEqual([]); - expect((await runGuard({ payload: payloadFor(mode), items: [ - { ...lifecycle, body: lifecycle.body.replace(/Researched|Planned/, 'Activated') }, - ] })).failures).toHaveLength(1); + it.each(['research', 'plan'] as const)('permits template-derived %s batches through the guard and writer', async mode => { + const item = mode === 'research' + ? { type: 'upsert_research_artifact', body: '## Squad Research\nResearch content' } + : { type: 'add_comment', body: '## Plan', data: { + squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [], + } }; + for (const source of [ + readFileSync(join(root, 'workflows/shared/squad-planning-ontology.md'), 'utf8'), + installedDispatcher, + readFileSync(join(root, 'docs/src/content/docs/guide/gh-aw.md'), 'utf8'), + ]) { + const body = lifecycleFromTemplate(source, mode); + const lifecycle = { type: 'upsert_lifecycle_state', body }; + const result = await runGuard({ payload: payloadFor(mode), permission: 'read', items: [item, lifecycle] }); + expect(result.failures).toEqual([]); + expect(result.permissionLookup).not.toHaveBeenCalled(); + const written = await writeLifecycle(body); + expect(written.failures).toEqual([]); + expect(written.writes).toHaveLength(1); + expect(written.writes[0].body).toBe(`${body}\n\nStructured data:\n\n\`\`\`json\n${JSON.stringify({ + squad_artifact: 'lifecycle-state', schema_version: '1', origin_issue: 42, phases: [], + })}\n\`\`\``); + const commandLine = `**Last command:** \`/squad ${mode}\``; + for (const invalid of [ + body.replace(/Researched|Planned/, 'Activated'), + body.replace(/Researched|Planned/, 'ScopeAccepted'), + body.replace(/Researched|Planned/, mode === 'research' ? 'Planned' : 'Researched'), + body.replace(commandLine, '**Last command:** `/squad activate`'), + body.replace(commandLine, `${commandLine} by @contributor at 2026-01-01T00:00:00Z`), + body.replace(commandLine, ''), + `${body}\n${commandLine}`, + `${body}\n- ${commandLine}`, + `${body}\n**State:** Activated`, + ]) { + const denied = await runGuard({ payload: payloadFor(mode), permission: 'read', + items: [item, { ...lifecycle, body: invalid }] }); + expect(denied.failures).toHaveLength(1); + expect(denied.failures[0]).toContain('open-mode lifecycle'); + expect(denied.mutations).toEqual([]); + } } }); + it('emits the lifecycle source import and strict tool contract in the compiled artifact', () => { + const compiled = JSON.stringify(dispatcher); + expect(compiled).toContain('{{#runtime-import .github/workflows/squad.md}}'); + expect(installedDispatcher).toContain('**Last command:** `/squad triage`\n**Updated by:** @user at '); + expect(compiled).toContain('Include exactly one last-command field containing only the backticked /squad command'); + expect(installedDispatcher).not.toContain('**Last command:** `/squad triage` by @user'); + }); + + it('rejects both open-mode batches when the live template regains its original attribution suffix', async () => { + const fixture = createFirstInstallFixture('a'.repeat(40), 'package'); + const mutationRoot = join(scratch, 'lifecycle-template-mutant'); + for (const [path, bytes] of fixture.consumerFiles) { + const destination = join(mutationRoot, path); + mkdirSync(dirname(destination), { recursive: true }); + writeFileSync(destination, bytes); + } + const sourcePath = join(mutationRoot, '.github/workflows/squad.md'); + const original = readFileSync(sourcePath, 'utf8'); + const mutant = original.replace( + '**Last command:** `/squad triage`\n**Updated by:** @user at ', + '**Last command:** `/squad triage` by @user at ', + ); + expect(mutant).not.toBe(original); + writeFileSync(sourcePath, mutant); + execFileSync('git', ['init', '--quiet'], { cwd: mutationRoot }); + compileWithPinnedActions(mutationRoot); + const lock = readFileSync(join(mutationRoot, '.github/workflows/squad.lock.yml'), 'utf8'); + expect(lock).toContain('{{#runtime-import .github/workflows/squad.md}}'); + expect(readFileSync(sourcePath, 'utf8')) + .toContain('**Last command:** `/squad triage` by @user at '); + const realDispatcher = dispatcher; + try { + dispatcher = parse(lock); + for (const mode of ['research', 'plan'] as const) { + const artifact = mode === 'research' + ? { type: 'upsert_research_artifact', body: '## Squad Research\nResearch content' } + : { type: 'add_comment', body: '## Plan', data: { + squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [], + } }; + const check = async (source: string) => runGuard({ + payload: payloadFor(mode), permission: 'read', items: [ + artifact, { type: 'upsert_lifecycle_state', body: lifecycleFromTemplate(source, mode) }, + ], + }); + const assertAccepted = (result: Awaited>) => { + expect(result.failures).toEqual([]); + }; + assertAccepted(await check(original)); + const denied = await check(readFileSync(sourcePath, 'utf8')); + expect(() => assertAccepted(denied)).toThrow(); + expect(denied.failures).toEqual([ + 'Squad open-mode lifecycle must describe only the requested research or plan.', + ]); + expect(denied.mutations).toEqual([]); + } + } finally { + dispatcher = realDispatcher; + } + }, 120_000); + it.each([ ['tilde fence', '~~~~text', '~~~~', '\n'], ['backtick fence', '````text', '````', '\n'], diff --git a/workflows/package/squad-deps-worker.md b/workflows/package/squad-deps-worker.md index 67a946b08..a387db064 100644 --- a/workflows/package/squad-deps-worker.md +++ b/workflows/package/squad-deps-worker.md @@ -152,7 +152,7 @@ safe-outputs: output: Lifecycle state updated. inputs: body: - description: Complete lifecycle Markdown with an H2 lifecycle heading plus state, last-command, and next-action fields. For a nonterminal state, the next-action value must consist of a backticked /squad command; put explanatory prose in a separate field. Structured data is normalized by the writer. + description: Lifecycle Markdown with an H2 lifecycle heading, state, last-command, and next-action fields. Include exactly one last-command field containing only the backticked /squad command requested; put optional actor/time attribution on a separate Updated by line. For nonterminal states, next-action must contain only a backticked /squad command; put explanatory prose in a separate field. The writer normalizes structured data. required: true type: string steps: diff --git a/workflows/package/squad-implement-worker.md b/workflows/package/squad-implement-worker.md index 219cb2db9..c21bdee5f 100644 --- a/workflows/package/squad-implement-worker.md +++ b/workflows/package/squad-implement-worker.md @@ -152,7 +152,7 @@ safe-outputs: output: Lifecycle state updated. inputs: body: - description: Complete lifecycle Markdown with an H2 lifecycle heading plus state, last-command, and next-action fields. For a nonterminal state, the next-action value must consist of a backticked /squad command; put explanatory prose in a separate field. Structured data is normalized by the writer. + description: Lifecycle Markdown with an H2 lifecycle heading, state, last-command, and next-action fields. Include exactly one last-command field containing only the backticked /squad command requested; put optional actor/time attribution on a separate Updated by line. For nonterminal states, next-action must contain only a backticked /squad command; put explanatory prose in a separate field. The writer normalizes structured data. required: true type: string steps: diff --git a/workflows/package/squad-retro.md b/workflows/package/squad-retro.md index 148d825f9..2d04d6416 100644 --- a/workflows/package/squad-retro.md +++ b/workflows/package/squad-retro.md @@ -152,7 +152,7 @@ safe-outputs: output: Lifecycle state updated. inputs: body: - description: Complete lifecycle Markdown with an H2 lifecycle heading plus state, last-command, and next-action fields. For a nonterminal state, the next-action value must consist of a backticked /squad command; put explanatory prose in a separate field. Structured data is normalized by the writer. + description: Lifecycle Markdown with an H2 lifecycle heading, state, last-command, and next-action fields. Include exactly one last-command field containing only the backticked /squad command requested; put optional actor/time attribution on a separate Updated by line. For nonterminal states, next-action must contain only a backticked /squad command; put explanatory prose in a separate field. The writer normalizes structured data. required: true type: string steps: diff --git a/workflows/package/squad.md b/workflows/package/squad.md index 6b4faacb5..e6150330d 100644 --- a/workflows/package/squad.md +++ b/workflows/package/squad.md @@ -152,7 +152,7 @@ safe-outputs: output: Lifecycle state updated. inputs: body: - description: Complete lifecycle Markdown with an H2 lifecycle heading plus state, last-command, and next-action fields. For a nonterminal state, the next-action value must consist of a backticked /squad command; put explanatory prose in a separate field. Structured data is normalized by the writer. + description: Lifecycle Markdown with an H2 lifecycle heading, state, last-command, and next-action fields. Include exactly one last-command field containing only the backticked /squad command requested; put optional actor/time attribution on a separate Updated by line. For nonterminal states, next-action must contain only a backticked /squad command; put explanatory prose in a separate field. The writer normalizes structured data. required: true type: string steps: @@ -1359,12 +1359,17 @@ This comment is created on first transition and updated on every subsequent tran | Activated | ⬚ Pending | — | — | **Current state:** Triaged -**Last command:** `/squad triage` by @user at +**Last command:** `/squad triage` +**Updated by:** @user at **Next action:** `/squad plan program` **Guidance:** Create a program plan from triage dispositions. **Also available:** `/squad triage revise ` — adjust triage before planning ``` +Use exactly one `Last command`, containing only the requested backticked command. +Keep actor/time attribution on `Updated by`. Open research/plan commands use +`Researched`/`Planned`, never acceptance or activation. + For nonterminal states, `Next action` is only the backticked `/squad` command; put prose elsewhere. `Activated` is terminal and may use terminal prose there. Its canonical granular command is `/squad plan activate`. diff --git a/workflows/shared/squad-planning-ontology.md b/workflows/shared/squad-planning-ontology.md index 7f95764e8..14891a317 100644 --- a/workflows/shared/squad-planning-ontology.md +++ b/workflows/shared/squad-planning-ontology.md @@ -402,12 +402,17 @@ This comment is created on first transition and updated on every subsequent tran | Activated | ⬚ Pending | — | — | **Current state:** Triaged -**Last command:** `/squad triage` by @user at +**Last command:** `/squad triage` +**Updated by:** @user at **Next action:** `/squad plan program` **Guidance:** Create a program plan from triage dispositions. **Also available:** `/squad triage revise ` — adjust triage before planning ``` +Use exactly one `Last command`, containing only the requested backticked command. +Keep actor/time attribution on `Updated by`. Open research/plan commands use +`Researched`/`Planned`, never acceptance or activation. + For nonterminal states, `Next action` is only the backticked `/squad` command; put prose elsewhere. `Activated` is terminal and may use terminal prose there. Its canonical granular command is `/squad plan activate`. diff --git a/workflows/shared/squad.md b/workflows/shared/squad.md index ebcacb794..2a9150636 100644 --- a/workflows/shared/squad.md +++ b/workflows/shared/squad.md @@ -188,7 +188,7 @@ safe-outputs: output: Lifecycle state updated. inputs: body: - description: Complete lifecycle Markdown with an H2 lifecycle heading plus state, last-command, and next-action fields. For a nonterminal state, the next-action value must consist of a backticked /squad command; put explanatory prose in a separate field. Structured data is normalized by the writer. + description: Lifecycle Markdown with an H2 lifecycle heading, state, last-command, and next-action fields. Include exactly one last-command field containing only the backticked /squad command requested; put optional actor/time attribution on a separate Updated by line. For nonterminal states, next-action must contain only a backticked /squad command; put explanatory prose in a separate field. The writer normalizes structured data. required: true type: string steps: diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index 460a7f754..9363064fb 100644 --- a/workflows/squad-workflows.manifest.json +++ b/workflows/squad-workflows.manifest.json @@ -14,18 +14,18 @@ "source": "workflows/package/squad.md", "destination": ".github/workflows/squad.md", "lock": ".github/workflows/squad.lock.yml", - "source_sha256": "d389941522f738c3a1f61f72ffef1d4254d19a8be722380761dd2db964253c25", - "lock_sha256": "8e28f4005512d986bc986a8fc120a3bbda64aa9f8a777e83805e9b24f234741f", - "package_lock_sha256": "57a400d43fd8862a2e827140c929048519eaf39e0f9b3e6e50733d86d1bec90a" + "source_sha256": "85f4fab3f545d1d2e2c8aabc511118e4d34b9bd2e008e4a4e0756d7f8fae9bf6", + "lock_sha256": "3c447de8a7998d659ef6a4155e49b7c4785ac2e8361a1d854ffbd5acd258348b", + "package_lock_sha256": "8823d25b1fc8aed5981d14363a3311a381d7207b89105203a1202dc6f78c2c78" }, { "name": "squad-implement-worker", "source": "workflows/package/squad-implement-worker.md", "destination": ".github/workflows/squad-implement-worker.md", "lock": ".github/workflows/squad-implement-worker.lock.yml", - "source_sha256": "9e34eb6d59478e18da65f7aaa19e29110abb3393028a706c59091d55ca24fb6a", - "lock_sha256": "d40e879f6eeb79b3ddc7c0fa81ad2dbec8cb761dae1a16b17042fc81088026af", - "package_lock_sha256": "aa0c05a81f76c8f001d19ffdbe76765a6dbbaca464a58346fb223b93809b1902" + "source_sha256": "adf3d810ae789453eef68c8ef168dd6327f2d0985ae0602ee91890ef1d7b81a9", + "lock_sha256": "6d209c356856b9aada9e3befb640517dd171b8ba39761c48686667907ebc2822", + "package_lock_sha256": "06a32b8bbda47b26182a82e16fca5be8ce7844a6098e1b13ab3c2daaf6360faf" }, { "name": "squad-review", @@ -41,18 +41,18 @@ "source": "workflows/package/squad-deps-worker.md", "destination": ".github/workflows/squad-deps-worker.md", "lock": ".github/workflows/squad-deps-worker.lock.yml", - "source_sha256": "d48c640b39baffc8f8ed5ead9d675e86cb5788f1177cf5827cdcf5e69732a353", - "lock_sha256": "68c9ee78cf91e75d700a8ddca40faa6b6d77728c25b43f676bafd8e47b4bc5d9", - "package_lock_sha256": "bbac67cbf1d9d88eeddb5d7cd44fec0b1c759db1c5f4d9361b45391c03384c67" + "source_sha256": "04c0135625218f5321bab8af5a0bac3b2a60434fb2364582b4ab2f8e1e570804", + "lock_sha256": "03cee66ac9ddf291dc5dc856757c9fb10d06d525e8fc349179892f94bb1560b3", + "package_lock_sha256": "57883c07743764e1f9fd7732e7959867661d388793a01c30dc008ed36c4d13a9" }, { "name": "squad-retro", "source": "workflows/package/squad-retro.md", "destination": ".github/workflows/squad-retro.md", "lock": ".github/workflows/squad-retro.lock.yml", - "source_sha256": "bf2e81e0d4f9c6498a5546f6a8118fa0dca1f85fdfba248365a1a3b35031ab07", - "lock_sha256": "be7fa80936260ee6ac8cd8f73c3e7682d15c5b3040e95af5a494567ba64a55b0", - "package_lock_sha256": "a4fc8b5fe055f3810ce60e7913ecea55bd2a07cc21031bc57b2d04b6e7a90103" + "source_sha256": "b511ba1b2e28db26efbb5ad7441b361dfcf96bdb489b01e61ea6b523fa1eba22", + "lock_sha256": "95a77ca182f00b6cd1b438fb862310c5a80a5d2624b3accd1410a7aa1e12fec8", + "package_lock_sha256": "716097c0d07d763b0998def79ddd14c7894e5dbf36660949fa3c7e2e10205ba9" }, { "name": "squad-improvement-worker", @@ -88,13 +88,13 @@ "workflows": [ { "name": "squad", - "lock_sha256": "8e28f4005512d986bc986a8fc120a3bbda64aa9f8a777e83805e9b24f234741f", - "package_lock_sha256": "57a400d43fd8862a2e827140c929048519eaf39e0f9b3e6e50733d86d1bec90a" + "lock_sha256": "3c447de8a7998d659ef6a4155e49b7c4785ac2e8361a1d854ffbd5acd258348b", + "package_lock_sha256": "8823d25b1fc8aed5981d14363a3311a381d7207b89105203a1202dc6f78c2c78" }, { "name": "squad-implement-worker", - "lock_sha256": "3c81adb77221dd0aae165f0cc2213dda37de13e41bcf061600f19058f28c1228", - "package_lock_sha256": "1bad10ffd74023be48eabfa20b476d61f0a481e1663195c4c12e488485f574dd" + "lock_sha256": "0dbb677dfe011e2bc45e0139dd160f160c79f60aa3c70cc44ec8a33d3b746ccf", + "package_lock_sha256": "c3d099614d956765351f9eb77ba2677280f6874811bf3a1f65a762f7a8553b7d" }, { "name": "squad-review", @@ -103,13 +103,13 @@ }, { "name": "squad-deps-worker", - "lock_sha256": "cc87ee3dfc3b199e3660cae6c8e83d971f6e76f56bc62d8cfba5ecd4a3ad8d7d", - "package_lock_sha256": "18501e1cdd156d3edc6a0eaae38fb519acd76aa962bdeb35a4dd19a79c60a18c" + "lock_sha256": "22c338f2cdad4cc3f8a5d04330ea482c35b147f12fa96de42cced2536c4979ac", + "package_lock_sha256": "8122bd80347464d301150c42dc5ec760fcc0d6388a22c3d125fcc9245adf60b5" }, { "name": "squad-retro", - "lock_sha256": "72530ac82909ae67f9aeb2a705c812a8b77e0db1b11e66a1fe8f083293387b1b", - "package_lock_sha256": "40b4cf1f357011fa8ab7bf638123982898e6a7fca7322a2afe58fc3cb63b040c" + "lock_sha256": "97baa50c5cd9616674b8a8793e560469b2713e1bac326ddc2467e6674b156b60", + "package_lock_sha256": "795ece98cb5a645f11292665a27d3c5181e8b41a0f936e538f05dbd52edb5322" }, { "name": "squad-improvement-worker", @@ -208,7 +208,7 @@ "package_destination": ".github/aw/squad/runtime/shared/squad.md", "destination": ".github/workflows/shared/squad.md", "ownership": "manifest", - "sha256": "d09adeac29dca305a7b37b81ca8886e81e985c05299d1a75e6dee6edd072311f" + "sha256": "812f86d56eb984f65c6c31572d9697f32e80452f7f02aff885eaf2516447d48a" }, { "path": "shared/squad-planning-ontology.md", @@ -216,7 +216,7 @@ "package_destination": ".github/aw/squad/runtime/shared/squad-planning-ontology.md", "destination": ".github/workflows/shared/squad-planning-ontology.md", "ownership": "manifest", - "sha256": "947cbd1514e22e55ee0ba6d9404b6a3f9ed419c9cfecbb0ecf6c7106b139b1d4" + "sha256": "c4037fb6c3a0eec6a74fc48a3506daba5c27e249df61f6b519865e0efa617af4" }, { "path": "shared/squad-planning-policy.md", From 91ab258d582ce784e6c9fa8cf426d9d22ce972c3 Mon Sep 17 00:00:00 2001 From: Brady Gaster Date: Sun, 11 Oct 2026 16:21:15 -0700 Subject: [PATCH 2/2] fix(gh-aw): validate serialized plan comment metadata and envelopes Closes #2201 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9cb2dad2-b638-45e5-840b-6ccc12e94499 --- docs/src/content/docs/guide/gh-aw.md | 19 ++ test/gh-aw-command-authorization.test.ts | 287 +++++++++++++++++++- test/helpers/gh-aw-diagnostic-runtime.ts | 41 ++- workflows/shared/squad-command-contract.mjs | 42 ++- workflows/squad-workflows.manifest.json | 2 +- 5 files changed, 372 insertions(+), 19 deletions(-) diff --git a/docs/src/content/docs/guide/gh-aw.md b/docs/src/content/docs/guide/gh-aw.md index 19d53403c..a30e45ab4 100644 --- a/docs/src/content/docs/guide/gh-aw.md +++ b/docs/src/content/docs/guide/gh-aw.md @@ -1000,6 +1000,25 @@ comment body. This restriction includes HTML comments and quoted/code regions: gh-aw preserves HTML comments inside code fences, and lifecycle readers inspect the complete posted text rather than its rendered visibility. +This tool-input rule is distinct from serialized output. The pinned gh-aw +v0.91.5 `add_comment` adapter adds a `temporary_id`; its collector appends the +`Structured data:` JSON suffix while retaining `data`. The guard accepts only +a string ID in the adapter's bare `aw_` format (3–12 alphanumeric or underscore +characters), not other mutation fields. For serialized plan comments, it first +validates the exact draft-data schema, originating issue, empty phases, and +allowed artifact kind (`plan`, `program`, `implementation`, or `validation`), +then requires exactly one terminal envelope semantically equal to that data. +Missing, malformed, duplicate, mismatching, or extra-field envelopes fail +closed. All remaining prose still receives the full body check. The envelope +and `data` are preserved for downstream planning readers, not stripped from +the output. Neither metadata nor markers grant authority. + +Raw Markdown-only comments without serializer metadata remain valid, with an +optional independently validated plan `data` field. Do not hand-author the +serialized suffix or rely on a `temporary_id` to authorize a comment. Custom +research and lifecycle tools continue to accept only their declared Markdown +body; they do not gain this built-in comment serialization exception. + The guard validates the entire emitted batch before the built-in handlers run. Research/lifecycle custom jobs and terminal lifecycle repair require a successful safe-output job. The command router permits no agent-authored mutations; only diff --git a/test/gh-aw-command-authorization.test.ts b/test/gh-aw-command-authorization.test.ts index 0386b9b98..039c1ff9f 100644 --- a/test/gh-aw-command-authorization.test.ts +++ b/test/gh-aw-command-authorization.test.ts @@ -79,9 +79,24 @@ beforeAll(async () => { return module.exports; }; sanitizeContent = load('sanitize_content.cjs').sanitizeContent; - diagnosticSources = await fetchDiagnosticRuntime(); + diagnosticSources = await fetchDiagnosticRuntime([ + 'mcp_server_core.cjs', 'safe_outputs_mcp_arguments.cjs', 'safe_outputs_bootstrap.cjs', + 'safe_outputs_handlers.cjs', 'safe_outputs_append.cjs', 'generate_safe_outputs_tools.cjs', + 'safe_outputs_tools.json', 'collect_ndjson_output.cjs', 'process_safe_outputs.cjs', 'add_comment.cjs', + ]); mkdirSync(trusted, { recursive: true }); cpSync(join(root, 'workflows/shared/squad-command-contract.mjs'), join(trusted, 'squad-command-contract.mjs')); + const evidence = process.env.SQUAD_CONTRACT_EVIDENCE_DIR; + if (evidence) { + for (const [path, bytes] of [ + ...fixture.consumerFiles, + ...[...diagnosticSources].map(([path, bytes]) => [`runtime/${path}`, bytes] as const), + ]) { + const destination = join(evidence, path); + mkdirSync(dirname(destination), { recursive: true }); + writeFileSync(destination, bytes); + } + } }, 180_000); afterAll(() => rmSync(scratch, { recursive: true, force: true })); @@ -190,7 +205,277 @@ async function writeLifecycle(body: string) { return { writes, failures }; } +function compiledEnv(step: Step) { + return Object.fromEntries(Object.entries(step.env || {}).filter(([, value]) => !value.includes('${{'))); +} + +async function serializeRequests(requests: { name: string; arguments: Item }[]) { + const configStep = dispatcher.jobs.agent.steps.find(step => step.name === 'Generate Safe Outputs Config')!; + const toolsStep = dispatcher.jobs.agent.steps.find(step => step.name === 'Generate Safe Outputs Tools')!; + const env = { + ...compiledEnv(toolsStep), + GH_AW_SAFE_OUTPUTS_CONFIG_PATH: `${scratch}/gh-aw/safeoutputs/config.json`, + GH_AW_SAFE_OUTPUTS: `${scratch}/gh-aw/safeoutputs/outputs.jsonl`, + }; + const runtime = diagnosticRuntime(diagnosticSources, scratch, [], env); + runtime.files.set(env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH, configStep.env!.GH_AW_SAFE_OUTPUTS_CONFIG!); + runtime.files.set(`${scratch}/gh-aw/actions/safe_outputs_tools.json`, + diagnosticSources.get('js/safe_outputs_tools.json')!); + await runtime.runScript(toolsStep.with!.script!); + + // The pinned server's real registration, argument validation, handlers, NDJSON + // appender and collector execute unchanged; only runner/GitHub IO is substituted. + const mcp = runtime.load('js/mcp_server_core.cjs'); + const argumentsModule = runtime.load('js/safe_outputs_mcp_arguments.cjs'); + const server = mcp.createServer({ name: 'safeoutputs', version: '1.0.0' }, { + normalizeArguments: (name: string, args: Item, tool: Item) => + argumentsModule.normalizeSafeOutputToolArguments(name, args, server, tool.inputSchema), + }); + const { config, outputFile, tools } = runtime.load('js/safe_outputs_bootstrap.cjs') + .bootstrapSafeOutputsServer(server); + const handlers = runtime.load('js/safe_outputs_handlers.cjs').createHandlers(server, + runtime.load('js/safe_outputs_append.cjs').createAppendFunction(outputFile), config); + const loader = runtime.load('js/safe_outputs_tools_loader.cjs'); + const attached = loader.attachHandlers(tools, handlers, server); + loader.registerPredefinedTools(server, attached, config, mcp.registerTool, mcp.normalizeTool); + loader.registerDynamicTools(server, attached, config, outputFile, mcp.registerTool, mcp.normalizeTool); + const responses = []; + for (const [index, params] of requests.entries()) { + responses.push(await mcp.handleRequest(server, { jsonrpc: '2.0', id: index + 1, method: 'tools/call', params }, + handlers.defaultHandler)); + } + const ndjson = runtime.files.get(outputFile); + await runtime.run('collect_ndjson_output.cjs'); + const constants = runtime.load('js/constants.cjs'); + const output = JSON.parse(runtime.files.get(`${constants.TMP_GH_AW_PATH}/${constants.AGENT_OUTPUT_FILENAME}`)!); + expect(runtime.failures).toEqual([]); + expect(runtime.ioErrors).toEqual([]); + return { output, responses, ndjson }; +} + +async function writeComments(items: Item[]) { + const step = dispatcher.jobs.safe_outputs.steps.find(step => step.id === 'process_safe_outputs')!; + const runtime = diagnosticRuntime(diagnosticSources, scratch, items, { + ...compiledEnv(step), + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: step.env!.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG! + .replaceAll('${{ github.event.repository.default_branch }}', 'dev'), + }); + await runtime.runScript(step.with!.script!); + expect(runtime.ioErrors).toEqual([]); + expect(runtime.failures).toEqual([]); + return runtime; +} + describe('compiled Squad command authorization boundary (#2185)', () => { + it('accepts real pinned MCP requests through serialized plan output and the compiled writers', async () => { + const data = { squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [] }; + const body = lifecycleFromTemplate(installedDispatcher, 'plan').replace('@contributor', 'Contributor'); + const requests = [ + { name: 'add_comment', arguments: { body: '## Plan\nDraft implementation tasks.', data, item_number: 42 } }, + { name: 'upsert_lifecycle_state', arguments: { body } }, + ]; + const { output, responses, ndjson } = await serializeRequests(requests); + expect(responses.every(response => !response.error && !response.result.isError)).toBe(true); + expect(output.errors).toEqual([]); + expect(output.items).toHaveLength(2); + const [comment, lifecycle] = output.items; + expect(Object.keys(comment).sort()).toEqual(['body', 'data', 'item_number', 'temporary_id', 'type']); + expect(comment.temporary_id).toMatch(/^aw_[A-Za-z0-9]{8}$/); + expect(comment.data).toEqual(data); + expect(comment.body).toBe(`${requests[0].arguments.body}\n\nStructured data:\n\`\`\`json\n${JSON.stringify(data, null, 2)}\n\`\`\``); + expect(lifecycle).toEqual({ type: 'upsert_lifecycle_state', body }); + const result = await runGuard({ payload: payloadFor('plan'), permission: 'read', items: output.items }); + const withoutMetadata = structuredClone(output.items); + delete withoutMetadata[0].temporary_id; + const metadataOnlyCheck = await runGuard({ + payload: payloadFor('plan'), permission: 'read', items: withoutMetadata, + }); + const evidence = process.env.SQUAD_CONTRACT_EVIDENCE_DIR; + if (evidence) { + mkdirSync(evidence, { recursive: true }); + writeFileSync(join(evidence, 'adapter-round-trip.json'), JSON.stringify({ + requests, responses, ndjson, output, guard: result, withoutMetadata: metadataOnlyCheck, + }, null, 2)); + } + expect(result.failures).toEqual([]); + expect(metadataOnlyCheck.failures).toHaveLength(1); + expect(result.permissionLookup).not.toHaveBeenCalled(); + const posted = await writeComments(output.items); + expect(posted.writes).toHaveLength(1); + expect(posted.writes[0].params.body).toContain(comment.body); + expect(posted.writes[0].params.issue_number).toBe(42); + expect(await interpretLifecycleComment(String(posted.writes[0].params.body))).toEqual([]); + const written = await writeLifecycle(lifecycle.body); + expect(written.failures).toEqual([]); + expect(written.writes).toHaveLength(1); + expect(written.writes[0].body).toContain('**Current state:** Planned'); + if (evidence) writeFileSync(join(evidence, 'writer-results.json'), + JSON.stringify({ comments: posted.writes, lifecycle: written.writes }, null, 2)); + }); + + it('accepts supported metadata and semantically equal draft envelopes, retaining raw Markdown compatibility', async () => { + for (const artifact of ['plan', 'program', 'implementation', 'validation']) { + const data = { squad_artifact: artifact, schema_version: '1', origin_issue: 42, phases: [] }; + const request = { name: 'add_comment', arguments: { body: '## Draft\nImplementation tasks.', data, item_number: 42 } }; + const { output } = await serializeRequests([request]); + expect(output.errors).toEqual([]); + const [comment] = output.items; + const reordered = JSON.stringify({ phases: [], origin_issue: 42, schema_version: '1', squad_artifact: artifact }); + for (const item of [ + comment, + { ...comment, body: comment.body.replace(JSON.stringify(data, null, 2), reordered) }, + { ...comment, body: comment.body.replaceAll('\n', '\r\n') }, + { type: 'add_comment', ...request.arguments }, + ]) { + const result = await runGuard({ payload: payloadFor('plan'), permission: 'read', items: [item] }); + expect(result.failures).toEqual([]); + const posted = await writeComments([item]); + expect(posted.writes).toHaveLength(1); + // Raw fixtures still satisfy the guard, but the real collector is what + // materializes data into the body consumed by the comment writer. + if ('temporary_id' in item) { + const body = String(posted.writes[0].params.body); + expect((body.match(/Structured data:/g) || [])).toHaveLength(1); + expect(JSON.parse(body.match(/Structured data:\s*```json\s*([\s\S]*?)```/)![1])).toEqual(data); + } + } + } + for (const temporary_id of ['aw_abc', 'aw_task_12345']) { + const { output, responses } = await serializeRequests([{ + name: 'add_comment', arguments: { body: 'Status: ready.', item_number: 42, temporary_id }, + }]); + expect(responses[0].error).toBeUndefined(); + expect(output.errors).toEqual([]); + expect(output.items[0].temporary_id).toBe(temporary_id); + for (const mode of ['status', 'review', 'research', 'plan']) { + expect((await runGuard({ payload: payloadFor(mode), permission: 'read', items: output.items })).failures) + .toEqual([]); + } + } + }); + + it('rejects missing, malformed, mismatching, duplicate and tampered serialized envelopes before writers', async () => { + const data = { squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [] }; + const { output } = await serializeRequests([{ + name: 'add_comment', arguments: { body: '## Plan\nDraft tasks.', data, item_number: 42 }, + }]); + expect(output.errors).toEqual([]); + const [comment] = output.items; + const envelope = comment.body.slice(comment.body.indexOf('Structured data:')); + const replaceData = (value: unknown) => ({ + ...comment, body: comment.body.replace(JSON.stringify(data, null, 2), JSON.stringify(value)), + }); + const cases: [string, Item][] = [ + ['missing envelope', { ...comment, body: '## Plan\nDraft tasks.' }], + ['missing data', { ...comment, data: undefined }], + ['missing metadata', { ...comment, temporary_id: undefined }], + ['malformed JSON', { ...comment, body: comment.body.replace('"schema_version":', '"schema_version"') }], + ['unclosed envelope', { ...comment, body: comment.body.slice(0, -3) }], + ['unlabelled JSON', { ...comment, body: comment.body.replace('Structured data:\n', '') }], + ['nonterminal envelope', { ...comment, body: `${comment.body}\nMore prose` }], + ['extra envelope field', replaceData({ ...data, command: '/squad activate' })], + ['duplicate JSON key', { ...comment, body: comment.body.replace('"squad_artifact":', '"squad_artifact":"activated",\n"squad_artifact":') }], + ['mismatching allowed draft', replaceData({ ...data, squad_artifact: 'program' })], + ['mismatching origin', replaceData({ ...data, origin_issue: 43 })], + ['mismatching phases', replaceData({ ...data, phases: [1] })], + ['mismatching version type', replaceData({ ...data, schema_version: 1 })], + ['tampered item data', { ...comment, data: { ...data, squad_artifact: 'program' } }], + ['array envelope', replaceData([data])], + ['non-string body', { ...comment, body: { text: comment.body } }], + ['duplicate envelope', { ...comment, body: `${comment.body}\n\n${envelope}` }], + ['hidden envelope', { ...comment, body: `\n\n${comment.body}` }], + ['code-region envelope', { ...comment, body: `~~~~text\n${envelope}\n~~~~\n\n${comment.body}` }], + ['arbitrary JSON', { ...comment, body: `\`\`\`json\n{"other":true}\n\`\`\`\n\n${comment.body}` }], + ['unsupported metadata', { ...comment, serializer: 'gh-aw' }], + ...[null, false, 42, {}, [], ['aw_abc'], '', 'aw_ab', 'aw_abcdefghijklmn', + '#aw_abc', 'AW_abc', 'aw_bad-id', ' aw_abc', 'aw_abc\n'].map(value => + [`invalid temporary_id ${JSON.stringify(value)}`, { ...comment, temporary_id: value }] as [string, Item]), + ]; + for (const [name, item] of cases) { + const result = await runGuard({ payload: payloadFor('plan'), permission: 'read', items: [item] }); + expect(result.failures, name).toHaveLength(1); + expect(result.mutations, name).toEqual([]); + } + for (const temporary_id of [42, false, {}, ['aw_abc']]) { + const { responses, ndjson } = await serializeRequests([{ + name: 'add_comment', arguments: { body: 'Draft.', data, item_number: 42, temporary_id }, + }]); + expect(responses[0].error).toBeDefined(); + expect(ndjson).toBeUndefined(); + } + }); + + it('never treats serializer metadata or a matching envelope as command, actor, target or lifecycle authority', async () => { + const data = { squad_artifact: 'plan', schema_version: '1', origin_issue: 42, phases: [] }; + const lifecycle = lifecycleFromTemplate(installedDispatcher, 'plan').replace('@contributor', 'Contributor'); + const { output } = await serializeRequests([ + { name: 'add_comment', arguments: { body: '## Plan\nDraft tasks.', data, item_number: 42 } }, + { name: 'upsert_lifecycle_state', arguments: { body: lifecycle } }, + ]); + expect(output.errors).toEqual([]); + const [comment, state] = output.items; + for (const request of [ + { name: 'add_comment', arguments: { body: '## Draft', data, item_number: 43 } }, + { name: 'add_labels', arguments: { labels: ['squad'], item_number: 42 } }, + ]) { + const serialized = await serializeRequests([request]); + expect(serialized.responses[0].error).toBeUndefined(); + expect(serialized.output.errors).toEqual([]); + expect(serialized.output.items).toHaveLength(1); + const result = await runGuard({ payload: payloadFor('plan'), permission: 'read', items: serialized.output.items }); + expect(result.failures).toHaveLength(1); + expect(result.mutations).toEqual([]); + } + for (const unauthorizedData of [ + ...['activated', 'scope-accepted', 'impl-accepted', 'plan-accepted', 'phases-activated', 'lifecycle-state'] + .map(squad_artifact => ({ ...data, squad_artifact })), + { ...data, origin_issue: 43 }, { ...data, origin_issue: '42' }, { ...data, schema_version: 1 }, + { ...data, phases: [1] }, { ...data, phases: {} }, { ...data, squad_owner: 'unlisted-agent' }, + { ...data, command: 'activate' }, [data], null, + ]) { + const item = { ...comment, data: unauthorizedData, + body: `## Draft\n\nStructured data:\n\`\`\`json\n${JSON.stringify(unauthorizedData)}\n\`\`\`` }; + expect((await runGuard({ payload: payloadFor('plan'), permission: 'read', items: [item] })).failures) + .toHaveLength(1); + } + for (const options of [ + { actor: 'other-human' }, { payload: { ...payloadFor('plan'), sender: bot }, actor: bot.login }, + { payload: payloadFor('activate') }, { payload: payloadFor('cast') }, + { payload: payloadFor('unknown-command') }, { payload: payloadFor('status') }, + { payload: payloadFor('review') }, { payload: payloadFor('research') }, + ]) { + expect((await runGuard({ payload: payloadFor('plan'), permission: 'read', items: output.items, ...options })).failures) + .toHaveLength(1); + } + for (const item of [ + { ...comment, item_number: 43 }, { ...comment, repo: 'other/repo' }, + { ...comment, comment_id: 123 }, { ...comment, hide_older_comments: true }, + { ...comment, target: '*' }, + ...['create_issue', 'create_pull_request', 'dispatch_workflow', 'add_labels', 'unknown_output'] + .map(type => ({ ...comment, type })), + ]) { + expect((await runGuard({ payload: payloadFor('plan'), items: [item] })).failures).toHaveLength(1); + } + for (const body of [ + state.body.replace('**Current state:** Planned', '**Current state:** Activated'), + state.body.replace('**Current state:** Planned', '**Current state:** Researched'), + state.body.replace('**Last command:** `/squad plan`', '**Last command:** `/squad activate`'), + `${state.body}\n**Last command:** \`/squad plan\``, + `${state.body}\n\n${comment.body.slice(comment.body.indexOf('Structured data:'))}`, + ]) { + expect((await runGuard({ payload: payloadFor('plan'), permission: 'read', + items: [comment, { ...state, body }] })).failures).toHaveLength(1); + } + for (const item of [ + { ...state, temporary_id: comment.temporary_id }, + { ...state, data }, + { type: 'upsert_research_artifact', body: comment.body, temporary_id: comment.temporary_id, data }, + ]) { + expect((await runGuard({ payload: payloadFor('plan'), permission: 'read', items: [item] })).failures) + .toHaveLength(1); + } + }); + function assertDiagnosticIsolation(lock: Lock) { const steps = lock.jobs.conclusion.steps; const usage = steps.filter(step => diff --git a/test/helpers/gh-aw-diagnostic-runtime.ts b/test/helpers/gh-aw-diagnostic-runtime.ts index f0fe570ee..32b7fe67a 100644 --- a/test/helpers/gh-aw-diagnostic-runtime.ts +++ b/test/helpers/gh-aw-diagnostic-runtime.ts @@ -12,9 +12,9 @@ const entrypoints = [ ]; /** Fetch the real pinned handler closure, including its templates, without executing network clients. */ -export async function fetchDiagnosticRuntime() { +export async function fetchDiagnosticRuntime(extraEntrypoints: string[] = []) { const sources = new Map(); - let pending = entrypoints.map(name => `js/${name}`); + let pending = [...entrypoints, ...extraEntrypoints].map(name => `js/${name}`); while (pending.length) { const batch = [...new Set(pending)].filter(name => !sources.has(name)); pending = []; @@ -65,6 +65,7 @@ export function diagnosticRuntime( debug: (text: string) => logs.push(text), error: (text: string) => logs.push(text), setFailed: (text: string) => failures.push(text), setOutput: () => {}, startGroup: () => {}, endGroup: () => {}, summary, + exportVariable: (name: string, value: string) => { runtimeEnv[name] = value; }, }; const api = (method: string) => async (params: Record = {}) => { calls.push(method); @@ -102,7 +103,7 @@ export function diagnosticRuntime( rest, request: (method: string, params: Record) => api(method)(params), hook: { before: () => {} }, }; - const runtimeEnv = { + const runtimeEnv: Record = { GITHUB_REPOSITORY: 'owner/repo', RUNNER_TEMP: scratch, GITHUB_WORKSPACE: scratch, GH_AW_WORKFLOW_NAME: 'Squad', GH_AW_RUN_URL: 'https://github.com/owner/repo/actions/runs/1', GH_AW_JOB_RESULTS: JSON.stringify({ upsert_lifecycle_state: { result: 'failure' } }), @@ -114,14 +115,20 @@ export function diagnosticRuntime( payload: { issue: { number: 42 }, repository: { default_branch: 'dev' } }, }; const sandbox = createContext({ - core, github, context, console, Buffer, URL, TextDecoder, TextEncoder, - process: { env: runtimeEnv, cwd: () => scratch }, + core, github, context, console, Buffer, URL, TextDecoder, TextEncoder, structuredClone, + process: { env: runtimeEnv, cwd: () => scratch, + stdout: { write: (text: string) => logs.push(text) }, + stderr: { write: (text: string) => logs.push(text) } }, }); sandbox.global = sandbox; const modules = new Map }>(); + // Runner paths (including the collector's fixed path) are virtual, never host IO. + const files = new Map(); const filesystem = { - existsSync: (path: string) => path === outputPath, + existsSync: (path: string) => files.has(path) || path === outputPath, readFileSync: (path: string) => { + if (files.has(path)) return files.get(path); + if (sources.has(path)) return sources.get(path); if (path === outputPath) return readFileSync(outputPath, 'utf8'); const template = sources.get(`prompts/${String(path).split('/prompts/')[1]}`); if (template !== undefined) return template; @@ -130,20 +137,22 @@ export function diagnosticRuntime( }, readdirSync: () => [], mkdirSync: () => {}, - writeFileSync: () => {}, - appendFileSync: () => {}, + writeFileSync: (path: string, value: string) => { files.set(path, value); }, + appendFileSync: (path: string, value: string) => { files.set(path, (files.get(path) || '') + value); }, }; const load = (name: string): Record => { if (modules.has(name)) return modules.get(name)!.exports; const source = sources.get(name); if (!source) throw new Error(`Unfetched pinned runtime dependency: ${name}`); + if (name.endsWith('.json')) return JSON.parse(source); const module = { exports: {} }; modules.set(name, module); const require = (request: string): any => { if (request.startsWith('.')) return load(posix.normalize(posix.join(posix.dirname(name), request))); if (request === 'fs' || request === 'node:fs') return filesystem; if (request === 'os' || request === 'node:os') return { ...requireNode(request), tmpdir: () => scratch }; - if (['path', 'node:path', 'crypto', 'node:crypto', 'util', 'node:util', 'vm', 'node:vm'].includes(request)) return requireNode(request); + if (['path', 'node:path', 'crypto', 'node:crypto', 'util', 'node:util', 'vm', 'node:vm', + 'buffer', 'events', 'string_decoder'].includes(request)) return requireNode(request); if (['https', 'http', 'child_process'].includes(request)) return new Proxy({}, { get: (_target, method: string) => (command: string) => { if (request === 'child_process' && method === 'execSync' && command === 'git rev-parse --abbrev-ref HEAD') return 'dev\n'; @@ -158,7 +167,19 @@ export function diagnosticRuntime( return module.exports; }; return { - logs, failures, writes, calls, ioErrors, + logs, failures, writes, calls, ioErrors, files, load, + runScript: async (script: string) => compileFunction( + `return (async () => { ${script} })();`, + ['core', 'github', 'context', 'exec', 'io', 'getOctokit', 'require'], + { parsingContext: sandbox }, + )(core, github, context, {}, {}, () => github, + (request: string) => { + if (request.startsWith(`${scratch}/gh-aw/actions/`)) { + return load(`js/${request.slice(`${scratch}/gh-aw/actions/`.length)}`); + } + if (request === 'path' || request === 'node:path') return requireNode(request); + throw new Error(`Forbidden compiled-script dependency: ${request}`); + }), run: async (name: string) => { load('js/setup_globals.cjs').setupGlobals(core, github, context, {}, {}, () => github); return load(`js/${name}`).main(); diff --git a/workflows/shared/squad-command-contract.mjs b/workflows/shared/squad-command-contract.mjs index d65f25cd2..0fc4c77ee 100644 --- a/workflows/shared/squad-command-contract.mjs +++ b/workflows/shared/squad-command-contract.mjs @@ -2,6 +2,7 @@ import { readFileSync, writeFileSync } from 'node:fs'; import { pathToFileURL } from 'node:url'; +import { isDeepStrictEqual } from 'node:util'; const NO_ARGUMENT_COMMANDS = new Map([ ['cast', 'cast'], @@ -333,6 +334,24 @@ export async function authorizeSquadCommand({ payload, eventName, actor, resolve } } +function serializedPlanBody(body, data) { + // gh-aw v0.91.5 appends this suffix in safe_output_type_validator.cjs. + // Validate, rather than rewrite, so the writer retains the planning artifact. + const match = /(?:^|\n\n)Structured data:\n```json\n([\s\S]*?)\n```$/.exec(body); + let envelope; + try { + envelope = match && JSON.parse(match[1]); + } catch { + // Malformed data is rejected along with missing or mismatching envelopes. + } + if (!match || !isDeepStrictEqual(envelope, data) || + // Draft data has exactly four flat properties; reject duplicate JSON keys. + (match[1].match(/"(?:[^"\\]|\\.)*"\s*:/g) || []).length !== 4) { + throw new Error('Squad serialized plan comment requires one matching structured data envelope.'); + } + return body.slice(0, match.index); +} + function validateOpenModeOutputs(result, payload, items) { const issueNumber = Number(result.source === 'workflow_dispatch' ? payload?.inputs?.issue_number : payload?.issue?.number); @@ -348,7 +367,7 @@ function validateOpenModeOutputs(result, payload, items) { throw new Error('Squad open-mode output requires a valid originating issue.'); } const keys = item.type === 'add_comment' - ? ['type', 'body', 'item_number', 'data'] + ? ['type', 'body', 'item_number', 'data', 'temporary_id'] : ['type', 'body']; if (Object.keys(item).some(key => !keys.includes(key))) { throw new Error('Squad open-mode output contains unsupported mutation fields.'); @@ -357,15 +376,16 @@ function validateOpenModeOutputs(result, payload, items) { String(item.item_number) !== String(issueNumber)) { throw new Error('Squad open-mode comments must target the originating issue.'); } - // Inspect the complete body: gh-aw preserves HTML comments in code regions, - // and downstream lifecycle readers parse their envelopes as ordinary text. - if (typeof item.body !== 'string' || - /structured\s+data\s*:|[`~]{3,}\s*json|"squad_artifact"\s*:/i.test(item.body)) { - throw new Error('Squad open-mode body must not supply a structured artifact envelope.'); + const serializedComment = Object.hasOwn(item, 'temporary_id'); + // The pinned add_comment adapter emits bare IDs: generated eight-character + // IDs or normalized caller IDs accepted by temporary_id.cjs (3–12 characters). + if (serializedComment && (typeof item.temporary_id !== 'string' || + !/^aw_[A-Za-z0-9_]{3,12}$/.test(item.temporary_id))) { + throw new Error('Squad open-mode comment has invalid serializer metadata.'); } if (item.data !== undefined) { const data = item.data; - if (result.mode !== 'plan' || !data || typeof data !== 'object' || + if (result.mode !== 'plan' || !data || typeof data !== 'object' || Array.isArray(data) || Object.keys(data).sort().join(',') !== 'origin_issue,phases,schema_version,squad_artifact' || !['plan', 'program', 'implementation', 'validation'].includes(data.squad_artifact) || data.schema_version !== '1' || data.origin_issue !== issueNumber || @@ -373,6 +393,14 @@ function validateOpenModeOutputs(result, payload, items) { throw new Error('Squad open mode cannot emit acceptance, activation, or unrelated artifacts.'); } } + // Inspect all prose, including hidden/code regions. Only a serializer suffix + // matching separately validated draft data is exempt; metadata grants no authority. + const body = typeof item.body === 'string' && serializedComment && item.data !== undefined + ? serializedPlanBody(normalizeNewlines(item.body), item.data) : item.body; + if (typeof body !== 'string' || + /structured\s+data\s*:|[`~]{3,}\s*json|"squad_artifact"\s*:/i.test(body)) { + throw new Error('Squad open-mode body must not supply a structured artifact envelope.'); + } if (item.type === 'upsert_lifecycle_state') { const expected = result.mode === 'research' ? 'Researched' : 'Planned'; const states = [...item.body.matchAll(/^(?:[-*]\s+)?\*\*(?:Current state|State):\*\*\s+(.+)$/gim)]; diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index 9363064fb..231df1d24 100644 --- a/workflows/squad-workflows.manifest.json +++ b/workflows/squad-workflows.manifest.json @@ -152,7 +152,7 @@ "package_destination": ".github/workflows/shared/squad-command-contract.mjs", "destination": ".github/workflows/shared/squad-command-contract.mjs", "ownership": "manifest", - "sha256": "04fc4a578cd87de13964d3e154b6683d9b5851c5713c6fbe183ee5d24ce38aeb" + "sha256": "ccef806935f1283f404641d57aeb604021e284ff6a4b6463b7fff271cf8c521c" }, { "path": "shared/squad-cast-validator.mjs",