Skip to content

base red: cloud-chat-resume tests prove #35206's resume wait, not the removed 60 s deadline - #35423

Merged
lawrencecchen merged 1 commit into
feat-cmux-nextfrom
fix/base-red-cloud-chat-resume
Nov 4, 2026
Merged

lawrencecchen merged 1 commit into
feat-cmux-nextfrom
fix/base-red-cloud-chat-resume

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red on feat-cmux-next: 5 tests in webviews/test/cloud-chat-resume.test.tsx fail on the tip.
#35206 removed useCloudChatRecovery's 60 s recovery deadline on purpose. Its gate table says
"removed; retries continue while active", and the code says "a busy machine never turns a slow
list into a terminal resume failure". The tests still asserted the deadline.

This PR changes only that test file. The tests now prove what ends a resume after #35206:

End Test
The request answers: the resume goes on a slow inventory/recheck keeps the resume waiting past the old 60 s deadline until it answers
The native call fails for good (non-retryable, the daemon is gone): the resume fails with the error new: a slow inventory/recheck whose daemon is gone fails the resume
The person leaves the chat (switch, unmount): the wait ends with no error and no timer switching chats cancels a hung inventory/recheck without failing the new recovery
A newer resume owns the card after an older resume ends an attach left pending when its resume ends: its late rejection leaves a newer resume running

All of them use fake timers and held requests that the test answers or fails. They have no wall-clock wait.

Gap: #35206 states no end for one case

A request that never answers while the native bridge stays alive keeps the resume waiting until
the person leaves the chat. While it waits, the card has no cancel and its actions are disabled.
#35206 names no other end for this case, so this PR does not add a timeout. The first two tests
assert today's behavior explicitly: 10 minutes with no timer, no error and no repeated read.
The PR author must decide if a cancel or a liveness end is needed.

Red / green

bun test test/cloud-chat-resume.test.tsx from webviews/:

Observed / Not run

Observed: the runs above. bun run check:fixed-waits adds no fixed wait in this file. On a
temporary merge with #35409: bun run check exits 0, and cloud-chat-resume plus
cloud-chat-recovery pass 18/18.

On this branch alone, scripts/web-lint.sh is red only on the base reds that #35409 fixes
(check:fixed-waits on transportDeadlines.test.ts, vp lint and vp check on files this PR does not touch).
I pushed with CMUX_WEB_LINT_SKIP_REASON and named the base sha.

Not run: the full webviews test suite.

Changelog

none


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 five failing tests in webviews/test/cloud-chat-resume.test.tsx on feat-cmux-next; this PR changes only that file. #35206 removed useCloudChatRecovery's 60-second recovery deadline on purpose, but the tests still asserted it, so they now prove the three ways a resume ends after #35206: the request answers, the native call fails for good, or the person leaves the chat.

All tests use fake timers and held requests the test answers or fails, with no wall-clock waits. An old chat's late answer or failure never touches a newer resume.

Gap
#35206 names no end for a request that never answers while the native bridge stays alive. The tests pin the current behavior — the resume waits until the person leaves the chat, with no cancel and disabled actions — and reviewers must decide if a cancel or liveness end is needed. The suite passes 12/12 here and fails 6/12 when run against the old deadline hook, so a return of the deadline would be caught.

Written for commit ee3c1e7. Summary will update on new commits.

View guided diff Turn on auto-fix

… removed 60 s deadline

#35206 removed useCloudChatRecovery's 60 s recovery deadline on purpose ("removed; retries
continue while active"; "a busy machine never turns a slow list into a terminal resume
failure"). Five tests still asserted that deadline and failed on the tip.

A resume now ends when its request answers (the resume goes on), when the native call fails
for good (non-retryable: the daemon is gone; the card shows the error), or when the person
leaves the chat (switch, deactivate, unmount). The tests now prove each end, with fake timers
and held requests the test answers or fails; no wall-clock wait:

- a slow inventory/recheck keeps the resume waiting past the old deadline (10 min) with no
  timer and no repeated read, until it answers; the answer runs the recheck and the next read.
- new: a slow inventory/recheck whose daemon is gone fails the resume with the error.
- switching chats ends a hung inventory/recheck with no error and no timer; the old chat's
  late answer or failure does not touch the new recovery or read for the old chat.
- an attach left pending when its resume ends (now: the daemon is gone, was: the deadline):
  its late rejection leaves a newer resume running; the current attach owns its failure.

Gap, not invented here: #35206 states no end for a request that never answers while the
bridge stays alive. Today that resume waits until the person leaves the chat (the card has no
cancel and disables its actions while resuming). The first two tests assert that explicitly.

bun test test/cloud-chat-resume.test.tsx: head 12/12 pass (3 runs). With the hook from
a628f50^, 6 fail (the waits now end at its 60 s deadline timer).
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 4, 2026
@cursor

cursor Bot commented Nov 4, 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.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at ee3c1e7

What I checked:

  • Spec: Webview agent pane waits on live daemon requests #35206 description and its diff to useCloudChatRecovery.ts. The new tests prove the three ends that the hook has now: the request answers (the resume goes on and reads again after 1 s), a non-retryable NativeError (the resume fails with the error and no retry timer), and leaving the chat (the session switch disposes the effect; a late answer from the old chat changes no state and starts no read, recheck or timer). The non-retryable path was in the hook before Webview agent pane waits on live daemon requests #35206 and Webview agent pane waits on live daemon requests #35206 kept it. The tests do not invent behavior.
  • No assertion is weaker without a reason. The attach-ownership test now ends the first resume through a non-retryable list error, not through the deadline. The ownership checks (old rejection ignored, current rejection releases the button) are the same.
  • Gap statement: correct for the hook. When a request stays pending, the hook has no timer. Only a dispose ends the wait (identity, session or active change, refresh, unmount).
  • Flake and hang: fake timers with now: 0, held requests that the test settles in act(), getTimerCount() checks, no wall-clock wait added. Requests that stay pending at unmount are guarded by disposed and do not keep the run alive.
  • bun test test/cloud-chat-resume.test.tsx: 12/12 pass, 5 runs, about 4.5 s each.
  • Head tests with the hook from a628f506790e^: 6 fail (all six new or rewritten wait tests). The base test file on the base hook: 5 fail, which is the base red. I restored the files and the worktree is clean.
  • bun run check:fixed-waits: the only red is transportDeadlines.test.ts (base red, fixed by base red: web-lint check:fixed-waits from #35206 + #35209 #35409). The two fixed waits in this file were there before this PR and have // fixed-wait: reasons.

Non-blocking notes:

  • In the switch test, held[0].fail() after held[0].answer() does nothing, because the promise is already settled. The late-failure path for the old chat is not tested. The comment says "would", so the text is honest.
  • In the daemon-gone test, the comment "nothing reads again" is not exact. After the resume ends, the effect does one non-resuming recoveryList read. The test checks only that there is no retry timer.
  • The header comment in useCloudChatRecovery.ts:14 still says "at most one minute". This PR does not touch that file. It is a follow-up.

@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 3 test failures on Blacksmith runners:

Failures

Test View Logs
nightly cmux-tui resolve/nightly cmux-tui resolve View Logs
nightly.yml must not read vars.NIGHTLY_SIGN while nightly-sign.yml is in shadow mode/
nightly.yml must not read vars.NIGHTLY_SIGN while nightly-sign.yml is in shadow mode
View Logs
the short nightly decide gate must use the pooled Linux route with a Blacksmith fallbac
k/
the short nightly decide gate must use the pooled Linux route with a Blacksmith fallbac
k
View Logs

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

@lawrencecchen
lawrencecchen merged commit a3abfc4 into feat-cmux-next Nov 4, 2026
217 of 237 checks passed
@lawrencecchen
lawrencecchen deleted the fix/base-red-cloud-chat-resume branch November 4, 2026 13:51
@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)

Unclear: web / react-apps-check (1) failed: no known signature; failed step: Fail on the gate checks. This PR merged before its CI finished: fix forward on main.

CI failed on ee3c1e7acb (run 41837790009 attempt 1): 2 unknown.

Job Verdict Why
web / react-apps-check (1) unknown no known signature; failed step: Fail on the gate checks
web / react-apps-check unknown no known signature; failed step: Check webview test shards

Not re-run automatically: web / react-apps-check (1), 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: web / Web status; a job it gathers is red on this change (its line here); do not merge until green; see lane-rules
red: web / react-apps-check; a job it gathers is red on this change (its line here); do not merge until green; see lane-rules
red: web / react-apps-check (1); no finished base run on c694fd0 to compare; do not merge until green; see lane-rules

Shipped in:

This branch was previously deployed

1 inactive deployment
ci-routing-agent — ee3c1e7a Deployed Nov 4, 2026 by lawrencecchen via wait for same-key tree publisher #111818
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:low Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant