fix: demote benign rootless cleanup errors - #6939
Conversation
Reuse the rootless artifact permission classification for direct cleanup chmod failures. Add coverage for diagnostic ordering and unexpected failures. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… into fix-rootless-cleanup-diagnostics
There was a problem hiding this comment.
Pull request overview
Demotes benign rootless cleanup permission errors so primary startup failures remain prominent.
Changes:
- Adds a reusable permission-error classifier.
- Uses debug logging for expected cleanup failures.
- Adds regression coverage for benign and unexpected failures.
Show a summary per file
| File | Description |
|---|---|
src/artifact-preservation.ts |
Classifies cleanup chmod failures. |
src/artifact-preservation-errors.test.ts |
Tests cleanup diagnostic behavior. |
src/artifact-permissions.ts |
Introduces the shared error classifier. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
| return /(?:^|\n)(?:chown|chmod):.*(?:operation not permitted|permission denied|\bEPERM\b|\bEACCES\b)/i.test( | ||
| details.join('\n'), | ||
| ); |
| if (isBenignArtifactPermissionError(error)) { | ||
| logger.debug( | ||
| `${permissionErrorMessage} Permission repair was denied for ${targetDir}; ` + | ||
| 'this is expected on restricted runners and does not affect the run.', | ||
| ); | ||
| } else { | ||
| logger.warn(permissionErrorMessage, error); |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.94% | 98.97% | 📈 +0.03% |
| Statements | 98.83% | 98.85% | 📈 +0.02% |
| Functions | 99.19% | 99.19% | ➡️ +0.00% |
| Branches | 95.16% | 95.14% | 📉 -0.02% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-preservation.ts |
96.3% → 96.3% (+0.07%) | 96.3% → 96.4% (+0.07%) |
src/artifact-permissions.ts |
97.4% → 97.9% (+0.48%) | 97.4% → 97.9% (+0.48%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
@copilot address review feedback |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✅ Smoke Claude passed |
|
🔌 Smoke Services — All services reachable! ✅ |
|
🛡️ Smoke Copilot Network Isolation confirmed the egress allowlist is enforced. ✅ |
|
✅ Contribution Check completed successfully! PR #6939 follows CONTRIBUTING.md on code style, docs, PR description, and file organization. The patch includes targeted tests for the new cleanup diagnostic behavior, and the PR description references Fixes #6934. No guideline issues to report. |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
✅ Smoke Gemini completed. All facets verified. 💎 test |
|
🌑 The shadows whisper... Smoke Codex failed. The oracle requires further meditation... |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
📡 Smoke OTel Tracing reports failed. OTel tracing regression detected. Smoke test OTEL tracing - all scenarios pass: (1) otel.js loads cleanly, exports startRequestSpan/setTokenAttributes/setBudgetAttributes/endSpan/isEnabled etc.; (2) test suite 61/61 passed (otel.test.js + otel-fanout.test.js); (3) env var forwarding confirmed - env-passthrough.ts forwards GITHUB_AW_OTEL_TRACE_ID/PARENT_SPAN_ID to agent, api-proxy-env-config.ts buildOtelEnv() forwards OTLP endpoint/headers/trace context to api-proxy container; (4) onUsage callback confirmed present in token-tracker-http.js as OTEL hook point; (5) diagnostics step output not independently re-verified beyond above, no unexpected failures found. Overall: PASS. |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
🚀 Security Guard has started processing this pull request |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
smoke-copilot-network-isolation
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🚀 Security Guard has started processing this pull request |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
🔌 Smoke Services — All services reachable! ✅ |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
📰 VERDICT: Smoke Docker Sbx has concluded. All systems operational. This is a developing story. 🎤 |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 98.94% | 98.96% | 📈 +0.02% |
| Statements | 98.83% | 98.84% | 📈 +0.01% |
| Functions | 99.19% | 99.19% | ➡️ +0.00% |
| Branches | 95.16% | 95.13% | 📉 -0.03% |
📁 Per-file Coverage Changes (3 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-preservation.ts |
96.3% → 95.5% (-0.77%) | 96.3% → 95.5% (-0.76%) |
src/artifact-permissions.ts |
97.4% → 98.0% (+0.52%) | 97.4% → 98.0% (+0.52%) |
src/log-directory-setup.ts |
96.2% → 100.0% (+3.78%) | 96.3% → 100.0% (+3.71%) |
Coverage comparison generated by scripts/ci/compare-coverage.ts
|
✅ Smoke Claude passed |
|
Smoke Test: Copilot Network Isolation Egress EGRESS_RESULT allow=pass deny=pass
Overall status: PASS cc @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "example.com"See Network Configuration for more information.
|
|
Smoke Test: Copilot Engine — @lpcox
Overall: PASS
|
Smoke Test: Copilot BYOK (Direct) Mode
Running in direct BYOK mode (COPILOT_PROVIDER_API_KEY) via api-proxy → api.githubcopilot.com Overall: PASS cc @lpcox
|
|
Smoke Test Results:
Overall: FAIL —
|
Smoke Test Results
Overall Status: FAIL Last 2 PRs identified: #6935, #6928 (Titles unavailable due to secrecy policy)
|
Smoke Test: Claude Engine Validation
Overall result: PASS
|
|
Smoke test summary
Overall: FAIL
|
Chroot Version Comparison Results
Overall: FAILED — Node.js version differs between host and chroot environment.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — PASS Notes:
|
|
Smoke Test: Docker Sbx — @lpcox
Overall: PASS
|
Summary
chmodfailuresEPERM/EACCEScleanup failures to concise debug outputFixes #6934