Skip to content

base red: webviews design tokens (What's New brief) and the transcript spacing test's after() helper - #34577

Merged
lawrencecchen merged 6 commits into
feat-cmux-nextfrom
base-red/wn-pop-more-token
Nov 4, 2026
Merged

lawrencecchen merged 6 commits into
feat-cmux-nextfrom
base-red/wn-pop-more-token

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Two web-lint base reds on feat-cmux-next, one commit each:

  1. bun run check:design-tokens: new design token violation (1 > 0): pages/changelog/updateSurfaces.css spacing margin-top 2px, from Update card: the What's New popover fits its lines; the Ready card is tighter #34477 (b7c9441). .wn-pop-brief .wn-pop-more now uses var(--ds-space-1) (4px), the gap .wn-pop-all already uses above it. The look moves by 2px.
  2. webviews typecheck: test/transcript-spacing-kinds.test.ts:247: Cannot find name 'after'. Agent chat: message rows on one line, names whole, in the text tone #34464 (1e8a45b) removed the below()/after() helpers; Agent pane: the live Thinking line starts on the icon column and stays the last row #34370 (8557b10), written on a base that still had them, added a new after(...) call. The two merged one after the other. This restores both helpers exactly as Agent pane: the live Thinking line starts on the icon column and stays the last row #34370 measured with them. No other test change.

Changelog

none

Observed / Not run


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.


Summary by cubic

Fixes the base reds on feat-cmux-next so the branch's web lint (format, type, and lint gates) passes again.

  • .wn-pop-more now uses var(--ds-space-1) (4px) instead of a hardcoded 2px, matching the gap .wn-pop-all uses above it — the look moves by 2px.
  • Restores the below() and after() helpers in the transcript spacing test that were removed after the test added a new after() call.
  • Builds the cloudRun test call label with String(params.cwd) to satisfy the restrict-template-expressions lint.
  • Reformats App.tsx, emojiShortcodes.test.ts, and composer-emoji.browser.test.ts to match vp fmt --check; no behavior change.
  • Updates the cloudRun source-check regex to allow whitespace between the granting(...) arguments, which the format pass split onto multiple lines.

Written for commit 35aff4b. Summary will update on new commits.

View guided diff Turn on auto-fix

…(design tokens)

check:design-tokens is red on feat since b7c9441 (#34477): the brief
popover's .wn-pop-more used margin-top 2px, off the token scale. It now
uses --ds-space-1 (4px), the same gap .wn-pop-all uses above it.
…echeck)

webviews typecheck is red on feat: test/transcript-spacing-kinds.test.ts:247
calls after(), which #34464 (1e8a45b) removed together with below()
while #34370 (8557b10, written on a base that still had them) added a
new use. Restore both helpers as #34370 measured with them; no other change.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 35aff4b: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr34577 -f ref=base-red/wn-pop-more-token -f platform=macos -f backend_mode=local

Gallery before/after for 35aff4b: 16 states rendered, 0 images differ, 1495 more touched states not rendered. Images are in the run's artifact, not on this public PR; the independent review of this head reads them on its own, so the PR needs no hand-made screenshots of these states.

gh run download 41766787142 -R manaflow-ai/cmux -n gallery-advisory-34577-35aff4b09737e39761277344499ff8285e1dad57 (then read gallery-review.json)

States
State Theme Status Diff Picked by
agent-pane.chat-width/standard Apple System Colors Light unchanged 0% own App.tsx
agent-pane.chat-width/standard Apple System Colors unchanged 0% own App.tsx
agent-pane.chat-width/wide Apple System Colors Light unchanged 0% own App.tsx
agent-pane.chat-width/wide Apple System Colors unchanged 0% own App.tsx
agent-pane.chat-width/wide-blocks Apple System Colors Light unchanged 0% own App.tsx
agent-pane.chat-width/wide-blocks Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-edge/short-edge Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-edge/short-edge Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-edge/wide-edge Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-edge/wide-edge Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-edge/no-edge Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-edge/no-edge Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-floats/scrolled-to-end Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-floats/scrolled-to-end Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-floats/composer-grown Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-floats/composer-grown Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-40 Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-40 Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-56 Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-56 Apple System Colors unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-72 Apple System Colors Light unchanged 0% own App.tsx
agent-pane.composer-rest-gap/gap-72 Apple System Colors unchanged 0% own App.tsx
agent-pane.continuation-spacing/shipped Apple System Colors Light unchanged 0% own App.tsx
agent-pane.continuation-spacing/shipped Apple System Colors unchanged 0% own App.tsx
agent-pane.continuation-spacing/two-thirds Apple System Colors Light unchanged 0% own App.tsx
agent-pane.continuation-spacing/two-thirds Apple System Colors unchanged 0% own App.tsx
agent-pane.continuation-spacing/half-tucked Apple System Colors Light unchanged 0% own App.tsx
agent-pane.continuation-spacing/half-tucked Apple System Colors unchanged 0% own App.tsx
agent-pane.continuation-spacing/before Apple System Colors Light unchanged 0% own App.tsx
agent-pane.continuation-spacing/before Apple System Colors unchanged 0% own App.tsx
agent-pane.copy-controls/output Apple System Colors Light unchanged 0% own App.tsx
agent-pane.copy-controls/output Apple System Colors unchanged 0% own App.tsx

Pick other states with a Gallery: entry.id, entry.id/variant line in the PR body.

Shipped in:

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:low Risk tier (docs/ci/risk-tiers.md) labels Nov 4, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-reviewer-34577): SHIP at debc368

Independent review, correctness first. Scratch worktree, bun install --frozen-lockfile in webviews/.

1. The base reds and the fix

  • Merge-base 8557b10: bun run check:design-tokens fails with only new design token violation (1 > 0): pages/changelog/updateSurfaces.css spacing margin-top 2px. bun run check has 3 red pieces: design-tokens, vp lint and vp check --no-fmt --no-lint. Both vp pieces show the same single error, test/transcript-spacing-kinds.test.ts:247:28 TS2304 Cannot find name 'after'. There is no other error.
  • Head debc368: design-tokens passes (Design token ratchet passed). bun run check exits 0 (Found no type errors in 4291 files). scripts/web-lint.sh passes (webviews).
  • The current feat-cmux-next tip (49d2b6e) still has margin-top: 2px and no after helper. This PR is still necessary.

2. The CSS
.wn-pop-brief .wn-pop-more changes from 2px to var(--ds-space-1) (4px, desktop.css). The file already uses --ds-space-* tokens. The rule applies only to the brief popover's More <div> (UpdateSurfaces.tsx:253). The More line now has the same 4px top margin as the stacked link above it (.wn-pop-brief > .wn-pop-all { margin: 4px 0 0 }). The full popover's footer <span> is not affected. The layout is good: there is 2px more space above one muted line.

3. The restored helpers

  • The bodies of below() and after() are byte-identical to the ones in 1e8a45ba8ce^ (test file lines 247-252). Only the two-line comment above them is different. They are in the same page.evaluate closure as the after(...) call (line 255), declared before it.
  • Semantics for Agent pane: the live Thinking line starts on the icon column and stays the last row #34370: a .cv-msgline__preview shows only in the stacked line look (MessageGroupHead.tsx:75, AgentMessageRow.tsx:89, SentMessageLine.tsx:176, IncomingMessageCard/InboxNoticeCard). The default is single (messageLineLook.ts), and this test never selects stacked. Thus the closed waiting group has no preview line, below() is 0, and waiting-to-live is the plain head-to-live diff. That is the correct measure for TRANSCRIPT_STATUS_ROWS waiting-to-live (22 centre to centre, one-line rows). If a stacked look is used, the helper subtracts the extra line, which is also correct. Non-blocking: the new comment and the line 254 comment say "stacked", but in this test the default look applies, so the subtraction is 0.

4. Scope
The diff from the merge-base has 2 files and +9/-1 lines. There are no other changes.

Observed / Not run

  • Observed: the base and head runs of check:design-tokens and bun run check, and scripts/web-lint.sh on the head. All are described above.
  • Not run: the browser-lane test transcript-spacing-kinds.test.ts itself (this PR fixes only its type check). There is no visual capture of the popover; the 2px change is a reading of the CSS only.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026
Merge-main commit by scripts/merge-main.sh.
Merged by scripts/merge-main.sh: origin/feat-cmux-next at fd7eb6f.

Merge-main-previous-head: debc368
Merge-main-base: fd7eb6f
…ict-template-expressions)

#34592 (31ffec4) builds a call label with ${params.cwd}, an unknown;
vp lint treats that as an error in the pre-push web lint, so the merge
service's hook refused #34577's squash. String() it; no behavior change.
feat's emoji shortcode change (#34364) left App.tsx, emojiShortcodes.test.ts
and composer-emoji.browser.test.ts unformatted, so vp fmt --check fails the
pre-push web lint on every webviews push. Output of scripts/web-lint.sh --fix.
@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-reviewer-34577): CHANGES at dca207c

Review of the delta from debc368 (my earlier SHIP) to dca207c.

