Skip to content

[codex] Fix runtime deps update from legacy symlinks#75288

Merged
steipete merged 3 commits intoopenclaw:mainfrom
goldmar:codex/fix-runtime-deps-symlink-update
May 1, 2026
Merged

[codex] Fix runtime deps update from legacy symlinks#75288
steipete merged 3 commits intoopenclaw:mainfrom
goldmar:codex/fix-runtime-deps-symlink-update

Conversation

@goldmar
Copy link
Copy Markdown
Contributor

@goldmar goldmar commented Apr 30, 2026

Summary

  • remove only legacy OpenClaw-owned bundled runtime dependency node_modules symlinks before materializing replacement deps
  • keep the existing refusal for arbitrary symlinked runtime dependency paths
  • cover the update migration case in the bundled runtime deps staging tests

Root Cause

Users updating from the older runtime dependency layout can have dist/extensions//node_modules symlinked into .local/bundled-plugin-runtime-deps. The newer staging path refuses to replace symlinked node_modules paths, so update builds can fail before the new materialized runtime deps are written.

Validation

  • pnpm exec vitest run test/scripts/stage-bundled-plugin-runtime-deps.test.ts
  • pnpm build

The test run emitted Rolldown plugin timing warnings only; no compiler deprecation warnings appeared during the build.

@openclaw-barnacle openclaw-barnacle Bot added scripts Repository scripts size: S labels Apr 30, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 30, 2026

Codex review: needs maintainer review before merge.

What this changes:

The branch adds a helper to remove legacy .local/bundled-plugin-runtime-deps/**/node_modules symlinks before staged bundled plugin dependency replacement/removal, plus a regression test and changelog entry.

Maintainer follow-up before merge:

This is an open, ready, mergeable implementation PR with no discrete automated repair finding; the next action is maintainer review plus exact-head CI/changed-gate validation.

Security review:

Security review cleared: The diff touches filesystem symlink handling but does not add workflows, dependency sources, lockfile changes, lifecycle hooks, or secret handling; the symlink exception is scoped to removing the plugin-local symlink itself before the existing guard handles all other symlinks.

Review details

Best possible solution:

Land this patch or an equivalent narrow fix after exact-head validation passes, keeping the legacy symlink exception scoped to OpenClaw-owned runtime-deps migration and leaving arbitrary symlink replacement blocked.

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

Yes. A focused reproduction is to make dist/extensions/<plugin>/node_modules a symlink to .local/bundled-plugin-runtime-deps/<id>/node_modules and run bundled runtime-deps staging; current main reaches the generic symlink refusal before materializing replacement deps.

Is this the best way to solve the issue?

Yes. The PR is the narrow maintainable approach because it unlinks only legacy runtime-deps symlinks resolving under the OpenClaw-owned root, then lets the existing replacement/removal and arbitrary-symlink guards continue to enforce the unsafe cases.

Acceptance criteria:

  • pnpm test test/scripts/stage-bundled-plugin-runtime-deps.test.ts
  • pnpm build
  • pnpm check:changed

What I checked:

Likely related people:

  • steipete: Current-main blame for assertPathIsNotSymlink, replaceDirAtomically, and both script staging call sites points to Peter Steinberger; he also owns the adjacent open runtime-deps repair PR fix: simplify bundled runtime dependency repair #75183 in the supplied context. (role: current staging helper owner and adjacent runtime-deps maintainer; confidence: high; commits: 48794b9f88c3, 3c4851037b6c; files: scripts/lib/bundled-runtime-deps-stage-state.mjs, scripts/lib/bundled-runtime-deps-materialize.mjs, scripts/stage-bundled-plugin-runtime-deps.mjs)
  • vincentkoc: Current changelog credits @vincentkoc for recent runtime-deps stage-key work in fix(plugins): canonicalize packageRoot before hashing runtime-deps stage key #75048, and local history shows the associated commit touching bundled runtime-deps behavior adjacent to this migration path. (role: recent adjacent runtime-deps maintainer; confidence: medium; commits: 4b98f0952934; files: src/plugins/bundled-runtime-deps.test.ts, src/plugins/bundled-runtime-deps.ts, CHANGELOG.md)

Remaining risk / open question:

  • Exact-head CI and pnpm check:changed results were not available from the provided context, and this read-only review did not run tests or builds.
  • The PR body reports a raw Vitest invocation; before merge, validation should use the repo wrapper command for the focused test plus the normal changed gate.

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

@goldmar goldmar force-pushed the codex/fix-runtime-deps-symlink-update branch from ba07db4 to df8b043 Compare May 1, 2026 05:32
@goldmar goldmar marked this pull request as ready for review May 1, 2026 05:33
@openclaw-barnacle openclaw-barnacle Bot added the docker Docker and sandbox tooling label May 1, 2026
@steipete steipete force-pushed the codex/fix-runtime-deps-symlink-update branch from b68a98b to 4a69feb Compare May 1, 2026 10:15
@steipete steipete merged commit 018f77c into openclaw:main May 1, 2026
69 of 70 checks passed
@steipete
Copy link
Copy Markdown
Contributor

steipete commented May 1, 2026

Landed via rebase onto main.

  • Source PR head: 4a69feb
  • Merge commit: 018f77c
  • Local proof: pnpm test test/scripts/stage-bundled-plugin-runtime-deps.test.ts -- --reporter=verbose
  • Local proof: bash -n scripts/e2e/upgrade-survivor-docker.sh scripts/e2e/lib/upgrade-survivor/run.sh && pnpm exec oxfmt --check --threads=1 scripts/lib/bundled-runtime-deps-stage-state.mjs test/scripts/stage-bundled-plugin-runtime-deps.test.ts && git diff --check
  • Docker proof: published openclaw@2026.4.26 baseline with seeded Telegram legacy runtime-deps symlink updated to candidate 2026.4.30, repaired the symlink, then passed doctor, gateway start, and gateway status.
  • Testbox proof: OPENCLAW_TESTBOX=1 pnpm check:changed on the rebased branch.

Thanks @goldmar!

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

Labels

docker Docker and sandbox tooling scripts Repository scripts size: M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants