Skip to content

base red: webviews check:browser-lanes, check:design-tokens, check:type-scale, vp lint - #29076

Closed
lawrencecchen wants to merge 1 commit into
feat-cmux-nextfrom
base-red/webviews-lint-a8615f
Closed

lawrencecchen wants to merge 1 commit into
feat-cmux-nextfrom
base-red/webviews-lint-a8615f

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The feat-cmux-next tip failed four webviews checks (seen by merge-batch at a8615fd, still red at 25c1584). The pre-push hook refused every squash that touches webviews. This PR fixes each finding in the code. No checker, baseline or allowlist changes.

Check Finding at 25c1584 Culprit Fix
check:browser-lanes test/composer-draft-overflow.test.ts:434 samples in a raw rAF loop dc0fd32 (Austin Wang, #28672 work, direct push) The sampler reads through window.cmuxAfterPaint, which open already installs
check:design-tokens ContextSection.css padding 2px 6px 0 fbea666 (#28969) 4px 8px 0; the link's own 2px side padding is taken back (margin-right: -2px) so its text stays on the rows' 8px trailing edge
check:type-scale pages/apps/styles.css: 8 raw sizes, baseline 7 (new .apps-instance 12px) d13d17a (direct push) var(--text-detail) (12px) with its letter spacing
vp lint SectionList.tsx:73 jsx-a11y(prefer-tag-over-role) on role="group" 6b7c28f (direct push) Reasoned disable: the suggested tags (fieldset, details, optgroup) are for form controls or disclosure, not a labelled run of nav links (same as DrillList)

vp fmt --check is already green at this tip (#28927).

Changelog

none

Observed / Not run

  • Observed: bun run check in webviews/ at base 25c1584: 4 red: check:browser-lanes, check:design-tokens, check:type-scale, vp lint. At head 95efc8b (rebased on a0d51f8): exit 0.
  • Observed: scripts/web-lint.sh passed; prepush-gate.py: no new violations.
  • Observed: CMUX_BROWSER_TESTS=1 bun test test/composer-draft-overflow.test.ts -t "never under a button|inset comes back": 5 pass, 0 fail (Chromium).
  • Not run: the WebKit lane of that test (WebKit is not installed on this Linux VM). No visual capture of the 2px change in the Context section's Compact row (the text position is the same; the row is 2px taller at the top).

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


Summary by cubic

Fixes four red webviews checks on the feat-cmux-next tip so the pre-push hook no longer blocks webviews squashes.

  • check:browser-lanes: the draft-overflow sampler now reads through window.cmuxAfterPaint instead of a hand-rolled rAF + MessageChannel loop.
  • check:design-tokens: the Context section's Compact row padding moves from 2px 6px 0 to 4px 8px 0; the link's own 2px side padding is offset so its text keeps the rows' 8px trailing edge.
  • check:type-scale: .apps-instance uses var(--text-detail) with its letter spacing instead of a raw 12px.
  • vp lint: SectionList gets a reasoned disable for role="group" since the suggested tags (fieldset, details, optgroup) target form controls or disclosure, not navigation links.

Written for commit 95efc8b. Summary will update on new commits.

View guided diff Turn on auto-fix

…pe-scale, vp lint

The feat-cmux-next tip (25c1584, red since a8615fd) failed four webviews
checks, so the pre-push hook refused every webviews squash.

- check:browser-lanes: composer-draft-overflow's button-edge sampler read in a
  hand-rolled rAF + MessageChannel loop (dc0fd32, #28672). It now reads
  through window.cmuxAfterPaint, the painted-frame reader `open` already installs.
- check:design-tokens: the Context section's Compact row padded 2px 6px 0
  (fbea666, #28969). Now 4px 8px 0 on the scale; the link's 2px side
  padding is taken back with margin-right: -2px, so its text keeps the rows'
  8px trailing edge.
- check:type-scale: .apps-instance used a raw 12px (d13d17a, direct push).
  Now var(--text-detail) (12px) with its letter spacing.
- vp lint: SectionList's sidebar section is role="group" (6b7c28f, direct
  push). The rule's suggested tags (fieldset, details, optgroup) are for form
  controls or disclosure, not a labelled run of nav links, so the line gets a
  reasoned disable, as DrillList's listbox groups have.

vp fmt --check is already green at this tip (#28927).
Verified: `bun run check` in webviews/ is green (was 4 red at 25c1584).
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
lawrencecchen referenced this pull request Oct 31, 2026
…r paints under Send (#28672)

#28672 follow-up: text typed near Send was sometimes
covered by a button's pill instead of wrapping. The prompt spans the actions' column and keeps
clear of them through --acpmux-actions-width, which followActionsWidth measured from the
actions element it found when the prompt mounted, with a cache of the last width it wrote. An
actions element rendered later or replaced was never measured, and a box that lost the property
(a field's cleanup in the same box) was never written again while the width stayed the same, so
the 64 px CSS fallback stood in, narrower than ↵, Stop and the mic during a turn.

- composerScrollLane.ts follows whichever actions element the box holds now and writes the width
  again whenever the box no longer carries it (a MutationObserver on the box that skips the
  prompt's own edits).
- test/composer-draft-overflow.test.ts: text typed up to Send (words, an unbroken run, spaces)
  with the pointer on the last action, sampled after every painted frame for any glyph under a
  button, idle at 260 and 420 px, during a turn and with prompts queued; and the inset coming
  back after the box loses it during a turn (without the fix a glyph paints under Steer now).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
lawrencecchen referenced this pull request Oct 31, 2026
… its instance

Hello Deploy's listing named no deploy target, so nothing told the person
it runs on their own machine or their cmux Cloud machine before Get.
- catalog.get sends deploy {targets, tenancy} for an app with a deploy
  block (none otherwise); the image and port stay out of the listing.
- The App Store page draws a "Runs on" section: "A machine you host (SSH
  with Docker or Podman)", "Your cmux Cloud machine", "Your team's cmux
  Cloud machine", "This Mac", each with its manifest name.
- cmux apps info prints "runs on: self-hosted, cloud-vm".
- An Installed row shows the deployed instance the owner already sent and
  the page dropped: "Running on <target>" / "Starting on <target>".

Seven new strings in all 21 languages (CmuxNextApps Localizable.xcstrings).
lawrencecchen referenced this pull request Oct 31, 2026
User (2026-10-30): "refactoring the settings page to look a lot better with
sections".

- Sidebar: the 21 categories now sit under small headers, the layout the
  Swift SettingsPageLayout.sidebarSections exports (page.sidebar_sections):
  General and Updates (untitled), Look and Feel, Tools, AI, Interaction,
  Accounts and Devices, Data and System. The category order follows the
  sections (Browser moves up to Tools; Import and Privacy join Data and
  System). One source: the sections hold the category specs, specs derive
  from them. A narrow pane keeps its one scrolling row; headers hide there.
- Cards: five walls of rows split into titled cards (cap 16 rows,
  SettingsPageLayout.maxRowsPerCard): General > Tabs 30 -> Tabs, Tab Widths,
  New Tab Page, New Tab Widgets; Columns 20 -> Columns, New Panes, Docked
  Columns, Scrolling and Focus; Appearance > Density and Motion -> + Sizes;
  Sidebar 57 -> Sidebar, Sidebar Workspaces, Sidebar Buttons and Spaces,
  Workspace Hover Cards, Group Hover Cards; Surfaces 28 -> Surfaces,
  Surfaces: Window, Surfaces: Viewers; Agents > Agent Chat 28 -> Agent Chat,
  Chat Layout, Chat Text, Message Box, Files and Links.
- 23 new catalog strings, all 21 locales (non-English needs_review).
- No row key, section, CLI path or search behavior changes; search still
  jumps to a row in its new card.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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 Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 95efc8b

Correctness review of the 4-file diff (HEAD~1..HEAD). The diff changes no checker, baseline or allowlist; only the 4 source files listed in the PR body changed.

Verification (on this head, Linux VM)

  • bun run check in webviews/: exit 0. check:browser-lanes ("Browser test lane imports checked"), check:design-tokens ("ratchet passed: 2136 recorded violations"), check:type-scale ("ok, 314 raw font sizes left in the baseline"), vp lint (clean), vp fmt --check and the type check pass. The check:system-blue and check:ui-review lines are warnings on files this PR does not touch.
  • CMUX_BROWSER_TESTS=1 bun test test/composer-draft-overflow.test.ts -t "never under a button|inset comes back": 5 pass, 0 fail (Chromium). WebKit is not installed here, so its lane was skipped. That is the same gap the PR body reports.

Per change

  • composer-draft-overflow.test.ts: the old loop posted a MessageChannel message from each rAF callback and ran check in that task. cmuxAfterPaint (test/support/paintedFrames.ts) does the same per frame: it asks for the next rAF in the callback, then runs the read as a task posted after the frame. It also runs a pending read at the first input event, before the page's handlers, so it never reads a layout that no frame painted. That is the same or stricter. check returns undefined, never false, so sampling keeps going for the whole test, as before. open calls installPaintedFrames(page) (line 192) before any watchButtonEdge call, and the global Window declaration comes in through the existing import. Not a hack: the test now uses the shared reader that the lane check requires.
  • ContextSection.css: the old horizontal geometry was container padding 6px plus the link's own 2px padding (inspector.css .acpmux-chat-inspector-link { padding: 0 2px }), so the text ended 8px from the edge. The new geometry is 8px padding with a -2px margin on the link plus the link's 2px padding, so the text still ends at 8px. That matches the rows (.acpmux-chat-inspector-row padding 0 8px). The focus ring and hover area are unchanged (the link box is in the same place). The only visual change is 2px more space above the Compact link, which the PR body states. The design-token scanner does not check margin, so the -2px could look like a way around it. I judge it a legitimate optical-alignment offset: it cancels a specific component padding, has a comment, and lets the container sit on the 4/8 scale. The other choice, changing the shared link padding, would move Environment's Save, Connectors and the subagent Back link.
  • apps/styles.css: --text-detail is calc(12px * var(--cmux-interface-scale, 1)) (pages/shared/desktop.css), so the size is the same 12px at scale 1. It now also follows the interface scale and gets the token's letter spacing, the same as the other --text-detail uses in this file (lines 147, 730, 801). This is the intended token use.
  • SectionList.tsx: the markup and ARIA are unchanged (role="group" with aria-labelledby stays), so accessibility does not change. A labelled group of nav links inside <nav> is correct ARIA. fieldset/details/optgroup would be wrong here. The disable has a reason and follows the same pattern as src/ui/DrillList.tsx:187.

Findings

  • None blocking.
  • Nit (no action needed): the PR body says the Context row is "2px taller at the top" and gives no capture. The change is small and the PR states it.

No hacks found: each fix changes the code that was flagged, and no checker inputs changed.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Superseded: direct pushes 2d93485, a19e400 and e7bab27 fixed the same four findings on feat-cmux-next while this PR was in review, and this PR now conflicts with them. bun run check in webviews/ exits 0 at a3f620e. No ready-to-land: nothing is left to fix. Differences that are not fixed (follow-up only if wanted): the Context Compact padding is now calc(var(--ds-space-1) / 2) calc(var(--ds-space-3) / 2) 0, still 2px/6px (off the 4/8 scale); .apps-instance has no --text-detail--letter-spacing; the composer test keeps its own rAF loop with a raf-read: comment instead of cmuxAfterPaint.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 95efc8b7 Deployed Oct 31, 2026 by lawrencecchen via wait for same-key tree publisher #88247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant