fix: create agentic failure issue when engine secret validation fails - #47893
Conversation
- Add GetSecretFailureMessage to WorkflowExecutor interface with BaseEngine no-op default - CopilotEngine overrides GetSecretFailureMessage with copilot-requests: write guidance - Compiler emits GH_AW_ENGINE_SECRET_FAILURE_MESSAGE in conclusion job when engine has validate-secret step and a custom message - Fix handle_agent_failure.cjs: add hasSecretVerificationFailed to early return guard so a failure issue is created when the secret validation step fails and the agent is skipped - Update buildSecretVerificationContext to use GH_AW_ENGINE_SECRET_FAILURE_MESSAGE instead of hardcoded engineId check - Add compiled tests: TestCopilotEngineSecretFailureMessageWiredInConclusionJob, TestNonCopilotEngineHasNoSecretFailureMessage - Recompile all 261 workflow lock files Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
Pull request overview
Fixes failure reporting when secret validation fails before the agent job runs.
Changes:
- Allows secret-validation failures to bypass the failure-handler guard.
- Adds engine-specific remediation guidance.
- Regenerates affected Copilot workflow lock files and adds compiler tests.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/handle_agent_failure.cjs |
Handles secret-validation failures and engine guidance. |
pkg/workflow/agentic_engine.go |
Adds the engine guidance interface. |
pkg/workflow/copilot_engine_installation.go |
Provides Copilot-specific remediation. |
pkg/workflow/notify_comment_conclusion_helpers.go |
Emits remediation into the conclusion job. |
pkg/workflow/secret_verification_output_test.go |
Tests compiler output. |
.github/workflows/workflow-skill-extractor.lock.yml |
Regenerates conclusion environment. |
.github/workflows/workflow-normalizer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/workflow-health-manager.lock.yml |
Regenerates conclusion environment. |
.github/workflows/workflow-generator.lock.yml |
Regenerates conclusion environment. |
.github/workflows/weekly-safe-outputs-spec-review.lock.yml |
Regenerates conclusion environment. |
.github/workflows/weekly-issue-summary.lock.yml |
Regenerates conclusion environment. |
.github/workflows/weekly-editors-health-check.lock.yml |
Regenerates conclusion environment. |
.github/workflows/weekly-blog-post-writer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/visual-regression-checker.lock.yml |
Regenerates conclusion environment. |
.github/workflows/video-analyzer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/update-astro.lock.yml |
Regenerates conclusion environment. |
.github/workflows/uk-ai-operational-resilience.lock.yml |
Regenerates conclusion environment. |
.github/workflows/ubuntu-image-analyzer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/tidy.lock.yml |
Regenerates conclusion environment. |
.github/workflows/terminal-stylist.lock.yml |
Regenerates conclusion environment. |
.github/workflows/technical-doc-writer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/super-linter.lock.yml |
Regenerates conclusion environment. |
.github/workflows/sub-issue-closer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/stale-repo-identifier.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-workflow-call.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-workflow-call-with-inputs.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-test-tools.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-temporary-id.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-service-ports.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-project.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-otel-backends.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-multi-pr.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-copilot.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-copilot-small.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-copilot-sdk.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-copilot-mai.lock.yml |
Regenerates conclusion environment. |
.github/workflows/smoke-copilot-arm.lock.yml |
Regenerates conclusion environment. |
.github/workflows/slide-deck-maintainer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/sighthound-security-scan.lock.yml |
Regenerates conclusion environment. |
.github/workflows/security-compliance.lock.yml |
Regenerates conclusion environment. |
.github/workflows/research.lock.yml |
Regenerates conclusion environment. |
.github/workflows/repository-quality-improver.lock.yml |
Regenerates conclusion environment. |
.github/workflows/repo-tree-map.lock.yml |
Regenerates conclusion environment. |
.github/workflows/repo-audit-analyzer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/release.lock.yml |
Regenerates conclusion environment. |
.github/workflows/refiner.lock.yml |
Regenerates conclusion environment. |
.github/workflows/q.lock.yml |
Regenerates conclusion environment. |
.github/workflows/python-data-charts.lock.yml |
Regenerates conclusion environment. |
.github/workflows/pr-triage-agent.lock.yml |
Regenerates conclusion environment. |
.github/workflows/pr-nitpick-reviewer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/pr-description-caveman.lock.yml |
Regenerates conclusion environment. |
.github/workflows/plan.lock.yml |
Regenerates conclusion environment. |
.github/workflows/pdf-summary.lock.yml |
Regenerates conclusion environment. |
.github/workflows/outcome-collector.lock.yml |
Regenerates conclusion environment. |
.github/workflows/org-health-report.lock.yml |
Regenerates conclusion environment. |
.github/workflows/objective-impact-report.lock.yml |
Regenerates conclusion environment. |
.github/workflows/notion-issue-summary.lock.yml |
Regenerates conclusion environment. |
.github/workflows/metrics-collector.lock.yml |
Regenerates conclusion environment. |
.github/workflows/mergefest.lock.yml |
Regenerates conclusion environment. |
.github/workflows/mcp-inspector.lock.yml |
Regenerates conclusion environment. |
.github/workflows/layout-spec-maintainer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/jsweep.lock.yml |
Regenerates conclusion environment. |
.github/workflows/issue-triage-agent.lock.yml |
Regenerates conclusion environment. |
.github/workflows/gpclean.lock.yml |
Regenerates conclusion environment. |
.github/workflows/glossary-maintainer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/github-remote-mcp-auth-test.lock.yml |
Regenerates conclusion environment. |
.github/workflows/functional-pragmatist.lock.yml |
Regenerates conclusion environment. |
.github/workflows/firewall.lock.yml |
Regenerates conclusion environment. |
.github/workflows/example-permissions-warning.lock.yml |
Regenerates conclusion environment. |
.github/workflows/example-failure-category-filter.lock.yml |
Regenerates conclusion environment. |
.github/workflows/designer-drift-audit.lock.yml |
Regenerates conclusion environment. |
.github/workflows/deployment-incident-monitor.lock.yml |
Regenerates conclusion environment. |
.github/workflows/dependabot-go-checker.lock.yml |
Regenerates conclusion environment. |
.github/workflows/dependabot-burner.lock.yml |
Regenerates conclusion environment. |
.github/workflows/dataflow-pr-discussion-dataset.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-windows-terminal-integration-builder.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-team-status.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-skill-optimizer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-sentrux-report.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-security-observability.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-regulatory.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-performance-summary.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-model-resolution.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-model-inventory.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-issues-report.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-firewall-report.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-credit-limit-test.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-compiler-threat-spec-optimizer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-community-attribution.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-cli-tools-tester.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-ambient-context-optimizer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/daily-agent-of-the-day-blog-writer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/copilot-opt.lock.yml |
Regenerates conclusion environment. |
.github/workflows/contribution-check.lock.yml |
Regenerates conclusion environment. |
.github/workflows/constraint-solving-potd.lock.yml |
Regenerates conclusion environment. |
.github/workflows/code-simplifier.lock.yml |
Regenerates conclusion environment. |
.github/workflows/bot-detection.lock.yml |
Regenerates conclusion environment. |
.github/workflows/agentic-token-trend-audit.lock.yml |
Regenerates conclusion environment. |
.github/workflows/agentic-token-optimizer.lock.yml |
Regenerates conclusion environment. |
.github/workflows/agentic-token-audit.lock.yml |
Regenerates conclusion environment. |
.github/workflows/ace-editor.lock.yml |
Regenerates conclusion environment. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/handle_agent_failure.cjs:3154
- The core regression path is not covered by a behavior test: the added Go tests only inspect compiler output. Add a
main()test withGH_AW_AGENT_CONCLUSION=skippedandGH_AW_SECRET_VERIFICATION_RESULT=failedthat verifies an issue is created and contains the secret-verification context; otherwise a later early return can silently reintroduce the original failure.
- Files reviewed: 101/101 changed files
- Comments generated: 2
- Review effort level: Medium
| * @returns {string} Formatted context string, or empty string if verification did not fail | ||
| */ | ||
| function buildSecretVerificationContext(secretVerificationResult, engineId) { | ||
| function buildSecretVerificationContext(secretVerificationResult, engineSecretFailureMessage) { |
There was a problem hiding this comment.
The fix is correct and well-structured. Root cause addressed: hasSecretVerificationFailed is now included in the early-return guard, so a skipped agent job (from a failed validate-secret step) correctly proceeds to failure issue creation. Design: Replacing the engineId === copilot hardcode with a data-driven GH_AW_ENGINE_SECRET_FAILURE_MESSAGE env var is a clean extensibility improvement. Tests: Both cases covered.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 21 AIC · ⌖ 4.48 AIC · ⊞ 5K
🧪 Test Quality Sentinel Report
📊 Metrics (2 tests)
Verdict
|
…message Auto-generated by the Design Decision Gate agent.
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (>100 new lines) but does not have a linked Architecture Decision Record (ADR). 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes on one test-correctness issue. The fix logic itself is clean and the Go-side design is solid.
📋 Key Themes & Highlights
Key Issues
- Stale JS unit tests:
buildSecretVerificationContexttests were not updated to match the new parameter contract — they still pass engine ID strings ("copilot","claude") instead of formatted message strings. The tests pass by coincidence and no longer verify the right behaviour. - Missing JS integration test for the core guard: there is no JS-level test asserting that
hasSecretVerificationFailedbypasses the early-return whenagentConclusion === "skipped", which is the exact bug this PR fixes.
Positive Highlights
- ✅ Core fix is correct —
hasSecretVerificationFailedis extracted cleanly and wired into both the early-return guard and downstream calls. - ✅ Data-driven design: replacing the hardcoded
engineId === "copilot"check withGH_AW_ENGINE_SECRET_FAILURE_MESSAGEis a clear improvement in extensibility. - ✅
GetSecretFailureMessage()onWorkflowExecutor+BaseEnginedefault follows the existing engine interface pattern. - ✅ Go-level tests (
TestCopilotEngineSecretFailureMessageWiredInConclusionJob,TestNonCopilotEngineHasNoSecretFailureMessage) are well-structured and cover both the positive and negative cases. - ✅
%qencoding viafmt.Sprintfproduces valid YAML double-quoted strings where\nis correctly interpreted as newlines by the YAML parser.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 61.4 AIC · ⌖ 4.81 AIC · ⊞ 6.7K
Comment /matt to run again
|
@copilot run pr-finisher skill |
There was a problem hiding this comment.
REQUEST_CHANGES — Two bypass guards missing !hasSecretVerificationFailed
The fix correctly adds !hasSecretVerificationFailed to the primary early-return guard (line 3151), but two subsequent auto-suppression guards (hasOnlyNoopOutputs at line 3182, hasCompletedDespiteJobFailure at line 3192) do not include the same check. An agent job that runs with a missing/invalid secret (it is not gated out when secret verification fails — the agent if: condition only blocks on daily_ai_credits_exceeded) could still trigger one of those bypass paths and silently suppress the failure issue this PR was written to fix.
Rest of diff — no issues
buildSecretVerificationContextparameter rename fromengineIdtoengineSecretFailureMessageis clean and data-driven.- Go interface addition (
GetSecretFailureMessage) andBaseEngine+CopilotEngineoverrides are correct. %qformat innotify_comment_conclusion_helpers.goproduces Go-escaped strings that YAML double-quoted parser correctly interprets as newlines — verified.- Lock file churn (96 files, 1 line each) is expected for a compiled output.
- Test coverage for the compiler side is solid.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 · 129.1 AIC · ⌖ 4.88 AIC · ⊞ 5.7K
Comment /review to run again
Comments that could not be inline-anchored
actions/setup/js/handle_agent_failure.cjs:3182
hasOnlyNoopOutputs bypass does not check !hasSecretVerificationFailed, allowing a failed secret-verification scenario to be silently dropped: If the agent runs while the secret is absent/invalid (the agent job if: condition only blocks on daily_ai_credits_exceeded, not on secret failure), exits with failure, and somehow produces only noop safe-outputs, this guard returns early and no failure issue is ever created — even though hasSecretVerificationFailed === true.
<details>
<su…
actions/setup/js/handle_agent_failure.cjs:3192
Same bypass gap applies to hasCompletedDespiteJobFailure: this guard also lacks !hasSecretVerificationFailed, meaning an agent that ran with a bad secret, logged terminal_reason: completed (e.g. it managed to call noop before failing), and produced valid non-noop outputs could slip through without triggering a failure issue.
<details>
<summary>💡 Suggested fix</summary>
if (hasCompletedDespiteJobFailure && !hasReportIncomplete && !hasCacheMissMisconfiguration && !hasSecretVe…
</details>…date JS tests to use message text Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 1ad5f5f. Two issues addressed:
|
|
@copilot please run the skill, confirm the branch is up to date, address unresolved review feedback, and rerun or retrigger checks as needed on the latest HEAD. Unresolved review feedback:
Run: https://github.com/github/gh-aw/actions/runs/30143146258
|
Branch is confirmed up to date (merge commit
The unresolved review thread on |
|
🎉 This pull request is included in a new release. Release: |
When the
validate-secretstep fails (missing engine secret), the activation job fails and the agent job is skipped — notfailure. Thehandle_agent_failure.cjsearly-return guard only checkedagentConclusion !== "failure", so it bailed out silently and never created a failure issue.Changes
handle_agent_failure.cjshasSecretVerificationFailedto the early-return guard so the failure handler proceeds whenGH_AW_SECRET_VERIFICATION_RESULT === "failed", even when the agent job was skippedengineId === "copilot"check inbuildSecretVerificationContextwith a data-drivenGH_AW_ENGINE_SECRET_FAILURE_MESSAGEenv varGo compiler — per-engine secret failure message
GetSecretFailureMessage(workflowData *WorkflowData) stringtoWorkflowExecutor;BaseEnginereturns""CopilotEngineoverrides it with thepermissions: copilot-requests: writealternative:GH_AW_ENGINE_SECRET_FAILURE_MESSAGEin the conclusion job when the engine has a validate-secret step and a non-empty messageTests (
secret_verification_output_test.go)TestCopilotEngineSecretFailureMessageWiredInConclusionJob— compiled lock file includesGH_AW_ENGINE_SECRET_FAILURE_MESSAGEwithcopilot-requests: writefor the Copilot engineTestNonCopilotEngineHasNoSecretFailureMessage— engines without a custom message (e.g. Claude) do not emit the env varRun: https://github.com/github/gh-aw/actions/runs/30143146258