Finding: the formatting commit dca207c makes a unit test red.
webviews/src/agent-session/acpmux/cloudRun.test.ts:409 reads App.tsx as text and asserts a single-line pattern:
/cloudFolderGrant\.granting\(folder\.cwd, pickFolder\(callNative, folder\.cwd/
The App.tsx reformat (line 8629) splits that call over several lines, so the pattern no longer matches.

  • At ef1b432 (before the format commit): bun test src/agent-session/acpmux/cloudRun.test.ts gives 21 pass, 0 fail.
  • At dca207c (head): 20 pass, 1 fail, the pane builds its Cloud start from grantedCloudStart and hands every Cloud pick's grant to it.

This is a red that the PR causes. scripts/web-lint.sh does not run unit tests, so the push hook did not find it. The current feat tip (7913736) still has the one-line App.tsx form, so feat is not red on this test.

Fix: make the regex accept whitespace, for example /cloudFolderGrant\.granting\(\s*folder\.cwd,\s*pickFolder\(callNative, folder\.cwd/. Another fix is to keep a one-line form that vp fmt accepts. Then run the same bun test command again.

The rest of the delta is good

  • Merge a3a2813: the diff from fd7eb6f has only this PR's 2 files (+9/-1). The merge is clean.
  • ef1b432 String(params.cwd) (cloudRun.test.ts:343): this is type-only. The pushed string is the same for a string cwd. The wrapped regex at line 406 is formatting only.
  • emojiShortcodes.test.ts and composer-emoji.browser.test.ts: whitespace only.
  • scripts/web-lint.sh at dca207c: passes (webviews, 25.2s).

Observed / Not run

  • Observed: the delta diffs; bun test cloudRun.test.ts at ef1b432 and at dca207c; scripts/web-lint.sh at the head.
  • Not run: other unit or browser test files. I only checked for other source-text tests on the reformatted App.tsx line with git grep 'granting\(folder': there is only this one.

…call

vp fmt puts cloudFolderGrant.granting(folder.cwd, pickFolder(...)) on
several lines, and cloudRun.test.ts read it as one line, so it failed
after the format commit. The pattern now allows whitespace between the
arguments. bun test cloudRun.test.ts: 21 pass, 0 fail.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-reviewer-34577): SHIP at 35aff4b

This review covers the delta from dca207c to 35aff4b. It is one line, in webviews/src/agent-session/acpmux/cloudRun.test.ts:409.

  • The new pattern /cloudFolderGrant\.granting\(\s*folder\.cwd,\s*pickFolder\(callNative, folder\.cwd/ matches the formatted App.tsx call at line 8629. It also still matches the one-line form. The test still checks the same thing: a Cloud pick sends its folder's pickFolder promise to cloudFolderGrant.granting.
  • bun test src/agent-session/acpmux/cloudRun.test.ts: 21 pass, 0 fail. On dca207c it was 20 pass, 1 fail, so this fixes my CHANGES finding.
  • bun test src/agent-session/acpmux/emojiShortcodes.test.ts: 12 pass, 0 fail.
  • I also ran all 21 test files that read App.tsx as text, in src/ and test/. Result: 195 pass, 3 skip (browser lane), 1 fail. The fail is test/agent-pane-changes-tab.test.ts, "the chat opens Changes as a tab for remote chats and non-repository turns...". It expects the text if (quick || !chatCwd) return false;. This is a base finding: the same test fails on this PR's merge base fd7eb6f and on the live feat-cmux-next tip aed6e9a. This PR does not change that App.tsx code: its App.tsx diff is only the formatted onMode call.
  • Scope: the diff from fd7eb6f has 6 files (+26/-7). These are the original 2 files, the 3 formatting files and the cloudRun.test.ts edits.

Observed / Not run

  • Observed: the bun tests above at 35aff4b, and agent-pane-changes-tab.test.ts at fd7eb6f and aed6e9a.
  • Not run: scripts/web-lint.sh at this head (the author's push hook ran it; I ran it at dca207c, and this delta changes only one regex). I did not run browser-lane tests.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 4, 2026
@blacksmith-sh

blacksmith-sh Bot commented Nov 4, 2026

Copy link
Copy Markdown

Found 5 test failures on Blacksmith runners:

Failures

Test View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_log2']/
CoreFoundation, missing 1: ['_log2']
View Logs
test_the_type_check_covers_every_webviews_test_file (main.WorkflowShape.test_the_ty
pe_check_covers_every_webviews_test_file)/
test_the_type_check_covers_every_webviews_test_file (main.WorkflowShape.test_the_ty
pe_check_covers_every_webviews_test_file)
View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 35aff4b0 Deployed Nov 4, 2026 by lawrencecchen via cmux-tui tree preflight #109140
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant