Skip to content

acpmux: bound peer starts and reads without blocking last on subscriptions - #26998

Merged
cmux-bot[bot] merged 15 commits into
feat-cmux-nextfrom
lane/hb-peer-deadlines
Nov 3, 2026
Merged

cmux-bot[bot] merged 15 commits into
feat-cmux-nextfrom
lane/hb-peer-deadlines

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Status: awaiting final-head regression proof/review (#27048 has landed).

Connected peers could hold interactive requests for ten minutes. This change bounds forwarded history reads at 30 seconds and forwarded session creation just past the peer's own whole start budget (300 seconds, ACPMUX_PEER_START_SECS), so a slow valid start answers instead of being dropped and orphaned, gives acpmux last one 30-second budget across session resolution and pagination, and collects briefs concurrently under one shared 10-second deadline.

last previously resolved through _acpmux/info, then used _acpmux/attach before paging _acpmux/events. It now reads a newest-events snapshot without opening a live subscription on the shared peer link. A running session can return its latest partial reply immediately. These read methods do not intentionally wait for the turn to finish; the observed deployed stall was not traced to a specific remote RPC. The confirmed problems are the unnecessary attachment and the generic 600-second read deadline.

Leo: "I do a hell of a lot of routing to remote machines, right? ... Are we covering the edge cases for when, you know, something isn't going right, if there's recovery, whatever, stuff like that."

Additional request (private host identifiers omitted): "acpmux last / hangs past 30 s ... while those peers show "connected", and the same acpmux last run on the host answers in under a second (a running session, turn in progress). Find where the forwarded read waits ... and give it the same bounded deadline and readable error; a running session's last reply should come back at once. Include a test."

The peer-origin briefs filter from #26912 and dead-peer fast refusal remain intact. Prompts and explicit waits retain their existing turn-length behavior. Each deadline ends when its request answers, fails or times out; cancellation removes pending peer replies through the existing guard. There is no retry loop or new background worker.

Live _acpmux/attach retains its existing subscription lifecycle and timeout; the new server read budget applies to _acpmux/info, _acpmux/events, and _acpmux/history.

Risk: only a peer whose handler hangs past its own start budget reaches the creation timeout; such a creation may still complete there, so its error asks the caller to inspect that host before retrying. The history reads are safe to retry. Existing policy and folder-recovery hints remain intact.

Validation:

  • Current head: 6e6cddda9f5ba66763dc5f9e689c2a7a880ddb6f, stacked on reviewed perf(acpmux): stream session summaries into list snapshots #27048 head 9d674ff806d5165422c7e571b3c0bc0f8496ba87; final review and green 40776972768 pending.
  • Final red proof 40776959687 restores only this PR's five implementation files to the stack base and runs both regression suites; pending. Earlier run 40775724345 found a fixture task-output compile mismatch, fixed at current head; it is not regression evidence.
  • Dispatch 40775653375 raced GitHub's PR-head update and pinned the prior head; it is not regression evidence.
  • Prior creation/briefs proof: red 40771648763, green 40772350563, five tests at prior head ec9b0e1.
  • New tests exercise the real CLI against a connected fake peer: partial reply while running without an attachment, hung lookup timeout, and info/attach/events timeout with pending-request cleanup.
  • Formatting and repository pre-push checks pass. Independent Astra high/fast review passed before the clean stack rebase; a review of the final rebased head is pending.
  • External signing-runner guard: 40774296224 reproduces the exact failure using byte-identical base workflow code and its reported nonsecret input. This is a controlled base-code reproduction, not a fresh base-workflow run. No workflow or repository settings changed.
  • pr-overlap was attempted; its GraphQL call is unsupported by the broker. Searches found no duplicate handler fix. The list-streaming implementation is untouched.

Agent session: agent session

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

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

@teamleaderleo
teamleaderleo marked this pull request as ready for review October 28, 2026 21:44
@teamleaderleo teamleaderleo changed the title acpmux: bound remote session starts and parallel brief listing acpmux: bound peer starts and reads without blocking last on subscriptions Oct 28, 2026
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 4ba421e: 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=pr26998 -f ref=lane/hb-peer-deadlines -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

@cursor

cursor Bot commented Oct 28, 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.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Orphaned: no activity for 12 h, and no lane holds a lease on it. The orphan sweep could not merge feat-cmux-next in: conflicts in cmux-tui/crates/acpmux/src/hub/list_guard.rs, cmux-tui/crates/acpmux/src/hub/peers.rs, cmux-tui/crates/acpmux/src/hub/remote_rows.rs, cmux-tui/crates/acpmux/tests/daemon_memory.rs. Listed for a lane to resolve.

@teamleaderleo

teamleaderleo commented Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator Author

Owned by an agent session · took over from an earlier session · session

@teamleaderleo teamleaderleo removed the risk:high Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
cmux-bot[bot]
cmux-bot Bot previously approved these changes Nov 2, 2026

@cmux-bot cmux-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmux-bot approves b141a41: an independent review passed this exact head (next-review receipt by teamleaderleo). A push needs a new review; a later BLOCK at this head dismisses this approval.

@github-actions github-actions Bot added the risk:high Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 20:26 Inactive
@cmux-bot
cmux-bot Bot dismissed their stale review November 2, 2026 20:27

A newer verdict blocks 179c9bf (no independent SHIP at the head).

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol high, sweep, via the review router; full diff from merge base 35e4794 to this head): BLOCK at 179c9bf

[P2] [blocking] files/cmux-tui/crates/acpmux/src/server/remote_guard/hint.rs:69 - Dropping session/new after 30 seconds lets the remote daemon finish creating a session whose response is discarded, so slow valid starts (the normal startup budget is 90 seconds) surface as failures and retries can leave duplicate orphan sessions. Preserve/reconcile the request or make creation idempotent, using the caller/remote startup deadline.

The 30-second bound on a forwarded session/new dropped slow valid starts
(a pooled start may take a claim wait plus three 90-second stages), so
the caller saw a failure while the session finished on the peer, and a
retry could leave a duplicate orphan. The deadline is now the peer's
whole session/new budget plus 15 seconds (LinkTimings::start, 300 s,
ACPMUX_PEER_START_SECS to override): the peer answers a slow start or
fails it itself first, and only a hung handler reaches the timeout.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Agent-Session: https://cmux.com/deeplink/agent?id=aid-159afad10a5ab45ea16c
@cursor

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

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6-astra high, next-review): BLOCK at 5644313

BLOCK at 5644313
Visual verdict: no images; Leo tries the build

  1. [P1] [blocking] server/remote_guard/hint.rs:71: The 300-second deadline can still orphan valid starts. Remote-origin sessions skip the pool (hub/pool/mod.rs:672); cold starts use configurable agentStartTimeoutMs (hub/lifecycle.rs:835), not the pooled-stage budget used to calculate this timeout. A peer configured for 360 seconds whose harness answers after 310 seconds succeeds under the base’s 600-second deadline, but now reports failure at 300 seconds and subsequently creates the session remotely. Preserve starts within the peer’s actual allowed budget and add a regression exercising the real creation lifecycle; the current numeric budget assertion cannot catch this.

@teamleaderleo teamleaderleo removed the risk:high Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@github-actions github-actions Bot added the risk:high Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (codex gpt-6.1-sol high, sweep, via the review router; full diff from merge base ed4bcc4 to this head): SHIP at 4ba421e

No P1/P2 findings.

@cmux-bot
cmux-bot Bot merged commit 18afb7e into feat-cmux-next Nov 3, 2026
197 of 198 checks passed
@cmux-bot
cmux-bot Bot deleted the lane/hb-peer-deadlines branch November 3, 2026 16:54
@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review (claude opus-5-5 medium, cross-family): SHIP at 4ba421e

Visual verdict: unchanged by this diff

Read the full diff and the _acpmux/events handler at this head: beforeSeq: u64::MAX stays u64 through events_page, so last gets the newest transcript page without attaching. The older-page loop and Transcript ordering are unchanged. The 30 s forwarded-read bound covers info/events/history only, and attach keeps its lifecycle. The forwarded session/new deadline (300 s) outlasts the peer's own claim-plus-three-stage budget, and dropping either future releases its pending reply, which the new tests assert. I found no compile, data or regression blocker. (The PR was already merged when I read it, and this review is at its merged head.)

Follow-ups: [follow-up] A large history page (_acpmux/events/_acpmux/history with a big maxBytes) on a slow but healthy tunnel could now fail at 30 s where it used to finish. Consider scaling the read bound with the page size, or making it configurable like ACPMUX_PEER_START_SECS. [follow-up] The fixed 30 s last budget includes local paging; a very long local transcript is unlikely to hit it, but no test covers that.

This branch was previously deployed

3 inactive deployments
ci-routing-agent — 4ba421e7 Deployed Nov 3, 2026 by github-actions[bot] via cmux-next macOS placement #66458
artifacts — 4ba421e7 Deployed Nov 3, 2026 by teamleaderleo via publish tree-addressed cmux-tui #106434
sentry-dsyms — 4ba421e7 Deployed Nov 3, 2026 by teamleaderleo via upload cmux-tui dSYMs to Sentry #106434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:high Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants