Skip to content

[codex] Fix npm-installed skill CLI PATH detection#80233

Closed
NianJiuZst wants to merge 1 commit into
openclaw:mainfrom
NianJiuZst:codex/fix-skills-npm-global-path
Closed

[codex] Fix npm-installed skill CLI PATH detection#80233
NianJiuZst wants to merge 1 commit into
openclaw:mainfrom
NianJiuZst:codex/fix-skills-npm-global-path

Conversation

@NianJiuZst

Copy link
Copy Markdown
Contributor

Summary

Root cause

  • Skills status checks requires.bins through the gateway process PATH, while npm-installed skill CLIs can live in npm global prefix directories that were not included by the gateway's minimal PATH bootstrap.

Verification

  • pnpm test src/infra/path-env.test.ts -- --reporter=verbose
  • pnpm test src/agents/skills.buildworkspaceskillstatus.test.ts -- --reporter=verbose
  • git diff --check
  • pnpm check:changed

Fixes #80206

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 10, 2026
@NianJiuZst
NianJiuZst marked this pull request as ready for review May 10, 2026 11:23
@clawsweeper

clawsweeper Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge.

Summary
The PR adds npm global prefix discovery to the gateway PATH bootstrap, updates path-env regression tests, and adds a Skills changelog entry.

Reproducibility: no. not as a high-confidence live reproduction. Source inspection shows the current gateway status path scans process.env.PATH, and current PATH bootstrap omits npm-global/prefix dirs, matching the reported sparse-service PATH failure.

Real behavior proof
Needs real behavior proof before merge: The PR only lists tests/checks; before merge the contributor should add redacted terminal output, logs, screenshot/recording, or copied live output from a real gateway/npm-global setup and update the PR body to trigger re-review, or ask for @clawsweeper re-review.

Next step before merge
External PR needs contributor-supplied real behavior proof; there is no narrow code defect for ClawSweeper to repair automatically.

Security
Cleared: The diff appends npm user bin paths after trusted system dirs and changes only tests/changelog around that behavior, with no concrete security or supply-chain regression found.

Review details

Best possible solution:

Land a narrow PATH-bootstrap fix after redacted real gateway/Skills proof shows npm-global clawhub and mcporter are detected without changing trust ordering.

Do we have a high-confidence way to reproduce the issue?

No, not as a high-confidence live reproduction. Source inspection shows the current gateway status path scans process.env.PATH, and current PATH bootstrap omits npm-global/prefix dirs, matching the reported sparse-service PATH failure.

Is this the best way to solve the issue?

Yes, the PR's narrow bootstrap change is the right layer because the UI is reporting backend Skills status. It still needs real behavior proof before merge.

Acceptance criteria:

  • pnpm test src/infra/path-env.test.ts -- --reporter=verbose
  • pnpm test src/agents/skills.buildworkspaceskillstatus.test.ts -- --reporter=verbose
  • git diff --check
  • pnpm check:changed

What I checked:

  • Current main omits npm-global PATH discovery: Current candidateBinDirs appends Homebrew, mise, pnpm, XDG, .local/bin, Bun, and Yarn locations after trusted system dirs, but it does not append npm global prefix directories or ~/.npm-global/bin. (src/infra/path-env.ts:103, 0451a9fb312c)
  • Skills status depends on process PATH: buildSkillStatus evaluates required binaries with hasBinary, and hasBinary scans process.env.PATH, which makes a sparse gateway PATH sufficient to produce false missing-requirements status. (src/agents/skills-status.ts:228, 0451a9fb312c)
  • PR diff is scoped to npm bin discovery: The diff adds npm prefix parsing from NPM_CONFIG_PREFIX, npm_config_prefix, user .npmrc, OpenClaw-managed npm tools, and ~/.npm-global/bin, then appends discovered directories after existing user-writable paths. (src/infra/path-env.ts:30, 0a8d90546525)
  • Regression coverage targets the new prefix cases: The PR adds path-env coverage for env npm prefixes, user .npmrc prefix, managed npm tools, and ~/.npm-global/bin, while preserving the after-system-dir ordering assertion. (src/infra/path-env.test.ts:290, 0a8d90546525)
  • Related issue supports the source-level root cause: The linked issue reports clawhub and mcporter working in terminal but missing in Skills UI, and a follow-up commenter reported clawhub at /home/azfar/.npm-global/bin/clawhub with a sparse systemd service PATH.
  • Real behavior proof is absent: The PR body lists unit/changed checks, and the PR discussion only contains a ClawSweeper placeholder comment; there is no terminal/log/screenshot/recording proof showing the fixed gateway Skills status in a real npm-global setup. (0a8d90546525)

Likely related people:

  • steipete: Recent commits shaped daemon and gateway PATH handling, and current source/blame in this checkout also routes the relevant PATH/bootstrap and requirement-evaluation area through his commits. (role: recent path/bootstrap contributor; confidence: high; commits: 85ce75c005a2, eb3e4f20a0af, abf5dea7ddb5; files: src/infra/path-env.ts, src/shared/config-eval.ts, src/daemon/service-env.ts)
  • mbelinky: Authored the recent merged Skills status visibility change in the central status report builder used by the Control UI. (role: recent skills status contributor; confidence: medium; commits: 3b347d1c7ea0; files: src/agents/skills-status.ts)
  • BunsDev: Authored the recent LaunchAgent PATH change that added Homebrew paths, which is adjacent to the service/gateway PATH mismatch being addressed here. (role: recent adjacent contributor; confidence: medium; commits: 84fe3c5409cb; files: src/daemon/service-env.ts)
  • optimol: Authored the path hardening commit that is relevant to preserving non-user-writable ordering while adding more package-manager paths. (role: path trust contributor; confidence: medium; commits: c40884d306e8; files: src/infra/path-env.ts)

Remaining risk / open question:

  • The contributor has not shown after-fix real behavior proof from a real gateway/npm-global setup.
  • No live macOS launchd or Linux systemd gateway run was inspected, so the exact reporter environment remains source-proven rather than live-proven.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 0451a9fb312c.

@hclsys

This comment was marked as low quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Skills UI shows "Missing requirements" for clawhub and mcporter despite both binaries being installed and functional

2 participants