Skip to content

test(acpmux): start-mode answers don't leak between test files - #22158

Merged
teamleaderleo merged 19 commits into
feat-cmux-nextfrom
leo/startmode-test-isolation
Oct 20, 2026
Merged

teamleaderleo merged 19 commits into
feat-cmux-nextfrom
leo/startmode-test-isolation

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

new-tab-composer-first-frame.test.tsx fails whenever access-chip-pending-warning.test.tsx runs before it in the same bun process. Whether a shard goes red then depends on file order.

startMode.ts keeps acpmux's start-mode answers in module memory, which it reads from storage once per page. Both test files cleared localStorage after each test but not that memory. So the first-frame test inherited an earlier file's answer that this folder starts with no mode, and its access chip never drew. The first-frame file's own cleanup also wrote that same answer for the next file. Both files now reset the module through resetStartModesForTests(), a test seam like modelCatalogHost's resetModelCatalogForTests(). App behavior doesn't change.

Testing

On huge-blue with bun 1.4.2, from webviews/:

  • bun test test/access-chip-pending-warning.test.tsx test/new-tab-composer-first-frame.test.tsx
    • Base 4b5c7ad9: 11 pass, 1 fail ("a new chat paints its last effort and its kept start mode on the first frame…").
    • Head 14da28ca: 12 pass. The reverse order also passes 12/12.
  • bunx tsc --noEmit -p . is clean.

Observed / Not run

Changelog

none

🤖 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

Fixes a test-ordering flake where new-tab-composer-first-frame.test.tsx failed when access-chip-pending-warning.test.tsx ran first in the same bun process.

startMode.ts keeps acpmux start-mode answers in module memory, read from storage once per page. Both test files cleared localStorage but not that memory, so the first-frame test inherited the earlier file's answer and its access chip never drew. Both files now reset the module through resetStartModesForTests(), a test seam like resetModelCatalogForTests(). App behavior doesn't change.

Testing

  • Both file orders pass 12/12.
  • bunx tsc --noEmit -p . is clean.

Written for commit 75ef674. Summary will update on new commits.

View guided diff Turn on auto-fix

startMode.ts keeps acpmux's answers in module memory, read from storage
once per page. Two test files cleared localStorage but not that memory,
so in one bun process new-tab-composer-first-frame failed whenever
access-chip-pending-warning ran first (an inherited 'no mode here' answer
hid the access chip). Both now reset the module through a test seam, as
modelCatalogHost's resetModelCatalogForTests does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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 75ef674: 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=pr22158 -f ref=leo/startmode-test-isolation -f platform=macos -f backend_mode=local

Passes: CI passes on 75ef67492f.

CI passes on 75ef67492f (run 39460047562 attempt 1).

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 88d14fb to compare; do not merge until green; see lane-rules

CI fast guards passes on 75ef67492f (run). This covers only the fast guards, not CI: CI's result is ci-status.

Tour of 75ef67492f (run): no media. The tour did not run: only webview code changed (3 path(s)), which the gallery advisory run checks in Linux Chromium on the pull request (its artifact holds any captures); add the media-tour-mac label to tour it on a Mac.

@blacksmith-sh

blacksmith-sh Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

Found 11 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmpqwxdbdnz/summary-fail.json (live):/summary-fail.json (live): View Logs
/tmp/tmps0xuvk7v/summary-missing.json (live):/summary-missing.json (live): View Logs
bundle-map.json does not require CEF's LICENSE.txt/
bundle-map.json does not require CEF's LICENSE.txt
View Logs
bundle-map.json does not require CEF's LICENSE.txt/
bundle-map.json does not require CEF's LICENSE.txt
View Logs
convention violations found (ERROR lines above)./
convention violations found (ERROR lines above).
View Logs
test_a_file_a_test_reads_selects_that_test (main.PullRequestTiers.test_a_file_a_tes
t_reads_selects_that_test)/
test_a_file_a_test_reads_selects_that_test (main.PullRequestTiers.test_a_file_a_tes
t_reads_selects_that_test)
View Logs
test_batch_dispatch_runs_every_dispatch_gated_job (main.PathRoutingStructure.test_b
atch_dispatch_runs_every_dispatch_gated_job) (job='request-cmux-next-dev-publish')/
test_batch_dispatch_runs_every_dispatch_gated_job (main.PathRoutingStructure.test_b
atch_dispatch_runs_every_dispatch_gated_job) (job='request-cmux-next-dev-publish')
View Logs
test_every_submodule_free_checkout_on_an_owned_runner_resets_submodules (main.Reuse
dWorkspaceSubmodules.test_every_submodule_free_checkout_on_an_owned_runner_resets_submo
dules)/
test_every_submodule_free_checkout_on_an_owned_runner_resets_submodules (main.Reuse
dWorkspaceSubmodules.test_every_submodule_free_checkout_on_an_owned_runner_resets_submo
dules)
View Logs
the repository has capability texts that say update/
the repository has capability texts that say update
View Logs
the repository has capability texts that say update/
the repository has capability texts that say update
View Logs

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

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

@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.

Summary

Prevents start-mode answers from leaking between the two test files.

  • Start-mode answers are cleared between the two webview test files.

Reviews (4) · Last reviewed commit: "Merge remote-tracking branch 'origin/fea..." · Reviewed by Greptile

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

cmux-next guards / defaults-snapshot is red on 22 of 23 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 (subagent, correctness first): SHIP at 45d6df7

No blocking findings. resetStartModesForTests clears the cached answers and bumps the snapshot version without dropping live subscribers; production never calls it. It follows the existing reset*ForTests seams in webviews.

Risk tier: normal (agent review plus checks; no high-tier paths touched).

@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

Copy link
Copy Markdown
Collaborator Author

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Browser chrome icons is red on 7 of 8 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

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Capability texts never say update the app is red on 9 of 9 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

CI / web / react-apps-check (3) is red on 9 of 15 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

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Checks job structure is red on 10 of 11 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

CI / web / react-apps-check (2) is red on 8 of 12 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

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Main-thread concurrency is red on 9 of 11 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

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › No process scanning (holders register themselves, plans/cmux-next/pty-holder.md 4) is red on 10 of 11 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

CI / web / react-apps-check (1) is red on 9 of 14 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

cmux-next / cmux-next checks (god files, concurrency, crash safety, l10n) › Package conventions lint is red on 10 of 10 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
teamleaderleo merged commit 404e56f into feat-cmux-next Oct 20, 2026
@teamleaderleo
teamleaderleo deleted the leo/startmode-test-isolation branch October 20, 2026 17:05
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

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