Skip to content

Fix bundled channel dist-runtime setup roots#82622

Closed
giodl73-repo wants to merge 2 commits into
openclaw:mainfrom
giodl73-repo:fix-bundled-dist-runtime-setup-roots
Closed

Fix bundled channel dist-runtime setup roots#82622
giodl73-repo wants to merge 2 commits into
openclaw:mainfrom
giodl73-repo:fix-bundled-dist-runtime-setup-roots

Conversation

@giodl73-repo
Copy link
Copy Markdown
Contributor

@giodl73-repo giodl73-repo commented May 16, 2026

Summary

  • Replaces the reverted Fix bundled channel dist-runtime setup roots #82472 fix with a narrow four-file change for bundled channel/plugin generated path resolution.
  • Adds dist-runtime/extensions/<plugin> as a packaged generated-entry lookup location before source fallback.
  • Uses dist-runtime/extensions/<channel> as the setup-entry module boundary root when the loaded setup module comes from packaged dist-runtime.
  • Keeps the replacement intentionally scoped: no gateway, cron, doctor, auto-reply, or unrelated fixture expectation changes from the reverted branch are included.

Fixes #77805. Replaces the relevant bundled-runtime portion of reverted #82472.

Why this shape

Packaged installs can have generated channel/plugin entries under dist-runtime/extensions, while source checkouts still have extensions. The previous behavior skipped dist-runtime in the generated-path search order, so a packaged setup entry could fall back to the source tree or use the wrong module boundary root. This PR makes packaged dist-runtime an explicit first-class generated root without broadening the change beyond bundled channel/plugin resolution.

Real behavior proof

  • Behavior or issue addressed: when dist/ is absent but dist-runtime/extensions/<plugin>/index.js exists, bundled generated entry resolution should prefer the packaged dist-runtime entry before falling back to extensions/<plugin>/index.ts.
  • Real environment tested: WSL Ubuntu-24.04 source worktrees created from upstream/main and PR head 3e53c8bb8165e01a79f9f43c6530f1eec41efaba.
  • Exact steps or command run after this patch: created clean detached worktrees for upstream/main and fork/fix-bundled-dist-runtime-setup-roots; in each worktree ran pnpm --silent exec tsx ./probe-dist-runtime-resolution.mjs. The probe created a temp package root containing both extensions/plugin/index.ts and dist-runtime/extensions/plugin/index.js, then called the real exported resolveBundledPluginGeneratedPath() and printed the resolved relative path.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output):

PR #82622 dist-runtime before/after proof

Fresh proof artifact: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/pr-82622-dist-runtime-before-after-proof.png
Proof summary: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/summary.txt

BEFORE upstream/main:
status=0
resolved=extensions/plugin/index.ts
usesDistRuntime=false
sourceFallback=true
exists=true

AFTER PR #82622 head:
status=0
resolved=dist-runtime/extensions/plugin/index.js
usesDistRuntime=true
sourceFallback=false
exists=true
  • Observed result after fix: upstream/main falls back to the source extensions/plugin/index.ts, while the PR head resolves the packaged dist-runtime/extensions/plugin/index.js entry. This demonstrates the intended packaged generated-root behavior without relying on the earlier broken screenshots.
  • What was not tested: a full packaged Windows channel setup run was not repeated for this replacement body update; CI and the focused resolver/shape regressions cover the changed code paths.

Regression coverage

  • src/plugins/bundled-plugin-metadata.test.ts now proves generated path resolution prefers dist-runtime/extensions before source fallback when packaged dist is absent.
  • src/channels/plugins/bundled.shape-guard.test.ts now proves packaged setup entries loaded from dist-runtime/extensions use that package directory as their module boundary root.

Validation

  • git diff --check
  • CI on this PR is currently green for the replacement branch.

@openclaw-barnacle openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 16, 2026
Reintroduce the bundled dist-runtime setup-root handling from the reverted PR
without the unrelated fixture expectation changes that broke main.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@giodl73-repo giodl73-repo force-pushed the fix-bundled-dist-runtime-setup-roots branch from 42872cb to 3e53c8b Compare May 16, 2026 14:39
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 16, 2026

Codex review: needs maintainer review before merge.

Summary
The PR adds dist-runtime/extensions to bundled generated-entry resolution, uses matching dist-runtime channel setup boundary roots, and adds focused regression coverage.

Reproducibility: yes. at source level: current main lacks the dist-runtime/extensions generated-entry search and setup boundary-root branch, and the inspected proof shows before/after resolver output. I did not run tests because this was a read-only review.

Real behavior proof
Sufficient (linked_artifact): The PR body’s fresh linked artifact shows after-fix real resolver output from clean WSL worktrees, with PR head choosing dist-runtime/extensions/plugin/index.js instead of source fallback.

Next step before merge
Manual review is needed because the PR has a protected maintainer label and overlaps an open maintainer-authored hardening PR; there is no narrow ClawSweeper repair to request.

Security
Cleared: The diff changes bundled path selection and tests only; it adds no dependencies, workflows, scripts, secrets handling, or new execution source outside the existing containment gate.

Review details

Best possible solution:

Land exactly one bundled-runtime path fix: either this narrow patch or the broader maintainer hardening PR, preserving dist-runtime before source fallback and the existing containment check.

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

Yes, at source level: current main lacks the dist-runtime/extensions generated-entry search and setup boundary-root branch, and the inspected proof shows before/after resolver output. I did not run tests because this was a read-only review.

Is this the best way to solve the issue?

Yes, the PR is a narrow maintainable fix for the reported packaged-runtime root mismatch. The only open solution choice is whether maintainers prefer this small patch or the broader related hardening PR.

