refactor(ui): dedupe chat styles and make the styles.css import order real#99900
Conversation
openclaw-barnacle
Bot
added
app: web-ui
|
Codex review: needs changes before merge. Reviewed July 4, 2026, 4:54 AM ET / 08:54 UTC. Summary PR surface: Source -406, Tests -2. Total -408 across 7 files. Reproducibility: yes. The duplicate import and the table-spacing regression are source-reproducible by comparing current Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Keep the CSS ownership cleanup, but preserve current chat markdown table spacing semantics before merge and rerun the visual/table coverage that supports the no-rendering-change claim. Do we have a high-confidence way to reproduce the issue? Yes. The duplicate import and the table-spacing regression are source-reproducible by comparing current Is this the best way to solve the issue? No. The ownership move is the right shape, but the implementation should keep table margins at the current computed behavior instead of making every chat table start with top margin. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3d404478b802. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source -406, Tests -2. Total -408 across 7 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9f97ea39a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -1,5 +1,3 @@ | |||
| @import "./chat.css"; | |||
|
|
|||
| /* =========================================== | |||
There was a problem hiding this comment.
With this import gone, components.css now loads before chat/grouped.css; however components.css:3979 still owns .chat-bubble.streaming while chat/grouped.css:412 later sets .chat-bubble.fade-in. Streaming bubbles include both classes in ui/src/ui/chat/grouped-render.ts:1877, so the later animation: fade-in ... shorthand overrides chatStreamPulse (and the reduced-motion animation: none) whenever a response is streaming. Move or reconcile the streaming rule after the fade-in rule so streaming messages keep the old pulse behavior.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
openclaw-barnacle
Bot
added
docs
openclaw-barnacle
Bot
added
agents
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6132b9dc5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const storePath = runtime.agent.session.resolveStorePath(undefined, { | ||
| agentId: context.agentId, | ||
| }); |
There was a problem hiding this comment.
When a user configures session.store, this still resolves the default store because undefined is passed here and getSessionEntry above also reads without a store path. The new fork/archive binding operations then read or write the .codex sidecar next to the default transcript, while the Codex app-server run path reads bindings from the actual active sessionFile, so custom-store sessions either fail to attach the fork or attach it somewhere the next turn will not see.
Useful? React with 👍 / 👎.
| const response = await request( | ||
| pluginConfig, | ||
| CODEX_CONTROL_METHODS.forkThread, | ||
| { threadId, threadSource: "user" }, | ||
| requestOptions(), |
There was a problem hiding this comment.
In sandboxed OpenClaw sessions, this request is rejected before it reaches Codex because extensions/codex/src/app-server/sandbox-guard.ts still classifies thread/fork as blocked-native-bypass (only name/archive/unarchive were allowed in the new test). Since codex_threads exposes fork as the safe way to continue a native thread, owner users with sandboxing enabled will see the advertised action fail every time unless the guard is updated or the tool hides/rejects the action earlier.
Useful? React with 👍 / 👎.
|
Merged via squash.
|
… real (openclaw#99900) * refactor(ui): dedupe chat styles and make styles.css import order real * refactor(ui): dedupe chat tool-row styles into chat files * refactor(ui): keep light bubble skin after role variants * refactor(ui): keep light bubble skin after role variants * refactor(ui): keep light bubble skin after role variants
1 participant
What Problem This Solves
Fixes #99899.
ui/src/styles/components.cssbegan with@import "./chat.css", and postcss-import's duplicate-skipping made that first occurrence win: everyui/src/styles/chat/*.cssfile loaded before the components.css body, while the laterchat.cssimport inui/src/styles.css(position 6) was dropped. The visible import order in styles.css was a lie, and components.css silently won all same-specificity ties against the chat stylesheets.Around 35 selector groups were defined in both files with diverging values (
.chat-textmarkdown rules,.chat-header*,.chat-controls*,.chat-session,.chat-new-messages,.chat-compose*,.chat-bubble,.chat-tool-card*,.chat-reading-indicator__dots, light-mode variants). Which copy rendered depended on the hidden hoist, and several chat-side declarations were dead but would resurrect (change rendering) on any import-graph change.Why This Change Was Made
chat/*.cssfile, carrying exactly the values that win onmaintoday (per-property cascade merge; components.css won ties)..chat-session140px, tool-shell chrome, copy/expand button background + 14px svg sizing,pulsing-border/reading-pulsekeyframes with their dead animation references, and the light bubble skin now sits below the role variants to keep its winning tie order.@import "./chat.css"line is removed, so the styles.css order is finally real..chat-compose*family is deleted (no markup emits those classes anywhere; the live composer isagent-chat__input/chat-composer-*), including its synthetic fixture entries inui/src/ui/form-controls.browser.test.ts.Note for #99763: that redesign previously relied on
pulsing-border(grouped.css) for streaming bubbles; this PR deletes that keyframes block because its only reference is dead onmain(components'chatStreamPulsewins). The redesign can reintroduce it together with its new streaming rule when it lands on top of this.User Impact
None intended: rendering is byte-for-byte identical. All 14 before/after screenshots (light+dark x desktop+mobile x top/markdown/bottom/new-messages-pill, fixed clock, animations disabled, deviceScaleFactor 2) are pixel-identical between
origin/mainand this branch: 0 differing pixels out of 8,256,000 (desktop) / 1,316,640 (mobile) per shot. Maintainers gain a truthful import order and single canonical definitions; the CSS bundle shrinks by ~380 lines.Evidence
ui/src/test-helpers/control-ui-e2e.ts) rendering a markdown-heavy conversation (headings, lists, task lists, links, inline code, hljs + plain + block-art fences, 3-level nested blockquotes, tables, hr), tool rows, header/controls, composer, reading states; Playwrightpage.clockfixed to a constant so timestamps are stable; pixel comparison via pngjs. Result: all 14 shots 0-diff vsorigin/main.node scripts/run-vitest.mjs ui/src/ui/chat- 26 files, 394 tests passed on this branch.node scripts/run-vitest.mjs run ui/src/ui/form-controls.browser.test.ts ui/src/ui/markdown.test.ts- 2 files, 93 tests passed (browser test runs real Chromium and asserts the touch-size contract for the remaining live controls).:where()-loosened matching): 0 duplicated chat groups remain; the only flags left are the intentional higher-specificity.md-preview-dialog__readeroverrides.background: var(--bg)on main viacomponents.css:2224, but components.css's own.chat-text :where(pre)at line 4171 setsvar(--secondary)later at equal specificity, so main rendersvar(--secondary)- exactly what the canonical rule now carries. The empirical 0-pixel diffs settle the remaining claims (e.g. main's copy/expand buttons demonstrably render 16px icons on--bg-elevated, proving components.css loaded last pre-diff).