fix: log stderr from rootless permission repair and make chroot-home removal non-fatal#6072
Conversation
…removal non-fatal - Capture and log stderr from the Docker-based permission repair container so failures are diagnosable (previously only exit code was logged) - Downgrade chroot-home removal failures from warn to debug since the post-step cleanup (install_copilot_cli.sh) handles it via sudo - This prevents permission repair failures from causing exit code 1 when the agent task itself completed successfully - Add tests for stderr logging behavior Closes #6070 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.15% | 99.21% | 📈 +0.06% |
| Statements | 99.11% | 99.17% | 📈 +0.06% |
| Functions | 99.45% | 99.45% | ➡️ +0.00% |
| Branches | 95.77% | 95.75% | 📉 -0.02% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-permissions.ts |
93.8% → 97.0% (+3.21%) | 93.8% → 97.0% (+3.21%) |
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
There was a problem hiding this comment.
Pull request overview
This PR improves post-run cleanup behavior in rootless Docker scenarios by making permission-repair failures more diagnosable (logging stderr) and preventing chroot-home cleanup issues from failing an otherwise successful agent run.
Changes:
- Include trimmed
stderrin the warning emitted when the rootless permission-repairdocker runexits non-zero. - Make chroot-home removal failures non-fatal by downgrading logging to
debugand swallowing retry failures. - Add tests to validate stderr inclusion and avoid a trailing colon when stderr is empty.
Show a summary per file
| File | Description |
|---|---|
| src/artifact-permissions.ts | Appends trimmed stderr to permission-repair failure warnings for better diagnostics. |
| src/artifact-preservation.ts | Makes chroot-home deletion failures non-fatal and lowers log severity to avoid failing successful runs. |
| src/artifact-permissions.test.ts | Adds regression tests for stderr formatting in permission-repair failure logs. |
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: 3
- Review effort level: Low
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
✅ Copilot review passed with no inline comments. @lpcox Add the |
|
❌ Smoke Copilot BYOK AOAI (Entra) reports failed. AOAI BYOK (Entra) mode investigation needed... |
|
📡 Smoke OTel Tracing completed. All tracing scenarios validated. ✅ |
|
✅ Smoke Gemini completed. All facets verified. 💎 |
|
🔑 Smoke Copilot PAT PAT auth validated. All systems operational. ✅ |
|
🔌 Smoke Services — All services reachable! ✅ |
|
❌ Smoke Copilot BYOK AOAI (api-key) reports failed. AOAI BYOK (api-key) mode investigation needed... |
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
|
✅ Build Test Suite completed successfully! |
|
Chroot tests passed! Smoke Chroot - All security and functionality tests succeeded. |
|
✅ Contribution Check completed successfully! PR #6072 follows the applicable CONTRIBUTING.md guidelines: it includes focused tests for the behavior change, has a clear description referencing issue #6070, uses appropriate src/ locations, and does not require documentation updates. |
|
🚀 Security Guard has started processing this pull request |
|
✅ Smoke Claude passed |
|
✅ Smoke Copilot BYOK completed. Copilot BYOK mode operational. 🔓 |
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.15% | 99.21% | 📈 +0.06% |
| Statements | 99.11% | 99.17% | 📈 +0.06% |
| Functions | 99.45% | 99.45% | ➡️ +0.00% |
| Branches | 95.77% | 95.75% | 📉 -0.02% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-permissions.ts |
93.8% → 97.0% (+3.21%) | 93.8% → 97.0% (+3.21%) |
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
1 similar comment
|
| Metric | Base | PR | Delta |
|---|---|---|---|
| Lines | 99.15% | 99.21% | 📈 +0.06% |
| Statements | 99.11% | 99.17% | 📈 +0.06% |
| Functions | 99.45% | 99.45% | ➡️ +0.00% |
| Branches | 95.77% | 95.75% | 📉 -0.02% |
📁 Per-file Coverage Changes (2 files)
| File | Lines (Before → After) | Statements (Before → After) |
|---|---|---|
src/artifact-permissions.ts |
93.8% → 97.0% (+3.21%) | 93.8% → 97.0% (+3.21%) |
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 Test: Claude Engine ValidationOverall result: ✅ PASS
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔥 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. ✅ Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot BYOK (Direct) Mode
Running in direct BYOK mode ( Overall: PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🔬 Smoke Test Results
Overall: PASS — core connectivity verified PR author: @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Copilot PAT Auth
Overall: PASS (partial — pre-step template data unavailable) Auth mode: PAT (COPILOT_GITHUB_TOKEN) | PR author: @lpcox Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Services Connectivity
Overall: FAIL — Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
Smoke Test: Gemini Engine Validation
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
Chroot Version Comparison Results
Overall: NOT all tests passed. Python and Node.js versions differ between host and chroot environments. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
fix: log stderr from rootless permission repair and make chroot-home removal non-fatal Warning Firewall blocked 2 domainsThe following domains were blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"
- "registry.npmjs.org"See Network Configuration for more information.
|
- Update D1: gVisor resolved in #6093; Kata Containers still open - Add B11: silent repair failure causes exit 1, fixed in #6072 - Add B11 error-string lookup entry in all three catalog files - Update Known unresolved items: narrow D1 to Kata only - Update symptom mapping and known-gaps step in both doctor files - Update CI test assertions for D1 and B11 Closes #6103
…nt repair exit-1 (#6118) * Initial plan * docs: update runner doctor catalog - D1 gVisor resolved, add B11 - Update D1: gVisor resolved in #6093; Kata Containers still open - Add B11: silent repair failure causes exit 1, fixed in #6072 - Add B11 error-string lookup entry in all three catalog files - Update Known unresolved items: narrow D1 to Kata only - Update symptom mapping and known-gaps step in both doctor files - Update CI test assertions for D1 and B11 Closes #6103 * docs: align runner-doctor B11 and D1 wording with implementation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
Fixes #6070 — rootless artifact permission repair failures cause exit code 1 even when the agent task completed successfully.
Observed in: https://github.com/github/gh-aw-mcpg/actions/runs/29042841176 (Large Payload Tester)
Problem
After the agent completes and containers are stopped,
fixArtifactPermissionsForRootless()runs adocker runto chown/chmod root-owned files back to the runner user. When this fails:(exit 1)was visible, making diagnosis impossibleEACCESerror fromfs.rmSyncpropagated as exit code 1, failing the entire step despite the agent task having succeededPost Setup Scriptsstep (compiled by gh-aw) successfully cleaned up the same directory viasudo— so AWF's Node.jsrimrafSyncattempt was redundantChanges
src/artifact-permissions.tsstderrfrom the repairdocker runcommand so failures are diagnosablesrc/artifact-preservation.tslogger.warntologger.debugsince the post-step cleanup handles it via sudosrc/artifact-permissions.test.tsTesting