Acceptance criteria:

  • node scripts/run-vitest.mjs src/plugins/bundled-plugin-metadata.test.ts src/channels/plugins/bundled.shape-guard.test.ts -t "dist-runtime"
  • node scripts/run-vitest.mjs src/plugins/bundled-plugin-metadata.test.ts src/channels/plugins/bundled.shape-guard.test.ts
  • git diff --check

What I checked:

  • Protected label: The provided live PR context lists the protected maintainer label, so this workflow should not close or auto-clean up the PR even though the patch looks useful.
  • Current main generated-path gap: Current main searches scan overrides, dist/extensions, then source extensions; it does not search dist-runtime/extensions before source fallback. (src/plugins/bundled-plugin-metadata.ts:231, 4b0f16d496e5)
  • Current main boundary-root gap: Current main recognizes override roots and dist/extensions before falling back to source extensions, so a packaged setup module under dist-runtime/extensions/<channel> is not selected as its own boundary root. (src/channels/plugins/bundled.ts:157, 4b0f16d496e5)
  • Containment gate preserved: Bundled channel module loading still goes through openRootFileSync with the selected boundary root, so the proposed path-root fix does not remove the existing filesystem containment check. (src/channels/plugins/module-loader.ts:103, 4b0f16d496e5)
  • PR diff scope: The provided PR diff is limited to two bundled channel/plugin resolver files and two regression tests; it adds the missing dist-runtime generated-root and setup boundary-root handling without changing workflows, dependencies, or package scripts. (src/plugins/bundled-plugin-metadata.ts:231, 73cde7221f2e)
  • Real behavior proof inspected: The fresh linked proof summary and image show upstream main resolving extensions/plugin/index.ts with source fallback, while the PR head resolves dist-runtime/extensions/plugin/index.js with usesDistRuntime=true. (3e53c8bb8165)

Likely related people:

  • @vincentkoc: Local history shows recent bundled runtime sidecar and channel seam work, and the provided related context shows this person authored the broader open hardening PR that references this PR. (role: recent area contributor and overlapping hardening PR owner; confidence: high; commits: 48fea1021a2f, 8f2ff2497a10, a67c770d849a; files: src/channels/plugins/bundled.ts, src/plugins/bundled-plugin-metadata.ts, src/channels/plugins/bundled.shape-guard.test.ts)
  • @gumadeiras: Local history shows bundled setup runtime stabilization and scan-dir override work in the same channel/plugin resolver and test surface. (role: recent bundled setup runtime contributor; confidence: medium; commits: 78ac1184274e, b2974da33a7e, 00fadb978ffe; files: src/channels/plugins/bundled.ts, src/plugins/bundled-plugin-metadata.ts, src/channels/plugins/bundled.shape-guard.test.ts)
  • @steipete: Local history shows repeated bundled channel/plugin seam generation, runtime bootstrap hardening, and metadata-startup work across the same files. (role: major adjacent refactor and runtime-boundary contributor; confidence: medium; commits: a10763e118d7, d69f20f45158, 171b24c5c5b6; files: src/channels/plugins/bundled.ts, src/plugins/bundled-plugin-metadata.ts, src/plugins/bundled-plugin-scan.ts)

Remaining risk / open question:

  • Maintainers still need to choose whether to land this narrow PR or the broader open maintainer hardening PR at fix(plugins): harden bundled runtime path resolution #82671.
  • The attached real behavior proof directly exercises the resolver path but does not repeat the full native Windows packaged openclaw doctor flow.

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

@giodl73-repo
Copy link
Copy Markdown
Contributor Author

giodl73-repo commented May 16, 2026

Superseded: these earlier QA artifacts are not the proof for #82622. The screenshots were generated incorrectly and should be ignored.

Use the fresh deterministic before/after proof now embedded in the PR body instead: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/pr-82622-dist-runtime-before-after-proof.png

@giodl73-repo
Copy link
Copy Markdown
Contributor Author

giodl73-repo commented May 16, 2026

Superseded: this earlier before/after screenshot pair was not reliable enough for #82622 review and should be ignored.

Use the fresh deterministic before/after proof now embedded in the PR body instead: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/pr-82622-dist-runtime-before-after-proof.png

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. P1 High-priority user-facing bug, regression, or broken workflow. labels May 16, 2026
@giodl73-repo
Copy link
Copy Markdown
Contributor Author

Updated #82622 with a fresh deterministic proof image and clearer PR body.

The earlier generated screenshots for this PR were not reliable, so I marked those artifact comments as superseded. The new proof compares upstream/main against PR head 3e53c8bb8165e01a79f9f43c6530f1eec41efaba using the real resolveBundledPluginGeneratedPath() export:

  • Before: resolves extensions/plugin/index.ts, usesDistRuntime=false, sourceFallback=true
  • After: resolves dist-runtime/extensions/plugin/index.js, usesDistRuntime=true, sourceFallback=false

Fresh proof image: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/pr-82622-dist-runtime-before-after-proof.png
Proof summary: https://raw.githubusercontent.com/giodl73-repo/openclaw/proof-artifacts/pr-82622-fresh/pr-82622/summary.txt

@steipete
Copy link
Copy Markdown
Contributor

Thanks for the focused replacement. This is now covered by the broader canonical fix in PR #82671.

I compared the touched files: this PR's dist-runtime generated-entry/setup-root changes are included there, and #82671 adds the surrounding containment, package-state fallback, public-surface, changelog, and broader proof coverage. Closing this one so review/landing can stay on the canonical PR.

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

Labels

maintainer Maintainer-authored PR P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. size: S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

telegram bundled channel setup fails on Windows: plugin module path escapes plugin root or fails alias checks

2 participants