Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3512a4ca-5de8-40fe-bb70-406542dbf684 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ure issue comments
- Add buildMissingToolContext() and loadMissingToolMessages() to handle_agent_failure.cjs
- Add hasMissingTool and hasMissingData detection to trigger failure handling
- Include missing_tool_context in both agent failure template paths
- Update agent_failure_comment.md and agent_failure_issue.md with {missing_tool_context}
- Change default create-issue to false for missing_tool and missing_data in Go compiler
- Update tests to reflect new defaults
- Add buildMissingToolContext tests to handle_agent_failure.test.cjs
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3512a4ca-5de8-40fe-bb70-406542dbf684
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3512a4ca-5de8-40fe-bb70-406542dbf684 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
May 2, 2026 18:47
View session
Collaborator
|
@copilot Ensure there is a feature flag configuration that allows changing this behavior back to include comments or disabling the feature entirely. |
Adds a `report-as-failure` boolean field to the `missing-tool` and `missing-data` safe-output configuration. When set to false, these signals no longer activate the agent failure code path (restoring pre-2026 behavior). Defaults to true. - Add ReportAsFailure field to IssueReportingConfig - Parse report-as-failure for missing-tool/missing-data, not report-incomplete - Pass GH_AW_MISSING_TOOL_REPORT_AS_FAILURE and GH_AW_MISSING_DATA_REPORT_AS_FAILURE env vars in notify_comment.go (default: true) - Read env vars in handle_agent_failure.cjs to conditionally activate hasMissingTool/hasMissingData - Add Go tests: TestMissingToolReportAsFailureConfig, TestReportIncompleteDoesNotHaveReportAsFailure - Add JS tests for feature flag behavior Agent-Logs-Url: https://github.com/github/gh-aw/sessions/56d2eb12-fcab-4482-a602-13b9a87c59a8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Added a Usage: safe-outputs:
missing-tool:
report-as-failure: false # restore old behavior: signals are shown in footers/issues but don't trigger failure handling
missing-data:
report-as-failure: false # same for missing_data signals
The env vars Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Updates safe-output handling so missing_tool and missing_data signals are treated as agent failures and included in the agent-failure issue/comment flow by default, with per-signal opt-out via report-as-failure.
Changes:
- Added
missing_toolparsing/formatting to the agent failure handler and surfaced it in failure issue/comment templates. - Extended workflow compiler config parsing to support
report-as-failureand changed defaultcreate-issuebehavior formissing-tool/missing-data. - Passed new
GH_AW_MISSING_{TOOL,DATA}_REPORT_AS_FAILUREenv vars into the conclusion job and updated/added tests.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/notify_comment.go |
Propagates missing-tool/missing-data report-as-failure flags to the conclusion job env. |
pkg/workflow/missing_issue_reporting.go |
Adds ReportAsFailure parsing and changes default create-issue behavior for missing-tool/data. |
pkg/workflow/missing_tool_test.go |
Updates defaults and adds coverage for report-as-failure parsing behavior. |
pkg/workflow/missing_data_test.go |
Updates expectations for the new default create-issue behavior. |
actions/setup/js/handle_agent_failure.cjs |
Loads/prints missing-tool info and treats missing-tool/data signals as failure triggers (gated by env flags). |
actions/setup/js/handle_agent_failure.test.cjs |
Adds unit tests for missing-tool context generation and the new flags. |
actions/setup/md/agent_failure_issue.md |
Adds {missing_tool_context} placeholder to failure issue template. |
actions/setup/md/agent_failure_comment.md |
Adds {missing_tool_context} placeholder to failure comment template. |
.github/workflows/tidy.lock.yml |
Updates locked env defaults for missing-tool issue creation. |
.github/workflows/poem-bot.lock.yml |
Updates locked env defaults for missing-tool issue creation. |
.github/workflows/hourly-ci-cleaner.lock.yml |
Updates locked env defaults for missing-tool issue creation. |
.github/workflows/daily-subagent-optimizer.lock.yml |
Updates sparse-checkout / base-restore lists (includes .pi/PI.md). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/workflow/missing_issue_reporting.go:86
parseIssueReportingConfigdoesn’t correctly handle the documentedmissing-tool: true/missing-data: trueform: whenconfigDatais booleantrue(or any non-nil, non-map value), the function falls through and returns an emptyIssueReportingConfigwithCreateIssue/TitlePrefix/Labelsunset. This can lead to missing env vars and incorrect defaults. Treattruethe same as the nil/defaults case, and returnnil(or log+nil) for unsupported types instead of returning a partially populated config.
// Explicitly disabled: missing-data: false
if configBool, ok := configData.(bool); ok && !configBool {
log.Printf("%s configuration explicitly disabled", yamlKey)
return nil
}
cfg := &IssueReportingConfig{}
// Enabled with no value: missing-data: (nil)
if configData == nil {
log.Printf("%s configuration enabled with defaults", yamlKey)
createIssueStr := strconv.FormatBool(defaultCreateIssue)
cfg.CreateIssue = &createIssueStr
if parseReportAsFailure {
trueVal := "true"
cfg.ReportAsFailure = &trueVal
}
cfg.TitlePrefix = defaultTitle
cfg.Labels = []string{}
return cfg
}
if configMap, ok := configData.(map[string]any); ok {
log.Printf("Parsing %s configuration from map", yamlKey)
- Files reviewed: 12/12 changed files
- Comments generated: 3
Comment on lines
139
to
158
| uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 | ||
| with: | ||
| persist-credentials: false | ||
| sparse-checkout: | | ||
| .github | ||
| .agents | ||
| actions/setup | ||
| .claude | ||
| .codex | ||
| .crush | ||
| .gemini | ||
| .opencode | ||
| .pi | ||
| sparse-checkout-cone-mode: true | ||
| fetch-depth: 1 | ||
| - name: Save agent config folders for base branch restoration | ||
| env: | ||
| GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github .opencode" | ||
| GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md opencode.jsonc" | ||
| GH_AW_AGENT_FOLDERS: ".agents .claude .codex .crush .gemini .github .opencode .pi" | ||
| GH_AW_AGENT_FILES: ".crush.json AGENTS.md CLAUDE.md GEMINI.md PI.md opencode.jsonc" | ||
| # poutine:ignore untrusted_checkout_exec |
Comment on lines
+1652
to
+1657
| // Build missing_data context | ||
| const missingDataContext = buildMissingDataContext(cacheMemoryEnabled); | ||
|
|
||
| // Build missing_tool context | ||
| const missingToolContext = buildMissingToolContext(); | ||
|
|
Comment on lines
+633
to
+676
| function loadMissingToolMessages() { | ||
| try { | ||
| const { loadAgentOutput } = require("./load_agent_output.cjs"); | ||
| const agentOutputResult = loadAgentOutput(); | ||
|
|
||
| if (!agentOutputResult.success || !agentOutputResult.items) { | ||
| return []; | ||
| } | ||
|
|
||
| const missingToolMessages = []; | ||
| for (const item of agentOutputResult.items) { | ||
| if (item.type === "missing_tool") { | ||
| if (item.reason) { | ||
| missingToolMessages.push({ | ||
| tool: item.tool || null, | ||
| reason: item.reason, | ||
| alternatives: item.alternatives || null, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return missingToolMessages; | ||
| } catch (error) { | ||
| core.warning(`Failed to load missing_tool messages: ${getErrorMessage(error)}`); | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Build missing_tool context string for display in failure issues/comments. | ||
| * @returns {string} Formatted missing tool context | ||
| */ | ||
| function buildMissingToolContext() { | ||
| const missingToolMessages = loadMissingToolMessages(); | ||
|
|
||
| if (missingToolMessages.length === 0) { | ||
| return ""; | ||
| } | ||
|
|
||
| core.info(`Found ${missingToolMessages.length} missing_tool message(s)`); | ||
|
|
||
| const formattedList = formatMissingTools(missingToolMessages); | ||
|
|
…agent failures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 97/100✅ Excellent test quality
Test Classification DetailsView all 18 test classifications
Minor Observations (Not Blocking)i️
|
| Test File | Lines Added | Production File | Lines Added | Ratio |
|---|---|---|---|---|
handle_agent_failure.test.cjs |
165 | handle_agent_failure.cjs |
107 | 1.54:1 ✅ |
missing_tool_test.go |
88 | missing_issue_reporting.go + notify_comment.go |
51 | 1.72:1 ✅ |
missing_data_test.go |
2 | (minor modifications only) | — | N/A ✅ |
No test inflation detected (threshold: 2:1).
Language Support
Tests analyzed:
- 🐹 Go (
*_test.go): 9 test functions — unit (//go:build !integration) - 🟨 JavaScript (
*.test.cjs): 9 test cases (vitest)
Verdict
✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). All 18 tests enforce observable behavioral contracts. Build tags are correct on all Go test files. No mock libraries used in Go. No assertion message violations detected.
📖 Understanding Test Classifications
Design Tests (High Value) verify what the system does:
- Assert on observable outputs, return values, or state changes
- Cover error paths and boundary conditions
- Would catch a behavioral regression if deleted
- Remain valid even after internal refactoring
Implementation Tests (Low Value) verify how the system does it:
- Assert on internal function calls (mocking internals)
- Only test the happy path with typical inputs
- Break during legitimate refactoring even when behavior is correct
- Give false assurance: they pass even when the system is wrong
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.
References: §25260257802
🧪 Test quality analysis by Test Quality Sentinel · ● 689.4K · ◷
Collaborator
|
@copilot review all comments |
…redundant IO - Gate buildMissingDataContext / buildMissingToolContext on report-as-failure flags so missing signals don't appear in failure reports when the feature is disabled - Pass agentOutputResult.items to context builders to avoid re-reading agent output from disk (eliminates redundant loadAgentOutput() calls in main()) - Refactor loadMissingDataMessages, loadMissingToolMessages, loadReportIncompleteMessages, buildMissingDataContext, buildMissingToolContext, buildReportIncompleteContext to accept optional items parameter with file-load fallback (preserves test compatibility) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2f1c08be-d547-4d90-8849-aab42b018cc5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Addressed all three review comments in commit
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot merge main and recompile |
…-for-missing-data-tools # Conflicts: # .github/workflows/daily-cache-strategy-analyzer.lock.yml # .github/workflows/daily-fact.lock.yml # .github/workflows/daily-observability-report.lock.yml # .github/workflows/duplicate-code-detector.lock.yml # .github/workflows/grumpy-reviewer.lock.yml # .github/workflows/issue-arborist.lock.yml # .github/workflows/schema-feature-coverage.lock.yml # .github/workflows/smoke-call-workflow.lock.yml # .github/workflows/smoke-codex.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/55156640-c0dc-49bb-ba19-981d471d3dde Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done. Merged Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This was referenced May 2, 2026
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.
Summary
Updates the policy so that
missing_toolandmissing_datasafe output signals are treated as agent failures and included in the agent failure issue comment, rather than being grouped into separate issues. Areport-as-failurefeature flag is provided to opt out of this behavior per signal type.Changes
JavaScript (
actions/setup/js/handle_agent_failure.cjs)loadMissingToolMessages()to readmissing_toolitems from agent output (parallel to the existingloadMissingDataMessages())buildMissingToolContext()to format missing tool messages for inclusion in failure reports (parallel tobuildMissingDataContext())hasMissingTooldetection: when the agent emitsmissing_toolmessages, failure handling is activated (same asreport_incomplete)hasMissingDatadetection: when the agent emitsmissing_datamessages, failure handling is activated (previously onlycache_memory_misstriggered this)missing_tool_contextbuildMissingToolContextfor testabilitymissingToolReportAsFailure/missingDataReportAsFailureflags (read fromGH_AW_MISSING_TOOL_REPORT_AS_FAILURE/GH_AW_MISSING_DATA_REPORT_AS_FAILUREenv vars) to conditionally activate failure handling per signal typebuildMissingDataContextandbuildMissingToolContextare only called (and injected into templates) when the respectivereport-as-failureflag istrue— when disabled, the context is empty and the signals do not appear in failure reports at allloadMissingDataMessages,loadMissingToolMessages,loadReportIncompleteMessagesand theirbuild*Contextwrappers to accept an optionalitemsparameter;main()passesagentOutputResult.itemsdirectly to eliminate redundantloadAgentOutput()re-readsTemplates
agent_failure_comment.md: Added{missing_tool_context}placeholder after{missing_data_context}agent_failure_issue.md: Added{missing_tool_context}placeholder after{missing_data_context}Go compiler (
pkg/workflow/missing_issue_reporting.go)create-issuefromtruetofalseformissing_toolandmissing_data— these signals now flow to the agent failure issue comment by default, not separate issuesreport_incompleteretains itstruedefault (unchanged behavior)strconvimport, addeddefaultCreateIssue boolparameter toparseIssueReportingConfigReportAsFailure *stringfield toIssueReportingConfig— parsed formissing-toolandmissing-data(default"true"), not forreport-incompleteparseReportAsFailure boolparameter toparseIssueReportingConfigto control which types support the fieldGo compiler (
pkg/workflow/notify_comment.go)GH_AW_MISSING_TOOL_REPORT_AS_FAILUREandGH_AW_MISSING_DATA_REPORT_AS_FAILUREenv vars to the conclusion job (default"true"when not configured), enabling the feature flag even for workflows that don't explicitly configuremissing-tool:ormissing-data:Feature flag
The
report-as-failurefield onmissing-toolandmissing-dataconfig sections controls whether those signals activate failure handling and appear in failure reports:true(new behavior)report-incomplete(which always triggers failure handling)false, the signal neither triggers failure handling nor appears in the failure issue/comment contextTests
missing_tool_test.go: expectations for defaultcreate-issueupdated from"true"to"false"in 7 test cases; addedTestMissingToolReportAsFailureConfigandTestReportIncompleteDoesNotHaveReportAsFailuremissing_data_test.go: expectations for defaultcreate-issueupdated from"true"to"false"in 2 test casesbuildMissingToolContext, feature flag behavior, and optionalitemsparameter inhandle_agent_failure.test.cjs