Skip to content

base red: Takes back #28786: the signed-out Cloud action sign-in recovery moves to CmuxNextDaemonService (app_lines +65) - #29589

Merged
lawrencecchen merged 4 commits into
feat-cmux-nextfrom
base-red/signin-recovery-move
Oct 31, 2026
Merged

lawrencecchen merged 4 commits into
feat-cmux-nextfrom
base-red/signin-recovery-move

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Base red, priority zero (tip arch red; lead 18:5xZ). Run on each first-parent commit since the last green arch tip b6eb849, the repo rule (arch-ratchet.py --base <c>^ --head <c>) refuses three commits:

The tip over b6eb849 is app_lines +139, refused.

Takes back #28786: its own sign-in recovery code leaves CmuxNextApp. The behavior is unchanged.

  • CloudHandlers+Account.swift's 53-line extension moves to CmuxNextDaemonService as CloudSignInRecovery, next to CloudStrings and the registry types it uses. The extension holds the exempt set, signInRecovers, bindSignInRecovery's replay and resumeAfterSignIn.
  • The App keeps CloudHandlers.bind unchanged, so it still calls bindSignInRecovery(into:cloud:). That is now a one-line App wrapper that builds CloudSignInRecovery with the Cloud service's reads (the pending continuation, sign-in, hasCurrentList(within: 30 s), isSignedIn, refresh). The defaults guard tracks CloudHandlers.bind, so this PR leaves it as it was.
  • CmuxNextDaemonService now declares its CmuxNextCloud dependency (service -> service; layer-gate ok).
  • CloudSignInRecoveryTests read CloudSignInRecovery.recovers.

CmuxNextApp -51. #28786's DEBUG-only fixture lines in CloudService stay in the App, so this PR alone covers 50 of its 65. The four base-red PRs together clear the row (see Testing).

Testing

The watcher's command (arch-ratchet.py --base b6eb84916c1a, row app_lines):

prepush-gate.py: no new violations. The namespace lint: OK.

Observed / Not run

Observed:

  • The checks above.
  • At f466009, the fleet compile failed: exempt was main-actor and was read from the nonisolated recovers. Fixed in 4b8e80c: exempt is now a nonisolated constant.
  • At 4b8e80c: the compile (job e2e7c51fdaa7a0c26b8c11a7) passed, and CloudSignInRecoveryTests 3/3 passed (step 5acf77d8b33b3aceb98fa388).
  • At this head b48a3a2: compile job 8a42c768108ce1acc732d024 and suite step 5997cec2ae059cf653c709a4 are pending. Their results are posted as a comment.
    Not run: no local Swift build (fleet only). No tagged build: no behavior change.

Changelog

none

Proof

No capture: no visible change; the recovery moves between targets with the same behavior.

…very moves to CmuxNextDaemonService (app_lines)

ac59cd2 (#28786, "signed-out Cloud actions sign in, then run") adds 65
CmuxNextApp Swift lines with no exception row; arch-ratchet's app_lines row
refuses it alone (--base ac59cd2^ --head ac59cd2: +65), part of the
tip's red (STATUS, lead 18:5xZ).

#28786's own recovery code (CloudHandlers+Account.swift's 53-line extension:
the exempt set, signInRecovers, bindSignInRecovery's replay and
resumeAfterSignIn) moves unchanged in behavior to CmuxNextDaemonService as
CloudSignInRecovery, next to CloudStrings and the registry types it uses.
The App's CloudHandlers builds it with the Cloud service's reads (pending
continuation, sign-in, hasCurrentList(within: 30 s), isSignedIn, refresh)
and binds it where it bound the extension. CloudSignInRecoveryTests read
CloudSignInRecovery.recovers.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Oct 31, 2026
…ble set), so the nonisolated recovers(_:reason:) reads it

The fleet compile at f466009 (step 4ef24f9505a217c46278ab28) refused
`exempt` from the nonisolated `recovers`: in CmuxNextApp the static was
main-actor by the module's default isolation; in the @mainactor struct it
kept that isolation. A Set<ActionID> literal is immutable Sendable data.
@github-actions

github-actions Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

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

Review b48a3a2: 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=pr29589 -f ref=base-red/signin-recovery-move -f platform=macos -f backend_mode=local

Default changes

No defaults change at e376b4e6ab74.

Shipped in:

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:low Risk tier (docs/ci/risk-tiers.md) labels Oct 31, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at 4b8e80c

Reviewed f466009 (the move) plus 4b8e80c (exempt made nonisolated), against the removed extension at HEAD~1:CloudHandlers+Account.swift.

Behavior: the same as before

  • Exempt set: the same (palette.auth.signOut, openTeamPicker). recovers uses the same rule (reason == CloudStrings.signInFirst && !exempt.contains(id)).
  • The same pending (cloud.signedOutCreation), the same reserve() on the run's own turn, and the same pending.perform(expected:signIn:resume:) inside the same Task { @MainActor in } with the task-owner comment.
  • Sign-in, list read and retry loop: the same calls, now through closures. { await cloud.auth.signIn() } (Bool), { cloud.hasCurrentList(within: .seconds(30)) }, { cloud.isSignedIn } and { await cloud.refresh() } (@discardableResult ... -> Bool). The for _ in 0..<2 where ... loop, the break and return true did not change.
  • Resume: the same guard let registry, isSignedIn(), then resume, which is resumeAfterSignIn word for word (unavailableReason -> refuse, else perform).
  • Lifetime: the strong graph is the same. Before, the closure in registry.unavailableRecoveries held cloud and pending strongly. Now it holds the struct copy self, whose closures hold cloud strongly, and pending. Registry stays weak. No new edge, so no new cycle and no lifetime change.

Compile (read only; no local Swift build, by the repo rule)

  • CmuxNextDaemonService uses uiSwiftSettings (default MainActor). The fleet error at f466009 (exempt read from the nonisolated recovers autoclosure) is fixed: public nonisolated static let exempt: Set<ActionID>, and ActionID is a nonisolated ... Sendable struct. CloudStrings.signInFirst and signInToContinue are nonisolated public static var.
  • That error came from the type-check stage, so SIL-stage diagnostics (definite init, region isolation) did not run at f466. I read them:
    • The tuple init of lets is a pattern used in many places in the package.
    • The Task { @MainActor in } captures are all Sendable or main-actor: the @mainactor struct, @mainactor closures, the MainActor CloudSignedOutCreationContinuation, and the Sendable ActionInvocation. They are captured from a MainActor context, as the old code did.
    • Implicit self in the nested closures is allowed for a value type.
  • public import CmuxNextActions and public import CmuxNextCloud are needed by the public API (ActionRegistry, CloudSignedOutCreationContinuation).
  • The App sees the type through DaemonServiceExport.swift's @_exported import CmuxNextDaemonService. CmuxNextAppTests already lists CmuxNextDaemonService.
  • No other references to the removed names are left (git grep).

Package edge

  • CmuxNextCloud depends only on Compat, Wakeups and auth products, so no cycle.
  • layer_gate.py --base HEAD~1 --head HEAD: ok.
  • arch-ratchet.py --base HEAD~1 --head HEAD: no rises.
  • prepush-gate.py --base d2089899a6b --head 4b8e80c2478: no new violations (app_lines -50).
  • check-package-dependency-lists.py: ok.

PR rules: the title starts with base red:, the body has Takes back #28786: ..., there is a risk:normal label, Changelog says none, and the PR gives a no-capture reason.

Not run

  • No Swift compile or test on this machine.
  • A green fleet compile at 4b8e80c is not yet confirmed here. The body's job 9ed94e63e0f4725af31bedcc is the failed f466 compile, so update the Observed section when the new run lands.

Nit (not blocking): recovers became nonisolated. The original was MainActor and every caller is MainActor, so this is harmless.

…rapper builds CloudSignInRecovery

The defaults guard fingerprints CloudHandlers.bind (a tunable-source
declaration) and reported its edit as a changed default. bind calls
bindSignInRecovery(into:cloud:) as before; that App wrapper, in
CloudHandlers+Account.swift, builds CloudSignInRecovery with the Cloud
service's reads. No behavior change.
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

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

I reviewed the full diff over the merge base d208989 (f466009, 4b8e80c and b48a3a2). The review is static only. I did no Swift compile or test on this machine.

Behavior: the same as the base

  • CloudHandlers.bind (CloudHandlers.swift) has no change. It still calls bindSignInRecovery(into: registry, cloud: cloud).
  • The new one-line wrapper gives CloudSignInRecovery the same reads that the removed body used: pending: cloud.signedOutCreation (read once at bind, as before), cloud.auth.signIn() (default selectAccount: false, as before), cloud.hasCurrentList(within: .seconds(30)), cloud.isSignedIn and await cloud.refresh().
  • bind(into:) keeps the same [weak registry] capture. It does the same reserve() on the run's turn and calls the same perform(expected:signIn:resume:). The for _ in 0..<2 where !hasCurrentList() && isSignedIn() loop is the same, and so are the guard let registry, isSignedIn() resume and the resume body (refuse a reason that still holds, else perform).
  • exempt and recovers are the same (palette.auth.signOut and openTeamPicker, reason == CloudStrings.signInFirst).
  • No reference to the removed signInRecovers, signInRecoveryExempt or resumeAfterSignIn remains (git grep over the repo).

Compile, from the code

  • The wrapper is a static func in extension CloudHandlers in the App (main-actor default). Its closures are @MainActor, as in the init's types. CloudSignInRecovery reaches the App through the @_exported import CmuxNextDaemonService in DaemonServiceExport.swift.
  • CmuxNextDaemonService -> CmuxNextCloud makes no cycle (CmuxNextCloud depends only on Compat, Wakeups and auth products). CloudSignedOutCreationContinuation is public and @MainActor.
  • recovers is nonisolated and reads the nonisolated exempt, which is the 4b8e80c fix. The fleet compile passed at 4b8e80c. The new head changes only App lines.
  • The test's added import CmuxNextDaemonService is redundant (the App re-exports it) but does no harm. CmuxNextAppTests lists CmuxNextDaemonService as a dependency.

Checks (run on this head over d208989)

  • defaults_guard.py check: exit 0. It tracks CloudHandlers.swift:bind, and that text has no change.
  • arch-ratchet.py: exit 0, app_lines -51, no count rises.
  • prepush-gate.py: exit 0. With no arguments it judged nothing, because the head is already on the remote. With --base d2089899a6b --head HEAD: exit 0, no new violations.

PR body: it describes the wrapper and bind with no change. It is current. The compile and suite results at this head are still pending in the body.

Nits (do not block)

  • The one-line wrapper is about 270 characters long. The gate accepts it.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet evidence at b48a3a2: compile with tests job 8a42c768108ce1acc732d024 (CmuxNext 244 s, app, cmux-tui passed). CloudSignInRecoveryTests 3/3 passed (step 5997cec2ae059cf653c709a4). defaults_guard.py check: exit 0.

@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 Oct 31, 2026
lawrencecchen pushed a commit that referenced this pull request Oct 31, 2026
…very moves to CmuxNextDaemonService (app_lines +65) (#29589)

merge-batch candidate: the squash merge of #29589 at b48a3a2
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 19:42 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent October 31, 2026 19:42 Inactive
lawrencecchen pushed a commit that referenced this pull request Oct 31, 2026
…very moves to CmuxNextDaemonService (app_lines +65) (#29589)

merge-batch candidate: the squash merge of #29589 at e376b4e
@lawrencecchen
lawrencecchen merged commit 5d6bb23 into feat-cmux-next Oct 31, 2026
157 of 162 checks passed
@lawrencecchen
lawrencecchen deleted the base-red/signin-recovery-move branch October 31, 2026 21:02

This branch was previously deployed

1 inactive deployment
ci-routing-agent — e376b4e6 Deployed Oct 31, 2026 by cmux-bot[bot] via namespace autofix #10110
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