Repository navigation
Emit incomplete outcomes with diagnostics when agents produce no safe outputs - #65564
Merged
pelikhan merged 7 commits intoOct 4, 2026
Merged
Conversation
3 tasks
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix agent job conclusion path to emit structured noop and clear diagnostics
Emit incomplete outcomes with diagnostics when agents produce no safe outputs
Oct 4, 2026
pelikhan
marked this pull request as ready for review
October 4, 2026 13:35
pelikhan
reviewed
Oct 4, 2026
|
|
||
| require ( | ||
| github.com/rhysd/actionlint v1.7.12 | ||
| go.yaml.in/yaml/v3 v3.0.5 |
Contributor
Author
There was a problem hiding this comment.
Switched the test to the existing github.com/goccy/go-yaml package and removed the direct go.yaml.in/yaml/v3 requirement in 802d277.
Contributor
Author
There was a problem hiding this comment.
This thread already has a substantive reply from 802d277 confirming the test now uses the existing goccy/go-yaml dependency; no additional reply is needed.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The schema breaks generated queue expressions, diagnostics omit supported failure formats, and routing regeneration removes active commands.
Review effort: Balanced
Findings: 2
Open (3)
What changed in this PR
Adds explicit incomplete outcomes and sanitized runtime diagnostics when agents emit no valid safe outputs.
Changes:
- Synthesizes
report_incompletefor missing, empty, or rejected outputs. - Collects and redacts denial diagnostics from session events and logs.
- Adds regression coverage for diagnostics, redaction, and output preservation.
| File | Description |
|---|---|
actions/setup/js/collect_ndjson_output.cjs |
Emits incomplete outcomes for empty results. |
actions/setup/js/collect_ndjson_output.test.cjs |
Updates collection regression coverage. |
actions/setup/js/empty_output_outcome.cjs |
Builds sanitized diagnostic outcomes. |
actions/setup/js/empty_output_outcome.test.cjs |
Tests diagnostic extraction and redaction. |
actions/setup/js/handle_agent_failure.test.cjs |
Tests failure-report rendering. |
actions/setup/js/unified_session_payload.cjs |
Preserves native tool arguments. |
actions/setup/js/unified_session_payload.test.cjs |
Tests argument normalization. |
pkg/workflow/schemas/github-workflow.json |
Restricts concurrency queue values. |
go.mod |
Marks YAML v3 as direct dependency. |
.github/workflows/agentic_commands.yml |
Regenerates centralized command routing. |
| # runner-guard:ignore RGS-016 -- routing tables below contain emoji variation selectors (U+FE0F) and zero-width joiners (U+200D) used to render standard emoji sequences, not steganographic payloads. | ||
| env: | ||
| GH_AW_SLASH_ROUTING: '{"*":[{"workflow":"skillet","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🍳","status_comment":true}],"ace":[{"workflow":"ace-editor","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"✏️","status_comment":true}],"approach-validator":[{"workflow":"approach-validator","events":["issue_comment","pull_request_comment"],"ai_reaction":"eyes","emoji":"✅","status_comment":true}],"archie":[{"workflow":"archie","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🏛️","status_comment":true}],"cloclo":[{"workflow":"cloclo","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"📊","status_comment":true}],"craft":[{"workflow":"craft","events":["issues"],"ai_reaction":"eyes","emoji":"✍️","status_comment":true}],"dependabot-burner":[{"workflow":"dependabot-burner","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔥","status_comment":true}],"grumpy":[{"workflow":"grumpy-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true}],"matt":[{"workflow":"mattpocock-skills-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true}],"mergefest":[{"workflow":"mergefest","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"🔀","status_comment":true}],"nit":[{"workflow":"pr-nitpick-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true}],"plan":[{"workflow":"plan","events":["discussion_comment","issue_comment"],"ai_reaction":"eyes","emoji":"📋","status_comment":true}],"poem-bot":[{"workflow":"poem-bot","events":["issues"],"ai_reaction":"eyes","emoji":"🎭","status_comment":true}],"ponytail":[{"workflow":"ponytail-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"✂️","status_comment":true}],"review":[{"workflow":"design-decision-gate","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🏗️","status_comment":true},{"workflow":"pr-code-quality-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true},{"workflow":"test-quality-sentinel","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"ruflo":[{"workflow":"ruflo-backed-task","events":["issue_comment"],"ai_reaction":"eyes","status_comment":true}],"scout":[{"workflow":"scout","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔭","status_comment":true}],"security-review":[{"workflow":"security-review","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔒","status_comment":true}],"smoke-agent-all-merged":[{"workflow":"smoke-agent-all-merged","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-agent-all-none":[{"workflow":"smoke-agent-all-none","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-agent-public-approved":[{"workflow":"smoke-agent-public-approved","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-agent-public-none":[{"workflow":"smoke-agent-public-none","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-agent-scoped-approved":[{"workflow":"smoke-agent-scoped-approved","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-aider":[{"workflow":"smoke-aider","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🧑✈️","status_comment":true}],"smoke-call-workflow":[{"workflow":"smoke-call-workflow","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-checkout-pr-dispatch":[{"workflow":"smoke-checkout-pr-dispatch","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-claude":[{"workflow":"smoke-claude","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"heart","emoji":"🧪","status_comment":true}],"smoke-claude-on-copilot":[{"workflow":"smoke-claude-on-copilot","events":["pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-codex":[{"workflow":"smoke-codex","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"hooray","emoji":"🧪","status_comment":true}],"smoke-copilot":[{"workflow":"smoke-copilot","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-aoai-apikey":[{"workflow":"smoke-copilot-aoai-apikey","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-aoai-entra":[{"workflow":"smoke-copilot-aoai-entra","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-arm":[{"workflow":"smoke-copilot-arm","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-mai":[{"workflow":"smoke-copilot-mai","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"⚡","status_comment":true}],"smoke-copilot-sdk":[{"workflow":"smoke-copilot-sdk","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🔬","status_comment":true}],"smoke-copilot-small":[{"workflow":"smoke-copilot-small","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🪶","status_comment":true}],"smoke-create-cross-repo-pr":[{"workflow":"smoke-create-cross-repo-pr","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-crush":[{"workflow":"smoke-crush","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-cursor":[{"workflow":"smoke-cursor","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🖱️","status_comment":true}],"smoke-deepseek-harness":[{"workflow":"smoke-deepseek-harness","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-drive":[{"workflow":"smoke-drive","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"💾","status_comment":true}],"smoke-gemini":[{"workflow":"smoke-gemini","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🧪","status_comment":true}],"smoke-github-claude":[{"workflow":"smoke-github-claude","events":["pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-goose":[{"workflow":"smoke-goose","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🪿","status_comment":true}],"smoke-kiro":[{"workflow":"smoke-kiro","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🧭","status_comment":true}],"smoke-multi-pr":[{"workflow":"smoke-multi-pr","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-opencode":[{"workflow":"smoke-opencode","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🧪","status_comment":true}],"smoke-otel-backends":[{"workflow":"smoke-otel-backends","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-pi":[{"workflow":"smoke-pi","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🧪","status_comment":true}],"smoke-project":[{"workflow":"smoke-project","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-pydantic":[{"workflow":"smoke-pydantic","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"rocket","emoji":"🐍","status_comment":true}],"smoke-service-ports":[{"workflow":"smoke-service-ports","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-temporary-id":[{"workflow":"smoke-temporary-id","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-test-tools":[{"workflow":"smoke-test-tools","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-update-cross-repo-pr":[{"workflow":"smoke-update-cross-repo-pr","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"souschef":[{"workflow":"pr-sous-chef","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"👨🍳","status_comment":true}],"squad-plan":[{"workflow":"squad-plan","events":["issue_comment"],"ai_reaction":"eyes","emoji":"🧑🤝🧑","status_comment":true}],"summarize":[{"workflow":"pdf-summary","events":["issue_comment","issues"],"ai_reaction":"eyes","emoji":"📄","status_comment":true}],"tidy":[{"workflow":"tidy","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"🧹","status_comment":true}],"unbloat":[{"workflow":"unbloat-docs","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"📝","status_comment":true}],"windows":[{"workflow":"windows","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🪟","status_comment":true}]}' | ||
| GH_AW_SLASH_ROUTING: '{"*":[{"workflow":"skillet","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🍳","status_comment":true}],"ace":[{"workflow":"ace-editor","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"✏️","status_comment":true}],"approach-validator":[{"workflow":"approach-validator","events":["issue_comment","pull_request_comment"],"ai_reaction":"eyes","emoji":"✅","status_comment":true}],"archie":[{"workflow":"archie","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🏛️","status_comment":true}],"cloclo":[{"workflow":"cloclo","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"📊","status_comment":true}],"craft":[{"workflow":"craft","events":["issues"],"ai_reaction":"eyes","emoji":"✍️","status_comment":true}],"dependabot-burner":[{"workflow":"dependabot-burner","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔥","status_comment":true}],"grumpy":[{"workflow":"grumpy-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true}],"mergefest":[{"workflow":"mergefest","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"🔀","status_comment":true}],"nit":[{"workflow":"pr-nitpick-reviewer","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔍","status_comment":true}],"plan":[{"workflow":"plan","events":["discussion_comment","issue_comment"],"ai_reaction":"eyes","emoji":"📋","status_comment":true}],"poem-bot":[{"workflow":"poem-bot","events":["issues"],"ai_reaction":"eyes","emoji":"🎭","status_comment":true}],"ruflo":[{"workflow":"ruflo-backed-task","events":["issue_comment"],"ai_reaction":"eyes","status_comment":true}],"scout":[{"workflow":"scout","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔭","status_comment":true}],"security-review":[{"workflow":"security-review","events":["pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🔒","status_comment":true}],"smoke-claude-on-copilot":[{"workflow":"smoke-claude-on-copilot","events":["pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot":[{"workflow":"smoke-copilot","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-aoai-apikey":[{"workflow":"smoke-copilot-aoai-apikey","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-aoai-entra":[{"workflow":"smoke-copilot-aoai-entra","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-copilot-mai":[{"workflow":"smoke-copilot-mai","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"⚡","status_comment":true}],"smoke-copilot-sdk":[{"workflow":"smoke-copilot-sdk","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🔬","status_comment":true}],"smoke-copilot-small":[{"workflow":"smoke-copilot-small","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🪶","status_comment":true}],"smoke-github-claude":[{"workflow":"smoke-github-claude","events":["pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-otel-backends":[{"workflow":"smoke-otel-backends","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"smoke-service-ports":[{"workflow":"smoke-service-ports","events":["issue_comment","issues","pull_request","pull_request_comment"],"ai_reaction":"eyes","emoji":"🧪","status_comment":true}],"souschef":[{"workflow":"pr-sous-chef","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"👨🍳","status_comment":true}],"squad-plan":[{"workflow":"squad-plan","events":["issue_comment"],"ai_reaction":"eyes","emoji":"🧑🤝🧑","status_comment":true}],"summarize":[{"workflow":"pdf-summary","events":["issue_comment","issues"],"ai_reaction":"eyes","emoji":"📄","status_comment":true}],"tidy":[{"workflow":"tidy","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"🧹","status_comment":true}],"unbloat":[{"workflow":"unbloat-docs","events":["pull_request_comment"],"ai_reaction":"eyes","emoji":"📝","status_comment":true}],"windows":[{"workflow":"windows","events":["discussion","discussion_comment","issue_comment","issues","pull_request","pull_request_comment","pull_request_review_comment"],"ai_reaction":"eyes","emoji":"🪟","status_comment":true}]}' |
Contributor
Author
There was a problem hiding this comment.
Updated and recompiled the routing artifact after merging main; it now includes /review, /matt, /ponytail, /smoke-codex, and the other existing active routes. Fixed in 7a0ff21.
Comment on lines
+45
to
+46
| "type": "string", | ||
| "enum": ["single", "max"], |
Contributor
Author
There was a problem hiding this comment.
Restored the expressionSyntax alternative in the queue schema and kept schema coverage for expression-valued queues. The merged main generator now omits queue on mixed PR triggers, so the compilation test reflects that behavior. Fixed in 7a0ff21.
| const key = `${event.provenance.path}:${event.session_id || ""}:${data.toolCallId}`; | ||
| if (event.type === "tool.execution_start") { | ||
| starts.set(key, data); | ||
| } else if (event.type === "tool.execution_complete" && data.success === false) { |
Contributor
Author
There was a problem hiding this comment.
Incomplete diagnostics now recognize canonical failure indicators including isError, is_error, error, failure statuses, and nonzero exitCode. Fixed in 7a0ff21.
Contributor
|
✅ PR Code Quality Reviewer completed the code quality review.
|
Contributor
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
Contributor
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
Contributor
|
🔍 Design Decision Gate 🏗️ is checking for design decision records on this pull request... |
Contributor
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
Contributor
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
Contributor
There was a problem hiding this comment.
Request changes
This patch is still carrying blocking regressions outside the intended incomplete-outcome fix.
Blocking themes
- The regenerated command routing table drops active slash commands and reviewer workflows that still exist in the repo, so merging this would silently disable supported entry points.
pkg/workflow/schemas/github-workflow.jsonnow rejects expression-valuedconcurrency.queue, which conflicts with the documented schema intent and existing compiler/tests that generate mixed queue expressions.buildEmptyOutputOutcome()still only treatstool.execution_completeas failed whensuccess === false, so normalized native/Copilot failures carried asisError,status,error, or nonzeroexitCodecan be omitted from the incomplete diagnostics you are trying to preserve.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 42.6 AIC · ⌖ 7.73 AIC · ⊞ 19.4K
Comment /review to run again
Contributor
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd (via pr-triage: change_type=bug_fix). Requesting changes: I locally reproduced two of the existing bot-flagged regressions as real test failures, plus added one new TDD gap.
📋 Verification & Key Themes
Verified locally
go test ./pkg/workflow/ -run 'TestCommandConcurrencyCompilation|TestConcurrencyQueueSchemaValidation'fails against this branch: the narrowedconcurrency.queueschema inpkg/workflow/schemas/github-workflow.jsonrejects the${{ github.event_name == 'pull_request' && 'single' || 'max' }}expression the compiler itself generates for mixed-trigger slash commands (pkg/workflow/concurrency.go:43).- Confirmed the
.github/workflows/agentic_commands.ymlrouting-table regression is real:design-decision-gate.md,pr-code-quality-reviewer.md,test-quality-sentinel.md,ponytail-reviewer.md,mattpocock-skills-reviewer.md,smoke-claude.md,smoke-codex.md,smoke-agent-all-*.mdall still exist on disk and declare centralized slash commands, but are absent from the regenerated routing table — so/review,/matt,/ponytail, and several/smoke-*commands are now unreachable. - Reviewed
empty_output_outcome.cjs: the existing bot comment on line 48 is correct — canonical session normalization (unified_session_payload.cjs:235) preserves denials asisError,status, or nonzeroexitCodeeven whensuccessis absent, butbuildEmptyOutputOutcomeonly checksdata.success === false, so some real denials would be silently excluded from diagnostics. Added a regression test suggestion for this gap.
Positive highlights
- ✅
empty_output_outcome.cjscorrectly reuses existing redaction helpers (safe_output_manifest.cjs,add_mask_redaction.cjs) instead of reinventing secret handling. - ✅ Good regression coverage added for the empty-output →
report_incompletehappy path, including native Copilot event fallback and truncation/redaction boundaries. - ✅
collect_ndjson_output.test.cjsupdates are thorough and consistently track the newreport_incompletebehavior across many pre-existing test cases.
Please resolve the two pre-existing confirmed-blocking review threads (schema queue regression, routing-table regression) before merge — these are compiler-breaking and command-routing regressions respectively, not style nits.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 98.4 AIC · ⌖ 14.6 AIC · ⊞ 10K
Comment /matt to run again
| expect(buildEmptyOutputOutcome([], rootDir)).toEqual({ | ||
| type: "report_incomplete", | ||
| reason: "Agent finished without emitting any valid safe outputs; task completion could not be confirmed.", | ||
| }); |
Contributor
There was a problem hiding this comment.
[/tdd] No regression test exercises the exact failure mode flagged in the existing review comment on empty_output_outcome.cjs line 48: a denied tool call where success is absent but the canonical session carries isError: true, a non-ok status, or a nonzero exitCode. Every fixture in this file sets success: false explicitly.
💡 Suggested test
it("treats isError/status/exitCode failures as denials even when success is omitted", () => {
writeEvents([
{ type: "tool.execution_start", data: { toolCallId: "shell", toolName: "bash", input: { command: "cat secret" } } },
{ type: "tool.execution_complete", data: { toolCallId: "shell", isError: true, error: "Permission denied" } },
]);
const outcome = buildEmptyOutputOutcome([], rootDir);
expect(outcome.details).toContain("cat secret");
expect(outcome.details).toContain("Permission denied");
});Adding this test red (today) would make the data.success === false gap in empty_output_outcome.cjs visible before merging, matching the diagnosing-bugs principle of proving the regression before trusting the fix.
@copilot please address this.
Contributor
Author
There was a problem hiding this comment.
Added a regression fixture with success omitted and isError, failed status, and nonzero exitCode indicators; it verifies the denied command and error text are included. Fixed in 7a0ff21.
Contributor
There was a problem hiding this comment.
Review summary
No UI/frontend changes in this PR, so Impeccable modes (audit/critique/harden) do not apply; performed a standard correctness-focused review instead, re-verifying the open bot review threads against the current diff.
Confirmed blocking issue (verified by running tests)
pkg/workflow/schemas/github-workflow.json (concurrency.queue schema narrowing) — already flagged in an existing review thread. I reproduced it locally:
go test ./pkg/workflow/ -run 'TestConcurrencyQueueSchemaValidation|TestCommandConcurrencyCompilation'
fails with:
- at '/concurrency': 'oneOf' failed, none matched
- at '/concurrency': got object, want string
- at '/concurrency/queue': value must be one of 'single', 'max'
TestCommandConcurrencyCompilation/centralized_command_and_PR, .../label_command_and_PR, and TestConcurrencyQueueSchemaValidation all fail because the PR drops the $ref: #/definitions/expressionSyntax branch from the queue schema, but concurrency.queue is still compiled as a GitHub Actions expression (${{ github.event_name == 'pull_request' && 'single' || 'max' }}) for mixed-trigger commands. This is a real regression, not a pre-existing failure — tests pass on go.mod's previous schema. Must fix before merge.
Other open threads I re-verified as still valid
actions/setup/js/empty_output_outcome.cjs:48— thedata.success === falsecheck misses failures the canonical session normalizer preserves only viaisError,error,status, or non-zeroexitCode(seeunified_session_payload.cjs's ownisErrorhandling added in this same PR), so some denied/failed tool calls would silently drop out of the incomplete-outcome diagnostics..github/workflows/agentic_commands.yml:115— the regenerated routing table drops active commands (/review,/matt,/ponytail, several/smoke-*) whose source workflow.mdfiles still exist in the tree (confirmed present:mattpocock-skills-reviewer.md,ponytail-reviewer.md,pr-code-quality-reviewer.md, etc.), indicating this lockfile was regenerated from an incomplete workflow set rather than intentionally pruned — looks like an unrelated/accidental side effect of this PR's build, not something to ship.
I did not add new inline comments since all three issues already have open review threads; the schema regression should now be treated as confirmed/blocking given the local test failure.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 113.7 AIC · ⌖ 13.2 AIC · ⊞ 8.1K
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Collaborator
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 802d277
|
…guarantee-noop-report-incomplete Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
pelikhan
deleted the
copilot/aw-top-10-09-guarantee-noop-report-incomplete
branch
October 4, 2026 16:33
This was referenced Oct 5, 2026
Contributor
|
🎉 This pull request is included in a new release. Release: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Agents can finish without safe outputs or an actionable explanation when commands or capabilities are denied. This change records an explicit incomplete outcome and carries available runtime diagnostics into failure reports.
report_incompletefor missing, empty, or wholly rejected outputs, including zero-tool gateway failures.nooprecords.argumentsas tool input.