diff --git a/apps/ade-cli/src/services/sync/syncHostService.test.ts b/apps/ade-cli/src/services/sync/syncHostService.test.ts index 7fb60dc0d..f49704d61 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.test.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.test.ts @@ -7846,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({ @@ -7977,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 42fb1782e..ba356fd29 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 HOST_AUTHORITATIVE_COLUMNS_BY_TABLE = new Map>([ + ["terminal_sessions", new Set(["settled_at", "settle_override", "settle_source"])], +]); + +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, ...SYNC_HOST_AUTHORITATIVE_TABLES, @@ -7596,7 +7625,21 @@ 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). + // The two rules are not symmetric: the table rule applies to every + // peer, the column rule only to phones. + // + // `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; + 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..dc6cbe8a1 100644 --- a/apps/ios/ADE/Services/Database.swift +++ b/apps/ios/ADE/Services/Database.swift @@ -2092,31 +2092,30 @@ 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 — 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, /// `.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 +2124,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 +2144,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..e29d422a6 --- /dev/null +++ b/apps/ios/ADE/Services/PendingSessionSettleStates.swift @@ -0,0 +1,330 @@ +import Foundation + +/// A settle-family change the phone has sent to the host and is still waiting on. +/// +/// 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 { + /// 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, 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 + /// 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 + /// 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?? + /// 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 + /// 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 + /// 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? + var settleOverride: String? + } + + static func settle(uptime: TimeInterval, timestamp: String) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .settle(timestamp: timestamp), startedAtUptime: uptime) + } + + static func unsettle(uptime: TimeInterval) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .unsettle, startedAtUptime: uptime) + } + + static func settleOverride(_ value: String?, uptime: TimeInterval) -> PendingSessionSettleIntent { + PendingSessionSettleIntent(kind: .override(value), startedAtUptime: uptime) + } + + func applied(to session: TerminalSessionSummary) -> TerminalSessionSummary { + var next = session + 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 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): + next.settleOverride = value + } + return next + } + + /// 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 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 + } +} + +/// 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. 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[sessionId] } + + /// 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, + baseline: TerminalSessionSummary? + ) -> UInt64 { + nextToken &+= 1 + 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) + } + stamped.replacedInFlight = intents[sessionId] != nil + intents[sessionId] = stamped + return nextToken + } + + /// Drop an intent because its command failed. The row snaps back to whatever + /// the host actually has, which is the honest answer. + /// + /// 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) + } + + /// 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]?.awaitingResponse = false + 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(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 + /// 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(uptime: TimeInterval) { + for key in intents.keys { + intents[key]?.startedAtUptime = uptime + } + } + + /// 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], uptime: TimeInterval) -> Bool { + guard !intents.isEmpty else { return false } + let before = intents.count + for session in sessions { + 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 !intent.replacedInFlight, movedSinceCommand, intent.isSatisfied(by: session) else { continue } + intents.removeValue(forKey: session.id) + } + intents = intents.filter { entry in + entry.value.awaitingResponse + || uptime - entry.value.startedAtUptime < PendingSessionSettleStates.staleAfter + } + return intents.count != before + } + + func apply(to session: TerminalSessionSummary) -> TerminalSessionSummary { + guard let intent = intents[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) } + } +} diff --git a/apps/ios/ADE/Services/SyncService.swift b/apps/ios/ADE/Services/SyncService.swift index 675dd5428..3b95377bf 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() @@ -5508,6 +5509,7 @@ final class SyncService: ObservableObject { roamTask?.cancel() reconnectStabilityTask?.cancel() pendingOperationFlushTask?.cancel() + pendingSessionSettleBackstopTask?.cancel() outboundCursorPersistTask?.cancel() remoteCursorProfilePersistTask?.cancel() lanePresenceHeartbeatTask?.cancel() @@ -8451,6 +8453,7 @@ final class SyncService: ObservableObject { saveProfile(nil) saveRemoteCommandDescriptors([]) clearPendingChatCreations() + resetPendingSessionSettleStates() resetChatEventState(clearHistory: true) resetTerminalSubscriptionState(clearHistory: true) activeHostProfile = nil @@ -9145,11 +9148,53 @@ final class SyncService: ObservableObject { } func fetchSessions() async throws -> [TerminalSessionSummary] { - database.fetchSessions() + localSessions() } func fetchSession(id sessionId: String) async throws -> TerminalSessionSummary? { - database.fetchSession(id: sessionId) + localSession(id: sessionId) + } + + /// **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 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(uptime: uptime) + } + guard pendingSessionSettleStates.prune(against: sessions, uptime: uptime) else { return } + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) } /// Best-effort hydration for a session whose local DB row may not have synced @@ -9163,10 +9208,10 @@ final class SyncService: ObservableObject { /// state. @discardableResult func ensureSessionRowHydrated(sessionId: String) async -> TerminalSessionSummary? { - if let existing = database.fetchSession(id: sessionId) { return existing } + if let existing = localSession(id: sessionId) { return existing } if canSendLiveRequests() { try? await refreshWorkSessions() - if let refreshed = database.fetchSession(id: sessionId) { return refreshed } + 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 @@ -9174,7 +9219,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 row } + if let row = localSession(id: sessionId) { return row } } return nil } @@ -9251,9 +9296,94 @@ 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** 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 + // 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 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 + /// `adeDatabaseDidChange`. + private func beginPendingSessionSettle( + _ intent: PendingSessionSettleIntent, + for sessionId: String + ) -> UInt64 { + // 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"]) + schedulePendingSessionSettleBackstopSweep() + 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) + scheduleProjectionRevisionBumpAfterDatabaseChange(touchedTables: ["terminal_sessions"]) + } + + /// 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. + /// 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() + // 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 /// desktop builds simply do not have them; the UI hides the affordances @@ -9281,65 +9411,21 @@ final class SyncService: ObservableObject { ]) } - /// Optimistic local write + host command, with rollback on failure. + /// Send a lifecycle host command and undo local optimism if it does not take. + /// + /// 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, - settledAt: String?? = nil, - settleOverride: String?? = nil, - settleSource: String?? = nil, - snoozedUntil: String?? = nil, - snoozedAt: String?? = nil, - wokeAt: String?? = nil, - wokeReason: String?? = nil - ) 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) - try? database.updateSessionLifecycle( - 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. - func rollback() { - guard let previous else { return } - func restored(_ requested: String??, _ value: String?) -> String?? { - requested == nil ? nil : .some(value) - } - try? database.updateSessionLifecycle( - 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), - wokeReason: restored(wokeReason, previous.wokeReason) - ) - } - + rollback: @escaping () -> Void + ) async throws -> String? { let scope = chatCommandScope(for: trimmed) let result: Any do { @@ -9363,15 +9449,129 @@ 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. Returns the queued + // operation's id so the caller can bind its optimistic state to it. + return syncQueuedCommandId(result) + } + + private func syncQueuedCommandId(_ result: Any) -> String? { + guard let record = result as? [String: Any], + record["ok"] == nil, + record["queued"] as? Bool == true + else { return nil } + 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 + /// 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) + let queuedCommandId = try await sendSessionLifecycleCommand( + sessionId: trimmed, + action: action, + args: args, + resultShape: resultShape, + rollback: { [weak self] in self?.clearPendingSessionSettle(trimmed, token: token) } + ) + // 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 — 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, + uptime: ProcessInfo.processInfo.systemUptime + ) + schedulePendingSessionSettleBackstopSweep() + } + + /// 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. 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 /// 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 @@ -9383,16 +9583,17 @@ final class SyncService: ObservableObject { if dismissPendingInput { args["dismissPendingInput"] = true } - try await sendSessionLifecycleCommand( + 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, - settledAt: .some(iso8601WithFractionalSecondsFormatter.string(from: Date())), - settleOverride: .some(nil), - settleSource: .some("user") + intent: .settle( + uptime: ProcessInfo.processInfo.systemUptime, + timestamp: iso8601WithFractionalSecondsFormatter.string(from: Date()) + ) ) } @@ -9401,15 +9602,12 @@ 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( + try await sendSessionSettleCommand( sessionId: sessionId, action: "session.unsettleSessions", args: ["sessionIds": [sessionId]], @@ -9417,22 +9615,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, - settledAt: .some(nil), - settleOverride: nil, - settleSource: .some(nil) + intent: .unsettle(uptime: ProcessInfo.processInfo.systemUptime) ) } /// 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"], - settleOverride: .some(override?.rawValue) + intent: .settleOverride(override?.rawValue, uptime: ProcessInfo.processInfo.systemUptime) ) } @@ -9457,7 +9653,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], @@ -9469,7 +9665,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], @@ -9482,7 +9678,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], @@ -16010,6 +16206,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) @@ -18541,6 +18749,21 @@ 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.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 // "Pending sync" snapshot so the synced row takes over. if operation.kind == "command", isQueuedChatCreationAction(operation.action) { @@ -18557,6 +18780,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) } @@ -18674,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 } @@ -18685,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 @@ -19245,6 +19477,16 @@ 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, and re-arm the sweep that enforces them. + pendingSessionSettleStates.holdBackstop(uptime: ProcessInfo.processInfo.systemUptime) + schedulePendingSessionSettleBackstopSweep() + if activeProjectId == nil { refreshProjectCatalog() } @@ -19643,7 +19885,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 } @@ -19652,7 +19897,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 @@ -19749,7 +19994,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, @@ -20870,7 +21121,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 = 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/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..6f5aa9a98 --- /dev/null +++ b/apps/ios/ADETests/PendingSessionSettleStatesTests.swift @@ -0,0 +1,608 @@ +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 { + /// 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", + 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(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. + let overlaid = states.apply(to: session(settleOverride: "active")) + XCTAssertEqual(overlaid.settledAt, "2026-08-10T12:00:00.000Z") + XCTAssertNil(overlaid.settleOverride) + } + + func testUnsettleLeavesAKeepActivePinAlone() { + var states = PendingSessionSettleStates() + 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. + 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(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")], uptime: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session()], uptime: now) + XCTAssertNil(states["session-1"]) + } + + func testSettleResolvesOnlyOnceTheHostAlsoClearedTheOverride() { + var states = PendingSessionSettleStates() + 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")], uptime: now) + XCTAssertNotNil(states["session-1"]) + + 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(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")], uptime: now) + + XCTAssertNil(states["session-1"]) + } + + func testIntentSurvivesUntilTheHostRowActuallyChanges() { + var states = PendingSessionSettleStates() + states.begin(.settle(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + states.prune(against: [session(settledAt: nil)], uptime: 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(uptime: now), for: "session-1", baseline: nil) + + states.prune(against: [session(settledAt: "2026-08-10T09:00:00.000Z")], uptime: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session(settledAt: nil)], uptime: now) + XCTAssertNil(states["session-1"]) + } + + func testOverrideIntentComparesTheExactValueWeAskedFor() { + var states = PendingSessionSettleStates() + 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")], uptime: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [session(settleOverride: "active")], uptime: now) + XCTAssertNil(states["session-1"]) + } + + func testClearingAnOverrideResolvesOnNull() { + var states = PendingSessionSettleStates() + 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)], uptime: now) + XCTAssertNil(states["session-1"]) + } + + func testAFailedCommandDropsTheIntentSoTheRowSnapsBack() { + var states = PendingSessionSettleStates() + 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) + + 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", 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) + + XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") + } + + func testAnIntentWhoseChangesetNeverArrivesExpires() { + var states = PendingSessionSettleStates() + 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) + XCTAssertNotNil(states["session-1"]) + + 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") + } + + /// 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() + 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 { + 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") + XCTAssertEqual(states.apply(to: session()).settledAt, "2026-08-10T12:00:00.000Z") + } + + func testTheBackstopResumesOnceTheHostIsReachableAgain() { + var states = PendingSessionSettleStates() + 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. + let reconnectedAt = addUptime(now, 600) + states.holdBackstop(uptime: reconnectedAt) + states.prune(against: [session(settledAt: nil)], uptime: reconnectedAt) + XCTAssertNotNil(states["session-1"]) + + 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(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(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(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) + } + + /// 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(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), 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. + 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], uptime: now) + XCTAssertNotNil(states["session-1"]) + XCTAssertNil(states.apply(to: settledByFirstCommand).settledAt) + + // 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], uptime: now) + XCTAssertNotNil(states["session-1"]) + + states.prune(against: [unsettled], uptime: addUptime(now, 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(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")], uptime: 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 + /// the pin and offer the wrong actions until replication caught up. + func testUnsettleAfterAnUnlandedSettleDoesNotResurrectAKeepActivePin() { + var states = PendingSessionSettleStates() + let pinned = session(settleOverride: "active") + 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) + 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(uptime: now), for: "session-1", baseline: pinned) + + 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"]) + } + + /// 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") + } + + /// 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() + 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(forOperation: "op-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"]) + } + + /// 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) + 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)) + 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) + + 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(uptime: now, timestamp: "2026-08-10T12:00:00.000Z"), for: "session-1", baseline: nil) + + states.removeAll() + + XCTAssertTrue(states.isEmpty) + XCTAssertNil(states.apply(to: session()).settledAt) + } + + func testASessionMissingFromAScopedReadKeepsItsIntent() { + var states = PendingSessionSettleStates() + 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)], uptime: now) + + XCTAssertNotNil(states["session-1"]) + } + + func testOverlayOnlyTouchesTheSessionItWasBegunFor() { + var states = PendingSessionSettleStates() + 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")]) + + XCTAssertEqual(others[0].settledAt, "2026-08-10T12:00:00.000Z") + 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(uptime: ProcessInfo.processInfo.systemUptime, 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(uptime: ProcessInfo.processInfo.systemUptime, 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(uptime: ProcessInfo.processInfo.systemUptime, 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..aaed1f32a 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -377,6 +377,55 @@ 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. +- **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 +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 — 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. + ## Architecture layers ``` @@ -1110,7 +1159,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 +2926,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 090ec12d8..85074a8b0 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,29 @@ 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 + 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. 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 + [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: @@ -2067,7 +2098,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 83aa97720..4aa0d3184 100644 --- a/docs/features/terminals-and-sessions/README.md +++ b/docs/features/terminals-and-sessions/README.md @@ -213,7 +213,11 @@ 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 + 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 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` @@ -1388,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 @@ -1848,6 +1864,19 @@ 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. 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 diff --git a/docs/features/terminals-and-sessions/settle-teardown-design.md b/docs/features/terminals-and-sessions/settle-teardown-design.md index 5a0caae9b..0f2cb1401 100644 --- a/docs/features/terminals-and-sessions/settle-teardown-design.md +++ b/docs/features/terminals-and-sessions/settle-teardown-design.md @@ -4,6 +4,9 @@ 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 "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 — burning tokens and holding ports behind a row that has left every live-work @@ -189,39 +192,84 @@ 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. +**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 +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 +(`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 +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 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. + +**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 **The revision must be local-only.** `terminal_sessions` is CRR, and C4 writes @@ -324,10 +372,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