Skip to content

fix: plugin tests fail on hosts with a usable /tmp/openclaw or a source checkout#101896

Merged
steipete merged 1 commit into
mainfrom
claude/sad-margulis-6008fc
Jul 7, 2026
Merged

fix: plugin tests fail on hosts with a usable /tmp/openclaw or a source checkout#101896
steipete merged 1 commit into
mainfrom
claude/sad-margulis-6008fc

Conversation

@steipete

@steipete steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Closes #101876

What Problem This Solves

Fixes an issue where three plugin test cases fail on any POSIX host with a normal /tmp/openclaw state or a source checkout (developer laptops, warm CI boxes), blocking green local runs of the plugins shard:

  • src/plugins/git-install.test.ts > falls back to OS temp when target workspace creation fails
  • src/plugins/manifest-model-id-normalization.test.ts > reuses manifest metadata while file fingerprints are unchanged
  • src/plugins/tools.optional.test.ts > reloads when gateway binding would otherwise reuse a default-mode active registry

Why This Change Was Made

The three failures have three distinct causes — recent prod changes landed with stale test expectations; this PR updates only the tests to the current prod contracts (no prod behavior changes):

  1. git-install: fix(security): route temp workspaces through private OpenClaw temp root (#101224) #101246 made withTempDir root fallback staging at resolvePreferredOpenClawTmpDir() (prefers /tmp/openclaw, degrades to a uid-scoped os.tmpdir() path when unsafe), but the test still asserted bare os.tmpdir(). The test now recomputes the resolver output, so the assertion holds in every resolver state.
  2. manifest scan: 26c0285 added the source-checkout manifest metadata fallback, so listOpenClawPluginManifestMetadata legitimately lists all repo extensions/ manifests when tests run from a checkout (140 records, not 1). The test now pins exactly one record for the plugin under test and keeps the fingerprint-reuse assertion (manifest file read exactly once across two scans).
  3. tools.optional: refactor(plugins): require initialized registry collections #100990 requires initialized registry collections (registryHasPluginHostCleanupWork no longer optional-chains), and updated most test registries but missed this hand-rolled {plugins, tools, diagnostics} as never fake, which crashes when retired. The test now uses createEmptyPluginRegistry(), which also drops the as never cast.

Note: the issue attributes all three failures to the temp-root mechanism; investigation showed only the git-install test is temp-root-coupled. The other two are stale expectations from the July 6–7 prod changes above.

User Impact

No user-visible behavior change. Developers and CI hosts get deterministic plugin-shard test results regardless of /tmp/openclaw state (present, absent, or unsafe) and regardless of running from a source checkout.

Evidence

node scripts/run-vitest.mjs src/plugins/git-install.test.ts src/plugins/manifest-model-id-normalization.test.ts src/plugins/tools.optional.test.ts (macOS, worktree of e7365d4):

  • Before: Tests 3 failed | 95 passed (98) — reproducing the issue exactly.
  • After, with /tmp/openclaw present and safe: Tests 98 passed (98).
  • After, with /tmp/openclaw absent (resolver creates it): Tests 98 passed (98).
  • After, with /tmp/openclaw unsafe (symlink → uid-scoped os.tmpdir() fallback): git-install.test.ts 23 passed (23) (the only resolver-coupled file).
  • Full plugins shard (node scripts/run-vitest.mjs src/plugins): pass (see CI for the hosted run).

oxfmt --check and scripts/run-oxlint.mjs clean on the three changed files.

@openclaw-barnacle openclaw-barnacle Bot added size: XS maintainer Maintainer-authored PR labels Jul 7, 2026
@clawsweeper

clawsweeper Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 7, 2026, 6:47 PM ET / 22:47 UTC.

Summary
Updates three plugin tests to align with the current temp-root resolver, source-checkout manifest metadata scan, and fully initialized plugin registry shape.

PR surface: Tests +7. Total +7 across 3 files.

Reproducibility: yes. source-reproducible: current main contains the stale os.tmpdir assertion, source-checkout one-record manifest assertion, and partial registry fixture while current production code supports the PR's expected contracts. I did not execute Vitest in this read-only review, but the PR body includes before/after terminal output.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #101876
Summary: This PR is a candidate fix for the linked plugin test reliability issue and overlaps a narrower PR that handles only the git-install temp-root assertion.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] The PR is mergeable but behind current main, so final landing should rely on refreshed exact-head or merge-queue CI rather than the stale branch result alone.

