Allow configuring conclusion failure issue expiration via aw.json#26688
Allow configuring conclusion failure issue expiration via aw.json#26688
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53b0076b-1124-4dc5-9a5f-dc0bbf662d73 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53b0076b-1124-4dc5-9a5f-dc0bbf662d73 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/53b0076b-1124-4dc5-9a5f-dc0bbf662d73 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot set the value to 12h in this repository in aw.json |
There was a problem hiding this comment.
Pull request overview
Adds repository-level configuration (via .github/workflows/aw.json) for the expiration window used when the conclusion job opens “action failure” issues, and wires that value through to the JS failure handler.
Changes:
- Extends the repo config schema and Go config loader/accessors to support
maintenance.action_failure_issue_expires(hours, min 1). - Updates conclusion job generation to pass
GH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURS(defaulting to 168h). - Updates
handle_agent_failure.cjsto apply the configured expiration to both grouped parent issues and per-run issues, with added tests/docs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/repo_config.go | Adds default constant + accessor for configured expiration hours. |
| pkg/parser/schemas/repo_config_schema.json | Adds maintenance.action_failure_issue_expires with integer/minimum validation. |
| pkg/workflow/notify_comment.go | Loads repo config and injects GH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURS into conclusion job env. |
| actions/setup/js/handle_agent_failure.cjs | Reads expiration hours from env and uses it when creating parent/per-run failure issues. |
| pkg/workflow/repo_config_test.go | Adds parsing + validation + default accessor tests. |
| pkg/workflow/notify_comment_test.go | Adds tests for default/configured env var propagation into conclusion job. |
| actions/setup/js/handle_agent_failure.test.cjs | Adds unit tests for env parsing helper. |
| docs/src/content/docs/guides/ephemerals.md | Documents the new action_failure_issue_expires config field. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 3
| actionFailureIssueExpiresHours := DefaultActionFailureIssueExpiresHours | ||
| repoConfig, repoConfigErr := LoadRepoConfig(c.gitRoot) | ||
| if repoConfigErr != nil { | ||
| notifyCommentLog.Printf( | ||
| "Warning: failed to load repo config for action failure issue expiration (using default %d hours): %v. Check that %s is valid JSON", | ||
| DefaultActionFailureIssueExpiresHours, | ||
| repoConfigErr, | ||
| RepoConfigFileName, | ||
| ) | ||
| } else { | ||
| actionFailureIssueExpiresHours = repoConfig.ActionFailureIssueExpiresHours() | ||
| } | ||
| agentFailureEnvVars = append(agentFailureEnvVars, fmt.Sprintf(" GH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURS: %q\n", strconv.Itoa(actionFailureIssueExpiresHours))) |
There was a problem hiding this comment.
buildConclusionJob loads and validates aw.json every time the conclusion job is generated. When compiling many workflows in one run, this repeatedly hits the filesystem and schema validation for a repo-level setting that is effectively constant. Consider loading repo config once (e.g., cache it on Compiler or pass it into buildConclusionJob) and reusing the parsed value.
| notifyCommentLog.Printf( | ||
| "Warning: failed to load repo config for action failure issue expiration (using default %d hours): %v. Check that %s is valid JSON", | ||
| DefaultActionFailureIssueExpiresHours, | ||
| repoConfigErr, | ||
| RepoConfigFileName, | ||
| ) |
There was a problem hiding this comment.
The warning text says “Check that … is valid JSON”, but LoadRepoConfig can fail for reasons other than JSON syntax (e.g., schema validation errors or read failures). Updating the message to mention schema validation (or to just surface the underlying error without implying JSON-only) would make debugging more accurate.
| func TestConclusionJobActionFailureIssueExpiration_DefaultFromRepoConfig(t *testing.T) { | ||
| compiler := NewCompiler() | ||
| workflowData := &WorkflowData{ | ||
| Name: "Test Workflow", | ||
| SafeOutputs: &SafeOutputsConfig{ | ||
| NoOp: &NoOpConfig{}, | ||
| }, | ||
| } | ||
|
|
||
| job, err := compiler.buildConclusionJob(workflowData, string(constants.AgentJobName), []string{}) |
There was a problem hiding this comment.
This test uses NewCompiler() without overriding compiler.gitRoot, so it will read the real repository’s .github/workflows/aw.json if one is present. That makes the test outcome dependent on repo state (or a developer’s local file) and can become flaky if aw.json is added/changed later. Set compiler.gitRoot to a temp dir without aw.json for this default-case assertion.
🧪 Test Quality Sentinel ReportTest Quality Score: 79/100
Test Classification DetailsView all 8 tests
Observations
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Test Quality Sentinel: 79/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). All 8 new/modified tests verify observable behavioral contracts (env-var defaults, config parsing, schema validation, YAML output content). Slight test inflation on notify_comment_test.go (3.9:1 ratio) is a metric artefact — the extra coverage is justified.
…expiration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Commit pushed:
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (122 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: 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
References:
|
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c471f529-3642-4010-863f-5527ff7c85b9 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/462a93b8-b4b3-4f60-9c79-ea83dc64e041 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/462a93b8-b4b3-4f60-9c79-ea83dc64e041 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 39d28f8 (with prior change efa4739): I reviewed and addressed all outstanding review comments by caching repo config loading in the compiler, updating the warning message to cover schema/read failures, isolating the default expiration test from repo state, and adding cache-behavior tests. |
|
Hey The PR is focused on a single concern, includes thorough test coverage across Go and JS layers (
|
|
@copilot recompile |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/12dce5c8-766b-468f-9ddf-d0c1f4548508 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Summary
aw.jsonsupport for configuring expiration of action failure issues created by the conclusion jobGH_AW_ACTION_FAILURE_ISSUE_EXPIRES_HOURShandle_agent_failure.cjsto use the configured expiration for both grouped parent issues and per-run failure issues.github/workflows/aw.jsonvalue to12hours per feedbackChanges
.github/workflows/aw.jsonschema withmaintenance.action_failure_issue_expires(integer hours, minimum 1)pkg/workflow/repo_config.gopkg/workflow/notify_comment.goto load repo config and pass expiration env var with default fallback (168h)pkg/workflow/repo_config_test.gopkg/workflow/notify_comment_test.goactions/setup/js/handle_agent_failure.test.cjsdocs/src/content/docs/guides/ephemerals.md.github/workflows/aw.jsonwith:maintenance.action_failure_issue_expires: 12Validation
make fmtgo test -v -run "TestConclusionJobActionFailureIssueExpiration|TestLoadRepoConfig_ActionFailureIssueExpires|TestActionFailureIssueExpiresHours_Default" ./pkg/workflow/cd actions/setup/js && npm run test -- handle_agent_failure.test.cjsmake buildmake agent-finish(fails due to pre-existing unrelated testifylint issues inpkg/stats/spec_test.goandpkg/testutil/spec_test.go)parallel_validation(Code Review + CodeQL) run twice; no security findingsgo test -v -run "TestLoadRepoConfig_ActionFailureIssueExpires|TestLoadRepoConfig_MaintenanceEmptyObject|TestLoadRepoConfig_FileNotFound" ./pkg/workflow/