Skip to content

fix(embed): match custom host colors before first paint - #134

Merged
steipete merged 2 commits into
mainfrom
codex/openclaw-initial-embed-palette
Jul 28, 2026
Merged

fix(embed): match custom host colors before first paint#134
steipete merged 2 commits into
mainfrom
codex/openclaw-initial-embed-palette

Conversation

@steipete

@steipete steipete commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Related: openclaw/openclaw#114795

Follow-up to #133

What Problem This Solves

A ClickClack sidebar could briefly render default colors before receiving the first custom host palette, even when the embedding application already knew the exact custom colors.

Why This Change Was Made

Allow the existing authenticated embed contract to carry a bounded JSON themeTokens snapshot alongside theme and hostOrigin. Validate every initial CSS color and each derived corner radius before applying the palette in the pre-paint document script. Ignore malformed snapshots while retaining the valid light or dark mode. Keep live exact-origin parent messaging and standalone account appearance unchanged.

User Impact

Custom host backgrounds, panels, text, borders, accents, and corner radii match before the discussion first paints; malformed input cannot override or break account appearance.

Evidence

  • Remote pnpm fmt:ts:check, pnpm lint, pnpm typecheck, pnpm --filter @clickclack/web typecheck, and pnpm build: passed.
  • node --test apps/web/src/lib/embed-theme.test.ts apps/web/src/lib/realtime-bootstrap.test.ts apps/web/src/lib/realtime-queue.test.ts: 16 passed.
  • CLICKCLACK_CAPTURE_UI_PROOF=1 pnpm exec playwright test tests/e2e/embed-theme.spec.ts --reporter=line: 1 passed (28.9s), against the actual Go application, real second HTTP origin, and production frame-ancestor policy.
  • The browser scenario verifies initial light tokens, custom first-paint colors, live dark and purple palette switching, spoof rejection, unchanged account preferences, host-theme revocation on navigation, malformed JSON fallback, and invalid scalable radius rejection.
  • Captured and visually inspected full-page light, dark, custom, and custom-first-paint screenshots.
  • Exact production assets were generated from the passing browser build and checked in.
  • Fresh independent first-paint security review: clean, zero accepted or actionable findings.

AI-assisted implementation and review.

@clawsweeper clawsweeper Bot added 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. P2 Normal priority bug or improvement with limited blast radius. labels Jul 28, 2026
@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 27, 2026, 8:41 PM ET / July 28, 2026, 00:41 UTC.

ClawSweeper review

What this changes

The PR adds a bounded, validated themeTokens snapshot to embedded discussion URLs so custom host colors and corner radius are applied before the ClickClack iframe first paints.

Merge readiness

Blocked until stronger real behavior proof is added - 4 items remain

This PR addresses a remaining first-paint gap in the host-theme work merged through #133: current main receives live custom palettes but the new URL snapshot is needed to render custom colors before the initial frame. The source, generated web assets, documentation, and browser scenario are aligned; no concrete patch defect was found. The contributor should attach inspectable, redacted real-browser proof before merge because the PR body describes the run but does not include an artifact reviewers can verify.

Priority: P2
Reviewed head: 142d4c4e7673f3e8441c93a61a710ff8d53380f6

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The patch is focused and has relevant automated coverage, but merge confidence is capped until the claimed real-browser first-paint proof is independently inspectable.
Proof confidence 🦪 silver shellfish (2/6) Needs stronger real behavior proof before merge: The PR body describes a passing real cross-origin Playwright run and captured screenshots, but it does not provide an inspectable screenshot, recording, redacted terminal output, log, or linked artifact showing the after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs stronger real behavior proof before merge: The PR body describes a passing real cross-origin Playwright run and captured screenshots, but it does not provide an inspectable screenshot, recording, redacted terminal output, log, or linked artifact showing the after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 5 items First-paint implementation: The branch extends the embed-only pre-paint path to parse a bounded themeTokens URL parameter, retain a valid light or dark mode when parsing fails, and set only validated color and radius CSS variables.
Runtime asset parity: The same pre-paint script change is included in both Go-served production HTML assets, avoiding a source-versus-embedded-asset mismatch.
Focused end-to-end coverage: The browser test adds a custom-token URL snapshot, checks CSS variables before subsequent live updates, and exercises malformed-token and invalid-radius fallback behavior.
Findings None None.
Security None None.

How this fits together

ClickClack embeds accept initial appearance inputs through iframe URL parameters and then synchronize live palette changes through exact-origin parent messages. The pre-paint document script validates those inputs and sets CSS variables before the embedded discussion application initializes.

flowchart LR
  Host[Embedding host]
  URL[Embed URL parameters]
  Script[Pre-paint theme script]
  Validate[Validate mode and tokens]
  CSS[Document CSS variables]
  UI[Embedded discussion UI]
  Live[Exact-origin live updates]
  Host --> URL --> Script --> Validate --> CSS --> UI
  Host --> Live --> UI
