Skip to content

fix: keep redhat theme nodes across a control UI reload - #2

Merged
steipete merged 2 commits into
openclaw:mainfrom
SebTardif:fix/tidepool-f001
Oct 7, 2026
Merged

steipete merged 2 commits into
openclaw:mainfrom
SebTardif:fix/tidepool-f001

Conversation

@SebTardif

@SebTardif SebTardif commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Reloading the Red Hat Control UI plugin activates its replacement before disposing the previous activation. The previous cleanup looked up nodes by ID and removed the replacement font link and artwork style.

Each activation now installs and owns its own DOM nodes, and disposal removes only those objects. The plugin remains self-contained. Maintainer follow-up simplifies the DOM adapter, documents reload behavior, and runs the new regression tests in CI.

Validation: reproduced the baseline failure against the real activation source with a synthetic document (both nodes disappeared after old disposal); both ownership regression tests pass. A clean Linux environment passed pnpm install --frozen-lockfile && pnpm test && pnpm build. Independent Codex review found no actionable P0–P2 findings. CI: https://github.com/openclaw/tidepool/actions/runs/37597362260

Thanks @SebTardif for the fix and original live reload investigation. Theme selection and full gateway appearance rendering were not revalidated in this maintenance pass.

The disposer removed font and artwork nodes by id. The host activates
the replacement before disposing the previous owner, so the new
activation reused those nodes and the old disposer deleted them.
Install fresh nodes and remove only the ones this activation created.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

🦞👀
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.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Oct 4, 2026
@clawsweeper

clawsweeper Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed October 6, 2026, 7:28 PM ET / 23:28 UTC (Revision 7).

ClawSweeper review

What this changes

The Red Hat theme creates fresh font and artwork nodes for each Control UI activation and removes only those nodes during cleanup, with regression coverage for overlapping activations.

Merge readiness

✅ Ready for maintainer review

This remains a useful, focused fix absent from main. The supplied live reload trace demonstrates the changed node lifecycle, and no blocking patch defect was found.

Priority: P2
Reviewed head: 9c0ac1b30a925afdf19a514c0922e1398269c989

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused lifecycle repair with directly relevant real-browser output and regression coverage.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The pinned-head Chrome trace exercises Red Hat activation through a real gateway revision reload and shows old objects disconnected while replacement font and artwork nodes survive, including a loaded font stylesheet. Theme selection was untested but is outside the changed lifecycle. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The pinned-head Chrome trace exercises Red Hat activation through a real gateway revision reload and shows old objects disconnected while replacement font and artwork nodes survive, including a loaded font stylesheet. Theme selection was untested but is outside the changed lifecycle. No stored-data contract changes.
Evidence reviewed 7 items Pinned introduced change: The exact base-to-head delta changes only the Red Hat activation, adds its node-ownership helper, and adds two regression tests. Cleanup retains object references rather than resolving replacement nodes by id.
Main still needs this fix: Pinned main reuses existing nodes and removes them by id, preserving the reported overlap failure. The live GitHub read confirms this PR remains open and unmerged at the pinned head; the closed-PR listing contains only the earlier Lobster feature.
Dependency contract signal: The production entry imports OpenClaw's Control UI SDK, and the PR explicitly relies on replacement activation preceding previous-owner disposal. This establishes OpenClaw as the relevant lifecycle dependency.
Findings None None.
Security None None.

How this fits together

Tidepool supplies optional OpenClaw themes. The Red Hat browser plugin adds font and artwork resources when the Control UI activates it, then cleans them up when that activation ends.

flowchart TD
  A[Plugin revision reload] --> B[Red Hat activation]
  B --> C[Create font and artwork nodes]
  C --> D[Replace previous nodes]
  D --> E[Control UI document]
  A --> F[Previous activation cleanup]
  F --> G[Remove owned nodes only]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test LOC production +71/-19, tests +123/-0 Production growth isolates activation-owned cleanup behind a small testable helper, with overlap and final-disposal coverage.

Technical review

Best possible solution:

Keep activation-owned DOM cleanup so a successful plugin revision reload retains its replacement font and artwork resources.

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

Yes, source establishes the failure: main reuses nodes, the host activates the replacement first, and previous cleanup removes the shared ids. No reviewer-side runtime reproduction was executed.

Is this the best way to solve the issue?

Yes, retaining each activation's node references is a narrow repair that preserves the existing SDK, resource URLs, configuration, and theme definitions.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This repairs optional theme resources disappearing during plugin reload, with limited blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The pinned-head Chrome trace exercises Red Hat activation through a real gateway revision reload and shows old objects disconnected while replacement font and artwork nodes survive, including a loaded font stylesheet. Theme selection was untested but is outside the changed lifecycle. No stored-data contract changes.
  • proof: sufficient: Contributor real behavior proof is sufficient. The pinned-head Chrome trace exercises Red Hat activation through a real gateway revision reload and shows old objects disconnected while replacement font and artwork nodes survive, including a loaded font stylesheet. Theme selection was untested but is outside the changed lifecycle. No stored-data contract changes.

Evidence

What I checked:

  • Pinned introduced change: The exact base-to-head delta changes only the Red Hat activation, adds its node-ownership helper, and adds two regression tests. Cleanup retains object references rather than resolving replacement nodes by id. (plugins/redhat-theme/src/control-ui-nodes.ts:20, 9c0ac1b30a92)
  • Main still needs this fix: Pinned main reuses existing nodes and removes them by id, preserving the reported overlap failure. The live GitHub read confirms this PR remains open and unmerged at the pinned head; the closed-PR listing contains only the earlier Lobster feature. (plugins/redhat-theme/src/control-ui.ts:34, 523c316407ff)
  • Dependency contract signal: The production entry imports OpenClaw's Control UI SDK, and the PR explicitly relies on replacement activation preceding previous-owner disposal. This establishes OpenClaw as the relevant lifecycle dependency. (plugins/redhat-theme/src/control-ui.ts:1, 9c0ac1b30a92)
  • Host lifecycle confirmed: The current OpenClaw runtime finishes replacement initialization, publishes the replacement owner, and then disposes the previous owner. This independently supports the overlap scenario addressed by the patch. (ui/src/plugins/control-ui-runtime.ts:393, 45228e1aeed6)
  • After-fix real browser proof: The captured PR body, source identity 3e6b2cdb5c7be932525a3e9ab448ea0b04485d64099eee6b8e28be944b5c37b1, records macOS Chrome with OpenClaw 2026.9.5 at the pinned patch head. A gateway plugin revision reload while the page stayed open left new font and artwork objects connected, disconnected the marked old objects, exposed a loaded font stylesheet, and retained the artwork rule. Theme selection and painted artwork were explicitly untested; those are outside this node-retention fix. (9c0ac1b30a92)
  • Feature-history routing: File history identifies the native-polish feature commit. Its raw commit records Peter Steinberger as author; GitHub maps that author to steipete and supplies the added activation patch. The recorded parent's file contains only the earlier stylesheet import. Local blame and detailed historical show could not complete because a historical blob fetch failed; API inspection supplied the relevant historical patch and parent content. (plugins/redhat-theme/src/control-ui.ts:16, b52e56072ee6)

Likely related people:

  • unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

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 (6 earlier review cycles)
  • reviewed 2026-10-04T02:51:51.987Z sha 9c0ac1b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-04T06:20:45.127Z sha 9c0ac1b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-04T22:59:12.466Z sha 9c0ac1b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-06T01:44:07.478Z sha 9c0ac1b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-06T18:35:57.264Z sha 9c0ac1b :: needs real behavior proof before merge. :: none
  • reviewed 2026-10-06T20:52:46.302Z sha 9c0ac1b :: needs maintainer review before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. 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. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Oct 6, 2026
@steipete
steipete merged commit 2f843aa into openclaw:main Oct 7, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants