test(ui): fix Node 25+ localStorage crashes in jsdom suites; document PR-open CI drops by steipete · Pull Request #110812 · openclaw/openclaw

steipete · 2026-07-18T16:34:20Z

What Problem This Solves

Developers on current Node (25/26) get 60+ spurious UI test failures locally: any jsdom test touching localStorage crashes with Cannot read properties of undefined (reading 'clear') while the same tests pass on Linux CI (Node 24). Six files fail on Node 26.5 (lobster-pet, lobster-dex, dock-panel-layout, activity-page, chat-realtime, chat-pull-requests), which makes real local red herrings out of green code and masks genuine failures.

Separately, three consecutive fresh PRs this week (#110472, #110528, #110623) had their pull_request-open CI run drop or die as startup_failure, each needing a close/reopen; repo history shows ~10–16 such runs daily.

Why This Change Was Made

Node 25+ enables WebStorage by default: globalThis.localStorage becomes an own-property getter that is dead without --localstorage-file (measured undefined on Node 26.5; other releases reportedly throw or return an inert proxy). During vitest jsdom global population the dead getter shadows jsdom's Storage — probed in-environment: globalThis === window and both localStorage bindings are undefined. Node 24 has no default WebStorage, so CI never sees it.

The shared UI test setup (lit-warnings.setup.ts, already the home for jsdom gap shims) now capability-probes the global with a throw-safe set/get/remove round-trip, prefers jsdom's own window.localStorage when only the global alias is dead, and otherwise installs a single in-memory Storage on both window and globalThis so the two names stay one object.

The CI-drop pattern was diagnosed to a GitHub-side race: PRs created immediately after their branch push fire the open event before the merge ref is computed (mergeable: null), and the CI run either never attaches or is created as startup_failure / BuildFailed / (Unknown event) — which gh run rerun refuses to retry. The workflow triggers in ci.yml are correct; the fix is operational, so AGENTS.md now instructs verifying the CI attach after PR creation and re-firing via close/reopen.

User Impact

Contributor/local-dev only: UI tests pass on Node 24, 25, and 26 without flags; no runtime, product, or CI behavior changes. Agents stop losing PR cycles to silently missing CI runs.

Evidence

Root cause probed in the failing environment: descriptor on Node 26.5 is a configurable getter returning undefined; in vitest jsdom, globalThis === window with both storage bindings dead.
Before: node scripts/run-vitest.mjs ui/src/components/lobster-pet.test.ts — 44/44 failing on Node 26.5 (reproduced on clean main). After: all six previously failing files pass; full ui/src suite exit 0 on Node 26.5.
CI-drop diagnosis: gh api on run 29634070586 shows path: BuildFailed, display_title: "(Unknown event)", empty referenced workflows; 100 most recent startup_failure runs span five days at ~10–16/day, all pull_request events across many authors' branches.
Structured autoreview (gpt-5.6-sol): three hardening findings accepted and fixed (throw-safe probe, round-trip verification against inert implementations, window/global identity binding); final run clean.

Node 25+ ships a default-on global localStorage that is dead without --localstorage-file (undefined on 26.5; throwing or inert elsewhere). It shadows jsdom's Storage during vitest global population, so storage-touching UI tests crash on newer local Node while Linux CI (Node 24) passes — six files failed locally on Node 26.5. The shared setup now capability-probes (round-trip, throw-safe), prefers jsdom's own window Storage when only the global alias is dead, and otherwise installs one in-memory Storage on both window and globalThis. Also documents the PR-open CI drop pattern in AGENTS.md: fresh PRs race GitHub's merge-ref computation and the open-event CI run can drop or die as startup_failure/BuildFailed (not rerunnable); verify attach and close/reopen to re-fire.

clawsweeper · 2026-07-18T16:41:14Z

Codex review: found issues before merge. Reviewed July 18, 2026, 12:55 PM ET / 16:55 UTC.

Summary
Adds a capability-probed localStorage shim to the shared UI jsdom setup for newer Node versions and documents a recovery step for pull-request-open CI startup failures.

PR surface: Source +67, Docs +1. Total +68 across 2 files.

Reproducibility: yes. The PR supplies a concrete current-main Node 26.5 path: affected jsdom tests fail when localStorage is unusable, then pass after the shared setup shim is applied.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: ui/src/test-helpers/lit-warnings.setup.ts. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

Revise the CI recovery guidance to identify valid pull-request runs through merge-ref or PR association before recommending close/reopen.

Risk before merge

[P1] The head-SHA-only CI-attachment instruction can cause agents to close and reopen a PR even when its valid pull_request workflow is associated with the merge ref, creating unnecessary duplicate CI activity and confusing recovery guidance.

Maintainer options:

Make CI identity verification merge-aware (recommended)
Update the instruction to check the PR-associated or merge-ref workflow run before using close/reopen as recovery.
Pause the operational guidance
Land only the local jsdom test shim and defer the CI recovery procedure until its GitHub Actions identity contract is confirmed.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Revise the PR-open CI recovery text so it validates the pull-request workflow by merge ref or PR association as well as any head-SHA signal; retain close/reopen only for genuinely absent or non-rerunnable runs.

Next step before merge

[P2] The remaining blocker is a repository-wide CI lifecycle policy choice under the protected maintainer label, so it needs explicit human review rather than an automated repair lane.

Maintainer decision needed

Question: Should the repository-wide CI recovery procedure require merge-ref or PR-associated workflow verification before a close/reopen, rather than treating the head SHA as the sole attachment identity?
Rationale: This is an operational policy instruction used beyond this PR; maintainers should confirm the intended GitHub Actions identity check before publishing a recovery procedure that can restart contributor CI.
Likely owner: steipete — They authored the proposed repository workflow instruction and have recent related UI/CI contribution context.
Options:
- Correct the recovery check before merge (recommended): Require verification of the pull-request workflow’s merge-ref or PR association, then retain close/reopen only when no valid run exists.
- Keep head-only verification: Accept that valid merge-ref-associated workflows may be treated as absent and that agents may unnecessarily re-fire CI.

Security
Cleared: The patch adds test-environment compatibility code and repository guidance without new dependencies, credential handling, permissions, or executable supply-chain paths.

Review findings

[P2] Verify pull-request CI through its merge-ref identity — AGENTS.md:235

Review details

Best possible solution:

Keep the capability-based UI test shim, but revise the CI recovery guidance to verify the applicable pull-request workflow/run against the merge commit or its PR association before recommending a close/reopen.

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

Yes. The PR supplies a concrete current-main Node 26.5 path: affected jsdom tests fail when localStorage is unusable, then pass after the shared setup shim is applied.

Is this the best way to solve the issue?

No. The capability-based storage shim is an appropriate narrow fix, but the accompanying CI recovery instruction should not use the head SHA as the only proof that a pull-request workflow attached.

Full review comments:

[P2] Verify pull-request CI through its merge-ref identity — AGENTS.md:235
The recovery text treats a workflow missing from the head SHA as missing altogether, but pull_request workflows may run on GitHub’s synthetic merge commit. Check the PR-associated or merge-ref run before asking agents to close and reopen, otherwise healthy CI can be restarted unnecessarily. This remains from the prior review cycle; the current follow-up did not change this instruction.
Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 837db49ebb24.

Label changes

Label changes:

add P2: The PR introduces repository-wide CI recovery guidance whose current head-only check can create incorrect operational actions across contributor PRs.
add merge-risk: 🚨 automation: The changed guidance governs CI-run detection and recovery and can spuriously re-fire healthy pull-request workflows.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides specific before/after Node 26.5 terminal results for the affected UI suites and a successful full ui/src run; redact any local details if that evidence is expanded in follow-up.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body provides specific before/after Node 26.5 terminal results for the affected UI suites and a successful full ui/src run; redact any local details if that evidence is expanded in follow-up.

Label justifications:

P2: The PR introduces repository-wide CI recovery guidance whose current head-only check can create incorrect operational actions across contributor PRs.
merge-risk: 🚨 automation: The changed guidance governs CI-run detection and recovery and can spuriously re-fire healthy pull-request workflows.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body provides specific before/after Node 26.5 terminal results for the affected UI suites and a successful full ui/src run; redact any local details if that evidence is expanded in follow-up.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides specific before/after Node 26.5 terminal results for the affected UI suites and a successful full ui/src run; redact any local details if that evidence is expanded in follow-up.

Evidence reviewed

PR surface:

Source +67, Docs +1. Total +68 across 2 files.

View PR surface stats

Area	Files	Added	Net
Source	1	67	+67
Tests	0	0	0
Docs	1	1	+1
Config	0	0	0
Generated	0	0	0
Other	0	0	0
Total	2	68	+68

What I checked:

Shared jsdom setup change: The branch adds a throw-safe round-trip probe and, when the global alias is unusable, aligns window.localStorage and globalThis.localStorage on a usable jsdom or in-memory storage implementation. (ui/src/test-helpers/lit-warnings.setup.ts:80, bd57a9693bc6)
Unresolved CI instruction defect: The added instruction says to verify that CI attached to the head SHA before closing and reopening the PR. Pull-request workflows can instead validate and report against GitHub’s synthetic merge commit, so head-only checking can incorrectly trigger the recovery procedure. (AGENTS.md:235, 5900c1cfdd7b)
Review continuity: The previous ClawSweeper cycle already raised “Check PR CI against the test-merge commit too”; the current follow-up commit only removes unnecessary String() conversions in the storage shim, so that blocker remains applicable. (AGENTS.md:235, bd57a9693bc6)
Real behavior evidence: The PR body reports a clean-main Node 26.5 failure in the affected UI suites and successful after-fix runs for all six affected files plus the full ui/src suite. This is concrete terminal-style after-fix evidence for the test-only behavior. (ui/src/test-helpers/lit-warnings.setup.ts:80, bd57a9693bc6)
Feature-history signal: The supplied related history identifies steipete as author of recent merged Control UI changes, including merged PRs 110472, 110528, and 110623; that makes them the strongest available routing candidate for this shared UI-test surface. (ui/src/test-helpers/lit-warnings.setup.ts:80, cad4e395d21f)

Likely related people:

steipete: Authored this shared UI-test setup change and the supplied context identifies multiple recent merged Control UI changes by the same contributor. (role: recent area contributor; confidence: medium; commits: 5900c1cfdd7b, bd57a9693bc6, cad4e395d21f; files: ui/src/test-helpers/lit-warnings.setup.ts, AGENTS.md)

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-18T16:38:13.093Z sha 5900c1c :: found issues before merge. :: [P2] Check PR CI against the test-merge commit too

steipete · 2026-07-18T17:19:48Z

Merged via squash.

Prepared head SHA: bd57a9693bc62b99c43f6b043ba57346bfd95acc
Landed commit: a54a292d62cce68421de4456303e2b9178393c94

… PR-open CI drops (openclaw#110812) * test(ui): shim dead Node 25+ WebStorage localStorage in jsdom test setup Node 25+ ships a default-on global localStorage that is dead without --localstorage-file (undefined on 26.5; throwing or inert elsewhere). It shadows jsdom's Storage during vitest global population, so storage-touching UI tests crash on newer local Node while Linux CI (Node 24) passes — six files failed locally on Node 26.5. The shared setup now capability-probes (round-trip, throw-safe), prefers jsdom's own window Storage when only the global alias is dead, and otherwise installs one in-memory Storage on both window and globalThis. Also documents the PR-open CI drop pattern in AGENTS.md: fresh PRs race GitHub's merge-ref computation and the open-event CI run can drop or die as startup_failure/BuildFailed (not rerunnable); verify attach and close/reopen to re-fire. * fix(test): drop unnecessary String() conversions in localStorage shim

openclaw-barnacle Bot added app: web-ui App: web-ui size: S maintainer Maintainer-authored PR labels Jul 18, 2026

steipete closed this Jul 18, 2026

steipete reopened this Jul 18, 2026

fix(test): drop unnecessary String() conversions in localStorage shim

bd57a96

steipete merged commit a54a292 into main Jul 18, 2026
131 of 138 checks passed

steipete deleted the claude/node26-localstorage-tests branch July 18, 2026 17:19

steipete mentioned this pull request Jul 18, 2026

ci: hourly sweeper re-fires dropped pull_request CI runs #110889

Merged

github-actions Bot mentioned this pull request Jul 18, 2026

📡 Upstream Digest — 2026-07-18 18:51 UTC curtismercier/openclaw-mods#1337

Open

steipete mentioned this pull request Jul 18, 2026

docs(agents): draft-then-ready PR creation prevents the merge-ref CI race #110970

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

test(ui): fix Node 25+ localStorage crashes in jsdom suites; document PR-open CI drops#110812

test(ui): fix Node 25+ localStorage crashes in jsdom suites; document PR-open CI drops#110812
steipete merged 2 commits into
mainfrom
claude/node26-localstorage-tests

steipete commented Jul 18, 2026

Uh oh!

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

steipete commented Jul 18, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

steipete commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented Jul 18, 2026 •

edited

Loading