Prevent Avenger startup failure from npm symlink mount#57946
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.
|
|
✅ PR Code Quality Reviewer completed the code quality review. 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
- "github.com"See Network Configuration for more information.
|
|
Lean already. Ship. 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
- "ab.chatgpt.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the diff: removing /usr/local/bin/npm:/usr/local/bin/npm:ro from sandbox.mounts in avenger.md, with avenger.lock.yml regenerated to match (updated frontmatter_hash, mount flags dropped from the awf command). This is a minimal, config-only change — no UI/component code, so no Impeccable UX modes apply.
Correctness check: the codex harness script still runs npm root -g inside the container (GH_AW_NPM_GLOBAL_ROOT="$(npm root -g 2>/dev/null || true)"). Without the npm binary mounted, this fails silently (|| true), leaving GH_AW_NPM_GLOBAL_ROOT empty and NODE_PATH unset for the global node_modules mount. If globally-installed npm packages were previously relied on via NODE_PATH, they may no longer resolve. Since node_modules mount is retained, worth confirming this doesn't silently break any global-module resolution in Avenger's actual run — but not blocking given the PR's stated goal (fixing the symlink-mount startup crash) takes priority and this was already broken.
No blocking issues found. Lock file and source are in sync.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 13.2 AIC · ⌖ 13.3 AIC · ⊞ 8.3K
There was a problem hiding this comment.
🟢 Approval recommended
The targeted change directly addresses the audited failure without weakening workflow protections.
Pull request overview
Removes the invalid npm symlink mount that caused Avenger’s sandbox startup failure.
Changes:
- Removes the
/usr/local/bin/npmbind mount. - Regenerates the compiled workflow and metadata hash.
File summaries
| File | Description |
|---|---|
.github/workflows/avenger.md |
Removes the failing npm mount. |
.github/workflows/avenger.lock.yml |
Synchronizes generated configuration. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment. 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
- "github.com"See Network Configuration for more information.
|
There was a problem hiding this comment.
Verdict
Blocking issue: the sandbox mount was removed, but the generated harness still shells out to npm root -g, so this can just fail one step later on runners where npm is not otherwise on PATH.
Why this blocks
The change only fixes the bind-mount shape. It does not remove the runtime dependency on npm, and the reviewed diff still contains that direct npm invocation inside the agent bootstrap command. That leaves Avenger with another startup failure mode instead of making startup robust.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 17.3 AIC · ⌖ 7.36 AIC · ⊞ 23.5K
Comment /review to run again
Comments that could not be inline-anchored
.github/workflows/avenger.lock.yml:16
This removes the symlink mount, but the generated bootstrap still calls npm root -g, so Avenger can still die during startup on runners where npm is not otherwise available on PATH.
<details><summary>💡 Why this is still a blocker</summary>
The fix only changes how the sandbox exposes Node tooling; it does not remove the runtime dependency on npm. The reviewed diff still contains a direct npm root -g call inside the container bootstrap command, so this PR trades one startup failure …
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — small, well-scoped fix, but flagging one risk worth confirming before merge.
📋 Key Themes & Highlights
Key Themes
- Symptom vs. root cause: removing the
/usr/local/bin/npmbind mount fixes the immediate sandbox rejection (symlink mounts aren't supported), but the harness still shells out tonpm root -gto buildNODE_PATH(seepkg/workflow/copilot_engine_execution.go'snodePathSetupCommand). Withnpmabsent from the sandbox, that call is silently swallowed by|| true, so it's worth confirming module resolution still works via the retained/usr/local/lib/node_modules:romount rather than assuming it does. - No regression test: the PR is a config-only change (mounts list + regenerated lock file) with no test verifying the sandbox still boots or that NODE_PATH/global-module resolution isn't silently broken.
Positive Highlights
- ✅ Correctly identifies and removes the actual cause of the reported startup failure (symlink bind mount unsupported by the sandbox).
- ✅ Minimal, targeted diff —
avenger.mdsource and its generated.lock.ymlare kept in sync.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 44.6 AIC · ⌖ 14.7 AIC · ⊞ 10.3K
Comment /matt to run again
|
@copilot Please review the current PR state, address the outstanding review feedback, refresh the branch if needed, and run the
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
- "github.com"See Network Configuration for more information.
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in f61074f: |
Status confirmed, no code or CI work remains: review thread #discussion_r3914748342 is resolved, the only blocking review ( |
|
@copilot Please take the next forward-progress pass on this PR.
|
No code or CI work remains. Re-verified: review thread #discussion_r3914748342 is resolved, the only blocking review ( |
|
@copilot Please take another pass on this PR. Please review the current status, refresh the branch if needed, address any outstanding review feedback or failing checks, and then use the
|
Re-verified, no code or CI work remains: review thread #discussion_r3914748342 is resolved, |
|
Category: bug | Risk: medium | Score: 74 | Action: fast_track | Batch: awf-runtime Note: Runtime issue blocking Avenger startup; high impact with passing CI.
|
|
@copilot Please take another pass on this PR: refresh the branch if needed, address any remaining review feedback, and run the pr-finisher skill before handing back to maintainers.
|
Re-verified again, no code or CI work remains: review thread #discussion_r3914748342 is resolved, |
|
@copilot Please take the next forward-progress pass on this PR.
|
Re-verified once more, no code or CI work remains: review thread #discussion_r3914748342 is resolved, |
…ger-failure # Conflicts: # .github/workflows/avenger.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Avenger failed before Codex execution because the sandbox rejected
/usr/local/bin/npmas a symlink bind mount.avenger.lock.ymlto keep it synchronized.Run: https://github.com/github/gh-aw/actions/runs/33679827993
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
github.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.