From 0097233ea475ca7569594a20856fa108cabcfa1a Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 13:31:23 -0400 Subject: [PATCH 01/15] Make settled_at host-authoritative (settle-teardown step 0) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `terminal_sessions` is a CRR table, and iOS wrote `settled_at` into its own replica optimistically before sending the settle command. That write replicates upstream carrying no host lifecycle revision, so it can win a CRDT merge against a host that REJECTED the settle — filing a live session as done. It is a guard defeated by a merge rather than by a caller, which is why it has to close before the settle-writer chokepoint (step 1) can promise anything. iOS no longer writes the settle columns at all. `updateSessionLifecycle` becomes `updateSessionSnoozeOverlay`, scoped to the snooze/woke columns, which keep their optimistic write plus rollback because they guard no host decision. Instant feedback for settle now comes from `PendingSessionSettleStates`: a local, non-persisted overlay applied when session rows are read, resolved when the host's changeset confirms it, when the command fails, or by a bounded staleness backstop. `settle_source` is left out — no iOS surface reads it. Removing the write only fixes new builds, so the host enforces the rule: `syncHostService` drops inbound `terminal_sessions.settled_at` / `settle_override` / `settle_source` from PHONE peers, alongside the existing `sync_cluster_state` filter. Per-column and silent — the rest of the batch still applies and still acks ok, since a rejected ack would stall the peer's outbound cursor. Desktop peers are exempt on purpose: they run the same `sessionService` chokepoint, so their settle writes are host-decided and must keep replicating. A pre-fix phone now diverges locally instead of corrupting the host, and self-heals on the next `refreshWorkSessions`. Recorded as amendment 6 in the design doc. --- .../src/services/sync/syncHostService.test.ts | 139 ++++++++++++++- .../src/services/sync/syncHostService.ts | 38 +++- apps/ios/ADE.xcodeproj/project.pbxproj | 8 + apps/ios/ADE/Services/Database.swift | 38 ++-- .../Services/PendingSessionSettleStates.swift | 148 ++++++++++++++++ apps/ios/ADE/Services/SyncService.swift | 143 ++++++++++----- .../Views/Work/WorkRootScreen+Actions.swift | 11 +- .../PendingSessionSettleStatesTests.swift | 167 ++++++++++++++++++ .../sync-and-multi-device/ios-companion.md | 16 ++ .../features/terminals-and-sessions/README.md | 18 +- .../settle-teardown-design.md | 28 +++ 11 files changed, 683 insertions(+), 71 deletions(-) create mode 100644 apps/ios/ADE/Services/PendingSessionSettleStates.swift create mode 100644 apps/ios/ADETests/PendingSessionSettleStatesTests.swift diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index 7fb60dc0d..1ebef769d 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -6683,7 +6683,7 @@ describe("outbound changeset ack retries", () => { projectRoot: string, state: { dbVersion: number; changes: CrsqlChangeRow[] }, logger = createDiscoveryLogger(), - options: { pollIntervalMs?: number } = {}, + options: { pollIntervalMs?: number; applyChanges?: (changes: CrsqlChangeRow[]) => void } = {}, ) { const base = createHostArgs(projectRoot, []); const exportChangesSince = vi.fn( @@ -6709,7 +6709,10 @@ describe("outbound changeset ack retries", () => { getSiteId: () => "site-host-controlled", getDbVersion: () => state.dbVersion, exportChangesSince, - applyChanges: () => ({ appliedCount: 0 }), + applyChanges: (changes: CrsqlChangeRow[]) => { + options.applyChanges?.(changes); + return { appliedCount: changes.length }; + }, discardUnpublishedChangesForTables: () => {}, }, }, @@ -6721,6 +6724,138 @@ describe("outbound changeset ack retries", () => { return { host, logger, exportChangesSince }; } + /** + * `settled_at` is host-authoritative (settle-teardown design §3c-i). A phone + * on a build that predates the fix still writes it into its own CRR replica + * optimistically, and `terminal_sessions` replicates — so without this filter + * the phone's row merges upstream and settles a session the host *rejected*. + * The guard has to live here because a CRDT merge never reaches the caller + * the host-side check guards. + */ + describe("settle columns are host-authoritative against a phone replica", () => { + function settleChange(overrides: Partial = {}): CrsqlChangeRow { + return { + table: "terminal_sessions", + pk: "session-1", + cid: "settled_at", + val: "2026-08-10T00:00:00.000Z", + col_version: 1, + db_version: 1, + site_id: "site-phone", + cl: 1, + seq: 0, + ...overrides, + }; + } + + async function sendInboundBatch( + peer: Awaited>, + changes: CrsqlChangeRow[], + ) { + peer.ws.send(encodeSyncEnvelope({ + type: "changeset_batch", + requestId: "inbound-settle", + payload: { + batchId: "inbound-settle", + reason: "broadcast", + fromDbVersion: 0, + toDbVersion: 1, + changes, + }, + })); + return waitForValue( + () => peer.envelopes.find((envelope) => + envelope.type === "changeset_ack" + && (envelope.payload as SyncChangesetAckPayload).batchId === "inbound-settle"), + "inbound settle changeset ack", + ); + } + + it("drops a phone's settle columns while applying the rest of the same batch", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applied: CrsqlChangeRow[][] = []; + const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; + const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { + applyChanges: (changes) => applied.push(changes), + }); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + peer = await connectPeer(port, host.getBootstrapToken(), "phone-settle"); + + const ack = await sendInboundBatch(peer, [ + settleChange({ cid: "settled_at" }), + settleChange({ cid: "settle_override", val: "settled", seq: 1 }), + settleChange({ cid: "settle_source", val: "user", seq: 2 }), + // The snooze overlay is NOT host-authoritative — the phone owns its + // optimistic write there and it must keep replicating. + settleChange({ cid: "snoozed_until", val: "2026-08-11T00:00:00.000Z", seq: 3 }), + settleChange({ cid: "title", val: "renamed from phone", seq: 4 }), + ]); + + expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); + expect(applied).toHaveLength(1); + expect(applied[0]?.map((change) => change.cid)).toEqual(["snoozed_until", "title"]); + } finally { + peer?.ws.close(); + await host.dispose(); + cleanup(); + } + }); + + it("acks a batch that was entirely settle columns without applying anything", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applied: CrsqlChangeRow[][] = []; + const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; + const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { + applyChanges: (changes) => applied.push(changes), + }); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + peer = await connectPeer(port, host.getBootstrapToken(), "phone-settle-only"); + + // A silent drop must still ack ok, or the phone re-sends the same + // poisoned range forever: its outbound cursor only advances on an ok. + const ack = await sendInboundBatch(peer, [settleChange()]); + expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); + expect((ack.payload as SyncChangesetAckPayload).appliedCount).toBe(0); + expect(applied).toHaveLength(0); + } finally { + peer?.ws.close(); + await host.dispose(); + cleanup(); + } + }); + + it("keeps applying settle columns from a paired desktop peer", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applied: CrsqlChangeRow[][] = []; + const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; + const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { + applyChanges: (changes) => applied.push(changes), + }); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + // A desktop runs the same `sessionService` chokepoint, so its settle + // writes are host-decided too and must keep replicating. + peer = await connectPeer(port, host.getBootstrapToken(), "desktop-peer", { + platform: "macOS", + deviceType: "desktop", + }); + + const ack = await sendInboundBatch(peer, [settleChange()]); + expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); + expect(applied[0]?.map((change) => change.cid)).toEqual(["settled_at"]); + } finally { + peer?.ws.close(); + await host.dispose(); + cleanup(); + } + }); + }); + it("reseeds a far-behind iOS replica once, then resumes incrementally from the acknowledged watermark", async () => { const { projectRoot, cleanup } = createTempProjectRoot(); const state = { diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index 42fb1782e..fda52a21f 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -353,6 +353,35 @@ const SYNC_HOST_AUTHORITATIVE_TABLES = new Set([ const isHostAuthoritativeTable = (change: CrsqlChangeRow): boolean => SYNC_HOST_AUTHORITATIVE_TABLES.has(change.table); +/** + * Settle columns on `terminal_sessions`. The host decides these — a settle + * arrives as a `session.settle*` remote command and is written by + * `sessionService`, which is the only place that can weigh the decision against + * live work. + * + * A phone must not author them over CRR. `terminal_sessions` replicates, so a + * phone's optimistic `settled_at` carries no host lifecycle revision and merges + * in regardless of what the host decided: the host can *reject* a settle and + * still end up with a settled row. That is a guard defeated by a merge rather + * than by a caller, and no amount of host-side checking closes it. + * + * Current iOS builds no longer write these (they use a local pending-UI overlay + * instead — see `PendingSessionSettleStates.swift`), but a paired phone on an + * older build still does, so the host enforces it rather than trusting the + * client version. The drop is silent and per-column: everything else in the + * batch, including the phone's own snooze overlay, applies normally. + * + * Scoped to phone peers on purpose. A paired *desktop* peer runs the same + * `sessionService` chokepoint, so its settle writes are host-decided too and + * must keep replicating. + */ +const MOBILE_HOST_AUTHORITATIVE_COLUMNS = new Map>([ + ["terminal_sessions", new Set(["settled_at", "settle_override", "settle_source"])], +]); + +const isMobileAuthoredHostAuthoritativeColumn = (change: CrsqlChangeRow): boolean => + MOBILE_HOST_AUTHORITATIVE_COLUMNS.get(change.table)?.has(change.cid) ?? false; + const MOBILE_REPLICA_RESEED_EXCLUDED_TABLES = [ ...MOBILE_CHANGESET_EXCLUDED_TABLES, ...SYNC_HOST_AUTHORITATIVE_TABLES, @@ -7596,7 +7625,14 @@ export function createSyncHostService(args: SyncHostServiceArgs) { } // Brain-seizure guard: never let a peer's CRR rows for host-authoritative // tables (e.g. sync_cluster_state) win and flip brain ownership. - const filtered = changes.filter((change) => !isHostAuthoritativeTable(change)); + // Settle-authority guard: never let a phone's optimistic settle column + // merge over the host's decision (older iOS builds still write them). + const dropMobileSettleColumns = isMobileChangesetPeer(peer); + const filtered = changes.filter((change) => { + if (isHostAuthoritativeTable(change)) return false; + if (dropMobileSettleColumns && isMobileAuthoredHostAuthoritativeColumn(change)) return false; + return true; + }); try { let appliedCount = 0; if (filtered.length > 0) { diff --git a/apps/ios/ADE.xcodeproj/project.pbxproj b/apps/ios/ADE.xcodeproj/project.pbxproj index b071d4341..777c90c1d 100644 --- a/apps/ios/ADE.xcodeproj/project.pbxproj +++ b/apps/ios/ADE.xcodeproj/project.pbxproj @@ -77,6 +77,8 @@ B70000000000000000000004 /* SyncRecoveryPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */; }; B7000000000000000000002F /* PairedHostCredentialStateTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7000000000000000000001F /* PairedHostCredentialStateTests.swift */; }; B70000000000000000000099 /* SyncAccountConnectRecoveryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */; }; + B7000000000000000000009B /* PendingSessionSettleStates.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7000000000000000000009A /* PendingSessionSettleStates.swift */; }; + B7000000000000000000009D /* PendingSessionSettleStatesTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B7000000000000000000009C /* PendingSessionSettleStatesTests.swift */; }; B90000000000000000000002 /* SyncTransportSelectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B90000000000000000000001 /* SyncTransportSelectionTests.swift */; }; B80000000000000000000002 /* SyncConnectionRace.swift in Sources */ = {isa = PBXBuildFile; fileRef = B80000000000000000000001 /* SyncConnectionRace.swift */; }; B80000000000000000000004 /* SyncTerminalInputQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = B80000000000000000000003 /* SyncTerminalInputQueue.swift */; }; @@ -481,6 +483,8 @@ B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncRecoveryPolicyTests.swift; path = ADETests/SyncRecoveryPolicyTests.swift; sourceTree = ""; }; B7000000000000000000001F /* PairedHostCredentialStateTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PairedHostCredentialStateTests.swift; path = ADETests/PairedHostCredentialStateTests.swift; sourceTree = ""; }; B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncAccountConnectRecoveryTests.swift; path = ADETests/SyncAccountConnectRecoveryTests.swift; sourceTree = ""; }; + B7000000000000000000009A /* PendingSessionSettleStates.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PendingSessionSettleStates.swift; path = ADE/Services/PendingSessionSettleStates.swift; sourceTree = ""; }; + B7000000000000000000009C /* PendingSessionSettleStatesTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = PendingSessionSettleStatesTests.swift; path = ADETests/PendingSessionSettleStatesTests.swift; sourceTree = ""; }; B90000000000000000000001 /* SyncTransportSelectionTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncTransportSelectionTests.swift; path = ADETests/SyncTransportSelectionTests.swift; sourceTree = ""; }; B80000000000000000000001 /* SyncConnectionRace.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncConnectionRace.swift; path = ADE/Services/SyncConnectionRace.swift; sourceTree = ""; }; B80000000000000000000003 /* SyncTerminalInputQueue.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; name = SyncTerminalInputQueue.swift; path = ADE/Services/SyncTerminalInputQueue.swift; sourceTree = ""; }; @@ -920,6 +924,7 @@ A90000000000000000000011 /* ProductAnalytics.swift */, 66B5024B0A05F3D9754101F1 /* SyncService.swift */, B70000000000000000000001 /* SyncRecoveryPolicy.swift */, + B7000000000000000000009A /* PendingSessionSettleStates.swift */, B80000000000000000000001 /* SyncConnectionRace.swift */, B80000000000000000000003 /* SyncTerminalInputQueue.swift */, A11700000000000000000001 /* MobileUsageQuotaStore.swift */, @@ -1095,6 +1100,7 @@ B70000000000000000000003 /* SyncRecoveryPolicyTests.swift */, B7000000000000000000001F /* PairedHostCredentialStateTests.swift */, B70000000000000000000098 /* SyncAccountConnectRecoveryTests.swift */, + B7000000000000000000009C /* PendingSessionSettleStatesTests.swift */, B90000000000000000000001 /* SyncTransportSelectionTests.swift */, AF00000000000000000000A4 /* PairingAndDpopTests.swift */, AC1000000000000000000008 /* ClipPairingHandoffTests.swift */, @@ -1431,6 +1437,7 @@ C5B00000000000000000000C /* SSHPairingViewModel.swift in Sources */, C5B00000000000000000000D /* SSHPairingView.swift in Sources */, B70000000000000000000002 /* SyncRecoveryPolicy.swift in Sources */, + B7000000000000000000009B /* PendingSessionSettleStates.swift in Sources */, B80000000000000000000002 /* SyncConnectionRace.swift in Sources */, B80000000000000000000004 /* SyncTerminalInputQueue.swift in Sources */, A11700000000000000000002 /* MobileUsageQuotaStore.swift in Sources */, @@ -1609,6 +1616,7 @@ B70000000000000000000004 /* SyncRecoveryPolicyTests.swift in Sources */, B7000000000000000000002F /* PairedHostCredentialStateTests.swift in Sources */, B70000000000000000000099 /* SyncAccountConnectRecoveryTests.swift in Sources */, + B7000000000000000000009D /* PendingSessionSettleStatesTests.swift in Sources */, B90000000000000000000002 /* SyncTransportSelectionTests.swift in Sources */, AF00000000000000000000C4 /* PairingAndDpopTests.swift in Sources */, AC1100000000000000000008 /* ClipPairingHandoffTests.swift in Sources */, diff --git a/apps/ios/ADE/Services/Database.swift b/apps/ios/ADE/Services/Database.swift index 56ddad903..17c1dc571 100644 --- a/apps/ios/ADE/Services/Database.swift +++ b/apps/ios/ADE/Services/Database.swift @@ -2092,31 +2092,35 @@ final class DatabaseService { notifyDidChange(touchedTables: ["terminal_sessions"]) } - /// Optimistic local write for the ADE-125 session lifecycle columns - /// (settle / settle override / snooze overlay / woke marker). The phone is a - /// controller and never owns these values — the host's remote command is the - /// source of truth and reconciles over sync — but writing locally first keeps - /// the row from flickering back for a round trip. + /// Optimistic local write for the snooze visibility overlay (`snoozed_until`, + /// `snoozed_at`) and the woke marker. The phone is a controller and never owns + /// these values — the host's remote command is the source of truth and + /// reconciles over sync — but writing locally first keeps the row from + /// flickering back for a round trip. + /// + /// The settle columns (`settled_at`, `settle_override`, `settle_source`) are + /// deliberately NOT writable from here. `terminal_sessions` is a CRR table + /// whose local writes replicate upstream, so an optimistic settle carries no + /// host revision and can defeat a host-side rejection by CRDT merge — the host + /// leaves `settled_at` null, and the phone's row settles it anyway. Settle is + /// host-authoritative; the phone shows a local pending state instead + /// (`SyncService.pendingSessionSettleStates`) and waits for the host's + /// changeset. See `docs/features/terminals-and-sessions/settle-teardown-design.md` + /// §3c-i. /// /// Each parameter is a two-level optional so "leave alone" and "clear" are /// distinguishable: `nil` skips the column, `.some(nil)` sets it to NULL, /// `.some(value)` writes the value. - func updateSessionLifecycle( + func updateSessionSnoozeOverlay( sessionId: String, - settledAt: String?? = nil, - settleOverride: String?? = nil, - settleSource: String?? = nil, snoozedUntil: String?? = nil, snoozedAt: String?? = nil, wokeAt: String?? = nil, wokeReason: String?? = nil ) throws { try withLock { - try updateSessionLifecycleLocked( + try updateSessionSnoozeOverlayLocked( sessionId: sessionId, - settledAt: settledAt, - settleOverride: settleOverride, - settleSource: settleSource, snoozedUntil: snoozedUntil, snoozedAt: snoozedAt, wokeAt: wokeAt, @@ -2125,11 +2129,8 @@ final class DatabaseService { } } - private func updateSessionLifecycleLocked( + private func updateSessionSnoozeOverlayLocked( sessionId: String, - settledAt: String?? = nil, - settleOverride: String?? = nil, - settleSource: String?? = nil, snoozedUntil: String?? = nil, snoozedAt: String?? = nil, wokeAt: String?? = nil, @@ -2148,9 +2149,6 @@ final class DatabaseService { values.append(update) } - assign("settled_at", settledAt) - assign("settle_override", settleOverride) - assign("settle_source", settleSource) assign("snoozed_until", snoozedUntil) assign("snoozed_at", snoozedAt) assign("woke_at", wokeAt) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift new file mode 100644 index 000000000..908fa7179 --- /dev/null +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -0,0 +1,148 @@ +import Foundation + +/// A settle-family change the phone has sent to the host and is still waiting on. +/// +/// The phone must NOT write `settled_at` / `settle_override` / `settle_source` +/// into its own replica. `terminal_sessions` is a CRR table, so a local write +/// replicates upstream carrying no host lifecycle revision — which means an +/// optimistic settle can win a CRDT merge against a host that *rejected* the +/// settle, and file a live session as done. See +/// `docs/features/terminals-and-sessions/settle-teardown-design.md` §3c-i. +/// +/// This is what replaces the write: a purely local overlay applied when session +/// rows are read, so the row responds immediately while the command is in +/// flight, and nothing leaves the device. +/// +/// `settle_source` is deliberately absent. No iOS surface reads it, so +/// overlaying it would buy nothing and only add a value to guess wrong. +struct PendingSessionSettleIntent: Equatable { + /// Two-level optionals mirror the column semantics: `nil` leaves the column + /// alone, `.some(nil)` shows it cleared, `.some(value)` shows that value. + var settledAt: String?? + var settleOverride: String?? + /// When the command was sent, for the staleness backstop. + var startedAt: Date + + static func settle(now: Date, timestamp: String) -> PendingSessionSettleIntent { + // A declared settle also clears a `"settled"` pin host-side. An `"active"` + // pin survives, but a row carrying one cannot be settled from this menu in + // the first place, so showing the override cleared is not a lie the user + // can reach. + PendingSessionSettleIntent(settledAt: .some(timestamp), settleOverride: .some(nil), startedAt: now) + } + + static func unsettle(now: Date) -> PendingSessionSettleIntent { + // Only `settled_at`. The host clears a `"settled"` override but preserves an + // `"active"` one, and the phone cannot know which branch it will take — + // exactly the reasoning that already kept `settle_override` out of the old + // optimistic write. + PendingSessionSettleIntent(settledAt: .some(nil), settleOverride: nil, startedAt: now) + } + + static func settleOverride(_ value: String?, now: Date) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(settledAt: nil, settleOverride: .some(value), startedAt: now) + } + + /// Whether the host's replicated row now reflects this intent. + /// + /// The two columns are checked differently on purpose. `settled_at` carries + /// the *host's* timestamp, so only its presence is ours to predict — matching + /// on the exact string would never resolve. `settle_override` is an exact + /// value we asked for, so it is compared as one. + func isSatisfied(by session: TerminalSessionSummary) -> Bool { + if let settledAt { + let intended = PendingSessionSettleIntent.normalized(settledAt) != nil + guard (PendingSessionSettleIntent.normalized(session.settledAt) != nil) == intended else { return false } + } + if let settleOverride { + guard PendingSessionSettleIntent.normalized(session.settleOverride) + == PendingSessionSettleIntent.normalized(settleOverride) else { return false } + } + return true + } + + func applied(to session: TerminalSessionSummary) -> TerminalSessionSummary { + var next = session + if let settledAt { + next.settledAt = settledAt + } + if let settleOverride { + next.settleOverride = settleOverride + } + return next + } + + static func normalized(_ value: String?) -> String? { + guard let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines), !trimmed.isEmpty else { return nil } + return trimmed + } +} + +/// The set of in-flight settle intents, keyed by session id. +/// +/// Deliberately a plain value type with no I/O: the whole point is that this +/// state never touches SQLite and so can never replicate. +struct PendingSessionSettleStates: Equatable { + /// Backstop for an intent whose confirming changeset never arrives — a host + /// that applied the settle but dropped the connection before replicating it, + /// say. The overlay is a bridge across one round trip, not durable state, so + /// it expires rather than lying indefinitely. + static let staleAfter: TimeInterval = 20 + + private var intents: [String: PendingSessionSettleIntent] = [:] + + init() {} + + var isEmpty: Bool { intents.isEmpty } + + subscript(sessionId: String) -> PendingSessionSettleIntent? { intents[normalizedKey(sessionId)] } + + mutating func begin(_ intent: PendingSessionSettleIntent, for sessionId: String) { + let key = normalizedKey(sessionId) + guard !key.isEmpty else { return } + intents[key] = intent + } + + /// Drop an intent because the command failed. The row snaps back to whatever + /// the host actually has, which is the honest answer. + mutating func clear(_ sessionId: String) { + intents.removeValue(forKey: normalizedKey(sessionId)) + } + + /// Drop intents the host has now confirmed, plus any that outlived the + /// backstop. Sessions absent from `sessions` are left alone — a partial or + /// scoped read must not be mistaken for "the host disagrees". + @discardableResult + mutating func prune(against sessions: [TerminalSessionSummary], now: Date) -> Bool { + guard !intents.isEmpty else { return false } + var next = intents + for session in sessions { + let key = normalizedKey(session.id) + guard let intent = next[key] else { continue } + if intent.isSatisfied(by: session) { + next.removeValue(forKey: key) + } + } + for (key, intent) in next + where now.timeIntervalSince(intent.startedAt) >= PendingSessionSettleStates.staleAfter { + next.removeValue(forKey: key) + } + guard next != intents else { return false } + intents = next + return true + } + + func apply(to session: TerminalSessionSummary) -> TerminalSessionSummary { + guard let intent = intents[normalizedKey(session.id)] else { return session } + return intent.applied(to: session) + } + + func apply(to sessions: [TerminalSessionSummary]) -> [TerminalSessionSummary] { + guard !intents.isEmpty else { return sessions } + return sessions.map { apply(to: $0) } + } + + private func normalizedKey(_ sessionId: String) -> String { + sessionId.trimmingCharacters(in: .whitespacesAndNewlines) + } +} diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 675dd5428..59b1487db 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9145,11 +9145,23 @@ final class SyncService: ObservableObject { } func fetchSessions() async throws -> [TerminalSessionSummary] { - database.fetchSessions() + sessionsWithPendingSettleOverlay(database.fetchSessions()) } func fetchSession(id sessionId: String) async throws -> TerminalSessionSummary? { - database.fetchSession(id: sessionId) + guard let session = database.fetchSession(id: sessionId) else { return nil } + return sessionsWithPendingSettleOverlay([session]).first + } + + /// Retire confirmed/stale settle intents against these rows, then show the + /// still-pending ones. Every session read goes through here, so an intent + /// cannot outlive the host's answer to it. + private func sessionsWithPendingSettleOverlay( + _ sessions: [TerminalSessionSummary] + ) -> [TerminalSessionSummary] { + guard !pendingSessionSettleStates.isEmpty else { return sessions } + prunePendingSessionSettleStates(against: sessions) + return pendingSessionSettleStates.apply(to: sessions) } /// Best-effort hydration for a session whose local DB row may not have synced @@ -9163,10 +9175,14 @@ final class SyncService: ObservableObject { /// state. @discardableResult func ensureSessionRowHydrated(sessionId: String) async -> TerminalSessionSummary? { - if let existing = database.fetchSession(id: sessionId) { return existing } + if let existing = database.fetchSession(id: sessionId) { + return sessionsWithPendingSettleOverlay([existing]).first + } if canSendLiveRequests() { try? await refreshWorkSessions() - if let refreshed = database.fetchSession(id: sessionId) { return refreshed } + if let refreshed = database.fetchSession(id: sessionId) { + return sessionsWithPendingSettleOverlay([refreshed]).first + } } // Absorb changeset lag right after an in-place project activation: the row // arrives via CRDT sync a beat after the switch. Bounded so a genuinely @@ -9174,7 +9190,9 @@ final class SyncService: ObservableObject { for _ in 0..<6 { try? await Task.sleep(nanoseconds: 300_000_000) if Task.isCancelled { break } - if let row = database.fetchSession(id: sessionId) { return row } + if let row = database.fetchSession(id: sessionId) { + return sessionsWithPendingSettleOverlay([row]).first + } } return nil } @@ -9251,9 +9269,47 @@ final class SyncService: ObservableObject { // // The phone is a controller and never runs agents, so every lifecycle change // is a host command — `session.*` in the ADE action registry — not a local - // write we then hope replicates. We still write the columns locally first so - // the row doesn't flicker for a round trip, and roll that write back if the - // host rejects the command. + // write we then hope replicates. + // + // The two halves are handled differently, and the split is load-bearing: + // + // - **Settle columns** (`settled_at`, `settle_override`, `settle_source`) are + // host-authoritative and are NEVER written to the local replica. Because + // `terminal_sessions` is a CRR table, such a write replicates upstream + // carrying no host lifecycle revision, so it can win a CRDT merge against a + // host that *rejected* the settle and file a live session as done. Instant + // feedback comes from `pendingSessionSettleStates`, a local overlay applied + // at read time that resolves when the host's changeset lands or the command + // fails. See the settle-teardown design, §3c-i. + // - **Snooze overlay** (`snoozed_until`, `snoozed_at`, `woke_*`) still writes + // optimistically with a rollback. Those columns are not guarded by a host + // revision and have no teardown attached, so a merge cannot defeat a host + // decision — there is none to defeat. + + /// In-flight settle intents, applied over session reads so a settle feels + /// immediate without a replicating write. Never persisted. + private(set) var pendingSessionSettleStates = PendingSessionSettleStates() + + /// Record an in-flight settle intent and nudge the projections, mirroring the + /// re-render the optimistic DB write used to trigger through + /// `adeDatabaseDidChange`. + private func beginPendingSessionSettle(_ intent: PendingSessionSettleIntent, for sessionId: String) { + pendingSessionSettleStates.begin(intent, for: sessionId) + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + } + + private func clearPendingSessionSettle(_ sessionId: String) { + guard pendingSessionSettleStates[sessionId] != nil else { return } + pendingSessionSettleStates.clear(sessionId) + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + } + + /// Retire intents the host's replicated rows have now confirmed (or that + /// outlived the staleness backstop). Called from every session read so the + /// overlay cannot outlive its answer. + private func prunePendingSessionSettleStates(against sessions: [TerminalSessionSummary]) { + pendingSessionSettleStates.prune(against: sessions, now: Date()) + } /// Whether this host advertises the ADE-125 lifecycle actions at all. Older /// desktop builds simply do not have them; the UI hides the affordances @@ -9281,7 +9337,12 @@ final class SyncService: ObservableObject { ]) } - /// Optimistic local write + host command, with rollback on failure. + /// Host command for a lifecycle change, with an optional optimistic local + /// write of the SNOOZE overlay only, rolled back on failure. + /// + /// The settle columns are not parameters here and must not become ones — they + /// are host-authoritative (see the section comment above). Settle callers pass + /// a `pendingSettle` intent instead, which is local-only. private func sendSessionLifecycleCommand( sessionId: String, action: String, @@ -9290,9 +9351,7 @@ final class SyncService: ObservableObject { // the actions that report nothing usable, mirroring the desktop // `lifecycleCall` call sites that pass no `applied` predicate. resultShape: SessionLifecycleResultShape? = .envelope, - settledAt: String?? = nil, - settleOverride: String?? = nil, - settleSource: String?? = nil, + pendingSettle: PendingSessionSettleIntent? = nil, snoozedUntil: String?? = nil, snoozedAt: String?? = nil, wokeAt: String?? = nil, @@ -9305,34 +9364,35 @@ final class SyncService: ObservableObject { } let previous = database.fetchSession(id: trimmed) - try? database.updateSessionLifecycle( + if let pendingSettle { + beginPendingSessionSettle(pendingSettle, for: trimmed) + } + try? database.updateSessionSnoozeOverlay( sessionId: trimmed, - settledAt: settledAt, - settleOverride: settleOverride, - settleSource: settleSource, snoozedUntil: snoozedUntil, snoozedAt: snoozedAt, wokeAt: wokeAt, wokeReason: wokeReason ) - // Undo the optimistic write. Restores ONLY the columns this call actually - // assigned: `terminal_sessions` is a CRR table whose local writes replicate - // upstream, so re-stamping a column we never touched would push our stale - // copy of a host-owned value back over whatever the machine has since - // written — the same hazard that keeps `snoozed_at` out of the forward - // write below. A half-applied lifecycle is still worse than none, so every - // column we DID write is restored together. + // Undo the optimistic snooze write and drop the local settle intent. + // Restores ONLY the columns this call actually assigned: `terminal_sessions` + // is a CRR table whose local writes replicate upstream, so re-stamping a + // column we never touched would push our stale copy of a host-owned value + // back over whatever the machine has since written — the same hazard that + // keeps `snoozed_at` out of the forward write below. A half-applied + // lifecycle is still worse than none, so every column we DID write is + // restored together. func rollback() { + if pendingSettle != nil { + clearPendingSessionSettle(trimmed) + } guard let previous else { return } func restored(_ requested: String??, _ value: String?) -> String?? { requested == nil ? nil : .some(value) } - try? database.updateSessionLifecycle( + try? database.updateSessionSnoozeOverlay( sessionId: trimmed, - settledAt: restored(settledAt, previous.settledAt), - settleOverride: restored(settleOverride, previous.settleOverride), - settleSource: restored(settleSource, previous.settleSource), snoozedUntil: restored(snoozedUntil, previous.snoozedUntil), snoozedAt: restored(snoozedAt, previous.snoozedAt), wokeAt: restored(wokeAt, previous.wokeAt), @@ -9365,7 +9425,8 @@ final class SyncService: ObservableObject { } } - /// Declared settle. Stamps `settled_at` locally to match what the host writes. + /// Declared settle. The host owns `settled_at`; this only shows the row as + /// settled locally until the host's changeset answers. /// /// `dismissPendingInput` is the needs-you variant — desktop's "Dismiss & /// settle" — and it must ONLY be passed for a row that genuinely has a pending @@ -9390,9 +9451,10 @@ final class SyncService: ObservableObject { // The bulk action answers with the ids it CHANGED, so an absent id means // the machine settled nothing. Mirrors the desktop `settleMany`. resultShape: .changedIdList, - settledAt: .some(iso8601WithFractionalSecondsFormatter.string(from: Date())), - settleOverride: .some(nil), - settleSource: .some("user") + pendingSettle: .settle( + now: Date(), + timestamp: iso8601WithFractionalSecondsFormatter.string(from: Date()) + ) ) } @@ -9401,13 +9463,10 @@ final class SyncService: ObservableObject { /// The host clears a `"settled"` override ONLY — see `sessionService`'s /// `settle_override = case when settle_override = 'settled' then null else /// settle_override end`. An `"active"` keep-alive pin deliberately SURVIVES - /// unsettle. So this must NOT write `settle_override` at all: the phone - /// cannot know which of the two branches the machine will take, and - /// `terminal_sessions` is a CRR table whose local writes are captured by the - /// update trigger and pushed upstream in `changeset_batch`. Claiming a clear - /// here would replicate a null back over the pin the host just preserved. - /// Leave the column alone and let hydration deliver the machine's answer — - /// this mirrors the web overlay's `UNSETTLE_PATCH`, which is `settledAt` only. + /// unsettle. So the pending overlay covers `settledAt` only: the phone cannot + /// know which of the two branches the machine will take, and showing a clear + /// it may not get would just be a local lie in place of the replicated one + /// this used to be. Mirrors the web overlay's `UNSETTLE_PATCH`. func unsettleSession(sessionId: String) async throws { try await sendSessionLifecycleCommand( sessionId: sessionId, @@ -9417,9 +9476,7 @@ final class SyncService: ObservableObject { // per-row verdict to check — so there is nothing to reject, exactly like // the desktop `unsettleMany`, which passes no `applied` predicate. resultShape: nil, - settledAt: .some(nil), - settleOverride: nil, - settleSource: .some(nil) + pendingSettle: .unsettle(now: Date()) ) } @@ -9432,7 +9489,7 @@ final class SyncService: ObservableObject { // The host reads "clear" as null; sending a JSON null through the // `[String: Any]` arg dictionary is not representable here. args: ["sessionId": sessionId, "override": override?.rawValue ?? "clear"], - settleOverride: .some(override?.rawValue) + pendingSettle: .settleOverride(override?.rawValue, now: Date()) ) } @@ -20870,7 +20927,7 @@ extension SyncService { guard let projectId = activeProjectId else { return nil } let lanes = database.fetchLanes(includeArchived: false) let visibleLaneIds = Set(lanes.map(\.id)) - let scopedSessions = database.fetchSessions().filter { session in + let scopedSessions = sessionsWithPendingSettleOverlay(database.fetchSessions()).filter { session in session.archivedAt == nil && visibleLaneIds.contains(session.laneId) } let topLevelIds = Set(scopedSessions.filter { isRosterTopLevelToolType($0.toolType) }.map(\.id)) diff --git a/apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift b/apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift index 27dd52c20..48453a09b 100644 --- a/apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift +++ b/apps/ios/ADE/Views/Work/WorkRootScreen+Actions.swift @@ -420,9 +420,11 @@ extension WorkRootScreen { // MARK: - Session lifecycle (ADE-125) // // Every one of these is a host command: the phone is a controller and never - // runs agents, so it never owns a lifecycle column. `SyncService` writes the - // column locally first (so the row doesn't flicker), rolls that back if the - // host rejects, and `reload()` reconciles against the replicated truth. + // runs agents, so it never owns a lifecycle column. `SyncService` shows the + // change immediately — a local pending overlay for the host-authoritative + // settle columns, an optimistic write for the snooze overlay — drops or rolls + // that back if the host rejects, and `reload()` reconciles against the + // replicated truth. private func runSessionLifecycle(_ work: @escaping () async throws -> Void) { Task { @@ -433,7 +435,8 @@ extension WorkRootScreen { ADEHaptics.error() let message = error.localizedDescription // Reconcile against replicated truth first — `SyncService` has already - // rolled the optimistic column back — and only then surface the + // dropped the pending settle overlay / rolled the optimistic snooze + // column back — and only then surface the // failure. This must NOT go through `errorMessage`: every successful // projection load clears that (`reload()` here, and // `reloadFromPersistedProjection()` on the very next CRDT tick, which diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift new file mode 100644 index 000000000..c632ea15b --- /dev/null +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -0,0 +1,167 @@ +import XCTest + +@testable import ADE + +/// The phone must never write `settled_at` into its CRR replica — that write +/// replicates upstream and can settle a session the host rejected. These cover +/// the local overlay that replaced it: it has to feel like the old optimistic +/// write, and it has to stop lying the moment the host answers. +final class PendingSessionSettleStatesTests: XCTestCase { + private let now = Date(timeIntervalSince1970: 1_760_000_000) + + private func session( + id: String = "session-1", + settledAt: String? = nil, + settleOverride: String? = nil + ) -> TerminalSessionSummary { + var summary = TerminalSessionSummary( + id: id, + laneId: "lane-1", + laneName: "Lane", + ptyId: nil, + tracked: true, + pinned: false, + manuallyNamed: nil, + goal: nil, + toolType: "claude-chat", + title: "Session", + status: "running", + startedAt: "2026-08-10T00:00:00.000Z", + endedAt: nil, + exitCode: nil, + transcriptPath: "", + headShaStart: nil, + headShaEnd: nil, + lastOutputPreview: nil, + summary: nil, + runtimeState: "idle", + resumeCommand: nil, + resumeMetadata: nil, + chatIdleSinceAt: nil + ) + summary.settledAt = settledAt + summary.settleOverride = settleOverride + return summary + } + + func testSettleIntentShowsTheRowSettledBeforeTheHostAnswers() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + let overlaid = states.apply(to: session()) + XCTAssertEqual(overlaid.settledAt, "2026-08-10T12:00:00.000Z") + // A declared settle clears a `"settled"` pin host-side, so the overlay + // shows that too. + XCTAssertNil(overlaid.settleOverride) + } + + func testUnsettleIntentLeavesTheOverrideToTheHost() { + var states = PendingSessionSettleStates() + states.begin(.unsettle(now: now), for: "session-1") + + // The host clears a `"settled"` override but PRESERVES an `"active"` pin, + // and the phone cannot know which branch it takes — so the overlay must not + // claim either. + let overlaid = states.apply(to: session(settledAt: "2026-08-10T09:00:00.000Z", settleOverride: "active")) + XCTAssertNil(overlaid.settledAt) + XCTAssertEqual(overlaid.settleOverride, "active") + } + + func testIntentResolvesOnTheHostsOwnTimestampNotOurs() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + // The host writes its own clock. Matching on the exact string would never + // resolve, so presence is what the settle intent predicts. + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + + XCTAssertNil(states["session-1"]) + XCTAssertTrue(states.isEmpty) + } + + func testIntentSurvivesUntilTheHostRowActuallyChanges() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + states.prune(against: [session(settledAt: nil)], now: now) + + XCTAssertNotNil(states["session-1"]) + XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") + } + + func testUnsettleIntentResolvesWhenTheRowGoesBackToNull() { + var states = PendingSessionSettleStates() + states.begin(.unsettle(now: now), for: "session-1") + + states.prune(against: [session(settledAt: "2026-08-10T09:00:00.000Z")], now: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session(settledAt: nil)], now: now) + XCTAssertNil(states["session-1"]) + } + + func testOverrideIntentComparesTheExactValueWeAskedFor() { + var states = PendingSessionSettleStates() + states.begin(.settleOverride("active", now: now), for: "session-1") + + // `settle_override` is a value we own, unlike the settle timestamp — a + // different non-null value is the host disagreeing, not confirming. + states.prune(against: [session(settleOverride: "settled")], now: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session(settleOverride: "active")], now: now) + XCTAssertNil(states["session-1"]) + } + + func testClearingAnOverrideResolvesOnNull() { + var states = PendingSessionSettleStates() + states.begin(.settleOverride(nil, now: now), for: "session-1") + + XCTAssertNil(states.apply(to: session(settleOverride: "active")).settleOverride) + + states.prune(against: [session(settleOverride: nil)], now: now) + XCTAssertNil(states["session-1"]) + } + + func testAFailedCommandDropsTheIntentSoTheRowSnapsBack() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + states.clear("session-1") + + XCTAssertNil(states.apply(to: session()).settledAt) + } + + func testAnIntentWhoseChangesetNeverArrivesExpires() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + let justBefore = now.addingTimeInterval(PendingSessionSettleStates.staleAfter - 1) + states.prune(against: [session(settledAt: nil)], now: justBefore) + XCTAssertNotNil(states["session-1"]) + + let after = now.addingTimeInterval(PendingSessionSettleStates.staleAfter) + states.prune(against: [session(settledAt: nil)], now: after) + XCTAssertNil(states["session-1"], "a pending overlay must not outlive its round trip indefinitely") + } + + func testASessionMissingFromAScopedReadKeepsItsIntent() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + // A partial or differently-scoped read is not the host disagreeing. + states.prune(against: [session(id: "session-2", settledAt: nil)], now: now) + + XCTAssertNotNil(states["session-1"]) + } + + func testOverlayOnlyTouchesTheSessionItWasBegunFor() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + let others = states.apply(to: [session(id: "session-1"), session(id: "session-2")]) + + XCTAssertEqual(others[0].settledAt, "2026-08-10T12:00:00.000Z") + XCTAssertNil(others[1].settledAt) + } +} diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index 090ec12d8..e970322e7 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -2030,6 +2030,22 @@ The iOS pieces: - `apps/ios/ADE/Services/SyncService.swift` holds the `session.*` remote-command callers. Mobile has no local write path for lifecycle, so these commands are the mechanism, and the connect-time descriptor list gates the affordances. +- **The settle columns are host-authoritative and the phone never writes them.** + `settled_at`, `settle_override`, and `settle_source` are decided by the host's + `sessionService`, which is the only place that can weigh a settle against live + work. `terminal_sessions` is a CRR table, so a local optimistic write on the + phone replicates upstream carrying no host lifecycle revision — it can win a + merge against a host that *rejected* the settle and file a live session as + done. `Database.updateSessionSnoozeOverlay` therefore cannot write them at all; + it is scoped to the snooze overlay, which keeps its optimistic write plus + rollback because those columns guard no host decision. Instant feedback for + settle comes from `PendingSessionSettleStates` — a local, non-persisted overlay + applied when session rows are read, resolved when the host's changeset confirms + it, when the command fails, or by a bounded staleness backstop. The host + enforces the same rule against phones on older builds by dropping those columns + from inbound phone changesets (`syncHostService`), and such a phone self-heals + on the next `refreshWorkSessions`. See + [settle-teardown design §3c-i](../terminals-and-sessions/settle-teardown-design.md). - `apps/ios/ADE/Views/Work/WorkSessionCanonicalState.swift` is the Swift mirror of the shared derivation, including the settle-override tier and `isSessionFiledAsSnoozed`. It also owns the row's status vocabulary: diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 83aa97720..90097cbeb 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -215,7 +215,11 @@ and in tests. settle stop work needs a synchronous lifecycle revision that teardown can be serialized against; it is not a wrapper around the existing write. Archive is the one lifecycle path that does stop processes — see - `laneService.archive`, where the ordering is load-bearing. + `laneService.archive`, where the ordering is load-bearing. The approved plan + for making settle stop work is + [settle-teardown-design.md](settle-teardown-design.md); its step 0 + precondition — `settled_at` becoming host-authoritative, so no replica can + defeat the revision guard by CRDT merge — has landed. `dismissPendingInput: true` first quiets an SDK chat through `agentChatService`, or clears a tracked CLI's explicit `ade chat ask` marker through `ptyService`; arbitrary native @@ -1848,6 +1852,18 @@ runtime and agent chat runtime both layer the same identity envs - **Process exit is not settlement.** A clean exit-0 row remains ended until an agent/user declaration or the enabled PR-merge policy settles it. New lifecycle surfaces must not infer task completion from process mechanics. +- **`settled_at` / `settle_override` / `settle_source` are host-authoritative — + a controller must never write them into its replica.** `terminal_sessions` is + a CRR table, so such a write replicates upstream carrying no host lifecycle + revision, and can win a merge against a host that *rejected* the settle. That + defeats the guard by merge rather than by caller, which no amount of host-side + checking closes. iOS shows a local pending overlay instead + (`PendingSessionSettleStates.swift`), and `syncHostService` drops those columns + from inbound **phone** changesets so an older build cannot bypass the rule. The + filter is deliberately not applied to desktop peers: they run the same + `sessionService` chokepoint, so their settle writes are host-decided and must + keep replicating. The snooze columns are exempt — the phone owns its optimistic + write there because no host decision is at stake. - **Settlement is not a pending-input response.** Never restore the old renderer sequence of `respondToInput` then settle. A provider decline may resume work, Codex plan declines may stage a revision, and a stale persisted diff --git a/docs/features/terminals-and-sessions/settle-teardown-design.md b/docs/features/terminals-and-sessions/settle-teardown-design.md index 5a0caae9b..e13e15e3e 100644 --- a/docs/features/terminals-and-sessions/settle-teardown-design.md +++ b/docs/features/terminals-and-sessions/settle-teardown-design.md @@ -4,6 +4,8 @@ to implement; step 3 (attaching real teardown) waits until 1 and 2 are merged and the race-matrix tests have been seen to pass. +**Step 0 is implemented** — see amendment 6 in §3c-i for the host-side half. + Settle currently writes a lifecycle column and stops nothing. A session filed as "done" can still own a background shell, a subagent fleet, or a Cursor cloud run — burning tokens and holding ports behind a row that has left every live-work @@ -222,6 +224,32 @@ Snooze columns (`snoozed_until`, `snoozed_at`, `woke_*`) are out of scope here; they are written by the same helper but are not guarded by a revision and have no teardown attached. +**Amendment 6 — how the host treats a pre-fix client (implemented, step 0).** +Removing the write from iOS fixes new builds and nothing else: a paired phone on +an older build keeps writing `settled_at` into its replica, and a CRDT merge +never reaches the caller a host-side check would guard. Waiting for clients to +update is not a guarantee, so the host enforces it. + +`syncHostService` drops inbound `terminal_sessions` changes for `settled_at`, +`settle_override`, and `settle_source` when the peer is a phone +(`isMobileChangesetPeer`), alongside the existing `sync_cluster_state` +brain-seizure filter. The drop is per-column and silent: the rest of the batch — +including the phone's own snooze overlay, which it legitimately owns — applies +normally, and the batch still acks `ok`, because a rejected ack would stall the +peer's outbound cursor and make it resend the same range forever. + +It is scoped to phone peers deliberately. A paired **desktop** runs the same +`sessionService` chokepoint, so its settle writes are host-decided too and must +keep replicating; broadening the filter would silently stop settle propagating +between two of a user's own machines. + +No capability negotiation is involved — no wire shape changes and the client +needs to know nothing. The visible consequence for a pre-fix phone is that its +optimistic value is now local-only divergence rather than authoritative +corruption, and it self-heals: `refreshWorkSessions` rewrites local rows from the +host's `work.listSessions` payload via `replaceTerminalSessions`. Host authority +wins, which is the whole point of the precondition. + ### 3c-ii. Where the revision column lives **The revision must be local-only.** `terminal_sessions` is CRR, and C4 writes From 2ed00077aae9aa002c9ddf2f086649c5625ed955 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:19:01 -0400 Subject: [PATCH 02/15] quality: fix the overlay bypass, the offline expiry, and the command split MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review rounds against the step 0 change. The findings that mattered: **The overlay was bypassed where it mattered most.** `refreshActiveSessionsAndSnapshot` read the database directly, so the widget, Live Activity, and Activity drawer kept showing a just-settled chat as a live agent — and worse, beginning a settle bumped the projection, which is precisely what re-ran that blind reader. Fixed by making the overlay a real read chokepoint (`localSessions` / `localSession(id:)`) and routing every UI-feeding read through it. Two raw reads remain and both are deliberate: the pre-command snapshot the snooze rollback restores, and an existence check the overlay cannot affect. **A settle taken offline un-settled itself after 20 seconds.** `session.settleSessions` is queueable, so offline it returns `{queued: true}` and waits — for minutes. The staleness backstop aged it out on wall clock, so the row snapped back to unsettled and then settled again when the queue drained. `holdBackstop` now holds the deadline while the host is unreachable, making `staleAfter` a budget of reachable time. Sampled at each read, not integrated — documented, and the safe direction. **`sendSessionLifecycleCommand` was two functions in one coat.** Settle and snooze apply completely different optimism (a local overlay vs. a replicating column write) and no caller ever mixes them. Split into a core that takes a `rollback` closure plus `sendSessionSettleCommand` / `sendSessionSnoozeCommand`. Guard ordering and error paths are unchanged. Also: `String??` pairs became a `Kind` enum; the overlay is dropped on unpair, not just project switch (an unreachable host would otherwise hold it for the app's life); `.unsettle` now predicts the host's `settle_override` branch, which is decidable from the row, so a session settled purely by a pin finally shows feedback; rollback clears are scoped by token so a slow command's failure cannot retire a newer intent. Two bugs caught in the fixes themselves: change-detection that reported the offline re-stamp as a resolution (would have looped the repaint), and a satisfaction rule I verified against the host SQL rather than assuming. Host side: `isPhonePeer` for the asymmetric rule, tests moved into the existing `inbound changeset_batch guards` block beside the `sync_cluster_state` sibling they mirror, and the shared outbound helper restored. Docs reconciled — §3c-i past-tensed, the "rollback becomes unnecessary" prediction corrected (it survived, scoped to snooze), and §5 step 0 marked landed. --- .../src/services/sync/syncHostService.test.ts | 281 +++++++++--------- .../src/services/sync/syncHostService.ts | 12 +- apps/ios/ADE/Services/Database.swift | 11 +- .../Services/PendingSessionSettleStates.swift | 201 ++++++++----- apps/ios/ADE/Services/SyncService.swift | 279 ++++++++++------- .../PendingSessionSettleStatesTests.swift | 129 +++++++- .../features/terminals-and-sessions/README.md | 12 +- .../settle-teardown-design.md | 87 +++--- 8 files changed, 630 insertions(+), 382 deletions(-) diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index 1ebef769d..f49704d61 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -6683,7 +6683,7 @@ describe("outbound changeset ack retries", () => { projectRoot: string, state: { dbVersion: number; changes: CrsqlChangeRow[] }, logger = createDiscoveryLogger(), - options: { pollIntervalMs?: number; applyChanges?: (changes: CrsqlChangeRow[]) => void } = {}, + options: { pollIntervalMs?: number } = {}, ) { const base = createHostArgs(projectRoot, []); const exportChangesSince = vi.fn( @@ -6709,10 +6709,7 @@ describe("outbound changeset ack retries", () => { getSiteId: () => "site-host-controlled", getDbVersion: () => state.dbVersion, exportChangesSince, - applyChanges: (changes: CrsqlChangeRow[]) => { - options.applyChanges?.(changes); - return { appliedCount: changes.length }; - }, + applyChanges: () => ({ appliedCount: 0 }), discardUnpublishedChangesForTables: () => {}, }, }, @@ -6724,138 +6721,6 @@ describe("outbound changeset ack retries", () => { return { host, logger, exportChangesSince }; } - /** - * `settled_at` is host-authoritative (settle-teardown design §3c-i). A phone - * on a build that predates the fix still writes it into its own CRR replica - * optimistically, and `terminal_sessions` replicates — so without this filter - * the phone's row merges upstream and settles a session the host *rejected*. - * The guard has to live here because a CRDT merge never reaches the caller - * the host-side check guards. - */ - describe("settle columns are host-authoritative against a phone replica", () => { - function settleChange(overrides: Partial = {}): CrsqlChangeRow { - return { - table: "terminal_sessions", - pk: "session-1", - cid: "settled_at", - val: "2026-08-10T00:00:00.000Z", - col_version: 1, - db_version: 1, - site_id: "site-phone", - cl: 1, - seq: 0, - ...overrides, - }; - } - - async function sendInboundBatch( - peer: Awaited>, - changes: CrsqlChangeRow[], - ) { - peer.ws.send(encodeSyncEnvelope({ - type: "changeset_batch", - requestId: "inbound-settle", - payload: { - batchId: "inbound-settle", - reason: "broadcast", - fromDbVersion: 0, - toDbVersion: 1, - changes, - }, - })); - return waitForValue( - () => peer.envelopes.find((envelope) => - envelope.type === "changeset_ack" - && (envelope.payload as SyncChangesetAckPayload).batchId === "inbound-settle"), - "inbound settle changeset ack", - ); - } - - it("drops a phone's settle columns while applying the rest of the same batch", async () => { - const { projectRoot, cleanup } = createTempProjectRoot(); - const applied: CrsqlChangeRow[][] = []; - const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; - const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { - applyChanges: (changes) => applied.push(changes), - }); - let peer: Awaited> | null = null; - try { - const port = await host.waitUntilListening(); - peer = await connectPeer(port, host.getBootstrapToken(), "phone-settle"); - - const ack = await sendInboundBatch(peer, [ - settleChange({ cid: "settled_at" }), - settleChange({ cid: "settle_override", val: "settled", seq: 1 }), - settleChange({ cid: "settle_source", val: "user", seq: 2 }), - // The snooze overlay is NOT host-authoritative — the phone owns its - // optimistic write there and it must keep replicating. - settleChange({ cid: "snoozed_until", val: "2026-08-11T00:00:00.000Z", seq: 3 }), - settleChange({ cid: "title", val: "renamed from phone", seq: 4 }), - ]); - - expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); - expect(applied).toHaveLength(1); - expect(applied[0]?.map((change) => change.cid)).toEqual(["snoozed_until", "title"]); - } finally { - peer?.ws.close(); - await host.dispose(); - cleanup(); - } - }); - - it("acks a batch that was entirely settle columns without applying anything", async () => { - const { projectRoot, cleanup } = createTempProjectRoot(); - const applied: CrsqlChangeRow[][] = []; - const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; - const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { - applyChanges: (changes) => applied.push(changes), - }); - let peer: Awaited> | null = null; - try { - const port = await host.waitUntilListening(); - peer = await connectPeer(port, host.getBootstrapToken(), "phone-settle-only"); - - // A silent drop must still ack ok, or the phone re-sends the same - // poisoned range forever: its outbound cursor only advances on an ok. - const ack = await sendInboundBatch(peer, [settleChange()]); - expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); - expect((ack.payload as SyncChangesetAckPayload).appliedCount).toBe(0); - expect(applied).toHaveLength(0); - } finally { - peer?.ws.close(); - await host.dispose(); - cleanup(); - } - }); - - it("keeps applying settle columns from a paired desktop peer", async () => { - const { projectRoot, cleanup } = createTempProjectRoot(); - const applied: CrsqlChangeRow[][] = []; - const state = { dbVersion: 0, changes: [] as CrsqlChangeRow[] }; - const { host } = createControlledChangesetHost(projectRoot, state, createDiscoveryLogger(), { - applyChanges: (changes) => applied.push(changes), - }); - let peer: Awaited> | null = null; - try { - const port = await host.waitUntilListening(); - // A desktop runs the same `sessionService` chokepoint, so its settle - // writes are host-decided too and must keep replicating. - peer = await connectPeer(port, host.getBootstrapToken(), "desktop-peer", { - platform: "macOS", - deviceType: "desktop", - }); - - const ack = await sendInboundBatch(peer, [settleChange()]); - expect((ack.payload as SyncChangesetAckPayload).ok).toBe(true); - expect(applied[0]?.map((change) => change.cid)).toEqual(["settled_at"]); - } finally { - peer?.ws.close(); - await host.dispose(); - cleanup(); - } - }); - }); - it("reseeds a far-behind iOS replica once, then resumes incrementally from the acknowledged watermark", async () => { const { projectRoot, cleanup } = createTempProjectRoot(); const state = { @@ -7981,6 +7846,14 @@ describe("inbound changeset_batch guards", () => { }; } + /** One column of one `terminal_sessions` row, as a peer would author it. */ + function makeSettleChange(cid: string, dbVersion: number, seq: number, val: string): CrsqlChangeRow { + const change = makePeerChange("terminal_sessions", dbVersion, seq, val); + change.cid = cid; + change.pk = "session-1"; + return change; + } + function createGuardHost(projectRoot: string, applyChanges: ReturnType) { const base = createHostArgs(projectRoot, []); return createSyncHostService({ @@ -8112,6 +7985,140 @@ describe("inbound changeset_batch guards", () => { cleanup(); } }); + + it("strips a phone's settle columns while applying the rest of the same batch", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applyChanges = vi.fn((changes: CrsqlChangeRow[]) => ({ appliedCount: changes.length })); + const host = createGuardHost(projectRoot, applyChanges); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + peer = await connectPeer(port, host.getBootstrapToken(), "ios-settler"); + + // `settled_at` is host-authoritative. A phone on a build that predates the + // fix still writes it into its own CRR replica optimistically, and + // `terminal_sessions` replicates — so without this filter the phone's row + // merges upstream and settles a session the host *rejected*. The guard has + // to live here because a CRDT merge never reaches the caller a host-side + // check would guard. + const requestId = "batch-settle"; + peer.ws.send(encodeSyncEnvelope({ + type: "changeset_batch", + requestId, + payload: { + batchId: requestId, + fromDbVersion: 0, + toDbVersion: 5, + changes: [ + makeSettleChange("settled_at", 1, 0, "2026-08-10T00:00:00.000Z"), + makeSettleChange("settle_override", 2, 1, "settled"), + makeSettleChange("settle_source", 3, 2, "user"), + // The snooze overlay is NOT host-authoritative — the phone owns its + // optimistic write there and it must keep replicating. + makeSettleChange("snoozed_until", 4, 3, "2026-08-11T00:00:00.000Z"), + makeSettleChange("title", 5, 4, "renamed from phone"), + ], + }, + })); + + const ack = await waitForEnvelope(peer.envelopes, "changeset_ack", requestId); + expect((ack.payload as { ok?: boolean }).ok).toBe(true); + expect(applyChanges).toHaveBeenCalledTimes(1); + const appliedRows = applyChanges.mock.calls[0]?.[0] as CrsqlChangeRow[]; + expect(appliedRows.map((row) => row.cid)).toEqual(["snoozed_until", "title"]); + } finally { + try { + peer?.ws.close(); + } catch { + // ignore + } + await host.dispose(); + cleanup(); + } + }); + + it("acks a batch that was entirely settle columns without applying anything", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applyChanges = vi.fn((changes: CrsqlChangeRow[]) => ({ appliedCount: changes.length })); + const host = createGuardHost(projectRoot, applyChanges); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + peer = await connectPeer(port, host.getBootstrapToken(), "ios-settler-only"); + + const requestId = "batch-settle-only"; + peer.ws.send(encodeSyncEnvelope({ + type: "changeset_batch", + requestId, + payload: { + batchId: requestId, + fromDbVersion: 0, + toDbVersion: 1, + changes: [makeSettleChange("settled_at", 1, 0, "2026-08-10T00:00:00.000Z")], + }, + })); + + // A silent drop must still ack ok, or the phone re-sends the same range + // forever: its outbound cursor only advances on an ok ack. + const ack = await waitForEnvelope(peer.envelopes, "changeset_ack", requestId); + const ackPayload = ack.payload as { ok?: boolean; appliedCount?: number }; + expect(ackPayload.ok).toBe(true); + expect(ackPayload.appliedCount).toBe(0); + expect(applyChanges).not.toHaveBeenCalled(); + } finally { + try { + peer?.ws.close(); + } catch { + // ignore + } + await host.dispose(); + cleanup(); + } + }); + + it("keeps applying settle columns from a paired desktop peer", async () => { + const { projectRoot, cleanup } = createTempProjectRoot(); + const applyChanges = vi.fn((changes: CrsqlChangeRow[]) => ({ appliedCount: changes.length })); + const host = createGuardHost(projectRoot, applyChanges); + let peer: Awaited> | null = null; + try { + const port = await host.waitUntilListening(); + // A desktop runs the same `sessionService` chokepoint, so its settle + // writes are host-decided too and must keep replicating. + peer = await connectPeer(port, host.getBootstrapToken(), "desktop-peer", { + platform: "macOS", + deviceType: "desktop", + }); + + const requestId = "batch-settle-desktop"; + peer.ws.send(encodeSyncEnvelope({ + type: "changeset_batch", + requestId, + payload: { + batchId: requestId, + fromDbVersion: 0, + toDbVersion: 1, + changes: [makeSettleChange("settled_at", 1, 0, "2026-08-10T00:00:00.000Z")], + }, + })); + + const ack = await waitForEnvelope(peer.envelopes, "changeset_ack", requestId); + const ackPayload = ack.payload as { ok?: boolean; appliedCount?: number }; + expect(ackPayload.ok).toBe(true); + expect(ackPayload.appliedCount).toBe(1); + expect(applyChanges).toHaveBeenCalledTimes(1); + const appliedRows = applyChanges.mock.calls[0]?.[0] as CrsqlChangeRow[]; + expect(appliedRows.map((row) => row.cid)).toEqual(["settled_at"]); + } finally { + try { + peer?.ws.close(); + } catch { + // ignore + } + await host.dispose(); + cleanup(); + } + }); }); describe("sync host handoff over a shared listener", () => { diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index fda52a21f..1c1c47653 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -375,12 +375,12 @@ const isHostAuthoritativeTable = (change: CrsqlChangeRow): boolean => * `sessionService` chokepoint, so its settle writes are host-decided too and * must keep replicating. */ -const MOBILE_HOST_AUTHORITATIVE_COLUMNS = new Map>([ +const HOST_AUTHORITATIVE_COLUMNS_BY_TABLE = new Map>([ ["terminal_sessions", new Set(["settled_at", "settle_override", "settle_source"])], ]); -const isMobileAuthoredHostAuthoritativeColumn = (change: CrsqlChangeRow): boolean => - MOBILE_HOST_AUTHORITATIVE_COLUMNS.get(change.table)?.has(change.cid) ?? false; +const isHostAuthoritativeColumn = (change: CrsqlChangeRow): boolean => + HOST_AUTHORITATIVE_COLUMNS_BY_TABLE.get(change.table)?.has(change.cid) ?? false; const MOBILE_REPLICA_RESEED_EXCLUDED_TABLES = [ ...MOBILE_CHANGESET_EXCLUDED_TABLES, @@ -7627,10 +7627,12 @@ export function createSyncHostService(args: SyncHostServiceArgs) { // tables (e.g. sync_cluster_state) win and flip brain ownership. // Settle-authority guard: never let a phone's optimistic settle column // merge over the host's decision (older iOS builds still write them). - const dropMobileSettleColumns = isMobileChangesetPeer(peer); + // The two rules are not symmetric: the table rule applies to every + // peer, the column rule only to phones. + const isPhonePeer = isMobileChangesetPeer(peer); const filtered = changes.filter((change) => { if (isHostAuthoritativeTable(change)) return false; - if (dropMobileSettleColumns && isMobileAuthoredHostAuthoritativeColumn(change)) return false; + if (isPhonePeer && isHostAuthoritativeColumn(change)) return false; return true; }); try { diff --git a/apps/ios/ADE/Services/Database.swift b/apps/ios/ADE/Services/Database.swift index 17c1dc571..dc6cbe8a1 100644 --- a/apps/ios/ADE/Services/Database.swift +++ b/apps/ios/ADE/Services/Database.swift @@ -2099,14 +2099,9 @@ final class DatabaseService { /// flickering back for a round trip. /// /// The settle columns (`settled_at`, `settle_override`, `settle_source`) are - /// deliberately NOT writable from here. `terminal_sessions` is a CRR table - /// whose local writes replicate upstream, so an optimistic settle carries no - /// host revision and can defeat a host-side rejection by CRDT merge — the host - /// leaves `settled_at` null, and the phone's row settles it anyway. Settle is - /// host-authoritative; the phone shows a local pending state instead - /// (`SyncService.pendingSessionSettleStates`) and waits for the host's - /// changeset. See `docs/features/terminals-and-sessions/settle-teardown-design.md` - /// §3c-i. + /// deliberately NOT writable from here — they are host-authoritative and a + /// replicating write can defeat a host rejection by CRDT merge. See + /// `PendingSessionSettleStates.swift` for the replacement. /// /// Each parameter is a two-level optional so "leave alone" and "clear" are /// distinguishable: `nil` skips the column, `.some(nil)` sets it to NULL, diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index 908fa7179..4df0b69dd 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -2,77 +2,95 @@ import Foundation /// A settle-family change the phone has sent to the host and is still waiting on. /// -/// The phone must NOT write `settled_at` / `settle_override` / `settle_source` -/// into its own replica. `terminal_sessions` is a CRR table, so a local write -/// replicates upstream carrying no host lifecycle revision — which means an -/// optimistic settle can win a CRDT merge against a host that *rejected* the -/// settle, and file a live session as done. See -/// `docs/features/terminals-and-sessions/settle-teardown-design.md` §3c-i. -/// -/// This is what replaces the write: a purely local overlay applied when session -/// rows are read, so the row responds immediately while the command is in -/// flight, and nothing leaves the device. +/// The phone does not write the settle columns into its replica — they are +/// host-authoritative, and a replicating write can defeat a host rejection by +/// CRDT merge. See the invariant in +/// `docs/features/terminals-and-sessions/README.md` ("Gotchas") for the full +/// argument. This overlay is what replaces that write: purely local, applied +/// when session rows are read, so the row responds immediately while the +/// command is in flight and nothing leaves the device. /// /// `settle_source` is deliberately absent. No iOS surface reads it, so /// overlaying it would buy nothing and only add a value to guess wrong. struct PendingSessionSettleIntent: Equatable { - /// Two-level optionals mirror the column semantics: `nil` leaves the column - /// alone, `.some(nil)` shows it cleared, `.some(value)` shows that value. - var settledAt: String?? - var settleOverride: String?? + /// The three things a settle-family command can ask for. Modelled as a closed + /// set rather than per-column optionals so an intent cannot be built that + /// means nothing, and so each one's confirmation rule is stated once. + enum Kind: Equatable { + /// `timestamp` is only what we display while waiting; the host stamps its own. + case settle(timestamp: String) + case unsettle + case override(String?) + } + + var kind: Kind /// When the command was sent, for the staleness backstop. var startedAt: Date + /// Identifies this specific command, so a slow one's failure cannot retire an + /// intent the user has since replaced. Assigned by `begin`. + fileprivate var token: UInt64 = 0 static func settle(now: Date, timestamp: String) -> PendingSessionSettleIntent { - // A declared settle also clears a `"settled"` pin host-side. An `"active"` - // pin survives, but a row carrying one cannot be settled from this menu in - // the first place, so showing the override cleared is not a lie the user - // can reach. - PendingSessionSettleIntent(settledAt: .some(timestamp), settleOverride: .some(nil), startedAt: now) + PendingSessionSettleIntent(kind: .settle(timestamp: timestamp), startedAt: now) } static func unsettle(now: Date) -> PendingSessionSettleIntent { - // Only `settled_at`. The host clears a `"settled"` override but preserves an - // `"active"` one, and the phone cannot know which branch it will take — - // exactly the reasoning that already kept `settle_override` out of the old - // optimistic write. - PendingSessionSettleIntent(settledAt: .some(nil), settleOverride: nil, startedAt: now) + PendingSessionSettleIntent(kind: .unsettle, startedAt: now) } static func settleOverride(_ value: String?, now: Date) -> PendingSessionSettleIntent { - PendingSessionSettleIntent(settledAt: nil, settleOverride: .some(value), startedAt: now) - } - - /// Whether the host's replicated row now reflects this intent. - /// - /// The two columns are checked differently on purpose. `settled_at` carries - /// the *host's* timestamp, so only its presence is ours to predict — matching - /// on the exact string would never resolve. `settle_override` is an exact - /// value we asked for, so it is compared as one. - func isSatisfied(by session: TerminalSessionSummary) -> Bool { - if let settledAt { - let intended = PendingSessionSettleIntent.normalized(settledAt) != nil - guard (PendingSessionSettleIntent.normalized(session.settledAt) != nil) == intended else { return false } - } - if let settleOverride { - guard PendingSessionSettleIntent.normalized(session.settleOverride) - == PendingSessionSettleIntent.normalized(settleOverride) else { return false } - } - return true + PendingSessionSettleIntent(kind: .override(value), startedAt: now) } func applied(to session: TerminalSessionSummary) -> TerminalSessionSummary { var next = session - if let settledAt { - next.settledAt = settledAt - } - if let settleOverride { - next.settleOverride = settleOverride + switch kind { + case .settle(let timestamp): + // A declared settle also clears any override host-side, including an + // `"active"` pin — `sessionService.settleMany` / `settleSession` both set + // `settle_override = null` unconditionally, so that a pin cannot silently + // veto the settle the user just asked for. + next.settledAt = timestamp + next.settleOverride = nil + case .unsettle: + next.settledAt = nil + // The host clears a `"settled"` override and PRESERVES an `"active"` pin + // (`settle_override = case when settle_override = 'settled' then null else + // settle_override end`). Which branch it takes is decided by the value + // already in the row, so we can predict it exactly rather than guess — + // and must, because a row settled purely BY that pin has a null + // `settled_at` already, so clearing the timestamp alone would show the + // user nothing at all. + if PendingSessionSettleIntent.normalized(session.settleOverride) == "settled" { + next.settleOverride = nil + } + case .override(let value): + next.settleOverride = value } return next } - static func normalized(_ value: String?) -> String? { + /// Whether the host's replicated row now reflects this intent. + func isSatisfied(by session: TerminalSessionSummary) -> Bool { + let settledAt = PendingSessionSettleIntent.normalized(session.settledAt) + let override = PendingSessionSettleIntent.normalized(session.settleOverride) + switch kind { + case .settle: + // `settled_at` carries the HOST's timestamp, so only its presence is ours + // to predict — matching the exact string would never resolve. The cleared + // override is ours to predict, because the host always clears it. + return settledAt != nil && override == nil + case .unsettle: + // A `"settled"` pin still on the row means the host has not applied the + // unsettle yet, even though `settled_at` may already read null. + return settledAt == nil && override != "settled" + case .override(let value): + // Unlike the settle timestamp, this is an exact value we asked for. + return override == PendingSessionSettleIntent.normalized(value) + } + } + + fileprivate static func normalized(_ value: String?) -> String? { guard let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines), !trimmed.isEmpty else { return nil } return trimmed } @@ -86,54 +104,89 @@ struct PendingSessionSettleStates: Equatable { /// Backstop for an intent whose confirming changeset never arrives — a host /// that applied the settle but dropped the connection before replicating it, /// say. The overlay is a bridge across one round trip, not durable state, so - /// it expires rather than lying indefinitely. + /// it expires rather than lying indefinitely. Measures reachable time, not + /// wall clock — see `holdBackstop`. static let staleAfter: TimeInterval = 20 private var intents: [String: PendingSessionSettleIntent] = [:] + private var nextToken: UInt64 = 0 init() {} var isEmpty: Bool { intents.isEmpty } - subscript(sessionId: String) -> PendingSessionSettleIntent? { intents[normalizedKey(sessionId)] } + subscript(sessionId: String) -> PendingSessionSettleIntent? { intents[sessionId] } - mutating func begin(_ intent: PendingSessionSettleIntent, for sessionId: String) { - let key = normalizedKey(sessionId) - guard !key.isEmpty else { return } - intents[key] = intent + /// Replaces any intent already in flight for the session: the newest command + /// is the one the user is waiting on. Returns the token that identifies it. + @discardableResult + mutating func begin(_ intent: PendingSessionSettleIntent, for sessionId: String) -> UInt64 { + nextToken &+= 1 + var stamped = intent + stamped.token = nextToken + intents[sessionId] = stamped + return nextToken } - /// Drop an intent because the command failed. The row snaps back to whatever + /// Drop an intent because its command failed. The row snaps back to whatever /// the host actually has, which is the honest answer. - mutating func clear(_ sessionId: String) { - intents.removeValue(forKey: normalizedKey(sessionId)) + /// + /// Scoped by token: two commands for one session can overlap (tap "Keep + /// active", then "Settle" before the first returns), and the loser's failure + /// must not retire the intent the user is now waiting on. + mutating func clear(_ sessionId: String, token: UInt64) { + guard intents[sessionId]?.token == token else { return } + intents.removeValue(forKey: sessionId) + } + + /// Forget everything in flight — used when the ground the overlay refers to + /// moves, e.g. a project or host switch, where the session ids it holds no + /// longer describe what is on screen. + mutating func removeAll() { + intents.removeAll() + } + + /// Hold every deadline open because the host is unreachable. + /// + /// Nothing can confirm an intent while we cannot talk to the host, so ageing + /// one out would only mean forgetting a command that is still on its way: a + /// settle taken offline is durably queued by `enqueueOperation` and can sit + /// for minutes. + /// + /// The deadline is sampled at each read rather than integrated, so a flapping + /// connection can stretch the real elapsed time well past `staleAfter`. That + /// is the safe direction — the durable queue means the command is still + /// coming — and it is why `staleAfter` is a backstop rather than a promise. + /// + /// Returns nothing on purpose — this can never resolve an intent, so it can + /// never be a reason to repaint. + mutating func holdBackstop(now: Date) { + for key in intents.keys { + intents[key]?.startedAt = now + } } /// Drop intents the host has now confirmed, plus any that outlived the /// backstop. Sessions absent from `sessions` are left alone — a partial or /// scoped read must not be mistaken for "the host disagrees". + /// + /// Returns whether anything was retired, so the caller can repaint — an + /// expiry changes what the row should show and no database write accompanies + /// it. @discardableResult mutating func prune(against sessions: [TerminalSessionSummary], now: Date) -> Bool { guard !intents.isEmpty else { return false } - var next = intents + let before = intents.count for session in sessions { - let key = normalizedKey(session.id) - guard let intent = next[key] else { continue } - if intent.isSatisfied(by: session) { - next.removeValue(forKey: key) - } - } - for (key, intent) in next - where now.timeIntervalSince(intent.startedAt) >= PendingSessionSettleStates.staleAfter { - next.removeValue(forKey: key) + guard let intent = intents[session.id], intent.isSatisfied(by: session) else { continue } + intents.removeValue(forKey: session.id) } - guard next != intents else { return false } - intents = next - return true + intents = intents.filter { now.timeIntervalSince($0.value.startedAt) < PendingSessionSettleStates.staleAfter } + return intents.count != before } func apply(to session: TerminalSessionSummary) -> TerminalSessionSummary { - guard let intent = intents[normalizedKey(session.id)] else { return session } + guard let intent = intents[session.id] else { return session } return intent.applied(to: session) } @@ -141,8 +194,4 @@ struct PendingSessionSettleStates: Equatable { guard !intents.isEmpty else { return sessions } return sessions.map { apply(to: $0) } } - - private func normalizedKey(_ sessionId: String) -> String { - sessionId.trimmingCharacters(in: .whitespacesAndNewlines) - } } diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 59b1487db..85f3bbaba 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -5120,6 +5120,7 @@ final class SyncService: ObservableObject { let scopeChanged = previousProjectId != nextProjectId || previousRootPath != nextRootPath if scopeChanged { resetCtoAttentionForProjectScopeChange() + resetPendingSessionSettleStates() cancelAllTerminalSnapshotRecovery() terminalSnapshotRequestTokens.removeAll() prepareOutboundStateForProjectScopeChange() @@ -8451,6 +8452,7 @@ final class SyncService: ObservableObject { saveProfile(nil) saveRemoteCommandDescriptors([]) clearPendingChatCreations() + resetPendingSessionSettleStates() resetChatEventState(clearHistory: true) resetTerminalSubscriptionState(clearHistory: true) activeHostProfile = nil @@ -9145,25 +9147,55 @@ final class SyncService: ObservableObject { } func fetchSessions() async throws -> [TerminalSessionSummary] { - sessionsWithPendingSettleOverlay(database.fetchSessions()) + localSessions() } func fetchSession(id sessionId: String) async throws -> TerminalSessionSummary? { - guard let session = database.fetchSession(id: sessionId) else { return nil } - return sessionsWithPendingSettleOverlay([session]).first + localSession(id: sessionId) } - /// Retire confirmed/stale settle intents against these rows, then show the - /// still-pending ones. Every session read goes through here, so an intent - /// cannot outlive the host's answer to it. - private func sessionsWithPendingSettleOverlay( - _ sessions: [TerminalSessionSummary] - ) -> [TerminalSessionSummary] { + /// **The session read chokepoint.** Every read that feeds UI must come through + /// here or `localSession(id:)`, never `database.fetchSessions()` directly: + /// these are the only readers that see an in-flight settle, and a raw read + /// silently renders the row as if the user had never tapped settle. The one + /// exception that reads lifecycle *columns* is the pre-command snapshot in + /// `sendSessionSnoozeCommand`, which wants the un-overlaid row; the existence + /// check in the session navigation destination is unaffected because the + /// overlay only rewrites columns and never adds or drops rows. + /// + /// Not a pure read: it retires resolved intents and may schedule a repaint. + /// Safe from a render path — the bump is debounced, and the pass it triggers + /// re-prunes, finds nothing, and stops. + private func localSessions() -> [TerminalSessionSummary] { + let sessions = database.fetchSessions() guard !pendingSessionSettleStates.isEmpty else { return sessions } prunePendingSessionSettleStates(against: sessions) return pendingSessionSettleStates.apply(to: sessions) } + private func localSession(id sessionId: String) -> TerminalSessionSummary? { + guard let session = database.fetchSession(id: sessionId) else { return nil } + guard !pendingSessionSettleStates.isEmpty else { return session } + prunePendingSessionSettleStates(against: [session]) + return pendingSessionSettleStates.apply(to: session) + } + + /// Retire confirmed or expired intents, and repaint if any went away. An + /// expiry is the one resolution with no accompanying database write, so + /// without this nudge it would only become visible on the next unrelated + /// read — which against a quiet host may be a long time. + private func prunePendingSessionSettleStates(against sessions: [TerminalSessionSummary]) { + let now = Date() + // Hold first, then measure: after a hold every deadline is `now`, so the + // backstop cannot fire against a command that is merely waiting for the + // connection to come back. + if !canSendLiveRequests() { + pendingSessionSettleStates.holdBackstop(now: now) + } + guard pendingSessionSettleStates.prune(against: sessions, now: now) else { return } + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + } + /// Best-effort hydration for a session whose local DB row may not have synced /// yet — e.g. a chat just created into (or opened from the hub against) a /// project that was activated in place, where the switch flips the active @@ -9175,14 +9207,10 @@ final class SyncService: ObservableObject { /// state. @discardableResult func ensureSessionRowHydrated(sessionId: String) async -> TerminalSessionSummary? { - if let existing = database.fetchSession(id: sessionId) { - return sessionsWithPendingSettleOverlay([existing]).first - } + if let existing = localSession(id: sessionId) { return existing } if canSendLiveRequests() { try? await refreshWorkSessions() - if let refreshed = database.fetchSession(id: sessionId) { - return sessionsWithPendingSettleOverlay([refreshed]).first - } + if let refreshed = localSession(id: sessionId) { return refreshed } } // Absorb changeset lag right after an in-place project activation: the row // arrives via CRDT sync a beat after the switch. Bounded so a genuinely @@ -9190,9 +9218,7 @@ final class SyncService: ObservableObject { for _ in 0..<6 { try? await Task.sleep(nanoseconds: 300_000_000) if Task.isCancelled { break } - if let row = database.fetchSession(id: sessionId) { - return sessionsWithPendingSettleOverlay([row]).first - } + if let row = localSession(id: sessionId) { return row } } return nil } @@ -9273,14 +9299,9 @@ final class SyncService: ObservableObject { // // The two halves are handled differently, and the split is load-bearing: // - // - **Settle columns** (`settled_at`, `settle_override`, `settle_source`) are - // host-authoritative and are NEVER written to the local replica. Because - // `terminal_sessions` is a CRR table, such a write replicates upstream - // carrying no host lifecycle revision, so it can win a CRDT merge against a - // host that *rejected* the settle and file a live session as done. Instant - // feedback comes from `pendingSessionSettleStates`, a local overlay applied - // at read time that resolves when the host's changeset lands or the command - // fails. See the settle-teardown design, §3c-i. + // - **Settle columns** are host-authoritative and NEVER written to the local + // replica; instant feedback comes from `pendingSessionSettleStates` + // (`PendingSessionSettleStates.swift`, which carries the full argument). // - **Snooze overlay** (`snoozed_until`, `snoozed_at`, `woke_*`) still writes // optimistically with a rollback. Those columns are not guarded by a host // revision and have no teardown attached, so a merge cannot defeat a host @@ -9288,27 +9309,37 @@ final class SyncService: ObservableObject { /// In-flight settle intents, applied over session reads so a settle feels /// immediate without a replicating write. Never persisted. - private(set) var pendingSessionSettleStates = PendingSessionSettleStates() + private var pendingSessionSettleStates = PendingSessionSettleStates() /// Record an in-flight settle intent and nudge the projections, mirroring the /// re-render the optimistic DB write used to trigger through /// `adeDatabaseDidChange`. - private func beginPendingSessionSettle(_ intent: PendingSessionSettleIntent, for sessionId: String) { - pendingSessionSettleStates.begin(intent, for: sessionId) + private func beginPendingSessionSettle( + _ intent: PendingSessionSettleIntent, + for sessionId: String + ) -> UInt64 { + let token = pendingSessionSettleStates.begin(intent, for: sessionId) scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + return token } - private func clearPendingSessionSettle(_ sessionId: String) { + private func clearPendingSessionSettle(_ sessionId: String, token: UInt64) { guard pendingSessionSettleStates[sessionId] != nil else { return } - pendingSessionSettleStates.clear(sessionId) + pendingSessionSettleStates.clear(sessionId, token: token) scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) } - /// Retire intents the host's replicated rows have now confirmed (or that - /// outlived the staleness backstop). Called from every session read so the - /// overlay cannot outlive its answer. - private func prunePendingSessionSettleStates(against sessions: [TerminalSessionSummary]) { - pendingSessionSettleStates.prune(against: sessions, now: Date()) + /// Drop every in-flight intent because the ground beneath it moved — a project + /// switch, or an unpair that clears the credentials. The ids it holds describe + /// sessions that are no longer on screen, and on unpair the host is + /// permanently unreachable, so `holdBackstop` would otherwise keep the overlay + /// painting for the rest of the app's life. + private func resetPendingSessionSettleStates() { + guard !pendingSessionSettleStates.isEmpty else { return } + pendingSessionSettleStates.removeAll() + // The project-switch caller reloads everything anyway; the unpair caller + // does not, so repaint here rather than relying on the caller. + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) } /// Whether this host advertises the ADE-125 lifecycle actions at all. Older @@ -9337,69 +9368,21 @@ final class SyncService: ObservableObject { ]) } - /// Host command for a lifecycle change, with an optional optimistic local - /// write of the SNOOZE overlay only, rolled back on failure. + /// Send a lifecycle host command and undo local optimism if it does not take. /// - /// The settle columns are not parameters here and must not become ones — they - /// are host-authoritative (see the section comment above). Settle callers pass - /// a `pendingSettle` intent instead, which is local-only. + /// The core knows nothing about WHICH optimism was applied — settle and snooze + /// apply very different kinds (a local overlay vs. a replicating column write) + /// and each caller hands in its own `rollback`. private func sendSessionLifecycleCommand( - sessionId: String, + sessionId trimmed: String, action: String, args: [String: Any], // How this action reports whether it actually changed the row; `nil` for // the actions that report nothing usable, mirroring the desktop // `lifecycleCall` call sites that pass no `applied` predicate. resultShape: SessionLifecycleResultShape? = .envelope, - pendingSettle: PendingSessionSettleIntent? = nil, - snoozedUntil: String?? = nil, - snoozedAt: String?? = nil, - wokeAt: String?? = nil, - wokeReason: String?? = nil + rollback: @escaping () -> Void ) async throws { - let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) - guard !trimmed.isEmpty else { return } - guard supportsRemoteAction(action) else { - throw sessionLifecycleUnsupportedError(action) - } - - let previous = database.fetchSession(id: trimmed) - if let pendingSettle { - beginPendingSessionSettle(pendingSettle, for: trimmed) - } - try? database.updateSessionSnoozeOverlay( - sessionId: trimmed, - snoozedUntil: snoozedUntil, - snoozedAt: snoozedAt, - wokeAt: wokeAt, - wokeReason: wokeReason - ) - - // Undo the optimistic snooze write and drop the local settle intent. - // Restores ONLY the columns this call actually assigned: `terminal_sessions` - // is a CRR table whose local writes replicate upstream, so re-stamping a - // column we never touched would push our stale copy of a host-owned value - // back over whatever the machine has since written — the same hazard that - // keeps `snoozed_at` out of the forward write below. A half-applied - // lifecycle is still worse than none, so every column we DID write is - // restored together. - func rollback() { - if pendingSettle != nil { - clearPendingSessionSettle(trimmed) - } - guard let previous else { return } - func restored(_ requested: String??, _ value: String?) -> String?? { - requested == nil ? nil : .some(value) - } - try? database.updateSessionSnoozeOverlay( - sessionId: trimmed, - snoozedUntil: restored(snoozedUntil, previous.snoozedUntil), - snoozedAt: restored(snoozedAt, previous.snoozedAt), - wokeAt: restored(wokeAt, previous.wokeAt), - wokeReason: restored(wokeReason, previous.wokeReason) - ) - } - let scope = chatCommandScope(for: trimmed) let result: Any do { @@ -9425,6 +9408,88 @@ final class SyncService: ObservableObject { } } + /// Settle-family command: shows the change through the local overlay, which is + /// dropped if the host does not take it. + private func sendSessionSettleCommand( + sessionId: String, + action: String, + args: [String: Any], + resultShape: SessionLifecycleResultShape? = .envelope, + intent: PendingSessionSettleIntent + ) async throws { + guard let trimmed = normalizedLifecycleSessionId(sessionId) else { return } + guard supportsRemoteAction(action) else { throw sessionLifecycleUnsupportedError(action) } + let token = beginPendingSessionSettle(intent, for: trimmed) + try await sendSessionLifecycleCommand( + sessionId: trimmed, + action: action, + args: args, + resultShape: resultShape, + rollback: { [weak self] in self?.clearPendingSessionSettle(trimmed, token: token) } + ) + } + + /// Snooze-family command: writes the snooze overlay columns optimistically and + /// restores them if the host does not take it. + /// + /// The rollback restores ONLY the columns this call actually assigned. + /// `terminal_sessions` is a CRR table whose local writes replicate upstream, so + /// re-stamping a column we never touched would push our stale copy of a + /// host-owned value back over whatever the machine has since written — the same + /// hazard that keeps `snoozed_at` out of the forward write. A half-applied + /// lifecycle is still worse than none, so every column we DID write is restored + /// together. + private func sendSessionSnoozeCommand( + sessionId: String, + action: String, + args: [String: Any], + resultShape: SessionLifecycleResultShape? = .envelope, + snoozedUntil: String?? = nil, + snoozedAt: String?? = nil, + wokeAt: String?? = nil, + wokeReason: String?? = nil + ) async throws { + guard let trimmed = normalizedLifecycleSessionId(sessionId) else { return } + guard supportsRemoteAction(action) else { throw sessionLifecycleUnsupportedError(action) } + + // Deliberately the RAW row, not `localSession(id:)`: this is the snapshot the + // rollback restores, so it must be what the database actually holds rather + // than what an in-flight settle overlay is painting. + let previous = database.fetchSession(id: trimmed) + try? database.updateSessionSnoozeOverlay( + sessionId: trimmed, + snoozedUntil: snoozedUntil, + snoozedAt: snoozedAt, + wokeAt: wokeAt, + wokeReason: wokeReason + ) + + try await sendSessionLifecycleCommand( + sessionId: trimmed, + action: action, + args: args, + resultShape: resultShape, + rollback: { [weak self] in + guard let self, let previous else { return } + func restored(_ requested: String??, _ value: String?) -> String?? { + requested == nil ? nil : .some(value) + } + try? self.database.updateSessionSnoozeOverlay( + sessionId: trimmed, + snoozedUntil: restored(snoozedUntil, previous.snoozedUntil), + snoozedAt: restored(snoozedAt, previous.snoozedAt), + wokeAt: restored(wokeAt, previous.wokeAt), + wokeReason: restored(wokeReason, previous.wokeReason) + ) + } + ) + } + + private func normalizedLifecycleSessionId(_ sessionId: String) -> String? { + let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) + return trimmed.isEmpty ? nil : trimmed + } + /// Declared settle. The host owns `settled_at`; this only shows the row as /// settled locally until the host's changeset answers. /// @@ -9432,7 +9497,7 @@ final class SyncService: ObservableObject { /// settle" — and it must ONLY be passed for a row that genuinely has a pending /// prompt: the host throws "Resolve the terminal input before settling this /// session." when the flag arrives for a row with nothing pending, which would - /// roll back the optimistic write and surface an error on an ordinary settle. + /// drop the pending overlay and surface an error on an ordinary settle. /// /// It rides on the SAME plural action the plain settle uses. The singular /// `session.settleSession` also accepts the flag but is not in the mobile @@ -9444,17 +9509,15 @@ final class SyncService: ObservableObject { if dismissPendingInput { args["dismissPendingInput"] = true } - try await sendSessionLifecycleCommand( + let now = Date() + try await sendSessionSettleCommand( sessionId: sessionId, action: "session.settleSessions", args: args, // The bulk action answers with the ids it CHANGED, so an absent id means // the machine settled nothing. Mirrors the desktop `settleMany`. resultShape: .changedIdList, - pendingSettle: .settle( - now: Date(), - timestamp: iso8601WithFractionalSecondsFormatter.string(from: Date()) - ) + intent: .settle(now: now, timestamp: iso8601WithFractionalSecondsFormatter.string(from: now)) ) } @@ -9468,7 +9531,7 @@ final class SyncService: ObservableObject { /// it may not get would just be a local lie in place of the replicated one /// this used to be. Mirrors the web overlay's `UNSETTLE_PATCH`. func unsettleSession(sessionId: String) async throws { - try await sendSessionLifecycleCommand( + try await sendSessionSettleCommand( sessionId: sessionId, action: "session.unsettleSessions", args: ["sessionIds": [sessionId]], @@ -9476,20 +9539,20 @@ final class SyncService: ObservableObject { // per-row verdict to check — so there is nothing to reject, exactly like // the desktop `unsettleMany`, which passes no `applied` predicate. resultShape: nil, - pendingSettle: .unsettle(now: Date()) + intent: .unsettle(now: Date()) ) } /// Set (or clear, with `nil`) the tri-state settle override. `"active"` is the /// "keep active" pin that suppresses an explicit settle. func setSessionSettleOverride(sessionId: String, override: SessionSettleOverride?) async throws { - try await sendSessionLifecycleCommand( + try await sendSessionSettleCommand( sessionId: sessionId, action: "session.setSettleOverride", // The host reads "clear" as null; sending a JSON null through the // `[String: Any]` arg dictionary is not representable here. args: ["sessionId": sessionId, "override": override?.rawValue ?? "clear"], - pendingSettle: .settleOverride(override?.rawValue, now: Date()) + intent: .settleOverride(override?.rawValue, now: Date()) ) } @@ -9514,7 +9577,7 @@ final class SyncService: ObservableObject { /// `unsettleSession` leaving `settle_override` to the machine. func snoozeSession(sessionId: String, until deadline: Date) async throws { let untilIso = iso8601WithFractionalSecondsFormatter.string(from: deadline) - try await sendSessionLifecycleCommand( + try await sendSessionSnoozeCommand( sessionId: sessionId, action: "session.snoozeSession", args: ["sessionId": sessionId, "untilIso": untilIso], @@ -9526,7 +9589,7 @@ final class SyncService: ObservableObject { /// Wake a snoozed session now, recording why. func wakeSession(sessionId: String, reason: SessionWakeReason = .manual) async throws { - try await sendSessionLifecycleCommand( + try await sendSessionSnoozeCommand( sessionId: sessionId, action: "session.wakeSession", args: ["sessionId": sessionId, "reason": reason.rawValue], @@ -9539,7 +9602,7 @@ final class SyncService: ObservableObject { /// Drop the "woke" marker once the user has visited the row. func clearSessionWokeMarker(sessionId: String) async throws { - try await sendSessionLifecycleCommand( + try await sendSessionSnoozeCommand( sessionId: sessionId, action: "session.clearWokeMarker", args: ["sessionId": sessionId], @@ -19806,7 +19869,13 @@ extension SyncService { /// and schedule a debounced snapshot write so widgets + live activities pick /// up the delta. func refreshActiveSessionsAndSnapshot() { - let sessions = database.fetchSessions() + // Through the chokepoint: this drops `.settled` rows below, and a plain + // settle the user just tapped must disappear from the widget and Live + // Activity immediately rather than waiting for the host's changeset. (A + // "Dismiss & settle" still waits: `needs_you` outranks the settled tier and + // the overlay does not touch the attention columns, which only the host + // clears.) + let sessions = localSessions() let now = Date() // `activeSessions` holds every relevant chat session — running, @@ -20927,7 +20996,7 @@ extension SyncService { guard let projectId = activeProjectId else { return nil } let lanes = database.fetchLanes(includeArchived: false) let visibleLaneIds = Set(lanes.map(\.id)) - let scopedSessions = sessionsWithPendingSettleOverlay(database.fetchSessions()).filter { session in + let scopedSessions = localSessions().filter { session in session.archivedAt == nil && visibleLaneIds.contains(session.laneId) } let topLevelIds = Set(scopedSessions.filter { isRosterTopLevelToolType($0.toolType) }.map(\.id)) diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index c632ea15b..61434107f 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -48,25 +48,58 @@ final class PendingSessionSettleStatesTests: XCTestCase { var states = PendingSessionSettleStates() states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") - let overlaid = states.apply(to: session()) + // A declared settle clears ANY override host-side — including a keep-active + // pin, so it cannot silently veto the settle — and the overlay does too. + let overlaid = states.apply(to: session(settleOverride: "active")) XCTAssertEqual(overlaid.settledAt, "2026-08-10T12:00:00.000Z") - // A declared settle clears a `"settled"` pin host-side, so the overlay - // shows that too. XCTAssertNil(overlaid.settleOverride) } - func testUnsettleIntentLeavesTheOverrideToTheHost() { + func testUnsettleLeavesAKeepActivePinAlone() { var states = PendingSessionSettleStates() states.begin(.unsettle(now: now), for: "session-1") - // The host clears a `"settled"` override but PRESERVES an `"active"` pin, - // and the phone cannot know which branch it takes — so the overlay must not - // claim either. + // The host PRESERVES an `"active"` pin through an unsettle, so the overlay + // must not claim it was cleared. let overlaid = states.apply(to: session(settledAt: "2026-08-10T09:00:00.000Z", settleOverride: "active")) XCTAssertNil(overlaid.settledAt) XCTAssertEqual(overlaid.settleOverride, "active") } + /// A row settled purely BY a `"settled"` pin has a null `settled_at` already, + /// so clearing the timestamp alone would show the user nothing at all. Which + /// branch the host takes is decided by the value already in the row, so the + /// overlay can predict it exactly. + func testUnsettleClearsASettledPinBecauseTheHostWill() { + var states = PendingSessionSettleStates() + states.begin(.unsettle(now: now), for: "session-1") + + let overlaid = states.apply(to: session(settleOverride: "settled")) + XCTAssertNil(overlaid.settleOverride) + + // And it must not resolve while that pin is still on the replicated row. + states.prune(against: [session(settleOverride: "settled")], now: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session()], now: now) + XCTAssertNil(states["session-1"]) + } + + func testSettleResolvesOnlyOnceTheHostAlsoClearedTheOverride() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + // `sessionService.settleMany` / `settleSession` both set + // `settle_override = null` unconditionally, so that a keep-active pin cannot + // silently veto the settle the user asked for. A row that still carries one + // has therefore not applied our settle yet. + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z", settleOverride: "active")], now: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + XCTAssertNil(states["session-1"]) + } + func testIntentResolvesOnTheHostsOwnTimestampNotOurs() { var states = PendingSessionSettleStates() states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") @@ -76,7 +109,6 @@ final class PendingSessionSettleStatesTests: XCTestCase { states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) XCTAssertNil(states["session-1"]) - XCTAssertTrue(states.isEmpty) } func testIntentSurvivesUntilTheHostRowActuallyChanges() { @@ -125,13 +157,26 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAFailedCommandDropsTheIntentSoTheRowSnapsBack() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + let token = states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") - states.clear("session-1") + states.clear("session-1", token: token) XCTAssertNil(states.apply(to: session()).settledAt) } + /// Two commands for one session can overlap — tap "Keep active", then "Settle" + /// before the first returns. The loser's failure must not retire the intent + /// the user is now waiting on. + func testAStaleFailureCannotRetireANewerIntent() { + var states = PendingSessionSettleStates() + let stale = states.begin(.settleOverride("active", now: now), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + states.clear("session-1", token: stale) + + XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") + } + func testAnIntentWhoseChangesetNeverArrivesExpires() { var states = PendingSessionSettleStates() states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") @@ -145,6 +190,70 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(states["session-1"], "a pending overlay must not outlive its round trip indefinitely") } + /// A settle taken offline is durably queued and can sit for minutes. Ageing + /// it out on wall clock would snap the row back to unsettled while the + /// command is still on its way, then settle it again when the queue drains. + func testAQueuedSettleDoesNotExpireWhileTheHostIsUnreachable() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + var clock = now + for _ in 0..<10 { + clock = clock.addingTimeInterval(PendingSessionSettleStates.staleAfter) + states.holdBackstop(now: clock) + states.prune(against: [session(settledAt: nil)], now: clock) + } + + XCTAssertNotNil(states["session-1"], "an unreachable host cannot confirm, so the backstop must not run") + XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") + } + + func testTheBackstopResumesOnceTheHostIsReachableAgain() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + // Offline for well past the budget, then reachable: the clock restarts from + // the moment we could have been answered, not from the tap. + let reconnectedAt = now.addingTimeInterval(600) + states.holdBackstop(now: reconnectedAt) + states.prune(against: [session(settledAt: nil)], now: reconnectedAt) + XCTAssertNotNil(states["session-1"]) + + let past = reconnectedAt.addingTimeInterval(PendingSessionSettleStates.staleAfter) + states.prune(against: [session(settledAt: nil)], now: past) + XCTAssertNil(states["session-1"]) + } + + func testPruneReportsOnlyRealResolutionsSoRepaintCannotLoop() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + // Re-stamping the offline deadline is not a resolution; reporting it as one + // would repaint on every read forever. + states.holdBackstop(now: now) + XCTAssertFalse(states.prune(against: [session(settledAt: nil)], now: now)) + XCTAssertTrue(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now)) + XCTAssertFalse(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now)) + } + + func testTheNewestCommandWins() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.unsettle(now: now), for: "session-1") + + XCTAssertNil(states.apply(to: session(settledAt: "2026-08-10T09:00:00.000Z")).settledAt) + } + + func testRemoveAllForgetsEverythingInFlight() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + + states.removeAll() + + XCTAssertTrue(states.isEmpty) + XCTAssertNil(states.apply(to: session()).settledAt) + } + func testASessionMissingFromAScopedReadKeepsItsIntent() { var states = PendingSessionSettleStates() states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 90097cbeb..2b37dacee 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -213,13 +213,13 @@ and in tests. every guard tried against it either read a column that turn-start never updates or had to be repeated at each of the settle entry points. Making settle stop work needs a synchronous lifecycle revision that teardown can be - serialized against; it is not a wrapper around the existing write. Archive is - the one lifecycle path that does stop processes — see - `laneService.archive`, where the ordering is load-bearing. The approved plan - for making settle stop work is + serialized against; it is not a wrapper around the existing write. The approved + plan for doing it is [settle-teardown-design.md](settle-teardown-design.md); its step 0 - precondition — `settled_at` becoming host-authoritative, so no replica can - defeat the revision guard by CRDT merge — has landed. + precondition — `settled_at` becoming host-authoritative, so no replica will be + able to defeat the coming revision guard by CRDT merge — has landed. Archive is + the one lifecycle path that does stop processes — see + `laneService.archive`, where the ordering is load-bearing. `dismissPendingInput: true` first quiets an SDK chat through `agentChatService`, or clears a tracked CLI's explicit `ade chat ask` marker through `ptyService`; arbitrary native diff --git a/docs/features/terminals-and-sessions/settle-teardown-design.md b/docs/features/terminals-and-sessions/settle-teardown-design.md index e13e15e3e..845f32471 100644 --- a/docs/features/terminals-and-sessions/settle-teardown-design.md +++ b/docs/features/terminals-and-sessions/settle-teardown-design.md @@ -4,7 +4,8 @@ to implement; step 3 (attaching real teardown) waits until 1 and 2 are merged and the race-matrix tests have been seen to pass. -**Step 0 is implemented** — see amendment 6 in §3c-i for the host-side half. +**Step 0 is implemented** — see "Host enforcement for pre-fix clients" in +§3c-i for the host-side half. Settle currently writes a lifecycle column and stops nothing. A session filed as "done" can still own a background shell, a subagent fleet, or a Cursor cloud run @@ -191,40 +192,44 @@ smallest honest change. ### 3c-i. Precondition: `settled_at` must become host-authoritative -**Verified, and the chokepoint is currently bypassable.** This is a hard -precondition, not a caveat. +**Verified, and the chokepoint was bypassable. Now closed — this section is +kept because the reasoning still governs step 1.** `terminal_sessions` is a CRR table (it is not in `LOCAL_ONLY_CRR_EXCLUDED_TABLES` -in `kvDb.ts:775`), and iOS writes `settled_at` into its **own replica** -optimistically before sending the remote command: -`apps/ios/ADE/Services/Database.swift:2128` `updateSessionLifecycleLocked` -assigns `settled_at`, called from `SyncService.swift:9286` on the settle path. -The existing code comment there states the hazard outright — *"`terminal_sessions` -is a CRR table whose local writes replicate upstream"* — and the call site does -carry a rollback for a failed remote command. - -Intent is not the problem: every iOS settle *does* route through the host's +in `kvDb.ts`), and iOS *used to write* `settled_at` into its **own replica** +optimistically before sending the remote command, through the lifecycle helper +in `Database.swift` called from the settle path in `SyncService.swift`. The code +comment there already stated the hazard outright — *"`terminal_sessions` is a CRR +table whose local writes replicate upstream"* — and the call site carried a +rollback for a failed remote command. + +Intent was never the problem: every iOS settle *does* route through the host's `session.settle*` remote command, and so through the chokepoint -(`syncRemoteCommandService.ts:4132` → `sessionService.settleSessions`). The -problem is the **replica write racing the chokepoint's decision**. The phone -cannot know the host's revision, so its optimistic `settled_at` carries no -revision bump. If the host's guard *rejects* the settle (a turn started, §3c), -the host leaves `settled_at` null — and the phone's optimistic row still -replicates in and settles it anyway. The guard is defeated by a merge, not by a -caller. - -**Required before the chokepoint lands:** iOS stops writing `settled_at` / -`settle_override` / `settle_source` into its replica. The optimistic -responsiveness those writes buy is preserved with a **local pending-UI state** -(not a CRR write) that resolves when the host's changeset arrives or the command -fails. The rollback path in `SyncService.swift:9286` becomes unnecessary and -should go with it — it exists only to undo a write we will no longer make. +(`syncRemoteCommandService.ts` → `sessionService.settleSessions`). The problem was +the **replica write racing the chokepoint's decision**. The phone cannot know the +host's revision, so its optimistic `settled_at` carried no revision bump. If the +host's guard *rejects* the settle (a turn started, §3c), the host leaves +`settled_at` null — and the phone's optimistic row would still replicate in and +settle it anyway. The guard is defeated by a merge, not by a caller. + +**What landed.** iOS no longer writes `settled_at` / `settle_override` / +`settle_source` into its replica: the lifecycle helper is now +`updateSessionSnoozeOverlay` and cannot express them. The optimistic +responsiveness those writes bought is preserved by `PendingSessionSettleStates` +— a local, non-persisted overlay applied at the session-read chokepoint, which +resolves when the host's changeset confirms the intent, when the command fails, +or via a bounded staleness backstop. + +One correction to the plan as written: **the rollback did not go away.** It was +predicted to become dead, but it is shared with the snooze path, which keeps its +optimistic write. The rollback survives, scoped to the snooze columns, and the +settle path drops its pending overlay instead. Snooze columns (`snoozed_until`, `snoozed_at`, `woke_*`) are out of scope here; -they are written by the same helper but are not guarded by a revision and have +they were written by the same helper but are not guarded by a revision and have no teardown attached. -**Amendment 6 — how the host treats a pre-fix client (implemented, step 0).** +**Host enforcement for pre-fix clients (implemented, step 0).** Removing the write from iOS fixes new builds and nothing else: a paired phone on an older build keeps writing `settled_at` into its replica, and a CRDT merge never reaches the caller a host-side check would guard. Waiting for clients to @@ -246,9 +251,20 @@ between two of a user's own machines. No capability negotiation is involved — no wire shape changes and the client needs to know nothing. The visible consequence for a pre-fix phone is that its optimistic value is now local-only divergence rather than authoritative -corruption, and it self-heals: `refreshWorkSessions` rewrites local rows from the -host's `work.listSessions` payload via `replaceTerminalSessions`. Host authority -wins, which is the whole point of the precondition. +corruption, and it heals when the row next comes back through hydration: +`refreshWorkSessions` rewrites local rows from the host's `work.listSessions` +payload via `replaceTerminalSessions`. That payload is capped (`limit: 200`) and +further filtered to sessions whose lane the phone has hydrated, so a row outside +that window stays locally wrong until it re-enters it. Local-only, never host +corruption. + +**What this filter is not.** `isMobileChangesetPeer` reads the peer's *own* +`hello` metadata, so a peer that declares itself a desktop is not filtered. That +is adequate for the stated threat — an older iOS build, which declares itself +honestly — but it is a compatibility guard, not a security boundary, and it +should not be read as one. The real closure is step 1's host-local lifecycle +revision: a settle write conditional on a revision no replica can author cannot +be won by a merge from any peer, however it identifies itself. ### 3c-ii. Where the revision column lives @@ -352,10 +368,11 @@ three, and that is why it produced a defect every round. ## 5. Sequencing -0. **Precondition (§3c-i):** make `settled_at` host-authoritative — iOS stops - writing the settle columns into its replica and uses a local pending-UI state - instead. Until this lands, a revision-guarded write is defeatable by CRR - merge, so the chokepoint would provide a guarantee it does not actually have. +0. **Precondition (§3c-i) — landed.** `settled_at` is host-authoritative: iOS no + longer writes the settle columns into its replica and uses a local + pending-UI state instead, and the host drops those columns from inbound phone + changesets. Until this landed, a revision-guarded write was defeatable by CRR + merge, so the chokepoint would have provided a guarantee it did not have. 1. Land the chokepoint + lifecycle revision (3a) **alone**, with no teardown. It is pure refactor with a testable invariant: no `settled_at` mutation outside one function, and every mutation bumps the revision. The revision From 25e5e94ed415ce29f9f9032739a5931e2f2317d7 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:50:35 -0400 Subject: [PATCH 03/15] test: pin the chokepoint, and prove the test fails without it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay type was already covered exhaustively, but the defect that actually shipped lived in the WIRING: `refreshActiveSessionsAndSnapshot` read the database directly, so a chat the user had just settled kept reporting as a live agent on the lock-screen widget, the Live Activity, and the Activity drawer. A test of the struct alone cannot see that. Adds three DB-backed tests through a `#if DEBUG` seam — `fetchSessions` and `fetchSession(id:)` overlay the row while the database stays untouched (nothing can replicate), and a settled chat leaves `activeSessions`. The last one was verified against the pre-fix behavior: reverting the read to `database.fetchSessions()` fails it with "a settle the user just tapped must not keep reporting as a live agent". Without that check the test would have been vacuous. Writing it also corrected the fixture rather than the code: a mid-stream chat deliberately does NOT leave the roster, because a declared settle is honored only at rest (`WorkSessionCanonicalState`). The test now uses the at-rest state a user actually settles from. Docs: the sync docs described host authority only at TABLE granularity, so the per-column rule had nowhere to live. `sync-and-multi-device/README.md` gains a host-authoritative-columns section, `crdt-model.md` gains the merge-semantics consequence (a merge is not a caller, so a host-side check cannot guard a replicated column), and `ARCHITECTURE.md`'s `terminal_sessions` row now names the authorship rule. Also corrects an overstatement in my own earlier wording: "no replica can defeat the revision guard" → "a phone replica cannot", since desktop peers still author these columns by design. --- apps/ios/ADE/Services/SyncService.swift | 12 ++ .../PendingSessionSettleStatesTests.swift | 131 ++++++++++++++++++ docs/ARCHITECTURE.md | 2 +- docs/features/sync-and-multi-device/README.md | 59 +++++++- .../sync-and-multi-device/crdt-model.md | 16 +++ .../sync-and-multi-device/ios-companion.md | 20 ++- .../features/terminals-and-sessions/README.md | 19 ++- 7 files changed, 250 insertions(+), 9 deletions(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 85f3bbaba..d73f1a145 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -16130,6 +16130,18 @@ final class SyncService: ObservableObject { } #if DEBUG + /// Seed an in-flight settle intent without a paired host, so the read + /// chokepoint and the surfaces that depend on it can be tested directly. The + /// bug this guards — a reader going to the database instead of the chokepoint + /// — is invisible to a test of the overlay type alone. + @discardableResult + func beginPendingSessionSettleForTesting( + _ intent: PendingSessionSettleIntent, + for sessionId: String + ) -> UInt64 { + beginPendingSessionSettle(intent, for: sessionId) + } + func seedRemoteProjectCatalogForTesting(_ catalog: [MobileProjectSummary]) { remoteProjectCatalog = catalog refreshProjectCatalog(preferRemoteSelection: true) diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 61434107f..977eed6c4 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -274,3 +274,134 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(others[1].settledAt) } } + +/// The overlay type is exhaustively covered above, but the defect that actually +/// shipped was in the WIRING: a reader that went to the database instead of the +/// read chokepoint, so a settle the user had just tapped stayed visible as a +/// live agent on the widget, the Live Activity, and the Activity drawer. These +/// pin the chokepoint itself. +final class PendingSessionSettleOverlayWiringTests: XCTestCase { + private func makeLane(id: String) -> LaneSummary { + LaneSummary( + id: id, name: "Lane", description: nil, laneType: "worktree", baseRef: "main", + branchRef: "feature/\(id)", worktreePath: "/tmp/\(id)", attachedRootPath: nil, + parentLaneId: nil, childCount: 0, stackDepth: 0, parentStatus: nil, isEditProtected: false, + status: LaneStatus(dirty: false, ahead: 0, behind: 0, remoteBehind: 0, rebaseInProgress: false), + color: nil, icon: nil, tags: [], folder: nil, linearIssue: nil, linearIssueLinks: nil, + createdAt: "", archivedAt: nil, devicesOpen: nil + ) + } + + private func makeSession(id: String, laneId: String) -> TerminalSessionSummary { + TerminalSessionSummary( + id: id, + laneId: laneId, + laneName: "Lane", + ptyId: nil, + tracked: true, + pinned: false, + manuallyNamed: nil, + goal: nil, + toolType: "codex-chat", + title: "Chat", + status: "running", + startedAt: "2026-08-10T00:00:00.000Z", + endedAt: nil, + archivedAt: nil, + exitCode: nil, + transcriptPath: "", + headShaStart: nil, + headShaEnd: nil, + lastOutputPreview: nil, + summary: nil, + // At rest between turns — the state a user actually settles from. A + // declared settle is honored only at rest (`WorkSessionCanonicalState`), + // so a mid-stream chat deliberately stays on the roster. + runtimeState: "idle", + resumeCommand: nil, + resumeMetadata: nil, + chatIdleSinceAt: nil, + chatSessionId: nil, + pendingInputItemId: nil + ) + } + + @MainActor + private func withService( + _ body: (SyncService, DatabaseService) async throws -> Void + ) async throws { + let baseURL = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: baseURL, withIntermediateDirectories: true) + let database = DatabaseService(baseURL: baseURL) + let service = SyncService(database: database) + defer { + service.disconnect(clearCredentials: false) + database.close() + try? FileManager.default.removeItem(at: baseURL) + } + try database.executeSqlForTesting(""" + insert into projects (id, root_path, display_name, default_base_ref, created_at, last_opened_at) values + ('project-1', '/tmp/p1', 'P1', 'main', '2026-08-10T00:00:00.000Z', '2026-08-10T00:00:00.000Z'); + """) + database.setActiveProjectId("project-1") + try database.replaceLaneSnapshots([makeLane(id: "lane-1")]) + try database.replaceTerminalSessions([makeSession(id: "session-1", laneId: "lane-1")]) + try await body(service, database) + } + + @MainActor + func testFetchSessionsAppliesTheOverlayWhileTheDatabaseStaysUntouched() async throws { + try await withService { service, database in + service.beginPendingSessionSettleForTesting( + .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + for: "session-1" + ) + + let overlaid = try await service.fetchSessions().first { $0.id == "session-1" } + XCTAssertEqual(overlaid?.settledAt, "2026-08-10T12:00:00.000Z") + + // The whole point of the overlay: nothing was written, so nothing can + // replicate upstream and defeat a host rejection by CRDT merge. + XCTAssertNil(database.fetchSession(id: "session-1")?.settledAt) + } + } + + @MainActor + func testFetchSessionByIdGoesThroughTheSameChokepoint() async throws { + try await withService { service, _ in + service.beginPendingSessionSettleForTesting( + .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + for: "session-1" + ) + + let single = try await service.fetchSession(id: "session-1") + XCTAssertEqual(single?.settledAt, "2026-08-10T12:00:00.000Z") + } + } + + /// The regression: `refreshActiveSessionsAndSnapshot` read the database + /// directly, so a just-settled chat stayed in `activeSessions` — which backs + /// the lock-screen widget, the Live Activity, and the in-app Activity drawer. + @MainActor + func testASettledChatLeavesTheWidgetAndActivityRosterImmediately() async throws { + try await withService { service, _ in + service.refreshActiveSessionsAndSnapshot() + XCTAssertTrue( + service.activeSessions.contains { $0.sessionId == "session-1" }, + "precondition: a running chat is on the active roster" + ) + + service.beginPendingSessionSettleForTesting( + .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + for: "session-1" + ) + service.refreshActiveSessionsAndSnapshot() + + XCTAssertFalse( + service.activeSessions.contains { $0.sessionId == "session-1" }, + "a settle the user just tapped must not keep reporting as a live agent" + ) + } + } +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 98e34153e..7cae3c061 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -500,7 +500,7 @@ Schema bootstrap in `kvDb.ts` creates ~104 tables. Anchor tables for agents read | `projects` | One row per opened repo. Keyed by `root_path`. | | `lanes` | Worktree-backed units of work. Types: `primary`, `worktree`, `attached`. Supports parent/child stacks, color/icon/tags. | | `local_worktree_residual_cleanups` | Machine-local lane-delete cleanup debt for residual managed worktree directories. Stores absolute paths and is excluded from CRR replication because only the runtime on that machine can safely retry removal. | -| `terminal_sessions` | Tracked PTY sessions per lane with transcript path and head SHAs. The `chat_session_id` column (indexed) marks terminals owned by a chat (chat terminal drawer, App Control launch terminal); `ptyService` exposes them through the `ade.terminal.*` IPC and the `terminal` ADE action domain. The `owner_pid` column (indexed) identifies the ADE OS process that owns the live runtime for the row — cross-process reconcile/dispose paths check it before sweeping so concurrent surfaces don't mark each other's live sessions dead. See §3.5. Lifecycle lives in five nullable text columns: `settle_override` (tri-state `settled` / `active` / null, consulted before the derived exit-0 settle) and the snooze visibility overlay `snoozed_until` / `snoozed_at` with its `woke_at` / `woke_reason` marker. None of them carry a unique index — the table replicates to iOS through cr-sqlite, and `crsql_as_crr` rejects any non-primary-key unique index — and all five are mirrored in both iOS schema halves (`DatabaseBootstrap.sql` and `Database.swift`'s `ensureColumn` migrations). | +| `terminal_sessions` | Tracked PTY sessions per lane with transcript path and head SHAs. The `chat_session_id` column (indexed) marks terminals owned by a chat (chat terminal drawer, App Control launch terminal); `ptyService` exposes them through the `ade.terminal.*` IPC and the `terminal` ADE action domain. The `owner_pid` column (indexed) identifies the ADE OS process that owns the live runtime for the row — cross-process reconcile/dispose paths check it before sweeping so concurrent surfaces don't mark each other's live sessions dead. See §3.5. Lifecycle lives in five nullable text columns: `settle_override` (tri-state `settled` / `active` / null, consulted before the derived exit-0 settle) and the snooze visibility overlay `snoozed_until` / `snoozed_at` with its `woke_at` / `woke_reason` marker. None of them carry a unique index — the table replicates to iOS through cr-sqlite, and `crsql_as_crr` rejects any non-primary-key unique index — and all five are mirrored in both iOS schema halves (`DatabaseBootstrap.sql` and `Database.swift`'s `ensureColumn` migrations). The settle columns (`settled_at`, `settle_override`, `settle_source`) are host-authoritative: only `sessionService` may decide them, so the sync host drops them from inbound phone changesets and iOS renders an in-flight settle through a local overlay rather than a replicating write. | | `runtime_processes` | Machine-local process-liveness registry. Every ADE process (desktop main, brain process, TUI runtime) inserts a row on boot keyed by the process incarnation (`pid`, `started_at`) and refreshes `last_seen` on a 5 s heartbeat. The table is excluded from CRR replication because PIDs are only meaningful on the current OS; reconcile / dispose paths cross-reference `terminal_sessions.owner_pid` and `owner_process_started_at` against locally known and live rows to tell "row whose local owner crashed" from "row a sibling process is actively managing" without detaching sessions owned by another synced machine. See §3.5. | | `session_deltas` | Post-session diff stats + touched files + failure lines. Input to pack generation. | | `operations` | Audit log of every significant mutation (git, pack updates). Pre/post HEAD SHAs enable undo. | diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index d44c416b0..8be9b21fa 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -377,6 +377,48 @@ scaffold through Git, but live chat/process state converges only when they join the same sync cluster (i.e. point at the same running sync authority). +### Host-authoritative columns are peer-scoped + +Replication is not only a question of which *tables* cross the boundary. A few +columns on tables that do replicate are decisions only the host can make, and +the host refuses to let a controller author them. + +The current set is `terminal_sessions.settled_at`, `settle_override`, and +`settle_source` (`HOST_AUTHORITATIVE_COLUMNS_BY_TABLE` in `syncHostService.ts`). +A settle is decided by `sessionService`, the only place that can weigh it +against live work. Because the table replicates and cr-sqlite merges +last-writer-wins per column, a controller that writes its own optimistic +`settled_at` sends a value carrying no host lifecycle revision — and it merges +in regardless of what the host decided, so a host that *rejected* the settle +still ends up with a settled row. That is a guard defeated by a merge rather +than by a caller, which no amount of host-side checking closes. + +The filter drops those columns from inbound changesets **from phone peers +only**. Two properties make it different from the table-level +`SYNC_HOST_AUTHORITATIVE_TABLES` rule: + +- **It is peer-scoped, and deliberately so.** A paired desktop runs the same + `sessionService` chokepoint, so its settle writes are host-decided too and + must keep replicating; broadening the filter would silently stop settle + propagating between two of one user's machines. +- **It is a compatibility guard, not a security boundary.** `isMobileChangesetPeer` + reads the peer's own `hello` metadata, so it holds against an older iOS build + — which declares itself honestly — and not against a peer that lies. Current + iOS never writes these columns at all; it shows an in-flight settle through a + local overlay instead (`PendingSessionSettleStates.swift`). + +The drop is silent and per-column: the rest of the batch applies, including the +phone's own snooze overlay (`snoozed_until` / `snoozed_at` / `woke_*`), which +the phone legitimately owns because no host decision rides on it. The batch +still acks `ok` — a rejecting ack would stall the peer's outbound cursor and +make it resend the same range forever. A pre-fix phone's dropped value is +therefore local-only divergence, never host corruption, and it heals when +`refreshWorkSessions` next rewrites the row from the host's `work.listSessions` +payload. + +See [terminals and sessions](../terminals-and-sessions/README.md#gotchas) for +the lifecycle side of this invariant. + ## Architecture layers ``` @@ -1110,7 +1152,12 @@ Canonical files (`apps/ade-cli/src/services/sync/`): CRR that governs brain ownership — never crosses the CRR boundary in either direction, so a peer can neither receive it nor author a winning `crsql_changes` row that would flip `brain_device_id`; brain - handover stays on the explicit host-transfer RPC), the inbound + handover stays on the explicit host-transfer RPC), the host-authoritative + *column* filter (`HOST_AUTHORITATIVE_COLUMNS_BY_TABLE`: + `terminal_sessions.settled_at` / `settle_override` / `settle_source`, + dropped from inbound changesets **from phone peers only** — see + [Host-authoritative columns](#host-authoritative-columns-are-peer-scoped)), + the inbound changeset-batch ceilings (`MAX_INBOUND_CHANGESET_ROWS` / `_BYTES` ≈ 40× the outbound 250-row / 256 KB caps, i.e. ~10k rows / ~10 MB; an oversized `changeset_batch` is rejected with a `changeset_too_large` @@ -2872,6 +2919,16 @@ feature is merged or because a deliberately isolated-port host is running. orthogonal question — `linear_ingress_events` and `worker_agent_runs` are on the mobile-exclusion list yet deliberately never age-pruned on the host. +- **A host-side check does not guard a replicated column.** If the host decides + a value, a controller writing that same column into its own replica can win + the merge and undo the decision — the check was never reached. The fix is to + stop the controller writing it (a local, non-persisted overlay is what buys + the optimistic feel) and to drop the column from that peer's inbound + changesets, not to add another host-side check. Both halves are needed: the + client change fixes new builds, the host filter covers every paired device + still on an old one. See + [Host-authoritative columns](#host-authoritative-columns-are-peer-scoped). + - **The wire and the stored transcript share one chat-event compaction policy, and the wire runs storage compaction first.** `compactChatEventEnvelopeForSync` is an adapter; the policy is `shared/chatEventCompaction.ts`. Two diff --git a/docs/features/sync-and-multi-device/crdt-model.md b/docs/features/sync-and-multi-device/crdt-model.md index 559483192..4b8a96872 100644 --- a/docs/features/sync-and-multi-device/crdt-model.md +++ b/docs/features/sync-and-multi-device/crdt-model.md @@ -251,6 +251,16 @@ standard SQL on the host side, so iOS stays in parity. - **Deletes are tombstones.** `cid = "-1"` (see `localDeleteColumnId` in `Database.swift`) marks the row dead. A resurrection from another device with a newer `col_version` wins over the tombstone. +- **A merge is not a caller, so a host-side check cannot guard a + replicated column.** Where a column encodes a decision only the host + may make, a controller's optimistic write to it merges in on the + ordinary LWW rule and simply overwrites the decision — including a + decision to *refuse*. Nothing on the host is reached, so no amount of + validation there closes it. Such columns are declared host-authoritative + and filtered out of the offending peer's inbound changesets; + `terminal_sessions.settled_at` / `settle_override` / `settle_source` are + the current set. See + [Host-authoritative columns](./README.md#host-authoritative-columns-are-peer-scoped). ## Schema implications @@ -318,6 +328,12 @@ machine-bound. and it is only safe because `applyChanges` now skips inbound rows for local-only tables** — see [Apply](#apply). +"The host owns all writes and controllers only read" is a design intent, +not a property the CRR gives you: nothing stops a controller writing the +column anyway, and the write replicates. When the value is one a host +*decision* rests on, back the intent with the host-authoritative column +filter described in [Merge semantics](#merge-semantics). + ### Local clears that must not propagate Some bookkeeping tables are CRRs on every device but occasionally diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index e970322e7..2d8ab49a8 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -244,6 +244,13 @@ apps/ios/ │ │ │ # account-wide "agent runs" activity │ │ ├── MobileUsageQuotaStore.swift # host-scoped cached Claude/Codex │ │ │ # quota snapshot + refresh state +│ │ ├── PendingSessionSettleStates.swift # in-flight settle/unsettle/ +│ │ │ # override intents, applied over session +│ │ │ # reads. Purely local and never persisted, +│ │ │ # because the settle columns are +│ │ │ # host-authoritative and a replicating +│ │ │ # write could defeat a host rejection by +│ │ │ # CRDT merge │ │ ├── SyncRecoveryPolicy.swift # deterministic reconnect, roam-trigger │ │ │ # policy (failover vs upgrade probe), │ │ │ # path-change, heartbeat-silence, @@ -278,7 +285,10 @@ apps/ios/ │ │ # discovery, personal-chat cache/actions/ │ │ # subscription routing, session.* lifecycle │ │ # (settle / override / snooze / wake / -│ │ # clear-woke-marker) callers +│ │ # clear-woke-marker) callers, and the +│ │ # session read chokepoint (localSessions() +│ │ # / localSession(id:)) that overlays +│ │ # in-flight settle intents onto rows │ ├── Shared/ │ │ ├── ADESharedContainer.swift # App Group UserDefaults + WorkspaceSnapshot helpers │ │ ├── ADESharedModels.swift # AgentSnapshot, PrSnapshot — shared with widgets @@ -2028,8 +2038,8 @@ The iOS pieces: optional `String` fields through `decodeIfPresent`, and include them in equality so a lifecycle-only change still redraws the row. - `apps/ios/ADE/Services/SyncService.swift` holds the `session.*` remote-command - callers. Mobile has no local write path for lifecycle, so these commands are - the mechanism, and the connect-time descriptor list gates the affordances. + callers. The phone never decides a lifecycle value, so these commands are the + mechanism, and the connect-time descriptor list gates the affordances. - **The settle columns are host-authoritative and the phone never writes them.** `settled_at`, `settle_override`, and `settle_source` are decided by the host's `sessionService`, which is the only place that can weigh a settle against live @@ -2083,7 +2093,9 @@ The iOS pieces: - `apps/ios/ADETests/WorkSessionCanonicalStateTests.swift` covers the derivation, the row status vocabulary, and scoped view-state parity; `WorkSessionGroupingTests.swift` covers the grouping, quiet lanes, and the - quiet-zone shelves. + quiet-zone shelves; `PendingSessionSettleStatesTests.swift` covers the settle + overlay — what each intent paints, which host row satisfies it, token-scoped + failure, and the staleness backstop. Two invariants govern changes here. The Swift derivation must stay behaviourally identical to `apps/desktop/src/shared/sessionCanonicalState.ts` — diff --git a/docs/features/terminals-and-sessions/README.md b/docs/features/terminals-and-sessions/README.md index 2b37dacee..4aa0d3184 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -216,8 +216,8 @@ and in tests. serialized against; it is not a wrapper around the existing write. The approved plan for doing it is [settle-teardown-design.md](settle-teardown-design.md); its step 0 - precondition — `settled_at` becoming host-authoritative, so no replica will be - able to defeat the coming revision guard by CRDT merge — has landed. Archive is + precondition — `settled_at` becoming host-authoritative, so a phone replica + cannot defeat the coming revision guard by CRDT merge — has landed. Archive is the one lifecycle path that does stop processes — see `laneService.archive`, where the ordering is load-bearing. `dismissPendingInput: true` @@ -1392,6 +1392,18 @@ iOS Work surfaces: mirrors the desktop capability affordances, installs the returned persisted session summary, and routes CLI imports to the terminal screen or chat imports to the chat screen. +- `apps/ios/ADE/Services/PendingSessionSettleStates.swift` — the phone's + counterpart to the hosted-web `sessionLifecycleOverlay`, and for the same + reason: the settle columns are host-authoritative, so the phone shows an + in-flight settle / unsettle / override through a purely local overlay rather + than a replicating write. `SyncService.localSessions()` / + `localSession(id:)` are the read chokepoint that applies it, and every UI + read must come through them or the row renders as if the user never tapped. + Each intent knows which host row satisfies it (a settle waits on a non-null + `settled_at` *and* the cleared override; an unsettle tolerates a surviving + `"active"` pin), and an intent retires on confirmation, on command failure, + or against a reachable-time staleness backstop. Snooze keeps its optimistic + column write plus rollback — see the invariant in [Gotchas](#gotchas). ## External CLI session import @@ -1863,7 +1875,8 @@ runtime and agent chat runtime both layer the same identity envs filter is deliberately not applied to desktop peers: they run the same `sessionService` chokepoint, so their settle writes are host-decided and must keep replicating. The snooze columns are exempt — the phone owns its optimistic - write there because no host decision is at stake. + write there because no host decision is at stake. See + [sync → Host-authoritative columns](../sync-and-multi-device/README.md#host-authoritative-columns-are-peer-scoped). - **Settlement is not a pending-input response.** Never restore the old renderer sequence of `respondToInput` then settle. A provider decline may resume work, Codex plan declines may stage a revision, and a stale persisted From 054e577ff23fc3a8ee0e68bebaa72fc5ad8e46b0 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:54:21 -0400 Subject: [PATCH 04/15] fix(ios): give a queued settle a full window when the connection returns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The offline hold kept a queued settle's overlay alive while unreachable, but the last hold was stamped at the last read BEFORE reconnect — which can be minutes earlier, since nothing reads the projection while offline. The first reachable read would then expire the overlay and flip the row back to unsettled moments before the flushed command's changeset landed, which is the exact flicker the hold exists to prevent. Re-stamp the deadlines as the pending-operation queue flushes: the command is going out now, so it gets a full window to be answered. Docs: the pre-fix-phone heal is a full replace within the active project's lanes, not a per-row merge — so the host's value always wins, and rows in a project the phone has not activated stay stale until it is. --- apps/ios/ADE/Services/SyncService.swift | 10 ++++++++++ docs/features/sync-and-multi-device/README.md | 4 +++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index d73f1a145..c875e0cc6 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -18673,6 +18673,16 @@ final class SyncService: ObservableObject { throw NSError(domain: "ADE", code: 13, userInfo: [NSLocalizedDescriptionKey: "Unknown queued operation type."]) } removePendingOperation(operation) + // A settle taken offline is durably queued, and `holdBackstop` only + // re-stamps its deadline on reads — none of which happen while offline, + // so the last hold can be minutes stale by the time the connection is + // back. Without this the first reachable read would expire the overlay + // and flip the row to unsettled moments before this command's changeset + // lands. Deliberately on the SUCCESS path only: the retry cadence is + // shorter than `staleAfter`, so re-stamping per attempt would let a + // queue that never drains hold the overlay open forever — an unbounded + // lie in place of a two-second flicker. + pendingSessionSettleStates.holdBackstop(now: Date()) // A drained chat creation produced a real session; drop the optimistic // "Pending sync" snapshot so the synced row takes over. if operation.kind == "command", isQueuedChatCreationAction(operation.action) { diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index 8be9b21fa..92749f4cc 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -414,7 +414,9 @@ still acks `ok` — a rejecting ack would stall the peer's outbound cursor and make it resend the same range forever. A pre-fix phone's dropped value is therefore local-only divergence, never host corruption, and it heals when `refreshWorkSessions` next rewrites the row from the host's `work.listSessions` -payload. +payload — a full replace within the active project's lanes, not a per-row merge, +so the host's value always wins. Rows in a project the phone has not activated +stay stale until it is. See [terminals and sessions](../terminals-and-sessions/README.md#gotchas) for the lifecycle side of this invariant. From 1a96e383b2e171e5f0b44ae213cb6ac9fcc1338e Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:24:45 -0400 Subject: [PATCH 05/15] fix(sync): identify a phone by its pairing record, not its self-declaration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (CodeRabbit, verified): the settle-column guard used `isMobileChangesetPeer`, which reads only the `hello` metadata the peer sends about itself, so a paired phone claiming `deviceType: "desktop"` would have slipped past it. `isMobilePeer` already exists and is stronger: for a record-backed peer (`paired` / `account`) it resolves through the PAIRING RECORD the host stored at pairing time, falling back to self-declared metadata only for bootstrap-token peers. Switching to it closes the spoofing gap the design doc had recorded as an accepted limitation, so the docs are corrected too rather than left claiming the weaker guarantee. Still a compatibility guard rather than a hard boundary — a bootstrap peer is classified from what it says about itself — and the complete closure remains step 1's host-local lifecycle revision. The docs say exactly that. --- .../src/services/sync/syncHostService.ts | 7 ++++++- docs/features/sync-and-multi-device/README.md | 15 +++++++++----- .../settle-teardown-design.md | 20 +++++++++++-------- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index 1c1c47653..ba356fd29 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -7629,7 +7629,12 @@ export function createSyncHostService(args: SyncHostServiceArgs) { // merge over the host's decision (older iOS builds still write them). // The two rules are not symmetric: the table rule applies to every // peer, the column rule only to phones. - const isPhonePeer = isMobileChangesetPeer(peer); + // + // `isMobilePeer`, not `isMobileChangesetPeer`: it resolves a + // record-backed peer through its PAIRING RECORD rather than the + // `hello` metadata the peer declares about itself, so a paired phone + // cannot opt out of the guard by claiming to be a desktop. + const isPhonePeer = isMobilePeer(peer); const filtered = changes.filter((change) => { if (isHostAuthoritativeTable(change)) return false; if (isPhonePeer && isHostAuthoritativeColumn(change)) return false; diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index 92749f4cc..aaed1f32a 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -401,11 +401,16 @@ only**. Two properties make it different from the table-level `sessionService` chokepoint, so its settle writes are host-decided too and must keep replicating; broadening the filter would silently stop settle propagating between two of one user's machines. -- **It is a compatibility guard, not a security boundary.** `isMobileChangesetPeer` - reads the peer's own `hello` metadata, so it holds against an older iOS build - — which declares itself honestly — and not against a peer that lies. Current - iOS never writes these columns at all; it shows an in-flight settle through a - local overlay instead (`PendingSessionSettleStates.swift`). +- **A paired phone cannot opt out of it.** `isMobilePeer` resolves a + record-backed peer through its **pairing record** — host-side truth — and + falls back to the peer's own `hello` metadata only when the auth kind is not + record-backed. Declaring `deviceType: "desktop"` therefore does not evade the + filter. It remains a compatibility guard rather than a hard boundary, because + a bootstrap-token peer is still classified from what it says about itself; the + complete closure is the host-local lifecycle revision in step 1 of the + settle-teardown design. Current iOS never writes these columns at all — it + shows an in-flight settle through a local overlay instead + (`PendingSessionSettleStates.swift`). The drop is silent and per-column: the rest of the batch applies, including the phone's own snooze overlay (`snoozed_until` / `snoozed_at` / `woke_*`), which diff --git a/docs/features/terminals-and-sessions/settle-teardown-design.md b/docs/features/terminals-and-sessions/settle-teardown-design.md index 845f32471..0f2cb1401 100644 --- a/docs/features/terminals-and-sessions/settle-teardown-design.md +++ b/docs/features/terminals-and-sessions/settle-teardown-design.md @@ -237,7 +237,7 @@ update is not a guarantee, so the host enforces it. `syncHostService` drops inbound `terminal_sessions` changes for `settled_at`, `settle_override`, and `settle_source` when the peer is a phone -(`isMobileChangesetPeer`), alongside the existing `sync_cluster_state` +(`isMobilePeer`), alongside the existing `sync_cluster_state` brain-seizure filter. The drop is per-column and silent: the rest of the batch — including the phone's own snooze overlay, which it legitimately owns — applies normally, and the batch still acks `ok`, because a rejected ack would stall the @@ -258,13 +258,17 @@ further filtered to sessions whose lane the phone has hydrated, so a row outside that window stays locally wrong until it re-enters it. Local-only, never host corruption. -**What this filter is not.** `isMobileChangesetPeer` reads the peer's *own* -`hello` metadata, so a peer that declares itself a desktop is not filtered. That -is adequate for the stated threat — an older iOS build, which declares itself -honestly — but it is a compatibility guard, not a security boundary, and it -should not be read as one. The real closure is step 1's host-local lifecycle -revision: a settle write conditional on a revision no replica can author cannot -be won by a merge from any peer, however it identifies itself. +**How the phone is identified.** The filter uses `isMobilePeer`, which resolves a +record-backed peer through its **pairing record** — host-side truth — and only +falls back to the peer's own `hello` metadata when the auth kind is not +record-backed. A paired phone therefore cannot opt out of the guard by declaring +itself a desktop. + +It is still a compatibility guard rather than a hard boundary: a peer +authenticated by bootstrap token alone is classified from self-declared +metadata. The complete closure is step 1's host-local lifecycle revision — a +settle write conditional on a revision no replica can author cannot be won by a +merge from any peer, however it identifies itself. ### 3c-ii. Where the revision column lives From 405e7c5270587728c75c6f1d12b582b2447ecad5 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:46:50 -0400 Subject: [PATCH 06/15] fix(ios): confirm a settle intent against movement, not just value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified and reproduced): two lifecycle commands can overlap, and value equality alone confirms the wrong one. The settle overlay makes a row read as settled, so the row menu offers Unsettle and the user can tap it before the settle has landed. The newer unsettle intent wants `settled_at = nil` — which is exactly what the stale row still holds — so the next read confirmed and retired it immediately. The first command's changeset then painted the row settled while the user's later unsettle was still in flight. An intent now records the row as it stood when its command was sent, and is confirmed only once the row has actually MOVED from that baseline and matches what was asked for. A nil baseline (row unknown at begin) falls back to value equality, which is all there is to go on. Pinned by `testAReplacementIntentSurvivesUntilTheRowActuallyMoves`, verified non-vacuous: forcing the movement gate true fails it on both the pre-landing and post-first-command assertions. --- .../Services/PendingSessionSettleStates.swift | 39 ++++++++- apps/ios/ADE/Services/SyncService.swift | 8 +- .../PendingSessionSettleStatesTests.swift | 80 ++++++++++++++----- 3 files changed, 103 insertions(+), 24 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index 4df0b69dd..023a8323d 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -29,6 +29,21 @@ struct PendingSessionSettleIntent: Equatable { /// Identifies this specific command, so a slow one's failure cannot retire an /// intent the user has since replaced. Assigned by `begin`. fileprivate var token: UInt64 = 0 + /// The row as it stood when the command was sent, and whether it has moved + /// since. Matching the intent is not enough on its own: two commands can + /// overlap (settle, then unsettle before the settle lands), and the newer + /// one's target value can be exactly what the stale row still holds. Without + /// a baseline it would confirm against the state it was issued *from* and + /// retire immediately, letting the first command's changeset paint the row + /// while the user's later intent is still in flight. `nil` means the row was + /// unknown at begin, in which case value equality alone has to do. + fileprivate var baseline: Baseline? + fileprivate var sawRowChange = false + + struct Baseline: Equatable { + var settledAt: String? + var settleOverride: String? + } static func settle(now: Date, timestamp: String) -> PendingSessionSettleIntent { PendingSessionSettleIntent(kind: .settle(timestamp: timestamp), startedAt: now) @@ -90,6 +105,13 @@ struct PendingSessionSettleIntent: Equatable { } } + fileprivate func currentBaseline(of session: TerminalSessionSummary) -> Baseline { + Baseline( + settledAt: PendingSessionSettleIntent.normalized(session.settledAt), + settleOverride: PendingSessionSettleIntent.normalized(session.settleOverride) + ) + } + fileprivate static func normalized(_ value: String?) -> String? { guard let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines), !trimmed.isEmpty else { return nil } return trimmed @@ -120,10 +142,15 @@ struct PendingSessionSettleStates: Equatable { /// Replaces any intent already in flight for the session: the newest command /// is the one the user is waiting on. Returns the token that identifies it. @discardableResult - mutating func begin(_ intent: PendingSessionSettleIntent, for sessionId: String) -> UInt64 { + mutating func begin( + _ intent: PendingSessionSettleIntent, + for sessionId: String, + baseline: TerminalSessionSummary? + ) -> UInt64 { nextToken &+= 1 var stamped = intent stamped.token = nextToken + stamped.baseline = baseline.map { stamped.currentBaseline(of: $0) } intents[sessionId] = stamped return nextToken } @@ -178,7 +205,15 @@ struct PendingSessionSettleStates: Equatable { guard !intents.isEmpty else { return false } let before = intents.count for session in sessions { - guard let intent = intents[session.id], intent.isSatisfied(by: session) else { continue } + guard var intent = intents[session.id] else { continue } + if let baseline = intent.baseline, !intent.sawRowChange { + if intent.currentBaseline(of: session) != baseline { + intent.sawRowChange = true + intents[session.id] = intent + } + } + let movedSinceCommand = intent.baseline == nil || intent.sawRowChange + guard movedSinceCommand, intent.isSatisfied(by: session) else { continue } intents.removeValue(forKey: session.id) } intents = intents.filter { now.timeIntervalSince($0.value.startedAt) < PendingSessionSettleStates.staleAfter } diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index c875e0cc6..5e232ec26 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9318,7 +9318,13 @@ final class SyncService: ObservableObject { _ intent: PendingSessionSettleIntent, for sessionId: String ) -> UInt64 { - let token = pendingSessionSettleStates.begin(intent, for: sessionId) + // The RAW row is the baseline: what the host had before this command. A + // cheap PK lookup, once per settle command, not on any read path. + let token = pendingSessionSettleStates.begin( + intent, + for: sessionId, + baseline: database.fetchSession(id: sessionId) + ) scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) return token } diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 977eed6c4..c57d18fea 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -46,7 +46,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testSettleIntentShowsTheRowSettledBeforeTheHostAnswers() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // A declared settle clears ANY override host-side — including a keep-active // pin, so it cannot silently veto the settle — and the overlay does too. @@ -57,7 +57,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testUnsettleLeavesAKeepActivePinAlone() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1") + states.begin(.unsettle(now: now), for: "session-1", baseline: nil) // The host PRESERVES an `"active"` pin through an unsettle, so the overlay // must not claim it was cleared. @@ -72,7 +72,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// overlay can predict it exactly. func testUnsettleClearsASettledPinBecauseTheHostWill() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1") + states.begin(.unsettle(now: now), for: "session-1", baseline: nil) let overlaid = states.apply(to: session(settleOverride: "settled")) XCTAssertNil(overlaid.settleOverride) @@ -87,7 +87,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testSettleResolvesOnlyOnceTheHostAlsoClearedTheOverride() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // `sessionService.settleMany` / `settleSession` both set // `settle_override = null` unconditionally, so that a keep-active pin cannot @@ -102,7 +102,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testIntentResolvesOnTheHostsOwnTimestampNotOurs() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // The host writes its own clock. Matching on the exact string would never // resolve, so presence is what the settle intent predicts. @@ -113,7 +113,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testIntentSurvivesUntilTheHostRowActuallyChanges() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.prune(against: [session(settledAt: nil)], now: now) @@ -123,7 +123,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testUnsettleIntentResolvesWhenTheRowGoesBackToNull() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1") + states.begin(.unsettle(now: now), for: "session-1", baseline: nil) states.prune(against: [session(settledAt: "2026-08-10T09:00:00.000Z")], now: now) XCTAssertNotNil(states["session-1"]) @@ -134,7 +134,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testOverrideIntentComparesTheExactValueWeAskedFor() { var states = PendingSessionSettleStates() - states.begin(.settleOverride("active", now: now), for: "session-1") + states.begin(.settleOverride("active", now: now), for: "session-1", baseline: nil) // `settle_override` is a value we own, unlike the settle timestamp — a // different non-null value is the host disagreeing, not confirming. @@ -147,7 +147,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testClearingAnOverrideResolvesOnNull() { var states = PendingSessionSettleStates() - states.begin(.settleOverride(nil, now: now), for: "session-1") + states.begin(.settleOverride(nil, now: now), for: "session-1", baseline: nil) XCTAssertNil(states.apply(to: session(settleOverride: "active")).settleOverride) @@ -157,7 +157,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAFailedCommandDropsTheIntentSoTheRowSnapsBack() { var states = PendingSessionSettleStates() - let token = states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + let token = states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.clear("session-1", token: token) @@ -169,8 +169,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// the user is now waiting on. func testAStaleFailureCannotRetireANewerIntent() { var states = PendingSessionSettleStates() - let stale = states.begin(.settleOverride("active", now: now), for: "session-1") - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + let stale = states.begin(.settleOverride("active", now: now), for: "session-1", baseline: nil) + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.clear("session-1", token: stale) @@ -179,7 +179,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAnIntentWhoseChangesetNeverArrivesExpires() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) let justBefore = now.addingTimeInterval(PendingSessionSettleStates.staleAfter - 1) states.prune(against: [session(settledAt: nil)], now: justBefore) @@ -195,7 +195,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// command is still on its way, then settle it again when the queue drains. func testAQueuedSettleDoesNotExpireWhileTheHostIsUnreachable() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) var clock = now for _ in 0..<10 { @@ -210,7 +210,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testTheBackstopResumesOnceTheHostIsReachableAgain() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // Offline for well past the budget, then reachable: the clock restarts from // the moment we could have been answered, not from the tap. @@ -226,7 +226,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testPruneReportsOnlyRealResolutionsSoRepaintCannotLoop() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // Re-stamping the offline deadline is not a resolution; reporting it as one // would repaint on every read forever. @@ -238,15 +238,53 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testTheNewestCommandWins() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") - states.begin(.unsettle(now: now), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.unsettle(now: now), for: "session-1", baseline: nil) XCTAssertNil(states.apply(to: session(settledAt: "2026-08-10T09:00:00.000Z")).settledAt) } + /// Two lifecycle commands can overlap: the settle overlay makes the row read + /// as settled, so the menu offers Unsettle, and the user can tap it before the + /// settle has landed. The newer intent's target value is exactly what the + /// stale row still holds, so confirming on value equality alone would retire + /// it immediately — and the first command's changeset would then paint the row + /// settled while the user's later unsettle was still in flight. + func testAReplacementIntentSurvivesUntilTheRowActuallyMoves() { + var states = PendingSessionSettleStates() + let unsettled = session() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) + states.begin(.unsettle(now: now), for: "session-1", baseline: unsettled) + + // The row has not moved yet — `settled_at` is still nil, which is also what + // the unsettle wants. It must NOT count as confirmation. + states.prune(against: [unsettled], now: now) + XCTAssertNotNil(states["session-1"]) + + // The first command lands. Still not our intent, so the overlay holds and + // keeps showing the row as the user last asked for it. + let settledByFirstCommand = session(settledAt: "2026-08-10T12:00:00.417Z") + states.prune(against: [settledByFirstCommand], now: now) + XCTAssertNotNil(states["session-1"]) + XCTAssertNil(states.apply(to: settledByFirstCommand).settledAt) + + // The unsettle lands: the row has now moved AND matches. + states.prune(against: [unsettled], now: now) + XCTAssertNil(states["session-1"]) + } + + func testAnUnknownBaselineFallsBackToValueEquality() { + var states = PendingSessionSettleStates() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + + XCTAssertNil(states["session-1"], "with no baseline the value match is all we have") + } + func testRemoveAllForgetsEverythingInFlight() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.removeAll() @@ -256,7 +294,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testASessionMissingFromAScopedReadKeepsItsIntent() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // A partial or differently-scoped read is not the host disagreeing. states.prune(against: [session(id: "session-2", settledAt: nil)], now: now) @@ -266,7 +304,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testOverlayOnlyTouchesTheSessionItWasBegunFor() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) let others = states.apply(to: [session(id: "session-1"), session(id: "session-2")]) From 43b8121f4d71dfe2910539074696a142087d1958 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:02:12 -0400 Subject: [PATCH 07/15] fix(ios): an unsettle follows what the user saw, not the stale row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): the same overlapping-command window, one layer down. A row with an `"active"` keep-active pin is settleable, so the user can tap Settle and then Unsettle before the settle lands. `.unsettle` chose its override branch by reading the LIVE row, which still carries the pin — so the overlay resurrected a keep-active pin and offered the wrong actions. Host-side the two commands run in order: the settle clears the pin unconditionally, then the unsettle preserves whatever is left, so the run actually ends with no override. An intent now records the override as it was PRESENTED when its command was issued — the raw row with any intent it replaced already applied over it — and the unsettle branch follows that. A standalone unsettle over a real pin still shows the pin, because there the presented value and the row agree. Two tests: the overlapping case, and the standalone case that must not regress. --- .../Services/PendingSessionSettleStates.swift | 31 +++++++++++++++---- .../PendingSessionSettleStatesTests.swift | 25 +++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index 023a8323d..caa649898 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -39,6 +39,13 @@ struct PendingSessionSettleIntent: Equatable { /// unknown at begin, in which case value equality alone has to do. fileprivate var baseline: Baseline? fileprivate var sawRowChange = false + /// The override as it was PRESENTED to the user when this command was issued + /// — the raw row with any intent this one replaced already applied over it. + /// An unsettle's override branch has to follow what the user acted on, not + /// the stale row: a settle overlay has already cleared a keep-active pin that + /// the row still carries, and the host will clear it too when it processes the + /// commands in order. Outer `nil` means the row was unknown at begin. + fileprivate var presentedOverride: String?? struct Baseline: Equatable { var settledAt: String? @@ -71,12 +78,19 @@ struct PendingSessionSettleIntent: Equatable { next.settledAt = nil // The host clears a `"settled"` override and PRESERVES an `"active"` pin // (`settle_override = case when settle_override = 'settled' then null else - // settle_override end`). Which branch it takes is decided by the value - // already in the row, so we can predict it exactly rather than guess — - // and must, because a row settled purely BY that pin has a null - // `settled_at` already, so clearing the timestamp alone would show the - // user nothing at all. - if PendingSessionSettleIntent.normalized(session.settleOverride) == "settled" { + // settle_override end`). Which branch it takes is decided by the value in + // the row when the host gets there, so we can predict it exactly rather + // than guess — and must, because a row settled purely BY that pin has a + // null `settled_at` already, so clearing the timestamp alone would show + // the user nothing at all. + // + // `presentedOverride`, not the live row: if this unsettle replaced a + // settle that has not landed yet, the host will clear the pin as part of + // that settle, so reading the stale row here would resurrect a pin that + // is on its way out. + if let presented = presentedOverride { + next.settleOverride = presented == "settled" ? nil : presented + } else if PendingSessionSettleIntent.normalized(session.settleOverride) == "settled" { next.settleOverride = nil } case .override(let value): @@ -151,6 +165,11 @@ struct PendingSessionSettleStates: Equatable { var stamped = intent stamped.token = nextToken stamped.baseline = baseline.map { stamped.currentBaseline(of: $0) } + // What the user was looking at when they issued this: the row with any + // intent this one replaces already applied over it. + stamped.presentedOverride = baseline.map { row in + PendingSessionSettleIntent.normalized((intents[sessionId]?.applied(to: row) ?? row).settleOverride) + } intents[sessionId] = stamped return nextToken } diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index c57d18fea..659f71c1e 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -273,6 +273,31 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(states["session-1"]) } + /// A keep-active pin plus an overlapping pair of commands. Host-side the + /// settle clears the pin and the unsettle then preserves whatever is left, so + /// the run ends with no override. Reading the stale row here would resurrect + /// the pin and offer the wrong actions until replication caught up. + func testUnsettleAfterAnUnlandedSettleDoesNotResurrectAKeepActivePin() { + var states = PendingSessionSettleStates() + let pinned = session(settleOverride: "active") + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: pinned) + states.begin(.unsettle(now: now), for: "session-1", baseline: pinned) + + let overlaid = states.apply(to: pinned) + XCTAssertNil(overlaid.settledAt) + XCTAssertNil(overlaid.settleOverride, "the settle the user already issued clears the pin host-side") + } + + /// The same branch with no overlapping command: a pin the host really will + /// preserve must still be shown. + func testAStandaloneUnsettleStillPreservesAKeepActivePin() { + var states = PendingSessionSettleStates() + let pinned = session(settledAt: "2026-08-10T09:00:00.000Z", settleOverride: "active") + states.begin(.unsettle(now: now), for: "session-1", baseline: pinned) + + XCTAssertEqual(states.apply(to: pinned).settleOverride, "active") + } + func testAnUnknownBaselineFallsBackToValueEquality() { var states = PendingSessionSettleStates() states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) From f94fd465d242a0f5c76b90315411ca11c457c99a Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:21:34 -0400 Subject: [PATCH 08/15] fix(ios): stop guessing when two commands are outstanding, and hold at reconnect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings (Codex, P2 each), both verified. **An earlier command's changeset could confirm a later one.** Movement off the baseline proves only that *some* settle-family mutation replicated, not that the latest command applied. With `settle → unsettle → settle` issued inside one replication window, the first settle both moves the row and matches the third intent, so it retired the overlay — then the intervening unsettle replicated and the row flipped. The fix Codex asks for is a per-command marker, and the phone has none: the host's lifecycle revision is host-local, and inventing a replicated one is step 1's job, not step 0's. So the overlay stops guessing instead. An intent that replaced one still in flight is deliberately unconfirmable — it keeps showing what the user last asked for and yields to replicated truth at the backstop, by which point the run has converged. Briefly trailing the truth beats confidently showing the wrong command's result. **A queued settle could still snap back on reconnect.** `performInitialHydration` reads (and posts a database change) before `flushPendingOperations` reaches its success-path hold, and reconnect makes `canSendLiveRequests()` true immediately — so a settle queued longer than `staleAfter` expired in the gap. Deadlines are now rebased at the top of hydration, the earliest point the connection is usable. This also closes the "best-effort, not deterministic" caveat the earlier revalidation had accepted. --- .../Services/PendingSessionSettleStates.swift | 18 ++++++++++++- apps/ios/ADE/Services/SyncService.swift | 9 +++++++ .../PendingSessionSettleStatesTests.swift | 27 ++++++++++++++++++- .../sync-and-multi-device/ios-companion.md | 7 ++++- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index caa649898..0a3ab5382 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -46,6 +46,21 @@ struct PendingSessionSettleIntent: Equatable { /// the row still carries, and the host will clear it too when it processes the /// commands in order. Outer `nil` means the row was unknown at begin. fileprivate var presentedOverride: String?? + /// Set when this intent replaced one that was still in flight. + /// + /// With two or more commands outstanding, a replicated row change cannot be + /// attributed to a particular command: `settle → unsettle → settle` sends the + /// first settle's changeset, which both moves the row off this intent's + /// baseline and matches it, so movement would confirm the wrong command and + /// the row would then flip when the intervening unsettle replicated. + /// + /// The phone has no per-command marker to fix that — the host's lifecycle + /// revision is host-local. So an intent in this state is deliberately NOT + /// confirmable: it keeps showing what the user last asked for and yields to + /// the replicated truth at the backstop, by which point the whole run has + /// converged. Briefly trailing the truth beats confidently showing the wrong + /// command's result. + fileprivate var replacedInFlight = false struct Baseline: Equatable { var settledAt: String? @@ -170,6 +185,7 @@ struct PendingSessionSettleStates: Equatable { stamped.presentedOverride = baseline.map { row in PendingSessionSettleIntent.normalized((intents[sessionId]?.applied(to: row) ?? row).settleOverride) } + stamped.replacedInFlight = intents[sessionId] != nil intents[sessionId] = stamped return nextToken } @@ -232,7 +248,7 @@ struct PendingSessionSettleStates: Equatable { } } let movedSinceCommand = intent.baseline == nil || intent.sawRowChange - guard movedSinceCommand, intent.isSatisfied(by: session) else { continue } + guard !intent.replacedInFlight, movedSinceCommand, intent.isSatisfied(by: session) else { continue } intents.removeValue(forKey: session.id) } intents = intents.filter { now.timeIntervalSince($0.value.startedAt) < PendingSessionSettleStates.staleAfter } diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 5e232ec26..89fff731c 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -19393,6 +19393,15 @@ final class SyncService: ObservableObject { connectionState == .connected else { return } + // Reconnect makes `canSendLiveRequests()` true immediately, so reads stop + // holding a queued settle's deadline — and hydration below performs reads + // (and posts a database change) BEFORE `flushPendingOperations` gets to + // re-stamp on its success path. Without this a settle queued for longer + // than `staleAfter` would snap back to unsettled in the moments between + // reconnecting and replaying it. Rebase the deadlines here, at the earliest + // point the connection is usable. + pendingSessionSettleStates.holdBackstop(now: Date()) + if activeProjectId == nil { refreshProjectCatalog() } diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 659f71c1e..ed052db5b 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -268,11 +268,36 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNotNil(states["session-1"]) XCTAssertNil(states.apply(to: settledByFirstCommand).settledAt) - // The unsettle lands: the row has now moved AND matches. + // It is NOT confirmable by movement either: with two commands outstanding a + // row change cannot be attributed to one of them. It holds what the user + // last asked for and yields at the backstop, by which point the run has + // converged. states.prune(against: [unsettled], now: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [unsettled], now: now.addingTimeInterval(PendingSessionSettleStates.staleAfter)) XCTAssertNil(states["session-1"]) } + /// `settle → unsettle → settle` before anything replicates. The first + /// settle's changeset both moves the row off the third command's baseline and + /// matches it, so confirming on movement would retire the overlay against the + /// WRONG command — and the row would then flip when the intervening unsettle + /// replicated. + func testAThirdOverlappingCommandIsNotConfirmedByAnEarlierOnesChangeset() { + var states = PendingSessionSettleStates() + let unsettled = session() + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) + states.begin(.unsettle(now: now), for: "session-1", baseline: unsettled) + states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:02.000Z"), for: "session-1", baseline: unsettled) + + // The FIRST settle replicates. It matches the third intent by value, and it + // moved the row — but it is not the third command. + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + + XCTAssertNotNil(states["session-1"], "an earlier command's changeset must not confirm the latest one") + } + /// A keep-active pin plus an overlapping pair of commands. Host-side the /// settle clears the pin and the unsettle then preserves whatever is left, so /// the run ends with no override. Reading the stale row here would resurrect diff --git a/docs/features/sync-and-multi-device/ios-companion.md b/docs/features/sync-and-multi-device/ios-companion.md index 2d8ab49a8..85074a8b0 100644 --- a/docs/features/sync-and-multi-device/ios-companion.md +++ b/docs/features/sync-and-multi-device/ios-companion.md @@ -2051,7 +2051,12 @@ The iOS pieces: rollback because those columns guard no host decision. Instant feedback for settle comes from `PendingSessionSettleStates` — a local, non-persisted overlay applied when session rows are read, resolved when the host's changeset confirms - it, when the command fails, or by a bounded staleness backstop. The host + it, when the command fails, or by a bounded staleness backstop. With two or + more commands outstanding for one session the overlay stops trying to confirm + at all: a replicated row change cannot be attributed to a particular command + without a per-command marker, and the host's lifecycle revision is host-local. + It keeps showing what the user last asked for and yields to replicated truth at + the backstop, rather than confirming against the wrong command. The host enforces the same rule against phones on older builds by dropping those columns from inbound phone changesets (`syncHostService`), and such a phone self-heals on the next `refreshWorkSessions`. See From 603e21d1efd893484f4f5f38eabb9ce3f9834baa Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 16:44:03 -0400 Subject: [PATCH 09/15] fix(ios): make the staleness backstop actually fire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): `staleAfter` was a promise the code did not keep. `prune` runs only from a session read, and reads are driven by database changes. A command whose confirming changeset never arrives produces no database change — so on a quiet screen nothing ever re-evaluated the deadline and the overlay could persist indefinitely. The immediate post-command reload happens well inside the window, so it does not help. Adds the timer that enforces it: a one-shot sweep armed when an intent begins and re-armed whenever deadlines are rebased, which reads through the chokepoint (so it prunes and repaints) and re-arms while any intent is still in flight. It stops as soon as the map empties, and is cancelled on teardown. An earlier revalidation had logged this as Low and accepted it as "bounded". It was not bounded, and calling it that in the docs was wrong — hence fixing rather than re-accepting it. --- apps/ios/ADE/Services/SyncService.swift | 35 ++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 89fff731c..a55a06f2b 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -5509,6 +5509,7 @@ final class SyncService: ObservableObject { roamTask?.cancel() reconnectStabilityTask?.cancel() pendingOperationFlushTask?.cancel() + pendingSessionSettleBackstopTask?.cancel() outboundCursorPersistTask?.cancel() remoteCursorProfilePersistTask?.cancel() lanePresenceHeartbeatTask?.cancel() @@ -9310,6 +9311,9 @@ final class SyncService: ObservableObject { /// In-flight settle intents, applied over session reads so a settle feels /// immediate without a replicating write. Never persisted. private var pendingSessionSettleStates = PendingSessionSettleStates() + /// Timer that guarantees the overlay's staleness backstop fires even when no + /// read is coming. See `schedulePendingSessionSettleBackstopSweep`. + private var pendingSessionSettleBackstopTask: Task? /// Record an in-flight settle intent and nudge the projections, mirroring the /// re-render the optimistic DB write used to trigger through @@ -9326,6 +9330,7 @@ final class SyncService: ObservableObject { baseline: database.fetchSession(id: sessionId) ) scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + schedulePendingSessionSettleBackstopSweep() return token } @@ -9340,6 +9345,33 @@ final class SyncService: ObservableObject { /// sessions that are no longer on screen, and on unpair the host is /// permanently unreachable, so `holdBackstop` would otherwise keep the overlay /// painting for the rest of the app's life. + /// Guarantee the staleness backstop actually fires. + /// + /// `prune` only runs from a session read, and reads are driven by database + /// changes. A command whose confirming changeset never arrives produces no + /// database change, so on a quiet screen nothing would ever re-evaluate the + /// deadline and the overlay would persist indefinitely — `staleAfter` would be + /// a promise the code does not keep. This is the timer that keeps it. + /// + /// Re-arms while any intent is still in flight (an offline hold keeps + /// re-stamping deadlines, so one shot is not enough) and stops as soon as the + /// map empties. + private func schedulePendingSessionSettleBackstopSweep() { + pendingSessionSettleBackstopTask?.cancel() + guard !pendingSessionSettleStates.isEmpty else { + pendingSessionSettleBackstopTask = nil + return + } + pendingSessionSettleBackstopTask = Task { @MainActor [weak self] in + try? await Task.sleep(nanoseconds: UInt64(PendingSessionSettleStates.staleAfter * 1_000_000_000) + 250_000_000) + guard let self, !Task.isCancelled else { return } + self.pendingSessionSettleBackstopTask = nil + // Reading through the chokepoint prunes and repaints if anything expired. + _ = self.localSessions() + self.schedulePendingSessionSettleBackstopSweep() + } + } + private func resetPendingSessionSettleStates() { guard !pendingSessionSettleStates.isEmpty else { return } pendingSessionSettleStates.removeAll() @@ -19399,8 +19431,9 @@ final class SyncService: ObservableObject { // re-stamp on its success path. Without this a settle queued for longer // than `staleAfter` would snap back to unsettled in the moments between // reconnecting and replaying it. Rebase the deadlines here, at the earliest - // point the connection is usable. + // point the connection is usable, and re-arm the sweep that enforces them. pendingSessionSettleStates.holdBackstop(now: Date()) + schedulePendingSessionSettleBackstopSweep() if activeProjectId == nil { refreshProjectCatalog() From f4f00d4d5b69c9c0ff24e161699fa8f25564f9f5 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:00:51 -0400 Subject: [PATCH 10/15] fix(ios): measure overlay staleness on the monotonic clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): the backstop compared wall-clock `Date`s, so an NTP correction or a manual time change could expire a fresh overlay the instant time jumped forward, or hold one far past the promised window when it jumped back. Deadlines now use `ProcessInfo.processInfo.systemUptime`. The displayed settle timestamp stays a real `Date` — that one is shown to the user, not measured with. This follows the precedent already in this file: `backgroundedAtUptime` is deliberately monotonic for the same reason, with a comment about a device whose clock moves backward during suspension. The tests now advance a numeric uptime rather than a wall clock, which also makes them independent of the machine date. --- .../Services/PendingSessionSettleStates.swift | 30 ++-- apps/ios/ADE/Services/SyncService.swift | 22 +-- .../PendingSessionSettleStatesTests.swift | 140 +++++++++--------- 3 files changed, 102 insertions(+), 90 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index 0a3ab5382..0ccaea5ad 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -24,8 +24,14 @@ struct PendingSessionSettleIntent: Equatable { } var kind: Kind - /// When the command was sent, for the staleness backstop. - var startedAt: Date + /// When the command was sent, on the MONOTONIC clock + /// (`ProcessInfo.processInfo.systemUptime`), for the staleness backstop. + /// Deliberately not wall clock: an NTP correction or a manual time change + /// would otherwise expire a fresh overlay the instant time jumps forward, or + /// hold one far past the promised window when it jumps back. Same reasoning + /// as `backgroundedAtUptime` in `SyncService`. The displayed settle timestamp + /// stays a real `Date` — that one is shown to the user, not measured with. + var startedAtUptime: TimeInterval /// Identifies this specific command, so a slow one's failure cannot retire an /// intent the user has since replaced. Assigned by `begin`. fileprivate var token: UInt64 = 0 @@ -67,16 +73,16 @@ struct PendingSessionSettleIntent: Equatable { var settleOverride: String? } - static func settle(now: Date, timestamp: String) -> PendingSessionSettleIntent { - PendingSessionSettleIntent(kind: .settle(timestamp: timestamp), startedAt: now) + static func settle(uptime: TimeInterval, timestamp: String) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .settle(timestamp: timestamp), startedAtUptime: uptime) } - static func unsettle(now: Date) -> PendingSessionSettleIntent { - PendingSessionSettleIntent(kind: .unsettle, startedAt: now) + static func unsettle(uptime: TimeInterval) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .unsettle, startedAtUptime: uptime) } - static func settleOverride(_ value: String?, now: Date) -> PendingSessionSettleIntent { - PendingSessionSettleIntent(kind: .override(value), startedAt: now) + static func settleOverride(_ value: String?, uptime: TimeInterval) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .override(value), startedAtUptime: uptime) } func applied(to session: TerminalSessionSummary) -> TerminalSessionSummary { @@ -222,9 +228,9 @@ struct PendingSessionSettleStates: Equatable { /// /// Returns nothing on purpose — this can never resolve an intent, so it can /// never be a reason to repaint. - mutating func holdBackstop(now: Date) { + mutating func holdBackstop(uptime: TimeInterval) { for key in intents.keys { - intents[key]?.startedAt = now + intents[key]?.startedAtUptime = uptime } } @@ -236,7 +242,7 @@ struct PendingSessionSettleStates: Equatable { /// expiry changes what the row should show and no database write accompanies /// it. @discardableResult - mutating func prune(against sessions: [TerminalSessionSummary], now: Date) -> Bool { + mutating func prune(against sessions: [TerminalSessionSummary], uptime: TimeInterval) -> Bool { guard !intents.isEmpty else { return false } let before = intents.count for session in sessions { @@ -251,7 +257,7 @@ struct PendingSessionSettleStates: Equatable { guard !intent.replacedInFlight, movedSinceCommand, intent.isSatisfied(by: session) else { continue } intents.removeValue(forKey: session.id) } - intents = intents.filter { now.timeIntervalSince($0.value.startedAt) < PendingSessionSettleStates.staleAfter } + intents = intents.filter { uptime - $0.value.startedAtUptime < PendingSessionSettleStates.staleAfter } return intents.count != before } diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index a55a06f2b..ad4fded66 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9186,14 +9186,14 @@ final class SyncService: ObservableObject { /// without this nudge it would only become visible on the next unrelated /// read — which against a quiet host may be a long time. private func prunePendingSessionSettleStates(against sessions: [TerminalSessionSummary]) { - let now = Date() - // Hold first, then measure: after a hold every deadline is `now`, so the + let uptime = ProcessInfo.processInfo.systemUptime + // Hold first, then measure: after a hold every deadline is `uptime`, so the // backstop cannot fire against a command that is merely waiting for the // connection to come back. if !canSendLiveRequests() { - pendingSessionSettleStates.holdBackstop(now: now) + pendingSessionSettleStates.holdBackstop(uptime: uptime) } - guard pendingSessionSettleStates.prune(against: sessions, now: now) else { return } + guard pendingSessionSettleStates.prune(against: sessions, uptime: uptime) else { return } scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) } @@ -9547,7 +9547,6 @@ final class SyncService: ObservableObject { if dismissPendingInput { args["dismissPendingInput"] = true } - let now = Date() try await sendSessionSettleCommand( sessionId: sessionId, action: "session.settleSessions", @@ -9555,7 +9554,10 @@ final class SyncService: ObservableObject { // The bulk action answers with the ids it CHANGED, so an absent id means // the machine settled nothing. Mirrors the desktop `settleMany`. resultShape: .changedIdList, - intent: .settle(now: now, timestamp: iso8601WithFractionalSecondsFormatter.string(from: now)) + intent: .settle( + uptime: ProcessInfo.processInfo.systemUptime, + timestamp: iso8601WithFractionalSecondsFormatter.string(from: Date()) + ) ) } @@ -9577,7 +9579,7 @@ final class SyncService: ObservableObject { // per-row verdict to check — so there is nothing to reject, exactly like // the desktop `unsettleMany`, which passes no `applied` predicate. resultShape: nil, - intent: .unsettle(now: Date()) + intent: .unsettle(uptime: ProcessInfo.processInfo.systemUptime) ) } @@ -9590,7 +9592,7 @@ final class SyncService: ObservableObject { // The host reads "clear" as null; sending a JSON null through the // `[String: Any]` arg dictionary is not representable here. args: ["sessionId": sessionId, "override": override?.rawValue ?? "clear"], - intent: .settleOverride(override?.rawValue, now: Date()) + intent: .settleOverride(override?.rawValue, uptime: ProcessInfo.processInfo.systemUptime) ) } @@ -18720,7 +18722,7 @@ final class SyncService: ObservableObject { // shorter than `staleAfter`, so re-stamping per attempt would let a // queue that never drains hold the overlay open forever — an unbounded // lie in place of a two-second flicker. - pendingSessionSettleStates.holdBackstop(now: Date()) + pendingSessionSettleStates.holdBackstop(uptime: ProcessInfo.processInfo.systemUptime) // A drained chat creation produced a real session; drop the optimistic // "Pending sync" snapshot so the synced row takes over. if operation.kind == "command", isQueuedChatCreationAction(operation.action) { @@ -19432,7 +19434,7 @@ final class SyncService: ObservableObject { // than `staleAfter` would snap back to unsettled in the moments between // reconnecting and replaying it. Rebase the deadlines here, at the earliest // point the connection is usable, and re-arm the sweep that enforces them. - pendingSessionSettleStates.holdBackstop(now: Date()) + pendingSessionSettleStates.holdBackstop(uptime: ProcessInfo.processInfo.systemUptime) schedulePendingSessionSettleBackstopSweep() if activeProjectId == nil { diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index ed052db5b..5b25067bf 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -7,7 +7,11 @@ import XCTest /// the local overlay that replaced it: it has to feel like the old optimistic /// write, and it has to stop lying the moment the host answers. final class PendingSessionSettleStatesTests: XCTestCase { - private let now = Date(timeIntervalSince1970: 1_760_000_000) + /// Monotonic uptime, not wall clock — the overlay measures staleness with + /// `ProcessInfo.systemUptime` so a clock change cannot expire or freeze it. + private let now: TimeInterval = 10_000 + + private func addUptime(_ base: TimeInterval, _ delta: TimeInterval) -> TimeInterval { base + delta } private func session( id: String = "session-1", @@ -46,7 +50,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testSettleIntentShowsTheRowSettledBeforeTheHostAnswers() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // A declared settle clears ANY override host-side — including a keep-active // pin, so it cannot silently veto the settle — and the overlay does too. @@ -57,7 +61,7 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testUnsettleLeavesAKeepActivePinAlone() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1", baseline: nil) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) // The host PRESERVES an `"active"` pin through an unsettle, so the overlay // must not claim it was cleared. @@ -72,50 +76,50 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// overlay can predict it exactly. func testUnsettleClearsASettledPinBecauseTheHostWill() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1", baseline: nil) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) let overlaid = states.apply(to: session(settleOverride: "settled")) XCTAssertNil(overlaid.settleOverride) // And it must not resolve while that pin is still on the replicated row. - states.prune(against: [session(settleOverride: "settled")], now: now) + states.prune(against: [session(settleOverride: "settled")], uptime: now) XCTAssertNotNil(states["session-1"]) - states.prune(against: [session()], now: now) + states.prune(against: [session()], uptime: now) XCTAssertNil(states["session-1"]) } func testSettleResolvesOnlyOnceTheHostAlsoClearedTheOverride() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // `sessionService.settleMany` / `settleSession` both set // `settle_override = null` unconditionally, so that a keep-active pin cannot // silently veto the settle the user asked for. A row that still carries one // has therefore not applied our settle yet. - states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z", settleOverride: "active")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z", settleOverride: "active")], uptime: now) XCTAssertNotNil(states["session-1"]) - states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now) XCTAssertNil(states["session-1"]) } func testIntentResolvesOnTheHostsOwnTimestampNotOurs() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // The host writes its own clock. Matching on the exact string would never // resolve, so presence is what the settle intent predicts. - states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now) XCTAssertNil(states["session-1"]) } func testIntentSurvivesUntilTheHostRowActuallyChanges() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) - states.prune(against: [session(settledAt: nil)], now: now) + states.prune(against: [session(settledAt: nil)], uptime: now) XCTAssertNotNil(states["session-1"]) XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") @@ -123,41 +127,41 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testUnsettleIntentResolvesWhenTheRowGoesBackToNull() { var states = PendingSessionSettleStates() - states.begin(.unsettle(now: now), for: "session-1", baseline: nil) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) - states.prune(against: [session(settledAt: "2026-08-10T09:00:00.000Z")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T09:00:00.000Z")], uptime: now) XCTAssertNotNil(states["session-1"]) - states.prune(against: [session(settledAt: nil)], now: now) + states.prune(against: [session(settledAt: nil)], uptime: now) XCTAssertNil(states["session-1"]) } func testOverrideIntentComparesTheExactValueWeAskedFor() { var states = PendingSessionSettleStates() - states.begin(.settleOverride("active", now: now), for: "session-1", baseline: nil) + states.begin(.settleOverride("active", uptime: now), for: "session-1", baseline: nil) // `settle_override` is a value we own, unlike the settle timestamp — a // different non-null value is the host disagreeing, not confirming. - states.prune(against: [session(settleOverride: "settled")], now: now) + states.prune(against: [session(settleOverride: "settled")], uptime: now) XCTAssertNotNil(states["session-1"]) - states.prune(against: [session(settleOverride: "active")], now: now) + states.prune(against: [session(settleOverride: "active")], uptime: now) XCTAssertNil(states["session-1"]) } func testClearingAnOverrideResolvesOnNull() { var states = PendingSessionSettleStates() - states.begin(.settleOverride(nil, now: now), for: "session-1", baseline: nil) + states.begin(.settleOverride(nil, uptime: now), for: "session-1", baseline: nil) XCTAssertNil(states.apply(to: session(settleOverride: "active")).settleOverride) - states.prune(against: [session(settleOverride: nil)], now: now) + states.prune(against: [session(settleOverride: nil)], uptime: now) XCTAssertNil(states["session-1"]) } func testAFailedCommandDropsTheIntentSoTheRowSnapsBack() { var states = PendingSessionSettleStates() - let token = states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.clear("session-1", token: token) @@ -169,8 +173,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// the user is now waiting on. func testAStaleFailureCannotRetireANewerIntent() { var states = PendingSessionSettleStates() - let stale = states.begin(.settleOverride("active", now: now), for: "session-1", baseline: nil) - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let stale = states.begin(.settleOverride("active", uptime: now), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.clear("session-1", token: stale) @@ -179,14 +183,14 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAnIntentWhoseChangesetNeverArrivesExpires() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) - let justBefore = now.addingTimeInterval(PendingSessionSettleStates.staleAfter - 1) - states.prune(against: [session(settledAt: nil)], now: justBefore) + let justBefore = addUptime(now, PendingSessionSettleStates.staleAfter - 1) + states.prune(against: [session(settledAt: nil)], uptime: justBefore) XCTAssertNotNil(states["session-1"]) - let after = now.addingTimeInterval(PendingSessionSettleStates.staleAfter) - states.prune(against: [session(settledAt: nil)], now: after) + let after = addUptime(now, PendingSessionSettleStates.staleAfter) + states.prune(against: [session(settledAt: nil)], uptime: after) XCTAssertNil(states["session-1"], "a pending overlay must not outlive its round trip indefinitely") } @@ -195,13 +199,13 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// command is still on its way, then settle it again when the queue drains. func testAQueuedSettleDoesNotExpireWhileTheHostIsUnreachable() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) var clock = now for _ in 0..<10 { - clock = clock.addingTimeInterval(PendingSessionSettleStates.staleAfter) - states.holdBackstop(now: clock) - states.prune(against: [session(settledAt: nil)], now: clock) + clock = addUptime(clock, PendingSessionSettleStates.staleAfter) + states.holdBackstop(uptime: clock) + states.prune(against: [session(settledAt: nil)], uptime: clock) } XCTAssertNotNil(states["session-1"], "an unreachable host cannot confirm, so the backstop must not run") @@ -210,36 +214,36 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testTheBackstopResumesOnceTheHostIsReachableAgain() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // Offline for well past the budget, then reachable: the clock restarts from // the moment we could have been answered, not from the tap. - let reconnectedAt = now.addingTimeInterval(600) - states.holdBackstop(now: reconnectedAt) - states.prune(against: [session(settledAt: nil)], now: reconnectedAt) + let reconnectedAt = addUptime(now, 600) + states.holdBackstop(uptime: reconnectedAt) + states.prune(against: [session(settledAt: nil)], uptime: reconnectedAt) XCTAssertNotNil(states["session-1"]) - let past = reconnectedAt.addingTimeInterval(PendingSessionSettleStates.staleAfter) - states.prune(against: [session(settledAt: nil)], now: past) + let past = addUptime(reconnectedAt, PendingSessionSettleStates.staleAfter) + states.prune(against: [session(settledAt: nil)], uptime: past) XCTAssertNil(states["session-1"]) } func testPruneReportsOnlyRealResolutionsSoRepaintCannotLoop() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // Re-stamping the offline deadline is not a resolution; reporting it as one // would repaint on every read forever. - states.holdBackstop(now: now) - XCTAssertFalse(states.prune(against: [session(settledAt: nil)], now: now)) - XCTAssertTrue(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now)) - XCTAssertFalse(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now)) + states.holdBackstop(uptime: now) + XCTAssertFalse(states.prune(against: [session(settledAt: nil)], uptime: now)) + XCTAssertTrue(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now)) + XCTAssertFalse(states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now)) } func testTheNewestCommandWins() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) - states.begin(.unsettle(now: now), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) XCTAssertNil(states.apply(to: session(settledAt: "2026-08-10T09:00:00.000Z")).settledAt) } @@ -253,18 +257,18 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAReplacementIntentSurvivesUntilTheRowActuallyMoves() { var states = PendingSessionSettleStates() let unsettled = session() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) - states.begin(.unsettle(now: now), for: "session-1", baseline: unsettled) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: unsettled) // The row has not moved yet — `settled_at` is still nil, which is also what // the unsettle wants. It must NOT count as confirmation. - states.prune(against: [unsettled], now: now) + states.prune(against: [unsettled], uptime: now) XCTAssertNotNil(states["session-1"]) // The first command lands. Still not our intent, so the overlay holds and // keeps showing the row as the user last asked for it. let settledByFirstCommand = session(settledAt: "2026-08-10T12:00:00.417Z") - states.prune(against: [settledByFirstCommand], now: now) + states.prune(against: [settledByFirstCommand], uptime: now) XCTAssertNotNil(states["session-1"]) XCTAssertNil(states.apply(to: settledByFirstCommand).settledAt) @@ -272,10 +276,10 @@ final class PendingSessionSettleStatesTests: XCTestCase { // row change cannot be attributed to one of them. It holds what the user // last asked for and yields at the backstop, by which point the run has // converged. - states.prune(against: [unsettled], now: now) + states.prune(against: [unsettled], uptime: now) XCTAssertNotNil(states["session-1"]) - states.prune(against: [unsettled], now: now.addingTimeInterval(PendingSessionSettleStates.staleAfter)) + states.prune(against: [unsettled], uptime: addUptime(now, PendingSessionSettleStates.staleAfter)) XCTAssertNil(states["session-1"]) } @@ -287,13 +291,13 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAThirdOverlappingCommandIsNotConfirmedByAnEarlierOnesChangeset() { var states = PendingSessionSettleStates() let unsettled = session() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) - states.begin(.unsettle(now: now), for: "session-1", baseline: unsettled) - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:02.000Z"), for: "session-1", baseline: unsettled) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: unsettled) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:02.000Z"), for: "session-1", baseline: unsettled) // The FIRST settle replicates. It matches the third intent by value, and it // moved the row — but it is not the third command. - states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now) XCTAssertNotNil(states["session-1"], "an earlier command's changeset must not confirm the latest one") } @@ -305,8 +309,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testUnsettleAfterAnUnlandedSettleDoesNotResurrectAKeepActivePin() { var states = PendingSessionSettleStates() let pinned = session(settleOverride: "active") - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: pinned) - states.begin(.unsettle(now: now), for: "session-1", baseline: pinned) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: pinned) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: pinned) let overlaid = states.apply(to: pinned) XCTAssertNil(overlaid.settledAt) @@ -318,23 +322,23 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAStandaloneUnsettleStillPreservesAKeepActivePin() { var states = PendingSessionSettleStates() let pinned = session(settledAt: "2026-08-10T09:00:00.000Z", settleOverride: "active") - states.begin(.unsettle(now: now), for: "session-1", baseline: pinned) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: pinned) XCTAssertEqual(states.apply(to: pinned).settleOverride, "active") } func testAnUnknownBaselineFallsBackToValueEquality() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) - states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], now: now) + states.prune(against: [session(settledAt: "2026-08-10T12:00:00.417Z")], uptime: now) XCTAssertNil(states["session-1"], "with no baseline the value match is all we have") } func testRemoveAllForgetsEverythingInFlight() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) states.removeAll() @@ -344,17 +348,17 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testASessionMissingFromAScopedReadKeepsItsIntent() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) // A partial or differently-scoped read is not the host disagreeing. - states.prune(against: [session(id: "session-2", settledAt: nil)], now: now) + states.prune(against: [session(id: "session-2", settledAt: nil)], uptime: now) XCTAssertNotNil(states["session-1"]) } func testOverlayOnlyTouchesTheSessionItWasBegunFor() { var states = PendingSessionSettleStates() - states.begin(.settle(now: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) let others = states.apply(to: [session(id: "session-1"), session(id: "session-2")]) @@ -442,7 +446,7 @@ final class PendingSessionSettleOverlayWiringTests: XCTestCase { func testFetchSessionsAppliesTheOverlayWhileTheDatabaseStaysUntouched() async throws { try await withService { service, database in service.beginPendingSessionSettleForTesting( - .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + .settle(uptime: ProcessInfo.processInfo.systemUptime, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1" ) @@ -459,7 +463,7 @@ final class PendingSessionSettleOverlayWiringTests: XCTestCase { func testFetchSessionByIdGoesThroughTheSameChokepoint() async throws { try await withService { service, _ in service.beginPendingSessionSettleForTesting( - .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + .settle(uptime: ProcessInfo.processInfo.systemUptime, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1" ) @@ -481,7 +485,7 @@ final class PendingSessionSettleOverlayWiringTests: XCTestCase { ) service.beginPendingSessionSettleForTesting( - .settle(now: Date(), timestamp: "2026-08-10T12:00:00.000Z"), + .settle(uptime: ProcessInfo.processInfo.systemUptime, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1" ) service.refreshActiveSessionsAndSnapshot() From cd6600fecb5436016b242bb834234b5068f020c8 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:15:25 -0400 Subject: [PATCH 11/15] fix(ios): start the overlay's countdown when the command is answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): a 20s backstop against a 30s request timeout is inconsistent. The window started at send, so a connected host that took longer than `staleAfter` to answer had the overlay expired underneath a command that was still perfectly valid — the row snapped back to replicated state and then flipped again when the host succeeded. A long host operation also left progressively less of the window for the confirming changeset. `staleAfter` bounds the wait for the CHANGESET, not the round trip, so the countdown now restarts when the command is answered. Token-scoped, so a slow command cannot extend an intent the user has since replaced — covered by its own test alongside the restart itself. --- .../Services/PendingSessionSettleStates.swift | 12 ++++++++ apps/ios/ADE/Services/SyncService.swift | 9 ++++++ .../PendingSessionSettleStatesTests.swift | 28 +++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index 0ccaea5ad..c7e69c766 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -207,6 +207,18 @@ struct PendingSessionSettleStates: Equatable { intents.removeValue(forKey: sessionId) } + /// Restart one intent's window because its command has just been answered. + /// + /// `staleAfter` bounds the wait for the CHANGESET, not the round trip: the + /// request itself may legitimately run to its own (longer) timeout, and + /// expiring at 20s while the command is still valid would snap the row back + /// and then flip it again when the host succeeded. Token-scoped so a slow + /// command cannot extend an intent the user has since replaced. + mutating func restartBackstop(for sessionId: String, token: UInt64, uptime: TimeInterval) { + guard intents[sessionId]?.token == token else { return } + intents[sessionId]?.startedAtUptime = uptime + } + /// Forget everything in flight — used when the ground the overlay refers to /// moves, e.g. a project or host switch, where the session ids it holds no /// longer describe what is on screen. diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index ad4fded66..de883f5cc 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9465,6 +9465,15 @@ final class SyncService: ObservableObject { resultShape: resultShape, rollback: { [weak self] in self?.clearPendingSessionSettle(trimmed, token: token) } ) + // Answered. `staleAfter` bounds the wait for the CHANGESET, and the request + // itself may run to a longer timeout than that — starting the countdown at + // send would expire a command that is still perfectly valid. + pendingSessionSettleStates.restartBackstop( + for: trimmed, + token: token, + uptime: ProcessInfo.processInfo.systemUptime + ) + schedulePendingSessionSettleBackstopSweep() } /// Snooze-family command: writes the snooze overlay columns optimistically and diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 5b25067bf..9fc4aa00d 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -327,6 +327,34 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertEqual(states.apply(to: pinned).settleOverride, "active") } + /// The request may legitimately run longer than `staleAfter`, so the window + /// has to measure the wait for the CHANGESET, not the round trip. + func testAnAnsweredCommandRestartsItsWindow() { + var states = PendingSessionSettleStates() + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + let answeredAt = addUptime(now, PendingSessionSettleStates.staleAfter - 1) + states.restartBackstop(for: "session-1", token: token, uptime: answeredAt) + + // Past the original deadline but inside the restarted one. + states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter + 1)) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session()], uptime: addUptime(answeredAt, PendingSessionSettleStates.staleAfter)) + XCTAssertNil(states["session-1"]) + } + + func testASlowCommandCannotExtendAnIntentTheUserReplaced() { + var states = PendingSessionSettleStates() + let stale = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) + + states.restartBackstop(for: "session-1", token: stale, uptime: addUptime(now, 100)) + + states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter)) + XCTAssertNil(states["session-1"], "the replaced command's answer must not extend the newer intent") + } + func testAnUnknownBaselineFallsBackToValueEquality() { var states = PendingSessionSettleStates() states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) From d617e60dc69e7dbe48804deefc9450ba681caca2 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:33:48 -0400 Subject: [PATCH 12/15] fix(ios): an outstanding request cannot be expired out from under itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): the previous fix restarted the window after the response, but the sweep armed at send still fired mid-await and removed the intent — and restarting a window cannot restore an intent that is already gone. An intent is now unexpirable while its own request is outstanding, and the window starts when the response arrives. Neither `prune` nor the sweep can retire it in between. Bounded by the request's own timeout: a failure throws and the rollback clears the intent. Three existing tests were asserting expiry on intents whose request had never answered, which is no longer the contract — they now answer first. Added a test for the outstanding-request case itself, and kept the one proving a late answer from a replaced command cannot extend the newer intent. --- .../Services/PendingSessionSettleStates.swift | 12 +++++- .../PendingSessionSettleStatesTests.swift | 38 ++++++++++++++++--- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index c7e69c766..f8e1bcf21 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -67,6 +67,12 @@ struct PendingSessionSettleIntent: Equatable { /// converged. Briefly trailing the truth beats confidently showing the wrong /// command's result. fileprivate var replacedInFlight = false + /// True until the command's own request returns. The backstop bounds the wait + /// for the CHANGESET, so it must not run while the request itself is still + /// legitimately outstanding — the request may take longer than `staleAfter`, + /// and an intent the sweep has already removed cannot be restored by + /// restarting its window afterwards. + fileprivate var awaitingResponse = true struct Baseline: Equatable { var settledAt: String? @@ -216,6 +222,7 @@ struct PendingSessionSettleStates: Equatable { /// command cannot extend an intent the user has since replaced. mutating func restartBackstop(for sessionId: String, token: UInt64, uptime: TimeInterval) { guard intents[sessionId]?.token == token else { return } + intents[sessionId]?.awaitingResponse = false intents[sessionId]?.startedAtUptime = uptime } @@ -269,7 +276,10 @@ struct PendingSessionSettleStates: Equatable { guard !intent.replacedInFlight, movedSinceCommand, intent.isSatisfied(by: session) else { continue } intents.removeValue(forKey: session.id) } - intents = intents.filter { uptime - $0.value.startedAtUptime < PendingSessionSettleStates.staleAfter } + intents = intents.filter { entry in + entry.value.awaitingResponse + || uptime - entry.value.startedAtUptime < PendingSessionSettleStates.staleAfter + } return intents.count != before } diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 9fc4aa00d..12955f106 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -183,7 +183,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testAnIntentWhoseChangesetNeverArrivesExpires() { var states = PendingSessionSettleStates() - states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.restartBackstop(for: "session-1", token: token, uptime: now) let justBefore = addUptime(now, PendingSessionSettleStates.staleAfter - 1) states.prune(against: [session(settledAt: nil)], uptime: justBefore) @@ -199,7 +200,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// command is still on its way, then settle it again when the queue drains. func testAQueuedSettleDoesNotExpireWhileTheHostIsUnreachable() { var states = PendingSessionSettleStates() - states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.restartBackstop(for: "session-1", token: token, uptime: now) var clock = now for _ in 0..<10 { @@ -214,7 +216,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { func testTheBackstopResumesOnceTheHostIsReachableAgain() { var states = PendingSessionSettleStates() - states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.restartBackstop(for: "session-1", token: token, uptime: now) // Offline for well past the budget, then reachable: the clock restarts from // the moment we could have been answered, not from the tap. @@ -258,7 +261,8 @@ final class PendingSessionSettleStatesTests: XCTestCase { var states = PendingSessionSettleStates() let unsettled = session() states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: unsettled) - states.begin(.unsettle(uptime: now), for: "session-1", baseline: unsettled) + let token = states.begin(.unsettle(uptime: now), for: "session-1", baseline: unsettled) + states.restartBackstop(for: "session-1", token: token, uptime: now) // The row has not moved yet — `settled_at` is still nil, which is also what // the unsettle wants. It must NOT count as confirmation. @@ -344,11 +348,35 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(states["session-1"]) } + /// The sweep armed when the command was sent must not remove an intent whose + /// request is still outstanding — restarting the window afterwards cannot + /// bring back an intent that is already gone. + func testAnOutstandingRequestCannotBeExpiredBySweepOrPrune() { + var states = PendingSessionSettleStates() + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + // Well past the deadline, but the request has not answered yet. + states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter * 3)) + XCTAssertNotNil(states["session-1"]) + + let answeredAt = addUptime(now, PendingSessionSettleStates.staleAfter * 3) + states.restartBackstop(for: "session-1", token: token, uptime: answeredAt) + + states.prune(against: [session()], uptime: addUptime(answeredAt, 1)) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session()], uptime: addUptime(answeredAt, PendingSessionSettleStates.staleAfter)) + XCTAssertNil(states["session-1"], "once answered, the window applies normally") + } + func testASlowCommandCannotExtendAnIntentTheUserReplaced() { var states = PendingSessionSettleStates() let stale = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) - states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) + let current = states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) + states.restartBackstop(for: "session-1", token: current, uptime: now) + // The replaced command answers late; it must not push the newer intent's + // deadline out. states.restartBackstop(for: "session-1", token: stale, uptime: addUptime(now, 100)) states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter)) From 09906b35bb796edf1a3d997fb038caf8a5c6c8aa Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:22:39 -0400 Subject: [PATCH 13/15] fix(ios): a queued command is not an answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): `{queued: true}` is durable acceptance by this device, not an answer from the host — it has not seen the command at all. Marking the intent answered started a window that could expire during reconnect, while hydration or the replay request (with its own longer timeout) was still running, snapping the row back before the queued settle finished. The lifecycle send now reports whether the host actually answered, and a queued command leaves its intent outstanding. `flushPendingOperations` marks exactly the session ids carried by the replayed command as answered and starts their window then, so the state is restored by the replay rather than never. --- .../Services/PendingSessionSettleStates.swift | 10 +++++ apps/ios/ADE/Services/SyncService.swift | 42 ++++++++++++++++--- .../PendingSessionSettleStatesTests.swift | 21 ++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index f8e1bcf21..d3d76f0c7 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -226,6 +226,16 @@ struct PendingSessionSettleStates: Equatable { intents[sessionId]?.startedAtUptime = uptime } + /// A durably-queued command has now been replayed and answered. Until this, + /// the `queued` sentinel is not an answer — the host has not seen the command + /// at all — so the intent stays outstanding rather than starting a window it + /// could expire inside while the replay is still running. + mutating func markAnswered(for sessionId: String, uptime: TimeInterval) { + guard intents[sessionId] != nil else { return } + intents[sessionId]?.awaitingResponse = false + intents[sessionId]?.startedAtUptime = uptime + } + /// Forget everything in flight — used when the ground the overlay refers to /// moves, e.g. a project or host switch, where the session ids it holds no /// longer describe what is on screen. diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index de883f5cc..c4d92e8bf 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9420,7 +9420,7 @@ final class SyncService: ObservableObject { // `lifecycleCall` call sites that pass no `applied` predicate. resultShape: SessionLifecycleResultShape? = .envelope, rollback: @escaping () -> Void - ) async throws { + ) async throws -> Bool { let scope = chatCommandScope(for: trimmed) let result: Any do { @@ -9444,6 +9444,14 @@ final class SyncService: ObservableObject { rollback() throw sessionLifecycleNotAppliedError(action) } + // `{queued: true}` is durable acceptance by THIS device, not an answer from + // the host — it has not seen the command yet. + return !syncCommandResultWasQueued(result) + } + + private func syncCommandResultWasQueued(_ result: Any) -> Bool { + guard let record = result as? [String: Any] else { return false } + return record["ok"] == nil && record["queued"] as? Bool == true } /// Settle-family command: shows the change through the local overlay, which is @@ -9458,16 +9466,19 @@ final class SyncService: ObservableObject { guard let trimmed = normalizedLifecycleSessionId(sessionId) else { return } guard supportsRemoteAction(action) else { throw sessionLifecycleUnsupportedError(action) } let token = beginPendingSessionSettle(intent, for: trimmed) - try await sendSessionLifecycleCommand( + let answered = try await sendSessionLifecycleCommand( sessionId: trimmed, action: action, args: args, resultShape: resultShape, rollback: { [weak self] in self?.clearPendingSessionSettle(trimmed, token: token) } ) - // Answered. `staleAfter` bounds the wait for the CHANGESET, and the request - // itself may run to a longer timeout than that — starting the countdown at - // send would expire a command that is still perfectly valid. + // Only a real answer starts the window. `staleAfter` bounds the wait for the + // CHANGESET, and the request itself may run to a longer timeout than that, + // so counting from send would expire a command that is still valid. A + // durably-queued command has not been answered at all — it stays + // outstanding until `flushPendingOperations` replays it. + guard answered else { return } pendingSessionSettleStates.restartBackstop( for: trimmed, token: token, @@ -9511,7 +9522,7 @@ final class SyncService: ObservableObject { wokeReason: wokeReason ) - try await sendSessionLifecycleCommand( + _ = try await sendSessionLifecycleCommand( sessionId: trimmed, action: action, args: args, @@ -9532,6 +9543,18 @@ final class SyncService: ObservableObject { ) } + /// Session ids a replayed settle-family command applies to, so the overlay + /// can tell which outstanding intents that replay just answered. + private func queuedLifecycleSessionIds(action: String, args: [String: Any]) -> [String] { + guard action.hasPrefix("session.") else { return [] } + var ids: [String] = [] + if let single = args["sessionId"] as? String { ids.append(single) } + if let many = args["sessionIds"] as? [Any] { + ids.append(contentsOf: many.compactMap { $0 as? String }) + } + return ids.compactMap(normalizedLifecycleSessionId) + } + private func normalizedLifecycleSessionId(_ sessionId: String) -> String? { let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) return trimmed.isEmpty ? nil : trimmed @@ -18731,7 +18754,14 @@ final class SyncService: ObservableObject { // shorter than `staleAfter`, so re-stamping per attempt would let a // queue that never drains hold the overlay open forever — an unbounded // lie in place of a two-second flicker. + for sessionId in queuedLifecycleSessionIds(action: operation.action, args: args) { + pendingSessionSettleStates.markAnswered( + for: sessionId, + uptime: ProcessInfo.processInfo.systemUptime + ) + } pendingSessionSettleStates.holdBackstop(uptime: ProcessInfo.processInfo.systemUptime) + schedulePendingSessionSettleBackstopSweep() // A drained chat creation produced a real session; drop the optimistic // "Pending sync" snapshot so the synced row takes over. if operation.kind == "command", isQueuedChatCreationAction(operation.action) { diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index 12955f106..f4239cff1 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -369,6 +369,27 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(states["session-1"], "once answered, the window applies normally") } + /// The `queued` sentinel is durable acceptance by this device, not an answer + /// from the host. Treating it as answered would start a window that can expire + /// while the reconnect replay — with its own longer timeout — is still running. + func testAQueuedCommandStaysOutstandingUntilTheReplayAnswers() { + var states = PendingSessionSettleStates() + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + // Queued: no `restartBackstop`. Far past the window, it must survive. + states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter * 5)) + XCTAssertNotNil(states["session-1"]) + + let replayedAt = addUptime(now, PendingSessionSettleStates.staleAfter * 5) + states.markAnswered(for: "session-1", uptime: replayedAt) + + states.prune(against: [session()], uptime: addUptime(replayedAt, 1)) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session()], uptime: addUptime(replayedAt, PendingSessionSettleStates.staleAfter)) + XCTAssertNil(states["session-1"]) + } + func testASlowCommandCannotExtendAnIntentTheUserReplaced() { var states = PendingSessionSettleStates() let stale = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) From 6d93b06538e77ddb6792c465094e3b035a7d9058 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:55:38 -0400 Subject: [PATCH 14/15] fix(ios): bind a queued settle to its operation, and retire it if the host refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings (Codex, P2 each, verified), both from one missing link: the durable queue entry carried no reference back to the intent that created it. **A refused replay left the overlay painting forever.** The terminal-error path removed the queued operation without touching the intent, which stayed `awaitingResponse` — and an outstanding intent is deliberately unexpirable. A dismiss-and-settle the host rejects on replay would have shown a refused state indefinitely. **A replay resolved the wrong intent.** With two commands for one session queued together, they drain in append order, but completion was matched on session id alone — so the first replay started the second intent's window before its own replay had begun, and a replay slower than `staleAfter` (inside its valid timeout) expired the overlay mid-request. The queued sentinel now carries its `commandId`, the intent records it, and both replay outcomes are matched on that id: success answers exactly that intent, terminal failure retires it. --- .../Services/PendingSessionSettleStates.swift | 33 +++++++-- apps/ios/ADE/Services/SyncService.swift | 67 ++++++++++--------- .../PendingSessionSettleStatesTests.swift | 36 +++++++++- 3 files changed, 100 insertions(+), 36 deletions(-) diff --git a/apps/ios/ADE/Services/PendingSessionSettleStates.swift b/apps/ios/ADE/Services/PendingSessionSettleStates.swift index d3d76f0c7..e29d422a6 100644 --- a/apps/ios/ADE/Services/PendingSessionSettleStates.swift +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -73,6 +73,11 @@ struct PendingSessionSettleIntent: Equatable { /// and an intent the sweep has already removed cannot be restored by /// restarting its window afterwards. fileprivate var awaitingResponse = true + /// The durable queue entry this command became, when it was queued offline. + /// Replay outcomes are matched on this rather than the session id: two + /// commands for one session can be queued together, and the first replay's + /// completion must not resolve the second's intent. + fileprivate var queuedOperationId: String? struct Baseline: Equatable { var settledAt: String? @@ -226,14 +231,34 @@ struct PendingSessionSettleStates: Equatable { intents[sessionId]?.startedAtUptime = uptime } + /// Bind an intent to the durable queue entry its command became. + mutating func attachQueuedOperation(_ operationId: String, for sessionId: String, token: UInt64) { + guard intents[sessionId]?.token == token else { return } + intents[sessionId]?.queuedOperationId = operationId + } + /// A durably-queued command has now been replayed and answered. Until this, /// the `queued` sentinel is not an answer — the host has not seen the command /// at all — so the intent stays outstanding rather than starting a window it /// could expire inside while the replay is still running. - mutating func markAnswered(for sessionId: String, uptime: TimeInterval) { - guard intents[sessionId] != nil else { return } - intents[sessionId]?.awaitingResponse = false - intents[sessionId]?.startedAtUptime = uptime + mutating func markAnswered(forOperation operationId: String, uptime: TimeInterval) { + guard let key = key(forOperation: operationId) else { return } + intents[key]?.awaitingResponse = false + intents[key]?.startedAtUptime = uptime + } + + /// The replay failed terminally — the host refused it. Retire the intent + /// rather than leave it outstanding, which would paint a refused state + /// indefinitely because an outstanding intent cannot expire. + @discardableResult + mutating func clear(forOperation operationId: String) -> Bool { + guard let key = key(forOperation: operationId) else { return false } + intents.removeValue(forKey: key) + return true + } + + private func key(forOperation operationId: String) -> String? { + intents.first { $0.value.queuedOperationId == operationId }?.key } /// Forget everything in flight — used when the ground the overlay refers to diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index c4d92e8bf..c368524dc 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9334,6 +9334,11 @@ final class SyncService: ObservableObject { return token } + private func clearPendingSessionSettle(forOperation operationId: String) { + guard pendingSessionSettleStates.clear(forOperation: operationId) else { return } + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + } + private func clearPendingSessionSettle(_ sessionId: String, token: UInt64) { guard pendingSessionSettleStates[sessionId] != nil else { return } pendingSessionSettleStates.clear(sessionId, token: token) @@ -9420,7 +9425,7 @@ final class SyncService: ObservableObject { // `lifecycleCall` call sites that pass no `applied` predicate. resultShape: SessionLifecycleResultShape? = .envelope, rollback: @escaping () -> Void - ) async throws -> Bool { + ) async throws -> String? { let scope = chatCommandScope(for: trimmed) let result: Any do { @@ -9445,13 +9450,17 @@ final class SyncService: ObservableObject { throw sessionLifecycleNotAppliedError(action) } // `{queued: true}` is durable acceptance by THIS device, not an answer from - // the host — it has not seen the command yet. - return !syncCommandResultWasQueued(result) + // the host — it has not seen the command yet. Returns the queued + // operation's id so the caller can bind its optimistic state to it. + return syncQueuedCommandId(result) } - private func syncCommandResultWasQueued(_ result: Any) -> Bool { - guard let record = result as? [String: Any] else { return false } - return record["ok"] == nil && record["queued"] as? Bool == true + private func syncQueuedCommandId(_ result: Any) -> String? { + guard let record = result as? [String: Any], + record["ok"] == nil, + record["queued"] as? Bool == true + else { return nil } + return (record["commandId"] as? String).flatMap(normalizedLifecycleSessionId) } /// Settle-family command: shows the change through the local overlay, which is @@ -9466,7 +9475,7 @@ final class SyncService: ObservableObject { guard let trimmed = normalizedLifecycleSessionId(sessionId) else { return } guard supportsRemoteAction(action) else { throw sessionLifecycleUnsupportedError(action) } let token = beginPendingSessionSettle(intent, for: trimmed) - let answered = try await sendSessionLifecycleCommand( + let queuedCommandId = try await sendSessionLifecycleCommand( sessionId: trimmed, action: action, args: args, @@ -9476,9 +9485,13 @@ final class SyncService: ObservableObject { // Only a real answer starts the window. `staleAfter` bounds the wait for the // CHANGESET, and the request itself may run to a longer timeout than that, // so counting from send would expire a command that is still valid. A - // durably-queued command has not been answered at all — it stays - // outstanding until `flushPendingOperations` replays it. - guard answered else { return } + // durably-queued command has not been answered at all — bind the intent to + // that operation so the replay's outcome, success or terminal failure, + // resolves this intent and no other. + if let queuedCommandId { + pendingSessionSettleStates.attachQueuedOperation(queuedCommandId, for: trimmed, token: token) + return + } pendingSessionSettleStates.restartBackstop( for: trimmed, token: token, @@ -9543,18 +9556,6 @@ final class SyncService: ObservableObject { ) } - /// Session ids a replayed settle-family command applies to, so the overlay - /// can tell which outstanding intents that replay just answered. - private func queuedLifecycleSessionIds(action: String, args: [String: Any]) -> [String] { - guard action.hasPrefix("session.") else { return [] } - var ids: [String] = [] - if let single = args["sessionId"] as? String { ids.append(single) } - if let many = args["sessionIds"] as? [Any] { - ids.append(contentsOf: many.compactMap { $0 as? String }) - } - return ids.compactMap(normalizedLifecycleSessionId) - } - private func normalizedLifecycleSessionId(_ sessionId: String) -> String? { let trimmed = sessionId.trimmingCharacters(in: .whitespacesAndNewlines) return trimmed.isEmpty ? nil : trimmed @@ -18754,12 +18755,10 @@ final class SyncService: ObservableObject { // shorter than `staleAfter`, so re-stamping per attempt would let a // queue that never drains hold the overlay open forever — an unbounded // lie in place of a two-second flicker. - for sessionId in queuedLifecycleSessionIds(action: operation.action, args: args) { - pendingSessionSettleStates.markAnswered( - for: sessionId, - uptime: ProcessInfo.processInfo.systemUptime - ) - } + pendingSessionSettleStates.markAnswered( + forOperation: operation.id, + uptime: ProcessInfo.processInfo.systemUptime + ) pendingSessionSettleStates.holdBackstop(uptime: ProcessInfo.processInfo.systemUptime) schedulePendingSessionSettleBackstopSweep() // A drained chat creation produced a real session; drop the optimistic @@ -18778,6 +18777,11 @@ final class SyncService: ObservableObject { } if isRemoteCommandApplicationError(error) || (stillLive && !isSyncRequestTimeoutError(error)) { removePendingOperation(operation) + // Terminal: the host rejected the replay (a dismiss-and-settle whose + // prompt has since changed, say). Retire the intent — leaving it + // outstanding would paint a state the host refused, indefinitely, + // because an outstanding intent is deliberately unexpirable. + clearPendingSessionSettle(forOperation: operation.id) if operation.kind == "command", isQueuedChatCreationAction(operation.action) { removePendingChatCreation(id: operation.id) } @@ -19874,7 +19878,10 @@ extension SyncService { if stillLive, isSyncRequestTimeoutError(error) { verifyTransportAliveAfterSilence(error as NSError, trigger: "request_timeout") } - return ["queued": true] + // `commandId` lets a caller tie local optimistic state to the exact + // queued operation, so the replay's outcome resolves that state and + // nothing else's. + return ["queued": true, "commandId": commandId] } throw error } @@ -19883,7 +19890,7 @@ extension SyncService { throw NSError(domain: "ADE", code: 15, userInfo: [NSLocalizedDescriptionKey: "Offline — command dropped."]) } try enqueueOperation(kind: "command", action: action, args: args, id: commandId) - return ["queued": true] + return ["queued": true, "commandId": commandId] } // MARK: - Workspace snapshot debounce diff --git a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift index f4239cff1..6f5aa9a98 100644 --- a/apps/ios/ADETests/PendingSessionSettleStatesTests.swift +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -374,14 +374,15 @@ final class PendingSessionSettleStatesTests: XCTestCase { /// while the reconnect replay — with its own longer timeout — is still running. func testAQueuedCommandStaysOutstandingUntilTheReplayAnswers() { var states = PendingSessionSettleStates() - states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.attachQueuedOperation("op-1", for: "session-1", token: token) // Queued: no `restartBackstop`. Far past the window, it must survive. states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter * 5)) XCTAssertNotNil(states["session-1"]) let replayedAt = addUptime(now, PendingSessionSettleStates.staleAfter * 5) - states.markAnswered(for: "session-1", uptime: replayedAt) + states.markAnswered(forOperation: "op-1", uptime: replayedAt) states.prune(against: [session()], uptime: addUptime(replayedAt, 1)) XCTAssertNotNil(states["session-1"]) @@ -390,6 +391,37 @@ final class PendingSessionSettleStatesTests: XCTestCase { XCTAssertNil(states["session-1"]) } + /// Two commands for one session queued together drain in append order. The + /// first replay's completion must not resolve the second's intent — that + /// would start its window before its own replay had even begun. + func testAReplayResolvesOnlyItsOwnQueuedIntent() { + var states = PendingSessionSettleStates() + let first = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.attachQueuedOperation("op-first", for: "session-1", token: first) + let second = states.begin(.unsettle(uptime: now), for: "session-1", baseline: nil) + states.attachQueuedOperation("op-second", for: "session-1", token: second) + + states.markAnswered(forOperation: "op-first", uptime: now) + + // Still the first operation's id on record? No — the live intent is the + // second, and it has not been replayed, so it stays outstanding. + states.prune(against: [session()], uptime: addUptime(now, PendingSessionSettleStates.staleAfter * 3)) + XCTAssertNotNil(states["session-1"]) + } + + /// A replay the host refuses must retire its intent. Leaving it outstanding + /// would paint a refused state indefinitely, since an outstanding intent + /// deliberately cannot expire. + func testATerminallyRejectedReplayRetiresItsIntent() { + var states = PendingSessionSettleStates() + let token = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + states.attachQueuedOperation("op-1", for: "session-1", token: token) + + XCTAssertTrue(states.clear(forOperation: "op-1")) + XCTAssertNil(states["session-1"]) + XCTAssertFalse(states.clear(forOperation: "op-1")) + } + func testASlowCommandCannotExtendAnIntentTheUserReplaced() { var states = PendingSessionSettleStates() let stale = states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) From e7b6f32fdd3cf0cf3d14a3a553b90cf44f61ec32 Mon Sep 17 00:00:00 2001 From: Arul Sharma <31745423+arul28@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:22:40 -0400 Subject: [PATCH 15/15] fix(ios): return the queue id from the sender the lifecycle path actually uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (Codex, P2, verified): the previous commit added `commandId` to `performCommandRequestSafe`, which the lifecycle path does not call. The sender it does use returned a bare `["queued": true]` — and its offline branch did not even pass an id, letting `enqueueOperation` generate one nobody could see. So the extraction always returned nil, the whole operation-binding fix was dead code, and a queued command was once again treated as answered: window started, no id attached, nothing for replay success or rejection to match. A fix in the wrong function reads as a fix and is worse than none, because the tests around it still pass. Both queue branches of that sender now return the enqueued id, with the offline branch generating it explicitly rather than leaving it implicit. --- apps/ios/ADE/Services/SyncService.swift | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index c368524dc..3b95377bf 100644 --- a/apps/ios/ADE/Services/SyncService.swift +++ b/apps/ios/ADE/Services/SyncService.swift @@ -9460,7 +9460,10 @@ final class SyncService: ObservableObject { record["ok"] == nil, record["queued"] as? Bool == true else { return nil } - return (record["commandId"] as? String).flatMap(normalizedLifecycleSessionId) + guard let id = (record["commandId"] as? String)?.trimmingCharacters(in: .whitespacesAndNewlines), + !id.isEmpty + else { return nil } + return id } /// Settle-family command: shows the change through the local overlay, which is @@ -18899,7 +18902,9 @@ final class SyncService: ObservableObject { if stillLive, isSyncRequestTimeoutError(error) { verifyTransportAliveAfterSilence(error as NSError, trigger: "request_timeout") } - return ["queued": true] + // Carry the queue entry's id so a caller holding optimistic state can + // bind it to this exact operation and resolve it on the replay. + return ["queued": true, "commandId": commandId] } throw error } @@ -18910,15 +18915,17 @@ final class SyncService: ObservableObject { guard policy.queueable == true else { throw NSError(domain: "ADE", code: 15, userInfo: [NSLocalizedDescriptionKey: "This action requires a live connection to the machine."]) } + let queuedCommandId = makeRequestId() try enqueueOperation( kind: "command", action: action, args: args, + id: queuedCommandId, targetProjectId: targetProjectId, targetProjectRootPath: targetProjectRootPath, fallbackToActiveProjectScope: fallbackToActiveProjectScope ) - return ["queued": true] + return ["queued": true, "commandId": queuedCommandId] } /// True when this device is paired to a machine (active or last-saved