Handle failure-issue permission denials and enforce timeout-specific failure messaging#38273
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #38273 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold). Neither Condition A nor Condition B is met. |
|
❌ Test Quality Sentinel was skipped during test quality analysis. |
|
🧠 Matt Pocock Skills Reviewer was skipped during the skills-based review. |
There was a problem hiding this comment.
Pull request overview
This PR tightens agent failure handling in actions/setup/js/handle_agent_failure.cjs so that (1) timeout-driven runs don’t include generic engine-failure context and (2) failure-issue creation/update degrades cleanly (info-level skip) when the token lacks permission to write issues. It also wires new “detect agent errors” outputs through many compiled .lock.yml workflows to feed richer failure classification signals into the conclusion/failure-handling path.
Changes:
- Added
shouldBuildEngineFailureContext()to suppress engine-failure context when tool-denials-exceeded or timeouts are present. - Added
isIssueWritePermissionError()and downgraded issue-write permission denials from warning noise to an informational skip. - Updated many compiled workflows to run
detect_agent_errors.cjsand export/wire new error classification outputs (timeout signal, inference access, MCP policy, model not supported).
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/handle_agent_failure.cjs | Adds timeout-first engine-failure context gating and permission-aware issue-write error handling. |
| actions/setup/js/handle_agent_failure.test.cjs | Adds unit tests for engine-failure context gating and issue-write permission error classification. |
| .github/workflows/unbloat-docs.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/typist.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/test-create-pr-error-handling.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/step-name-alignment.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/static-analysis-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/spec-enforcer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-codex.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-claude.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-call-workflow.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-agent-scoped-approved.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-agent-public-none.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-agent-public-approved.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-agent-all-none.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/smoke-agent-all-merged.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/sergo.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/semantic-function-refactor.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/scout.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/schema-feature-coverage.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/schema-consistency-checker.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/safe-output-health.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/ruflo-backed-task.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/prompt-clustering-analysis.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/necromancer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/lockfile-stats.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/issue-arborist.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/instructions-janitor.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/hourly-ci-cleaner.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/grumpy-reviewer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/go-pattern-detector.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/go-logger.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/go-fan.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/github-mcp-tools-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/github-mcp-structural-analysis.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/example-workflow-analyzer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/duplicate-code-detector.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/developer-docs-consolidator.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/dev.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/design-decision-gate.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/deep-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-token-consumption-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-team-evolution-insights.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-security-red-team.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-safeoutputs-git-simulator.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-safe-outputs-conformance.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-safe-output-optimizer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-rendering-scripts-verifier.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-reliability-review.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-observability-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-multi-device-docs-tester.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-function-namer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-fact.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-doc-updater.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-doc-healer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-code-metrics.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-choice-test.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-caveman-optimizer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-cache-strategy-analyzer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-awf-spec-compiler-surfacing.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-aw-cross-repo-compile-check.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-astrostylelite-markdown-spellcheck.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/daily-agentrx-trace-optimizer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/copilot-session-insights.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/copilot-agent-analysis.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/commit-changes-analyzer.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/codex-github-remote-mcp-test.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/cloclo.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/cli-version-checker.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/claude-code-user-docs-review.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/ci-doctor.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/changeset.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/blog-auditor.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/aw-failure-investigator.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/avenger.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/audit-workflows.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/approach-validator.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/api-consumption-report.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
| .github/workflows/ai-moderator.lock.yml | Adds detect-agent-errors step + exports/wires agent error outputs (timeout/inference/MCP policy/model support). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 79/79 changed files
- Comments generated: 2
| function isIssueWritePermissionError(error) { | ||
| const status = error && typeof error === "object" && "status" in error ? Number(/** @type {{status?: unknown}} */ error.status) : NaN; | ||
| const message = getErrorMessage(error).toLowerCase(); | ||
| return status === 403 && (message.includes("resource not accessible by integration") || message.includes("resource not accessible by personal access token") || message.includes("insufficient permissions")); |
| if (isIssueWritePermissionError(error)) { | ||
| core.info("Skipping failure tracking issue creation/update: token lacks issues:write permission (Resource not accessible by integration)"); | ||
| } else { | ||
| core.warning(`Failed to create or update failure tracking issue: ${getErrorMessage(error)}`); |
There was a problem hiding this comment.
REQUEST_CHANGES — three issues block merge
The shouldBuildEngineFailureContext extraction and the isTimedOut guard are correct and a welcome clean-up. Three issues need to be addressed before merge.
🔍 Findings summary
High
core.infoalways prints"Resource not accessible by integration"in its parenthetical, even when the matched pattern was"insufficient permissions"or"resource not accessible by personal access token". An operator reading the Actions log will be pointed at the wrong error type.
Medium
- The
"resource not accessible by personal access token"message variant is listed inisIssueWritePermissionErrorbut has no test case. Any typo in that string would silently regress. "insufficient permissions"is a very broad substring — it matches GitHub 403s from many unrelated operations (labels, sub-issue linking, etc.). Since the function is now exported, the nameisIssueWritePermissionErrorsets a narrower expectation than the implementation delivers.
🔎 Code quality review by PR Code Quality Reviewer · 8.63 AIC · ⌖ 13.1 AIC
| } catch (error) { | ||
| core.warning(`Failed to create or update failure tracking issue: ${getErrorMessage(error)}`); | ||
| if (isIssueWritePermissionError(error)) { | ||
| core.info("Skipping failure tracking issue creation/update: token lacks issues:write permission (Resource not accessible by integration)"); |
There was a problem hiding this comment.
Misleading log message: the hardcoded text always says "Resource not accessible by integration" regardless of which of the three patterns was actually matched, so when the real error was "insufficient permissions" or "resource not accessible by personal access token", the core.info message points operators at the wrong GitHub error type.
💡 Suggested fix
Replace the hardcoded parenthetical with the actual error message so the log is always accurate:
core.info(
`Skipping failure tracking issue creation/update: token lacks issues:write permission (${ getErrorMessage(error) })`
);This preserves the actionable explanation ("token lacks issues:write permission") while surfacing the exact error text that matches the real scenario — "insufficient permissions", "resource not accessible by personal access token", or "resource not accessible by integration" — so anyone reading the Actions log can immediately correlate it with the correct GitHub API error documentation.
|
|
||
| it("returns false for non-403 errors", () => { | ||
| expect(isIssueWritePermissionError({ status: 500, message: "Internal server error" })).toBe(false); | ||
| }); |
There was a problem hiding this comment.
Missing test for "resource not accessible by personal access token" message pattern: isIssueWritePermissionError explicitly matches three strings, but the test suite only exercises two of them — the third pattern goes untested and could silently break if the string is ever typo-ed or refactored.
💡 Suggested fix
Add a third test case in the same describe block:
it("returns true for 403 resource not accessible by personal access token", () => {
expect(
isIssueWritePermissionError({ status: 403, message: "Resource not accessible by personal access token" })
).toBe(true);
});All three listed patterns should be covered so a future edit to the match list produces a test failure rather than a silent regression.
| function isIssueWritePermissionError(error) { | ||
| const status = error && typeof error === "object" && "status" in error ? Number(/** @type {{status?: unknown}} */ error.status) : NaN; | ||
| const message = getErrorMessage(error).toLowerCase(); | ||
| return status === 403 && (message.includes("resource not accessible by integration") || message.includes("resource not accessible by personal access token") || message.includes("insufficient permissions")); |
There was a problem hiding this comment.
Overly broad "insufficient permissions" match string: this substring appears in many GitHub API 403 responses (labeling, sub-issue linking, repo content writes, etc.), not only issue write failures. Because isIssueWritePermissionError is now exported, a future caller in a different catch block could inadvertently silence warnings for unrelated permission errors.
💡 Suggested fix
Either:
- Tighten the match to the specific GitHub token-permission phrasing (
"insufficient scopes"is more precise for that class of error, though you should verify the exact strings returned by your Octokit version), or - Rename the predicate to
isTokenPermissionErrorand document in its JSDoc that it is deliberately broad across all 403 permission flavors so future callers understand the intent.
If the function is truly meant to be a general-purpose 403-classifier for any GitHub write, the current name isIssueWritePermissionError is misleading; callers will be surprised when it also matches a 403 on a label API call.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. I ran the pr-finisher flow and pushed follow-up fixes in 2a8d51d. Local validation passed ( CI on the new agent HEAD is stale and must be re-triggered by a maintainer before merge. |
|
@copilot merge main and recompile, fix conflicts |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review all comments and address unresolved review feedback.
|
|
@copilot summarize the remaining blockers after the branch update.
|
The failure handler was emitting noisy warnings when issue creation was blocked by token permissions, and timeout-driven failures could still include generic engine-failure context. This change tightens failure classification so timeout runs get dedicated timeout messaging and permission-limited issue writes degrade cleanly.
Permission-aware failure issue handling
isIssueWritePermissionError(error)to classify403issue-write permission failures (including “Resource not accessible by integration”).Timeout-first failure context selection
shouldBuildEngineFailureContext(agentConclusion, hasToolDenialsExceeded, isTimedOut).Targeted unit coverage
pr-sous-chef: update branch requested for run https://github.com/github/gh-aw/actions/runs/27253983367