Maintainer options:

  1. Decide the mitigation before merge
    Land this broader test-only fix after maintainer review and refreshed exact-head CI, then use it as the canonical fix for [Bug]: plugin temp-dir tests fail on any host where /tmp/openclaw is preferable (env-coupled os.tmpdir assertions) #101876 and supersede the narrower overlapping fix(tests): pin plugin temp-dir assertions to resolvePreferredOpenClawTmpDir() (#101876) #101884 if maintainers prefer this branch.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed because the open PR already contains the narrow fix; maintainers should choose the landing path against the protected label and overlapping narrower PR.

Maintainer decision needed

  • Question: Should maintainers land this broader test-only PR as the canonical fix for [Bug]: plugin temp-dir tests fail on any host where /tmp/openclaw is preferable (env-coupled os.tmpdir assertions) #101876, or prefer the narrower overlapping PR plus separate follow-ups?
  • Rationale: The patch is source-supported and clean, but the protected maintainer label and open overlapping PR make the landing path a human maintainer choice rather than an automated cleanup decision.
  • Likely owner: vincentkoc — vincentkoc is the best single routing candidate because the current branch spans git-install and manifest metadata history, and he appears in recent history for the manifest fallback plus adjacent git staging work.
  • Options:
    • Land this broader PR (recommended): Merge this test-only branch after refreshed exact-head CI, then close the linked issue and supersede the narrower overlapping PR.
    • Land the narrower PR and split follow-ups: Use fix(tests): pin plugin temp-dir assertions to resolvePreferredOpenClawTmpDir() (#101876) #101884 for only the git-install assertion and open separate work for the manifest and registry test failures.
    • Pause for batching: Keep both PRs open while maintainers decide whether these plugin test fixes should be batched with broader test-helper cleanup.

Security
Cleared: The diff is test-only and does not change workflows, dependencies, lockfiles, scripts, secrets handling, package resolution, or runtime security boundaries.

Review details

Best possible solution:

Land this broader test-only fix after maintainer review and refreshed exact-head CI, then use it as the canonical fix for #101876 and supersede the narrower overlapping #101884 if maintainers prefer this branch.

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

Yes, source-reproducible: current main contains the stale os.tmpdir assertion, source-checkout one-record manifest assertion, and partial registry fixture while current production code supports the PR's expected contracts. I did not execute Vitest in this read-only review, but the PR body includes before/after terminal output.

Is this the best way to solve the issue?

Yes. Updating only the tests to match current production contracts is the narrowest maintainable solution; changing temp-root selection, disabling source-checkout scanning, or weakening the initialized-registry invariant would be the wrong layer.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9dbb1ed3d246.

Label changes

Label justifications:

  • P2: This is a normal-priority plugin test reliability fix with limited blast radius and no runtime behavior change.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied before/after terminal output for the focused tests, full plugin shard, and /tmp/openclaw safe, absent, and unsafe states.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied before/after terminal output for the focused tests, full plugin shard, and /tmp/openclaw safe, absent, and unsafe states.
Evidence reviewed

PR surface:

Tests +7. Total +7 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 3 20 13 +7
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 20 13 +7

What I checked:

  • Root policy read: Root AGENTS.md and scoped src/plugins/AGENTS.md were read fully; the plugin boundary guidance and test guidance supported reviewing the changed test contracts against current plugin runtime/discovery behavior. (src/plugins/AGENTS.md:1, 9dbb1ed3d246)
  • Current temp-root contract: Current main defaults withTempDir to resolvePreferredOpenClawTmpDir, and the resolver prefers /tmp/openclaw on POSIX when it is available or safely creatable before falling back to a user-scoped OS temp path. (src/infra/install-source-utils.ts:144, 9dbb1ed3d246)
  • Current stale git-install assertion: Current main still asserts the fallback staging parent equals fs.realpath(os.tmpdir()), which disagrees with the temp-root contract on hosts where /tmp/openclaw is preferred. (src/plugins/git-install.test.ts:571, 9dbb1ed3d246)
  • PR git-install fix: The PR head recomputes resolvePreferredOpenClawTmpDir for the fallback staging assertion and keeps the change in the test file only. (src/plugins/git-install.test.ts:575, 3042a19439c3)
  • Current manifest scan contract: Current main includes source-checkout plugin manifests in listOpenClawPluginManifestMetadata, while the existing test still expects the total record count to be exactly one. (src/plugins/manifest-metadata-scan.ts:186, 9dbb1ed3d246)
  • PR manifest test fix: The PR head filters manifest metadata to the fixture pluginDir before asserting count, preserving the fingerprint-reuse check without depending on source-checkout fanout. (src/plugins/manifest-model-id-normalization.test.ts:343, 3042a19439c3)

Likely related people:

  • Bartok9: The merged git plugin staging change added the fallback staging test and os.tmpdir-oriented assertion surface now updated by this PR. (role: introduced affected git-install test surface; confidence: high; commits: 5dff031467f1; files: src/plugins/git-install.test.ts, src/plugins/git-install.ts, src/infra/install-source-utils.ts)
  • yangxiansheng: The merged private temp-root PR routed withTempDir through resolvePreferredOpenClawTmpDir, making raw os.tmpdir expectations stale. (role: introduced adjacent temp-root behavior; confidence: high; commits: 7db7487a5d46; files: src/infra/install-source-utils.ts, src/infra/tmp-openclaw-dir.ts)
  • vincentkoc: The source-checkout manifest metadata fallback commit added the scan path that makes the one-record manifest test expectation environment-coupled in a source checkout. (role: recent manifest metadata contributor; confidence: high; commits: 26c0285812b4; files: src/plugins/manifest-metadata-scan.ts, src/plugins/manifest-model-id-normalization.test.ts)
  • RomneyDa: The initialized-registry refactor made cleanup collections required and made partial registry literals invalid in this test path. (role: introduced initialized registry collection contract; confidence: high; commits: a45c0999cec7; files: src/plugins/runtime.ts, src/plugins/registry-empty.ts, src/plugins/registry-types.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-07T22:41:51.691Z sha 3042a19 :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal backlog priority with limited blast radius. labels Jul 7, 2026
@steipete
steipete merged commit f2c6a0e into main Jul 7, 2026
146 of 152 checks passed
@steipete
steipete deleted the claude/sad-margulis-6008fc branch July 7, 2026 22:49
@steipete

steipete commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

@steipete steipete mentioned this pull request Jul 7, 2026
Closed
This was referenced Jul 7, 2026
Open
Open
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 8, 2026
@steipete
fix(tests): align plugin temp-dir and registry tests with current con…
e220496 
…tracts (openclaw#101896)
@clawsweeper clawsweeper Bot mentioned this pull request Jul 8, 2026
Closed
4 tasks
giodl73-repo pushed a commit to giodl73-repo/openclaw that referenced this pull request Jul 8, 2026
@steipete @giodl73-repo
fix(tests): align plugin temp-dir and registry tests with current con…
59c5792 
…tracts (openclaw#101896)
@vincentkoc vincentkoc mentioned this pull request Jul 9, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XS status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: plugin temp-dir tests fail on any host where /tmp/openclaw is preferable (env-coupled os.tmpdir assertions)

1 participant

@steipete