Repository navigation
fix: keep lobster theme nodes across a control UI reload - #3
Conversation
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>
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 5:07 AM ET / 09:07 UTC (Revision 8). ClawSweeper reviewWhat this changesThe Lobster theme gives each activation its own font and artwork elements, adds cleanup regression tests, and documents reload behavior. Merge readiness✅ Ready for maintainer review This PR remains necessary: current main still removes Lobster’s replacement styling during reload. The focused repair preserves the previously demonstrated behavior, with no blocking findings. Priority: P2 Review scores
Verification
How this fits togetherThe Lobster plugin adds fonts and background artwork to OpenClaw’s Control UI when custom plugin UI is enabled. Reloading activates a replacement before disposing the previous activation. flowchart LR
A[Plugin reload] --> B[Lobster activation]
B --> C[Create font and artwork elements]
C --> D[Replace previous elements]
D --> E[Dispose previous activation]
E --> F[Replacement styling remains]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep activation-owned cleanup inside the Lobster plugin so replacement fonts and artwork survive reloads. Do we have a high-confidence way to reproduce the issue? Yes, source establishes the path: activate Lobster twice, then dispose the first activation; main’s ID-based cleanup removes the reused elements. This review did not execute target code. Is this the best way to solve the issue? Yes, exact element ownership is a narrow repair consistent with the host lifecycle and the repository’s self-contained plugin design. AGENTS.md: not found in the target repository. Codex review notes: model internal, reasoning medium; reviewed against 2f843aab51eb. LabelsLabel changes: No label changes. Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
Reloading the Lobster 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 incorporates the regression-test CI wiring landed in #2.
Validation: reproduced the baseline failure against the real activation source with a synthetic document (both nodes disappeared after old disposal); all four 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/37597849171Thanks @SebTardif for the fix and original live reload investigation. Theme selection and full gateway appearance rendering were not revalidated in this maintenance pass.