Skip to content

test: stop the CLI no-socket regression waiting 90 s for an app it never starts - #14334

Merged
teamleaderleo merged 1 commit into
mainfrom
test/cli-restore-startup-timeout
Sep 25, 2026
Merged

teamleaderleo merged 1 commit into
mainfrom
test/cli-restore-startup-timeout

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Why

In run 36075060865 (#10366), testLaunchCapableCommandsReachTheirDispatchPathWithoutLiveImplicitSocket took 90.5 s of app-host shard 4's "CLI no-socket regressions" step (218 s). Its restore and fork cases expect cmux is still opening, which the CLI reports only after restoreSocketStartupTimeoutSeconds (45 s) passes with no app socket. The test deliberately starts no app, so it waits 45 s twice.

Change

  • CLI/cmux.swift: CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS can shorten the wait, clamped to 0.05 to 45 s. It cannot lengthen it. This follows CMUX_DIFF_VIEWER_WAIT_TIMEOUT_SECONDS and CMUX_SSH_PTY_BRIDGE_READY_TIMEOUT_SECONDS. The default stays 45 s.
  • The test sets it to 1 s. It still asserts the same still opening errors and that the CLI never reports No live cmux socket found.

Proof

CI's changed-suites lane runs CMUXCLIErrorOutputRegressionTests; the test should drop from about 90 s to about 2 s. I typechecked the getter standalone: default 45, set to 1 gives 1, set to 999 gives 45.

🤖 Generated with Claude Code


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


Summary by cubic

Shortens the CLI no-socket regression test from about 90 seconds to about 2 seconds by letting tests override the restore startup timeout.

  • restoreSocketStartupTimeoutSeconds now reads CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS, clamped between 0.05 and 45 seconds; the default stays 45 seconds.
  • The test sets the override to 1 second and continues asserting the same "still opening" errors.

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

Review in cubic

Summary by CodeRabbit

  • Improvements
    • The restore socket startup timeout can now be configured with CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS. Valid numeric values are limited to 0.05–45 seconds; missing, invalid, or non-finite values use the 45-second default.

testLaunchCapableCommandsReachTheirDispatchPathWithoutLiveImplicitSocket took
90 s: restore and fork each waited the full 45 s for an app socket that the
test deliberately never starts.

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

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 75fe88c6-3e31-41ec-a94f-f8b105e0702b

📥 Commits

Reviewing files that changed from the base of the PR and between 24efd87 and e00ab8a.

📒 Files selected for processing (2)
  • CLI/cmux.swift
  • cmuxTests/CMUXCLIErrorOutputRegressionTests.swift

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The CLI now supports a bounded restore socket startup timeout override through CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS. The regression test sets this value to one second to avoid the default wait.

Changes

Restore socket startup timeout

Layer / File(s) Summary
Timeout override and regression test
CLI/cmux.swift, cmuxTests/CMUXCLIErrorOutputRegressionTests.swift
The CLI uses the environment value when it is finite and numeric, clamps it between 0.05 and 45 seconds, and retains 45 seconds for invalid or missing values. The regression test sets the value to 1.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Suggested reviewers: austinywang

Merge Risk: ⚪ Minimal · up to e00ab

The change preserves the default timeout, and the regression test uses the valid one-second override. No actionable current-head risk remains; it is ready to merge subject to normal checks.

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ❓ Inconclusive Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: reducing the no-socket regression test wait time when no app starts.
Description check ✅ Passed The description explains the problem, implementation, preserved default behavior, test override, expected performance improvement, and validation evidence. It does not use the exact Summary and Testin…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The diff only adds an environment-controlled timeout for restore socket startup and sets it in a no-socket CLI regression test. It does not change Cloud terminal creation, cmux-tui client or phy…
Cmux Swift Actor Isolation ✅ Passed The production diff adds a computed timeout property to the unannotated CMUXCLI struct. It does not add an implicitly @MainActor model or protocol, a Sendable reference type, or a UI-bound store…
Cmux Swift Blocking Runtime ✅ Passed PASS: The production diff only replaces the fixed 45-second value with an environment-derived value clamped to 0.05–45 seconds. It does not add a semaphore, sleep, delayed dispatch, polling loop, main…
Cmux Browser Automation Off-Main ✅ Passed The check is not applicable. The PR changes only the CLI restore-socket timeout and a regression-test environment variable. It does not add or route any browser.* command, WebKit/AppKit access, work…
Cmux Expensive Synchronous Load ✅ Passed The diff does not add or move an expensive synchronous agent-history load. CLI/cmux.swift only adds environment parsing and clamps the existing socket startup timeout before passing it to the existi…
Cmux Cache Substitution Correctness ✅ Passed PASS: The production Swift diff adds an environment-controlled timeout getter and uses it for socket startup waiting. It does not replace an authoritative read with a cached or opportunistic value, an…
Cmux No Hacky Sleeps ✅ Passed PASS: The pull request changes only CLI/cmux.swift and cmuxTests/CMUXCLIErrorOutputRegressionTests.swift. The custom check applies to production non-Swift TypeScript, JavaScript, shell, and build/…
Cmux Algorithmic Complexity ✅ Passed The production diff adds only scalar environment parsing and min/max clamping for the timeout. It performs no collection scan, nested lookup, sorting, filtering, join, or per-target rescan. The test c…
Cmux Swift Concurrency ✅ Passed PASS: The diff adds only synchronous environment parsing and clamping for restoreSocketStartupTimeoutSeconds, plus a test environment variable. It introduces no background Dispatch queue, Combine st…
Cmux Swift @Concurrent ✅ Passed PASS: The diff adds only a synchronous computed timeout property and an environment assignment in a synchronous test. It adds no async, nonisolated, @MainActor, or @concurrent declarations. Th…
Cmux Swift Package Boundaries ✅ Passed PASS. The production diff adds a small private environment-variable timeout getter in CLI/cmux.swift and passes it to the existing CLI socket startup wait. This is CLI startup/lifecycle composition,…
Cmux Swiftpm Lockfiles ✅ Passed The pull request changes only CLI/cmux.swift and cmuxTests/CMUXCLIErrorOutputRegressionTests.swift. It does not change a SwiftPM package manifest, Package.resolved, an Xcode project package refe…
Cmux Swift Logging ✅ Passed The diff adds no print, debugPrint, dump, NSLog, file logging, or Logger declarations. It only adds environment-based timeout parsing in CLI/cmux.swift and sets that environment variable i…
Cmux User-Facing Error Privacy ✅ Passed The diff does not add or alter user-facing error text. It only reads CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS to shorten an internal startup wait and adds a test-only environment setting. The ide…
Cmux Full Internationalization ✅ Passed PASS. The production diff changes only the restore socket timeout selection and adds the literal configuration key CMUX_RESTORE_SOCKET_STARTUP_TIMEOUT_SECONDS; it adds no user-facing Swift text, loc…
Cmux Swiftui State Layout ✅ Passed PASS: The diff changes CLI timeout parsing and test-process environment setup only. It adds no SwiftUI views, ObservableObject/@published state, GeometryReader, lazy/list row store references, or rend…
Cmux Architecture Rethink ✅ Passed The diff does not introduce a new sleep, polling loop, lock, observer, dispatch delay, duplicate action path, or split UI owner. It changes the existing restore socket wait from a fixed 45-second cons…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The PR changes only CLI timeout handling and test environment setup. The diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, window identifier, or close-shortcut cod…
Cmux Source Artifacts ✅ Passed The diff changes only CLI/cmux.swift and cmuxTests/CMUXCLIErrorOutputRegressionTests.swift. Both are intentional hand-written source and test files. The changes add an environment-controlled timeo…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS. The PR changes only CLI/cmux.swift and cmuxTests/CMUXCLIErrorOutputRegressionTests.swift. Neither path matches the check scope of a Swift file under **/Sources/**. The production change ad…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 OpenGrep (1.30.0)
CLI/cmux.swift

OpenGrep scan timed out


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.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 25, 2026 00:55
@teamleaderleo
teamleaderleo merged commit fa353a8 into main Sep 25, 2026
59 of 60 checks passed
@teamleaderleo
teamleaderleo deleted the test/cli-restore-startup-timeout branch September 25, 2026 01:14
rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
e20651a ci: accept a bare count or a class in CI_OWNED_POOL_SLOTS (manaflow-ai#14340)
b9060a0 ci: route CI helper and ci-macos.yml edits to the lanes that run them (manaflow-ai#14339)
d7a119f ci: declare the queue janitor's workflow_run source file (manaflow-ai#14345)
b11c6d9 ci: retry a refused owned job once on the fleet before Blacksmith (manaflow-ai#14325)
622e64e ci: keep the owned-pool snapshot fresh when the janitor cron drifts (manaflow-ai#14341)
fa353a8 test: let the CLI no-socket test shorten the restore startup wait (manaflow-ai#14334)
cb54edf ci: place each PR macOS job on a free owned mini, overflow the rest (manaflow-ai#14318)

# Conflicts:
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci-queue-janitor.yml
#	.github/workflows/ci.yml
#	.github/workflows/cli-pipe-regressions.yml
#	.github/workflows/remote-daemon.yml
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Measured effect on CI after merge (01:14Z, 2026-09-25), from ci.yml "app-host unit tests (4/7)" jobs, test times read from the job logs.

before after
window (run created) 2026-09-24 16:29Z to 2026-09-25 01:03Z 2026-09-25 01:16Z to 02:43Z
runs 28 5
testLaunchCapableCommandsReachTheirDispatchPathWithoutLiveImplicitSocket median (range) 90.4 s (90.2 to 90.6) 2.4 s (2.35 to 2.54)
CMUXCLIErrorOutputRegressionTests suite median 120.3 s 33.2 s
shard 4 "Run unit tests" step median (range) 331 s (288 to 554) 263 s (254 to 305)
shard 4 job median 653 s 590 s

Correction to the PR body: this test runs in shard 4's "Run unit tests" step, not in "Run CLI no-socket regressions". That step stayed at 218 to 252 s before and after.

Six runs (three on each side, e.g. 36035977168, 36087977915) ran the test in 0.6 to 0.8 s and the suite in 8 to 13 s; they are a different selection and are excluded from both columns. Other merges landed in the same windows, so the step and job medians are not all attributable to this change; the test time is.

Before run ids: 36027687674 36032040988 36036314182 36041784298 36043411778 36048715431 36049292327 36051706301 36055048824 36056333476 36056381503 36056804560 36058204322 36059281883 36061664233 36062245739 36065917642 36065922591 36067111220 36069812059 36070257306 36071110988 36074615385 36074752682 36075060865 36077788169 36079476920 36080335136
After run ids: 36081295482 36083450833 36085429780 36085886212 36087415499

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant