From ee11edee8a9cdda66090dcd6cefa289012e494e3 Mon Sep 17 00:00:00 2001 From: Dennison Date: Mon, 3 Aug 2026 18:37:41 +0200 Subject: [PATCH 1/2] fix(macapp): bind external runs to controls --- docs/logs/engineering-log.md | 16 ++ docs/logs/long-term-thinking-log.md | 14 + docs/logs/observational-log.md | 11 + docs/logs/system-log.md | 9 + ...e-1007-external-run-controls-impact-map.md | 69 +++++ ...3-issue-1007-external-run-controls-plan.md | 67 +++++ docs/plans/INDEX.md | 5 + docs/plans/active-plan.md | 8 + macapp/Sources/GoCodeUI/ChatView.swift | 13 +- .../GoCodeUI/RunSession+RunControls.swift | 7 +- macapp/Sources/GoCodeUI/RunSession.swift | 244 ++++++++++++++++-- macapp/Sources/HarnessKit/Transcript.swift | 10 + .../RunSessionExternalControlTests.swift | 234 +++++++++++++++++ 13 files changed, 687 insertions(+), 20 deletions(-) create mode 100644 docs/plans/2026-08-03-issue-1007-external-run-controls-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1007-external-run-controls-plan.md create mode 100644 macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index af10fecc..63b93fa9 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -57,6 +57,22 @@ normal/race passed in 13.200s/14.719s. Isolated foreground `./scripts/test-regression.sh` then passed normal/race plus 85.5% total coverage and zero uncovered functions in 2m26s. +## 2026-08-03 (Issue #1007 External Scheduled-Run Controls Rebase) + +- System/component: `RunSession` control-owner reducer, accounting fence, + request generations, `RunSession+RunControls`, and `InlineRunStatus`. +- Ownership/order: scoped conversation events select control ownership before + accounting. A selected terminal is rendered before a live fallback resumes; + a foreign terminal is tombstoned without changing the selected lifecycle or + accounting. Selection changes invalidate outstanding answer/input/control + requests. +- Visibility/safety: the scheduled-run accessibility/status text is composed + alongside—not hidden by—project status. A second Stop cancels a per-run + stream only when that stream belongs to the selected run. +- Verification: focused `RunSessionExternalControlTests` (6), full macapp + (217 tests/43 suites), and `./scripts/test-regression.sh` normal/race/ + coverage pass from the exact rebased tree (85.5% total, zero uncovered + functions). Hosted CI and independent review remain promotion gates. ## 2026-08-03 (Issue #1120 blocked-heartbeat fixture) diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 1fa04ebf..e690de30 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -38,6 +38,20 @@ later same-run admission; normal/race/package/full gates are green. - Guardrails: no product source, API, SQLite schema, config, client, UI, or lifecycle change; no sleep/timeout increase or synthetic production defect. +## 2026-08-03 (Issue #1007 External Scheduled-Run Control Rebase) + +- Command intent: make scheduled callback/cron continuations visibly active + and safely actionable in native chat without regressing main's accounting or + acknowledged control behavior. +- User intent: a deployment-watch continuation must carry its conversation + forward in the GUI, including visible status and correct Stop/approval/input/ + steering targeting—not merely fire on the harness. +- Success definition: selected external runs visibly resume activity; all + actions target only that run; stale/replayed/foreign events cannot resurrect + or displace it; full macapp/repository and later live acceptance evidence + pass before merge/proof claims. +- Guardrails: no server protocol/persistence changes, no status fallback that + hides scheduled state, and no account mutation before control authority. ## 2026-08-03 (Issue #1120 blocked heartbeat fixture) diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 45be4ba4..1a1ee68a 100644 --- a/docs/logs/observational-log.md +++ b/docs/logs/observational-log.md @@ -31,6 +31,17 @@ - The pre-deadline checkpoint includes retry state, exact due time, reserved run ID, attempt one, and empty token/lease; checking all of them prevents a no-call assertion from masking an accidental claim or fence leak. +## 2026-08-03 (Issue #1007 Rebase Observation) + +- The original #1007 action fixture attempted approve, deny, answer, steer, + and cancel concurrently. Main's #994 contract intentionally permits exactly + one acknowledged control request; the repaired test now proves the competing + calls are rejected until the same run emits a decision lifecycle frame. +- Focused native evidence covers external action routing, stale conversation + rejection, foreign-terminal shielding, terminal tombstones, first active + evidence, and timestamp ordering. Full macapp (217 tests/43 suites) and + repository normal/race/coverage gates now pass; this remains distinct from + the later live installed-app acceptance proof. ## 2026-08-03 (Issue #1120 heartbeat ordering observation) diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index 76ab9a1a..3d26bfa7 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -38,6 +38,15 @@ complete tools normal/race (13.200s/14.719s) pass on the final source tree; isolated repository regression also passes normal/race, 85.5% coverage, and zero uncovered functions. +## 2026-08-03 (Issue #1007 Native External-Run Reducer) + +- `RunSession` now has separate in-memory active/control ownership from + transcript accounting. It rejects prior-conversation frames before mutation, + preserves local provisional ownership until timestamped evidence, and keeps + terminal run IDs as replay tombstones for the selected conversation. +- No harness API, persistence, scheduler, callback, cron, TUI, configuration, + or deployment surface changes in this slice. Full local combined verification + passes; hosted CI/review remain required before promotion. ## 2026-08-03 (Issue #1120 fixture contract) diff --git a/docs/plans/2026-08-03-issue-1007-external-run-controls-impact-map.md b/docs/plans/2026-08-03-issue-1007-external-run-controls-impact-map.md new file mode 100644 index 00000000..3951cf85 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1007-external-run-controls-impact-map.md @@ -0,0 +1,69 @@ +# Impact Map: #1007 external scheduled-run controls + +## Task + +- Task / issue: #1007, child of #1000. +- Plan link: `2026-08-03-issue-1007-external-run-controls-plan.md`. +- Owner: macOS client. +- Status: rebased implementation; final combined verification pending. + +## Current Ownership, Callers, and Data Flow + +- Entry points: `RunSession.load/rebind` start `conversationEvents`; `submit` + starts a per-run stream; ChatView binds `isBusy`, pending approval/question, + and `approve/deny/answer/steer/cancel` to RunSession. +- Source of truth: `RunSession.currentRunID`; `Transcript` owns visual + lifecycle reduction but is not a per-run control authority. +- Consumers: `InlineRunStatus`, `ApprovalBar`, `AskUserView`, and composer + steering all derive state/actions from RunSession. +- Search evidence: `rg -n --glob '*.swift' 'currentRunID|apply.*Event|run\\.started|approval|steer|cancel|SubmitInput' macapp`; direct inspection of + `RunSession.swift`, `ChatView.swift`, `HarnessClient.swift`, and existing + `RunSessionConversationStreamTests.swift`. +- Conclusion: RunSession is the sole correct binding boundary; adding a second + active-run owner in Transcript or ChatView would race the two SSE paths. + +## Config, API, CLI, and Tools + +- User-facing config/defaults/environment: none. +- API/wire: existing conversation SSE and run-control endpoints only; no server + fields/routes change. CLI/tools: none. +- Errors: action failures retain existing connection-error behavior. + +## Persistence and Compatibility + +- Schemas/migrations/cache: none. The in-memory active-run selection resets on + conversation switch/reset. +- Compatibility: old harness events remain valid; self-submitted run response + still establishes the initial identity. Replayed event IDs remain deduped. +- Mixed rollout: newer client safely ignores stale previous-conversation events; + no server/client version dependency. + +## Lifecycle, Concurrency, Security, and Privacy + +- Lifecycle: control ownership is selected before accounting. A first external + active event can bind an empty session and resume visible activity; a + timestamp-less local start remains provisional; terminal tombstones block + late replay resurrection. A selected terminal renders before a fallback run + is resumed, while a foreign terminal cannot change the selected lifecycle. +- Concurrency: main-actor RunSession serializes the two SSE feeds. A stream + delivery must be scoped to the currently selected conversation before it can + mutate transcript, questions, or controls. +- Security/privacy: targeting remains an authenticated existing endpoint; + client never infers cross-conversation authority from a foreign stream. + +## Product Clients, Catalogs, Deployment, Compatibility + +- Product clients: native macOS UI affected; API/TUI/web are unchanged. +- Provider/model/tool catalogs: none. +- Deployment/observability: no service/config change; status copy exposes a + scheduled run as active rather than silently losing control. +- Rollback: revert the RunSession identity reducer; no durable state is left. + +## Tests and Documentation + +- Tests: URLProtocol-backed tests cover single-flight routing for all five + action endpoints, foreign conversation rejection, selected and foreign + terminal ordering, replay tombstones, first active evidence, and timestamp + ordering. Final full macapp/repository gates remain pending. +- Docs: plan/impact plus plans index, active plan, engineering, observational, + system, and long-term-thinking logs. diff --git a/docs/plans/2026-08-03-issue-1007-external-run-controls-plan.md b/docs/plans/2026-08-03-issue-1007-external-run-controls-plan.md new file mode 100644 index 00000000..8a1ef618 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1007-external-run-controls-plan.md @@ -0,0 +1,67 @@ +# Plan: External scheduled-run controls in macOS chat + +## Context + +- Governing GitHub issue: #1007 (child of #1000; coordinates with merged #994). +- Problem: `RunSession` renders cron/callback conversation SSE but only assigns + `currentRunID` after its own `startRun` response. External runs therefore + leave approval, input, steer, and cancel controls without a safe target; a + terminal event for an older run can also make a newer run appear inactive. +- User impact: an operator watching a deployment cannot safely take the next + conversational action when a scheduled continuation asks for approval/input. +- Constraints: Swift-only state/UI slice; no harness protocol, persistence, or + scheduler change; preserve self-submitted stream/replay behavior. + +## Scope + +- In scope: centralized RunSession active-run identity reduction from scoped + conversation SSE; correct terminal/replay behavior; action targeting; + accessible scheduled-run status; Swift regression coverage and required docs. +- Out of scope: cron/callback durability (#1005/#1006), SSE durability (#1008), + task-lifecycle/activity UI (#1009), and backend/API changes. + +## Documentation Contract + +- Feature status: in implementation. +- Public docs affected: none; this is an internal macOS state contract. +- Spec docs updated before code: this plan and its impact map. +- Implementation notes after code: engineering, observational, system logs. + +## Test Plan (TDD) + +- Add a stubbed conversation-SSE test whose external `run.started` followed by + approval/input events is actionable; run it red before changing RunSession. +- Cover the endpoints/run IDs for approve, deny, input, steer, and cancel; + terminal A while B is selected; duplicate/replayed A after B; and stale + events arriving from a conversation no longer selected. +- Preserve existing self-submitted dedupe coverage. Run focused RunSession + tests, all macapp tests, then `./scripts/test-regression.sh`. + +## Cross-Surface Impact Map + +See `2026-08-03-issue-1007-external-run-controls-impact-map.md`. + +## Implementation Checklist + +- [x] Verify structured issue #1007 and its #994 dependency. +- [x] Record architecture and impact map before code. +- [x] Add and capture the meaningful failing Swift regression test. +- [x] Centralize scoped active-run identity reduction in RunSession. +- [x] Add accessible scheduled-run state copy. +- [x] Run the focused external-control regression suite. +- [x] Run full macapp and repository regression gates from the final rebased tree. +- [x] Update logs/indexes with final exact evidence; draft PR remains unmerged. + +## Risks and Mitigations + +- Concurrent/replayed streams could retarget controls: order control ownership + before accounting; retain terminal tombstones; preserve a timestamp-less + local provisional owner; and apply a selected terminal before resuming a + still-active fallback run. +- A cancelled old conversation stream can still yield: bind stream deliveries + to the currently selected conversation before any transcript/control update. +- A self-started run is delivered twice: keep event-ID dedupe and preserve the + response-derived current ID. +- macOS Keychain integration blocks under a detached tmux launch context: + record that attempt, but use the logged-in foreground regression gate as the + authoritative repository check. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index b9ce2568..42d79771 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -30,6 +30,11 @@ deterministic callback retry-wait recovery fixture plan. - `2026-08-03-issue-1124-retry-wait-fixture-impact-map.md` — Cross-surface impact map for the Issue #1124 test-only repair. +- `2026-08-03-issue-1007-external-run-controls-plan.md` — Issue #1007 macOS + external cron/callback control-ownership rebase repair. +- `2026-08-03-issue-1007-external-run-controls-impact-map.md` — Cross-surface + impact map for #1007 external scheduled-run visibility and controls. + - `2026-08-03-issue-1120-heartbeat-fixture-plan.md` — Issue #1120 test-only blocked-heartbeat callback fixture, stacked on #1119. - `2026-08-03-issue-1120-heartbeat-fixture-impact-map.md` — Cross-surface diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index 0113fee8..e34024e3 100644 --- a/docs/plans/active-plan.md +++ b/docs/plans/active-plan.md @@ -29,6 +29,14 @@ early fire must not admit or mutate it, then an explicit post-deadline fire must reuse the reserved run identity exactly once. Callback manager, SQLite, API/task visibility, TUI, and native GUI behaviour remain unchanged. Final focused/package/full validation, review, hosted checks, and promotion remain. +Current status: Issue #1007 rebases external scheduled cron/callback control +ownership onto main's accounting and request-generation reducer. The combined +reducer selects control authority before accounting, retains terminal +tombstones, invalidates displaced asynchronous action ownership, and resumes a +fallback only after rendering the selected terminal. Focused Swift (6), full +macapp (217 tests/43 suites), and complete normal/race/coverage regression +(85.5%, zero uncovered functions) are green. Independent review, hosted CI, +and merge reconciliation remain pending. Current status: Issue #1120 is a test-and-documentation-only blocked-heartbeat fixture repair merged through #1121 and #1119 into the current #1106 stack. It preserves production callback semantics diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index b994afab..16b5b28d 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -104,7 +104,10 @@ struct TranscriptView: View { row(for: item).id(item.id) } if run.isBusy { - InlineRunStatus(run: run, statusMessage: statusMessage) + InlineRunStatus( + run: run, + statusMessage: statusMessage, + scheduledRunStatus: run.scheduledRunStatus) } Color.clear.frame(height: Spacing.hairline).id(bottomAnchor) } @@ -694,6 +697,7 @@ struct NoticeRow: View { struct InlineRunStatus: View { @Bindable var run: RunSession let statusMessage: String? + let scheduledRunStatus: String? @Environment(\.accessibilityReduceMotion) private var reduceMotion var body: some View { @@ -711,6 +715,12 @@ struct InlineRunStatus: View { .foregroundStyle(Theme.foregroundQuaternary) .lineLimit(1) } + if let scheduledRunStatus { + Text("· \(scheduledRunStatus)") + .foregroundStyle(Theme.foregroundQuaternary) + .lineLimit(1) + .accessibilityLabel(scheduledRunStatus) + } Spacer() if run.isBusy { Button(run.cancelInFlight ? "Stopping…" : "Stop") { run.cancel() } @@ -725,6 +735,7 @@ struct InlineRunStatus: View { notification: .announcementRequested, userInfo: [.announcement: error, .priority: NSAccessibilityPriorityLevel.high]) } + .accessibilityElement(children: .combine) } private var label: String { diff --git a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift index 19404e3e..35cba921 100644 --- a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift +++ b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift @@ -14,9 +14,12 @@ extension RunSession { } switch cancelState { case .requested: - currentRunID = nil - streamTask?.cancel() + // The selected continuation may be external while a distinct local + // per-run stream is still draining. Never cancel that unrelated + // stream on an external force-stop. + if localStreamRunID == runID { streamTask?.cancel() } transcript.markCancelled() + retire(runID: runID) cancelState = .idle case .requesting: return diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index 3bbe40da..e5178d53 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -67,6 +67,19 @@ public final class RunSession { /// from treating another run's terminal event as the current run. private(set) var accountingRunID: String? private var accountingTimestamp: Date? + /// Active runs in activation order. A conversation stream may legitimately + /// contain overlapping local and scheduled work, but the native transcript + /// has one visual lifecycle and one action target. + private var activeRunIDs: [String] = [] + private var externalRunIDs: Set = [] + private var activeRunTimestamps: [String: Date?] = [:] + /// A terminal run may be replayed after a reconnect. Do not let a late + /// non-terminal frame resurrect it as the action target. + private var terminalRunIDs: Set = [] + /// Only the per-run stream for this local run may be force-cancelled by a + /// second Stop press. A scheduled run must never cancel an unrelated local + /// stream merely because it is currently selected. + var localStreamRunID: String? public init(client: HarnessClient) { self.client = client @@ -84,6 +97,12 @@ public final class RunSession { public var canSubmit: Bool { !draft.trimmed.isEmpty && !isBusy && !runControlInFlight } /// True while a run is active, so the composer can offer steering instead. public var canSteer: Bool { isBusy && transcript.pendingApproval == nil } + /// Accessible copy shown while a cron/callback continuation, rather than a + /// prompt submitted by this app instance, owns the active controls. + public var scheduledRunStatus: String? { + guard let currentRunID, externalRunIDs.contains(currentRunID), isBusy else { return nil } + return "Scheduled run active" + } // MARK: - Running @@ -125,7 +144,8 @@ public final class RunSession { let started = try await client.startRun(request) startedRunID = started.runID - currentRunID = started.runID + localStreamRunID = started.runID + activate(runID: started.runID, isExternal: false, timestamp: nil, select: true) activateAccounting(for: started.runID, timestamp: nil) if self.conversationID == nil { self.conversationID = started.runID } // Keyed by conversation, not by this run: on a conversation's @@ -136,7 +156,7 @@ public final class RunSession { } for try await event in client.events(runID: started.runID) { - await apply(event, runID: started.runID) + await applyRunEvent(event, expectedRunID: started.runID) } } catch let error as HarnessError { connectionError = error.message @@ -147,7 +167,7 @@ public final class RunSession { transcript.markFailed() if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } - currentRunID = nil + finishRunIfCurrent(startedRunID: startedRunID) } } @@ -160,7 +180,7 @@ public final class RunSession { accountingRunID = nil accountingTimestamp = nil self.conversationID = conversationID - currentRunID = nil + clearActiveRuns() connectionError = nil trackConversationStream(conversationID) } @@ -199,7 +219,7 @@ public final class RunSession { accountingRunID = nil accountingTimestamp = nil conversationID = nil - currentRunID = nil + clearActiveRuns() connectionError = nil pendingQuestions = nil cancelState = .idle @@ -249,7 +269,7 @@ public final class RunSession { conversationID: conversationID, lastEventID: lastEventID) { lastEventID = event.id - _ = await apply(event, runID: event.runID) + await applyConversationEvent(event, conversationID: conversationID) // A fresh app can open a durable message snapshot and then // receive the same completed run in the conversation // replay. Reconcile at each terminal boundary so replay @@ -283,24 +303,195 @@ public final class RunSession { /// stream started by `submit()` and the conversation-wide stream deliver /// the same events for a run this app started, and rendering both copies /// would double every message (issue #950 requirement 4). + /// + /// Control ownership is deliberately decided before accounting. A foreign + /// terminal can be newer in wall-clock time while a different run remains + /// selected; it must not steal accounting or clear the visible controls. @discardableResult func apply(_ event: HarnessEvent, runID: String) async -> Bool { - guard seenEventIDs.insert(event.id).inserted else { return false } - let includedAccounting = admitAccounting(for: event) - // Conversation replay can deliver any lifecycle frame for an older - // run after a newer run has taken ownership. Its durable rows still - // need terminal reconciliation, but an old `run.started`, approval, - // waiting, or terminal frame must not mutate the newer run's visible - // state or make the reconciliation busy-guard reject those rows. - if suppressesForeignStateMutation(event, includedAccounting: includedAccounting) { + guard !event.runID.isEmpty, seenEventIDs.insert(event.id).inserted else { return false } + + let wasSelected = currentRunID == event.runID + let hadSelection = currentRunID != nil + if event.type.isTerminal { + // A terminal for another active/replayed run is authoritative for + // that run alone. Remember it so late replay frames cannot revive + // it, but do not let it mutate the selected run's visual state. + guard wasSelected || !hadSelection else { + retire(runID: event.runID) + return false + } + let includedAccounting = admitAccounting(for: event) + // A terminal-only replay is authoritative only when it can own + // accounting. Once a newer terminal/local run owns accounting, + // an old terminal is still retained for durable reconciliation but + // cannot replace that newer visual terminal state. + if !includedAccounting, let accountingRunID, accountingRunID != event.runID { + retire(runID: event.runID, terminalTimestamp: event.timestamp) + return false + } + transcript.apply(event, includingAccounting: includedAccounting) + advanceAcknowledgedRunControl(for: event) + retire(runID: event.runID, terminalTimestamp: event.timestamp) + // Terminal must render before restoring the fallback run's active + // state. This preserves the durable terminal row while returning + // Stop/steer/input authority to the still-live continuation. + if let fallback = currentRunID { + activateAccounting(for: fallback, timestamp: activeRunTimestamps[fallback] ?? nil) + transcript.resumeActiveRun() + } + return includedAccounting + } + + let admitsControlEvidence = registerActiveEvidence(for: event) + let appliesToSelectedRun = currentRunID == event.runID + let includedAccounting = appliesToSelectedRun && admitAccounting(for: event) + if (!admitsControlEvidence && eventChangesLifecycleOrControls(event)) + || suppressesForeignStateMutation(event, includedAccounting: includedAccounting) + { return false } + if appliesToSelectedRun, transcript.runState.isTerminalState { + // Reconnects can start at an assistant/tool frame after the + // `run.started` cursor was trimmed. First active external evidence + // must visibly continue the conversation, not merely set an ID. + transcript.resumeActiveRun() + } transcript.apply(event, includingAccounting: includedAccounting) - advanceAcknowledgedRunControl(for: event) - await handleSideEffects(of: event, runID: runID) + if appliesToSelectedRun { advanceAcknowledgedRunControl(for: event) } + await handleSideEffects(of: event, runID: runID, appliesToSelectedRun: appliesToSelectedRun) return includedAccounting } + /// The conversation entry point is internal for stale-stream acceptance + /// tests. A cancelled previous conversation stream can still yield once. + func applyConversationEvent(_ event: HarnessEvent, conversationID: String) async { + guard self.conversationID == conversationID else { return } + _ = await apply(event, runID: event.runID) + } + + private func applyRunEvent(_ event: HarnessEvent, expectedRunID: String) async { + guard event.runID == expectedRunID else { return } + _ = await apply(event, runID: expectedRunID) + } + + private func registerActiveEvidence(for event: HarnessEvent) -> Bool { + guard !terminalRunIDs.contains(event.runID) else { return false } + let isLifecycleStart = + event.type == .runQueued || event.type == .runStarted || event.type == .runResumed + let select: Bool + if currentRunID == nil { + // After a terminal leaves no live action target, a replay start + // still must not displace newer accounting. Timestamp-less legacy + // frames remain admissible because their order cannot be compared. + if let accountingTimestamp, let timestamp = event.timestamp { + select = timestamp >= accountingTimestamp + } else { + select = true + } + } else if isLifecycleStart, let currentRunID { + // A local `startRun` result is a provisional owner until its first + // timestamped event. Do not replace it with an older replay simply + // because the replay supplies a timestamp first. + if activeRunTimestamps[currentRunID] == nil, !externalRunIDs.contains(currentRunID) { + select = false + } else if let timestamp = event.timestamp, + let current = activeRunTimestamps[currentRunID] ?? nil + { + select = timestamp >= current + } else { + select = false + } + } else { + select = false + } + if !activeRunIDs.contains(event.runID) { + activate( + runID: event.runID, isExternal: event.runID != localStreamRunID, + timestamp: event.timestamp, select: select) + } else if select { + selectActive(runID: event.runID) + } + return true + } + + private func eventChangesLifecycleOrControls(_ event: HarnessEvent) -> Bool { + switch event.type { + case .runQueued, .runStarted, .runResumed, + .toolApprovalRequired, .planApprovalRequired, + .toolApprovalGranted, .toolApprovalDenied, + .planApprovalGranted, .planApprovalDenied, + .runWaitingForUser: + return true + default: + return false + } + } + + private func activate(runID: String, isExternal: Bool, timestamp: Date?, select: Bool) { + activeRunIDs.removeAll { $0 == runID } + activeRunIDs.append(runID) + activeRunTimestamps[runID] = timestamp + if isExternal { externalRunIDs.insert(runID) } else { externalRunIDs.remove(runID) } + if select { selectActive(runID: runID) } + } + + private func selectActive(runID: String) { + guard currentRunID != runID else { return } + invalidateRequestOwnership() + currentRunID = runID + cancelState = .idle + } + + func retire(runID: String, terminalTimestamp: Date? = nil) { + terminalRunIDs.insert(runID) + activeRunIDs.removeAll { $0 == runID } + externalRunIDs.remove(runID) + activeRunTimestamps.removeValue(forKey: runID) + guard currentRunID == runID else { return } + // An older run may still have an active row, but it is not a valid + // fallback after a newer terminal. Only a run activated no earlier + // than this terminal may resume the visible lifecycle. + let fallback = activeRunIDs.last.flatMap { candidate -> String? in + guard let terminalTimestamp else { return candidate } + guard let candidateTimestamp = activeRunTimestamps[candidate] ?? nil, + candidateTimestamp >= terminalTimestamp + else { return nil } + return candidate + } + currentRunID = fallback + // A terminal of the run that owns a still-pending control POST must + // not invalidate that request. Its completion decides whether the + // composer is released or an error/draft is restored. Switching to a + // real fallback does invalidate displaced A ownership. + if fallback != nil { invalidateRequestOwnership() } + cancelState = .idle + } + + private func finishRunIfCurrent(startedRunID: String?) { + guard let startedRunID else { return } + if localStreamRunID == startedRunID { localStreamRunID = nil } + // A stream can end after an external continuation became selected. It + // may retire its own run, never clear the newer selected target. + if currentRunID == startedRunID { + retire(runID: startedRunID) + } else { + activeRunIDs.removeAll { $0 == startedRunID } + externalRunIDs.remove(startedRunID) + activeRunTimestamps.removeValue(forKey: startedRunID) + } + } + + private func clearActiveRuns() { + activeRunIDs = [] + externalRunIDs = [] + activeRunTimestamps = [:] + terminalRunIDs = [] + currentRunID = nil + localStreamRunID = nil + cancelState = .idle + } + /// Accounting admission is the run-ownership fence. Content events from a /// foreign replay remain useful transcript history, but only the owner may /// change lifecycle or approval state. This deliberately covers more than @@ -368,6 +559,13 @@ public final class RunSession { if let timestamp = event.timestamp { accountingTimestamp = timestamp } return true } + // The control reducer selected this lifecycle start first. That is the + // ordering authority for timestamp-less compatibility frames; the + // comparison below remains for terminal-only replay. + if isStart, currentRunID == runID { + activateAccounting(for: runID, timestamp: event.timestamp) + return true + } let isNewer: Bool if let timestamp = event.timestamp { // `submit()` owns the run before its first SSE frame supplies a @@ -391,10 +589,13 @@ public final class RunSession { // MARK: - Side effects - private func handleSideEffects(of event: HarnessEvent, runID: String) async { + private func handleSideEffects( + of event: HarnessEvent, runID: String, appliesToSelectedRun: Bool + ) async { // The question text lives behind a separate endpoint, not in the event. guard event.type == .runWaitingForUser || event.type == .other("run.waiting_for_user") else { return } + guard appliesToSelectedRun, currentRunID == runID else { return } pendingInputRequestGeneration &+= 1 let generation = pendingInputRequestGeneration do { @@ -439,6 +640,15 @@ extension String { var trimmed: String { trimmingCharacters(in: .whitespacesAndNewlines) } } +extension RunState { + fileprivate var isTerminalState: Bool { + switch self { + case .completed, .failed, .cancelled: true + default: false + } + } +} + extension Transcript { /// Marks the run failed when the transport dies before a terminal event /// arrives — otherwise the spinner would never stop. diff --git a/macapp/Sources/HarnessKit/Transcript.swift b/macapp/Sources/HarnessKit/Transcript.swift index 03dd80b1..cac60128 100644 --- a/macapp/Sources/HarnessKit/Transcript.swift +++ b/macapp/Sources/HarnessKit/Transcript.swift @@ -447,6 +447,16 @@ extension Transcript { } } + /// Keeps the single visual lifecycle active when a different concurrent + /// run remains selected after another run's terminal event. The session + /// owns which run that is; Transcript only clears interaction state that + /// belonged to the terminated visual run. + public mutating func resumeActiveRun() { + pendingApproval = nil + pendingPlan = nil + runState = .running + } + /// Clears everything for a new conversation. public mutating func reset() { self = Transcript() diff --git a/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift b/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift new file mode 100644 index 00000000..4fdb8959 --- /dev/null +++ b/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift @@ -0,0 +1,234 @@ +import Foundation +import HarnessKit +import Testing + +@testable import GoCodeUI + +private final class ExternalRunControlStub: URLProtocol, @unchecked Sendable { + nonisolated(unsafe) private static var recorded: [URLRequest] = [] + private static let lock = NSLock() + + static func reset() { lock.withLock { recorded = [] } } + static var requests: [URLRequest] { lock.withLock { recorded } } + + override class func canInit(with request: URLRequest) -> Bool { true } + override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + + override func startLoading() { + let request = self.request + Self.lock.withLock { Self.recorded.append(request) } + let path = request.url?.path ?? "" + let body: Data + if path.hasSuffix("/input") && request.httpMethod == "GET" { + body = Data( + #"{"run_id":"run_b","call_id":"call_b","questions":[{"question":"Continue?","options":[]}]}"# + .utf8) + } else { + body = Data("{}".utf8) + } + let response = HTTPURLResponse( + url: request.url!, statusCode: 200, httpVersion: "HTTP/1.1", + headerFields: ["Content-Type": "application/json"])! + client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: body) + client?.urlProtocolDidFinishLoading(self) + } + + override func stopLoading() {} +} + +@Suite("RunSession external scheduled-run controls", .serialized) +@MainActor +struct RunSessionExternalControlTests { + private func makeSession() -> RunSession { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [ExternalRunControlStub.self] + return RunSession( + client: HarnessClient( + baseURL: URL(string: "http://127.0.0.1:8898")!, + session: URLSession(configuration: configuration))) + } + + private func event( + _ id: String, _ runID: String, _ type: String, timestamp: String? = nil + ) throws -> HarnessEvent { + let timestampField = timestamp.map { ",\"timestamp\":\"\($0)\"" } ?? "" + return try HarnessEvent( + frame: SSEFrame( + id: id, event: type, + data: + #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)"\#(timestampField),"payload":{}}"# + )) + } + + private func wait( + timeout: Duration = .seconds(2), for condition: () -> Bool + ) async throws { + let deadline = ContinuousClock.now.advanced(by: timeout) + while ContinuousClock.now < deadline { + if condition() { return } + try await Task.sleep(for: .milliseconds(10)) + } + Issue.record("timed out waiting for condition") + } + + /// The external run must become the single action target. A terminal for + /// the older run and a replayed old non-terminal event cannot steal or + /// clear that identity while the newer scheduled run is live. + @Test("external scheduled run binds every action to its own run ID") + func externalScheduledRunBindsActions() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_a") + + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started"), conversationID: "conversation_a") + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started"), conversationID: "conversation_a") + try await session.applyConversationEvent( + event("run_a:1", "run_a", "assistant.message"), conversationID: "conversation_a") + try await session.applyConversationEvent( + event("run_a:2", "run_a", "run.completed"), conversationID: "conversation_a") + + #expect(session.currentRunID == "run_b") + #expect(session.isBusy, "run_a terminal must not make live run_b inactive") + #expect(session.scheduledRunStatus == "Scheduled run active") + + try await session.applyConversationEvent( + event("run_b:1", "run_b", "tool.approval_required"), + conversationID: "conversation_a") + try await session.applyConversationEvent( + event("run_b:2", "run_b", "run.waiting_for_user"), conversationID: "conversation_a") + #expect(session.currentRunID == "run_b") + try await wait { session.pendingQuestions?.runID == "run_b" } + + session.approve() + session.deny() + session.draft = "continue watching" + session.steer() + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/approve" } + } + #expect(session.runControlInFlight) + #expect( + !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/deny") == true }) + #expect( + !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/steer") == true }) + try await session.applyConversationEvent( + event("run_b:3", "run_b", "tool.approval_granted"), conversationID: "conversation_a") + try await wait { !session.runControlInFlight } + + session.deny() + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/deny" } + } + try await session.applyConversationEvent( + event("run_b:4", "run_b", "tool.approval_denied"), conversationID: "conversation_a") + try await wait { !session.runControlInFlight } + + session.answer(["0:Continue?": "yes"]) + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/input" } + } + session.draft = "continue watching" + session.steer() + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/steer" } + } + session.cancel() + + try await wait { + let paths = Set(ExternalRunControlStub.requests.compactMap(\.url?.path)) + return [ + "/v1/runs/run_b/approve", "/v1/runs/run_b/deny", "/v1/runs/run_b/input", + "/v1/runs/run_b/steer", "/v1/runs/run_b/cancel", + ].allSatisfy(paths.contains) + } + session.reset() + } + + /// A cancelled old stream can finish delivering after the user selects a + /// different conversation. It must have no transcript or control effect. + @Test("foreign conversation event is ignored") + func foreignConversationEventIsIgnored() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_b") + + try await session.applyConversationEvent( + event("run_old:0", "run_old", "run.started"), conversationID: "conversation_a") + + #expect(session.currentRunID == nil) + #expect(session.transcript.runState == .completed) + session.reset() + } + + @Test("each selected terminal event clears only that run") + func selectedTerminalEventsClearTheirRun() async throws { + let expectations: [(String, RunState)] = [ + ("run.completed", .completed), + ("run.failed", .failed), + ("run.cancelled", .cancelled), + ] + for (type, expectedState) in expectations { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_terminal") + try await session.applyConversationEvent( + event("run_terminal:0", "run_terminal", "run.started"), + conversationID: "conversation_terminal") + try await session.applyConversationEvent( + event("run_terminal:1", "run_terminal", type), + conversationID: "conversation_terminal") + #expect(session.currentRunID == nil) + #expect(session.transcript.runState == expectedState) + session.reset() + } + } + + @Test("terminal tombstone prevents late replay resurrection") + func terminalTombstonePreventsResurrection() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_tombstone") + try await session.applyConversationEvent( + event("run_old:0", "run_old", "run.started"), conversationID: "conversation_tombstone") + try await session.applyConversationEvent( + event("run_old:1", "run_old", "run.completed"), conversationID: "conversation_tombstone" + ) + try await session.applyConversationEvent( + event("run_old:2", "run_old", "run.started"), conversationID: "conversation_tombstone") + #expect(session.currentRunID == nil) + #expect(!session.isBusy) + session.reset() + } + + @Test("first external active evidence resumes visible conversation") + func firstExternalActiveEvidenceResumesConversation() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_resume") + try await session.applyConversationEvent( + event("run_old:0", "run_old", "run.completed"), conversationID: "conversation_resume") + try await session.applyConversationEvent( + event("run_scheduled:0", "run_scheduled", "assistant.message"), + conversationID: "conversation_resume") + #expect(session.currentRunID == "run_scheduled") + #expect(session.isBusy) + #expect(session.scheduledRunStatus == "Scheduled run active") + session.reset() + } + + @Test("timestamp ordering rejects an older replay start") + func timestampOrderingRejectsOlderReplayStart() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_ordering") + try await session.applyConversationEvent( + event("run_new:0", "run_new", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_ordering") + try await session.applyConversationEvent( + event("run_old:0", "run_old", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_ordering") + #expect(session.currentRunID == "run_new") + #expect(session.accountingRunID == "run_new") + session.reset() + } +} From 4c337d65f95cb739c908c54001b15de31e050ee8 Mon Sep 17 00:00:00 2001 From: Dennison Bertram Date: Tue, 4 Aug 2026 01:18:10 +0200 Subject: [PATCH 2/2] fix(macapp): fence stale interactive affordances (#1123) * fix(macapp): fence stale interactive affordances * style(macapp): format interactive control views * fix(macapp): fence stale stop and steer actions (#1127) * fix(macapp): fence stale stop and steer actions * fix(macapp): bind ToolWalk to submitted run identity (#1129) * fix(macapp): bind ToolWalk to submitted run identity * fix(macapp): retain submitted run outcomes (#1131) * fix(toolwalk): observe displaced submission outcomes (#1138) * fix(toolwalk): observe displaced submission outcomes * fix(toolwalk): retain immutable timeout authority (#1139) * fix(toolwalk): retain immutable timeout authority * test(toolwalk): prove displaced timeout orchestration * docs(plan): record timeout authority regression evidence * docs: correct focused orchestration evidence count * fix(toolwalk): gate timeout cancellation by deadline ticket (#1146) --- docs/logs/INDEX.md | 17 + docs/logs/engineering-log.md | 106 +++ docs/logs/long-term-thinking-log.md | 66 ++ docs/logs/observational-log.md | 62 ++ docs/logs/system-log.md | 70 ++ ...issue-1122-interactive-state-impact-map.md | 73 ++ ...08-03-issue-1122-interactive-state-plan.md | 55 ++ ...8-03-issue-1125-action-owner-impact-map.md | 26 + ...2026-08-03-issue-1125-action-owner-plan.md | 31 + ...issue-1128-submission-handle-impact-map.md | 33 + ...08-03-issue-1128-submission-handle-plan.md | 41 + ...sue-1130-submission-outcomes-impact-map.md | 73 ++ ...-03-issue-1130-submission-outcomes-plan.md | 43 + ...3-issue-1133-passive-outcome-impact-map.md | 77 ++ ...6-08-03-issue-1133-passive-outcome-plan.md | 62 ++ ...issue-1136-timeout-authority-impact-map.md | 38 + ...08-03-issue-1136-timeout-authority-plan.md | 59 ++ docs/plans/INDEX.md | 30 + docs/plans/active-plan.md | 44 ++ macapp/Package.swift | 5 +- macapp/Sources/GoCodeUI/ChatView.swift | 91 ++- macapp/Sources/GoCodeUI/ProjectSession.swift | 25 +- .../Sources/GoCodeUI/RunSession+Answers.swift | 13 +- .../GoCodeUI/RunSession+RunControls.swift | 137 +++- macapp/Sources/GoCodeUI/RunSession.swift | 297 +++++-- macapp/Sources/GoCodeUI/RunSubmission.swift | 149 ++++ macapp/Sources/HarnessKit/Transcript.swift | 16 + macapp/Sources/ToolWalk/Runner.swift | 195 ++++- ...iveSubmissionOutcomeIntegrationTests.swift | 737 ++++++++++++++++++ .../RunSessionExternalControlTests.swift | 361 ++++++++- .../GoCodeUITests/RunSubmissionTests.swift | 525 +++++++++++++ .../HarnessKitTests/TranscriptTests.swift | 2 + .../SubmissionOutcomeTests.swift | 30 + 33 files changed, 3426 insertions(+), 163 deletions(-) create mode 100644 docs/plans/2026-08-03-issue-1122-interactive-state-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1122-interactive-state-plan.md create mode 100644 docs/plans/2026-08-03-issue-1125-action-owner-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1125-action-owner-plan.md create mode 100644 docs/plans/2026-08-03-issue-1128-submission-handle-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1128-submission-handle-plan.md create mode 100644 docs/plans/2026-08-03-issue-1130-submission-outcomes-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1130-submission-outcomes-plan.md create mode 100644 docs/plans/2026-08-03-issue-1133-passive-outcome-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1133-passive-outcome-plan.md create mode 100644 docs/plans/2026-08-03-issue-1136-timeout-authority-impact-map.md create mode 100644 docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md create mode 100644 macapp/Sources/GoCodeUI/RunSubmission.swift create mode 100644 macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift create mode 100644 macapp/Tests/GoCodeUITests/RunSubmissionTests.swift create mode 100644 macapp/Tests/ToolWalkTests/SubmissionOutcomeTests.swift diff --git a/docs/logs/INDEX.md b/docs/logs/INDEX.md index f0aff8de..5c995efa 100644 --- a/docs/logs/INDEX.md +++ b/docs/logs/INDEX.md @@ -22,6 +22,23 @@ - 2026-08-03 — Issue #1124 deterministic retry-wait callback fixture evidence is recorded in the engineering, observational, and system logs. +- 2026-08-03 — Issue #1136 records immutable submission timeout capability, + one-shot dispatch, and reset/load all-stream detachment separately from #1133. + +- 2026-08-03 — Issue #1133 corrects the prior displaced-result wording: + displacement revokes controls but does not end A outcome observation. + +- 2026-08-03 — Issue #1130 submission-local outcome ownership, deterministic + barriers, and ToolWalk timeout ordering are recorded in all durable logs. + +- 2026-08-03 — Issue #1128 native submitted-run ownership evidence is recorded + in the engineering, observational, system, and long-term logs. + +- 2026-08-03 — Issue #1125 native Stop/steer/ToolWalk ownership evidence is + recorded in the engineering, observational, system, and long-term logs. + +- 2026-08-03 — Issue #1122 native interactive-state ownership evidence is + recorded in the engineering, observational, system, and long-term logs. - 2026-08-03 — Issue #1120 deterministic blocked-heartbeat callback fixture evidence is recorded in the engineering, observational, and system logs. diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 63b93fa9..bc418ce4 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -57,6 +57,96 @@ normal/race passed in 13.200s/14.719s. Isolated foreground `./scripts/test-regression.sh` then passed normal/race plus 85.5% total coverage and zero uncovered functions in 2m26s. +## 2026-08-03 (Issue #1136 immutable timeout authority) + +- Replaced the provisional public handle cancel with a package-visible opaque + `TimedOutSubmissionTicket`. Its initializer is fileprivate to `Runner`; the + only mint point is `waitForTerminal`'s final deadline-edge lifecycle check. + Ticket consumption retains the private `RunSubmission` owner-token/generation + recheck and is transport-only. Terminal, failure, reset, and load revoke it. +- `RunSession` now tracks every submission stream by handle. Reset/load cancels + both displaced A and selected C rather than only the most recent stream. +- TDD red: removing the old API produced nine expected focused compile errors + at former direct call sites. Gated proof now requires no ticket/action before + deadline, B -> C -> A exact-one dispatch, duplicate refusal, and post-ticket + terminal/failure/reset revocation. Remaining full-gate evidence is recorded + by this corrected PR rather than inherited from the superseded implementation. +- Review correction: the first ticket implementation left a package-scoped raw + transport method callable before deadline. The ticket, constructor, and + transport closure now live in GoCodeUI; ToolWalk binds the immutable duration + at submission and `submissionTimeoutGate(for:)` alone verifies the derived + deadline and mints once. The #1146 CI-flake repair introduces an internal + `RunSession` monotonic-now seam shared by `RunSubmission.markStarted` and + `SubmissionTimeoutGate`; tests freeze/advance it at epsilon and exact + deadline instead of sleeping. `Runner.waitForTerminal` now accepts only its + poll interval so a caller cannot silently pass a conflicting timeout after + submission. A direct + gate regression plus a source-surface drift test prevents that bypass. + +## 2026-08-03 (Issue #1133 passive displaced-submission outcome) + +- Corrected the #1130 wait-policy gap: displacement is now a permanent action + fence, not a terminal ToolWalk result. Runner waits for its immutable A + handle's terminal/failure through deadline and never auto-answers/approves a + mismatched or displaced selected B. +- `cancelTimedOutSubmission` retains exact locally owned A transport authority + after B selection, but its displaced path is transport-only: it cannot alter + B selection, transcript, pending UI, or cancellation state. +- Test-first evidence: four URLSession-gated `RunSession.submit()` + Runner + tests were red before the repair (terminal/EOF/timeout returned displaced; + delayed ACK returned without A identity). The resulting #1133 tests prove + passive terminal, EOF failure, delayed acknowledgement, and B-safe timeout + policy. The stronger B -> C authority and revocation proof is tracked in + the separate #1136 entry above. +- Verification: final combined focused `PassiveSubmissionOutcomeIntegrationTests` + passes 10/10 (not the earlier intermediate 4/4 or 8/8 counts); strict format + (0/7 touched Swift files require formatting) and + full `swift test --package-path macapp` (244 tests / 46 suites) pass. Full + regression, independent review, and hosted checks remain. + +## 2026-08-03 (Issue #1130 submission-local outcomes) + +- Split `RunSubmission` into independent A-local `Lifecycle` and displacement + facts. A terminal or failure therefore remains available to the initiating + caller after scheduled B selection instead of becoming a false timeout. +- A delayed `startRun` acknowledgement now binds A's handle first, but only an + exact, undisplaced active handle may select/activate/account it. Stream EOF + and start/transport errors always settle A locally; they fail visible state + only while that same A still owns it. `finishRunIfCurrent` clears by object + identity rather than a reusable run-id lookup. +- ToolWalk now uses typed wait outcomes. Terminal/failure precede displacement, + and only `.timedOut` reaches guarded A cancellation. New deterministic gate + tests cover late acknowledgement, late start/EOF failure, reset/load + detachment, and zero B mutation; outcome tests cover ordering and cancellation. +- Verification: strict Swift formatting, focused submission/ToolWalk suites + (13 tests/2 suites), full `swift test` (238 tests/45 suites), and the + retained-pane `./scripts/test-regression.sh` pass (normal, race, 85.5% total + coverage, zero uncovered functions). + +## 2026-08-03 (Issue #1128 submitted-run ownership) + +- Added `RunSubmission`, returned by both native submit layers. It records A's + `startRun` identity, per-run transcript, terminal result, failure, and + displacement independently from the conversation's selected run. +- ToolWalk now waits, auto-controls, times out, and judges the handle. A + selected B produces an explicit displaced result before any B endpoint call. + A local lifecycle timestamp is retained so later authoritative B selection + works without weakening provisional stale-replay protection. +- Composer captures `.submit` or `.steer(A)` once; the pure execution seam + proves stale steering cannot fall through to a new submission. +- Verification: strict Swift format; focused submission/external/ToolWalk + suite (37 tests/5 suites); full Swift package (230 tests/44 suites); exact + repository normal/race regression; coverage 85.5% with zero uncovered + functions. + +## 2026-08-03 (Issue #1125 native action-owner fence) + +- Added expected-run cancel/steer boundaries. Chat Stop, Composer, and ToolWalk + timeout carry their rendered/decision A identity; a B mismatch exits before + local mutation, Task creation, or HTTP. +- Deterministic A-to-B tests prove stale Stop, steer, and timeout send zero B + endpoint requests while legitimate B actions still reach their endpoints. + ## 2026-08-03 (Issue #1007 External Scheduled-Run Controls Rebase) - System/component: `RunSession` control-owner reducer, accounting fence, @@ -4040,3 +4130,19 @@ Skipped creating separate issues for Op/EventMsg protocol (already covered by SS - Three callback deadline-release fixtures now use a one-second test lease and causal gates: admitted callback, heartbeat renewal entry, deadline signal, then cancellation-aware starter cancellation before unblocking renewal. - This is test-only. The strengthened assertions retain the durable safe error, attempt/run identity, retry state, and token/lease clearing contracts. Focused normal x20 (60.593s), race x20 (62.087s), and the full normal/race/coverage gate passed at 85.5% total coverage with zero uncovered functions. +## 2026-08-03 (Issue #1122 native interactive-state ownership) + +- `PendingApproval` and `PendingPlan` now retain their originating SSE + `run_id`. `RunSession` clears approval, plan, and pending input synchronously + when selection changes, a selected run retires/falls back, or active runs are + cleared. Chat and ToolWalk pass that captured id to guarded action APIs. +- Deterministic external-run tests cover approval, plan, and input from A being + displaced by timestamp-newer B; stale captured actions produce no B endpoint + request. They also cover selected terminal clearing and a foreign terminal + preserving B's interaction. The expected-red focused Swift build initially + failed because run IDs and explicit actions did not exist; the focused green + suite passes after implementation. +- Exact final verification: focused external ownership (11 tests), complete + Swift package (222 tests / 43 suites), and `scripts/test-regression.sh` all + pass; the repository coverage gate reports 85.5% total and zero uncovered + functions. diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index e690de30..5cc895b5 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -38,6 +38,72 @@ later same-run admission; normal/race/package/full gates are green. - Guardrails: no product source, API, SQLite schema, config, client, UI, or lifecycle change; no sleep/timeout increase or synthetic production defect. +## 2026-08-03 (Issue #1136 immutable timeout authority) + +- Command intent: make a timed-out submitted A independently and exactly + cancellable after B/C selection without allowing the timeout path to affect + B or C. +- Success: only a Runner deadline-minted opaque ticket can consume A's + started-only capability once; terminal, failure, reset, and load revoke it; + reset/load physically stop every A/C submission stream. Direct deterministic + proof complements #1133 policy waits. +- Non-goal: reintroducing run-ID lookup, changing server cancellation, or + mutating selected-run UI from timeout transport. + +## 2026-08-03 (Issue #1133 intent correction) + +- Command intent: a callback/cron continuation must visibly continue B while + the initiating tool walk truthfully observes its own submitted A outcome. +- Success: B displacement is sticky for control authority; A terminal/failure + after B is a valid A verdict; an A deadline cancels only A and produces no B + action or visible-state mutation. +- Non-goal: making B a hidden fallback, treating displacement as success, or + using shared `currentRunID` to judge/control A. + +## 2026-08-03 (Issue #1130 submission-local outcomes) + +- Command intent: repair the #1128 review findings without weakening the + native external-run ownership fences. +- User intent: an initiating A turn must receive its real terminal/failure + outcome, while a visible callback/cron B remains correct and controllable. +- Success: barrier-proven A terminal/failure survives B selection; late A work + cannot change B; ToolWalk cancels only a genuine A timeout; reset/load/EOF + are deterministic ownership boundaries. +- Guardrails: stacked native/ToolWalk-only change; preserve #1122/#1125 + expected-run guards and #1128 A-only transcript/displacement behavior. + +## 2026-08-03 (Issue #1128 submitted-run ownership) + +- Command intent: close the remaining native composer/ToolWalk A-to-B ownership + gaps identified by Sol review after #1125. +- User intent: an agent-walk or user click must continue/control the submitted + conversation turn, never a later cron/callback continuation that happens to + be visually selected. +- Success: immutable composer action selection plus an A-only handle whose + identity comes only from `startRun`, with deterministic zero-B-action proof, + retained A terminal verdict, and safe failure/reset behavior. + +## 2026-08-03 (Issue #1125 native action-owner fence) + +- Command intent: repair the remaining stale native action paths identified in + Sol review of #1123 as a single stacked slice. +- User intent: no scheduled continuation receives Stop, steer, or timeout cancel + meant for an earlier rendered run. +- Success: expected-run guards and captured client identities with deterministic + zero-B-endpoint proof, while #994 and #1122 semantics remain intact. + +## 2026-08-03 (Issue #1122 Native Interactive-State Ownership) + +- Command intent: prevent a stale approval, plan, or input affordance from + directing a user decision to a newer scheduled continuation. +- User intent: cron/callback conversation continuation must be visibly and + safely controllable, not merely runnable through the harness. +- Success definition: every pending affordance carries its exact run identity; + owner changes and retirement synchronously erase stale UI; captured stale + actions cannot start a B request; selected B survives foreign terminal replay. +- Guardrails: stack on #1118, preserve #994 delayed acknowledgement ownership, + change no server/TUI/callback behavior, and keep a deterministic TDD record. + ## 2026-08-03 (Issue #1007 External Scheduled-Run Control Rebase) - Command intent: make scheduled callback/cron continuations visibly active diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 1a1ee68a..d3c0d54d 100644 --- a/docs/logs/observational-log.md +++ b/docs/logs/observational-log.md @@ -31,6 +31,59 @@ - The pre-deadline checkpoint includes retry state, exact due time, reserved run ID, attempt one, and empty token/lease; checking all of them prevents a no-call assertion from masking an accidental claim or fence leak. +## 2026-08-03 (Issue #1136 timeout capability proof) + +- A real deadline is suitable for #1133 wait-policy coverage but is not enough + if any caller can turn a submission handle into authority. The opaque ticket + is absent before deadline and can be constructed only at Runner's deadline + boundary; deterministic consumption makes B -> C -> A exact-one dispatch + and terminal/failure/reset non-dispatch observable. +- A single mutable stream task would leave displaced A running when C starts. + The handle-keyed task registry permits reset/load to stop both streams. + +## 2026-08-03 (Issue #1133 passive outcome observation) + +- The #1130 handle correctly retained A lifecycle after B selection, but the + consumer stopped polling it on `.displaced`; durable A evidence was therefore + present but unobserved. A control authority and outcome observation are + separate concerns. +- Gated integration runs show B can precede A terminal, A stream EOF, A timeout, + or A start acknowledgement. Each retains B as the selected scheduled run; + only the deadline scenario emits an A cancel request. +- B can itself terminal before A while a user submits C. This proved timeout + authorization must follow A's stream lifetime, not `activeSubmission` or the + one current local-stream pointer. The final contract uses an immutable A + handle owner token plus reset/load generation and cancels every live local + submission stream when detaching a session. + +## 2026-08-03 (Issue #1130 submission-outcome observation) + +- The original single `State` made displacement overwrite terminal/failure + evidence. ToolWalk then saw only a nonterminal displaced handle and could + report an A timeout even when A had completed. +- A late `startRun` response is not a stale response to discard: its run ID is + needed for A-local diagnostics and outcome handling. It is stale only for + shared selection, accounting, streams, and visible error state. +- EOF is an ownership-sensitive failure: the visible A must stop spinning, but + the same EOF after B selection must not make B look failed. + +## 2026-08-03 (Issue #1128 submission observation) + +- A rendered run ID is insufficient when the action type is re-derived at + click time. Both the mode and owner must be captured together. Likewise, + shared session state is a presentation authority, not proof of which run a + ToolWalk submission started. +- The red regression additionally showed that a local run must record its own + first timestamped lifecycle frame. Otherwise it remains permanently + provisional and a genuinely newer scheduled continuation cannot become the + selected owner. + +## 2026-08-03 (Issue #1125 action-owner observation) + +- Stop and steer are authority-bearing UI actions: retaining a SwiftUI closure + across A-to-B selection means a fresh current-ID lookup changes the user's + target. The identity must be captured and checked before draft/cancel state. + ## 2026-08-03 (Issue #1007 Rebase Observation) - The original #1007 action fixture attempted approve, deny, answer, steer, @@ -824,3 +877,12 @@ Use this file for observations about system behavior without immediately prescri ## 2026-08-03 (Issue #1141 callback deadline-release observation) - A heartbeat's absence from `ExtendLease` is not evidence that deadline cancellation failed: under CI load the independent deadline can cancel the admission first. Fixtures now observe both the deadline and starter context cancellation, so their outcome no longer depends on heartbeat scheduling. Normal and race stress x20 and the full regression passed without callback runtime changes. +## 2026-08-03 (Issue #1122 ownership observation) + +- A visible native interaction is an authority-bearing object, not just a + transcript decoration. If it is retained across `currentRunID` replacement, + resolving the action at click time changes the user's target from A to B. +- Generation checks alone cannot protect this: they invalidate asynchronous + completions, while a stale SwiftUI closure can issue a fresh request. The + captured run ID must be checked before creating its task and again before + issuing its network operation. diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index 3d26bfa7..0aa91911 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -38,6 +38,68 @@ complete tools normal/race (13.200s/14.719s) pass on the final source tree; isolated repository regression also passes normal/race, 85.5% coverage, and zero uncovered functions. +## 2026-08-03 (Issue #1136 immutable timeout capability) + +- `RunSubmission` privately binds owner token, generation, lifecycle, and a + consumed bit. ToolWalk alone binds its configured duration at submission, + `markStarted` derives the absolute deadline, and `Runner.waitForTerminal` + mints a package-visible ticket with a fileprivate constructor only through + `RunSession.submissionTimeoutGate(for:)` after its exact deadline check. The + ticket captures a fileprivate transport closure; + no raw package or public `RunSubmission` handle-cancel API remains. A + handle-keyed task registry lets reset/load cancel all local streams. +- Deterministic native timing: production uses `ContinuousClock.now`; the + internal-only RunSession initializer injects the same monotonic closure into + `markStarted` deadline creation and gate expiry checks, so tests advance + epsilon/exact-deadline state without scheduler-dependent sleeps. +- Wait API boundary: `Runner.waitForTerminal` accepts only a polling interval. + The timeout is immutable submission configuration, not a second wait-phase + parameter that can conflict with the stored deadline. + +## 2026-08-03 (Issue #1133 passive A outcome after B selection) + +- Flow: ToolWalk captures `RunSubmission(A)` -> conversation stream selects B + and marks A displaced -> Runner disables all automatic controls yet continues + reading A-local lifecycle -> A terminal/failure is judged, or deadline sends + the existing A cancel endpoint through a local-ownership fence. +- The selected-run reducer remains the only B UI authority. The displaced A + timeout path intentionally performs no shared-state transition, so it cannot + clear B pending controls, selection, transcript, or acknowledgement state. +- `RunSubmission` carries a session-owner token and reset/load generation plus + a one-shot started-only timeout capability. It preserves A-only authority + through B -> C replacement without reconstructing it from an ID/set; terminal + or failure consume no capability, and reset/load cancels all live submission + tasks while invalidating old handles. + +## 2026-08-03 (Issue #1130 submission-local outcome flow) + +- Flow: local composer/ToolWalk A -> `RunSubmission.lifecycle` plus + `isDisplaced`; conversation SSE can select scheduled B without rewriting A. + A start/stream tasks settle the handle, while `RunSession` shared transcript, + accounting, and controls require exact active-handle identity and selected A. +- ToolWalk order is terminal -> failure -> displaced -> timeout. The first two + are judged from A's transcript, displacement performs no automatic control, + and only timeout calls existing expected-run cancellation for A. +- Reset/load synchronously displace and detach active A. A late response cannot + select the replacement conversation; a cancelled detached task is not + reported as a transport failure. + +## 2026-08-03 (Issue #1128 submission lifecycle) + +- Flow: Composer/ToolWalk -> `ProjectSession.submit` -> `RunSession.submit` -> + `RunSubmission` -> `startRun` response assigns A -> A-only per-run SSE + reduces the handle -> terminal/failure/displacement is observed by ToolWalk. +- A selected B synchronously marks a started A handle displaced. ToolWalk then + performs no automatic input/approval/timeout action against B. Reset/load + displaces unresolved submissions; a late server response exits before it can + reactivate the reset session. + +## 2026-08-03 (Issue #1125 native action owner) + +- Ownership path: rendered Stop/Composer or ToolWalk timeout -> expected run ID + -> RunSession guard -> existing run-specific HarnessClient cancel/steer endpoint. + Mismatch returns before local force-stop, draft clear, Task, or network. + ## 2026-08-03 (Issue #1007 Native External-Run Reducer) - `RunSession` now has separate in-memory active/control ownership from @@ -1311,3 +1373,11 @@ Use this file to document systems, interfaces, and interactions as they are buil ## 2026-08-03 (Issue #1141 callback deadline fixture boundary) - Callback lease deadline ownership is independent of heartbeat I/O. Tests that model blocked renewal must await the manager-owned deadline and the starter's canceled context before releasing renewal; SQLite's durable state remains the observable outcome for API, TUI, and native replay. +## 2026-08-03 (Issue #1122 native interaction owner) + +- Ownership path: conversation SSE `HarnessEvent.runID` -> `Transcript` + approval/plan or `AskUserPrompt.runID` -> `RunSession.currentRunID` fence -> + Chat/ToolWalk captured ID -> run-specific HarnessClient endpoint. +- Selection/selected terminal/fallback/reset clear all pending interaction + state immediately. A foreign terminal retires only its own run and cannot + clear the currently selected run's interaction state. diff --git a/docs/plans/2026-08-03-issue-1122-interactive-state-impact-map.md b/docs/plans/2026-08-03-issue-1122-interactive-state-impact-map.md new file mode 100644 index 00000000..7ff9e70a --- /dev/null +++ b/docs/plans/2026-08-03-issue-1122-interactive-state-impact-map.md @@ -0,0 +1,73 @@ +# Cross-Surface Impact Map: Issue #1122 + +## Task + +- Task / issue: #1122 native interactive-state ownership. +- Plan link: `2026-08-03-issue-1122-interactive-state-plan.md`. +- Owner: Codex. +- Status: in implementation; stacked on #1118 `34219699`. + +## Current Ownership, Callers, and Data Flow + +- Entry points: conversation SSE reaches `RunSession.apply`; Chat and ToolWalk + render pending state and invoke `approve`, `deny`, and `answer`. +- Source of truth: `RunSession.currentRunID`; `Transcript.pendingApproval` and + `pendingPlan`; `RunSession.pendingQuestions` (already carries `runID`). +- Search evidence: `rg -n "PendingApproval|PendingPlan|pendingQuestions|currentRunID|approve" macapp -g'*.swift'`. +- Conclusion: pending approvals/plans lack identity, while actions currently + resolve a fresh `currentRunID`, allowing displaced UI to address B. + +## Config, API, CLI, and Tools + +- Config/defaults/environment: None. +- API/wire: no server change; native client continues existing run-specific + endpoints. +- CLI/tools: ToolWalk captures each affordance origin before action; no tool + catalog/routing change. +- Error validation: mismatched expected/current/pending owner is a no-op before + a Task or network call. + +## Persistence and Compatibility + +- No schemas, migrations, caches, or stored wire shapes change. +- Native in-memory structs gain run identity derived from the already present + SSE event `run_id`; no compatibility migration is needed. +- Mixed client rollout is safe because server endpoints are unchanged. + +## Lifecycle, Security, and Reliability + +- Selection, terminal retirement, fallback, clear/reset synchronously discard + stale A pending state and invalidate asynchronous request ownership. +- This prevents a human decision intended for A from authorizing B; no auth or + secret boundary changes. +- Preserve #994 control generation/lifecycle ACK semantics; foreign terminals + retain selected B interactions. + +## Product and Integration Surfaces + +- Server/runtime: None beyond existing SSE and action endpoints. +- macOS: Chat and ToolWalk bind visible action to captured run identity. +- TUI/web: None; their state owners are separate. +- Provider/catalog/automation: None. +- UX/accessibility: stale affordances disappear synchronously; valid controls + retain their labels and disabled states. + +## Deployment and Operations + +- No migration or flag. Deploy with #1118 after its base merges and checks pass. +- Existing action endpoint logs expose any unexpected network request; no new + metrics needed. +- Rollback: revert this stacked native-only commit; server behavior remains + untouched. + +## Regression Tests + +- Expected red: pending A survives selected B and stale action can hit B. +- Acceptance: approval, plan, input A-to-B; selected terminal/no fallback; + foreign terminal preserves B; action endpoint capture is zero for stale UI. +- Commands: focused `swift test --package-path macapp --filter RunSessionExternalControlTests`, full `swift test --package-path macapp`, and `./scripts/test-regression.sh`. + +## Documentation and Handoff + +- No public docs before code; plan/map and durable logs/indexes after code. +- PR records red/green commands, stack base, and no-merge handoff. diff --git a/docs/plans/2026-08-03-issue-1122-interactive-state-plan.md b/docs/plans/2026-08-03-issue-1122-interactive-state-plan.md new file mode 100644 index 00000000..5fb6c1a2 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1122-interactive-state-plan.md @@ -0,0 +1,55 @@ +# Plan: Issue #1122 native interactive-state ownership + +## Context + +- Governing GitHub issue: #1122. +- Problem: a timestamp-newer selected run could leave approval, plan, or input UI + owned by the displaced run visible; its action then targeted `currentRunID`. +- User impact: an operator can accidentally approve, deny, steer, or answer a + different scheduled continuation. +- Constraints: stack on draft PR #1118; preserve #994 request generations and + delayed terminal acknowledgement semantics; no server or callback change. + +## Scope + +- In scope: run-scoped native pending models, synchronous selection/retirement + invalidation, guarded action APIs, Chat and ToolWalk call sites, regressions. +- Out of scope: harness API/wire changes, callback dispatch, TUI, and #1118's + external-run selection policy. + +## Documentation Contract + +- Feature status: implemented; stacked draft pending independent review. +- Public docs affected: None; this is internal native ownership repair. +- Spec docs to update before code: this plan and its impact map. +- Implementation notes to add after code: durable logs and indexes. + +## Test Plan (TDD) + +- New failing tests first: A approval/plan/input then newer B selection clears + all A UI; stale UI action emits no B endpoint; selected terminal/no-fallback + clears interaction state; foreign terminal preserves B state. +- Existing tests to update: transcript pending model construction and UI/ToolWalk + action invocations. +- Regression tests required: strict Swift focused/package/full and repository + regression gate. + +## Cross-Surface Impact Map + +- `docs/plans/2026-08-03-issue-1122-interactive-state-impact-map.md`. + +## Implementation Checklist + +- [x] Verify structured issue and exact #1118 head. +- [x] Record ownership/caller search evidence and cross-surface map. +- [x] Write and capture expected-red acceptance tests. +- [x] Add run identity and synchronous ownership invalidation. +- [x] Bind UI and ToolWalk actions to captured run identity. +- [x] Run focused, full Swift, repository regression, and update logs/indexes. +- [ ] Push stacked draft PR with `Closes #1122`; do not merge. + +## Risks and Mitigations + +- Risk: clearing state could break an in-flight #994 acknowledgement. +- Mitigation: invalidate state ownership only; retain lifecycle generation + bookkeeping so a terminal completion still releases its own control request. diff --git a/docs/plans/2026-08-03-issue-1125-action-owner-impact-map.md b/docs/plans/2026-08-03-issue-1125-action-owner-impact-map.md new file mode 100644 index 00000000..040b8cdb --- /dev/null +++ b/docs/plans/2026-08-03-issue-1125-action-owner-impact-map.md @@ -0,0 +1,26 @@ +# Cross-Surface Impact Map: Issue #1125 + +## Ownership and Data Flow + +- Chat captures `currentRunID` for Stop/Composer; ToolWalk snapshots the timeout + owner; `RunSession` validates it before local mutation, Task creation, or HTTP. +- Search evidence: `rg -n "run.cancel|run.steer|currentRunID|ToolWalk" macapp + -g '*.swift'` located exactly the three stale action paths. + +## API, Persistence, Compatibility + +- No API/wire/schema/persistence change. Existing run-specific endpoints remain. +- Legacy dynamic APIs remain for programmatic compatibility; visible clients use + expected-run overloads. Mixed client rollout is safe. + +## Lifecycle, Clients, and Operations + +- Stale actions cannot clear B's draft, local-force-stop B, or send B traffic. +- #994 delayed ACK and external-run isolation remain in the same RunSession path. +- Only macOS and ToolWalk change; TUI, harness, providers, deployment, and public + docs are unaffected by source search. Rollback is a native-only revert. + +## Tests + +- A-to-B stale Stop, steer, and ToolWalk timeout assert zero B endpoints; + legitimate B cancel/steer retains positive endpoint evidence. diff --git a/docs/plans/2026-08-03-issue-1125-action-owner-plan.md b/docs/plans/2026-08-03-issue-1125-action-owner-plan.md new file mode 100644 index 00000000..0b1ceb3f --- /dev/null +++ b/docs/plans/2026-08-03-issue-1125-action-owner-plan.md @@ -0,0 +1,31 @@ +# Plan: Issue #1125 native run-action ownership + +## Context and Scope + +- Governing issue: #1125, stacked on #1122 head `d1931ae2`. +- Stop, Composer steer, and ToolWalk timeout previously resolved `currentRunID` + at action time, so stale A UI could target newly selected B. +- In scope: expected-run cancel/steer APIs, captured identities in native UI and + ToolWalk, deterministic A-to-B tests, and inherited strict-format repair. +- Out of scope: harness API, callback dispatch, TUI, selection policy, and + #1122 approval/plan/input fences. + +## TDD and Verification + +- Expected red: a captured A action can issue B cancel/steer after selection. +- Green: stale A yields zero B endpoint calls; B remains selected and draft is + preserved; legitimate B actions, #994 delayed ACK/force-stop, and external + isolation remain covered. +- Gates: strict Swift format, focused native/ToolWalk tests, full Swift package, + repository regression, then hosted checks and independent review. + +## Impact Map + +- `2026-08-03-issue-1125-action-owner-impact-map.md`. + +## Checklist + +- [x] Verify issue and exact #1122 stack base. +- [x] Add expected-run fences, visible captures, and deterministic regressions. +- [x] Repair strict Swift formatting. +- [ ] Run complete gates, push draft `Closes #1125`, and obtain cheap review. diff --git a/docs/plans/2026-08-03-issue-1128-submission-handle-impact-map.md b/docs/plans/2026-08-03-issue-1128-submission-handle-impact-map.md new file mode 100644 index 00000000..b7a45f60 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1128-submission-handle-impact-map.md @@ -0,0 +1,33 @@ +# Cross-Surface Impact Map: Issue #1128 + +## Ownership and Data Flow + +- Composer captures `ComposerAction` once: either `.submit` or `.steer(A)`. +- `ProjectSession.submit` returns `RunSession.submit`'s `RunSubmission`; only + the successful `startRun` response writes its A identity. The per-run stream + alone reduces the handle transcript and terminal state. +- ToolWalk polls, auto-answers, auto-approves, times out, and judges this + handle. It returns a displaced result rather than resolving B from shared UI + state. + +## API, Persistence, Compatibility + +- No API, wire, schema, or persistence changes. Existing run-specific control + endpoints remain unchanged. +- Existing callers may discard the newly returned submission value; visible + Composer and ToolWalk use the ownership-aware path. Rollback is native-only. + +## Lifecycle, Clients, and Operations + +- A local run records its first timestamped lifecycle frame, allowing a later + authoritative scheduled continuation to displace it while retaining the + provisional-replay protection from #1007. +- Reset/load marks an unresolved handle displaced, so a late `startRun` + response cannot resurrect a torn-down conversation. TUI, harness, providers, + deployment, cron, and callbacks are unaffected by source search. + +## Tests + +- Deterministic stubs prove B before response cannot become A; later B causes + A displacement and zero B cancel; A transcript/terminal excludes B; failure + and reset do not resurrect A; captured steer never calls submit. diff --git a/docs/plans/2026-08-03-issue-1128-submission-handle-plan.md b/docs/plans/2026-08-03-issue-1128-submission-handle-plan.md new file mode 100644 index 00000000..397e170b --- /dev/null +++ b/docs/plans/2026-08-03-issue-1128-submission-handle-plan.md @@ -0,0 +1,41 @@ +# Plan: Issue #1128 submitted-run ownership + +## Context and Scope + +- Governing issue: #1128, stacked on #1127 head `d2cd29cf`. +- A composer closure captured only its rendered run ID but chose steer versus + submit using live state. ToolWalk similarly re-read global run/transcript + state after submitting, allowing a scheduled B to replace local A. +- In scope: immutable composer action selection, one A-only `RunSubmission` + handle returned by both native submit layers, ToolWalk lifecycle ownership, + and deterministic native regressions. +- Out of scope: harness API/persistence, callback or cron execution, TUI, and + scheduled-run selection policy beyond recording an existing local run's + first lifecycle timestamp. + +## TDD and Verification + +- Red: B selected before A's response can be mistaken for A; after A capture, + B can receive A's timeout action; a stale steer can fall through to submit. +- Green: the handle resolves only from `startRun`, retains A-only events and + terminal evidence, becomes displaced on selected B, and ToolWalk exits with + no B action. A real terminal A still receives its normal verdict. +- Gates: strict format, focused submission/external-control/ToolWalk tests, + full Swift package, repository normal/race/coverage gate, then independent + cheap review and hosted checks. + +## Impact Map + +- `2026-08-03-issue-1128-submission-handle-impact-map.md`. + +## Checklist + +- [x] Verify #1128 acceptance criteria and stacked #1127 base. +- [x] Add immutable composer action and A-only `RunSubmission` evidence. +- [x] Add deterministic B-before-response, B-after-capture, A-terminal, and + start-failure/reset regressions. +- [x] Run strict format and complete Swift package. +- [x] Run repository regression: normal/race passed; coverage passed at 85.5% + with zero uncovered functions. +- [ ] Publish stacked draft `Closes #1128` and obtain independent cheap review + plus hosted checks. diff --git a/docs/plans/2026-08-03-issue-1130-submission-outcomes-impact-map.md b/docs/plans/2026-08-03-issue-1130-submission-outcomes-impact-map.md new file mode 100644 index 00000000..91d00a8a --- /dev/null +++ b/docs/plans/2026-08-03-issue-1130-submission-outcomes-impact-map.md @@ -0,0 +1,73 @@ +# Issue #1130 Cross-Surface Impact Map + +## Task + +- Task / issue: #1130, submission-local terminal and failure outcomes. +- Plan link: `2026-08-03-issue-1130-submission-outcomes-plan.md`. +- Owner: native-client delivery lane. +- Status: implementation planned; stacked on #1128 `654b7da`. + +## Current Ownership, Callers, and Data Flow + +- Entry points: `ProjectSession.submit` and composer -> `RunSession.submit` -> + `RunSubmission`; headless `ToolWalk.Runner.walk` waits and judges that handle. +- Source of truth: `RunSubmission` owns A-local lifecycle/transcript; selected + conversation state remains `RunSession.currentRunID`/`Transcript`. +- Consumers: run-specific SSE, conversation SSE, reset/load, guarded controls, + ToolWalk wait/timeout/verdict. +- Search evidence: `rg -n "RunSubmission|activeSubmission|waitForTerminal|markDisplaced" macapp`. +- Conclusion: lifecycle must be local to the handle and selection/displacement + must remain independent; shared session state cannot judge A. + +## Config, API, CLI, and Tools + +- Config/defaults/environment: None. +- HTTP endpoint/schema/server wiring: existing run start/events/control endpoints + only; no request or response change. +- CLI/tools: ToolWalk internal waiting semantics only; no command grammar change. +- Errors: A-local failure is preserved; B-visible error remains untouched. + +## Persistence and Compatibility + +- Schemas/migrations/caches: None. +- Compatibility: Swift internal types only; the existing public submission + handle retains `state`, `failure`, and `isDisplaced` accessors. +- Mixed-version: None; a client update is self-contained. + +## Lifecycle, Security, and Reliability + +- Concurrency/cancellation: response, per-run SSE, and conversation SSE may + race selection/reset. Identity and run guards make late work harmless. +- Auth/secrets: no changes. +- Failure/recovery: EOF/start error becomes A-local; only the owning A can + fail shared visible state; reset/load permanently detach unresolved A. + +## Product and Integration Surfaces + +- Server/runtime and TUI: None; API/TUI retain existing behavior. +- macOS GUI: scheduled B remains visible while A result is retained privately + for the initiating UI/ToolWalk flow. +- Provider/tool routing/external systems: None. +- UX/accessibility: no new controls; prevents an incorrect B error/timeout. + +## Deployment and Operations + +- Deployment: normal native/ToolWalk PR rollout, no migration/flag. +- Diagnostics: exact A failure/result stays on `RunSubmission`; no new logs. +- Rollback: revert the PR if ownership evidence regresses. + +## Regression Tests + +- First expected red: terminal A followed by B reports no terminal wait outcome + under the old mutually-exclusive `.displaced` state. +- Acceptance: barrier sequencing for terminal/failure/late ACK/reset/load/EOF; + typed ToolWalk outcome proves only timeout cancels A. +- Negative: zero B endpoint requests and no B transcript/lifecycle/error mutation. +- Integration: focused Swift targets, full `swift test`, format, and + `./scripts/test-regression.sh`; live GUI/TUI/API acceptance remains #1010. + +## Documentation and Handoff + +- Before code: this plan/map and #1130 design/TDD comment. +- After code: active plan, four durable logs, both indexes, PR test evidence. +- No public documentation/release note: internal ownership repair only. diff --git a/docs/plans/2026-08-03-issue-1130-submission-outcomes-plan.md b/docs/plans/2026-08-03-issue-1130-submission-outcomes-plan.md new file mode 100644 index 00000000..fe77cc4d --- /dev/null +++ b/docs/plans/2026-08-03-issue-1130-submission-outcomes-plan.md @@ -0,0 +1,43 @@ +# Issue #1130: Submission-local Outcomes Plan + +## Intent and scope + +An A submission must retain its own start, terminal, and failure outcome when +a callback/cron continuation B becomes the visible conversation run. This is a +native-client and ToolWalk ownership repair stacked on #1128 (`654b7da`): no +server, scheduler, TUI, or callback implementation changes are in scope. + +## Design + +`RunSubmission` will hold two independent facts: an A-local lifecycle +(`starting`, `started`, `terminal`, or `failed`) and a displacement bit. +Displacement prevents controls from being sent to selected B; it must not erase +an A terminal/failure that ToolWalk needs to judge truthfully. `RunSession` +will retain/clear `activeSubmission` by object identity, bind a late A +`startRun` acknowledgement to its handle before checking displacement, and +apply visible failure/accounting/activation only while that exact submission is +still the selected A owner. Reset/load detach the handle; stream EOF produces +an A-local failure and only marks the visible transcript failed when A owns it. + +`ToolWalk.Runner` will use a typed wait outcome. Terminal and failure are +consumed before displacement; only an actual timeout invokes A's guarded cancel +endpoint. A displaced or failed submission produces a result without any B +action. + +## Test-first plan + +1. Add deterministic barrier tests that currently fail: terminal A then + select B before ToolWalk observes it; delayed A acknowledgement after B; + late A start/stream failure after B; reset/load detachment; and EOF failure + ownership. +2. Add ToolWalk outcome tests proving only `.timedOut` requests cancel and that + A terminal is judged rather than reported as a timeout. +3. Implement the smallest lifecycle/identity changes, then run focused Swift, + all Swift tests, format, and the full repository regression gate. + +## Rollout and rollback + +This is an in-process macOS/ToolWalk reducer change with no persisted or wire +format change. Rollback is a normal PR revert. The safety trigger is any +evidence that an A-local completion changes B's visible state or that a timeout +action targets B; the barrier regressions prevent both. diff --git a/docs/plans/2026-08-03-issue-1133-passive-outcome-impact-map.md b/docs/plans/2026-08-03-issue-1133-passive-outcome-impact-map.md new file mode 100644 index 00000000..4dbe45d0 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1133-passive-outcome-impact-map.md @@ -0,0 +1,77 @@ +# Cross-Surface Impact Map: Issue #1133 + +## Task + +- Task / issue: #1133 passive observation of displaced A submission outcomes. +- Plan link: `2026-08-03-issue-1133-passive-outcome-plan.md`. +- Owner: native-client delivery lane. +- Status: implemented locally, stacked on #1131 `63cf9fcd`. + +## Current Ownership, Callers, and Data Flow + +- Entry points: `ProjectSession.submit`/composer -> `RunSession.submit` -> + `RunSubmission`; `ToolWalk.Runner` waits, controls, and judges the handle. +- Source of truth: `RunSubmission.lifecycle` is A outcome; `isDisplaced` and + `RunSession.currentRunID` are authority fences for shared UI/actions. +- Search evidence: `rg -n "RunSubmission|waitForTerminal|cancelTimedOutSubmission|activeSubmission" macapp`. +- Conclusion: submission-local result and shared selected-run authority are + intentionally separate; wait policy must observe the former while honoring + the latter. + +## Config, API, CLI, and Tools + +- Config/defaults/environment: none. +- HTTP/schema/server: uses existing exact `/v1/runs/{A}/cancel`; no schema or + server change. +- CLI/tools: ToolWalk wait policy only; command grammar/tool catalog unchanged. +- Errors: A EOF/failure remains a truthful ToolWalk result, never B UI error. + +## Persistence and Compatibility + +- Schemas/migrations/caches: none. +- Compatibility: no public wire change; `RunSubmission` keeps existing state + projection and sticky displacement semantics. +- Mixed version: client-local behavior; no server compatibility concern. + +## Lifecycle, Security, and Reliability + +- Concurrency: A SSE, B conversation SSE, start ACK, and timeout race. A is + passive after displacement until terminal/failure/deadline. #1136 owns the + immutable B/C authority and revocation contract. +- Authorization/trust: #1133 relies on the existing exact A-only path; #1136 + hardens the capability boundary. No credentials change. +- Failure/recovery: timeout sends A-only transport without touching B's + selection, pending UI, transcript, or cancel state. + +## Product and Integration Surfaces + +- Server/runtime and TUI: none. +- macOS/ToolWalk: B stays visually selected; initiating ToolWalk receives the + actual A terminal/failure verdict. +- Providers/catalog/automation: none; cron/callback B is the motivating normal + conversation continuation. +- UX/accessibility: no new controls; avoids stale controls acting on B. + +## Deployment and Operations + +- Deployment/flags: standard native PR; no migration. +- Diagnostics: the retained submission handle is the A-local evidence. +- Rollback: revert PR if an A deadline can affect B; gated tests isolate this. + +## Regression Tests + +- Red: four initial actual gated URLSession tests failed pre-fix: A terminal/EOF + and timeout returned `.displaced`; delayed ACK returned before A had an + identity. +- Acceptance: B-before-terminal, EOF, timeout, and delayed ACK use + `RunSession.submit()` plus `Runner`, assert B selection and zero B endpoint + actions. #1136 owns B -> C exact-one A dispatch and revocation coverage. +- Full commands: strict format, focused native/ToolWalk tests, `swift test + --package-path macapp`, and `./scripts/test-regression.sh`. + +## Documentation and Handoff + +- Public docs: none. +- Internal docs: plan/map, active plan, four durable logs, and both indexes. +- PR handoff: `Closes #1133`, red/green commands, exact stacked base, review, + and hosted check evidence. diff --git a/docs/plans/2026-08-03-issue-1133-passive-outcome-plan.md b/docs/plans/2026-08-03-issue-1133-passive-outcome-plan.md new file mode 100644 index 00000000..a4f004f1 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1133-passive-outcome-plan.md @@ -0,0 +1,62 @@ +# Plan: Issue #1133 passive displaced-submission outcomes + +## Context + +- Governing GitHub issue: #1133. +- Problem: after B replaces visible submitted A, ToolWalk returned `.displaced` + immediately. A terminal/failure therefore could never be judged even though + `RunSubmission` still retained the correct A-only evidence. +- User impact: scheduled callbacks/crons can continue the visible conversation + without causing the initiating tool walk to falsely fail or control B. +- Constraints: stack on #1131 `63cf9fcd`; no harness, TUI, persistence, or wire + contract change; B selection permanently revokes A automatic controls. + +## Scope + +- In scope: retain passive A observation through terminal/failure/deadline and + prove the timeout policy sends A-only transport without a B action, using + gated `RunSession.submit()` plus `Runner` integration coverage. #1136 owns + the immutable capability implementation and its stronger authority proof. +- Out of scope: changing selected-run UI ownership, server scheduling, ToolWalk + grammar, retry behavior, or live #1010 acceptance. + +## Documentation Contract + +- Feature status: implemented locally, pending review and hosted gates. +- Public docs affected: none; this is an internal native/ToolWalk ownership + policy. +- Implementation notes: durable logs and indexes record the corrected contract. + +## Test Plan (TDD) + +- First failing test: B is selected before A terminal/EOF/timeout/delayed ACK; + old Runner returns `.displaced`, so it neither observes A nor cancels timed-out + A. +- Acceptance tests: actual URLSession-gated `RunSession.submit()` + + `Runner.waitFor…` tests prove A terminal, A EOF failure, A-only timeout POST, + and delayed A acknowledgement, each with zero B action endpoints. #1136 + owns the B -> C immutable authority, revocation, and all-stream proof. +- Regression tests: existing action-owner, delayed-ACK, submission-handle, and + ToolWalk outcome suites; full Swift and repository regression gates. + +## Implementation Checklist + +- [x] Verify #1133 and stacked #1131 base. +- [x] Write and capture the gated integration red. +- [x] Keep displacement sticky while observing terminal/failure passively. +- [x] Make timeout transport-only for exact locally owned displaced A. +- [x] Update plans/logs/indexes, including the stale displaced-result wording. +- [x] Run strict format and focused/full Swift; final combined focused + `PassiveSubmissionOutcomeIntegrationTests` evidence is 10/10 (not the + intermediate #1133-only 4/4 count). +- [ ] Run full regression after the independent #1135 baseline fixture repair. +- [ ] Publish stacked draft `Closes #1133`; obtain independent cheap review. + +## Risks and Mitigations + +- Risk: passive waiting accidentally sends an A prompt/approval to selected B. + Mitigation: explicit `isDisplaced || currentRunID != A` control fence and + per-test zero-B endpoint assertions. +- Risk: A timeout does nothing because B is selected, or mutates B state. + Mitigation: narrow local-submission ownership predicate with an A-only + transport POST that does not change shared selected/transcript state. diff --git a/docs/plans/2026-08-03-issue-1136-timeout-authority-impact-map.md b/docs/plans/2026-08-03-issue-1136-timeout-authority-impact-map.md new file mode 100644 index 00000000..41528407 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1136-timeout-authority-impact-map.md @@ -0,0 +1,38 @@ +# Cross-Surface Impact Map: Issue #1136 + +## Task and ownership + +- Issue: #1136 immutable submitted-run timeout authority. +- Source of truth: a `RunSubmission` created by its owning `RunSession`. +- Search evidence: `RunSubmission`, `consumeTimeoutCancellation`, + `cancelTimedOutSubmission`, and `submissionStreamTasks` under `macapp/`. + +## Surfaces + +- Native model: private owner UUID plus reset/load generation and lifecycle + form the A-only capability. A package-visible ticket with a fileprivate + initializer prevents public callers from constructing pre-deadline authority. + ToolWalk-only submission binds the immutable duration; `markStarted` derives + the deadline. `RunSession.submissionTimeoutGate(for:)` is the only package + gate and refuses pre-deadline or duplicate minting. +- ToolWalk: consumes that ticket only through the GoCodeUI gate. Its + timeout is transport-only; it + cannot alter B/C selection, transcript, controls, or cancellation state. + `waitForTerminal` receives only a poll interval; the configured timeout is + bound before start and cannot be contradicted by a later wait argument. +- HTTP/API: unchanged existing `POST /v1/runs/{A}/cancel` endpoint only. +- Persistence, harness, TUI, schema, CLI, providers: none; search found no + changed contract or stored state. + +## Reliability, test, and rollback + +- Concurrent A SSE, B/C conversation events, delayed ACK, timeout, and reset + are scoped by immutable handle. Terminal/failure/reset/load make later A + dispatch impossible; displacement deliberately does not. +- Gated URLProtocol integration uses actual `RunSession.submit()` and proves + no ticket/action before deadline; B -> C -> A sends exactly one A cancel; + duplicate, terminal, failure, and reset consume attempts fail; zero B/C + actions; and reset stops both concurrent A/C event streams. The test-only + internal RunSession clock seam freezes the same monotonic source used by + `markStarted` and the gate, removing wall-clock sleep races. +- Rollback is the stacked native PR; no data migration or server rollback. diff --git a/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md b/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md new file mode 100644 index 00000000..9f7d581a --- /dev/null +++ b/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md @@ -0,0 +1,59 @@ +# Plan: Issue #1136 immutable timeout authority + +## Context and scope + +- Governing issue: #1136, stacked after #1133 on the #1131 native ownership + line. +- Problem: a mutable session pointer cannot prove that an A timeout remains + owned by A after B terminals and C starts. +- In scope: private A-handle capability authority, reset/load invalidation, + one-shot transport dispatch, and deterministic native proof. +- Out of scope: harness endpoints, selected-run reducer behavior, tool grammar, + and production scheduling semantics. + +## TDD and implementation + +- Red: B -> C -> A timeout lost authority when only mutable session pointers + were consulted. +- Repair: each `RunSubmission` captures a private owner token and session + generation. `RunSession` exposes no public handle-based timeout cancel. + ToolWalk binds its configured immutable `Duration` at submission; GUI uses + the parameter-free submit path. `RunSubmission.markStarted` derives the + sole absolute deadline, and `RunSession.submissionTimeoutGate(for:)` alone + mints a package-visible opaque ticket with a fileprivate constructor at that + deadline. `Runner.waitForTerminal` accepts only its poll interval: it cannot + reinterpret a timeout duration after submission. No caller supplies a + post-submit duration and no raw + transport API exists. Ticket consumption is transport-only and atomically rechecks started owner, + generation, and one-shot state; terminal, failure, reset, and load revoke it. + Reset/load cancel every live submission stream by immutable handle, including + displaced A plus selected C. +- Deterministic proof: direct capability dispatch proves exactly one A cancel + after B -> C, zero B/C actions, no cancel after terminal/failure/reset, and + physical A+C stream detachment. #1133 continues to prove ToolWalk timeout + policy and passive terminal/failure observation. + +## Status and gates + +- [x] Write red and repair the authority model. +- [x] Add deterministic capability/revocation/detachment tests. +- [x] Re-run strict format and full Swift after the final submission-bound + deadline repair: formatter passed; 252 tests in 46 suites passed. +- [x] Focused `PassiveSubmissionOutcomeIntegrationTests`: 14/14 cases on the + final stacked head (the earlier 4/4 and 8/8 counts were intermediate slices). +- [x] Run `./scripts/test-regression.sh` after the final repair: normal, race, + and coverage passed (85.5% total; zero uncovered production functions). +- [x] Capture the API-removal red: former direct handle callers no longer + compile after `cancelTimedOutSubmission(_:)` is removed. +- [x] Add deadline-ticket proof for pre-expiry absence, B -> C -> A exact-one + dispatch, terminal/failure/reset revocation, and duplicate refusal. +- [x] Correct the review-found package raw-transport bypass: GoCodeUI owns the + ticket and fileprivate transport closure, while the submission-bound gate + returns the same immutable gate to repeated callers. A test-only internal + monotonic-now seam shared by `markStarted` and the gate advances epsilon and + exact-deadline state without wall-clock sleeps, proving no ticket/action + before expiry and then one A-only ticket/dispatch; + source-surface tests forbid both raw transport and `armSubmissionTimeout`. +- [x] Run exact strict Swift format, full Swift, and full repository regression + on the final opaque-ticket head. +- [ ] Publish the separate stacked draft PR with `Closes #1136`. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index 42d79771..77816a83 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -30,6 +30,36 @@ deterministic callback retry-wait recovery fixture plan. - `2026-08-03-issue-1124-retry-wait-fixture-impact-map.md` — Cross-surface impact map for the Issue #1124 test-only repair. +- `2026-08-03-issue-1136-timeout-authority-plan.md` — Issue #1136 immutable + A-handle timeout cancellation authority across B/C selection. +- `2026-08-03-issue-1136-timeout-authority-impact-map.md` — Cross-surface map + for #1136 owner-token/generation capability and stream detachment. + +- `2026-08-03-issue-1133-passive-outcome-plan.md` — Issue #1133 passive + A-outcome observation after scheduled B displacement. +- `2026-08-03-issue-1133-passive-outcome-impact-map.md` — Cross-surface map + for #1133 submission-local outcome versus selected-run authority. + +- `2026-08-03-issue-1130-submission-outcomes-plan.md` — Issue #1130 + submission-local lifecycle and ToolWalk outcome repair. +- `2026-08-03-issue-1130-submission-outcomes-impact-map.md` — Cross-surface + map for #1130 A-local outcomes after scheduled-run displacement. + +- `2026-08-03-issue-1128-submission-handle-plan.md` — Issue #1128 immutable + composer action and submitted-run ownership for ToolWalk. +- `2026-08-03-issue-1128-submission-handle-impact-map.md` — Cross-surface map + for Issue #1128 A-only native submission evidence. + +- `2026-08-03-issue-1125-action-owner-plan.md` — Issue #1125 expected-run + fences for native Stop, Composer steer, and ToolWalk timeout. +- `2026-08-03-issue-1125-action-owner-impact-map.md` — Cross-surface map for + Issue #1125 native action ownership. + +- `2026-08-03-issue-1122-interactive-state-plan.md` — Stacked native + interactive-affordance ownership repair for #1122. +- `2026-08-03-issue-1122-interactive-state-impact-map.md` — Cross-surface + map for #1122's run-scoped pending state and guarded actions. + - `2026-08-03-issue-1007-external-run-controls-plan.md` — Issue #1007 macOS external cron/callback control-ownership rebase repair. - `2026-08-03-issue-1007-external-run-controls-impact-map.md` — Cross-surface diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index e34024e3..45b80270 100644 --- a/docs/plans/active-plan.md +++ b/docs/plans/active-plan.md @@ -29,6 +29,50 @@ early fire must not admit or mutate it, then an explicit post-deadline fire must reuse the reserved run identity exactly once. Callback manager, SQLite, API/task visibility, TUI, and native GUI behaviour remain unchanged. Final focused/package/full validation, review, hosted checks, and promotion remain. +Current status: Issue #1130 is implemented locally on #1128 `654b7da`. It +Current status: Issue #1133 is implemented locally on #1131 `63cf9fcd`. It +corrects the residual #1130 wait-policy defect: B displacement revokes A +controls but not A outcome observation. ToolWalk now passively waits for A +terminal/failure through its deadline, while a displaced timeout can POST only +the exact locally owned A without mutating selected B. Eight URLSession-gated +`RunSession.submit()` + `Runner` tests cover B-before-A terminal, EOF, timeout, +B -> C -> A timeout authority, one-shot/terminal/failure/reset revocation, +and delayed acknowledgement with zero B/C actions. Strict formatting and full +Swift pass; full repository regression awaits the independent #1135 baseline +fixture repair, followed by review, hosted checks, and stacked PR promotion. + +Historical status: Issue #1130 was implemented locally on #1128 `654b7da`. It +separates A's submission lifecycle from displacement, makes late +acknowledgement/EOF/failure identity-safe, and gives ToolWalk typed +terminal/failure/displaced/timeout outcomes so only timeout cancels A. Strict +formatting, focused suites, full Swift, and repository normal/race/coverage +pass; review, draft publication, and hosted checks remain required before +promotion. + +Current status: Issue #1128 stacks on #1127 `d2cd29cf` and replaces the last +dynamic submission identity lookup. `RunSubmission` resolves only from the +local `startRun` response, retains A-only events/transcript/terminal state, and +is displaced rather than permitted to operate selected scheduled B. Composer +captures its action mode once; ToolWalk uses the handle for every wait, +automatic interaction, timeout, and verdict. Strict format and complete Swift +tests pass; repository gate, draft publication, review, and hosted checks +remain. + +Current status: Issue #1125 stacks on #1122 `d1931ae2` to fence the remaining +native actions. Stop, Composer steer, and ToolWalk timeout carry their rendered +or decision run identity to an expected-run guard, so stale A cannot issue B's +endpoint or mutate B's state. Strict format and focused native/ToolWalk tests +pass; complete Swift/repository gates and draft handoff remain. + +Current status: Issue #1122 is a stacked native ownership repair on draft PR +#1118 head `34219699`. It stamps approval/plan affordances with their SSE +`run_id`, clears every pending interaction synchronously when visible run +ownership changes or retires, and makes Chat/ToolWalk retain that identity for +approval, plan, and answer actions. Deterministic A-to-B, terminal/fallback, +and foreign-terminal regressions are green. Exact final Swift is 222 tests / 43 +suites; full repository regression passes at 85.5% coverage with zero uncovered +functions. Independent review remains before the stacked draft is handed off. + Current status: Issue #1007 rebases external scheduled cron/callback control ownership onto main's accounting and request-generation reducer. The combined reducer selects control authority before accounting, retains terminal diff --git a/macapp/Package.swift b/macapp/Package.swift index 82f2c1fd..8bb38f37 100644 --- a/macapp/Package.swift +++ b/macapp/Package.swift @@ -25,7 +25,10 @@ let package = Package( dependencies: ["HarnessKit"], resources: [.copy("Fixtures")] ), - .testTarget(name: "GoCodeUITests", dependencies: ["GoCodeUI"]), + // RunSubmission/ToolWalk ownership integration tests need both sides + // of the public app boundary: the real `RunSession.submit()` entry + // point and ToolWalk's waiting policy. + .testTarget(name: "GoCodeUITests", dependencies: ["GoCodeUI", "ToolWalk"]), .testTarget(name: "ToolWalkTests", dependencies: ["ToolWalk"]), ] ) diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 16b5b28d..e50b5fb1 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -27,13 +27,15 @@ struct ChatView: View { selected: $selected, project: project ) - if let plan = run.transcript.pendingPlan { + if let plan = run.transcript.pendingPlan, plan.runID == run.currentRunID { PlanApprovalView(plan: plan, run: run) - } else if let prompt = run.pendingQuestions { + } else if let prompt = run.pendingQuestions, prompt.runID == run.currentRunID { AskUserView(prompt: prompt, answerInFlight: run.answerInFlight) { - run.answer($0) + run.answer($0, expectedRunID: prompt.runID) } - } else if let approval = run.transcript.pendingApproval { + } else if let approval = run.transcript.pendingApproval, + approval.runID == run.currentRunID + { ApprovalBar(approval: approval, run: run) } Composer(project: project, run: run) @@ -701,6 +703,10 @@ struct InlineRunStatus: View { @Environment(\.accessibilityReduceMotion) private var reduceMotion var body: some View { + // Capture the identity this status row rendered for. SwiftUI can keep + // this closure alive after a scheduled continuation selects a newer + // run; resolving currentRunID in the action would then stop B. + let renderedRunID = run.currentRunID MetadataRow(spacing: Spacing.standard) { ProgressView() .controlSize(.small) @@ -722,10 +728,12 @@ struct InlineRunStatus: View { .accessibilityLabel(scheduledRunStatus) } Spacer() - if run.isBusy { - Button(run.cancelInFlight ? "Stopping…" : "Stop") { run.cancel() } - .controlSize(.small) - .disabled(run.cancelInFlight) + if run.isBusy, let renderedRunID { + Button(run.cancelInFlight ? "Stopping…" : "Stop") { + run.cancel(expectedRunID: renderedRunID) + } + .controlSize(.small) + .disabled(run.cancelInFlight) } } .onChange(of: run.connectionError) { _, error in @@ -807,9 +815,12 @@ struct ApprovalBar: View { Button(showArguments ? "Hide" : "Details") { showArguments.toggle() } .buttonStyle(.plain).font(Typography.caption).foregroundStyle( Theme.foregroundTertiary) - Button("Deny") { run.deny() }.disabled(run.runControlInFlight) - Button("Allow") { run.approve() }.buttonStyle(.borderedProminent) - .disabled(run.runControlInFlight) + Button("Deny") { run.deny(expectedRunID: approval.runID) }.disabled( + run.runControlInFlight) + Button("Allow") { run.approve(expectedRunID: approval.runID) }.buttonStyle( + .borderedProminent + ) + .disabled(run.runControlInFlight) } if showArguments { ScrollView { @@ -838,6 +849,9 @@ struct Composer: View { @State private var mentionTask: Task? var body: some View { + // The same composer can remain on screen while a continuation replaces + // its active run. Send must retain the run it rendered as steering. + let action = ComposerAction.capture(canSteer: run.canSteer, runID: run.currentRunID) VStack(alignment: .leading, spacing: Spacing.standard) { if !mentions.isEmpty { MentionPopup(matches: mentions) { match in @@ -856,7 +870,7 @@ struct Composer: View { .textFieldStyle(.plain) .lineLimit(1...10) .focused($focused) - .onSubmit(send) + .onSubmit { send(action) } .onChange(of: run.draft) { _, text in updateMentions(for: text) } HStack(spacing: Spacing.comfortable) { @@ -869,7 +883,7 @@ struct Composer: View { .buttonStyle(.plain).font(Typography.caption).foregroundStyle( Theme.foregroundTertiary) - Button(action: send) { + Button(action: { send(action) }) { Image( systemName: run.canSteer ? "arrow.turn.up.right" : "arrow.up.circle.fill" @@ -881,9 +895,9 @@ struct Composer: View { } .buttonStyle(.plain) .disabled(run.draft.trimmed.isEmpty || run.runControlInFlight) - .help(run.canSteer ? "Steer the running task" : "Send") + .help(action.isSteer ? "Steer the running task" : "Send") .accessibilityLabel( - run.canSteer ? "Steer the running task" : "Send message") + action.isSteer ? "Steer the running task" : "Send message") } } .padding(.horizontal, Spacing.large).padding(.vertical, Spacing.inset) @@ -926,11 +940,43 @@ struct Composer: View { /// While a run is active the same control steers instead of queueing a /// second run, matching the TUI's mid-turn steering. - private func send() { - if run.canSteer { - run.steer() - } else if run.canSubmit { - project.submit() + private func send(_ action: ComposerAction) { + action.perform( + canSubmit: run.canSubmit, + steer: { run.steer(expectedRunID: $0) }, + submit: { project.submit() }) + } +} + +/// Captured once for a rendered composer interaction. In particular, a Send +/// closure rendered as A's steer action must remain a steer request for A if a +/// scheduled B replaces it before the click/Return handler executes. +enum ComposerAction: Equatable { + case submit + case steer(String) + + static func capture(canSteer: Bool, runID: String?) -> Self { + if canSteer, let runID { return .steer(runID) } + return .submit + } + + var isSteer: Bool { + if case .steer = self { return true } + return false + } + + /// Keeps the rendered branch immutable through a delayed button/Return + /// callback. This is intentionally a tiny pure seam so ownership can be + /// regression-tested without a SwiftUI view host. + func perform( + canSubmit: Bool, steer: (String) -> Void, submit: () -> Void + ) { + switch self { + case .steer(let runID): + steer(runID) + case .submit: + guard canSubmit else { return } + submit() } } } @@ -1109,8 +1155,9 @@ struct PlanApprovalView: View { HStack { Spacer() - Button("Keep Planning") { run.deny() }.disabled(run.runControlInFlight) - Button("Approve") { run.approve(option: selected) } + Button("Keep Planning") { run.deny(expectedRunID: plan.runID) }.disabled( + run.runControlInFlight) + Button("Approve") { run.approve(expectedRunID: plan.runID, option: selected) } .buttonStyle(.borderedProminent) // With approaches offered, one must be chosen. .disabled((!plan.options.isEmpty && selected == nil) || run.runControlInFlight) diff --git a/macapp/Sources/GoCodeUI/ProjectSession.swift b/macapp/Sources/GoCodeUI/ProjectSession.swift index c8c59d7c..e3446b8e 100644 --- a/macapp/Sources/GoCodeUI/ProjectSession.swift +++ b/macapp/Sources/GoCodeUI/ProjectSession.swift @@ -136,6 +136,18 @@ public final class ProjectSession { self.serverEnvironment = serverEnvironment } + /// Deterministic native/ToolWalk integration seam. Production callers use + /// the URL/supervisor initializer above; tests inject the same client used + /// by their URLProtocol fixture so `Runner.walk` exercises ProjectSession. + init(workspace: URL, client: HarnessClient) { + self.workspace = workspace + externalBaseURL = nil + serverEnvironment = [:] + self.client = client + run = RunSession(client: client) + phase = .ready + } + public var name: String { workspace.lastPathComponent } public var isReady: Bool { phase == .ready } @@ -329,12 +341,20 @@ public final class ProjectSession { extraDirs.removeAll { $0 == url } } - public func submit() { + @discardableResult + public func submit() -> RunSubmission? { + submit(timeoutAfter: nil) + } + + /// ToolWalk alone supplies a bounded timeout; GUI submission remains + /// deliberately parameter-free. + @discardableResult + package func submit(timeoutAfter: Duration?) -> RunSubmission? { run?.model = selectedModel run?.planMode = planMode run?.extraDirs = extraDirs.map(\.path) run?.profile = selectedProfile - run?.submit() + let submission = run?.submit(timeoutAfter: timeoutAfter) Task { // `run.submit()` starts its own unstructured task that only sets // `conversationID` once harnessd has actually minted one — a @@ -348,6 +368,7 @@ public final class ProjectSession { } await refreshConversations() } + return submission } public func openConversation(_ conversation: ConversationInfo) async { diff --git a/macapp/Sources/GoCodeUI/RunSession+Answers.swift b/macapp/Sources/GoCodeUI/RunSession+Answers.swift index 1ada9a5f..4470c56c 100644 --- a/macapp/Sources/GoCodeUI/RunSession+Answers.swift +++ b/macapp/Sources/GoCodeUI/RunSession+Answers.swift @@ -2,12 +2,13 @@ import Foundation import HarnessKit extension RunSession { - public func answer(_ answers: [String: String]) { - guard let runID = currentRunID, - let prompt = pendingQuestions, + public func answer(_ answers: [String: String], expectedRunID: String) { + guard currentRunID == expectedRunID, + let prompt = pendingQuestions, prompt.runID == expectedRunID, AskUserAnswers.isComplete(prompt: prompt, answers: answers), !answerInFlight else { return } + let runID = expectedRunID answerInFlight = true connectionError = nil @@ -20,6 +21,7 @@ extension RunSession { } } do { + guard currentRunID == runID, pendingQuestions?.runID == runID else { return } try await client.answerInput(runID: runID, answers: answers) guard currentRunID == runID, pendingQuestions?.callID == prompt.callID else { return @@ -34,4 +36,9 @@ extension RunSession { } } } + + public func answer(_ answers: [String: String]) { + guard let runID = currentRunID else { return } + answer(answers, expectedRunID: runID) + } } diff --git a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift index 35cba921..b6bd37ca 100644 --- a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift +++ b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift @@ -1,17 +1,99 @@ import Foundation import HarnessKit +/// Opaque timeout authority. Its initializer is file-private: callers cannot +/// manufacture a ticket from a submission before the owning RunSession gates +/// it at the deadline. +@MainActor +package struct TimedOutSubmissionTicket { + private let consumeTransport: @MainActor () -> Bool + + fileprivate init(consumeTransport: @escaping @MainActor () -> Bool) { + self.consumeTransport = consumeTransport + } + + @discardableResult + package func consume() -> Bool { + consumeTransport() + } +} + +@MainActor +package final class SubmissionTimeoutGate { + private weak var session: RunSession? + private let submission: RunSubmission + private let deadline: ContinuousClock.Instant + private let now: @MainActor () -> ContinuousClock.Instant + + fileprivate init( + session: RunSession, submission: RunSubmission, deadline: ContinuousClock.Instant, + now: @escaping @MainActor () -> ContinuousClock.Instant + ) { + self.session = session + self.submission = submission + self.deadline = deadline + self.now = now + } + + package func ticketIfExpired() -> TimedOutSubmissionTicket? { + guard now() >= deadline, + let session, + submission.mintTimeoutTicket( + owner: session.submissionOwnerToken, generation: session.submissionGeneration + ) + else { return nil } + return TimedOutSubmissionTicket { [weak session, submission, client = session.client] in + guard let session, + let runID = session.consumeTimeoutCancellation(for: submission) + else { return false } + Task { try? await client.cancel(runID: runID) } + return true + } + } +} + extension RunSession { /// True only while the first, cooperative cancel request awaits harnessd's /// acknowledgement. Once it succeeds, a second press remains available /// for the existing local force-stop behavior. - public var cancelInFlight: Bool { cancelState == .requesting } + public var cancelInFlight: Bool { + cancelState == .requesting + } + /// Requests cancellation only if the run that rendered the affordance is + /// still selected. A later scheduled/local continuation must never inherit + /// an old Stop button's action. + public func cancel(expectedRunID: String) { + guard currentRunID == expectedRunID else { return } + let runID = expectedRunID + cancel(runID: runID) + } + + /// The sole package boundary for timeout authority. It arms a fixed + /// deadline gate; only that gate can later mint an opaque ticket. + package func submissionTimeoutGate(for submission: RunSubmission) -> SubmissionTimeoutGate? { + let id = ObjectIdentifier(submission) + if let gate = submissionTimeoutGates[id] { return gate } + guard let deadline = submission.timeoutDeadlineIfStarted() else { return nil } + let gate = SubmissionTimeoutGate( + session: self, submission: submission, deadline: deadline, now: submissionTimeoutNow + ) + submissionTimeoutGates[id] = gate + return gate + } + + /// Compatibility entry point for programmatic callers that do not retain + /// a rendered action identity. Visible UI must use + /// `cancel(expectedRunID:)` instead. public func cancel() { guard let runID = currentRunID else { streamTask?.cancel() return } + cancel(runID: runID) + } + + private func cancel(runID: String) { switch cancelState { case .requested: // The selected continuation may be external while a distinct local @@ -44,15 +126,43 @@ extension RunSession { } } + public func approve(expectedRunID: String, option: String? = nil) { + guard currentRunID == expectedRunID, + transcript.pendingApproval?.runID == expectedRunID + || transcript.pendingPlan?.runID == expectedRunID + else { return } + let runID = expectedRunID + runControlTask(runID: runID, awaitingLifecycle: true) { [client] in + try await client.approve(runID: runID, option: option) + } + } + public func approve(option: String? = nil) { guard let runID = currentRunID else { return } + // Legacy programmatic control remains available to existing callers + // that have a run but do not render an approval affordance (for + // example diagnostic/control tests). UI and ToolWalk use the explicit + // `expectedRunID` overload above, which is the stale-closure fence. runControlTask(runID: runID, awaitingLifecycle: true) { [client] in try await client.approve(runID: runID, option: option) } } + public func deny(expectedRunID: String) { + guard currentRunID == expectedRunID, + transcript.pendingApproval?.runID == expectedRunID + || transcript.pendingPlan?.runID == expectedRunID + else { return } + let runID = expectedRunID + runControlTask(runID: runID, awaitingLifecycle: true) { [client] in + try await client.deny(runID: runID) + } + } + public func deny() { guard let runID = currentRunID else { return } + // See `approve(option:)`: this compatibility overload is not used by + // visible interaction UI, which always carries the pending run id. runControlTask(runID: runID, awaitingLifecycle: true) { [client] in try await client.deny(runID: runID) } @@ -60,20 +170,32 @@ extension RunSession { /// Redirects an in-flight run without cancelling it. Applied at the run's /// next step boundary. - public func steer() { + /// Steers only the run identity captured when the composer rendered. This + /// fence belongs before draft ownership, so a stale Send action leaves the + /// newer run and the user's draft untouched. + public func steer(expectedRunID: String) { // The guard must happen before reading/clearing the draft: keyboard // submission can invoke steer while the first POST is still awaiting // acknowledgement, and that later draft remains the user's text. - guard !runControlInFlight else { return } + guard currentRunID == expectedRunID, !runControlInFlight else { return } let originalDraft = draft let prompt = originalDraft.trimmed - guard !prompt.isEmpty, let runID = currentRunID else { return } + guard !prompt.isEmpty else { return } + let runID = expectedRunID draft = "" runControlTask(runID: runID, restoreDraft: originalDraft) { [client] in try await client.steer(runID: runID, prompt: prompt) } } + /// Compatibility entry point for non-rendered programmatic callers. The + /// composer and ToolWalk retain an explicit run id and use the overload + /// above so a stale closure cannot steer a newer run. + public func steer() { + guard let runID = currentRunID else { return } + steer(expectedRunID: runID) + } + private func runControlTask( runID: String, restoreDraft: String? = nil, @@ -88,6 +210,13 @@ extension RunSession { let lifecycleGeneration = runControlLifecycleGenerationByRunID[runID, default: 0] Task { do { + // The explicit action must still own the selected run when + // the asynchronous task begins. Otherwise stale SwiftUI + // callbacks could make a network request for a new owner. + guard currentRunID == runID else { + if runControlRequestGeneration == generation { runControlInFlight = false } + return + } try await operation() // The run stream can reach a terminal event while the daemon // is still returning this POST. A terminal clears diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index e5178d53..8d8621c7 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -80,23 +80,61 @@ public final class RunSession { /// second Stop press. A scheduled run must never cancel an unrelated local /// stream merely because it is currently selected. var localStreamRunID: String? + /// One RunSubmission stream can survive visual displacement while a later + /// C stream starts. Reset/load must detach every such local stream, not + /// only the last compatibility `streamTask`. + private var submissionStreamTasks: [ObjectIdentifier: Task] = [:] + /// The locally submitted run whose caller may need A-only lifecycle and + /// transcript evidence. A selected external continuation displaces this + /// handle; it must never cause its caller to act on the continuation. + private var activeSubmission: RunSubmission? + /// Each submission captures this unforgeable owner token and the current + /// generation. It remains independent from selected-run UI state. + let submissionOwnerToken = UUID() + /// The single monotonic source used to derive and observe submission + /// deadlines. Production uses `ContinuousClock.now`; the internal + /// initializer makes deterministic native timing tests possible without + /// exposing a clock choice to GUI or ToolWalk callers. + let submissionTimeoutNow: @MainActor () -> ContinuousClock.Instant + /// Reset/load detach the old session permanently; their generation invalidates + /// every outstanding submission timeout capability. + var submissionGeneration: UInt = 0 + var submissionTimeoutGates: [ObjectIdentifier: SubmissionTimeoutGate] = [:] public init(client: HarnessClient) { self.client = client + submissionTimeoutNow = { ContinuousClock.now } + } + + init( + client: HarnessClient, + submissionTimeoutNow: @escaping @MainActor () -> ContinuousClock.Instant + ) { + self.client = client + self.submissionTimeoutNow = submissionTimeoutNow } public convenience init(baseURL: URL, token: String? = nil) { self.init(client: HarnessClient(baseURL: baseURL, token: token)) } - public var isBusy: Bool { transcript.runState.isActive } + public var isBusy: Bool { + transcript.runState.isActive + } + /// Keyboard submission must share the composer button's single-flight /// boundary. A control POST can outlive its run's terminal SSE; allowing /// a new run during that acknowledgement would let the old completion /// mutate the newer conversation. - public var canSubmit: Bool { !draft.trimmed.isEmpty && !isBusy && !runControlInFlight } + public var canSubmit: Bool { + !draft.trimmed.isEmpty && !isBusy && !runControlInFlight + } + /// True while a run is active, so the composer can offer steering instead. - public var canSteer: Bool { isBusy && transcript.pendingApproval == nil } + public var canSteer: Bool { + isBusy && transcript.pendingApproval == nil + } + /// Accessible copy shown while a cron/callback continuation, rather than a /// prompt submitted by this app instance, owns the active controls. public var scheduledRunStatus: String? { @@ -106,9 +144,24 @@ public final class RunSession { // MARK: - Running - public func submit() { + @discardableResult + public func submit() -> RunSubmission? { + submit(timeoutAfter: nil) + } + + /// ToolWalk's bounded execution path is the sole caller permitted to bind + /// a timeout policy to a submission. GUI callers intentionally receive + /// the parameter-free overload above. + @discardableResult + package func submit(timeoutAfter: Duration?) -> RunSubmission? { let prompt = draft.trimmed - guard !prompt.isEmpty, !isBusy, !runControlInFlight else { return } + guard !prompt.isEmpty, !isBusy, !runControlInFlight else { return nil } + let submission = RunSubmission( + prompt: prompt, timeoutOwner: submissionOwnerToken, + timeoutGeneration: submissionGeneration, timeoutAfter: timeoutAfter, + timeoutNow: submissionTimeoutNow + ) + activeSubmission = submission draft = "" connectionError = nil cancelState = .idle @@ -124,7 +177,8 @@ public final class RunSession { // a brand-new conversation) forever instead of the id this run just // minted -- exactly the bug that left the conversation stream never // started for the run that most needs it. - streamTask = Task { + let submissionID = ObjectIdentifier(submission) + let task = Task { [client, model, planMode, startingConversationID = conversationID, extraDirs, profile] in var startedRunID: String? @@ -144,31 +198,66 @@ public final class RunSession { let started = try await client.startRun(request) startedRunID = started.runID - localStreamRunID = started.runID - activate(runID: started.runID, isExternal: false, timestamp: nil, select: true) - activateAccounting(for: started.runID, timestamp: nil) - if self.conversationID == nil { self.conversationID = started.runID } - // Keyed by conversation, not by this run: on a conversation's - // later runs `self.conversationID` is already the first run's - // id, so this must not retarget the stream to `started.runID`. - if let conversationID = self.conversationID { - trackConversationStream(conversationID) + // A reset/load can cancel this task while its HTTP response + // races back. The server may have admitted A, but a torn-down + // session must not revive it or let it displace whatever + // conversation the user selected next. + submission.markStarted(runID: started.runID) + // The server accepted A even if scheduled B became visible + // while the HTTP response was in flight. Bind that immutable + // identity to A's handle for truthful local outcome reporting, + // but never let the late response steal B's selection, + // accounting, or conversation stream. + if ownsVisibleSubmission(submission, runID: started.runID) { + localStreamRunID = started.runID + activate(runID: started.runID, isExternal: false, timestamp: nil, select: true) + activateAccounting(for: started.runID, timestamp: nil) + if self.conversationID == nil { self.conversationID = started.runID } + // Keyed by conversation, not by this run: on a conversation's + // later runs `self.conversationID` is already the first run's + // id, so this must not retarget the stream to `started.runID`. + if let conversationID = self.conversationID { + trackConversationStream(conversationID) + } } for try await event in client.events(runID: started.runID) { - await applyRunEvent(event, expectedRunID: started.runID) + submission.apply(event) + // Once B owns the visible conversation, A's stream is + // private evidence for the submission handle. Feeding it + // through the shared reducer would let an old A mutate + // B's lifecycle/accounting despite the selected-run fence. + if !submission.isDisplaced { + await applyRunEvent(event, expectedRunID: started.runID) + } + } + if !submission.isTerminal { + recordSubmissionFailure( + submission, + runID: started.runID, + message: "run event stream ended before a terminal event" + ) } + } catch is CancellationError { + // reset/load intentionally detaches the handle and cancels its + // stream. That is not an A transport failure. } catch let error as HarnessError { - connectionError = error.message - transcript.markFailed() + recordSubmissionFailure(submission, runID: startedRunID, message: error.message) if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } catch { - connectionError = error.localizedDescription - transcript.markFailed() + if !Task.isCancelled { + recordSubmissionFailure( + submission, runID: startedRunID, message: error.localizedDescription + ) + } if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } - finishRunIfCurrent(startedRunID: startedRunID) + finishRunIfCurrent(startedRunID: startedRunID, submission: submission) + submissionStreamTasks.removeValue(forKey: submissionID) } + streamTask = task + submissionStreamTasks[submissionID] = task + return submission } // MARK: - Conversation switching @@ -198,13 +287,14 @@ public final class RunSession { let preserveAccounting = runID != nil && runID == accountingRunID transcript.reconcile( messages: messages, preservingUsage: preserveAccounting, - preservingRunState: preservingRunState) + preservingRunState: preservingRunState + ) if !preserveAccounting { accountingRunID = nil accountingTimestamp = nil } connectionError = nil - pendingQuestions = nil + clearPendingInteractions() } /// Stops following this session's conversation. Called on a fresh @@ -221,7 +311,7 @@ public final class RunSession { conversationID = nil clearActiveRuns() connectionError = nil - pendingQuestions = nil + clearPendingInteractions() cancelState = .idle } @@ -266,8 +356,8 @@ public final class RunSession { while !Task.isCancelled { do { for try await event in client.conversationEvents( - conversationID: conversationID, lastEventID: lastEventID) - { + conversationID: conversationID, lastEventID: lastEventID + ) { lastEventID = event.id await applyConversationEvent(event, conversationID: conversationID) // A fresh app can open a durable message snapshot and then @@ -284,7 +374,8 @@ public final class RunSession { reconcilePersistedMessages( messages, retainingAccountingFor: accountingRunID, - preservingRunState: isStaleTerminal) + preservingRunState: isStaleTerminal + ) } } } catch is CancellationError { @@ -379,36 +470,44 @@ public final class RunSession { guard !terminalRunIDs.contains(event.runID) else { return false } let isLifecycleStart = event.type == .runQueued || event.type == .runStarted || event.type == .runResumed - let select: Bool - if currentRunID == nil { - // After a terminal leaves no live action target, a replay start - // still must not displace newer accounting. Timestamp-less legacy - // frames remain admissible because their order cannot be compared. - if let accountingTimestamp, let timestamp = event.timestamp { - select = timestamp >= accountingTimestamp - } else { - select = true - } - } else if isLifecycleStart, let currentRunID { - // A local `startRun` result is a provisional owner until its first - // timestamped event. Do not replace it with an older replay simply - // because the replay supplies a timestamp first. - if activeRunTimestamps[currentRunID] == nil, !externalRunIDs.contains(currentRunID) { - select = false - } else if let timestamp = event.timestamp, - let current = activeRunTimestamps[currentRunID] ?? nil - { - select = timestamp >= current + let select: Bool = + if currentRunID == nil { + // After a terminal leaves no live action target, a replay start + // still must not displace newer accounting. Timestamp-less legacy + // frames remain admissible because their order cannot be compared. + if let accountingTimestamp, let timestamp = event.timestamp { + timestamp >= accountingTimestamp + } else { + true + } + } else if isLifecycleStart, let currentRunID { + // A local `startRun` result is a provisional owner until its first + // timestamped event. Do not replace it with an older replay simply + // because the replay supplies a timestamp first. + if activeRunTimestamps[currentRunID] == nil, !externalRunIDs.contains(currentRunID) + { + false + } else if let timestamp = event.timestamp, + let current = activeRunTimestamps[currentRunID] ?? nil + { + timestamp >= current + } else { + false + } } else { - select = false + false } - } else { - select = false - } if !activeRunIDs.contains(event.runID) { activate( runID: event.runID, isExternal: event.runID != localStreamRunID, - timestamp: event.timestamp, select: select) + timestamp: event.timestamp, select: select + ) + } else if event.runID == currentRunID, let timestamp = event.timestamp { + // A locally admitted run is provisional only until its own first + // timestamped lifecycle evidence arrives. Preserve that timestamp + // so a genuinely later scheduled continuation can take visual + // ownership, while an older replay still cannot. + activeRunTimestamps[event.runID] = timestamp } else if select { selectActive(runID: event.runID) } @@ -422,9 +521,9 @@ public final class RunSession { .toolApprovalGranted, .toolApprovalDenied, .planApprovalGranted, .planApprovalDenied, .runWaitingForUser: - return true + true default: - return false + false } } @@ -438,7 +537,11 @@ public final class RunSession { private func selectActive(runID: String) { guard currentRunID != runID else { return } + if let activeSubmission, activeSubmission.runID != runID { + activeSubmission.markDisplaced() + } invalidateRequestOwnership() + clearPendingInteractions() currentRunID = runID cancelState = .idle } @@ -449,6 +552,7 @@ public final class RunSession { externalRunIDs.remove(runID) activeRunTimestamps.removeValue(forKey: runID) guard currentRunID == runID else { return } + clearPendingInteractions() // An older run may still have an active row, but it is not a valid // fallback after a newer terminal. Only a run activated no earlier // than this terminal may resume the visible lifecycle. @@ -468,9 +572,13 @@ public final class RunSession { cancelState = .idle } - private func finishRunIfCurrent(startedRunID: String?) { + private func finishRunIfCurrent(startedRunID: String?, submission: RunSubmission) { + submissionStreamTasks.removeValue(forKey: ObjectIdentifier(submission)) guard let startedRunID else { return } if localStreamRunID == startedRunID { localStreamRunID = nil } + // A late completion must never clear a newer submission solely because + // both runs have sequentially occupied this session. + if activeSubmission === submission { activeSubmission = nil } // A stream can end after an external continuation became selected. It // may retire its own run, never clear the newer selected target. if currentRunID == startedRunID { @@ -483,15 +591,48 @@ public final class RunSession { } private func clearActiveRuns() { + submissionGeneration &+= 1 + for task in submissionStreamTasks.values { + task.cancel() + } + submissionStreamTasks = [:] + submissionTimeoutGates = [:] + activeSubmission?.markDisplaced() + activeSubmission = nil activeRunIDs = [] externalRunIDs = [] activeRunTimestamps = [:] terminalRunIDs = [] currentRunID = nil + clearPendingInteractions() localStreamRunID = nil cancelState = .idle } + /// Shared visible state belongs to the exact `RunSubmission` that still + /// owns selected A. A displacement (including one before `startRun` + /// acknowledges) makes all later A errors handle-local. + private func ownsVisibleSubmission(_ submission: RunSubmission, runID: String?) -> Bool { + guard activeSubmission === submission, !submission.isDisplaced else { return false } + guard let runID else { return currentRunID == nil } + return currentRunID == nil || currentRunID == runID + } + + func consumeTimeoutCancellation(for submission: RunSubmission) -> String? { + submission.consumeTimeoutCancellation( + owner: submissionOwnerToken, generation: submissionGeneration + ) + } + + private func recordSubmissionFailure( + _ submission: RunSubmission, runID: String?, message: String + ) { + submission.markFailed(message) + guard ownsVisibleSubmission(submission, runID: runID) else { return } + connectionError = message + transcript.markFailed() + } + /// Accounting admission is the run-ownership fence. Content events from a /// foreign replay remain useful transcript history, but only the owner may /// change lifecycle or approval state. This deliberately covers more than @@ -550,11 +691,11 @@ public final class RunSession { private func admitAccounting(for event: HarnessEvent) -> Bool { let runID = event.runID guard !runID.isEmpty else { return false } - let isStart: Bool - switch event.type { - case .runQueued, .runStarted, .runResumed: isStart = true - default: isStart = false - } + let isStart = + switch event.type { + case .runQueued, .runStarted, .runResumed: true + default: false + } if accountingRunID == runID { if let timestamp = event.timestamp { accountingTimestamp = timestamp } return true @@ -566,21 +707,21 @@ public final class RunSession { activateAccounting(for: runID, timestamp: event.timestamp) return true } - let isNewer: Bool - if let timestamp = event.timestamp { - // `submit()` owns the run before its first SSE frame supplies a - // timestamp. That provisional ownership is still authoritative: - // a replay from another run must not steal it merely because it - // has a timestamp while the current owner does not yet have one. - isNewer = accountingTimestamp.map { timestamp >= $0 } ?? false - } else { - isNewer = false - } - if isStart && (accountingRunID == nil || isNewer) { + let isNewer: Bool = + if let timestamp = event.timestamp { + // `submit()` owns the run before its first SSE frame supplies a + // timestamp. That provisional ownership is still authoritative: + // a replay from another run must not steal it merely because it + // has a timestamp while the current owner does not yet have one. + accountingTimestamp.map { timestamp >= $0 } ?? false + } else { + false + } + if isStart, accountingRunID == nil || isNewer { activateAccounting(for: runID, timestamp: event.timestamp) return true } - if event.type.isTerminal && (accountingRunID == nil || isNewer) { + if event.type.isTerminal, accountingRunID == nil || isNewer { activateAccounting(for: runID, timestamp: event.timestamp) return true } @@ -620,6 +761,14 @@ public final class RunSession { acknowledgedRunControlRunID = nil } + /// Pending UI is owned by `currentRunID`, not merely by the transcript. + /// Clear synchronously at each ownership fence so the following SwiftUI + /// render cannot offer A's action against B while async fetches unwind. + private func clearPendingInteractions() { + transcript.clearPendingInteractions() + pendingQuestions = nil + } + private func advanceAcknowledgedRunControl(for event: HarnessEvent) { switch event.type { case .toolApprovalGranted, .toolApprovalDenied, @@ -637,7 +786,9 @@ public final class RunSession { } extension String { - var trimmed: String { trimmingCharacters(in: .whitespacesAndNewlines) } + var trimmed: String { + trimmingCharacters(in: .whitespacesAndNewlines) + } } extension RunState { diff --git a/macapp/Sources/GoCodeUI/RunSubmission.swift b/macapp/Sources/GoCodeUI/RunSubmission.swift new file mode 100644 index 00000000..4fb372c3 --- /dev/null +++ b/macapp/Sources/GoCodeUI/RunSubmission.swift @@ -0,0 +1,149 @@ +import Foundation +import HarnessKit + +/// The identity and evidence owned by one locally submitted prompt. +/// +/// `RunSession` has one rendered conversation lifecycle, which a scheduled +/// callback or cron continuation may legitimately replace while a local run's +/// HTTP response or SSE stream is still in flight. Callers that need to judge +/// or control *their* submission must therefore retain this handle instead of +/// consulting `RunSession.currentRunID` or its shared transcript later. +@MainActor +public final class RunSubmission { + /// Compatibility projection for clients which predate the split lifecycle + /// and displacement model. New waiting code must inspect `outcome` so an + /// A terminal/failure cannot be erased by an independently selected B. + public enum State: Equatable { + case starting + case started(String) + case terminal(String) + case failed(String) + case displaced + } + + public enum Lifecycle: Equatable { + case starting + case started(String) + case terminal(String) + case failed(String) + } + + /// The result owned by A. It intentionally does not contain displacement: + /// B selection is an authority boundary, not a rewrite of A's history. + public private(set) var lifecycle: Lifecycle = .starting + /// A compatibility projection. A terminal/failure remains visible here + /// even when `isDisplaced` is also true. + public var state: State { + switch lifecycle { + case .starting: .starting + case .started(let runID): .started(runID) + case .terminal(let runID): .terminal(runID) + case .failed(let message): .failed(message) + } + } + + public private(set) var transcript = Transcript() + /// Assigned only from a successful `startRun` response. It remains + /// available after a later stream failure or displacement so cleanup and + /// diagnostics can still name A without consulting shared session state. + private var resolvedRunID: String? + public private(set) var isDisplaced = false + /// An exact timeout capability is bound to the owning RunSession instance + /// and its reset/load generation. It is consumed at most once, rather than + /// being reconstructed from mutable selected-run state later. + private let timeoutOwner: UUID + private let timeoutGeneration: UInt + private let timeoutAfter: Duration? + private let timeoutNow: @MainActor () -> ContinuousClock.Instant + private var timeoutDeadline: ContinuousClock.Instant? + private var timeoutTicketMinted = false + private var timeoutCancellationConsumed = false + + public var runID: String? { + resolvedRunID + } + + public var failure: String? { + guard case .failed(let message) = lifecycle else { return nil } + return message + } + + public var isTerminal: Bool { + if case .terminal = lifecycle { return true } + return false + } + + init( + prompt: String, timeoutOwner: UUID, timeoutGeneration: UInt, + timeoutAfter: Duration? = nil, + timeoutNow: @escaping @MainActor () -> ContinuousClock.Instant = { ContinuousClock.now } + ) { + self.timeoutOwner = timeoutOwner + self.timeoutGeneration = timeoutGeneration + self.timeoutAfter = timeoutAfter + self.timeoutNow = timeoutNow + transcript.appendUserPrompt(prompt) + } + + /// Reducer-only construction has no session cancellation authority. + convenience init(prompt: String) { + self.init(prompt: prompt, timeoutOwner: UUID(), timeoutGeneration: 0) + } + + /// Returns A's immutable timeout cancellation capability exactly once. + /// Terminal/failure are definitive A outcomes and revoke it even if their + /// per-run task has not yet unwound. + func consumeTimeoutCancellation(owner: UUID, generation: UInt) -> String? { + guard timeoutOwner == owner, + timeoutGeneration == generation, + !timeoutCancellationConsumed, + case .started(let runID) = lifecycle + else { return nil } + timeoutCancellationConsumed = true + return runID + } + + /// The deadline gate is intentionally separate from consumption: a ticket + /// can be minted only once after its exact deadline, while terminal, + /// failure, reset, and load still revoke the captured authority before it + /// is consumed. + func mintTimeoutTicket(owner: UUID, generation: UInt) -> Bool { + guard timeoutOwner == owner, + timeoutGeneration == generation, + !timeoutTicketMinted, + case .started = lifecycle + else { return false } + timeoutTicketMinted = true + return true + } + + func markStarted(runID: String) { + guard case .starting = lifecycle else { return } + resolvedRunID = runID + if let timeoutAfter { timeoutDeadline = timeoutNow().advanced(by: timeoutAfter) } + lifecycle = .started(runID) + } + + func timeoutDeadlineIfStarted() -> ContinuousClock.Instant? { timeoutDeadline } + + func apply(_ event: HarnessEvent) { + guard runID == event.runID else { return } + transcript.apply(event) + // A's terminal lifecycle is local evidence. B selection prevents + // automatic controls, but it must never turn an actual A completion + // into a false timeout or discard its transcript for ToolWalk. + if event.type.isTerminal, !isTerminal, failure == nil { + lifecycle = .terminal(event.runID) + } + } + + func markFailed(_ message: String) { + guard !isTerminal, failure == nil else { return } + lifecycle = .failed(message) + transcript.markFailed() + } + + func markDisplaced() { + isDisplaced = true + } +} diff --git a/macapp/Sources/HarnessKit/Transcript.swift b/macapp/Sources/HarnessKit/Transcript.swift index cac60128..ca6c5a20 100644 --- a/macapp/Sources/HarnessKit/Transcript.swift +++ b/macapp/Sources/HarnessKit/Transcript.swift @@ -45,6 +45,9 @@ public struct AssistantMessage: Sendable, Hashable { /// A tool call blocked awaiting the operator's decision. public struct PendingApproval: Sendable, Hashable { + /// The exact run whose event requested this decision. A visible approval + /// must never be resolved against a later selected continuation. + public let runID: String public let callID: String public let tool: String public let arguments: String @@ -58,6 +61,8 @@ public struct PendingPlan: Sendable, Hashable { public let description: String? } + /// The exact run whose event requested this plan decision. + public let runID: String public let plan: String /// Approaches parsed out of the plan; approving sends the chosen id. public let options: [Approach] @@ -220,6 +225,7 @@ public struct Transcript: Sendable { case .toolApprovalRequired: guard let callID = payload["call_id"]?.stringValue else { break } pendingApproval = PendingApproval( + runID: event.runID, callID: callID, tool: payload["tool"]?.stringValue ?? "tool", arguments: payload["arguments"]?.stringValue ?? "") @@ -227,6 +233,7 @@ public struct Transcript: Sendable { case .planApprovalRequired: pendingPlan = PendingPlan( + runID: event.runID, plan: payload["plan"]?.stringValue ?? "", options: (payload["options"]?.arrayValue ?? []).compactMap { entry in guard let id = entry["id"]?.stringValue, @@ -266,6 +273,15 @@ public struct Transcript: Sendable { } } + /// Drops affordances belonging to a run that is no longer the visible + /// action owner. It intentionally does not change lifecycle state: a + /// terminal still renders as terminal and a newly selected lifecycle + /// frame supplies its own active state. + public mutating func clearPendingInteractions() { + pendingApproval = nil + pendingPlan = nil + } + // MARK: - Helpers private mutating func appendDelta(_ chunk: String) { diff --git a/macapp/Sources/ToolWalk/Runner.swift b/macapp/Sources/ToolWalk/Runner.swift index aca48a50..242c874d 100644 --- a/macapp/Sources/ToolWalk/Runner.swift +++ b/macapp/Sources/ToolWalk/Runner.swift @@ -2,12 +2,13 @@ import Foundation import GoCodeUI import HarnessKit -struct RunnerConfig: Sendable { +struct RunnerConfig { var timeoutPerTool: Duration var pollInterval: Duration static let `default` = RunnerConfig( - timeoutPerTool: .seconds(240), pollInterval: .milliseconds(200)) + timeoutPerTool: .seconds(240), pollInterval: .milliseconds(200) + ) } /// Drives every `ToolSpec` through the app's own `ProjectSession`/`RunSession` @@ -15,6 +16,17 @@ struct RunnerConfig: Sendable { /// conversation per tool, and judges each on its transcript. @MainActor enum Runner { + /// A wait result is deliberately richer than a boolean. A terminal A + /// which raced B selection is not a timeout, and neither displacement nor + /// failure authorizes a control request against whatever run is selected. + enum SubmissionWaitOutcome: Equatable { + case started + case terminal + case failed(String) + case displaced + case timedOut + } + static func walk( project: ProjectSession, specs: [ToolSpec], config: RunnerConfig = .default ) async -> [ToolResult] { @@ -30,64 +42,181 @@ enum Runner { project.newConversation() guard let run = project.run else { let result = ToolResult( - name: spec.name, verdict: "fail", reply: "no active RunSession on project") + name: spec.name, verdict: "fail", reply: "no active RunSession on project" + ) results.append(result) print(" FAIL (no run session)") continue } run.draft = spec.prompt - project.submit() + guard let submission = project.submit(timeoutAfter: config.timeoutPerTool) else { + let result = ToolResult( + name: spec.name, verdict: "fail", reply: "submission was not accepted" + ) + results.append(result) + print(" FAIL (submission was not accepted)") + continue + } - let finished = await waitForTerminal(run: run, config: config) - if !finished { - run.cancel() + // Capture the run this walk submission owns before its generic + // polling loop can observe a later scheduled continuation. The + // timeout is for this tool's A, never for whichever run is current + // at the deadline. + let started = await waitForStartedSubmission(submission, config: config) + guard started == .started else { + if started == .terminal { + let result = judge( + tool: spec.name, observed: observe(submission, timedOut: false) + ) + results.append(result) + print(" \(result.verdict.uppercased())") + continue + } + let result = failedResult(tool: spec.name, outcome: started) + results.append(result) + print(" FAIL (\(result.reply))") + continue + } + let finished = await waitForTerminal( + run: run, submission: submission, pollInterval: config.pollInterval + ) { ticket in + _ = ticket.consume() + } + switch finished { + case .terminal: + break + case .timedOut: // Give the cooperative cancel a moment to land before moving // on, or the next tool's newConversation() races its teardown. try? await Task.sleep(for: .seconds(1)) + case .failed, .displaced: + let result = failedResult(tool: spec.name, outcome: finished) + results.append(result) + print(" FAIL (\(result.reply))") + continue + case .started: + // `waitForTerminal` cannot return started; retain an explicit + // failure if a future implementation violates that contract. + let result = failedResult(tool: spec.name, outcome: .failed("invalid wait outcome")) + results.append(result) + print(" FAIL (\(result.reply))") + continue } - let result = judge(tool: spec.name, observed: observe(run, timedOut: !finished)) + let result = judge( + tool: spec.name, observed: observe(submission, timedOut: finished == .timedOut) + ) results.append(result) print(" \(result.verdict.uppercased())") } return results } + static func waitForStartedSubmission( + _ submission: RunSubmission, config: RunnerConfig + ) async -> SubmissionWaitOutcome { + let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) + while ContinuousClock.now < deadline { + // Displacement removes authority over the rendered session, not + // ownership of A's eventual result. In particular, B can arrive + // before A's start response: keep waiting for A's immutable id so + // the subsequent passive wait can report its terminal/failure. + switch submission.lifecycle { + case .terminal: + return .terminal + case .failed(let message): + return .failed(message) + case .starting, .started: + if submission.runID != nil { return .started } + } + try? await Task.sleep(for: config.pollInterval) + } + return .timedOut + } + /// Polls until the run reaches a terminal state, answering any pending /// question or approval exactly as the composer's own controls would. /// Without this, AskUserQuestion (and any tool a permission rule gates) /// would simply hang every walk until the timeout. - private static func waitForTerminal(run: RunSession, config: RunnerConfig) async -> Bool { - let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) - while ContinuousClock.now < deadline { + static func waitForTerminal( + run: RunSession, submission: RunSubmission, pollInterval: Duration, + onTimeout: @escaping @MainActor (TimedOutSubmissionTicket) -> Void = { _ in } + ) async -> SubmissionWaitOutcome { + while true { + let outcome = outcome(for: submission) + switch outcome { + case .terminal, .failed: return outcome + case .started, .displaced: break + case .timedOut: return .timedOut + } + if let ticket = run.submissionTimeoutGate(for: submission)?.ticketIfExpired() { + onTimeout(ticket) + return .timedOut + } + guard let runID = submission.runID else { + try? await Task.sleep(for: pollInterval) + continue + } + // Once B owns visible state, A's handle remains an observation + // source only. Do not return early: A may still terminal/fail, + // and ToolWalk must judge that exact outcome. The same guard also + // fails closed if a future selection path fails to mark the handle + // displaced: mismatched selected state never authorizes a control. + guard !submission.isDisplaced, run.currentRunID == runID else { + try? await Task.sleep(for: pollInterval) + continue + } if let prompt = run.pendingQuestions { + guard prompt.runID == runID else { return .displaced } var answers: [String: String] = [:] for question in prompt.questions { answers[question.id] = question.options?.first?.label ?? "yes" } - run.answer(answers) + run.answer(answers, expectedRunID: prompt.runID) } - if run.transcript.pendingApproval != nil { - run.approve() + if let approval = run.transcript.pendingApproval { + guard approval.runID == runID else { return .displaced } + run.approve(expectedRunID: approval.runID) } if let plan = run.transcript.pendingPlan { - run.approve(option: plan.options.first?.id) + guard plan.runID == runID else { return .displaced } + run.approve(expectedRunID: plan.runID, option: plan.options.first?.id) } - if !run.isBusy { return true } - try? await Task.sleep(for: config.pollInterval) + try? await Task.sleep(for: pollInterval) } - return false + } + + /// Lifecycle has priority over displacement. A selected B must prevent + /// further A controls, but cannot make a completed/failed A look timed + /// out to the tool-verdict layer. + static func outcome(for submission: RunSubmission) -> SubmissionWaitOutcome { + outcome(for: submission.lifecycle, isDisplaced: submission.isDisplaced) + } + + static func outcome( + for lifecycle: RunSubmission.Lifecycle, isDisplaced: Bool + ) -> SubmissionWaitOutcome { + switch lifecycle { + case .terminal: .terminal + case .failed(let message): .failed(message) + case .starting, .started: + isDisplaced ? .displaced : .started + } + } + + static func shouldCancel(for outcome: SubmissionWaitOutcome) -> Bool { + outcome == .timedOut } /// Reduces a real transcript to the primitives `judge` reasons over. - static func observe(_ run: RunSession, timedOut: Bool) -> ObservedRun { + static func observe(_ submission: RunSubmission, timedOut: Bool) -> ObservedRun { var completed: [String] = [] var blocked: [String] = [] var failed: [String] = [] var replies: [String] = [] - for item in run.transcript.items { + for item in submission.transcript.items { switch item.kind { case .toolActivity(let activity): switch activity.status { @@ -106,9 +235,27 @@ enum Runner { return ObservedRun( toolCompleted: completed, toolBlocked: blocked, toolFailed: failed, finalReply: replies.last ?? "", - runFailed: run.transcript.runState == .failed, - runCancelled: run.transcript.runState == .cancelled, - connectionError: run.connectionError, - timedOut: timedOut) + runFailed: submission.transcript.runState == .failed, + runCancelled: submission.transcript.runState == .cancelled, + connectionError: submission.failure, + timedOut: timedOut + ) + } + + private static func failedResult(tool: String, outcome: SubmissionWaitOutcome) -> ToolResult { + let reply = + switch outcome { + case .displaced: + "submission was displaced by another run; no action was sent to that run" + case .failed(let message): + "submission failed: \(message)" + case .terminal: + "submission reached terminal state before ToolWalk could observe it" + case .timedOut: + "submission timed out waiting for a terminal outcome" + case .started: + "submission did not reach a controllable started state" + } + return ToolResult(name: tool, verdict: "fail", reply: reply) } } diff --git a/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift b/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift new file mode 100644 index 00000000..a8d6df7f --- /dev/null +++ b/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift @@ -0,0 +1,737 @@ +import Foundation +import HarnessKit +import Testing + +@testable import GoCodeUI +@testable import ToolWalk + +/// Exercises `RunSession.submit()` and ToolWalk's wait policy through a real +/// URLSession transport. Gates establish the ownership boundary (B selected) +/// before the A response/stream is allowed to advance; sleeps would make these +/// exact ordering proofs flaky and could accidentally test the opposite order. +private final class PassiveOutcomeProtocol: URLProtocol, @unchecked Sendable { + struct Response { + var status = 200 + var headers = ["Content-Type": "application/json"] + var body = Data() + var neverFinishes = false + var waitForGate: String? + } + + private nonisolated(unsafe) static var handler: (@Sendable (URLRequest) -> Response)? + private nonisolated(unsafe) static var requests: [URLRequest] = [] + private nonisolated(unsafe) static var stoppedPaths: Set = [] + private nonisolated(unsafe) static var startRunResponses = 0 + private static let lock = NSLock() + private static let gateLock = NSCondition() + private nonisolated(unsafe) static var openGates: Set = [] + + static func set(_ handler: @escaping @Sendable (URLRequest) -> Response) { + lock.withLock { self.handler = handler } + } + + static func reset() { + lock.withLock { + handler = nil + requests = [] + stoppedPaths = [] + startRunResponses = 0 + } + gateLock.lock() + openGates = [] + gateLock.unlock() + } + + static func paths() -> [String] { + lock.withLock { requests.compactMap(\.url?.path) } + } + + static func stopped(_ path: String) -> Bool { + lock.withLock { stoppedPaths.contains(path) } + } + + /// URLProtocol does not guarantee `httpBody` remains materialized when the + /// request is recreated by URLSession. The integration fixture uses a + /// request-order counter rather than relying on that transport detail. + static func nextStartRunID() -> String { + lock.withLock { + startRunResponses += 1 + return startRunResponses == 1 ? "run_a" : "run_c" + } + } + + static func openGate(_ gate: String) { + gateLock.lock() + openGates.insert(gate) + gateLock.broadcast() + gateLock.unlock() + } + + override class func canInit(with _: URLRequest) -> Bool { + true + } + + override class func canonicalRequest(for request: URLRequest) -> URLRequest { + request + } + + override func startLoading() { + let request = request + let handler = Self.lock.withLock { + Self.requests.append(request) + return Self.handler + } + // The handler may advance a fixture counter protected by the same + // lock. Invoke it after recording the request, not recursively inside + // the lock, so a multi-submit ordering test cannot deadlock itself. + let response = handler?(request) ?? Response() + DispatchQueue.global().async { + if let gate = response.waitForGate { Self.waitForGate(gate) } + let http = HTTPURLResponse( + url: request.url!, statusCode: response.status, httpVersion: "HTTP/1.1", + headerFields: response.headers + )! + self.client?.urlProtocol(self, didReceive: http, cacheStoragePolicy: .notAllowed) + self.client?.urlProtocol(self, didLoad: response.body) + if !response.neverFinishes { self.client?.urlProtocolDidFinishLoading(self) } + } + } + + override func stopLoading() { + _ = Self.lock.withLock { Self.stoppedPaths.insert(request.url?.path ?? "") } + } + + private static func waitForGate(_ gate: String) { + gateLock.lock() + defer { gateLock.unlock() } + let deadline = Date().addingTimeInterval(5) + while !openGates.contains(gate), gateLock.wait(until: deadline) {} + } +} + +@Suite("ToolWalk displaced submission outcomes", .serialized) +@MainActor +struct PassiveSubmissionOutcomeIntegrationTests { + private func session() -> RunSession { + RunSession(client: client()) + } + + private func session( + submissionTimeoutNow: @escaping @MainActor () -> ContinuousClock.Instant + ) -> RunSession { + RunSession(client: client(), submissionTimeoutNow: submissionTimeoutNow) + } + + private func client() -> HarnessClient { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [PassiveOutcomeProtocol.self] + return HarnessClient( + baseURL: URL(string: "http://127.0.0.1:8897")!, + session: URLSession(configuration: configuration) + ) + } + + private func project() -> ProjectSession { + ProjectSession(workspace: URL(fileURLWithPath: "/tmp"), client: client()) + } + + private func event(_ id: String, _ runID: String, _ type: String, timestamp: String) throws + -> HarnessEvent + { + try HarnessEvent( + frame: SSEFrame( + id: id, event: type, + data: + #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)","timestamp":"\#(timestamp)","payload":{}}"# + )) + } + + private nonisolated static func terminalStream(for runID: String) -> Data { + let id = "\(runID):terminal" + return Data( + "id: \(id)\nevent: run.completed\ndata: {\"id\":\"\(id)\",\"run_id\":\"\(runID)\",\"type\":\"run.completed\",\"payload\":{}}\n\n" + .utf8) + } + + private nonisolated static func startedStream(for runID: String, timestamp: String) -> Data { + let id = "\(runID):started" + return Data( + "id: \(id)\nevent: run.started\ndata: {\"id\":\"\(id)\",\"run_id\":\"\(runID)\",\"type\":\"run.started\",\"timestamp\":\"\(timestamp)\",\"payload\":{}}\n\n" + .utf8) + } + + private func wait(timeout: Duration = .seconds(2), _ condition: () -> Bool) async throws { + let deadline = ContinuousClock.now.advanced(by: timeout) + while ContinuousClock.now < deadline { + if condition() { return } + try await Task.sleep(for: .milliseconds(10)) + } + Issue.record("timed out waiting for gated integration condition") + } + + /// The only way test code receives timeout authority mirrors production: + /// Runner mints the opaque ticket at the actual wait deadline. Holding a + /// submission before this helper returns never exposes a cancel API. + private func waitForTimeoutTicket( + _ run: RunSession, submission: RunSubmission + ) async -> (Runner.SubmissionWaitOutcome, TimedOutSubmissionTicket?) { + var ticket: TimedOutSubmissionTicket? + let outcome = await Runner.waitForTerminal( + run: run, submission: submission, + pollInterval: .milliseconds(5) + ) { ticket = $0 } + return (outcome, ticket) + } + + private func displaceA(_ session: RunSession, submission: RunSubmission) async throws { + try await wait { submission.runID == "run_a" && session.currentRunID == "run_a" } + try await session.applyConversationEvent( + event("run_a:started", "run_a", "run.started", timestamp: "2026-08-03T22:00:00Z"), + conversationID: "conversation" + ) + try await session.applyConversationEvent( + event("run_b:started", "run_b", "run.started", timestamp: "2026-08-03T22:00:01Z"), + conversationID: "conversation" + ) + #expect(session.currentRunID == "run_b") + #expect(submission.isDisplaced) + } + + private func assertNoAction(for runIDs: [String]) { + let actions = Set( + runIDs.flatMap { runID in + ["cancel", "approve", "deny", "input", "steer"].map { "/v1/runs/\(runID)/\($0)" } + } + ).intersection(PassiveOutcomeProtocol.paths()) + #expect(actions.isEmpty) + } + + @Test("B before A terminal is judged as A terminal without a B action") + func terminalAfterDisplacementRemainsObservable() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], + body: Self.terminalStream(for: "run_a"), waitForGate: "a-terminal") + case ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let now = ContinuousClock.now + let run = session(submissionTimeoutNow: { now }) + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(100))) + try await displaceA(run, submission: submission) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/events") } + let waitTask = Task { + await Runner.waitForTerminal( + run: run, submission: submission, + pollInterval: .milliseconds(5)) + } + PassiveOutcomeProtocol.openGate("a-terminal") + try await wait { submission.isTerminal } + #expect(await waitTask.value == .terminal) + #expect(run.currentRunID == "run_b") + assertNoAction(for: ["run_a", "run_b", "run_c"]) + run.reset() + } + + @Test("Runner.walk judges displaced A terminal without B control") + func walkObservesTerminalAAfterBSelection() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], + body: Self.terminalStream(for: "run_a"), waitForGate: "walk-terminal" + ) + case ("GET", "/v1/conversations/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], + body: Self.startedStream(for: "run_a", timestamp: "2026-08-03T22:00:00Z") + + Self.startedStream(for: "run_b", timestamp: "2026-08-03T22:00:01Z"), + neverFinishes: true + ) + case ("GET", "/v1/conversations"): + .init(body: Data(#"{"conversations":[]}"#.utf8)) + default: .init() + } + } + let project = project() + let task = Task { + await Runner.walk( + project: project, specs: [.init(name: "x", prompt: "A")], + config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5)) + ) + } + try await wait { project.run?.currentRunID == "run_b" } + PassiveOutcomeProtocol.openGate("walk-terminal") + let results = await task.value + #expect(results.count == 1) + #expect(results[0].verdict == "fail") + #expect(results[0].reply == "the tool 'x' was never invoked") + assertNoAction(for: ["run_b"]) + } + + @Test("Runner.walk timeout cancels only A after B then C selection") + func walkTimeoutCancelsOnlyAAfterBThenC() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("GET", "/v1/conversations/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], + body: Self.startedStream(for: "run_a", timestamp: "2026-08-03T22:00:00Z") + + Self.startedStream(for: "run_b", timestamp: "2026-08-03T22:00:01Z") + + Self.startedStream(for: "run_c", timestamp: "2026-08-03T22:00:02Z"), + neverFinishes: true + ) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + case ("GET", "/v1/conversations"): + .init(body: Data(#"{"conversations":[]}"#.utf8)) + default: .init() + } + } + let project = project() + let task = Task { + await Runner.walk( + project: project, specs: [.init(name: "x", prompt: "A")], + config: .init(timeoutPerTool: .milliseconds(80), pollInterval: .milliseconds(5)) + ) + } + try await wait { project.run?.currentRunID == "run_c" } + let results = await task.value + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel") } + #expect(results[0].reply.contains("timed out")) + #expect(project.run?.currentRunID == "run_c") + #expect(PassiveOutcomeProtocol.paths().filter { $0 == "/v1/runs/run_a/cancel" }.count == 1) + assertNoAction(for: ["run_b", "run_c"]) + } + + @Test("B before A EOF is judged as A failure without failing B") + func eofAfterDisplacementRemainsObservable() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], waitForGate: "a-eof") + case ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let now = ContinuousClock.now + let run = session(submissionTimeoutNow: { now }) + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await displaceA(run, submission: submission) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/events") } + let waitTask = Task { + await Runner.waitForTerminal( + run: run, submission: submission, + pollInterval: .milliseconds(5)) + } + PassiveOutcomeProtocol.openGate("a-eof") + try await wait { + if case .failed = submission.lifecycle { return true } + return false + } + #expect(await waitTask.value == .failed("run event stream ended before a terminal event")) + #expect(run.currentRunID == "run_b") + #expect(run.transcript.runState != .failed) + assertNoAction(for: ["run_a", "run_b", "run_c"]) + run.reset() + } + + @Test("B before A timeout cancels exactly A without changing B") + func timeoutAfterDisplacementCancelsOnlyA() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: .init() + } + } + let run = session() + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await displaceA(run, submission: submission) + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: submission) + #expect(outcome == .timedOut) + #expect(ticket?.consume() == true) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel") } + #expect(run.currentRunID == "run_b") + assertNoAction(for: ["run_b"]) + run.reset() + } + + @Test("a later local C cannot revoke timed-out displaced A cancellation") + func timeoutRetainsAOwnershipAfterBThenC() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + let runID = PassiveOutcomeProtocol.nextStartRunID() + return .init( + status: 202, + body: Data(#"{"run_id":"\#(runID)","status":"queued"}"#.utf8) + ) + case ("GET", "/v1/runs/run_a/events"), + ("GET", "/v1/runs/run_c/events"), + ("GET", "/v1/conversations/conversation/events"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + return .init(status: 204) + default: + return .init() + } + } + let run = session() + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await displaceA(run, submission: submission) + try await run.applyConversationEvent( + event("run_b:completed", "run_b", "run.completed", timestamp: "2026-08-03T22:00:02Z"), + conversationID: "conversation" + ) + #expect(run.currentRunID == nil) + + // B completion makes the shared view available, but A's per-run + // stream is deliberately still active. C must not replace A's timeout + // authority just because session-level mutable pointers now name C. + run.draft = "C" + let c = try #require(run.submit()) + try await wait { c.runID == "run_c" && run.currentRunID == "run_c" } + // Only the deadline wait may mint A's authority. C cannot replace + // that ticket even though it now owns selected shared state. + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: submission) + #expect(outcome == .timedOut) + #expect(ticket?.consume() == true) + #expect(ticket?.consume() == false) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel") } + #expect(PassiveOutcomeProtocol.paths().filter { $0 == "/v1/runs/run_a/cancel" }.count == 1) + #expect(run.currentRunID == "run_c") + assertNoAction(for: ["run_b", "run_c"]) + run.reset() + try await wait { + PassiveOutcomeProtocol.stopped("/v1/runs/run_a/events") + && PassiveOutcomeProtocol.stopped("/v1/runs/run_c/events") + } + } + + @Test("exact timeout capability sends A cancel once and never reuses it") + func timeoutCapabilityIsSingleUse() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + let run = session() + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await wait { submission.runID == "run_a" } + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: submission) + #expect(outcome == .timedOut) + #expect(ticket?.consume() == true) + #expect(ticket?.consume() == false) + try await wait { + PassiveOutcomeProtocol.paths().filter { $0 == "/v1/runs/run_a/cancel" }.count == 1 + } + run.reset() + } + + @Test("terminal or reset submission cannot retain timeout cancellation") + func terminalAndResetRevokeTimeoutCapability() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], + body: Self.terminalStream(for: "run_a"), + waitForGate: "a-terminal" + ) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + let run = session() + run.draft = "A" + let terminal = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await wait { terminal.runID == "run_a" } + PassiveOutcomeProtocol.openGate("a-terminal") + try await wait { terminal.isTerminal } + // A terminal run can never mint a deadline ticket. + #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel")) + + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_b","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_b/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_b/cancel"): + .init(status: 204) + default: + .init() + } + } + run.draft = "B" + let reset = try #require(run.submit()) + try await wait { reset.runID == "run_b" } + run.reset() + try await wait { PassiveOutcomeProtocol.stopped("/v1/runs/run_b/events") } + // Reset detaches the stream before another deadline can mint a ticket. + #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_b/cancel")) + } + + @Test("failed A submission revokes its timeout capability before dispatch") + func failedSubmissionRevokesTimeoutCapability() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + // An empty completed stream is the real RunSession failure + // path, not a manually forged lifecycle transition. + .init(headers: ["Content-Type": "text/event-stream"]) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + let run = session() + run.draft = "A" + let submission = try #require(run.submit()) + try await wait { + if case .failed = submission.lifecycle { return true } + return false + } + // A failed run can never mint a deadline ticket. + #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel")) + } + + @Test("deadline ticket is absent before expiry and reset revokes it after expiry") + func ticketCannotExistBeforeDeadlineAndResetRevokesIt() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + var now = ContinuousClock.now + let run = session(submissionTimeoutNow: { now }) + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(100))) + try await wait { submission.runID == "run_a" } + + let gateA = try #require(run.submissionTimeoutGate(for: submission)) + let gateB = try #require(run.submissionTimeoutGate(for: submission)) + #expect(gateA === gateB) + #expect(gateA.ticketIfExpired() == nil) + now = now.advanced(by: .milliseconds(99)) + #expect(gateB.ticketIfExpired() == nil) + assertNoAction(for: ["run_a", "run_b", "run_c"]) + + now = now.advanced(by: .milliseconds(1)) + let minted = try #require(gateA.ticketIfExpired()) + #expect(gateB.ticketIfExpired() == nil) + #expect(minted.consume()) + #expect(!minted.consume()) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel") } + #expect(PassiveOutcomeProtocol.paths().filter { $0 == "/v1/runs/run_a/cancel" }.count == 1) + assertNoAction(for: ["run_b", "run_c"]) + run.reset() + try await wait { PassiveOutcomeProtocol.stopped("/v1/runs/run_a/events") } + #expect(!minted.consume()) + } + + @Test("GUI submissions have no timeout gate and delayed start binds its own deadline") + func guiSubmissionHasNoTimeoutAndDeadlineStartsAtAcknowledgement() async throws { + var now = ContinuousClock.now + let delayed = RunSubmission( + prompt: "A", timeoutOwner: UUID(), timeoutGeneration: 0, + timeoutAfter: .milliseconds(60), timeoutNow: { now } + ) + #expect(delayed.timeoutDeadlineIfStarted() == nil) + now = now.advanced(by: .seconds(1)) + delayed.markStarted(runID: "run_delayed") + let deadline = try #require(delayed.timeoutDeadlineIfStarted()) + #expect(now < deadline) + + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_gui","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_gui/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let run = session() + run.draft = "GUI" + let submission = try #require(run.submit()) + try await wait { submission.runID == "run_gui" } + #expect(run.submissionTimeoutGate(for: submission) == nil) + assertNoAction(for: ["run_gui"]) + run.reset() + } + + @Test("terminal and failure after deadline revoke an already-minted ticket") + func terminalAndFailureAfterTicketRevokeTransport() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + let run = session() + run.draft = "A" + let terminal = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await wait { terminal.runID == "run_a" } + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: terminal) + #expect(outcome == .timedOut) + try terminal.apply( + event( + "run_a:terminal", "run_a", "run.completed", + timestamp: "2026-08-03T22:00:04Z" + ) + ) + let minted = try #require(ticket) + #expect(!minted.consume()) + + // Failure uses the same RunSession-owned submission path; it must + // revoke a ticket minted just before its stream reports EOF/failure. + run.reset() + run.draft = "failed A" + let failed = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await wait { failed.runID == "run_a" } + let (failureOutcome, failureTicket) = await waitForTimeoutTicket(run, submission: failed) + #expect(failureOutcome == .timedOut) + failed.markFailed("stream ended") + let mintedFailure = try #require(failureTicket) + #expect(!mintedFailure.consume()) + #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel")) + run.reset() + } + + @Test("loading another conversation revokes an already-minted A ticket") + func loadRevokesMintedTicketWithoutActingOnReplacement() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + case ("POST", "/v1/runs/run_a/cancel"): + .init(status: 204) + default: + .init() + } + } + let run = session() + run.draft = "A" + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) + try await wait { submission.runID == "run_a" } + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: submission) + #expect(outcome == .timedOut) + let minted = try #require(ticket) + + run.load(messages: [], conversationID: "replacement") + try await wait { PassiveOutcomeProtocol.stopped("/v1/runs/run_a/events") } + #expect(!minted.consume()) + #expect(run.currentRunID == nil) + #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel")) + } + + @Test("B before delayed A acknowledgement still waits for A identity") + func delayedAcknowledgementPreservesDisplacementAndObservation() async throws { + PassiveOutcomeProtocol.reset() + PassiveOutcomeProtocol.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init( + status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), + waitForGate: "a-ack") + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let run = session() + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit()) + try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs") } + try await run.applyConversationEvent( + event("run_b:started", "run_b", "run.started", timestamp: "2026-08-03T22:00:01Z"), + conversationID: "conversation" + ) + #expect(run.currentRunID == "run_b") + PassiveOutcomeProtocol.openGate("a-ack") + let started = await Runner.waitForStartedSubmission( + submission, config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5)) + ) + #expect(started == .started) + #expect(submission.runID == "run_a") + #expect(submission.isDisplaced) + #expect(run.currentRunID == "run_b") + assertNoAction(for: ["run_b"]) + run.reset() + } +} diff --git a/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift b/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift index 4fdb8959..be4548cf 100644 --- a/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift +++ b/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift @@ -5,30 +5,43 @@ import Testing @testable import GoCodeUI private final class ExternalRunControlStub: URLProtocol, @unchecked Sendable { - nonisolated(unsafe) private static var recorded: [URLRequest] = [] + private nonisolated(unsafe) static var recorded: [URLRequest] = [] private static let lock = NSLock() - static func reset() { lock.withLock { recorded = [] } } - static var requests: [URLRequest] { lock.withLock { recorded } } + static func reset() { + lock.withLock { recorded = [] } + } - override class func canInit(with request: URLRequest) -> Bool { true } - override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + static var requests: [URLRequest] { + lock.withLock { recorded } + } + + override class func canInit(with _: URLRequest) -> Bool { + true + } + + override class func canonicalRequest(for request: URLRequest) -> URLRequest { + request + } override func startLoading() { - let request = self.request + let request = request Self.lock.withLock { Self.recorded.append(request) } let path = request.url?.path ?? "" let body: Data - if path.hasSuffix("/input") && request.httpMethod == "GET" { + if path.hasSuffix("/input"), request.httpMethod == "GET" { + let runID = path.split(separator: "/").dropLast().last.map(String.init) ?? "unknown" body = Data( - #"{"run_id":"run_b","call_id":"call_b","questions":[{"question":"Continue?","options":[]}]}"# - .utf8) + #"{"run_id":"\#(runID)","call_id":"call_\#(runID)","questions":[{"question":"Continue?","options":[]}]}"# + .utf8 + ) } else { body = Data("{}".utf8) } let response = HTTPURLResponse( url: request.url!, statusCode: 200, httpVersion: "HTTP/1.1", - headerFields: ["Content-Type": "application/json"])! + headerFields: ["Content-Type": "application/json"] + )! client?.urlProtocol(self, didReceive: response, cacheStoragePolicy: .notAllowed) client?.urlProtocol(self, didLoad: body) client?.urlProtocolDidFinishLoading(self) @@ -46,19 +59,23 @@ struct RunSessionExternalControlTests { return RunSession( client: HarnessClient( baseURL: URL(string: "http://127.0.0.1:8898")!, - session: URLSession(configuration: configuration))) + session: URLSession(configuration: configuration) + ) + ) } private func event( - _ id: String, _ runID: String, _ type: String, timestamp: String? = nil + _ id: String, _ runID: String, _ type: String, timestamp: String? = nil, + payload: String = "{}" ) throws -> HarnessEvent { let timestampField = timestamp.map { ",\"timestamp\":\"\($0)\"" } ?? "" return try HarnessEvent( frame: SSEFrame( id: id, event: type, data: - #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)"\#(timestampField),"payload":{}}"# - )) + #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)"\#(timestampField),"payload":\#(payload)}"# + ) + ) } private func wait( @@ -82,23 +99,32 @@ struct RunSessionExternalControlTests { session.load(messages: [], conversationID: "conversation_a") try await session.applyConversationEvent( - event("run_a:0", "run_a", "run.started"), conversationID: "conversation_a") + event("run_a:0", "run_a", "run.started"), conversationID: "conversation_a" + ) try await session.applyConversationEvent( - event("run_b:0", "run_b", "run.started"), conversationID: "conversation_a") + event("run_b:0", "run_b", "run.started"), conversationID: "conversation_a" + ) try await session.applyConversationEvent( - event("run_a:1", "run_a", "assistant.message"), conversationID: "conversation_a") + event("run_a:1", "run_a", "assistant.message"), conversationID: "conversation_a" + ) try await session.applyConversationEvent( - event("run_a:2", "run_a", "run.completed"), conversationID: "conversation_a") + event("run_a:2", "run_a", "run.completed"), conversationID: "conversation_a" + ) #expect(session.currentRunID == "run_b") #expect(session.isBusy, "run_a terminal must not make live run_b inactive") #expect(session.scheduledRunStatus == "Scheduled run active") try await session.applyConversationEvent( - event("run_b:1", "run_b", "tool.approval_required"), - conversationID: "conversation_a") + event( + "run_b:1", "run_b", "tool.approval_required", + payload: #"{"call_id":"call_b_1","tool":"bash","arguments":"{}"}"# + ), + conversationID: "conversation_a" + ) try await session.applyConversationEvent( - event("run_b:2", "run_b", "run.waiting_for_user"), conversationID: "conversation_a") + event("run_b:2", "run_b", "run.waiting_for_user"), conversationID: "conversation_a" + ) #expect(session.currentRunID == "run_b") try await wait { session.pendingQuestions?.runID == "run_b" } @@ -111,19 +137,30 @@ struct RunSessionExternalControlTests { } #expect(session.runControlInFlight) #expect( - !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/deny") == true }) + !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/deny") == true } + ) #expect( - !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/steer") == true }) + !ExternalRunControlStub.requests.contains { $0.url?.path.hasSuffix("/steer") == true } + ) try await session.applyConversationEvent( - event("run_b:3", "run_b", "tool.approval_granted"), conversationID: "conversation_a") + event("run_b:3", "run_b", "tool.approval_granted"), conversationID: "conversation_a" + ) try await wait { !session.runControlInFlight } + try await session.applyConversationEvent( + event( + "run_b:4", "run_b", "tool.approval_required", + payload: #"{"call_id":"call_b_2","tool":"bash","arguments":"{}"}"# + ), + conversationID: "conversation_a" + ) session.deny() try await wait { ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/deny" } } try await session.applyConversationEvent( - event("run_b:4", "run_b", "tool.approval_denied"), conversationID: "conversation_a") + event("run_b:5", "run_b", "tool.approval_denied"), conversationID: "conversation_a" + ) try await wait { !session.runControlInFlight } session.answer(["0:Continue?": "yes"]) @@ -156,7 +193,8 @@ struct RunSessionExternalControlTests { session.load(messages: [], conversationID: "conversation_b") try await session.applyConversationEvent( - event("run_old:0", "run_old", "run.started"), conversationID: "conversation_a") + event("run_old:0", "run_old", "run.started"), conversationID: "conversation_a" + ) #expect(session.currentRunID == nil) #expect(session.transcript.runState == .completed) @@ -176,10 +214,12 @@ struct RunSessionExternalControlTests { session.load(messages: [], conversationID: "conversation_terminal") try await session.applyConversationEvent( event("run_terminal:0", "run_terminal", "run.started"), - conversationID: "conversation_terminal") + conversationID: "conversation_terminal" + ) try await session.applyConversationEvent( event("run_terminal:1", "run_terminal", type), - conversationID: "conversation_terminal") + conversationID: "conversation_terminal" + ) #expect(session.currentRunID == nil) #expect(session.transcript.runState == expectedState) session.reset() @@ -191,12 +231,14 @@ struct RunSessionExternalControlTests { let session = makeSession() session.load(messages: [], conversationID: "conversation_tombstone") try await session.applyConversationEvent( - event("run_old:0", "run_old", "run.started"), conversationID: "conversation_tombstone") + event("run_old:0", "run_old", "run.started"), conversationID: "conversation_tombstone" + ) try await session.applyConversationEvent( event("run_old:1", "run_old", "run.completed"), conversationID: "conversation_tombstone" ) try await session.applyConversationEvent( - event("run_old:2", "run_old", "run.started"), conversationID: "conversation_tombstone") + event("run_old:2", "run_old", "run.started"), conversationID: "conversation_tombstone" + ) #expect(session.currentRunID == nil) #expect(!session.isBusy) session.reset() @@ -207,10 +249,12 @@ struct RunSessionExternalControlTests { let session = makeSession() session.load(messages: [], conversationID: "conversation_resume") try await session.applyConversationEvent( - event("run_old:0", "run_old", "run.completed"), conversationID: "conversation_resume") + event("run_old:0", "run_old", "run.completed"), conversationID: "conversation_resume" + ) try await session.applyConversationEvent( event("run_scheduled:0", "run_scheduled", "assistant.message"), - conversationID: "conversation_resume") + conversationID: "conversation_resume" + ) #expect(session.currentRunID == "run_scheduled") #expect(session.isBusy) #expect(session.scheduledRunStatus == "Scheduled run active") @@ -223,12 +267,261 @@ struct RunSessionExternalControlTests { session.load(messages: [], conversationID: "conversation_ordering") try await session.applyConversationEvent( event("run_new:0", "run_new", "run.started", timestamp: "2026-08-03T18:00:02Z"), - conversationID: "conversation_ordering") + conversationID: "conversation_ordering" + ) try await session.applyConversationEvent( event("run_old:0", "run_old", "run.started", timestamp: "2026-08-03T18:00:01Z"), - conversationID: "conversation_ordering") + conversationID: "conversation_ordering" + ) #expect(session.currentRunID == "run_new") #expect(session.accountingRunID == "run_new") session.reset() } + + /// A newer selected continuation cannot inherit approval UI from the + /// displaced run. The visible button must retain A's origin and never + /// authorize B. + @Test("newer run clears displaced approval before it can target B") + func newerRunClearsDisplacedApproval() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_approval_owner") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_approval_owner" + ) + try await session.applyConversationEvent( + event( + "run_a:1", "run_a", "tool.approval_required", + payload: #"{"call_id":"call_a","tool":"bash","arguments":"{}"}"# + ), + conversationID: "conversation_approval_owner" + ) + let approval = try #require(session.transcript.pendingApproval) + #expect(approval.runID == "run_a") + + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_approval_owner" + ) + + #expect(session.currentRunID == "run_b") + #expect(session.transcript.pendingApproval == nil) + session.approve(expectedRunID: approval.runID) + try await Task.sleep(for: .milliseconds(50)) + #expect( + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/approve" } + ) + session.reset() + } + + @Test("newer run clears displaced plan and input") + func newerRunClearsDisplacedPlanAndInput() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_plan_input_owner") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_plan_input_owner" + ) + try await session.applyConversationEvent( + event( + "run_a:1", "run_a", "plan.approval_required", + payload: #"{"plan":"A plan","options":[]}"# + ), + conversationID: "conversation_plan_input_owner" + ) + try await session.applyConversationEvent( + event("run_a:2", "run_a", "run.waiting_for_user"), + conversationID: "conversation_plan_input_owner" + ) + try await wait { session.pendingQuestions?.runID == "run_a" } + let plan = try #require(session.transcript.pendingPlan) + let prompt = try #require(session.pendingQuestions) + #expect(plan.runID == "run_a") + + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_plan_input_owner" + ) + + #expect(session.currentRunID == "run_b") + #expect(session.transcript.pendingPlan == nil) + #expect(session.pendingQuestions == nil) + session.approve(expectedRunID: plan.runID) + session.answer(["0:Continue?": "yes"], expectedRunID: prompt.runID) + try await Task.sleep(for: .milliseconds(50)) + let bPaths = ExternalRunControlStub.requests.compactMap(\.url?.path) + #expect(!bPaths.contains("/v1/runs/run_b/approve")) + #expect(!bPaths.contains("/v1/runs/run_b/input")) + session.reset() + } + + @Test("selected terminal clears every interaction when no fallback exists") + func selectedTerminalClearsInteractionState() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_terminal_owner") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started"), conversationID: "conversation_terminal_owner" + ) + try await session.applyConversationEvent( + event( + "run_a:1", "run_a", "tool.approval_required", + payload: #"{"call_id":"call_a","tool":"bash","arguments":"{}"}"# + ), + conversationID: "conversation_terminal_owner" + ) + try await session.applyConversationEvent( + event("run_a:2", "run_a", "run.waiting_for_user"), + conversationID: "conversation_terminal_owner" + ) + try await wait { session.pendingQuestions?.runID == "run_a" } + + try await session.applyConversationEvent( + event("run_a:3", "run_a", "run.completed"), + conversationID: "conversation_terminal_owner" + ) + + #expect(session.currentRunID == nil) + #expect(session.transcript.pendingApproval == nil) + #expect(session.transcript.pendingPlan == nil) + #expect(session.pendingQuestions == nil) + session.reset() + } + + @Test("selected terminal clears interactions before selecting an active fallback") + func selectedTerminalFallbackClearsInteractionState() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_terminal_fallback_owner") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started"), + conversationID: "conversation_terminal_fallback_owner" + ) + try await session.applyConversationEvent( + event( + "run_a:1", "run_a", "plan.approval_required", + payload: #"{"plan":"A plan","options":[]}"# + ), + conversationID: "conversation_terminal_fallback_owner" + ) + // B is active but cannot take selection until A ends because neither + // lifecycle frame supplies a comparable timestamp. + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started"), + conversationID: "conversation_terminal_fallback_owner" + ) + #expect(session.currentRunID == "run_a") + + try await session.applyConversationEvent( + event("run_a:2", "run_a", "run.completed"), + conversationID: "conversation_terminal_fallback_owner" + ) + + #expect(session.currentRunID == "run_b") + #expect(session.transcript.pendingApproval == nil) + #expect(session.transcript.pendingPlan == nil) + #expect(session.pendingQuestions == nil) + session.reset() + } + + @Test("foreign terminal preserves selected run interaction state") + func foreignTerminalPreservesSelectedInteractionState() async throws { + let session = makeSession() + session.load(messages: [], conversationID: "conversation_foreign_terminal") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_foreign_terminal" + ) + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_foreign_terminal" + ) + try await session.applyConversationEvent( + event( + "run_b:1", "run_b", "tool.approval_required", + payload: #"{"call_id":"call_b","tool":"bash","arguments":"{}"}"# + ), + conversationID: "conversation_foreign_terminal" + ) + try await session.applyConversationEvent( + event("run_a:1", "run_a", "run.completed"), + conversationID: "conversation_foreign_terminal" + ) + + #expect(session.currentRunID == "run_b") + #expect(session.transcript.pendingApproval?.runID == "run_b") + session.reset() + } + + @Test("a stale Stop captured for A cannot cancel or force-stop newer B") + func staleStopDoesNotTargetNewerRun() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_stale_stop") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_stale_stop" + ) + let renderedRunID = try #require(session.currentRunID) + // First press is a real A cancel and receives its acknowledgement. + // The retained second-press closure is the force-stop hazard: it must + // not locally abandon B after a continuation selects it. + session.cancel(expectedRunID: renderedRunID) + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_a/cancel" } + } + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_stale_stop" + ) + #expect(session.currentRunID == "run_b") + + session.cancel(expectedRunID: renderedRunID) + try await Task.sleep(for: .milliseconds(50)) + #expect( + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/cancel" } + ) + #expect(session.currentRunID == "run_b") + #expect(session.transcript.runState != .cancelled) + + session.cancel(expectedRunID: "run_b") + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/cancel" } + } + #expect(session.connectionError == nil) + session.reset() + } + + @Test("a stale Composer steer captured for A leaves newer B and its draft untouched") + func staleSteerDoesNotTargetNewerRun() async throws { + ExternalRunControlStub.reset() + let session = makeSession() + session.load(messages: [], conversationID: "conversation_stale_steer") + try await session.applyConversationEvent( + event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), + conversationID: "conversation_stale_steer" + ) + let renderedRunID = try #require(session.currentRunID) + session.draft = "keep watching" + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), + conversationID: "conversation_stale_steer" + ) + #expect(session.currentRunID == "run_b") + + session.steer(expectedRunID: renderedRunID) + try await Task.sleep(for: .milliseconds(50)) + #expect( + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/steer" } + ) + #expect(session.draft == "keep watching") + #expect(session.currentRunID == "run_b") + + session.steer(expectedRunID: "run_b") + try await wait { + ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/steer" } + } + #expect(session.draft.isEmpty) + session.reset() + } } diff --git a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift new file mode 100644 index 00000000..6f251dd4 --- /dev/null +++ b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift @@ -0,0 +1,525 @@ +import Foundation +import HarnessKit +import Testing + +@testable import GoCodeUI + +private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { + struct Response { + var status = 200 + var headers = ["Content-Type": "application/json"] + var body = Data() + var delay: TimeInterval = 0 + var neverFinishes = false + /// The test opens this only after it has established the ownership + /// boundary it needs to race. It is deliberately an application-state + /// barrier, not a sleep. + var waitForGate: String? + } + + private nonisolated(unsafe) static var handler: (@Sendable (URLRequest) -> Response)? + private nonisolated(unsafe) static var requests: [URLRequest] = [] + private nonisolated(unsafe) static var completedPaths: Set = [] + private static let lock = NSLock() + private static let gateLock = NSCondition() + private nonisolated(unsafe) static var openGates: Set = [] + + static func set(_ handler: @escaping @Sendable (URLRequest) -> Response) { + lock.withLock { self.handler = handler } + } + + static func reset() { + lock.withLock { + handler = nil + requests = [] + completedPaths = [] + } + gateLock.lock() + openGates = [] + gateLock.unlock() + } + + static func paths() -> [String] { + lock.withLock { requests.compactMap(\.url?.path) } + } + + static func completed(_ path: String) -> Bool { + lock.withLock { completedPaths.contains(path) } + } + + static func openGate(_ gate: String) { + gateLock.lock() + openGates.insert(gate) + gateLock.broadcast() + gateLock.unlock() + } + + override class func canInit(with _: URLRequest) -> Bool { + true + } + + override class func canonicalRequest(for request: URLRequest) -> URLRequest { + request + } + + override func startLoading() { + let request = request + let response = Self.lock.withLock { + Self.requests.append(request) + return Self.handler?(request) ?? Response() + } + DispatchQueue.global().async { + if let gate = response.waitForGate { Self.waitForGate(gate) } + if response.delay > 0 { Thread.sleep(forTimeInterval: response.delay) } + self.deliver(response, request: request) + } + } + + private func deliver(_ response: Response, request: URLRequest) { + let http = HTTPURLResponse( + url: request.url!, statusCode: response.status, httpVersion: "HTTP/1.1", + headerFields: response.headers + )! + client?.urlProtocol(self, didReceive: http, cacheStoragePolicy: .notAllowed) + client?.urlProtocol(self, didLoad: response.body) + if !response.neverFinishes { client?.urlProtocolDidFinishLoading(self) } + _ = Self.lock.withLock { Self.completedPaths.insert(request.url?.path ?? "") } + } + + override func stopLoading() {} + + private static func waitForGate(_ gate: String) { + gateLock.lock() + defer { gateLock.unlock() } + let deadline = Date().addingTimeInterval(5) + while !openGates.contains(gate), gateLock.wait(until: deadline) {} + } +} + +@Suite("RunSubmission ownership", .serialized) +@MainActor +struct RunSubmissionTests { + private func makeSession() -> RunSession { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [SubmissionHandleStub.self] + return RunSession( + client: HarnessClient( + baseURL: URL(string: "http://127.0.0.1:8896")!, + session: URLSession(configuration: configuration) + ) + ) + } + + private func event(_ id: String, _ runID: String, _ type: String, payload: String = "{}") throws + -> HarnessEvent + { + try HarnessEvent( + frame: SSEFrame( + id: id, event: type, + data: + #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)","payload":\#(payload)}"# + ) + ) + } + + private func wait(timeout: Duration = .seconds(2), for condition: () -> Bool) async throws { + let deadline = ContinuousClock.now.advanced(by: timeout) + while ContinuousClock.now < deadline { + if condition() { return } + try await Task.sleep(for: .milliseconds(10)) + } + Issue.record("timed out waiting for condition") + } + + @Test("a captured steer action never falls through to a later submit") + func capturedSteerNeverSubmits() { + let action = ComposerAction.capture(canSteer: true, runID: "run_a") + var steeredRunID: String? + var submitted = false + + action.perform( + canSubmit: true, + steer: { steeredRunID = $0 }, + submit: { submitted = true } + ) + + #expect(steeredRunID == "run_a") + #expect(!submitted) + } + + @Test("a scheduled B before A start response never becomes A's handle") + func scheduledRunBeforeStartResponseDoesNotBecomeSubmission() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init( + status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), + delay: 0.15 + ) + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + + try await wait { SubmissionHandleStub.paths().contains("/v1/runs") } + + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started"), conversationID: "conversation" + ) + #expect(session.currentRunID == "run_b") + + try await wait { submission.runID == "run_a" } + #expect(submission.runID == "run_a") + #expect(submission.state == .started("run_a")) + let bActionPaths = Set(SubmissionHandleStub.paths()).intersection([ + "/v1/runs/run_b/cancel", "/v1/runs/run_b/approve", "/v1/runs/run_b/deny", + "/v1/runs/run_b/input", "/v1/runs/run_b/steer", + ]) + #expect(bActionPaths.isEmpty) + session.reset() + } + + @Test("a B that replaces captured A displaces A without a B cancel") + func replacementAfterCaptureAbortsSubmissionWithoutBAction() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { submission.runID == "run_a" && session.currentRunID == "run_a" } + + try await session.applyConversationEvent( + event("run_b:0", "run_b", "run.started"), conversationID: "conversation" + ) + #expect( + session.currentRunID == "run_a", "timestamp-less B cannot displace provisional local A" + ) + try await session.applyConversationEvent( + event("run_b:1", "run_b", "run.started"), conversationID: "conversation" + ) + // A local run remains provisional until timestamped evidence. Model an + // authoritative scheduled continuation by using a timestamped B frame. + let earlyB = try HarnessEvent( + frame: SSEFrame( + id: "run_b:2", event: "run.started", + data: + #"{"id":"run_b:2","run_id":"run_b","type":"run.started","timestamp":"2026-08-03T20:00:00Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(earlyB, conversationID: "conversation") + #expect(session.currentRunID == "run_a", "provisional local A intentionally resists replay") + + // A current local A transitions out of provisional ownership when it + // receives its own timestamped lifecycle evidence; only a later B is + // permitted to replace it. + let startedA = try HarnessEvent( + frame: SSEFrame( + id: "run_a:0", event: "run.started", + data: + #"{"id":"run_a:0","run_id":"run_a","type":"run.started","timestamp":"2026-08-03T20:00:01Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(startedA, conversationID: "conversation") + let laterB = try HarnessEvent( + frame: SSEFrame( + id: "run_b:3", event: "run.started", + data: + #"{"id":"run_b:3","run_id":"run_b","type":"run.started","timestamp":"2026-08-03T20:00:02Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(laterB, conversationID: "conversation") + #expect(session.currentRunID == "run_b") + #expect(submission.isDisplaced) + + // A's late terminal remains useful A-only evidence. It coexists with + // displacement so ToolWalk can judge A and still never control B. + try submission.apply(event("run_a:late", "run_a", "run.completed")) + #expect(submission.isDisplaced) + #expect(submission.isTerminal) + #expect(submission.state == .terminal("run_a")) + + // A submission handle alone is intentionally not cancellation + // authority. Only ToolWalk's deadline-minted opaque ticket can make + // the transport request, so this terminal A observation cannot act on + // selected B. + #expect(!SubmissionHandleStub.paths().contains("/v1/runs/run_b/cancel")) + session.reset() + } + + @Test("A-only handle retains A terminal transcript and never reads B") + func submissionRetainsItsOwnTerminalTranscript() throws { + let submission = RunSubmission(prompt: "run A") + submission.markStarted(runID: "run_a") + try submission.apply( + event( + "run_a:0", "run_a", "assistant.message", payload: #"{"content":"A replied"}"# + ) + ) + try submission.apply(event("run_a:1", "run_a", "run.completed")) + // B's event cannot enter A's handle because `apply` is run-id scoped. + try submission.apply( + event( + "run_b:0", "run_b", "assistant.message", payload: #"{"content":"B replied"}"# + ) + ) + #expect(submission.runID == "run_a") + #expect(submission.transcript.runState == .completed) + #expect( + submission.transcript.items.contains { item in + if case .assistantMessage(let message) = item.kind { + return message.text == "A replied" + } + return false + } + ) + } + + @Test("start failure and reset leave no resurrected submission") + func startFailureAndResetAreSubmissionLocal() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + guard request.httpMethod == "POST", request.url?.path == "/v1/runs" else { + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + } + return .init(status: 503, body: Data(#"{"error":"unavailable"}"#.utf8)) + } + let failedSession = makeSession() + failedSession.draft = "fail" + let failed = try #require(failedSession.submit()) + try await wait { failed.failure != nil } + #expect(failed.runID == nil) + #expect(failedSession.currentRunID == nil) + + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init( + status: 202, body: Data(#"{"run_id":"late_a","status":"queued"}"#.utf8), + delay: 0.15 + ) + default: + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + } + } + let resetSession = makeSession() + resetSession.draft = "reset" + let reset = try #require(resetSession.submit()) + resetSession.reset() + try await wait { reset.isDisplaced } + try await Task.sleep(for: .milliseconds(200)) + #expect(reset.runID == nil) + #expect(resetSession.currentRunID == nil) + } + + @Test("late A acknowledgement binds A but cannot replace selected scheduled B") + func lateAcknowledgementDoesNotReactivateDisplacedSubmission() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init( + status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), + waitForGate: "release_a_ack" + ) + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { SubmissionHandleStub.paths().contains("/v1/runs") } + + let scheduledB = try HarnessEvent( + frame: SSEFrame( + id: "run_b:0", event: "run.started", + data: + #"{"id":"run_b:0","run_id":"run_b","type":"run.started","timestamp":"2026-08-03T21:00:00Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(scheduledB, conversationID: "conversation") + #expect(session.currentRunID == "run_b") + SubmissionHandleStub.openGate("release_a_ack") + try await wait { submission.runID == "run_a" } + + #expect(submission.runID == "run_a") + #expect(submission.state == .started("run_a")) + #expect(submission.isDisplaced) + #expect(session.currentRunID == "run_b") + #expect(session.scheduledRunStatus == "Scheduled run active") + session.reset() + } + + @Test("A start failure after B selection remains A-local") + func lateStartFailureCannotFailSelectedB() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + guard request.httpMethod == "POST", request.url?.path == "/v1/runs" else { + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + } + return .init( + status: 503, body: Data(#"{"error":"A unavailable"}"#.utf8), waitForGate: "fail_a" + ) + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { SubmissionHandleStub.paths().contains("/v1/runs") } + let scheduledB = try HarnessEvent( + frame: SSEFrame( + id: "run_b:0", event: "run.started", + data: + #"{"id":"run_b:0","run_id":"run_b","type":"run.started","timestamp":"2026-08-03T21:00:00Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(scheduledB, conversationID: "conversation") + #expect(session.currentRunID == "run_b") + SubmissionHandleStub.openGate("fail_a") + try await wait { submission.failure != nil } + + #expect(submission.failure?.contains("A unavailable") == true) + #expect(submission.isDisplaced) + #expect(session.currentRunID == "run_b") + #expect(session.transcript.runState != .failed) + #expect(session.connectionError == nil) + session.reset() + } + + @Test("A stream EOF after B selection fails A without failing B") + func eofAfterDisplacementIsSubmissionLocal() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], waitForGate: "eof_a") + case ("GET", "/v1/conversations/conversation/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { submission.runID == "run_a" && session.currentRunID == "run_a" } + let startedA = try HarnessEvent( + frame: SSEFrame( + id: "run_a:0", event: "run.started", + data: + #"{"id":"run_a:0","run_id":"run_a","type":"run.started","timestamp":"2026-08-03T21:00:00Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(startedA, conversationID: "conversation") + let scheduledB = try HarnessEvent( + frame: SSEFrame( + id: "run_b:0", event: "run.started", + data: + #"{"id":"run_b:0","run_id":"run_b","type":"run.started","timestamp":"2026-08-03T21:00:01Z","payload":{}}"# + ) + ) + await session.applyConversationEvent(scheduledB, conversationID: "conversation") + #expect(session.currentRunID == "run_b") + SubmissionHandleStub.openGate("eof_a") + try await wait { submission.failure != nil } + + #expect(submission.failure == "run event stream ended before a terminal event") + #expect(session.currentRunID == "run_b") + #expect(session.transcript.runState != .failed) + session.reset() + } + + @Test("owned stream EOF fails the visible A transcript") + func ownedEOFMarksVisibleATerminalFailure() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + switch (request.httpMethod, request.url?.path) { + case ("POST", "/v1/runs"): + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + .init( + headers: ["Content-Type": "text/event-stream"], waitForGate: "owned_eof" + ) + case ("GET", "/v1/conversations/run_a/events"): + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() + } + } + let session = makeSession() + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { submission.runID == "run_a" && session.currentRunID == "run_a" } + SubmissionHandleStub.openGate("owned_eof") + try await wait { submission.failure != nil } + + #expect(submission.failure == "run event stream ended before a terminal event") + #expect(session.currentRunID == nil, "EOF retires terminal A after recording its failure") + #expect(session.transcript.runState == .failed) + session.reset() + } + + @Test("load detaches a delayed A acknowledgement from the replacement conversation") + func loadDetachesLateAcknowledgement() async throws { + SubmissionHandleStub.reset() + SubmissionHandleStub.set { request in + guard request.httpMethod == "POST", request.url?.path == "/v1/runs" else { + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + } + return .init( + status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), + waitForGate: "late_a_after_load" + ) + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation_a") + session.draft = "run A" + let submission = try #require(session.submit()) + try await wait { SubmissionHandleStub.paths().contains("/v1/runs") } + + // This is the application ownership barrier: the new conversation is + // installed before the old A HTTP acknowledgement is released. + session.load(messages: [], conversationID: "conversation_b") + #expect(submission.isDisplaced) + #expect(session.conversationID == "conversation_b") + SubmissionHandleStub.openGate("late_a_after_load") + try await wait { SubmissionHandleStub.completed("/v1/runs") } + + #expect(session.conversationID == "conversation_b") + #expect(session.currentRunID == nil) + #expect(session.transcript.runState != .failed) + session.reset() + } + + @Test("timeout transport has no raw submission-handle API") + func timeoutTransportSurfaceCannotDriftBackToRawHandle() throws { + let macappRoot = URL(fileURLWithPath: #filePath) + .deletingLastPathComponent() + .deletingLastPathComponent() + .deletingLastPathComponent() + let controls = try String( + contentsOf: macappRoot.appending(path: "Sources/GoCodeUI/RunSession+RunControls.swift") + ) + #expect(!controls.contains("consumeTimedOutSubmissionTransport")) + #expect(!controls.contains("armSubmissionTimeout")) + #expect(controls.contains("submissionTimeoutGate")) + } +} diff --git a/macapp/Tests/HarnessKitTests/TranscriptTests.swift b/macapp/Tests/HarnessKitTests/TranscriptTests.swift index 791fc126..7f4bebe5 100644 --- a/macapp/Tests/HarnessKitTests/TranscriptTests.swift +++ b/macapp/Tests/HarnessKitTests/TranscriptTests.swift @@ -185,6 +185,7 @@ struct TranscriptTests { event( .toolApprovalRequired, ["call_id": "a", "tool": "bash", "arguments": "{}"])) + #expect(transcript.pendingApproval?.runID == "run_t") #expect(transcript.pendingApproval?.tool == "bash") #expect(transcript.runState == .waitingForUser) @@ -210,6 +211,7 @@ struct TranscriptTests { ])) let plan = transcript.pendingPlan + #expect(plan?.runID == "run_t") #expect(plan?.plan.contains("Do the thing") == true) #expect(plan?.options.map(\.id) == ["a", "b"]) #expect(plan?.options.first?.description == "Smaller steps") diff --git a/macapp/Tests/ToolWalkTests/SubmissionOutcomeTests.swift b/macapp/Tests/ToolWalkTests/SubmissionOutcomeTests.swift new file mode 100644 index 00000000..ef8e7a5d --- /dev/null +++ b/macapp/Tests/ToolWalkTests/SubmissionOutcomeTests.swift @@ -0,0 +1,30 @@ +import GoCodeUI +import Testing + +@testable import ToolWalk + +@Suite("ToolWalk submission wait outcomes") +@MainActor +struct SubmissionOutcomeTests { + @Test("terminal A wins over later B displacement") + func terminalPrecedesDisplacement() { + #expect( + Runner.outcome(for: .terminal("run_a"), isDisplaced: true) == .terminal) + } + + @Test("failed A wins over later B displacement") + func failurePrecedesDisplacement() { + #expect( + Runner.outcome(for: .failed("A stream ended"), isDisplaced: true) + == .failed("A stream ended")) + } + + @Test("only a genuine timeout permits guarded cancellation") + func onlyTimeoutCancels() { + #expect(Runner.shouldCancel(for: .timedOut)) + #expect(!Runner.shouldCancel(for: .terminal)) + #expect(!Runner.shouldCancel(for: .failed("A failed"))) + #expect(!Runner.shouldCancel(for: .displaced)) + #expect(!Runner.shouldCancel(for: .started)) + } +}