Skip to content

fix(test): preserve Control UI coverage under Bun - #139799

Merged
steipete merged 2 commits into
mainfrom
codex/bun-ui-test-portability
Sep 6, 2026
Merged

fix(test): preserve Control UI coverage under Bun#139799
steipete merged 2 commits into
mainfrom
codex/bun-ui-test-portability

Conversation

@steipete

@steipete steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes an issue where developers running the Control UI tests under Bun encountered false failures from runtime-specific data URL formatting, VM APIs, and worker environments. The board timeout fixture also lacked the successful document fetch needed to reach its intended never-ready condition.

Why This Change Was Made

Validate restored attachment bytes and media metadata independently of optional MIME parameters. Execute the mock Gateway in JSDOM's existing private outside-only realm. Supply the board test's missing successful fetch and run the filesystem/Worker artifact tests in Vitest's native environment.

User Impact

No production behavior changes. Existing UI coverage now checks the same attachment, isolation, timer, storage, and worker lifecycle contracts on Node and Bun.

Evidence

  • The first three repairs pass 103 focused checks on Node and 103 on true Bun, using maintained test configurations.
  • The artifact owner passes all seven tests on each runtime after its environment annotation. A separate startup audit accounts for all five files and 27 tests on each runtime.
  • Original Bun failures were reproduced before repair. The board probe confirmed the same bounded retry sequence on both runtimes once document fetching succeeds.
  • Complete four-file changed gate passes, including types, exports, lint, and UI guards. Independent P0–P2 review is clean.
  • Every original lifecycle, isolation, and timeout assertion is retained. No production source or timeout limit changes.
  • The broader Bun compatibility campaign remains in progress.

@clawsweeper

clawsweeper Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS maintainer Maintainer-authored PR labels Sep 6, 2026
@steipete
steipete marked this pull request as ready for review September 6, 2026 05:41
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 6, 2026
@clawsweeper

clawsweeper Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 2:17 AM ET / 06:17 UTC.

ClawSweeper review

What this changes

Adjust Control UI test fixtures, attachment assertions, and script and worker environments so existing coverage works consistently under Node and Bun.

Merge readiness

✅ Ready for maintainer review

Keep open: the focused portability repairs remain useful on current main, and no introduced correctness defect was found. Maintainer-authored, protected work is not eligible for automatic closure.

Priority: P2
Reviewed head: 5391850342bea31b8a7f3db0b2133f3e2be63e79

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, maintainable test repair with reported cross-runtime validation and no actionable introduced defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The maintainer-authored PR changes test execution only, so the external-contributor proof gate does not apply; its body reports Node/Bun coverage for the changed fixtures and worker owner.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The maintainer-authored PR changes test execution only, so the external-contributor proof gate does not apply; its body reports Node/Bun coverage for the changed fixtures and worker owner.
Evidence reviewed 9 items Verified introduced scope: The local merge-base-to-head diff matches the host-provided four-file introduced patch. It changes tests and test support only; endpoint drift was not attributed to this PR.
Current main still needs the repairs: Comparison against fetched main shows all four portability changes remain absent. The supplied merged related PR concerns capture retention rather than Bun portability.
Latest release comparison: The supplied latest release, v2026.9.2, still executes mock Gateway scripts through node:vm Script and getInternalVMContext, so it does not contain this portability replacement.
Findings None None.
Security None None.

How this fits together

Control UI tests exercise browser components, queued attachment recovery, and mock Gateway behavior. Their fixtures supply isolated browser environments and filesystem workers, while assertions check recovery, timeout, and evidence-retention contracts.

flowchart TD
  A[Node or Bun test runner] --> B[Control UI test fixtures]
  B --> C[Board document and proxy readiness]
  B --> D[Attachment byte recovery]
  B --> E[Private mock Gateway realm]
  B --> F[Native filesystem workers]
  C --> G[Existing behavior assertions]
  D --> G
  E --> G
  F --> G
Loading

Before merge

None.

Agent review details

Security

None.

PR surface

Tests +14. Total +14 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 4 21 7 +14
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 21 7 +14

Review metrics

None.

Technical review

Best possible solution:

Preserve the existing behavioral assertions while using runtime-independent byte comparisons and the appropriate isolated browser and native worker environments.

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

Yes, the affected fixture paths and focused Node/Bun checks are concrete, and the author reports reproducing the original failures; this review inspected source without independently running them.

Is this the best way to solve the issue?

Yes: the patch repairs fixture prerequisites and runtime-sensitive assertions without changing production behavior, timeout limits, or the existing lifecycle checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against e19a7694cef6.

Labels

Label justifications:

  • P2: Repairs concrete developer test-portability failures without changing product runtime behavior.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The maintainer-authored PR changes test execution only, so the external-contributor proof gate does not apply; its body reports Node/Bun coverage for the changed fixtures and worker owner.

Evidence

What I checked:

  • Verified introduced scope: The local merge-base-to-head diff matches the host-provided four-file introduced patch. It changes tests and test support only; endpoint drift was not attributed to this PR. (5391850342be)
  • Current main still needs the repairs: Comparison against fetched main shows all four portability changes remain absent. The supplied merged related PR concerns capture retention rather than Bun portability. (e19a7694cef6)
  • Latest release comparison: The supplied latest release, v2026.9.2, still executes mock Gateway scripts through node:vm Script and getInternalVMContext, so it does not contain this portability replacement. (ui/src/test-helpers/mock-gateway-page.test-support.ts:44, 3928bad9badf)
  • Board timeout remains meaningful: The sandbox owner loads document bytes alongside proxy readiness and retries an unready proxy every 10 seconds. Supplying a successful document fetch isolates that scenario while retaining the three-refresh assertion and terminal error assertion. (ui/src/lib/widget-sandbox-host.ts:122, 5391850342be)
  • Attachment contract preserved: The revised helper checks one attachment, text/plain media type, base64 encoding, and exact decoded source bytes. The surrounding tests retain failed-transfer, foreign-principal, pending-owner, and retirement assertions. (ui/src/pages/chat/outbox-blob-recovery.test.ts:89, 5391850342be)
  • JSDOM dependency signal and retained isolation: The changed helper directly imports JSDOM, retains runScripts: outside-only, and invokes that window's eval. Existing consumer tests verify host globals and storage remain untouched and closing the page retires queued work and listeners. (ui/src/test-helpers/mock-gateway-page.test-support.ts:37, 5391850342be)

Likely related people:

  • Peter Steinberger: Raw commit e8ffa35 adds ui/src/test-helpers/control-ui-e2e-artifacts.node.test.ts:20 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: e8ffa3557d72; files: ui/src/test-helpers/control-ui-e2e-artifacts.node.test.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-09-06T05:46:56.834Z sha 2198ec2b7ab462c34b27ed4dc601230ffd325511 :: needs maintainer review before merge. :: none

@steipete
steipete force-pushed the codex/bun-ui-test-portability branch from 2198ec2 to 5391850 Compare September 6, 2026 06:11
@steipete
steipete merged commit fe33b72 into main Sep 6, 2026
225 of 227 checks passed
@steipete
steipete deleted the codex/bun-ui-test-portability branch September 6, 2026 08:39
@steipete

steipete commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Sep 7, 2026
@steipete
fix(test): preserve Control UI coverage under Bun (openclaw#139799)
030e8c6 
* test(ui): preserve runtime-independent browser fixture contracts

* fix(test): run artifact workers in the native environment
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

app: web-ui App: web-ui maintainer Maintainer-authored PR P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary 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.

1 participant

@steipete