Loading

Before merge

  • Add real behavior proof - Needs stronger real behavior proof before merge: The PR body describes a passing real cross-origin Playwright run and captured screenshots, but it does not provide an inspectable screenshot, recording, redacted terminal output, log, or linked artifact showing the after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - The URL snapshot extends the existing embed contract; reviewers should verify the linked embedding-host work supplies the documented encoded token object without changing behavior for hosts that omit it.
  • Resolve merge risk (P1) - The PR body reports successful real cross-origin browser proof and screenshots, but no inspectable redacted artifact or live output is attached for independent review.
  • Complete next step (P2) - No code repair is indicated; the remaining merge gate is contributor-supplied, inspectable real-browser proof for the claimed first-paint behavior.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Changed surface 5 files: 244 added, 3 removed The implementation is focused on one pre-paint source script, two synchronized production assets, embed documentation, and one browser scenario.
Production asset copies 2 generated HTML assets updated Both Go-served HTML copies must remain byte-equivalent in behavior to the web source for embeds to receive the fix in production.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Merge the bounded validation path after attaching a redacted browser recording or screenshots plus terminal/live output that visibly demonstrates the custom first-paint palette and malformed-token fallback in the real cross-origin embed.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Merge the bounded validation path after attaching a redacted browser recording or screenshots plus terminal/live output that visibly demonstrates the custom first-paint palette and malformed-token fallback in the real cross-origin embed.

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

Yes from source, at medium confidence: an embed URL can select a mode before paint but, before this patch, cannot carry the custom token palette used by the later parent message. The PR describes a real cross-origin browser reproduction, but its runtime artifacts are not attached for independent verification.

Is this the best way to solve the issue?

Yes, with the stated validation boundary: applying an initial snapshot in the existing pre-paint URL path is narrower than changing account preferences or weakening the exact-origin live-message path. The two production HTML assets should continue to be generated and reviewed alongside the source template.

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P2: This is a bounded visual correctness improvement for embedded discussions, with no evidence of data loss, security bypass, or core availability impact.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body describes a passing real cross-origin Playwright run and captured screenshots, but it does not provide an inspectable screenshot, recording, redacted terminal output, log, or linked artifact showing the after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a bounded visual correctness improvement for embedded discussions, with no evidence of data loss, security bypass, or core availability impact.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body describes a passing real cross-origin Playwright run and captured screenshots, but it does not provide an inspectable screenshot, recording, redacted terminal output, log, or linked artifact showing the after-fix result. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

Likely related people:

  • steipete: Authored the merged predecessor that established ClickClack’s URL-based first-paint theme and exact-origin live theme synchronization; this PR is an explicit follow-up on that implementation. (role: introduced the current embed-theme behavior; confidence: high; commits: d85eb7cae837, ebd327ac7333; files: apps/web/src/app.html, tests/e2e/embed-theme.spec.ts, docs/features/embedding.md)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach a redacted screenshot or short recording that visibly shows the custom host palette on the iframe’s first rendered frame.
  • Attach redacted terminal or live output for the cross-origin Playwright run, including the first-paint and malformed-token cases.

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.

@steipete

Copy link
Copy Markdown
Contributor Author

Inspectable real two-origin browser proof

The checked-in E2E starts the real production-built ClickClack Go server on http://127.0.0.1:18082 and a separate real HTTP parent on http://127.0.0.1:18083; no frame-policy bypass or mocked ClickClack receiver is used.

$ pnpm fmt:ts:check
All matched files use the correct format.

$ pnpm lint
Found 0 warnings and 0 errors.

$ pnpm typecheck
passed

$ pnpm --filter @clickclack/web typecheck
passed

$ pnpm build
vite production build passed; Go-embedded HTML regenerated

$ CLICKCLACK_CAPTURE_UI_PROOF=1 pnpm exec playwright test tests/e2e/embed-theme.spec.ts --reporter=line
Running 1 test using 1 worker
[chromium] embedded channels follow the exact cross-origin host theme without changing account appearance
1 passed (28.9s)

The actual browser test asserts:

initial light: background #faf9f7, panel #ffffff, accent #bd4531
live dark:     background #0e1015, panel #161920, accent #ff5c5c
live custom:   background #171229, panel #211a36, accent #c084fc
first paint:   dark, background #171229, panel #211a36, accent #c084fc
spoofed self-frame message: ignored
standalone localStorage and account appearance writes: unchanged
embed-to-app navigation: host-owned custom properties removed
malformed themeTokens JSON: valid dark mode retained
unscalable radius 8px 16px: rejected

Full-page light, dark, custom, and custom-first-paint screenshots were captured during this exact successful run. The production Go asset files committed in this PR are from the same passing build.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@steipete
steipete merged commit 4843afe into main Jul 28, 2026
15 of 16 checks passed
@steipete
steipete deleted the codex/openclaw-initial-embed-palette branch July 28, 2026 00:58
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. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant