Skip to content

Cloud: a hold answered on the phone is known from the feed push, not a poll (#32354) - #32368

Merged
cmux-bot[bot] merged 9 commits into
feat-cmux-nextfrom
cloud-hold-race
Nov 2, 2026
Merged

cmux-bot[bot] merged 9 commits into
feat-cmux-nextfrom
cloud-hold-race

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #32354. Follow-up to #32271, #32300, #32336 and #32270.

Cause

The person answers a Cloud create's approval hold on the phone. The backend closes the approve item in the person's FeedDO in that commit, and the feed owner pushes the event to the Mac's feed mirror at once. But FeedWireDecode dropped the item's action.input.approval, so the Mac could not link the closed item to its create. When this Mac then answered the hold, the approved op still read pending, no open item carried the request, and the reply was .mismatch(reason: "feed item or team"). That reply also means "this item belongs to another team".

#32270 (landed while this PR was open) removed the sheet for a person's create, and now sends every closed-item mismatch to the same-key retries. That keeps the phone case going, but it also excuses a true team mismatch: such a create retries into stillPending instead of failing with the mismatch. The issue asked for exactly that difference.

Fix (Mac only, no server change)

  • CmuxNextFeed keeps the approve action's hold (Approve.Action.hold: request, team). FeedItem.cloudHoldRequest reads it only for a cloud item that its team's Cloud owner (system:cloud:<team>) posted, which is the backend's approvalOf rule.
  • CmuxNextCloud CloudApprovalHolds records every closed Cloud hold (request and team) that the feed mirror sees (snapshot and events, FeedService.observe). The push is how the Mac learns of the phone's answer: no poll and no timer.
  • CloudApprovalAnswer (CmuxNextDaemonService):
    • It answers the new .answered(request:) when the feed already shows the create's own team's item for the request closed (no lookup, no answer). Another team's closed hold on the feed counts for nothing here, so the normal checks run.
    • It also answers .answered when no open item carries the request but this team's closed item does (same op, digest, team, poster scope). This covers an answer that comes before the push.
    • Another team's item, or no item, is still .mismatch(reason: "feed item or team").
  • The flow sends only .answered to cmux NEXT Cloud: a person's own Cloud send creates the machine with no second approval prompt #32270's answered-elsewhere path (same-key retries, nothing recorded). A true team mismatch fails at once again. The two judgments (alreadyAnswered, answeredElsewhere) moved from CloudMachineCreateFlow to CloudApprovalAnswer.Failure.

App lines: -12 (arch-ratchet.py --base <merge-base> --head HEAD).

Tests

CloudApprovalHoldRaceTests (no sleeps; it drives the real CloudApprovalAnswer over a fake session backend):

  • aHoldAnsweredOnThePhoneFirstGoesOnWithTheOpsAnswer: the create goes on with the op's answer, answers nothing, and records nothing.
  • aHoldOfAnotherTeamFailsAtOnce (answered or not): the create fails with the mismatch. On feat it retries into stillPending: this is the red.
  • aHoldTheFeedShowsAnsweredIsNotAskedAboutAgain: after the push, no session call is made.
  • anotherTeamsHoldTheFeedShowsAnsweredStillFailsAtOnce: the feed shows team_b's hold answered, and the create is team_a's. The create still fails with the mismatch, after the normal lookup (review finding at ce2f694).
  • theFeedKeepsTheCloudHoldOfItsOwnTeamsItemOnly: the decode and the poster-scope rule.

CloudPersonCreateApprovalTests (#32270): its answered-elsewhere fixture now throws .answered, which is what CloudApprovalAnswer returns for that case. CloudCreateRetryKeyTests: it now calls the moved judgments and has two new cases (this team's closed item versus another team's).

Command: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --ref SHA --arg=suite --arg=Packages/macOS/CmuxNext --arg=SUITES

  • Red on feat: branch cloud-hold-race-red2 (f28f673 = the feat merge base 2f1caeb plus the two tests that compile on feat): job 77b23e7ba2106602adbcd526 failed as expected. aHoldOfAnotherTeamFailsAtOnce gets the wrong error (stillPending) when the hold is not answered, and no error at all when it is answered on the phone. aHoldAnsweredOnThePhoneFirstGoesOnWithTheOpsAnswer passes on feat.
  • Head 4cca854, suites CloudApprovalHoldRaceTests, CloudCreateRetryKeyTests, CloudPersonCreateApprovalTests, CloudEnvironmentBootFlowTests: job 35f7e10a299e092ef1b5c01c, all passed (5, 14, 5 and 4 tests)
  • App compile at 4cca854: job 115204a9572e53c945b366da passed
  • Red for the team check on the push path: branch cloud-hold-race-red3 (9b8753c = ce2f694 plus the new test, written against the old API), job 5e3c32a2cd7decf27a48df85. anotherTeamsHoldTheFeedShowsAnsweredStillFailsAtOnce fails: no error is thrown, and sessionCalls is 0.
  • Before the merge (fd6359c, with the sheet design): green 5/5 (eecebdede0f9c35cbeb711ac); red at ee959d0 (3810740ba612543d25004587).
  • prepush-gate.py: no new violations. Arch ratchet: app_lines -12.

Observed / Not run

  • Observed: the fleet jobs above, the gate and the arch ratchet.
  • Not run: an end-to-end run on a DEV build with a real phone answer against staging. The post-merge suites ran once, not 5 times. No local compile or test.

Changelog

  • Fixed: A Cloud machine create no longer fails when you approve it on your phone first, and a create held for another team fails at once with the right error.

No capture: there is no UI change.

…not fail the create (#32354)

Red first: answered on the phone, then yes in the Mac sheet before the
next watch check. The approved op still reads pending, its feed item is
closed, so the answer finds no open item and the create fails with the
closed-item mismatch. A hold of another team must still fail.
…h, not a poll (#32354)

The person answers a Cloud create's approval hold on the phone; the
approve item in their FeedDO closes in that commit, and the owner pushes
the event to the Mac's feed mirror at once. The feed decode dropped the
item's approval reference, so the create flow could not use it and
waited for its same-key watch (1 to 8 s). A yes in the sheet before that
check answered the hold again and failed with the closed-item mismatch.

- CmuxNextFeed keeps the approve action's hold ({request, team}) and
  FeedItem.cloudHoldRequest reads it only for the team's Cloud owner's
  cloud item.
- CloudApprovalHolds (CmuxNextCloud) records each closed hold from the
  feed mirror; CloudApprovalSheet (moved out of the App flow) ends the
  sheet on the push, the click or the watch, whichever comes first, and
  reads a yes against the hold's state.
- CloudApprovalAnswer: when no open item carries the request, the same
  team's closed item means answered elsewhere (.answered); another
  team's item still fails with the mismatch.

App lines: +24 -34.
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Nov 2, 2026
@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.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

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

Review 4cca854: 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=pr32368 -f ref=cloud-hold-race -f platform=macos -f backend_mode=local

Default changes

This pull request changes 2 defaults at db9193a9b913 (what everyone gets without configuring anything):

  • tunable-source.CmuxNextFeed/Wire/FeedWireDecode.swift:hold: (none) -> "1c53a2262c8f60f76374ef0b0ed5c889d72a79162f2d05c3efec0510efafa858"
  • tunable-source.CmuxNextFeed/Wire/FeedWireDecode.swift:prompt: "aa38b40b6c8d724c2c3d6b8ebc831b5ac213af099091d2243f803b4e83af502f" -> "5785a8f55704ab6b90d94dc6c13b3c2c02409004dc93c32ff8bdd78c7117b72e"

Signed off on this commit by review (claude review subagent), carried from 4cca854a7d7d across base merges.

Merge receipt: merged as 70d801cabc51 with these defaults changed, signed off by review (claude review subagent), carried from 4cca854a7d7d across base merges.

Regenerate the snapshot with python3 scripts/ci/defaults_guard.py snapshot.

Shipped in:

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude review subagent): SHIP at eb85e7e

I read the code only. I did not compile or run tests. eb85e7e is 6e02d93 plus public import Observation in CloudApprovalHolds.swift. Other CmuxNextCloud @Observable types (CloudAuth, ActiveTeam, CloudVmHostStore) use the same import.

What I checked:

  • A real team mismatch still fails. .answered needs a closed item that matches the same request, op, digest and team, with poster.kind == integration and scope system:cloud:<team>. This is the same item(carrying:) rule as the open-item check. Another team's item, or no item, still throws .mismatch(closedItem). aHoldOfAnotherTeamStillFails covers both cases.
  • Backend behavior. feed.list state: "closed" returns i.state !== "open", so it includes answered, cancelled and expired items. order: "recent" sorts newest first. Archived and snoozed items are left out, so those cases fall back to the mismatch, which is safe.
  • A hold denied on the phone. .answered and .answeredElsewhere never answer the hold. They only set approved and retry with the same key. The phone's deny reaches CloudDO through the outbox, so the same-key call returns approval.pending for a short time and then approval.denied. The loop's catch handles only approvalPending, so the denial is thrown out and the create fails with it. It does not hang and it does not succeed. A Retry with the same key replays the denial.
  • CloudApprovalSheet / CloudApprovalHolds. Everything runs on the main actor and nothing blocks. Every task ends when it is cancelled:
    • firstAnswer cancels the losing task when it returns and when its caller is cancelled.
    • ObservationStream returns nil on cancel, so answer(of:) returns false.
    • The same-key watch stops when its pause throws.
    • When a watch gives up (returns false), it maps to nil and does not end the sheet. A click that comes in the same turn as the push is read as answered elsewhere. A hold that is already answered shows no sheet.
  • FeedService wiring. The holds are recorded from the confirmed snapshot and events only, never from the overlay. cloudHoldRequest follows the backend approvalOf rule: an integration poster, a system:cloud:<team> scope that matches input.approval.team, and a cloud home.
  • Tests. No sleeps. The watch check is an AsyncStream that ends only on cancel. At 3b164c7, the first test reaches .mismatch(closedItem) with answeredEarlier == false, so it is red on the base. The three tests that use the push go through the real flow. CloudApprovalAnsweredElsewhereTests (watch path, holds nil) and the alreadyAnswered cases in CloudCreateRetryKeyTests are unchanged and still match the code.
  • Repo rules. No try!, no new CI gate. The sheet race moved out of CmuxNextApp into CmuxNextCloud.

Non-blocking notes:

  1. The closed-item check does not look at why the item closed (answered, denied, expired or cancelled). For a hold whose feed item was cancelled while CloudDO still shows it pending, a Mac yes now waits through about 47 s of same-key retries and ends in stillPending. Before, it failed at once with the mismatch. Both outcomes are failures, and this case is rare.
  2. The closedItem doc comment ("closed, or another team's") and the type's step-2 doc comment still describe the old rule. Now an own-team closed item gives .answered.
  3. FeedItem.cloudHoldRequest does not require digest in the item. The backend's approvalOf does. This is harmless here, because the closed-list check in CloudApprovalAnswer does compare the digest.
  4. The PR body's fleet evidence (test 5/5 green, app compile) is for 6e02d93. Please add a run at eb85e7e, because the head moved to fix a build failure.

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

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude review subagent): SHIP at fd6359c

My earlier review at eb85e7e still holds. I read only the change from eb85e7e to fd6359c. I did not compile or run tests.

  • Test stub nextWatchCheck. It now uses AsyncStream<Void>.makeStream() and stores the continuation. The loop still ends only when the task is cancelled, because an AsyncStream iterator returns nil on cancellation. Then the stub throws CancellationError. This is still deterministic, with no sleeps.
  • Doc comments in CloudApprovalAnswer.swift. The type doc now names the .answered case. The closedItem doc says "another team's, or none". Both now match the code.
  • Red proof. ee959d0 differs from 3b164c7 only by the same stub fix (git diff 3b164c78 ee959d0d). So the red run (job 3810740ba612543d25004587, closed-item mismatch) is a real behavioral red, not a compile failure. The first red result for 3b164c7 (job 50f237ea) came from a test that did not compile, so it was not valid proof. ee959d0 replaces it.

The non-blocking notes in my earlier comment still apply: a closed item counts as answered whatever its close reason, and cloudHoldRequest does not require a digest.

@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 2, 2026
…race

# Conflicts:
#	Packages/macOS/CmuxNext/Sources/CmuxNextApp/Cloud/CloudMachineCreateFlow.swift
#	Packages/macOS/CmuxNext/Sources/CmuxNextApp/Cloud/CloudService+AppServer.swift
…closed item mean answered; another team's still fails (#32354)

After merging #32270 (no sheet for a person's create; every closed-item
mismatch went to the same-key retries):
- CloudApprovalAnswer answers .answered(request) for this team's closed
  item and for a hold the feed push already shows answered (no lookup,
  no answer); another team's item stays the closed-item mismatch.
- The flow's answered-elsewhere path takes only .answered, so a true team
  mismatch fails at once again instead of retrying into stillPending.
- alreadyAnswered/answeredElsewhere move to CloudApprovalAnswer.Failure
  (CmuxNextDaemonService); CloudApprovalSheet goes (no sheet to race).
@lawrencecchen lawrencecchen removed 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 2, 2026
…'s own team (review on #32368)

CloudApprovalHolds keeps each closed hold's team, FeedItem.cloudHold
carries it, and CloudApprovalAnswer's feed predicate takes the create's
team: another team's closed hold on the phone still ends in the
closed-item mismatch. Test: anotherTeamsHoldTheFeedShowsAnsweredStillFailsAtOnce.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude review subagent): SHIP at 4cca854

I read the whole PR at this head, which merges feat and keeps #32270's flow with no create sheet. I also read the fix commit since ce2f694. I did not compile or run tests.

  • My blocking finding at ce2f694 is fixed. CloudApprovalHolds now keeps [request: team], and isAnswered(_:team:) matches only the same team. FeedItem.cloudHold returns the item's (request, team) only when its poster scope is system:cloud:<that team>. CloudApprovalAnswer.checkedApprove asks answered(request, team) with the create's own team. So another team's closed hold no longer skips the team check: it goes through the server lookup and fails with .mismatch(closedItem). The new test anotherTeamsHoldTheFeedShowsAnsweredStillFailsAtOnce covers it, and the fleet red at 9b8753c (ce2f694 + that test) shows the old code failed it.
  • No stale callers. git grep finds no other users of cloudHoldRequest or the old one-argument isAnswered.
  • Closed items that were denied, cancelled or expired, counted as .answered, do no harm. The Mac answers nothing and records nothing; only same-key retries follow. A denied hold returns approval.denied, which the create throws. An expired hold returns approval.expired. A cancelled item whose hold stays pending ends in stillPending, which cmux NEXT Cloud: a person's own Cloud send creates the machine with no second approval prompt #32270's aClosedItemWhoseHoldStaysPendingEndsStillPending covers.
  • cmux NEXT Cloud: a person's own Cloud send creates the machine with no second approval prompt #32270's intent is kept. A person's own create shows no second prompt, and a hold answered elsewhere records nothing. answeredElsewhere matches only .answered, so another team's .mismatch(closedItem) fails at once again. That closes the gap cmux NEXT Cloud: a person's own Cloud send creates the machine with no second approval prompt #32270's itemClosed opened. alreadyAnswered and answeredElsewhere are correct in their new place on CloudApprovalAnswer.Failure.
  • Merge: the head merges cleanly into the feat tip.

Non-blocking: CloudApprovalHolds is never cleared at sign-out or an account change. This is harmless, because request ids are random and the match now needs the team too.

@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 2, 2026
@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.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 16:20 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 16:20 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 2, 2026 16:21 Inactive
@cmux-bot
cmux-bot Bot merged commit 70d801c into feat-cmux-next Nov 2, 2026
152 of 154 checks passed
@cmux-bot
cmux-bot Bot deleted the cloud-hold-race branch November 2, 2026 16:32

This branch was previously deployed

1 inactive deployment
ci-routing-agent — db9193a9 Deployed Nov 2, 2026 by cmux-bot[bot] via wait for same-key tree publisher #101972
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:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant