diff --git a/docs/src/content/docs/guide/gh-aw.md b/docs/src/content/docs/guide/gh-aw.md index 513d18a15..a1d878839 100644 --- a/docs/src/content/docs/guide/gh-aw.md +++ b/docs/src/content/docs/guide/gh-aw.md @@ -741,6 +741,25 @@ PR conversation comment. On a pull request, post the command in the Commands are matched longest-prefix-first, so the most specific command string wins: `/squad plan accept scope` is not treated as `/squad plan`. +### Command diagnostics + +The dispatcher and command router disable automatic failure, failed-job, +detection and no-op tracking-issue reporting, plus generic activation/completion +comments. Those framework paths run outside the deterministic safe-output guard; +completion comments can also append raw agent no-op text even when that guard +rejects the output. This restriction applies to maintainer runs as well as open +modes. Guarded command replies, research/planning artifacts and Cast failure +comments remain supported. + +Use the Actions run's logs and summaries for framework diagnostics. No-op messages +remain visible there, `report_incomplete` fails the conclusion step, and +`missing_data` remains logged (it does not by itself guarantee a red run). +Threat detection still runs and retains its enforcement behavior; only its +tracking-issue reporting is disabled. No diagnostic is permission to bypass +command authorization or the open-mode artifact restrictions. + +### Command reference + | Category | Command | Purpose | Notes | |----------|---------|---------|-------| | Team | `/squad` | Cast a new team | Same as `/squad cast` | @@ -845,6 +864,41 @@ together. Direct manual dispatches of either worker fail closed; use `/squad implement` or the retrospective relay so the dispatcher can create the authoritative receipt and bound session inputs. +### Deterministic command authorization + +Direct slash commands and discovered commands share a trusted runtime check, +independent of agent instructions. Mutating commands require live **write, +maintain, or admin** repository permission for both the event actor and the +original issue/comment author. Missing identities, bot-authored text, failed +permission lookups, and edited events without changed-command evidence fail +closed. Editing a maintainer's old comment does not inherit their authority. + +`status`, `review`, `research`, and `plan` remain open to identifiable human +contributors without write permission. Their safe outputs are restricted to +comments on the originating issue and mode-specific draft artifacts. They +cannot create issues or PRs, change labels, dispatch workers, or write acceptance +or activation artifacts, even when the caller is a maintainer. Research and +plan can update only their own nonterminal lifecycle state. Put structured data +in the tool's `data` field, not JSON fences or `Structured data:` blocks in the +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. + +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 +its deterministic handler can forward a validated command. + +Authenticated `workflow_dispatch` events retain GitHub's platform Actions-write +authorization, including the router and implementation worker's `GITHUB_TOKEN` +continuations. This exception is bound to the actual event and matching platform +actor/sender, never an `aw_context` claim, issue text, or a blanket bot exemption. +Open-mode output restrictions still apply to dispatched commands. Empty activation +probes cannot produce safe outputs. + +Install this protection as a complete immutable package update using +[Upgrading](#upgrading), not by editing an integrity-pinned runtime file. + ### Where you can use slash commands | Surface | How it works | @@ -1447,14 +1501,13 @@ interchangeable — the trigger, the wording, and the remedy differ: | Issues not created | Created count is below the plan's declared total | `N of M issues created so far — rerun the identical activation command to continue.` | | Labels not applied | An activated issue had no `add_labels` call accepted | `{labeled} of {activated} activated issues had a label operation accepted` | -Either one calls `report_incomplete`. **This does not fail the run.** The workflow -run still concludes `success`, so `gh run view --json conclusion` is not a way to -detect a truncated activation. The durable, user-visible signal is a tracking -issue in your repository titled `[aw] ... reported incomplete result`, which -gh-aw opens or updates: +Either one calls `report_incomplete`. With pinned gh-aw v0.89.22, **this fails the +conclusion step** and records the reason in the Actions logs. The dispatcher +does not create or update a separate tracking issue. Inspect the run and the +guarded activation summary on the originating issue: ```bash -gh issue list --search '"reported incomplete result" in:title' --state all +gh run view --log-failed ``` If a cap was actually reached, the report names which cap and lists the work @@ -1469,7 +1522,9 @@ IDs the run minted — `#aw_epic{K}` and `#aw_task{N}` on the hierarchical path, — rather than by issue number, because at that point creation is still deferred and no real number exists yet. -Do not read a green run as a complete activation. Check for that tracking issue. +Do not read a green run as proof of complete activation: an omitted +`report_incomplete` signal cannot be detected by that handler. Check the +activation summary and verify the resulting issues and labels. #### Verifying an activation: the bindings block @@ -2130,8 +2185,8 @@ other refs. | `/squad` command is ignored | Lock file not committed or workflow not compiled | Run `gh aw compile --strict`, commit the lock file, and push | | Universe is full on cast-member | All character names in the universe are allocated | Retire an unused member first, or re-cast with `/squad cast` | | "No plan found" on plan accept | No `/squad plan` comment exists yet | Run `/squad plan` first to generate a plan for review | -| Plan activation creates fewer issues than the accepted plan declares | The run ended early, or it reached the `create-issue` (75) or `add-labels` (110) safe-output cap | Look for an `[aw] ... reported incomplete result` tracking issue — it names the shortfall and, when a cap was reached, which cap and what did not fit. Re-run the identical activation command (title matching resumes without duplicating existing issues), or activate one phase at a time with `/squad plan activate phase {N}` | -| Activation run is green but some issues are missing or unlabeled | `report_incomplete` records truncation without failing the run | A green run is not proof of a complete activation. Check for the `[aw] ... reported incomplete result` tracking issue, then verify with `gh issue list --label squad` | +| Plan activation creates fewer issues than the accepted plan declares | The run ended early, or it reached the `create-issue` (75) or `add-labels` (110) safe-output cap | Inspect the Actions logs and originating issue's activation summary for the shortfall and any cap reached. Re-run the identical activation command (title matching resumes without duplicating existing issues), or activate one phase at a time with `/squad plan activate phase {N}` | +| Activation run is green but some issues are missing or unlabeled | The agent may have omitted `report_incomplete`, which would fail the conclusion step if emitted | A green run is not proof of a complete activation. Check the activation summary and Actions logs, then verify with `gh issue list --label squad`; automatic tracking-issue reporting is disabled | | A Squad label has no description and an unexpected color | It was auto-created on a fresh repo by `create-if-missing` | Expected, not a failure. Edit the label if you want a description or a specific color | | `/squad implement` cannot create a PR | Expected under Profile A (recommended); Actions is not allowed to create pull requests and there is no automatic fallback for this workflow | Push the worker's branch and open the PR manually, or switch to [Profile B](#profile-b-opt-in-automatic-pr-creation) if you accept its repository-wide self-approval tradeoff | | Epic implementation dispatches no workers | Every child is blocked or already has an open implementation PR | Merge dependency PRs, then run `/squad implement` on the epic again | diff --git a/test/gh-aw-activation-capacity.test.ts b/test/gh-aw-activation-capacity.test.ts index ca92b1dae..291699984 100644 --- a/test/gh-aw-activation-capacity.test.ts +++ b/test/gh-aw-activation-capacity.test.ts @@ -14,16 +14,15 @@ * that believed the label reading would conclude a 50-issue activation (up to 100 label * names) had overrun its budget, and could stop labeling early or batch issues together. * This is agent self-truncation, and it was reachable at 80. - * 2. **Neither enforcement layer fails the run.** Even when a cap is genuinely hit, the - * workflow run still concludes successfully, so nothing forces the truncation to - * surface. See below. + * 2. **Neither cap-enforcement layer fails the run by itself.** Even when a cap is + * genuinely hit, truncation needs an explicit incomplete report to surface. See below. * * Note what is *not* claimed: at `max: 80`, 50 `add_labels` calls did **not** overflow the * operation cap. Runtime truncation was not reachable at the documented maximum. The cap * moves to 110 to defeat the misleading injected wording and to hold a bounded margin — * not to fix a proven 80-item overflow. * - * ## Runtime semantics this suite is written against (gh-aw v0.87.10, the CI pin) + * ## Runtime semantics this suite is written against (gh-aw v0.89.22, the CI pin) * * **Cap enforcement is dual (Safe Outputs Specification MCE4) and neither half is fatal.** * Invocation time — `safe_outputs_handlers.cjs`, `enforcePerTypeMax` via @@ -38,20 +37,17 @@ * That is a JSON-RPC error the agent **does** observe. Collection time — * `collect_ndjson_output.cjs` — is the second half: a surplus item is dropped with * `continue` and reported via `core.warning`. Neither path calls `core.setFailed`, so the - * run concludes successfully with label operations missing. + * run can conclude successfully with label operations missing unless incompletion is reported. * * `max` therefore caps **operations of that type**, not label names inside one call. One * `add_labels` call carrying two labels costs one unit of budget, not two. * - * **`report_incomplete` does not turn the run red.** gh-aw's own tool description claims it - * is "treated as a failure signal even when the agent exits successfully" — misleading in - * precisely the way this issue is about. In the pinned runtime, - * `report_incomplete_handler.cjs` emits `core.warning` only, and `handle_agent_failure.cjs` - * contains no `core.setFailed` or `process.exit`: its "failure handling" opens or updates an - * `[aw] {workflow} reported incomplete result` tracking issue/comment. That is a durable, - * human-actionable record — it satisfies #1961's "explicit incomplete result" — but the run - * conclusion stays green. No narrower supported mechanism in v0.87.10 makes it red, so the - * workflow states that limitation rather than implying a failure it cannot produce. + * **`report_incomplete` fails the conclusion step.** In v0.89.22, + * `report_incomplete_handler.cjs` logs a warning, then `handle_agent_failure.cjs` calls + * `core.setFailed` even when the agent succeeded. #2185 disables that conclusion handler's + * independent tracking-issue writes, not its failure status or logged evidence. The guarded + * activation summary must also report the shortfall on the originating issue. The real pinned + * handler and zero-write boundary are exercised by gh-aw-command-authorization.test.ts. * * ## The capacity calculation this suite locks in * @@ -412,33 +408,24 @@ describe('gh-aw: self-validation reconciles activated items with label operation ).toBe(true); }); - it('states report_incomplete semantics accurately — a tracking record, not a red run', () => { - // Verified against pinned gh-aw v0.87.10 rather than gh-aw's own tool description - // (which says "treated as a failure signal even when the agent exits successfully" - // — misleading in exactly the way this issue is about): - // report_incomplete_handler.cjs -> core.warning only. - // handle_agent_failure.cjs -> contains no core.setFailed / process.exit; - // "failure handling" opens/updates an - // "[aw] ... reported incomplete result" issue. - // The run still concludes successfully. The workflow must say so, so the agent does - // not assume a red run is carrying the signal for it. + it('describes pinned report_incomplete failure status without tracking-issue writes', () => { + // v0.89.22 fails conclusion for report_incomplete. #2185 disables its independent + // issue-writing path; the real-handler regression lives in command-authorization. expect( - /reported incomplete result/i.test(activateProse), - 'The workflow must name the durable artifact report_incomplete actually produces.', + /`report_incomplete` records the reason in Actions logs and fails the conclusion step/i.test(activateProse), + 'The workflow must describe the failure status and logged evidence from the pinned runtime.', ).toBe(true); expect( - /does \*\*not\*\* change the run's conclusion/i.test(activateProse), - 'The workflow must state that report_incomplete does not change the run conclusion.', + /automatic tracking-issue reporting is disabled/i.test(activateProse), + 'The workflow must not promise an issue from the disabled conclusion reporting path.', ).toBe(true); expect( - /never rely on a red run/i.test(activateProse), - 'The workflow must forbid relying on a failed run to carry the incompletion signal.', + /Include the shortfall in the guarded activation summary as well, so the originating issue retains an actionable signal/i.test(activateProse), + 'The authorized originating-issue summary must still identify incomplete activation.', ).toBe(true); - // Guard against reintroducing the overclaim this test previously asserted. expect( - /failure signal even when the agent exits successfully/i.test(activateProse), - 'The workflow must not repeat gh-aw\'s misleading "failure signal" phrasing: the ' + - 'pinned runtime emits a warning and a tracking issue, and never fails the run.', + /does \*\*not\*\* change the run's conclusion/i.test(activateProse), + 'The obsolete v0.87.10 success-conclusion claim must not return.', ).toBe(false); }); diff --git a/test/gh-aw-command-authorization.test.ts b/test/gh-aw-command-authorization.test.ts new file mode 100644 index 000000000..232153147 --- /dev/null +++ b/test/gh-aw-command-authorization.test.ts @@ -0,0 +1,553 @@ +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'; +import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { execFileSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { compileFunction, constants } from 'node:vm'; +import { parse } from 'yaml'; +import { createFirstInstallFixture } from './helpers/gh-aw-install-fixture.js'; +import { authorizeSquadCommand, classifySquadCommand } from '../workflows/shared/squad-command-contract.mjs'; +import { compileWithPinnedActions } from '../workflows/shared/squad-install-verifier.mjs'; +import { DIAGNOSTIC_RUNTIME_SHA, diagnosticRuntime, fetchDiagnosticRuntime } from './helpers/gh-aw-diagnostic-runtime.js'; + +interface Step { + name?: string; + id?: string; + if?: string; + env?: Record; + with?: { script?: string; ref?: string; path?: string }; +} +interface Lock { + jobs: Record; steps: Step[] }>; +} +type Item = Record; +const root = process.cwd(); +const scratch = mkdtempSync(join(root, '.test-command-authorization-')); +const trusted = join(scratch, '.squad-command-trusted-base/.github/workflows/shared'); +const outputPath = join(scratch, 'agent-output.json'); +const nodeRequire = createRequire(import.meta.url); +const human = (login = 'maintainer') => ({ login, type: 'User' }); +const bot = { login: 'github-actions[bot]', type: 'Bot' }; +const payloadFor = (command: string, eventName = 'issue_comment') => ({ + action: eventName === 'issues' ? 'opened' : 'created', + sender: human(), + repository: { default_branch: 'dev' }, + issue: { number: 42, body: `/squad ${command}`, user: human() }, + ...(eventName === 'issue_comment' + ? { comment: { id: 123, body: `/squad ${command}`, user: human() } } + : eventName === 'workflow_dispatch' + ? { inputs: { command, issue_number: '42' } } + : {}), +}); +let dispatcher: Lock; +let router: Lock; +let sanitizeContent: (body: string, options?: { allowedAliases: string[] }) => string; +let diagnosticSources: Map; + +beforeAll(async () => { + const fixture = createFirstInstallFixture('a'.repeat(40), 'package'); + 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, + // in memory, with no GitHub clients or writes and no approximate sanitizer stubs. + const runtimeSha = DIAGNOSTIC_RUNTIME_SHA; + expect(fixture.consumerFiles.get('.github/workflows/squad.lock.yml')!.toString()) + .toContain(`github/gh-aw-actions/setup@${runtimeSha} # v0.89.22`); + const sources = new Map(await Promise.all([ + 'sanitize_content.cjs', 'sanitize_content_core.cjs', 'markdown_code_region_balancer.cjs', + 'repo_helpers.cjs', 'slash_command_matcher.cjs', 'glob_pattern_helpers.cjs', 'error_codes.cjs', + ].map(async name => { + const response = await fetch( + `https://raw.githubusercontent.com/github/gh-aw-actions/${runtimeSha}/setup/js/${name}`, + { signal: AbortSignal.timeout(30_000) }, + ); + if (!response.ok) throw new Error(`Pinned sanitizer fetch failed: ${name} HTTP ${response.status}`); + return [name, await response.text()] as const; + }))); + const modules = new Map }>(); + const load = (name: string): Record => { + if (modules.has(name)) return modules.get(name)!.exports; + const source = sources.get(name); + if (!source) throw new Error(`Unexpected sanitizer dependency: ${name}`); + const module = { exports: {} }; + modules.set(name, module); + compileFunction(source, ['module', 'exports', 'require', 'process'])(module, module.exports, + (request: string) => request.startsWith('./') ? load(request.slice(2)) : nodeRequire(request), + { env: { GITHUB_REPOSITORY: 'owner/repo' } }); + return module.exports; + }; + sanitizeContent = load('sanitize_content.cjs').sanitizeContent; + diagnosticSources = await fetchDiagnosticRuntime(); + mkdirSync(trusted, { recursive: true }); + cpSync(join(root, 'workflows/shared/squad-command-contract.mjs'), join(trusted, 'squad-command-contract.mjs')); +}, 180_000); +afterAll(() => rmSync(scratch, { recursive: true, force: true })); + +async function runGuard({ + eventName = 'issue_comment', payload = payloadFor('cast'), actor = 'maintainer', + permission = 'write', items = [{ type: 'create_issue', title: 'must be gated' }], + permissionFailure = false, omitOutput = false, source = 'dispatcher', +}: { + eventName?: string; payload?: ReturnType; actor?: string; + permission?: string | Record; items?: Item[]; + permissionFailure?: boolean; omitOutput?: boolean; source?: 'dispatcher' | 'router'; +} = {}) { + const lock = source === 'dispatcher' ? dispatcher : router; + const stepName = source === 'dispatcher' + ? 'Reject unknown or malformed Squad commands' : 'Route or reject discovered command'; + const step = lock.jobs.safe_outputs.steps.find(candidate => candidate.name === stepName)!; + expect(step?.with?.script).toBeTruthy(); + expect(step.env?.GH_AW_AGENT_OUTPUT).toBe('${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }}'); + writeFileSync(outputPath, JSON.stringify({ items })); + const mutations: string[] = []; + const failures: string[] = []; + const permissionLookup = vi.fn(async ({ username }: { username: string }) => { + if (permissionFailure) throw new Error('permission API unavailable'); + return { data: { permission: typeof permission === 'string' ? permission : permission[username] } }; + }); + const github = { rest: { + repos: { getCollaboratorPermissionLevel: permissionLookup }, + issues: { createComment: async () => { mutations.push('diagnostic comment'); } }, + actions: { createWorkflowDispatch: async () => { mutations.push('dispatch'); } }, + } }; + const context = { actor, payload, repo: { owner: 'owner', repo: 'repo' } }; + const core = { setFailed: (message: string) => { failures.push(message); }, info: vi.fn() }; + const script = compileFunction(`return (async () => { ${step.with!.script} })();`, + ['github', 'context', 'core', 'process', 'require'], + { importModuleDynamically: constants.USE_MAIN_CONTEXT_DEFAULT_LOADER }); + try { + await script(github, context, core, { env: { + GITHUB_WORKSPACE: scratch, SQUAD_EVENT_NAME: eventName, + GH_AW_AGENT_OUTPUT: omitOutput ? undefined : outputPath, + } }, nodeRequire); + } catch (error) { + failures.push(String(error)); + } + return { failures, mutations, permissionLookup }; +} + +async function interpretLifecycleComment(body?: string) { + const step = dispatcher.jobs.repair_activated_lifecycle.steps + .find(candidate => candidate.name === 'Repair terminal lifecycle after idempotent activation')!; + expect(step?.with?.script).toBeTruthy(); + const mutations: Record[] = []; + const failures: string[] = []; + const github = { + paginate: async () => body === undefined ? [] : [{ + id: 123, body, created_at: '2026-10-08T00:00:00Z', user: bot, + }], + rest: { issues: { + listComments: vi.fn(), + createComment: async (params: Record) => mutations.push(params), + updateComment: async (params: Record) => mutations.push(params), + } }, + }; + await compileFunction(`return (async () => { ${step.with!.script} })();`, + ['github', 'context', 'core', 'process'])(github, + { repo: { owner: 'owner', repo: 'repo' } }, + { setFailed: (message: string) => failures.push(message), info: vi.fn() }, + { env: { ISSUE_NUMBER: '42', SQUAD_EVENT_NAME: 'workflow_dispatch', SQUAD_COMMAND: 'plan accept' } }); + expect(failures).toEqual([]); + return mutations; +} + +describe('compiled Squad command authorization boundary (#2185)', () => { + function assertDiagnosticIsolation(lock: Lock) { + const steps = lock.jobs.conclusion.steps; + const handlers = steps.filter(step => step.with?.script); + expect(handlers.every(step => ['noop', 'missing_tool', 'report_incomplete', + 'handle_agent_failure', 'conclusion'].includes(step.id || ''))).toBe(true); + expect(steps.find(step => step.id === 'handle_agent_failure')?.env?.GH_AW_FAILURE_REPORT_AS_ISSUE) + .toBe('false'); + expect(steps.find(step => step.id === 'noop')?.env?.GH_AW_NOOP_REPORT_AS_ISSUE).toBe('false'); + expect(steps.some(step => step.id === 'detection_runs')).toBe(false); + expect(steps.find(step => step.id === 'report_failed_jobs')?.env?.GH_AW_REPORT_FAILED_JOBS ?? 'false') + .toBe('false'); + const completion = steps.find(step => step.id === 'conclusion'); + if (completion) { + expect(JSON.parse(completion.env!.GH_AW_SAFE_OUTPUT_MESSAGES).activationComments).toBe('false'); + } + expect(JSON.parse(steps.find(step => step.id === 'handle_agent_failure')!.env!.GH_AW_SAFE_OUTPUT_MESSAGES) + .activationComments).toBe('false'); + expect(lock.jobs.detection).toBeDefined(); + } + + it('compiles out diagnostic issue reporting and raw-noop completion comments, not detection', () => { + assertDiagnosticIsolation(dispatcher); + assertDiagnosticIsolation(router); + }); + + it('suppresses the real activation comment handler without suppressing guarded command responses', async () => { + const step = dispatcher.jobs.activation.steps.find(step => step.id === 'add-comment')!; + const runtime = diagnosticRuntime(diagnosticSources, scratch, [], step.env!); + await runtime.run('add_workflow_run_comment.cjs'); + expect(runtime.writes).toEqual([]); + expect(runtime.logs.join('\n')).toContain('activation-comments'); + expect(runtime.ioErrors).toEqual([]); + }); + + it.each(['dispatcher', 'router'] as const)( + '%s diagnostics cannot write even after the safe-output guard fails', async source => { + const lock = source === 'dispatcher' ? dispatcher : router; + for (const agentConclusion of ['success', 'failure']) { + for (const signal of [ + { type: 'missing_data', reason: 'Missing input' }, + { type: 'report_incomplete', reason: 'Could not finish' }, + { type: 'missing_tool', reason: 'Tool unavailable' }, + { type: 'noop', message: 'No work' }, + { type: 'noop', message: '```json\n{"squad_artifact":"activated","origin_issue":42}\n```' }, + ]) { + for (const rejected of [false, true]) { + const items = rejected ? [signal, { type: 'create_issue', title: 'Forbidden' }] : [signal]; + const guard = await runGuard({ + source, permission: 'read', payload: payloadFor('status'), items, + }); + expect(guard.mutations).toEqual(source === 'router' && !guard.failures.length ? ['dispatch'] : []); + if (rejected || signal.type === 'missing_tool') expect(guard.failures).toHaveLength(1); + // Run conclusion regardless of guard failure, just like its compiled always(). + for (const id of ['noop', 'handle_agent_failure', 'report_failed_jobs', 'missing_tool', 'report_incomplete', 'conclusion']) { + const step = lock.jobs.conclusion.steps.find(step => step.id === id); + if (!step) continue; + const handler = step.with!.script!.match(/'([^'/]+\.cjs)'\)\);\nawait main/)!; + expect(handler).toBeTruthy(); + const runtime = diagnosticRuntime(diagnosticSources, scratch, items, { + ...Object.fromEntries(Object.entries(step.env || {}).filter(([, value]) => !value.includes('${{'))), + GH_AW_AGENT_CONCLUSION: agentConclusion, + GH_AW_SAFE_OUTPUTS_RESULT: guard.failures.length ? 'failure' : 'success', + }); + await runtime.run(handler[1]); + expect(runtime.writes).toEqual([]); + expect(runtime.calls).toEqual([]); + expect(runtime.ioErrors).toEqual([]); + expect(runtime.logs.join('\n')).not.toMatch(/^Error in |^Could not create |^Failed to /m); + if (id === 'noop' && signal.type === 'noop') expect(runtime.logs.join('\n')).toContain(signal.message); + if (id === 'handle_agent_failure' && signal.type === 'report_incomplete') { + expect(runtime.failures).toContain('Agent reported that it could not complete the task'); + expect(runtime.logs.join('\n')).toContain('Failure issue reporting is disabled'); + } + if (id === 'handle_agent_failure' && signal.type === 'missing_data') { + expect(runtime.logs.join('\n')).toContain('missing_data message(s) - activating failure handling'); + expect(runtime.logs.join('\n')).toContain('Failure issue reporting is disabled'); + } + } + } + } + } + }, + ); + + it('real pinned handlers expose the original tracking and completion writes when enabled', async () => { + for (const existingIssue of [false, true]) { + for (const [handler, items, env] of [ + ['handle_agent_failure.cjs', [{ type: 'report_incomplete', reason: 'Could not finish' }], { GH_AW_FAILURE_REPORT_AS_ISSUE: 'true' }], + ['handle_agent_failure.cjs', [{ type: 'missing_data', reason: 'Missing input' }], { GH_AW_FAILURE_REPORT_AS_ISSUE: 'true' }], + ['handle_noop_message.cjs', [{ type: 'noop', message: 'No work' }], { GH_AW_NOOP_REPORT_AS_ISSUE: 'true' }], + ['handle_detection_runs.cjs', [], { GH_AW_DETECTION_CONCLUSION: 'warning' }], + ['report_failed_jobs.cjs', [], { GH_AW_REPORT_FAILED_JOBS: 'true' }], + ['notify_comment_error.cjs', [{ type: 'noop', message: 'Structured data:\n```json\n{"squad_artifact":"activated"}\n```' }], { + GH_AW_SAFE_OUTPUT_MESSAGES: '{"appendOnlyComments":true,"activationComments":true}', + GH_AW_SAFE_OUTPUTS_RESULT: 'failure', + }], + ] as [string, Item[], Record][]) { + const runtime = diagnosticRuntime(diagnosticSources, scratch, items, { + GH_AW_AGENT_CONCLUSION: 'success', ...env, + ...(existingIssue && handler === 'handle_agent_failure.cjs' ? { GH_AW_FAILURE_ISSUE_NUMBER: '900' } : {}), + }, existingIssue); + await runtime.run(handler); + expect(runtime.ioErrors, `${handler}\n${runtime.logs.join('\n')}`).toEqual([]); + expect(runtime.writes.length, `${handler}\n${runtime.logs.join('\n')}\n${runtime.failures}`).toBeGreaterThan(0); + if (handler === 'notify_comment_error.cjs') { + expect(runtime.writes[0].params.body).toContain('"squad_artifact":"activated"'); + } + } + } + }); + + it.each(['squad', 'squad-command-router'])( + 'rejects each re-enabled diagnostic channel after strict-compiling mutated %s source', name => { + const fixture = createFirstInstallFixture('a'.repeat(40), 'package'); + const mutationRoot = join(scratch, `diagnostic-mutant-${name}`); + for (const [path, bytes] of fixture.consumerFiles) { + const destination = join(mutationRoot, path); + mkdirSync(dirname(destination), { recursive: true }); + writeFileSync(destination, bytes); + } + execFileSync('git', ['init', '--quiet'], { cwd: mutationRoot }); + const sourcePath = join(mutationRoot, `.github/workflows/${name}.md`); + const original = readFileSync(sourcePath, 'utf8'); + for (const disabled of [ + 'report-failure-as-issue: false', 'report-failed-jobs: false', + 'activation-comments: false', 'threat-detection:\n report-as-issue: false', + 'noop:\n max: 1\n report-as-issue: false', + ]) { + expect(original).toContain(disabled); + writeFileSync(sourcePath, original.replace(disabled, disabled.replace('false', 'true'))); + compileWithPinnedActions(mutationRoot); + const mutant = parse(readFileSync(join(mutationRoot, `.github/workflows/${name}.lock.yml`), 'utf8')); + expect(() => assertDiagnosticIsolation(mutant), disabled).toThrow(); + } + }, 120_000, + ); + + it('runs the guard after output download but before privileged handlers, with dependent custom jobs', () => { + for (const [lock, name] of [ + [dispatcher, 'Reject unknown or malformed Squad commands'], + [router, 'Route or reject discovered command'], + ] as const) { + const steps = lock.jobs.safe_outputs.steps; + const gate = steps.findIndex(step => step.name === name); + expect(gate).toBeGreaterThan(steps.findIndex(step => step.id === 'setup-agent-output-env')); + expect(steps[gate - 1].with).toMatchObject({ + ref: '${{ github.workflow_sha }}', path: '.squad-command-trusted-base', + }); + const handlers = steps.filter(step => /Process safe outputs/i.test(step.name || '')); + expect(handlers.length).toBeGreaterThan(0); + for (const handler of handlers) { + expect(steps.indexOf(handler)).toBeGreaterThan(gate); + expect(handler.if || '').not.toMatch(/always\(\)|!cancelled\(\)/); + } + } + for (const job of ['upsert_research_artifact', 'upsert_lifecycle_state', 'repair_activated_lifecycle']) { + expect(dispatcher.jobs[job].needs).toContain('safe_outputs'); + expect(dispatcher.jobs[job].if).toContain("needs.safe_outputs.result == 'success'"); + } + }); + + it.each(['issues', 'issue_comment'])('denies read/triage/noncollaborator commands on %s', async eventName => { + for (const permission of ['read', 'triage', 'none', 'unresolved', '']) { + const result = await runGuard({ eventName, payload: payloadFor('cast', eventName), permission }); + expect(result.failures).toHaveLength(1); + expect(result.mutations).toEqual([]); + } + }); + + it.each(['write', 'maintain', 'admin'])('allows %s command authors', async permission => { + const result = await runGuard({ permission }); + expect(result.failures).toEqual([]); + expect(result.permissionLookup).toHaveBeenCalledTimes(1); + }); + + it('denies missing identities, mismatched senders, unknown events, and API failures', async () => { + const missingAuthor = payloadFor('cast'); + missingAuthor.comment!.user.login = ''; + const missingSender = payloadFor('cast'); + missingSender.sender.login = ''; + for (const options of [ + { actor: '' }, { payload: missingAuthor }, { payload: missingSender }, + { eventName: 'repository_dispatch' }, { permissionFailure: true }, + ]) { + expect((await runGuard(options)).failures).toHaveLength(1); + } + }); + + it.each(['issues', 'issue_comment'])('binds changed %s text to both editor and original author', async eventName => { + const payload = { ...payloadFor('cast', eventName), action: 'edited', + changes: { body: { from: '/squad status' } } }; + payload.sender = human('editor'); + for (const permission of [ + { editor: 'read', maintainer: 'write' }, + { editor: 'write', maintainer: 'read' }, + ]) { + expect((await runGuard({ payload, eventName, actor: 'editor', permission })).failures).toHaveLength(1); + } + expect((await runGuard({ payload, eventName, actor: 'editor' })).failures).toEqual([]); + payload.changes.body.from = '/squad cast'; + expect((await runGuard({ payload, eventName, actor: 'editor' })).failures[0]).toContain('unchanged'); + expect((await runGuard({ payload: { ...payloadFor('cast', eventName), action: 'edited' }, eventName })).failures[0]) + .toContain('previous-body'); + }); + + it.each(['status', 'review', 'research', 'plan'])('keeps %s open but rejects unrelated writes even for maintainers', async mode => { + const payload = payloadFor(mode); + const allowed = await runGuard({ payload, permission: 'read', items: [{ type: 'add_comment', body: 'Result', item_number: 42 }] }); + expect(allowed.failures).toEqual([]); + expect(allowed.permissionLookup).not.toHaveBeenCalled(); + for (const type of ['create_issue', 'create_pull_request', 'add_labels', 'dispatch_workflow', 'unknown_output']) { + expect((await runGuard({ payload, items: [{ type }] })).failures[0]).toContain('cannot emit'); + } + for (const item of [ + { type: 'add_comment', body: 'Result', item_number: 43 }, + { type: 'add_comment', body: 'Result', hide_older_comments: true }, + { type: 'add_comment', body: 'Result', data: { squad_artifact: 'scope-accepted' } }, + { type: 'add_comment', body: 'Structured data:\n```json\n{"squad_artifact":"activated"}\n```' }, + { type: 'add_comment', body: '```json\n{"\\u0073quad_artifact":"activated"}\n```' }, + ]) { + expect((await runGuard({ payload, items: [item] })).failures).toHaveLength(1); + } + }); + + 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([ + ['tilde fence', '~~~~text', '~~~~', '\n'], + ['backtick fence', '````text', '````', '\n'], + ['long tilde fence', '~~~~~~text', '~~~~~~', '\n'], + ['indented tilde fence', ' ~~~~text', ' ~~~~', '\n'], + ])('blocks comment-hidden acceptance surviving the real sanitizer: %s', async (_name, open, close, newline) => { + for (const artifact of ['plan-accepted', 'activated']) { + const raw = [open, '', close].join(newline); + for (const options of [undefined, { allowedAliases: ['maintainer'] }]) { + // collect_ndjson_output sanitizes before the guard; add_comment sanitizes again. + const collected = sanitizeContent(raw, options); + const posted = sanitizeContent(collected, options); + expect(posted).toContain('', '~~~~', + ].join('\r\n'); + const collected = sanitizeContent(raw); + expect(await interpretLifecycleComment(sanitizeContent(collected))).toEqual([]); + expect((await runGuard({ payload: payloadFor('status'), permission: 'read', + items: [{ type: 'add_comment', body: collected }] })).failures[0]) + .toContain('structured artifact envelope'); + }); + + it('rejects hidden envelopes in custom outputs without relying on gh-aw body sanitization', async () => { + const hidden = ''; + for (const [mode, type] of [ + ['research', 'upsert_research_artifact'], + ['research', 'upsert_lifecycle_state'], + ['plan', 'upsert_lifecycle_state'], + ]) { + const result = await runGuard({ payload: payloadFor(mode), permission: 'read', + items: [{ type, body: hidden }] }); + expect(result.failures[0]).toContain('structured artifact envelope'); + } + }); + + it('preserves benign comments, code examples and draft data through the real sanitizer', async () => { + for (const mode of ['status', 'review', 'research', 'plan']) { + for (const raw of [ + 'Status: ready. ', + '~~~~text\n\n~~~~', + '```typescript\nconst ready = true;\n```', + ]) { + const collected = sanitizeContent(raw); + const result = await runGuard({ payload: payloadFor(mode), permission: 'read', + items: [{ type: 'add_comment', body: collected, item_number: 42 }] }); + expect(result.failures).toEqual([]); + expect(await interpretLifecycleComment(sanitizeContent(collected))).toEqual([]); + } + } + for (const artifact of ['plan', 'program', 'implementation', 'validation']) { + const item = { type: 'add_comment', body: sanitizeContent('Draft result'), data: { + squad_artifact: artifact, schema_version: '1', origin_issue: 42, phases: [], + } }; + expect((await runGuard({ payload: payloadFor('plan'), permission: 'read', items: [item] })).failures) + .toEqual([]); + expect(await interpretLifecycleComment( + `${sanitizeContent(item.body)}\n\nStructured data:\n\`\`\`json\n${JSON.stringify(item.data)}\n\`\`\``, + )).toEqual([]); + } + }); + + it('accepts authenticated manual dispatch, router relay and worker continuation, not bot text', async () => { + for (const command of ['cast', 'status', 'implement']) { + for (const sender of [human(), bot, { login: 'automation[bot]', type: 'Bot' }]) { + const payload = { ...payloadFor(command, 'workflow_dispatch'), sender }; + const result = await runGuard({ payload, actor: sender.login, eventName: 'workflow_dispatch', + permissionFailure: true, items: [{ type: 'noop', message: 'No work' }] }); + expect(result.failures).toEqual([]); + expect(result.permissionLookup).not.toHaveBeenCalled(); + } + } + for (const mode of ['cast', 'status']) { + const payload = { ...payloadFor(mode), sender: bot }; + expect((await runGuard({ payload, actor: bot.login })).failures).toHaveLength(1); + } + expect((await runGuard({ payload: payloadFor('', 'workflow_dispatch'), eventName: 'workflow_dispatch' })).failures[0]) + .toContain('No explicit Squad command'); + }); + + it('fails closed without agent-output evidence and blocks mixed batches before any handler', async () => { + expect((await runGuard({ omitOutput: true })).failures[0]).toContain('GH_AW_AGENT_OUTPUT'); + const result = await runGuard({ payload: payloadFor('plan'), items: [ + { type: 'add_comment', body: 'Allowed' }, { type: 'dispatch_workflow' }, + ] }); + expect(result.failures).toHaveLength(1); + expect(result.mutations).toEqual([]); + }); + + it('router permits only deterministic dispatch and rejects malicious agent outputs first', async () => { + const allowed = await runGuard({ source: 'router', items: [{ type: 'noop', message: 'Done' }] }); + expect(allowed.failures).toEqual([]); + expect(allowed.mutations).toEqual(['dispatch']); + for (const type of ['dispatch_workflow', 'add_comment', 'create_issue']) { + const result = await runGuard({ source: 'router', items: [{ type }] }); + expect(result.failures[0]).toContain('diagnostics only'); + expect(result.mutations).toEqual([]); + } + }); + + it('requires the same permission policy at router and dispatcher call sites', async () => { + const payload = payloadFor('cast'); + const result = classifySquadCommand(payload, 'issue_comment'); + await expect(authorizeSquadCommand({ + payload, eventName: 'issue_comment', actor: 'maintainer', result, + resolvePermission: async () => 'triage', + })).rejects.toThrow('refused mutating mode'); + }); + + it('detects a live-source mutation that removes authorization after strict compilation', async () => { + const fixture = createFirstInstallFixture('a'.repeat(40), 'package'); + const mutationRoot = join(scratch, '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('await contract.enforceSquadCommandContract({', 'await (async () => {})({'); + expect(mutant).not.toBe(original); + writeFileSync(sourcePath, mutant); + execFileSync('git', ['init', '--quiet'], { cwd: mutationRoot }); + compileWithPinnedActions(mutationRoot); + const realDispatcher = dispatcher; + const assertDenial = (result: Awaited>) => { + expect(result.failures).toHaveLength(1); + }; + assertDenial(await runGuard({ permission: 'read' })); + try { + dispatcher = parse(readFileSync(join(mutationRoot, '.github/workflows/squad.lock.yml'), 'utf8')); + const result = await runGuard({ permission: 'read' }); + expect(() => assertDenial(result)).toThrow(); + expect(result.permissionLookup).not.toHaveBeenCalled(); + } finally { + dispatcher = realDispatcher; + } + }, 120_000); +}); diff --git a/test/gh-aw-command-parse.test.ts b/test/gh-aw-command-parse.test.ts index 0ec8273df..a19782436 100644 --- a/test/gh-aw-command-parse.test.ts +++ b/test/gh-aw-command-parse.test.ts @@ -279,7 +279,7 @@ describe('gh-aw: shared /squad command contract (#1824)', () => { expect(posted[0].body).toContain('Issue-body revocations are not durable.'); }); - it('does not comment or throw for valid commands and no-invocation non-events', async () => { + it('fails closed for commands without identity and no-invocation non-events', async () => { const posted: string[] = []; const createComment = async (_issueNumber: number, body: string) => { posted.push(body); @@ -290,14 +290,14 @@ describe('gh-aw: shared /squad command contract (#1824)', () => { eventName: 'issue_comment', createComment, }), - ).resolves.toMatchObject({ status: 'accepted', mode: 'status' }); + ).rejects.toThrow('verified event actor'); await expect( enforceSquadCommandContract({ payload: comment('ordinary discussion'), eventName: 'issue_comment', createComment, }), - ).resolves.toEqual({ status: 'none', source: 'comment' }); + ).rejects.toThrow('No explicit Squad command'); expect(posted).toEqual([]); }); @@ -337,9 +337,9 @@ describe('gh-aw: shared /squad command contract (#1824)', () => { expect(DISCOVERY_WORKFLOW).toContain("startsWith(github.event.comment.body, '/squad ')"); expect(DISCOVERY_WORKFLOW).toContain('squad-command-contract.mjs'); expect(DISCOVERY_WORKFLOW).toContain('classifySquadCommand'); - expect(DISCOVERY_WORKFLOW).toContain('commandRequiresAuthorization'); + expect(DISCOVERY_WORKFLOW).toContain('authorizeSquadCommand'); expect(DISCOVERY_WORKFLOW).toContain('getCollaboratorPermissionLevel'); - expect(DISCOVERY_WORKFLOW).toContain('isAuthorizedPermission'); + expect(DISCOVERY_WORKFLOW).toContain('enforceSquadRouterOutputs'); expect(DISCOVERY_WORKFLOW).toContain('No standalone Squad command was found'); expect(DISCOVERY_WORKFLOW).toContain('rejectionComment'); expect(DISCOVERY_WORKFLOW).toContain('github.rest.issues.createComment'); diff --git a/test/gh-aw-command-router-relay.test.ts b/test/gh-aw-command-router-relay.test.ts index 103a6b11f..08f939cbc 100644 --- a/test/gh-aw-command-router-relay.test.ts +++ b/test/gh-aw-command-router-relay.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { afterAll, describe, expect, it } from 'vitest'; import { cpSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { execFileSync } from 'node:child_process'; import { tmpdir } from 'node:os'; @@ -37,6 +37,10 @@ const ROOT = process.cwd(); const ROUTER = readFileSync(join(ROOT, 'workflows/squad-command-router.md'), 'utf8'); const PACKAGE_ROUTER = readFileSync(join(ROOT, 'workflows/package/squad-command-router.md'), 'utf8'); const SHARED_SQUAD = readFileSync(join(ROOT, 'workflows/shared/squad.md'), 'utf8'); +const OUTPUT_DIR = mkdtempSync(join(tmpdir(), 'squad-router-output-')); +const OUTPUT_PATH = join(OUTPUT_DIR, 'output.json'); +writeFileSync(OUTPUT_PATH, JSON.stringify({ items: [{ type: 'noop', message: 'Routing complete' }] })); +afterAll(() => rmSync(OUTPUT_DIR, { recursive: true, force: true })); /** Read a `script: |` literal block starting at `marker`, dedented. */ function extractScript(source: string, marker: string): string { @@ -177,7 +181,10 @@ async function runRouter({ process.env.GITHUB_WORKSPACE = ROOT; process.env.SQUAD_EVENT_NAME = eventName; try { - await compileScript(inlined)(github, context, core, process, nodeRequire); + await compileScript(inlined)(github, context, core, { + cwd: () => ROOT, + env: { ...process.env, GH_AW_AGENT_OUTPUT: OUTPUT_PATH }, + }, nodeRequire); } finally { if (previousWorkspace === undefined) delete process.env.GITHUB_WORKSPACE; else process.env.GITHUB_WORKSPACE = previousWorkspace; @@ -188,14 +195,10 @@ async function runRouter({ } function assertAuthorBinding(source: string): void { - expect(source).toContain("process.env.SQUAD_EVENT_NAME === 'issue_comment'"); - expect(source).toContain('context.payload.comment?.user?.login'); - expect(source).toContain("process.env.SQUAD_EVENT_NAME === 'issues'"); - expect(source).toContain('context.payload.issue?.user?.login'); - expect(source).toContain('const actorAuthorized = Boolean(eventActor)'); - expect(source).toContain('const authorAuthorized = Boolean(commandAuthor)'); - expect(source).toContain('if (!actorAuthorized || !authorAuthorized)'); - expect(source).toContain('both the event actor and the author of the classified command text'); + expect(source).toContain('contract.authorizeSquadCommand({'); + expect(source).toContain('payload: context.payload,'); + expect(source).toContain('actor: context.actor,'); + expect(source).toContain('getCollaboratorPermissionLevel'); } describe('gh-aw: router-to-repair workflow_dispatch relay (#2, #3 findings)', () => { @@ -207,9 +210,7 @@ describe('gh-aw: router-to-repair workflow_dispatch relay (#2, #3 findings)', () // roles: all only removes gh-aw's blanket write-role activation gate; the // router's own in-script permission check for mutating modes must be the // thing still enforcing authorization afterward. - expect(ROUTER).toContain('contract.commandRequiresAuthorization(result)'); - expect(ROUTER).toContain('contract.isAuthorizedPermission(actorPermission)'); - expect(ROUTER).toContain('contract.isAuthorizedPermission(authorPermission)'); + expect(ROUTER).toContain('contract.authorizeSquadCommand({'); expect(ROUTER).toContain('getCollaboratorPermissionLevel'); assertAuthorBinding(ROUTER); expect(commandRequiresAuthorization({ status: 'accepted', mode: 'cast' })).toBe(true); @@ -240,7 +241,8 @@ describe('gh-aw: router-to-repair workflow_dispatch relay (#2, #3 findings)', () actor: 'maintainer', payload: { issue: { number: 4242 }, - comment: { id: 55001, body: commentBody, user: { login: 'maintainer' } }, + comment: { id: 55001, body: commentBody, user: { login: 'maintainer', type: 'User' } }, + sender: { login: 'maintainer', type: 'User' }, repository: { default_branch: 'dev' }, }, }; @@ -281,7 +283,10 @@ describe('gh-aw: router-to-repair workflow_dispatch relay (#2, #3 findings)', () process.env.GITHUB_WORKSPACE = ROOT; process.env.SQUAD_EVENT_NAME = 'issue_comment'; try { - await compileScript(inlined)(github, context, core, process, nodeRequire); + await compileScript(inlined)(github, context, core, { + cwd: () => ROOT, + env: { ...process.env, GH_AW_AGENT_OUTPUT: OUTPUT_PATH }, + }, nodeRequire); } finally { if (previousWorkspace === undefined) delete process.env.GITHUB_WORKSPACE; else process.env.GITHUB_WORKSPACE = previousWorkspace; @@ -409,7 +414,7 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () expect(result.permissionLookups).toEqual(['maintainer-editor', 'unprivileged-author']); expect(result.postedComments).toEqual([ expect.stringContaining( - 'Event actor: @maintainer-editor (write); command author: @unprivileged-author (read).', + 'command author unprivileged-author=read', ), ]); }); @@ -450,7 +455,7 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () expect(result.dispatchedInputs).toBeNull(); expect(result.failure).toContain('event actor unprivileged-editor=read'); expect(result.postedComments[0]).toContain( - 'Event actor: @unprivileged-editor (read); command author: @maintainer-author (admin).', + 'event actor unprivileged-editor=read', ); }); @@ -467,12 +472,12 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () }); expect(result.dispatchedInputs).toBeNull(); - expect(result.failure).toContain('command author unresolved=unresolved'); - expect(result.permissionLookups).toEqual(['maintainer']); - expect(result.postedComments[0]).toContain('command author: unresolved (unresolved)'); + expect(result.failure).toContain('identifiable human actor and command author'); + expect(result.permissionLookups).toEqual([]); + expect(result.postedComments[0]).toContain('identifiable human actor and command author'); }); - it('does not permission-gate non-mutating modes when author identity is absent', async () => { + it('fails closed on missing author identity even for open modes', async () => { const result = await runRouter({ eventName: 'issue_comment', action: 'edited', @@ -482,9 +487,9 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () permissions: {}, }); - expect(result.failure).toBeNull(); + expect(result.failure).toContain('identifiable human actor and command author'); expect(result.permissionLookups).toEqual([]); - expect(result.dispatchedInputs).toMatchObject({ command: 'status', issue_number: '4242' }); + expect(result.dispatchedInputs).toBeNull(); }); it.each([ @@ -741,7 +746,9 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () expect(result.dispatchedInputs).toBeNull(); expect(result.failure).toContain(failure); - expect(result.permissionLookups).toEqual(['reader', 'external-author']); + expect(result.permissionLookups).toEqual( + isAuthorizedPermission(actorPermission) ? ['reader', 'external-author'] : ['reader'], + ); }); it('allows comment revoke-improvement only when both the event actor and original author are authorized', async () => { @@ -761,7 +768,7 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () expect(result.dispatchedInputs).toMatchObject({ command: 'revoke-improvement' }); }); - it('kills the replay-gate mutation that redispatches an unchanged edited command', async () => { + it('retains replay protection in the shared authorization gate when the router check is removed', async () => { const gate = [ 'if (!contract.editedCommandShouldRoute(', ' context.payload,', @@ -784,18 +791,15 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () permissions: { maintainer: 'write' }, scriptMutation: script => script.replace(gate, ''), }); - expect(vulnerable.dispatchedInputs).toMatchObject({ command: 'status' }); + expect(vulnerable.dispatchedInputs).toBeNull(); + expect(vulnerable.failure).toContain('unchanged'); }); it('kills the author-binding mutation that substitutes the editor for the command author', async () => { expect(() => assertAuthorBinding( ROUTER.replace( - "const commandAuthorCandidate = process.env.SQUAD_EVENT_NAME === 'issue_comment'\n" + - ' ? context.payload.comment?.user?.login\n' + - " : process.env.SQUAD_EVENT_NAME === 'issues'\n" + - ' ? context.payload.issue?.user?.login\n' + - ' : null;', - 'const commandAuthorCandidate = context.actor;', + 'payload: context.payload,', + 'payload: untrustedPayload,', ), )).toThrow(); @@ -811,12 +815,8 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () 'unprivileged-author': 'read', }, scriptMutation: script => script.replace( - "const commandAuthorCandidate = process.env.SQUAD_EVENT_NAME === 'issue_comment'\n" + - ' ? context.payload.comment?.user?.login\n' + - " : process.env.SQUAD_EVENT_NAME === 'issues'\n" + - ' ? context.payload.issue?.user?.login\n' + - ' : null;', - 'const commandAuthorCandidate = context.actor;', + 'payload: context.payload,', + 'payload: { ...context.payload, comment: { ...context.payload.comment, user: context.payload.sender } },', ), }); expect(vulnerable.dispatchedInputs).toMatchObject({ command: 'cast' }); @@ -843,8 +843,7 @@ describe('gh-aw: mutating router authorization is bound to text provenance', () ); assertAuthorBinding(compiled); expect(compiled).toContain('contract.editedCommandShouldRoute('); - expect(compiled).toContain('context.payload.comment?.user?.login'); - expect(compiled).toContain('context.payload.issue?.user?.login'); + expect(compiled).toContain('contract.authorizeSquadCommand({'); } finally { rmSync(workspace, { recursive: true, force: true }); } @@ -919,7 +918,7 @@ describe('gh-aw: bot-authored /squad command text is never replayed as a trusted expect(result.dispatchedInputs).toMatchObject({ command: 'cast', issue_number: '4242' }); }); - it('kills the mutation that drops the bot-authorship block entirely', async () => { + it('retains shared bot-authorship enforcement if the router early return is removed', async () => { const dedentedMarker = "if (commandAuthorTypeCandidate === 'Bot' || commandSenderTypeCandidate === 'Bot') {\n" + " core.info('Ignoring a /squad command discovered in bot-authored issue or comment text.');\n" + ' return;\n' @@ -941,10 +940,11 @@ describe('gh-aw: bot-authored /squad command text is never replayed as a trusted return script.replace(dedentedMarker, ''); }, }); - expect(vulnerable.dispatchedInputs).toMatchObject({ command: 'cast' }); + expect(vulnerable.dispatchedInputs).toBeNull(); + expect(vulnerable.failure).toContain('identifiable human'); }); - it('kills the mutation that checks only the original author type and ignores the current sender', async () => { + it('retains shared sender enforcement if the router checks only the original author', async () => { const marker = "commandAuthorTypeCandidate === 'Bot' || commandSenderTypeCandidate === 'Bot'"; expect(extractScript(ROUTER, 'Route or reject discovered command')).toContain(marker); @@ -963,6 +963,7 @@ describe('gh-aw: bot-authored /squad command text is never replayed as a trusted return script.replace(marker, "commandAuthorTypeCandidate === 'Bot'"); }, }); - expect(vulnerable.dispatchedInputs).toMatchObject({ command: 'cast' }); + expect(vulnerable.dispatchedInputs).toBeNull(); + expect(vulnerable.failure).toContain('identifiable human'); }); }); diff --git a/test/gh-aw-quality.test.ts b/test/gh-aw-quality.test.ts index d5646c387..ddff3406e 100644 --- a/test/gh-aw-quality.test.ts +++ b/test/gh-aw-quality.test.ts @@ -348,7 +348,7 @@ describe('gh-aw: safe-output configuration', () => { it('each safe-output has a max value that is a positive integer ≤ 1000', () => { for (const [name, config] of Object.entries(safeOutputs)) { - if (name === 'data' || name === 'messages' || name === 'jobs' || name === 'steps' || name === 'allowed-domains') continue; + if (['data', 'messages', 'jobs', 'steps', 'allowed-domains', 'threat-detection'].includes(name)) continue; expect(config.max, `${name} should have a max field`).toBeDefined(); const max = config.max as number; expect(max, `${name}.max should be > 0`).toBeGreaterThan(0); @@ -1636,15 +1636,13 @@ describe('gh-aw: compiled workflow shell input security contract', () => { 'GitHub rejects the emitted lockfile before any job starts.', ).toEqual([]); expect(compiled).toContain( - "activationComments\\\":\\\"${{ !(startsWith(github.event.comment.body, '/squad approve-improvement') || startsWith(github.event.comment.body, '/squad revoke-improvement')) }}", + 'activationComments\\\":\\\"false', ); }, 20000); it('detects the previous activation-comments serialization mutation', () => { const source = readText(SQUAD_WORKFLOW); - const safeExpression = - "${{ !(startsWith(github.event.comment.body, '/squad approve-improvement') || " + - "startsWith(github.event.comment.body, '/squad revoke-improvement')) }}"; + const safeExpression = 'activation-comments: false'; const unsafeExpression = "${{ !startsWith(github.event.comment.body, '/squad approve-improvement') && " + "!startsWith(github.event.comment.body, '/squad revoke-improvement') }}"; @@ -1654,7 +1652,7 @@ describe('gh-aw: compiled workflow shell input security contract', () => { execFileSync('git', ['init', '--quiet'], { cwd: workspace }); cpSync(WORKFLOWS_DIR, join(workspace, '.github', 'workflows'), { recursive: true }); const mutantPath = join(workspace, '.github', 'workflows', 'squad.md'); - writeFileSync(mutantPath, readText(mutantPath).replace(safeExpression, unsafeExpression)); + writeFileSync(mutantPath, readText(mutantPath).replace(safeExpression, `activation-comments: ${unsafeExpression}`)); execFileSync('gh', ['aw', 'compile', mutantPath, '--strict', '--approve'], { cwd: workspace, @@ -3160,10 +3158,10 @@ describe('gh-aw: Cast replaces disposable bootstrap state (#1909)', () => { expect(frontmatter).toContain( 'run-success: "🤖 [{workflow_name}]({run_url}) finished processing. This completion message does not indicate Cast success. For Cast, only a linked Cast pull request indicates success."', ); - expect(cast).toContain('Built-in `report_incomplete` only warns'); - expect(cast).toContain('it does not fail the run'); + expect(frontmatter).toContain('activation-comments: false'); + expect(cast).toContain('makes `report_incomplete` fail the conclusion step'); + expect(cast).toContain('Diagnostics remain in Actions\nlogs and summaries'); expect(cast).toContain('Only a linked Cast PR is the success signal'); - expect(cast).toContain('A red `cast_failure` job can therefore still select this\nneutral hook; it does not select `run-failure`'); }); it('kills claims that the post-agent job independently enforces failure or PR authorization', () => { @@ -4004,8 +4002,8 @@ describe('gh-aw: canonical package integrity contract', () => { expect(mutable).not.toContain(actionReference); expect(createHash('sha256').update(normalizeCompiledLock(mutable, revisionA)).digest('hex')) .toBe(sourceBinding === 'workflow' - ? 'e28d35cd3ae0b920479279c777b26c68a339163a6886794ba82c1c04e28e75d6' - : '7077085c3f31824fbea10ad7e963e10e5cb67819523a1815b9795839fe97fb02'); + ? '8bf36b5dcef66c5983d35962b4da32140ca7a4942bf579a03cb43a04114b98b3' + : '205d84067db90e20a2753b60c329d258f27a9e4ba2b10a971bf8c31df4504048'); expect(() => validateCompilerActionPins(mutable)).toThrow(/invalid immutable action pin/); writeFileSync(lockPath, mutable); expect(verifyInstall(root).failures.join('\n')) @@ -4059,7 +4057,7 @@ describe('gh-aw: canonical package integrity contract', () => { compile(); const unpinned = readText(lockPath); expect(createHash('sha256').update(normalizeCompiledLock(unpinned, revisionA)).digest('hex')) - .toBe('7077085c3f31824fbea10ad7e963e10e5cb67819523a1815b9795839fe97fb02'); + .toBe('205d84067db90e20a2753b60c329d258f27a9e4ba2b10a971bf8c31df4504048'); expect(verifyInstall(root).failures.join('\n')).toContain('Installed digest mismatch'); const seedPins = () => spawnSync(process.execPath, ['--input-type=module', '-e', seed!], { diff --git a/test/helpers/gh-aw-diagnostic-runtime.ts b/test/helpers/gh-aw-diagnostic-runtime.ts new file mode 100644 index 000000000..0b8f44da6 --- /dev/null +++ b/test/helpers/gh-aw-diagnostic-runtime.ts @@ -0,0 +1,164 @@ +import { compileFunction, createContext } from 'node:vm'; +import { createRequire } from 'node:module'; +import { dirname, posix } from 'node:path'; +import { readFileSync, writeFileSync } from 'node:fs'; + +export const DIAGNOSTIC_RUNTIME_SHA = '2fbab69bfca02bebd76cd0fc43f2d12acfed994f'; +const requireNode = createRequire(import.meta.url); +const entrypoints = [ + 'handle_agent_failure.cjs', 'handle_noop_message.cjs', 'handle_detection_runs.cjs', + 'report_failed_jobs.cjs', 'notify_comment_error.cjs', 'missing_tool.cjs', + 'report_incomplete_handler.cjs', 'setup_globals.cjs', 'add_workflow_run_comment.cjs', +]; + +/** Fetch the real pinned handler closure, including its templates, without executing network clients. */ +export async function fetchDiagnosticRuntime() { + const sources = new Map(); + let pending = entrypoints.map(name => `js/${name}`); + while (pending.length) { + const batch = [...new Set(pending)].filter(name => !sources.has(name)); + pending = []; + await Promise.all(batch.map(async name => { + const response = await fetch( + `https://raw.githubusercontent.com/github/gh-aw-actions/${DIAGNOSTIC_RUNTIME_SHA}/setup/${name.replace(/^prompts\//, 'md/')}`, + { signal: AbortSignal.timeout(30_000) }, + ); + if (!response.ok) throw new Error(`Pinned runtime fetch failed: ${name} HTTP ${response.status}`); + sources.set(name, await response.text()); + })); + for (const name of batch.filter(name => name.endsWith('.cjs'))) { + const source = sources.get(name)!; + for (const [, dependency] of source.matchAll(/require\(["'](\.[^"']+)["']\)/g)) { + pending.push(posix.normalize(posix.join(posix.dirname(name), dependency))); + } + for (const [, template] of source.matchAll(/getPromptPath\(["']([^"']+)["']\)/g)) { + pending.push(`prompts/${template}`); + } + } + } + return sources; +} + +/** + * Only IO is substituted: real handlers, loaders, boolean parsing, sanitizers and + * templates execute unchanged. GitHub writes are recorded locally, never sent. + */ +export function diagnosticRuntime( + sources: Map, scratch: string, items: Record[], + env: Record, existingIssue = false, +) { + const outputPath = `${scratch}/diagnostic-output.json`; + writeFileSync(outputPath, JSON.stringify({ items })); + const logs: string[] = []; + const failures: string[] = []; + const writes: { method: string; params: Record }[] = []; + const calls: string[] = []; + const ioErrors: string[] = []; + const summary = { + addRaw: (text: string) => { logs.push(text); return summary; }, + addHeading: (text: string) => { logs.push(text); return summary; }, + addTable: () => summary, + write: async () => {}, + }; + const core = { + info: (text: string) => logs.push(text), warning: (text: string) => logs.push(text), + debug: (text: string) => logs.push(text), error: (text: string) => logs.push(text), + setFailed: (text: string) => failures.push(text), setOutput: () => {}, + startGroup: () => {}, endGroup: () => {}, summary, + }; + const api = (method: string) => async (params: Record = {}) => { + calls.push(method); + if (method === 'search.issuesAndPullRequests' && String(params.q).includes('is:pr')) return { data: { total_count: 0, items: [] } }; + if (method === 'search.issuesAndPullRequests') return { data: { + total_count: existingIssue ? 1 : 0, + items: existingIssue ? [{ number: 900, node_id: 'I_tracking', html_url: 'https://github.com/owner/repo/issues/900', body: '' }] : [], + } }; + if (method === 'actions.listJobsForWorkflowRun') return { data: { + jobs: [{ name: 'upsert_lifecycle_state', conclusion: 'failure', html_url: 'https://github.com/owner/repo/actions/runs/1/job/2' }], + } }; + if (method === 'rateLimit.get') return { data: { resources: { core: { remaining: 5000, limit: 5000 } } } }; + if (method === 'issues.listComments') return { data: [] }; + if (method === 'issues.get') return { data: { + number: 900, state: 'open', labels: [], body: '', html_url: 'https://github.com/owner/repo/issues/900', + } }; + if (method === 'pulls.list') return { data: [] }; + if (method === 'repos.get') return { data: { default_branch: 'dev' } }; + if (['issues.create', 'issues.update', 'issues.createComment', 'issues.addLabels', + 'POST /repos/{owner}/{repo}/issues/{issue_number}/comments', + 'POST /repos/{owner}/{repo}/issues/{issue_number}/labels', + 'PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}'].includes(method)) { + writes.push({ method, params }); + return { data: { number: 900, id: 901, node_id: 'I_tracking', html_url: 'https://github.com/owner/repo/issues/900' } }; + } + ioErrors.push(`Unexpected GitHub IO: ${method}`); + throw new Error(ioErrors.at(-1)); + }; + const rest = new Proxy({}, { + get: (_target, group: string) => new Proxy({}, { + get: (_target, method: string) => api(`${group}.${method}`), + }), + }); + const github = { + rest, request: (method: string, params: Record) => api(method)(params), + hook: { before: () => {} }, + }; + const runtimeEnv = { + 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', + ...env, GH_AW_AGENT_OUTPUT: outputPath, + }; + const context = { + repo: { owner: 'owner', repo: 'repo' }, runId: 1, eventName: 'issue_comment', + payload: { issue: { number: 42 }, repository: { default_branch: 'dev' } }, + }; + const sandbox = createContext({ + core, github, context, console, Buffer, URL, TextDecoder, TextEncoder, + process: { env: runtimeEnv, cwd: () => scratch }, + }); + sandbox.global = sandbox; + const modules = new Map }>(); + const filesystem = { + existsSync: (path: string) => path === outputPath, + readFileSync: (path: string) => { + if (path === outputPath) return readFileSync(outputPath, 'utf8'); + const template = sources.get(`prompts/${String(path).split('/prompts/')[1]}`); + if (template !== undefined) return template; + // Optional runner logs are absent; never read outside this fixture. + throw Object.assign(new Error(`Fixture file absent: ${path}`), { code: 'ENOENT' }); + }, + readdirSync: () => [], + mkdirSync: () => {}, + writeFileSync: () => {}, + }; + 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}`); + 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'].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'; + ioErrors.push(`Forbidden external IO: ${request}.${method}`); + throw new Error(ioErrors.at(-1)); + }, + }); + throw new Error(`Forbidden runtime dependency: ${request}`); + }; + compileFunction(source, ['module', 'exports', 'require', '__dirname', '__filename'], + { parsingContext: sandbox })(module, module.exports, require, dirname(name), name); + return module.exports; + }; + return { + logs, failures, writes, calls, ioErrors, + run: async (name: string) => { + load('js/setup_globals.cjs').setupGlobals(core, github, context, {}, {}, () => github); + return load(`js/${name}`).main(); + }, + }; +} diff --git a/workflows/package/squad-command-router.md b/workflows/package/squad-command-router.md index abab7e901..ef01615e6 100644 --- a/workflows/package/squad-command-router.md +++ b/workflows/package/squad-command-router.md @@ -35,6 +35,11 @@ permissions: contents: read copilot-requests: write safe-outputs: + report-failure-as-issue: false + report-failed-jobs: false + threat-detection: + report-as-issue: false + activation-comments: false steps: - name: Checkout executing workflow commit for command routing uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 @@ -46,6 +51,7 @@ safe-outputs: uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 env: SQUAD_EVENT_NAME: ${{ github.event_name }} + GH_AW_AGENT_OUTPUT: ${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }} with: script: | const nodePath = require('node:path'); @@ -55,6 +61,7 @@ safe-outputs: trustedRoot, '.github/workflows/shared/squad-command-contract.mjs', )).href); + contract.enforceSquadRouterOutputs(process.env.GH_AW_AGENT_OUTPUT); const result = contract.classifySquadCommand( context.payload, process.env.SQUAD_EVENT_NAME, @@ -119,57 +126,25 @@ safe-outputs: core.info('No standalone Squad command was found after excluding code contexts.'); return; } - if (contract.commandRequiresAuthorization(result)) { - const eventActor = typeof context.actor === 'string' && context.actor.trim() - ? context.actor.trim() - : null; - const commandAuthorCandidate = process.env.SQUAD_EVENT_NAME === 'issue_comment' - ? context.payload.comment?.user?.login - : process.env.SQUAD_EVENT_NAME === 'issues' - ? context.payload.issue?.user?.login - : null; - const commandAuthor = typeof commandAuthorCandidate === 'string' && commandAuthorCandidate.trim() - ? commandAuthorCandidate.trim() - : null; - const permissionByLogin = new Map(); - const resolvePermission = async (login, principal) => { - if (!login) return 'unresolved'; - if (permissionByLogin.has(login)) return permissionByLogin.get(login); - let permission = 'unresolved'; - try { - permission = (await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: login, - })).data.permission || 'unresolved'; - } catch (error) { - core.warning(`Unable to resolve repository permission for ${principal} ${login}: ${error.message}`); - } - permissionByLogin.set(login, permission); - return permission; - }; - const actorPermission = await resolvePermission(eventActor, 'event actor'); - const authorPermission = await resolvePermission(commandAuthor, 'command author'); - const actorAuthorized = Boolean(eventActor) && - contract.isAuthorizedPermission(actorPermission); - const authorAuthorized = Boolean(commandAuthor) && - contract.isAuthorizedPermission(authorPermission); - if (!actorAuthorized || !authorAuthorized) { - const actorEvidence = eventActor - ? `@${eventActor} (${actorPermission})` - : 'unresolved (unresolved)'; - const authorEvidence = commandAuthor - ? `@${commandAuthor} (${authorPermission})` - : 'unresolved (unresolved)'; - await github.rest.issues.createComment({ + try { + await contract.authorizeSquadCommand({ + payload: context.payload, + eventName: process.env.SQUAD_EVENT_NAME, + actor: context.actor, + result, + resolvePermission: async (login) => (await github.rest.repos.getCollaboratorPermissionLevel({ ...context.repo, - issue_number: issueNumber, - body: `⛔ /squad ${result.argumentText || 'cast'} was refused. Mutating /squad modes require write, maintain, or admin repository permission for both the event actor and the author of the classified command text. Event actor: ${actorEvidence}; command author: ${authorEvidence}. Ask a repository maintainer to author and run this command.`, - }); - core.setFailed( - `Squad refused mutating mode ${result.mode}; event actor ${eventActor || 'unresolved'}=${actorPermission}, command author ${commandAuthor || 'unresolved'}=${authorPermission}.`, - ); - return; - } + username: login, + })).data.permission, + }); + } catch (error) { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: issueNumber, + body: `Squad refused this command: ${error.message}`, + }); + core.setFailed(error.message); + return; } // Forward the originating comment (when this run was triggered by // one) through the typed `aw_context` relay input. The dispatched @@ -203,6 +178,7 @@ safe-outputs: max: 1 noop: max: 1 + report-as-issue: false --- diff --git a/workflows/package/squad-deps-worker.md b/workflows/package/squad-deps-worker.md index 898db0684..9353031ba 100644 --- a/workflows/package/squad-deps-worker.md +++ b/workflows/package/squad-deps-worker.md @@ -12,6 +12,7 @@ safe-outputs: description: Create or replace the single Squad research artifact for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -147,6 +148,7 @@ safe-outputs: description: Update the single Squad planning lifecycle comment for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write diff --git a/workflows/package/squad-implement-worker.md b/workflows/package/squad-implement-worker.md index 32e862cc7..f888e1978 100644 --- a/workflows/package/squad-implement-worker.md +++ b/workflows/package/squad-implement-worker.md @@ -12,6 +12,7 @@ safe-outputs: description: Create or replace the single Squad research artifact for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -147,6 +148,7 @@ safe-outputs: description: Update the single Squad planning lifecycle comment for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write diff --git a/workflows/package/squad-retro.md b/workflows/package/squad-retro.md index c71e1ab66..3c8d908f4 100644 --- a/workflows/package/squad-retro.md +++ b/workflows/package/squad-retro.md @@ -12,6 +12,7 @@ safe-outputs: description: Create or replace the single Squad research artifact for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -147,6 +148,7 @@ safe-outputs: description: Update the single Squad planning lifecycle comment for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write diff --git a/workflows/package/squad.md b/workflows/package/squad.md index e0861daf6..0dc798f89 100644 --- a/workflows/package/squad.md +++ b/workflows/package/squad.md @@ -12,6 +12,7 @@ safe-outputs: description: Create or replace the single Squad research artifact for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -147,6 +148,7 @@ safe-outputs: description: Update the single Squad planning lifecycle comment for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -353,10 +355,17 @@ safe-outputs: 'Stderr:', failure.stderr, ].join('\n')); + report-failure-as-issue: false + report-failed-jobs: false + threat-detection: + report-as-issue: false + noop: + max: 1 + report-as-issue: false allowed-domains: - learn.microsoft.com - aspire.dev - activation-comments: ${{ !(startsWith(github.event.comment.body, '/squad approve-improvement') || startsWith(github.event.comment.body, '/squad revoke-improvement')) }} + activation-comments: false steps: - name: Checkout executing workflow commit for command enforcement uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 @@ -369,6 +378,7 @@ safe-outputs: env: GITHUB_TOKEN: ${{ github.token }} SQUAD_EVENT_NAME: ${{ github.event_name }} + GH_AW_AGENT_OUTPUT: ${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }} with: script: | const nodePath = require('node:path'); @@ -382,6 +392,12 @@ safe-outputs: await contract.enforceSquadCommandContract({ payload: context.payload, eventName: process.env.SQUAD_EVENT_NAME, + actor: context.actor, + agentOutputPath: process.env.GH_AW_AGENT_OUTPUT, + resolvePermission: async (login) => (await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: login, + })).data.permission, createComment: async (issueNumber, body) => { await github.rest.issues.createComment({ ...context.repo, @@ -2228,14 +2244,14 @@ gate `create-pull-request`. It diagnoses a `cast_failure` and outputs run separately after the agent; this diagnosis cannot prevent a pull request that is materialized concurrently. -Built-in `report_incomplete` only warns and creates or updates a durable tracking -issue; it does not fail the run. The factual failure comment and, when emitted, -the red `cast_failure` job are the Cast failure signals. The generic completion -hook is deliberately neutral because pinned gh-aw chooses `run-success` from -the agent and consolidated `safe_outputs` results without considering the -custom job result. A red `cast_failure` job can therefore still select this -neutral hook; it does not select `run-failure`. Processing completion does not -indicate Cast success. Only a linked Cast PR is the success signal. +Pinned gh-aw v0.89.22 makes `report_incomplete` fail the conclusion step. +Automatic failure, failed-job, detection, no-op tracking issues and generic +activation/completion comments are disabled in the dispatcher and router: +conclusion runs independently of safe-output authorization and completion +comments otherwise append unvalidated no-op text. Diagnostics remain in Actions +logs and summaries; detection still runs. The factual, guarded failure comment +and, when emitted, the red `cast_failure` job remain the Cast failure signals. +Only a linked Cast PR is the success signal. ##### Step 8: Open PR @@ -3566,7 +3582,7 @@ Root → Epics → Tasks. Phase-specific: filter to matching phase heading. While Steps 2b/2c run, keep two counts: `activated` (issues created or recognized this run) and `labeled` (issues whose `add_labels` call was accepted). An `add_labels` call that was never made, was rejected, or returned an error counts as **unlabeled**. These counts track *label operations*, not labels present on GitHub: acceptance means the call was queued for a specific target this turn, and gh-aw applies it in the post-agent job. Never state or imply that a counted label was applied, landed, or was confirmed on the issue — nothing here reads labels back. At the end of Step 2: 1. `labeled == activated` → the activation is complete; proceed to Step 3. -2. `labeled < activated` → **this is an incomplete activation, not a successful one.** Call `report_incomplete` with a `reason` naming the shortfall (`{labeled} of {activated} activated issues had a label operation accepted`) and `details` listing **every affected work item — the identifier you used to target its `add_labels` call, its title, and the label set that missing operation targeted**. For an item created this run that identifier is the `temporary_id` you minted under the Temporary-ID Contract (`#aw_epic{K}` / `#aw_task{N}`) — not a GitHub issue number, because creation is deferred to the safe-output job and no real number exists yet. Quote a real number only where one is independently verified: an epic or task matched by dedup-by-title, or an issue recognized by Step 1's idempotent-rerun path. Never predict, infer, or invent a number. `report_incomplete` logs a warning and opens or updates a durable `[aw] ... reported incomplete result` tracking issue; it does **not** change the run's conclusion — the run still reports success. That record and the rule below keep a truncated activation from passing as clean, so never rely on a red run to carry the signal. +2. `labeled < activated` → **this is an incomplete activation, not a successful one.** Call `report_incomplete` with a `reason` naming the shortfall (`{labeled} of {activated} activated issues had a label operation accepted`) and `details` listing **every affected work item — the identifier you used to target its `add_labels` call, its title, and the label set that missing operation targeted**. For an item created this run that identifier is the `temporary_id` you minted under the Temporary-ID Contract (`#aw_epic{K}` / `#aw_task{N}`) — not a GitHub issue number, because creation is deferred to the safe-output job and no real number exists yet. Quote a real number only where one is independently verified: an epic or task matched by dedup-by-title, or an issue recognized by Step 1's idempotent-rerun path. Never predict, infer, or invent a number. `report_incomplete` records the reason in Actions logs and fails the conclusion step; automatic tracking-issue reporting is disabled. Include the shortfall in the guarded activation summary as well, so the originating issue retains an actionable signal. **Cap exhaustion is a reportable, nameable cause.** If the shortfall is because a cap was reached, say so in the `reason`, name which cap (`create-issue` 75 or `add_labels` 110) and list the work items that did not fit, and recommend `/squad plan activate phase {N}`. This is the one case where a cap may be named as the cause: it was observed, not guessed. Do not infer a cap from a rejection you never received, and do not treat the absence of an `E002` error as proof that every label operation was accepted: the count comparison, not the error stream, is the authority. diff --git a/workflows/shared/squad-command-contract.mjs b/workflows/shared/squad-command-contract.mjs index b153619af..d65f25cd2 100644 --- a/workflows/shared/squad-command-contract.mjs +++ b/workflows/shared/squad-command-contract.mjs @@ -292,9 +292,126 @@ export function rejectionComment(result) { ].join('\n'); } -export async function enforceSquadCommandContract({ payload, eventName, createComment }) { +export async function authorizeSquadCommand({ payload, eventName, actor, resolvePermission, result }) { + if (result?.status !== 'accepted') throw new Error('An accepted command result is required.'); + const sender = payload?.sender; + if (typeof actor !== 'string' || !actor.trim() || sender?.login !== actor || + !['User', 'Bot'].includes(sender?.type)) { + throw new Error('Squad command authorization requires a verified event actor and sender.'); + } + // GitHub authorizes workflow_dispatch with Actions write permission, including + // GITHUB_TOKEN router/worker continuations. Inputs never select this trust path. + if (eventName === 'workflow_dispatch' && result.source === 'workflow_dispatch') return; + if (!['issues', 'issue_comment'].includes(eventName)) { + throw new Error(`Unsupported Squad command event: ${eventName}`); + } + const author = eventName === 'issue_comment' ? payload?.comment?.user : payload?.issue?.user; + if (sender.type !== 'User' || author?.type !== 'User' || + typeof author.login !== 'string' || !author.login.trim()) { + throw new Error('Squad issue commands require identifiable human actor and command author.'); + } + if (!editedCommandShouldRoute(payload, eventName, result)) { + throw new Error('Squad edited command is unchanged or previous-body evidence is unavailable.'); + } + if (!commandRequiresAuthorization(result)) return; + if (typeof resolvePermission !== 'function') { + throw new Error('Squad command permission resolver is unavailable.'); + } + const checked = new Set(); + for (const [principal, login] of [['event actor', actor], ['command author', author.login]]) { + if (checked.has(login)) continue; + let permission; + try { + permission = await resolvePermission(login); + } catch (error) { + throw new Error(`Unable to resolve Squad repository permission for ${login}: ${error.message}`); + } + if (!isAuthorizedPermission(permission)) { + throw new Error(`Squad refused mutating mode ${result.mode}: ${principal} ${login}=${permission || 'unresolved'}. Both the event actor and command author require write, maintain, or admin.`); + } + checked.add(login); + } +} + +function validateOpenModeOutputs(result, payload, items) { + const issueNumber = Number(result.source === 'workflow_dispatch' + ? payload?.inputs?.issue_number : payload?.issue?.number); + const types = new Set(['noop', 'missing_data', 'report_incomplete', 'add_comment']); + if (result.mode === 'research') types.add('upsert_research_artifact'); + if (['research', 'plan'].includes(result.mode)) types.add('upsert_lifecycle_state'); + for (const item of items) { + if (!item || !types.has(item.type)) { + throw new Error(`Squad open mode ${result.mode} cannot emit ${item?.type || 'untyped output'}.`); + } + if (['noop', 'missing_data', 'report_incomplete'].includes(item.type)) continue; + if (!Number.isSafeInteger(issueNumber) || issueNumber <= 0) { + 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']; + if (Object.keys(item).some(key => !keys.includes(key))) { + throw new Error('Squad open-mode output contains unsupported mutation fields.'); + } + if (item.type === 'add_comment' && item.item_number !== undefined && + 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.'); + } + if (item.data !== undefined) { + const data = item.data; + if (result.mode !== 'plan' || !data || typeof data !== 'object' || + 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 || + !Array.isArray(data.phases) || data.phases.length !== 0) { + throw new Error('Squad open mode cannot emit acceptance, activation, or unrelated artifacts.'); + } + } + if (item.type === 'upsert_lifecycle_state') { + const expected = result.mode === 'research' ? 'Researched' : 'Planned'; + const states = [...item.body.matchAll(/^(?:[-*]\s+)?\*\*(?:Current state|State):\*\*\s+(.+)$/gim)]; + const commands = [...item.body.matchAll(/^(?:[-*]\s+)?\*\*Last command:\*\*\s+`([^`]+)`[ \t]*$/gim)]; + if (states.length !== 1 || states[0][1].trim() !== expected || + commands.length !== 1 || + ![`/squad ${result.mode}`, `/squad ${result.argumentText}`].includes(commands[0][1])) { + throw new Error('Squad open-mode lifecycle must describe only the requested research or plan.'); + } + } + } +} + +function readOutputItems(agentOutputPath) { + if (!agentOutputPath) throw new Error('GH_AW_AGENT_OUTPUT is required for Squad output authorization.'); + const output = JSON.parse(readFileSync(agentOutputPath, 'utf8')); + if (!Array.isArray(output?.items)) throw new Error('Squad agent output must contain an items array.'); + return output.items; +} + +export function enforceSquadRouterOutputs(agentOutputPath) { + for (const item of readOutputItems(agentOutputPath)) { + if (!item || !['noop', 'missing_data', 'report_incomplete'].includes(item.type)) { + throw new Error('Squad router agent may emit diagnostics only; routing is deterministic.'); + } + } +} + +export async function enforceSquadCommandContract({ + payload, eventName, createComment, actor, resolvePermission, agentOutputPath, +}) { const result = classifySquadCommand(payload, eventName); - if (result.status !== 'rejected') return result; + if (result.status === 'accepted') { + await authorizeSquadCommand({ payload, eventName, actor, resolvePermission, result }); + const items = readOutputItems(agentOutputPath); + if (!commandRequiresAuthorization(result)) validateOpenModeOutputs(result, payload, items); + return result; + } + if (result.status === 'none') throw new Error('No explicit Squad command; refusing all safe outputs.'); const issueNumber = Number(payload?.issue?.number ?? payload?.pull_request?.number); if (!Number.isInteger(issueNumber) || issueNumber <= 0) { throw new Error(`Rejected command has no valid issue or pull request target: ${result.rejectedCommand}`); diff --git a/workflows/shared/squad.md b/workflows/shared/squad.md index 07776be17..1d5e21c5b 100644 --- a/workflows/shared/squad.md +++ b/workflows/shared/squad.md @@ -46,6 +46,7 @@ safe-outputs: description: Create or replace the single Squad research artifact for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write @@ -182,6 +183,7 @@ safe-outputs: description: Update the single Squad planning lifecycle comment for this issue. runs-on: ubuntu-slim needs: safe_outputs + if: needs.safe_outputs.result == 'success' permissions: issues: write pull-requests: write diff --git a/workflows/squad-command-router.md b/workflows/squad-command-router.md index c16d57e11..067567c8d 100644 --- a/workflows/squad-command-router.md +++ b/workflows/squad-command-router.md @@ -32,6 +32,12 @@ permissions: resources: - shared/squad-command-contract.mjs safe-outputs: + # Conclusion runs even when deterministic routing rejects the agent output. + report-failure-as-issue: false + report-failed-jobs: false + threat-detection: + report-as-issue: false + activation-comments: false steps: - name: Checkout executing workflow commit for command routing uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -43,6 +49,7 @@ safe-outputs: uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 env: SQUAD_EVENT_NAME: ${{ github.event_name }} + GH_AW_AGENT_OUTPUT: ${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }} with: script: | const nodePath = require('node:path'); @@ -52,6 +59,7 @@ safe-outputs: trustedRoot, '.github/workflows/shared/squad-command-contract.mjs', )).href); + contract.enforceSquadRouterOutputs(process.env.GH_AW_AGENT_OUTPUT); const result = contract.classifySquadCommand( context.payload, process.env.SQUAD_EVENT_NAME, @@ -116,57 +124,25 @@ safe-outputs: core.info('No standalone Squad command was found after excluding code contexts.'); return; } - if (contract.commandRequiresAuthorization(result)) { - const eventActor = typeof context.actor === 'string' && context.actor.trim() - ? context.actor.trim() - : null; - const commandAuthorCandidate = process.env.SQUAD_EVENT_NAME === 'issue_comment' - ? context.payload.comment?.user?.login - : process.env.SQUAD_EVENT_NAME === 'issues' - ? context.payload.issue?.user?.login - : null; - const commandAuthor = typeof commandAuthorCandidate === 'string' && commandAuthorCandidate.trim() - ? commandAuthorCandidate.trim() - : null; - const permissionByLogin = new Map(); - const resolvePermission = async (login, principal) => { - if (!login) return 'unresolved'; - if (permissionByLogin.has(login)) return permissionByLogin.get(login); - let permission = 'unresolved'; - try { - permission = (await github.rest.repos.getCollaboratorPermissionLevel({ - ...context.repo, - username: login, - })).data.permission || 'unresolved'; - } catch (error) { - core.warning(`Unable to resolve repository permission for ${principal} ${login}: ${error.message}`); - } - permissionByLogin.set(login, permission); - return permission; - }; - const actorPermission = await resolvePermission(eventActor, 'event actor'); - const authorPermission = await resolvePermission(commandAuthor, 'command author'); - const actorAuthorized = Boolean(eventActor) && - contract.isAuthorizedPermission(actorPermission); - const authorAuthorized = Boolean(commandAuthor) && - contract.isAuthorizedPermission(authorPermission); - if (!actorAuthorized || !authorAuthorized) { - const actorEvidence = eventActor - ? `@${eventActor} (${actorPermission})` - : 'unresolved (unresolved)'; - const authorEvidence = commandAuthor - ? `@${commandAuthor} (${authorPermission})` - : 'unresolved (unresolved)'; - await github.rest.issues.createComment({ + try { + await contract.authorizeSquadCommand({ + payload: context.payload, + eventName: process.env.SQUAD_EVENT_NAME, + actor: context.actor, + result, + resolvePermission: async (login) => (await github.rest.repos.getCollaboratorPermissionLevel({ ...context.repo, - issue_number: issueNumber, - body: `⛔ /squad ${result.argumentText || 'cast'} was refused. Mutating /squad modes require write, maintain, or admin repository permission for both the event actor and the author of the classified command text. Event actor: ${actorEvidence}; command author: ${authorEvidence}. Ask a repository maintainer to author and run this command.`, - }); - core.setFailed( - `Squad refused mutating mode ${result.mode}; event actor ${eventActor || 'unresolved'}=${actorPermission}, command author ${commandAuthor || 'unresolved'}=${authorPermission}.`, - ); - return; - } + username: login, + })).data.permission, + }); + } catch (error) { + await github.rest.issues.createComment({ + ...context.repo, + issue_number: issueNumber, + body: `Squad refused this command: ${error.message}`, + }); + core.setFailed(error.message); + return; } // Forward the originating comment (when this run was triggered by // one) through the typed `aw_context` relay input. The dispatched @@ -199,6 +175,7 @@ safe-outputs: max: 1 noop: max: 1 + report-as-issue: false --- Command routing and rejection are enforced deterministically by the safe-output diff --git a/workflows/squad-workflows.manifest.json b/workflows/squad-workflows.manifest.json index 3164bc8cb..6d3853d73 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": "1c199b9d97f76a84dbd3e10124d70a70abe3fdf4317670c2fa77ebb9a10b1e0a", - "lock_sha256": "3b5dfcb59cea903a2df3bdd3c9d72547f1edf6577110095fd431a5f92433bf8f", - "package_lock_sha256": "1b8859bb3b7b257e9ef99324c0f13f31604b74f423f614f97603bf59f22a84a3" + "source_sha256": "1d794f17bb04a511a10c9d1f405100f939783242425bccc8c3a824efa86f0342", + "lock_sha256": "c83c06fe7157c5b888c929ee3ed895a5c170f8483633208ccff6f893850d758b", + "package_lock_sha256": "821520d5a76ed7a6641b643831367e63fc87d06baa79d7cc5448fd5340d96355" }, { "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": "390e8c6f2614032b4b21113184a3d258d105633a94bff4bdb12d34b4a318bc02", - "lock_sha256": "84f55c7040274ef65e4e22f0a262644599a49f8508f587fb8c228ec76ca6016b", - "package_lock_sha256": "cb222eb5d14d7ec3ecee863ba3f709dc84cb00c952eeeb0fa105bc430e42f6e0" + "source_sha256": "4e278da658ea7e117a8c2297c399e05b76d8db8310f8d09e29688b9d1df25a8c", + "lock_sha256": "28c6abc965f919d1a2aa2e62c852afafa9852b981dd0296e27c5a0c991be339b", + "package_lock_sha256": "322e77768d24f43133c332531c0511d84440c6750218d2a0b5b7c8437ef03c32" }, { "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": "825d55fcec5fa1e736e2ddac8d30815e86c8c9fc45f8d2729298669e3130f70d", - "lock_sha256": "e10c2324df670321f5029c61a1508c27274782f67106aabf15577063141c7ecb", - "package_lock_sha256": "45511377b5852617d265a7c6ea49ee02fba253ed8dda7de90369af0d24d36480" + "source_sha256": "59400d542e3968a2836f83206e2e29533bead5fdc72ee8874bce2a866cc67053", + "lock_sha256": "8cde3407a88e3cdd7a2fbed4f587bae9106267ac9a3e77cd75cce5f29afee0dc", + "package_lock_sha256": "3546ee6f4706da3fdbfca77fa249c958115d6758bd986895e2c16bea81c52b94" }, { "name": "squad-retro", "source": "workflows/package/squad-retro.md", "destination": ".github/workflows/squad-retro.md", "lock": ".github/workflows/squad-retro.lock.yml", - "source_sha256": "a30247754ca0d3d2f48f131f617fde48829bd265202575b28f8512b30daeb35d", - "lock_sha256": "6153ee5dd2d5810e7aeb81f2173b672bb61c0d6989642f07d21d69d9111f7eda", - "package_lock_sha256": "addc84da8c083e57d05068b3ae1b8092c3156fee9d7a202eac9bd4aaebaf4b51" + "source_sha256": "d50fd229085c859008fd84a306d70ef16c3467700128fda489837389e975e55d", + "lock_sha256": "42ef0631810e60dbd935357d02e93a8a0c8bfb67e1f86c1cc869ac7daa1c863c", + "package_lock_sha256": "1cfbad4d25e78341dea078f51eb65a6c8191dd9ebc9f900f19f748081292dcd0" }, { "name": "squad-improvement-worker", @@ -77,9 +77,9 @@ "source": "workflows/package/squad-command-router.md", "destination": ".github/workflows/squad-command-router.md", "lock": ".github/workflows/squad-command-router.lock.yml", - "source_sha256": "451d5b1b11d5a5778595f9bed8d532c09b353156ef763efe1170e0b95f967011", - "lock_sha256": "39aa2b8705b817a47b9e7d4f4406bf2e69243c0b4b21184b65d577b23b4382d9", - "package_lock_sha256": "1a11377af6525bdfd0c2ec20d8aadf2636a3c6989f83569068410b6e0afadde4" + "source_sha256": "61d2f598c5f0ad5ef2e26cbc55650cde5eabb3fc5bd4808a56eb8d36a1fa4c39", + "lock_sha256": "5584565e384ac76c0ce59a8317bfc5c63ce203a8fc46ef04941d6201bb73b158", + "package_lock_sha256": "350153e91b71ac0b84d3bcc7d965844e2a0291fc8cde5bb6576f18bd1a05cf49" } ], "shared_runtime": [ @@ -105,7 +105,7 @@ "package_destination": ".github/workflows/shared/squad-command-contract.mjs", "destination": ".github/workflows/shared/squad-command-contract.mjs", "ownership": "manifest", - "sha256": "cbe2506688c4137f9074452ba0d55eed073fb0f33aec39bae1be82b71806c2ec" + "sha256": "04fc4a578cd87de13964d3e154b6683d9b5851c5713c6fbe183ee5d24ce38aeb" }, { "path": "shared/squad-cast-validator.mjs", @@ -161,7 +161,7 @@ "package_destination": ".github/aw/squad/runtime/shared/squad.md", "destination": ".github/workflows/shared/squad.md", "ownership": "manifest", - "sha256": "62d48dcaaffcb7ed2c2713e92351a15073f780a252ed7d951bea0ea157778022" + "sha256": "b0dfa64dcf61aea6eaef65a0eac030a7e595de2b19a33d1a2a1089bf100808d0" }, { "path": "shared/squad-planning-ontology.md", diff --git a/workflows/squad.md b/workflows/squad.md index 1a93f9385..1310960f5 100644 --- a/workflows/squad.md +++ b/workflows/squad.md @@ -209,10 +209,19 @@ pre-agent-steps: const result = await gate.validateImprovementCommand(context.payload, process.env); if (!result.ok) core.setFailed(gate.describeViolations(result.violations).join('; ')); safe-outputs: + # Conclusion runs independently of the command guard; diagnostics must not write tracking issues. + report-failure-as-issue: false + report-failed-jobs: false + threat-detection: + report-as-issue: false + noop: + max: 1 + report-as-issue: false allowed-domains: - learn.microsoft.com - aspire.dev - activation-comments: ${{ !(startsWith(github.event.comment.body, '/squad approve-improvement') || startsWith(github.event.comment.body, '/squad revoke-improvement')) }} + # Completion comments append raw noop text even when the safe-output guard fails. + activation-comments: false steps: - name: Checkout executing workflow commit for command enforcement uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -225,6 +234,7 @@ safe-outputs: env: GITHUB_TOKEN: ${{ github.token }} SQUAD_EVENT_NAME: ${{ github.event_name }} + GH_AW_AGENT_OUTPUT: ${{ steps.setup-agent-output-env.outputs.GH_AW_AGENT_OUTPUT }} with: script: | const nodePath = require('node:path'); @@ -238,6 +248,12 @@ safe-outputs: await contract.enforceSquadCommandContract({ payload: context.payload, eventName: process.env.SQUAD_EVENT_NAME, + actor: context.actor, + agentOutputPath: process.env.GH_AW_AGENT_OUTPUT, + resolvePermission: async (login) => (await github.rest.repos.getCollaboratorPermissionLevel({ + ...context.repo, + username: login, + })).data.permission, createComment: async (issueNumber, body) => { await github.rest.issues.createComment({ ...context.repo, @@ -1100,14 +1116,14 @@ gate `create-pull-request`. It diagnoses a `cast_failure` and outputs run separately after the agent; this diagnosis cannot prevent a pull request that is materialized concurrently. -Built-in `report_incomplete` only warns and creates or updates a durable tracking -issue; it does not fail the run. The factual failure comment and, when emitted, -the red `cast_failure` job are the Cast failure signals. The generic completion -hook is deliberately neutral because pinned gh-aw chooses `run-success` from -the agent and consolidated `safe_outputs` results without considering the -custom job result. A red `cast_failure` job can therefore still select this -neutral hook; it does not select `run-failure`. Processing completion does not -indicate Cast success. Only a linked Cast PR is the success signal. +Pinned gh-aw v0.89.22 makes `report_incomplete` fail the conclusion step. +Automatic failure, failed-job, detection, no-op tracking issues and generic +activation/completion comments are disabled in the dispatcher and router: +conclusion runs independently of safe-output authorization and completion +comments otherwise append unvalidated no-op text. Diagnostics remain in Actions +logs and summaries; detection still runs. The factual, guarded failure comment +and, when emitted, the red `cast_failure` job remain the Cast failure signals. +Only a linked Cast PR is the success signal. ##### Step 8: Open PR @@ -2438,7 +2454,7 @@ Root → Epics → Tasks. Phase-specific: filter to matching phase heading. While Steps 2b/2c run, keep two counts: `activated` (issues created or recognized this run) and `labeled` (issues whose `add_labels` call was accepted). An `add_labels` call that was never made, was rejected, or returned an error counts as **unlabeled**. These counts track *label operations*, not labels present on GitHub: acceptance means the call was queued for a specific target this turn, and gh-aw applies it in the post-agent job. Never state or imply that a counted label was applied, landed, or was confirmed on the issue — nothing here reads labels back. At the end of Step 2: 1. `labeled == activated` → the activation is complete; proceed to Step 3. -2. `labeled < activated` → **this is an incomplete activation, not a successful one.** Call `report_incomplete` with a `reason` naming the shortfall (`{labeled} of {activated} activated issues had a label operation accepted`) and `details` listing **every affected work item — the identifier you used to target its `add_labels` call, its title, and the label set that missing operation targeted**. For an item created this run that identifier is the `temporary_id` you minted under the Temporary-ID Contract (`#aw_epic{K}` / `#aw_task{N}`) — not a GitHub issue number, because creation is deferred to the safe-output job and no real number exists yet. Quote a real number only where one is independently verified: an epic or task matched by dedup-by-title, or an issue recognized by Step 1's idempotent-rerun path. Never predict, infer, or invent a number. `report_incomplete` logs a warning and opens or updates a durable `[aw] ... reported incomplete result` tracking issue; it does **not** change the run's conclusion — the run still reports success. That record and the rule below keep a truncated activation from passing as clean, so never rely on a red run to carry the signal. +2. `labeled < activated` → **this is an incomplete activation, not a successful one.** Call `report_incomplete` with a `reason` naming the shortfall (`{labeled} of {activated} activated issues had a label operation accepted`) and `details` listing **every affected work item — the identifier you used to target its `add_labels` call, its title, and the label set that missing operation targeted**. For an item created this run that identifier is the `temporary_id` you minted under the Temporary-ID Contract (`#aw_epic{K}` / `#aw_task{N}`) — not a GitHub issue number, because creation is deferred to the safe-output job and no real number exists yet. Quote a real number only where one is independently verified: an epic or task matched by dedup-by-title, or an issue recognized by Step 1's idempotent-rerun path. Never predict, infer, or invent a number. `report_incomplete` records the reason in Actions logs and fails the conclusion step; automatic tracking-issue reporting is disabled. Include the shortfall in the guarded activation summary as well, so the originating issue retains an actionable signal. **Cap exhaustion is a reportable, nameable cause.** If the shortfall is because a cap was reached, say so in the `reason`, name which cap (`create-issue` 75 or `add_labels` 110) and list the work items that did not fit, and recommend `/squad plan activate phase {N}`. This is the one case where a cap may be named as the cause: it was observed, not guessed. Do not infer a cap from a rejection you never received, and do not treat the absence of an `E002` error as proof that every label operation was accepted: the count comparison, not the error stream, is the authority.