Skip to content

ci(web): lint and budget gates no longer skip the webview and sidecar tests - #22197

Merged
teamleaderleo merged 6 commits into
feat-cmux-nextfrom
ci/web-gates-keep-tests-v2
Oct 20, 2026
Merged

teamleaderleo merged 6 commits into
feat-cmux-nextfrom
ci/web-gates-keep-tests-v2

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Two jobs in ci-web.yml run lint or budget gates before tests in the same job, with no if: !cancelled() on the tests. A gate failure skips the tests silently, so their results are lost.

  • react-apps-check-shard: bun run check (Oxlint, Oxfmt, type check), the React Compiler check, the generated-bundle merge-driver test and the PR-scoped rules all run before Test webviews. A format miss skips every webview test on both shards. Keep shiki and the empty state off the markdown page's first load #22127 already fixed this for the eager JS budget step only.
  • diff-sidecar-check: one Check Rust sidecar step runs cargo fmt --check and clippy before cargo test, so a format miss also skips the sidecar tests, the generated-type check and the diff-viewer perf budget.

Change

  • react-apps-check-shard: each gate gets an id and continue-on-error: true. A final Fail on the gate checks step (if: !cancelled()) fails the job and names every gate that failed. This replaces the budget-only step from Keep shiki and the empty state off the markdown page's first load #22127. A failed bundle build or Playwright install still stops the tests, because the tests need them.
  • diff-sidecar-check: fmt and clippy move into their own Lint Rust sidecar (fmt, clippy) step with continue-on-error. Test Rust sidecar runs next, and a final step fails the job if the lint failed.

Before: one gate red, and the tests show as skipped. After: the job is still red, and the test results are there too.

The job's matrix (#22146) and runs-on are unchanged.

Verification

  • actionlint (with shellcheck) is clean on ci-web.yml.
  • tests/test_web_validation.py and tests/test_ci_change_areas.py have the same pass and fail set before and after this change. The failures that show up locally come from a sparse checkout and are unrelated.
  • Editing ci-web.yml routes every web subarea, so this PR's own run exercises both jobs.

Proof

Not UI: this changes CI workflow step ordering only, and nothing user-visible.

🤖 Generated with Claude Code


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


Summary by cubic

Ensures lint, format, and budget failures no longer hide webview and sidecar test results in ci-web.yml. Previously, a failing gate in react-apps-check-shard or diff-sidecar-check skipped dependent test runs without any trace.

Changes

  • In react-apps-check-shard, each gate (merge driver, eager JS budget, React Compiler, webviews check, PR-scoped rules) records its outcome with continue-on-error, and a final step fails the job listing any failed gate.
  • In diff-sidecar-check, fmt and clippy move to their own lint step with continue-on-error, and a final step fails the job when lint fails, so cargo test and downstream checks still run.
  • Failed bundle builds or engine installs still stop dependent tests, since those steps are prerequisites.

Written for commit 571cb9d. Summary will update on new commits.

View guided diff Turn on auto-fix

… tests

react-apps-check-shard ran bun run check, the React Compiler check, the
merge-driver test and the PR-scoped rules before Test webviews, so a lint or
format miss skipped every webview test. diff-sidecar-check ran cargo fmt and
clippy in the same step as cargo test. Each gate now records its outcome
with continue-on-error and a final step fails the job on any of them; a
failed bundle build or engine install still stops the tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cursor

cursor Bot commented Oct 19, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 571cb9d: 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=pr22197 -f ref=ci/web-gates-keep-tests-v2 -f platform=macos -f backend_mode=local

Probably yours: composer-caret-focus.test.ts, composer-edges.test.ts, composer-quiet-chips.test.ts fails here but not on main's latest full suite. Seen on other PRs too (likely flaky): agent-pane-no-capsules.test.ts, composer-chip-plain-rest.test.ts, composer-chip-plain-rest.test.ts and 7 more. This PR merged before its CI finished: fix forward on main.

CI failed on 571cb9d62c (run 39356148244 attempt 1): 3 code, 1 unknown.

Job Verdict Why
web / react-apps-check (2) code a web test failed
web / react-apps-check (1) code a web test failed
web / react-apps-check (4) code a web test failed
web / react-apps-check unknown no known signature; failed step: Check webview test shards

Failures:

  • seen on other PRs agent-pane-no-capsules.test.ts agent pane page > rounds its controls as small-radius rects, not capsules: failed (job)
  • seen on other PRs composer-chip-plain-rest.test.ts agent-pane.composer new-chat-reasoning: .acpmux-model .acpmux-picker-button is plain at rest; hover ...: failed (job)
  • seen on other PRs composer-chip-plain-rest.test.ts agent-pane.composer new-chat-reasoning: .acpmux-effort .acpmux-picker-button is plain at rest; hover...: failed (job)
  • seen on other PRs composer-chip-plain-rest.test.ts agent-pane.new-tab reasoning-chip: .acpmux-model .acpmux-picker-button is plain at rest; hover and f...: failed (job)
  • seen on other PRs composer-chip-plain-rest.test.ts agent-pane.new-tab reasoning-chip: .acpmux-effort .acpmux-picker-button is plain at rest; hover and ...: failed (job)
  • seen on other PRs markdown-wide-width-layout.test.ts markdown wide width (chromium) > the Aa popover: a typed width and the preset menu change the column...: failed (job)
  • seen on other PRs markdown-wide-width-layout.test.ts markdown wide width (webkit) > the Aa popover: a typed width and the preset menu change the column, ...: failed (job)
  • seen on other PRs chat-find.test.ts Find in Chat > keeps a Find match in view while what is above it keeps measuring: failed (job)
  • new in this PR composer-caret-focus.test.ts (unnamed): failed (job)
  • new in this PR composer-edges.test.ts (unnamed): failed (job)
  • ... 3 more in the job logs
Matched log lines
web / react-apps-check (2): (fail) agent pane page > rounds its controls as small-radius rects, not capsules [16.95ms]
web / react-apps-check (1): (fail) Find in Chat > keeps a Find match in view while what is above it keeps measuring [8652.00ms]
web / react-apps-check (4): (fail) agent-pane.composer model-menu-starred draws its text on the type scale [1632.70ms]

Not re-run automatically: web / react-apps-check (2), web / react-apps-check (1), web / react-apps-check (4), web / react-apps-check are not machine failures.

Written by scripts/ci/classify_failures.py (ci-failure-attribution.yml); signatures are its SIGNATURES table. A machine verdict is the runner's fault, not this PR's; yours means the failing file is one this PR changes, also red on main that main's latest full suite fails the same way, seen on other PRs that it failed on another pull request's run lately.

red: cmux-next checks (god files, concurrency, crash safety, l10n); no finished base run on fee6636 to compare; do not merge until green; see lane-rules
red: web / Web status; NEW: green on the base fee6636; do not merge until green; see lane-rules
red: web / react-apps-check; no finished base run on fee6636 to compare; do not merge until green; see lane-rules
red: web / react-apps-check (1); no finished base run on fee6636 to compare; do not merge until green; see lane-rules
red: web / react-apps-check (2); no finished base run on fee6636 to compare; do not merge until green; see lane-rules
red: web / react-apps-check (4); no finished base run on fee6636 to compare; do not merge until green; see lane-rules

CI fast guards failed on f6a2af2df6 (https://github.com/manaflow-ai/cmux/actions/runs/39355150738). It does not block the merge; a red guard merged into main breaks it for every open PR.
The log named no failed step; see the run.

Agents: python3 scripts/ci/guard_attribution.py fix applies the mechanical fixes locally.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Bead cx-jtbw4 created for this PR by the cmux coordinator.

@greptile-apps

greptile-apps Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The PR appears safe to merge; tests can run after gate failures, and those failures still fail CI.

Summary

This PR lets webview and Rust sidecar tests run after lint or budget failures without making those failures pass.

  • Webview gate failures are reported after the browser tests run.
  • Rust sidecar lint failures are reported after the tests and follow-up checks run.

No actionable issues found. No previous review threads were supplied. The merge bases differed, so this review covered the full PR diff. Verification was by code inspection; no workflows or tests were run.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Build web bundles] --> B[Run gates and record failures]
  B --> C[Install browser engines]
  C --> D[Run webview tests]
  D --> E[Fail job for recorded gate failures]
  F[Run Rust lint and record failure] --> G[Run Rust tests]
  G --> H[Check generated types and performance]
  H --> I[Fail job for recorded lint failure]
Loading

Reviews (6) · Last reviewed commit: "Merge branch 'feat-cmux-next' into ci/we..." · Reviewed by Greptile

@blacksmith-sh

blacksmith-sh Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Found 18 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp0czfuh0g/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpqacauaz_/summary-fail.json (live):/summary-fail.json (live): View Logs
test/agent-pane-no-capsules.test.ts/agent-pane-no-capsules.test View Logs
test/agent-pane-no-capsules.test.ts/agent-pane-no-capsules.test View Logs
test/chat-find.test.ts/chat-find.test View Logs
test/chat-find.test.ts/chat-find.test View Logs
test/composer-caret-focus.test.ts/composer-caret-focus.test View Logs
test/composer-caret-focus.test.ts/composer-caret-focus.test View Logs
test/composer-chip-plain-rest.test.ts/composer-chip-plain-rest.test View Logs
test/composer-chip-plain-rest.test.ts/composer-chip-plain-rest.test View Logs

...and 8 more test failures. View all on Blacksmith

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Subagent review at 0c6226b (correctness first): approve. The PR touches YAML only and actionlint is clean. Every red is a base red or infra: the Vite warm-up lint red; 16 webview test failures that were hidden behind that lint red before this PR and are now visible (#22118 is fixing them); and a cancelled release-compile. None is caused by this diff, so I'm adding ready-to-land.

@teamleaderleo teamleaderleo 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 Oct 20, 2026
@teamleaderleo teamleaderleo added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 20, 2026
# Conflicts:
#	.github/workflows/ci-web.yml
@teamleaderleo teamleaderleo added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 20, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

I merged feat-cmux-next at its tip again, 02d57c8. ci-web.yml conflicted with #22237, which moved the shard-independent gates to shard 1. I kept both changes: each gate step keeps its id and continue-on-error and also gets if: matrix.shard == 1. On other shards those steps are skipped, which Fail on the gate checks doesn't count as a failure. actionlint is clean. tests/test_web_validation.py and test_webviews_tests_per_file.py pass locally; the two yaml-dependent tests are left to CI.

@teamleaderleo teamleaderleo added ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds and removed ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds labels Oct 20, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next guards / defaults-snapshot is red on 13 of 13 fresh feat-cmux-next PR heads in the last 2:00:00, so it is most likely a base red, not this PR's. The base-reds lane has it; a base merge picks up the fix once it lands.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (correctness first, subagent), head 571cb9d62c

The first review was at 4de9daa4; the conflict it found with #22237 is now resolved. This pass re-checked only the rebased delta against feat-cmux-next (which contains #22237).

In react-apps-check-shard, the merge driver, eager JS budget, React Compiler, bun run check and PR-scoped rules steps each get an id and continue-on-error. A final Fail on the gate checks step fails the job and names every check that failed. In diff-sidecar-check, fmt and clippy move into their own continue-on-error step ahead of Test Rust sidecar, and a final step fails the job on that step.

  • The resolution keeps every if: matrix.shard == 1 from ci: shard the webviews suite twelve wide #22237 and adds the id: and continue-on-error: true lines beside it. So test_webviews_tests_shard_on_wide_runners still holds, and lint still runs once, not once per shard. On shards 2 to 4 the gated steps report skipped, which the final step does not count as a failure.
  • The final step is gated on !cancelled() and reads each env-mapped outcome through ${!var} indirection, so it catches every failed check. A failed bundle build or Playwright install still skips the tests, and the final step still runs without masking that failure.
  • In the sidecar job, a lint failure no longer skips the tests, the generated-type check or the perf budget.
  • Non-blocking: scripts/ci/pr_red_checks_comment.py builds its comparison from failed step conclusions. With continue-on-error, a failed check step reports success, so the red-checks comment now sees only the final step as failed. It can still tell a new red from a base red, because each per-check ::error::Gate failed; see the step '<name>' annotation is compared. The fixtures at tests/test_pr_red_checks_comment.py:165-175 still show the old step shape.

Verdict: MERGE

@teamleaderleo
teamleaderleo merged commit 9d18629 into feat-cmux-next Oct 20, 2026
@teamleaderleo
teamleaderleo deleted the ci/web-gates-keep-tests-v2 branch October 20, 2026 02:01
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

teamleaderleo added a commit that referenced this pull request Oct 22, 2026
…ecome instantiable structs (#24330)

`cmux-next checks` is red on every feat-cmux-next PR. Its namespace-type rule
rejects `AgentTurnRing` (AttentionHighlightLook.swift) and
`AgentPaneNeedsYouSetting`, both all-static public enums, and a lint red also
skips the checks that run after it in the same job (#22197), so it costs more
than the two lines it names.

`scripts/lint-ios-package-conventions.sh --namespace-fix`, the repo's own fixer,
makes each an instantiable struct with a public init. Every reference is a static
member (`AgentTurnRing.tunable`, `AgentPaneNeedsYouSetting.enabledPath` and
`.fallback`); nothing extends or conforms either type, and neither is used as a
metatype, generic argument or switch subject, so no call site changes and no
behavior does.

Base red, not this lane's: the same five checks fail with a byte-identical
failure set on a markdown-only PR (#24269) and a Swift PR (#24262), and both
violations are present at base tip 50a7cb4.


Agent-Session: https://cmux.com/deeplink/agent?harness=claude&session=01a15cb8-bbf0-7010-8d04-f9f2c0cb55df&host=big-red

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
teamleaderleo added a commit that referenced this pull request Oct 26, 2026
…g the web lint base red (#25543)

* webviews: give the workflow gallery's three a11y roles the reasons the lint asks for

`vp lint` reports three warnings in this file and exits 1, so the `web lint
(fast)` job fails for every push that touches webviews, and a base lint red
skips the checks after it in the same job (#22197).

Nothing here is actually wrong: an inline SVG drawn from live numbers is not an
`<img>`, a bar whose fill is a child span cannot be a `<progress>`, and a
tablist with a roving tabindex must not be a tab stop itself. Every other site
in this package that makes one of these choices already carries an
`oxlint-disable-next-line` with its reason (`charts.tsx`, `ThemePreview.tsx`,
`MessageActions.tsx`, `AttachmentViews.tsx`, `Tree.tsx`); this file was the one
that did not. So this follows that convention rather than loosening the rule or
changing the markup: comments only, no behaviour and no rendering change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-9cc710fd4775017aab15

* Say precisely why an img cannot stand in for the sparkline

The review called the old reason broad: an `<img>` can of course render an SVG,
once it is serialized. What it cannot do is host geometry built from the
workflow's own numbers and update it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-9cc710fd4775017aab15

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant