Skip to content

Fix the Cloud command timeout test after the request-deadline error split - #20502

Merged
austinywang merged 1 commit into
mainfrom
agent-ci-fix-main-app-host-tests-5f0ba298
Oct 14, 2026
Merged

austinywang merged 1 commit into
mainfrom
agent-ci-fix-main-app-host-tests-5f0ba298

Conversation

@austinywang

@austinywang austinywang commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Main CI run 38524277453 (aad8350) reported five app-host tests as RATCHET_NEW_FAILURE. Four were already fixed by #20459, which landed after that run. This PR fixes the fifth.

Test Cause Fix
CloudWorkspaceCreationRevealTests/closingTheWindowDuringShortcutReleasesItsManager(entryPoint:) (shard 5/7) The test sampled the closed manager while the close's own autoreleased notifications and queued main-queue work still held it. The Cloud create path holds it only weakly. Already fixed in #20459
SurfaceResumeAgentBindingGenerationTests/staleCachedSessionCannotAuthorizeCurrentBinding() (shard 4/7) App bug from #20246: Codex index authority came from isComplete(), so a rejected stale checkpoint left the agent counted as running and restore replayed a dead binding. Already fixed in #20459
MuseVaultRegistrationTests/vaultListsMuseSessions() (shard 3/7) Stale test: registered agents have quoted arguments and added a cd prefix since August. Already fixed in #20459
GhosttyMinimumContrastLayerBackgroundTests/darkTextOnLightThemeStaysDark() (shard 3/7) On Blacksmith runners the app-host window is never key or visible, so the renderer never drew. Already fixed in #20459 (window reports on-screen occlusion)
CloudCommandOperationTests/commandTimeoutLeavesRecoverableOperationVisibleAfterLaterSuccess() (shard 6/7) Stale test, not a timing problem. It already uses an injected deadline clock. #20376 added LinkError.requestTimedOut for a channel's per-request deadline, and the app classifies it as .timeout and retryable. The test still caught only .timedOut, so the expected deadline error escaped as Caught error: requestTimedOut after 4 ms. This PR: catch requestTimedOut and check that the same error is retryable

Verification

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 the Cloud command timeout test, which broke after #20376 split the per-request deadline error from the connect timeout. The app now throws LinkError.requestTimedOut for a channel's per-request deadline, but the test still caught only .timedOut, so the expected deadline escaped as an uncaught error.

  • Catch requestTimedOut and assert that the same error is retryable.

Written for commit 198f6dd. Summary will update on new commits.

View guided diff Turn on auto-fix

#20376 gave a persistent channel's per-request deadline its own error,
LinkError.requestTimedOut, separate from the connect timeout. The app
already classifies it as a timeout and as retryable, but this test still
caught only .timedOut, so the deadline it waits for escaped as an
uncaught error. Catch the error the channel now throws, and check that
same error is retryable.

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

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 4 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 9460e14e-5105-43eb-a610-8a02ee796f09
📥 Commits

Reviewing files that changed from the base of the PR and between 28e8640 and 198f6dd.

📒 Files selected for processing: 1
  • cmuxTests/CloudCommandOperationTests.swift
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@austinywang
austinywang merged commit 4d9d0f7 into main Oct 14, 2026
18 of 20 checks passed
@austinywang
austinywang deleted the agent-ci-fix-main-app-host-tests-5f0ba298 branch October 14, 2026 08:16
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 198f6dda5e, merged 2026-10-14 08:16:14 UTC

  • Not verified at merge: backend migrations applied (not reported), ci-status (not reported), CI fast guards (in progress), command-regressions (failure), Fast static checks (in progress), plan (in progress)
  • Verified: web-validation
  • Skipped by policy: web-build, web-database-tests, web-tests
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 14, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Machine: the runner failed, not this PR. Not re-run: this PR has merged.

CI failed on 198f6dda5e (run 38533883266 attempt 1): 1 machine.

Job Verdict Why
macos / macOS compile admission machine the owned runner refused the job (host busy or out of capacity) (runner cmux-austin-mini-1-glaeda)
Matched log lines
macos / macOS compile admission: glaeda-cmux-runner-hook: refused: node not eligible (rust toolchain 1.98.1-aarch64-apple-darwin is not installed)

Not re-run: this PR has merged.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant