From 8d8301aa0b4d194c3cfcf8f920aed7a655b3bad4 Mon Sep 17 00:00:00 2001 From: Dennison Date: Mon, 3 Aug 2026 19:58:54 +0200 Subject: [PATCH 1/4] fix(macapp): bind ToolWalk to submitted run identity --- docs/logs/INDEX.md | 3 + docs/logs/engineering-log.md | 16 ++ docs/logs/long-term-thinking-log.md | 11 + docs/logs/observational-log.md | 11 + docs/logs/system-log.md | 10 + ...issue-1128-submission-handle-impact-map.md | 33 +++ ...08-03-issue-1128-submission-handle-plan.md | 41 +++ docs/plans/INDEX.md | 5 + docs/plans/active-plan.md | 9 + macapp/Sources/GoCodeUI/ChatView.swift | 53 +++- macapp/Sources/GoCodeUI/ProjectSession.swift | 6 +- macapp/Sources/GoCodeUI/RunSession.swift | 38 ++- macapp/Sources/GoCodeUI/RunSubmission.swift | 75 +++++ macapp/Sources/ToolWalk/Runner.swift | 89 ++++-- .../GoCodeUITests/RunSubmissionTests.swift | 267 ++++++++++++++++++ 15 files changed, 635 insertions(+), 32 deletions(-) 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 macapp/Sources/GoCodeUI/RunSubmission.swift create mode 100644 macapp/Tests/GoCodeUITests/RunSubmissionTests.swift diff --git a/docs/logs/INDEX.md b/docs/logs/INDEX.md index 6612677b..d439e815 100644 --- a/docs/logs/INDEX.md +++ b/docs/logs/INDEX.md @@ -22,6 +22,9 @@ - 2026-08-03 — Issue #1124 deterministic retry-wait callback fixture evidence is recorded in the engineering, observational, and system 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. diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index bf8c24a6..9f2f224d 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 #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 diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 59e8754e..698ba6f5 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -38,6 +38,17 @@ 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 #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 diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 200ab63b..1e26b45b 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 #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 diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index ac86b0a9..df90a6a0 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -38,6 +38,16 @@ 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 #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 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/INDEX.md b/docs/plans/INDEX.md index c138333c..deccb912 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-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 diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index d2d2dcfa..a68e32ad 100644 --- a/docs/plans/active-plan.md +++ b/docs/plans/active-plan.md @@ -29,6 +29,15 @@ 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 #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 diff --git a/macapp/Sources/GoCodeUI/ChatView.swift b/macapp/Sources/GoCodeUI/ChatView.swift index 0110c488..e50b5fb1 100644 --- a/macapp/Sources/GoCodeUI/ChatView.swift +++ b/macapp/Sources/GoCodeUI/ChatView.swift @@ -851,7 +851,7 @@ struct Composer: View { 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 renderedRunID = run.currentRunID + let action = ComposerAction.capture(canSteer: run.canSteer, runID: run.currentRunID) VStack(alignment: .leading, spacing: Spacing.standard) { if !mentions.isEmpty { MentionPopup(matches: mentions) { match in @@ -870,7 +870,7 @@ struct Composer: View { .textFieldStyle(.plain) .lineLimit(1...10) .focused($focused) - .onSubmit { send(expectedRunID: renderedRunID) } + .onSubmit { send(action) } .onChange(of: run.draft) { _, text in updateMentions(for: text) } HStack(spacing: Spacing.comfortable) { @@ -883,7 +883,7 @@ struct Composer: View { .buttonStyle(.plain).font(Typography.caption).foregroundStyle( Theme.foregroundTertiary) - Button(action: { send(expectedRunID: renderedRunID) }) { + Button(action: { send(action) }) { Image( systemName: run.canSteer ? "arrow.turn.up.right" : "arrow.up.circle.fill" @@ -895,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) @@ -940,12 +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(expectedRunID: String?) { - if run.canSteer { - guard let expectedRunID else { return } - run.steer(expectedRunID: expectedRunID) - } 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() } } } diff --git a/macapp/Sources/GoCodeUI/ProjectSession.swift b/macapp/Sources/GoCodeUI/ProjectSession.swift index c8c59d7c..21070014 100644 --- a/macapp/Sources/GoCodeUI/ProjectSession.swift +++ b/macapp/Sources/GoCodeUI/ProjectSession.swift @@ -329,12 +329,13 @@ public final class ProjectSession { extraDirs.removeAll { $0 == url } } - public func submit() { + @discardableResult + public func submit() -> RunSubmission? { run?.model = selectedModel run?.planMode = planMode run?.extraDirs = extraDirs.map(\.path) run?.profile = selectedProfile - run?.submit() + let submission = run?.submit() Task { // `run.submit()` starts its own unstructured task that only sets // `conversationID` once harnessd has actually minted one — a @@ -348,6 +349,7 @@ public final class ProjectSession { } await refreshConversations() } + return submission } public func openConversation(_ conversation: ConversationInfo) async { diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index bfebfc36..ea14babe 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -80,6 +80,10 @@ 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? + /// 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? public init(client: HarnessClient) { self.client = client @@ -106,9 +110,12 @@ public final class RunSession { // MARK: - Running - public func submit() { + @discardableResult + public func submit() -> 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) + activeSubmission = submission draft = "" connectionError = nil cancelState = .idle @@ -144,6 +151,12 @@ public final class RunSession { let started = try await client.startRun(request) startedRunID = started.runID + // 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. + guard !submission.isDisplaced else { return } + submission.markStarted(runID: started.runID) localStreamRunID = started.runID activate(runID: started.runID, isExternal: false, timestamp: nil, select: true) activateAccounting(for: started.runID, timestamp: nil) @@ -156,19 +169,26 @@ public final class RunSession { } for try await event in client.events(runID: started.runID) { + submission.apply(event) await applyRunEvent(event, expectedRunID: started.runID) } + if !submission.isTerminal { + submission.markFailed("run event stream ended before a terminal event") + } } catch let error as HarnessError { connectionError = error.message transcript.markFailed() + submission.markFailed(error.message) if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } catch { connectionError = error.localizedDescription transcript.markFailed() + submission.markFailed(error.localizedDescription) if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } finishRunIfCurrent(startedRunID: startedRunID) } + return submission } // MARK: - Conversation switching @@ -409,6 +429,12 @@ public final class RunSession { activate( runID: event.runID, isExternal: event.runID != localStreamRunID, 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) } @@ -438,6 +464,11 @@ public final class RunSession { private func selectActive(runID: String) { guard currentRunID != runID else { return } + if let activeSubmission, activeSubmission.runID != nil, + activeSubmission.runID != runID + { + activeSubmission.markDisplaced() + } invalidateRequestOwnership() clearPendingInteractions() currentRunID = runID @@ -473,6 +504,7 @@ public final class RunSession { private func finishRunIfCurrent(startedRunID: String?) { guard let startedRunID else { return } if localStreamRunID == startedRunID { localStreamRunID = nil } + if activeSubmission?.runID == startedRunID { 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 { @@ -485,6 +517,8 @@ public final class RunSession { } private func clearActiveRuns() { + activeSubmission?.markDisplaced() + activeSubmission = nil activeRunIDs = [] externalRunIDs = [] activeRunTimestamps = [:] diff --git a/macapp/Sources/GoCodeUI/RunSubmission.swift b/macapp/Sources/GoCodeUI/RunSubmission.swift new file mode 100644 index 00000000..4b294a8c --- /dev/null +++ b/macapp/Sources/GoCodeUI/RunSubmission.swift @@ -0,0 +1,75 @@ +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 { + public enum State: Equatable { + case starting + case started(String) + case terminal(String) + case failed(String) + case displaced + } + + public private(set) var state: State = .starting + 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 var runID: String? { resolvedRunID } + + public var failure: String? { + guard case .failed(let message) = state else { return nil } + return message + } + + public var isTerminal: Bool { + if case .terminal = state { return true } + return false + } + + public var isDisplaced: Bool { + if case .displaced = state { return true } + return false + } + + init(prompt: String) { + transcript.appendUserPrompt(prompt) + } + + func markStarted(runID: String) { + guard case .starting = state else { return } + resolvedRunID = runID + state = .started(runID) + } + + func apply(_ event: HarnessEvent) { + guard runID == event.runID else { return } + transcript.apply(event) + // Retain A's evidence for diagnostics, but once B displaced this + // submission its later terminal frame must not turn ToolWalk back into + // an apparent successful A lifecycle. The caller must still abort + // rather than act on or judge through B's selected state. + if event.type.isTerminal, !isDisplaced { state = .terminal(event.runID) } + } + + func markFailed(_ message: String) { + guard !isTerminal, !isDisplaced else { return } + state = .failed(message) + transcript.markFailed() + } + + func markDisplaced() { + guard !isTerminal, failure == nil else { return } + state = .displaced + } +} diff --git a/macapp/Sources/ToolWalk/Runner.swift b/macapp/Sources/ToolWalk/Runner.swift index d3ba675f..d5a7590c 100644 --- a/macapp/Sources/ToolWalk/Runner.swift +++ b/macapp/Sources/ToolWalk/Runner.swift @@ -37,47 +37,85 @@ enum Runner { } run.draft = spec.prompt - project.submit() + guard let submission = project.submit() else { + let result = ToolResult( + name: spec.name, verdict: "fail", reply: "submission was not accepted") + results.append(result) + print(" FAIL (submission was not accepted)") + continue + } // 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 walkedRunID = await waitForStartedRunID(run: run, config: config) - let finished = await waitForTerminal(run: run, config: config) + let started = await waitForStartedSubmission(submission, run: run, config: config) + guard case .started = started else { + if case .terminal = started { + 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, state: started) + results.append(result) + print(" FAIL (\(result.reply))") + continue + } + let finished = await waitForTerminal(run: run, submission: submission, config: config) if !finished { - run.cancelTimedOutRun(expectedRunID: walkedRunID) + if submission.isDisplaced { + let result = failedResult(tool: spec.name, state: .displaced) + results.append(result) + print(" FAIL (\(result.reply))") + continue + } + run.cancelTimedOutRun(expectedRunID: submission.runID) // 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)) } - let result = judge(tool: spec.name, observed: observe(run, timedOut: !finished)) + let result = judge( + tool: spec.name, observed: observe(submission, timedOut: !finished)) results.append(result) print(" \(result.verdict.uppercased())") } return results } - private static func waitForStartedRunID(run: RunSession, config: RunnerConfig) async -> String? - { + private static func waitForStartedSubmission( + _ submission: RunSubmission, run: RunSession, config: RunnerConfig + ) async -> RunSubmission.State { let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) while ContinuousClock.now < deadline { - if let runID = run.currentRunID { return runID } - if !run.isBusy, run.connectionError != nil { return nil } + switch submission.state { + case .started(let runID): + return run.currentRunID == runID ? submission.state : .displaced + case .failed, .terminal, .displaced: + return submission.state + case .starting: + break + } try? await Task.sleep(for: config.pollInterval) } - return nil + return .failed("timed out waiting for startRun response") } /// 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 { + private static func waitForTerminal( + run: RunSession, submission: RunSubmission, config: RunnerConfig + ) async -> Bool { let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) while ContinuousClock.now < deadline { + if submission.isDisplaced || submission.failure != nil { return false } + guard let runID = submission.runID, run.currentRunID == runID else { return false } if let prompt = run.pendingQuestions { + guard prompt.runID == runID else { return false } var answers: [String: String] = [:] for question in prompt.questions { answers[question.id] = question.options?.first?.label ?? "yes" @@ -85,25 +123,27 @@ enum Runner { run.answer(answers, expectedRunID: prompt.runID) } if let approval = run.transcript.pendingApproval { + guard approval.runID == runID else { return false } run.approve(expectedRunID: approval.runID) } if let plan = run.transcript.pendingPlan { + guard plan.runID == runID else { return false } run.approve(expectedRunID: plan.runID, option: plan.options.first?.id) } - if !run.isBusy { return true } + if submission.isTerminal { return true } try? await Task.sleep(for: config.pollInterval) } return false } /// 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 { @@ -122,9 +162,24 @@ 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, + runFailed: submission.transcript.runState == .failed, + runCancelled: submission.transcript.runState == .cancelled, + connectionError: submission.failure, timedOut: timedOut) } + + private static func failedResult(tool: String, state: RunSubmission.State) -> ToolResult { + let reply: String + switch state { + case .displaced: + reply = "submission was displaced by another run; no action was sent to that run" + case .failed(let message): + reply = "submission failed before it started: \(message)" + case .terminal: + reply = "submission reached terminal state before ToolWalk could observe it" + case .starting, .started: + reply = "submission did not reach a controllable started state" + } + return ToolResult(name: tool, verdict: "fail", reply: reply) + } } diff --git a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift new file mode 100644 index 00000000..4f6bc7d6 --- /dev/null +++ b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift @@ -0,0 +1,267 @@ +import Foundation +import HarnessKit +import Testing + +@testable import GoCodeUI + +private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { + struct Response: Sendable { + var status = 200 + var headers = ["Content-Type": "application/json"] + var body = Data() + var delay: TimeInterval = 0 + var neverFinishes = false + } + + nonisolated(unsafe) private static var handler: (@Sendable (URLRequest) -> Response)? + nonisolated(unsafe) private static var requests: [URLRequest] = [] + private static let lock = NSLock() + + static func set(_ handler: @escaping @Sendable (URLRequest) -> Response) { + lock.withLock { self.handler = handler } + } + + static func reset() { + lock.withLock { + handler = nil + requests = [] + } + } + static func paths() -> [String] { lock.withLock { requests.compactMap(\.url?.path) } } + + 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 + let response = Self.lock.withLock { + Self.requests.append(request) + return Self.handler?(request) ?? Response() + } + if response.delay > 0 { + DispatchQueue.global().asyncAfter(deadline: .now() + response.delay) { + self.deliver(response, request: request) + } + } else { + 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) } + } + + override func stopLoading() {} +} + +@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"): + return .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"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: return .init() + } + } + let session = makeSession() + session.load(messages: [], conversationID: "conversation") + session.draft = "run A" + let submission = try #require(session.submit()) + + 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"): + return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: return .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 but must not erase + // the displacement outcome that tells ToolWalk to stop before B. + submission.apply(try event("run_a:late", "run_a", "run.completed")) + #expect(submission.isDisplaced) + + session.cancelTimedOutRun(expectedRunID: submission.runID) + try await Task.sleep(for: .milliseconds(50)) + #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() async throws { + let submission = RunSubmission(prompt: "run A") + submission.markStarted(runID: "run_a") + submission.apply( + try event( + "run_a:0", "run_a", "assistant.message", payload: #"{"content":"A replied"}"#)) + submission.apply(try event("run_a:1", "run_a", "run.completed")) + // B's event cannot enter A's handle because `apply` is run-id scoped. + submission.apply( + try 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"): + return .init( + status: 202, body: Data(#"{"run_id":"late_a","status":"queued"}"#.utf8), + delay: 0.15) + default: + return .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) + } +} From eb64af3f0c44152d4a0d25c42027dd2ec850001e Mon Sep 17 00:00:00 2001 From: Dennison Bertram Date: Mon, 3 Aug 2026 20:36:23 +0200 Subject: [PATCH 2/4] fix(macapp): retain submitted run outcomes (#1131) --- docs/logs/INDEX.md | 3 + docs/logs/engineering-log.md | 19 ++ docs/logs/long-term-thinking-log.md | 12 + docs/logs/observational-log.md | 11 + docs/logs/system-log.md | 13 + ...sue-1130-submission-outcomes-impact-map.md | 73 ++++++ ...-03-issue-1130-submission-outcomes-plan.md | 43 ++++ docs/plans/INDEX.md | 5 + docs/plans/active-plan.md | 8 + macapp/Sources/GoCodeUI/RunSession.swift | 83 +++++-- macapp/Sources/GoCodeUI/RunSubmission.swift | 56 +++-- macapp/Sources/ToolWalk/Runner.swift | 118 ++++++--- .../GoCodeUITests/RunSubmissionTests.swift | 223 +++++++++++++++++- .../SubmissionOutcomeTests.swift | 30 +++ 14 files changed, 608 insertions(+), 89 deletions(-) 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 macapp/Tests/ToolWalkTests/SubmissionOutcomeTests.swift diff --git a/docs/logs/INDEX.md b/docs/logs/INDEX.md index d439e815..2d6ea5b1 100644 --- a/docs/logs/INDEX.md +++ b/docs/logs/INDEX.md @@ -22,6 +22,9 @@ - 2026-08-03 — Issue #1124 deterministic retry-wait callback fixture evidence is recorded in the engineering, observational, and system logs. +- 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. diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 9f2f224d..0012a551 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -57,6 +57,25 @@ 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 #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 diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 698ba6f5..79c4bfac 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -38,6 +38,18 @@ 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 #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 diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 1e26b45b..c1cd6264 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 #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 diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index df90a6a0..ff2c5ee9 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -38,6 +38,19 @@ 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 #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` -> 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/INDEX.md b/docs/plans/INDEX.md index deccb912..23b26153 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-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 diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index a68e32ad..d633f197 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 #1130 is 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 diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index ea14babe..5fe2ef28 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -155,38 +155,55 @@ public final class RunSession { // 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. - guard !submission.isDisplaced else { return } submission.markStarted(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) + // 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) { submission.apply(event) - await applyRunEvent(event, expectedRunID: started.runID) + // 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 { - submission.markFailed("run event stream ended before a terminal event") + 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() - submission.markFailed(error.message) + recordSubmissionFailure(submission, runID: startedRunID, message: error.message) if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } catch { - connectionError = error.localizedDescription - transcript.markFailed() - submission.markFailed(error.localizedDescription) + if !Task.isCancelled { + recordSubmissionFailure( + submission, runID: startedRunID, message: error.localizedDescription) + } if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } - finishRunIfCurrent(startedRunID: startedRunID) + finishRunIfCurrent(startedRunID: startedRunID, submission: submission) } return submission } @@ -464,9 +481,7 @@ public final class RunSession { private func selectActive(runID: String) { guard currentRunID != runID else { return } - if let activeSubmission, activeSubmission.runID != nil, - activeSubmission.runID != runID - { + if let activeSubmission, activeSubmission.runID != runID { activeSubmission.markDisplaced() } invalidateRequestOwnership() @@ -501,10 +516,12 @@ public final class RunSession { cancelState = .idle } - private func finishRunIfCurrent(startedRunID: String?) { + private func finishRunIfCurrent(startedRunID: String?, submission: RunSubmission) { guard let startedRunID else { return } if localStreamRunID == startedRunID { localStreamRunID = nil } - if activeSubmission?.runID == startedRunID { activeSubmission = 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 { @@ -529,6 +546,24 @@ public final class RunSession { 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 + } + + 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 diff --git a/macapp/Sources/GoCodeUI/RunSubmission.swift b/macapp/Sources/GoCodeUI/RunSubmission.swift index 4b294a8c..543f832d 100644 --- a/macapp/Sources/GoCodeUI/RunSubmission.swift +++ b/macapp/Sources/GoCodeUI/RunSubmission.swift @@ -10,6 +10,9 @@ import HarnessKit /// 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) @@ -18,27 +21,42 @@ public final class RunSubmission { case displaced } - public private(set) var state: State = .starting + 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? + private(set) public var isDisplaced = false public var runID: String? { resolvedRunID } public var failure: String? { - guard case .failed(let message) = state else { return nil } + guard case .failed(let message) = lifecycle else { return nil } return message } public var isTerminal: Bool { - if case .terminal = state { return true } - return false - } - - public var isDisplaced: Bool { - if case .displaced = state { return true } + if case .terminal = lifecycle { return true } return false } @@ -47,29 +65,29 @@ public final class RunSubmission { } func markStarted(runID: String) { - guard case .starting = state else { return } + guard case .starting = lifecycle else { return } resolvedRunID = runID - state = .started(runID) + lifecycle = .started(runID) } func apply(_ event: HarnessEvent) { guard runID == event.runID else { return } transcript.apply(event) - // Retain A's evidence for diagnostics, but once B displaced this - // submission its later terminal frame must not turn ToolWalk back into - // an apparent successful A lifecycle. The caller must still abort - // rather than act on or judge through B's selected state. - if event.type.isTerminal, !isDisplaced { state = .terminal(event.runID) } + // 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, !isDisplaced else { return } - state = .failed(message) + guard !isTerminal, failure == nil else { return } + lifecycle = .failed(message) transcript.markFailed() } func markDisplaced() { - guard !isTerminal, failure == nil else { return } - state = .displaced + isDisplaced = true } } diff --git a/macapp/Sources/ToolWalk/Runner.swift b/macapp/Sources/ToolWalk/Runner.swift index d5a7590c..f295ad1a 100644 --- a/macapp/Sources/ToolWalk/Runner.swift +++ b/macapp/Sources/ToolWalk/Runner.swift @@ -15,6 +15,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] { @@ -49,73 +60,83 @@ enum Runner { // 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, run: run, config: config) - guard case .started = started else { - if case .terminal = started { + 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, state: started) + let result = failedResult(tool: spec.name, outcome: started) results.append(result) print(" FAIL (\(result.reply))") continue } let finished = await waitForTerminal(run: run, submission: submission, config: config) - if !finished { - if submission.isDisplaced { - let result = failedResult(tool: spec.name, state: .displaced) - results.append(result) - print(" FAIL (\(result.reply))") - continue + switch finished { + case .terminal: + break + case .timedOut: + if shouldCancel(for: finished) { + run.cancelTimedOutRun(expectedRunID: submission.runID) } - run.cancelTimedOutRun(expectedRunID: submission.runID) // 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(submission, timedOut: !finished)) + tool: spec.name, observed: observe(submission, timedOut: finished == .timedOut)) results.append(result) print(" \(result.verdict.uppercased())") } return results } - private static func waitForStartedSubmission( - _ submission: RunSubmission, run: RunSession, config: RunnerConfig - ) async -> RunSubmission.State { + static func waitForStartedSubmission( + _ submission: RunSubmission, config: RunnerConfig + ) async -> SubmissionWaitOutcome { let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) while ContinuousClock.now < deadline { - switch submission.state { - case .started(let runID): - return run.currentRunID == runID ? submission.state : .displaced - case .failed, .terminal, .displaced: - return submission.state - case .starting: - break - } + let outcome = outcome(for: submission) + if outcome != .started || submission.runID != nil { return outcome } try? await Task.sleep(for: config.pollInterval) } - return .failed("timed out waiting for startRun response") + 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( + static func waitForTerminal( run: RunSession, submission: RunSubmission, config: RunnerConfig - ) async -> Bool { + ) async -> SubmissionWaitOutcome { let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) while ContinuousClock.now < deadline { - if submission.isDisplaced || submission.failure != nil { return false } - guard let runID = submission.runID, run.currentRunID == runID else { return false } + let outcome = outcome(for: submission) + switch outcome { + case .terminal, .failed, .displaced: return outcome + case .started: break + case .timedOut: return .timedOut + } + guard let runID = submission.runID, run.currentRunID == runID else { return .displaced } if let prompt = run.pendingQuestions { - guard prompt.runID == runID else { return false } + guard prompt.runID == runID else { return .displaced } var answers: [String: String] = [:] for question in prompt.questions { answers[question.id] = question.options?.first?.label ?? "yes" @@ -123,17 +144,38 @@ enum Runner { run.answer(answers, expectedRunID: prompt.runID) } if let approval = run.transcript.pendingApproval { - guard approval.runID == runID else { return false } + guard approval.runID == runID else { return .displaced } run.approve(expectedRunID: approval.runID) } if let plan = run.transcript.pendingPlan { - guard plan.runID == runID else { return false } + guard plan.runID == runID else { return .displaced } run.approve(expectedRunID: plan.runID, option: plan.options.first?.id) } - if submission.isTerminal { return true } try? await Task.sleep(for: config.pollInterval) } - return false + return .timedOut + } + + /// 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: return .terminal + case .failed(let message): return .failed(message) + case .starting, .started: + return isDisplaced ? .displaced : .started + } + } + + static func shouldCancel(for outcome: SubmissionWaitOutcome) -> Bool { + outcome == .timedOut } /// Reduces a real transcript to the primitives `judge` reasons over. @@ -168,16 +210,18 @@ enum Runner { timedOut: timedOut) } - private static func failedResult(tool: String, state: RunSubmission.State) -> ToolResult { + private static func failedResult(tool: String, outcome: SubmissionWaitOutcome) -> ToolResult { let reply: String - switch state { + switch outcome { case .displaced: reply = "submission was displaced by another run; no action was sent to that run" case .failed(let message): - reply = "submission failed before it started: \(message)" + reply = "submission failed: \(message)" case .terminal: reply = "submission reached terminal state before ToolWalk could observe it" - case .starting, .started: + case .timedOut: + reply = "submission timed out waiting for a terminal outcome" + case .started: reply = "submission did not reach a controllable started state" } return ToolResult(name: tool, verdict: "fail", reply: reply) diff --git a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift index 4f6bc7d6..ee4f5ca7 100644 --- a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift +++ b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift @@ -11,11 +11,18 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { 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? } nonisolated(unsafe) private static var handler: (@Sendable (URLRequest) -> Response)? nonisolated(unsafe) private static var requests: [URLRequest] = [] + nonisolated(unsafe) private static var completedPaths: Set = [] private static let lock = NSLock() + private static let gateLock = NSCondition() + nonisolated(unsafe) private static var openGates: Set = [] static func set(_ handler: @escaping @Sendable (URLRequest) -> Response) { lock.withLock { self.handler = handler } @@ -25,9 +32,23 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { 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 request: URLRequest) -> Bool { true } override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } @@ -38,12 +59,10 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { Self.requests.append(request) return Self.handler?(request) ?? Response() } - if response.delay > 0 { - DispatchQueue.global().asyncAfter(deadline: .now() + response.delay) { - self.deliver(response, request: request) - } - } else { - deliver(response, request: request) + 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) } } @@ -54,9 +73,17 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { 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) @@ -125,6 +152,8 @@ struct RunSubmissionTests { 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") @@ -195,13 +224,14 @@ struct RunSubmissionTests { #expect(session.currentRunID == "run_b") #expect(submission.isDisplaced) - // A's late terminal remains useful A-only evidence but must not erase - // the displacement outcome that tells ToolWalk to stop before B. + // A's late terminal remains useful A-only evidence. It coexists with + // displacement so ToolWalk can judge A and still never control B. submission.apply(try event("run_a:late", "run_a", "run.completed")) #expect(submission.isDisplaced) + #expect(submission.isTerminal) + #expect(submission.state == .terminal("run_a")) session.cancelTimedOutRun(expectedRunID: submission.runID) - try await Task.sleep(for: .milliseconds(50)) #expect(!SubmissionHandleStub.paths().contains("/v1/runs/run_b/cancel")) session.reset() } @@ -264,4 +294,179 @@ struct RunSubmissionTests { #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"): + return .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"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: return .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"): + return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + return .init(headers: ["Content-Type": "text/event-stream"], waitForGate: "eof_a") + case ("GET", "/v1/conversations/conversation/events"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: return .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"): + return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + case ("GET", "/v1/runs/run_a/events"): + return .init( + headers: ["Content-Type": "text/event-stream"], waitForGate: "owned_eof") + case ("GET", "/v1/conversations/run_a/events"): + return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: return .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() + } } 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)) + } +} From 626f2bb2f3abf8237fd1bdaff2f1194edd714abc Mon Sep 17 00:00:00 2001 From: Dennison Bertram Date: Mon, 3 Aug 2026 23:47:41 +0200 Subject: [PATCH 3/4] 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 --- docs/logs/INDEX.md | 6 + docs/logs/engineering-log.md | 32 + docs/logs/long-term-thinking-log.md | 21 + docs/logs/observational-log.md | 23 + docs/logs/system-log.md | 22 + ...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 | 28 + ...08-03-issue-1136-timeout-authority-plan.md | 38 ++ docs/plans/INDEX.md | 10 + docs/plans/active-plan.md | 12 + macapp/Package.swift | 5 +- macapp/Sources/GoCodeUI/ProjectSession.swift | 12 + .../GoCodeUI/RunSession+RunControls.swift | 21 +- macapp/Sources/GoCodeUI/RunSession.swift | 158 +++-- macapp/Sources/GoCodeUI/RunSubmission.swift | 35 +- macapp/Sources/ToolWalk/Runner.swift | 86 ++- ...iveSubmissionOutcomeIntegrationTests.swift | 560 ++++++++++++++++++ .../RunSessionExternalControlTests.swift | 217 ++++--- .../GoCodeUITests/RunSubmissionTests.swift | 156 +++-- 20 files changed, 1340 insertions(+), 241 deletions(-) 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/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift diff --git a/docs/logs/INDEX.md b/docs/logs/INDEX.md index 2d6ea5b1..5c995efa 100644 --- a/docs/logs/INDEX.md +++ b/docs/logs/INDEX.md @@ -22,6 +22,12 @@ - 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. diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 0012a551..cba3bceb 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -57,6 +57,38 @@ 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 mutable-pointer authorization with a private + `RunSubmission` owner-token/generation capability. It atomically dispatches + only once while A is started; 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. +- Deterministic gated evidence proves B -> C -> A emits exactly one A cancel, + zero B/C actions, and terminal/failure/reset have no dispatch. Full native + Swift passes 245 tests in 46 suites; repository regression awaits #1135. + +## 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 diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 79c4bfac..1d4e85ac 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -38,6 +38,27 @@ 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: one A handle can consume its 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 diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index c1cd6264..10aee4b7 100644 --- a/docs/logs/observational-log.md +++ b/docs/logs/observational-log.md @@ -31,6 +31,29 @@ - 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 an + authority proof. Direct synchronous capability consumption makes B -> C -> A + exact-one dispatch and terminal/failure/reset non-dispatch deterministic. +- 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 diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index ff2c5ee9..8f3a95e5 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -38,6 +38,28 @@ 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. `RunSession` is the only authority that can consume it, and + `cancelTimedOutSubmission` dispatches a transport-only A cancel only on that + success. A handle-keyed task registry lets reset/load cancel all local streams. + +## 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 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..8a62c242 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1136-timeout-authority-impact-map.md @@ -0,0 +1,28 @@ +# 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 an unforgeable, one-shot A cancellation capability. +- ToolWalk: invokes only the handle API. Its timeout is transport-only; it + cannot alter B/C selection, transcript, controls, or cancellation state. +- 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 + B -> C -> A sends exactly one A cancel, zero B/C actions, and reset stops + both concurrent A/C event streams. +- 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..814f85d7 --- /dev/null +++ b/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md @@ -0,0 +1,38 @@ +# 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` atomically consumes a started-only capability once; + 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 (0/7 touched Swift files) and full Swift (245 tests + / 46 suites) after the final proof update. +- [x] Focused `PassiveSubmissionOutcomeIntegrationTests`: 10/10 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 #1135 baseline repair: + normal, race, and coverage passed (85.5% total; zero uncovered production + functions). +- [x] Publish the separate stacked draft PR with `Closes #1136`. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index 23b26153..77816a83 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -30,6 +30,16 @@ 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 diff --git a/docs/plans/active-plan.md b/docs/plans/active-plan.md index d633f197..45b80270 100644 --- a/docs/plans/active-plan.md +++ b/docs/plans/active-plan.md @@ -30,6 +30,18 @@ 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 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/ProjectSession.swift b/macapp/Sources/GoCodeUI/ProjectSession.swift index 21070014..3986ed7f 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 } diff --git a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift index 3de6636c..9b65624b 100644 --- a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift +++ b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift @@ -5,7 +5,9 @@ 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 @@ -16,12 +18,17 @@ extension RunSession { cancel(runID: runID) } - /// ToolWalk's timeout action has already decided which run timed out. Keep - /// that captured identity at the RunSession boundary rather than resolving - /// the currently selected continuation during cancellation. - public func cancelTimedOutRun(expectedRunID: String?) { - guard let expectedRunID else { return } - cancel(expectedRunID: expectedRunID) + /// Consumes the exact submitted A timeout capability. Unlike a bare run + /// string, this cannot be redirected to selected B, replayed after reset, + /// or re-used after terminal/failure. The transport-only path deliberately + /// makes no shared UI state change. + @discardableResult + public func cancelTimedOutSubmission(_ submission: RunSubmission) -> Bool { + guard let runID = consumeTimeoutCancellation(for: submission) else { return false } + Task { [client] in + try? await client.cancel(runID: runID) + } + return true } /// Compatibility entry point for programmatic callers that do not retain diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index 5fe2ef28..4387be9c 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -80,10 +80,20 @@ 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. + private let submissionOwnerToken = UUID() + /// Reset/load detach the old session permanently; their generation invalidates + /// every outstanding submission timeout capability. + private var submissionGeneration: UInt = 0 public init(client: HarnessClient) { self.client = client @@ -93,14 +103,23 @@ public final class RunSession { 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? { @@ -114,7 +133,10 @@ public final class RunSession { public func submit() -> RunSubmission? { let prompt = draft.trimmed guard !prompt.isEmpty, !isBusy, !runControlInFlight else { return nil } - let submission = RunSubmission(prompt: prompt) + let submission = RunSubmission( + prompt: prompt, timeoutOwner: submissionOwnerToken, + timeoutGeneration: submissionGeneration + ) activeSubmission = submission draft = "" connectionError = nil @@ -131,7 +153,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? @@ -188,7 +211,8 @@ public final class RunSession { recordSubmissionFailure( submission, runID: started.runID, - message: "run event stream ended before a terminal event") + message: "run event stream ended before a terminal event" + ) } } catch is CancellationError { // reset/load intentionally detaches the handle and cancels its @@ -199,12 +223,16 @@ public final class RunSession { } catch { if !Task.isCancelled { recordSubmissionFailure( - submission, runID: startedRunID, message: error.localizedDescription) + submission, runID: startedRunID, message: error.localizedDescription + ) } if let startedRunID { releaseUnstartedAccounting(for: startedRunID) } } finishRunIfCurrent(startedRunID: startedRunID, submission: submission) + submissionStreamTasks.removeValue(forKey: submissionID) } + streamTask = task + submissionStreamTasks[submissionID] = task return submission } @@ -235,7 +263,8 @@ 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 @@ -303,8 +332,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 @@ -321,7 +350,8 @@ public final class RunSession { reconcilePersistedMessages( messages, retainingAccountingFor: accountingRunID, - preservingRunState: isStaleTerminal) + preservingRunState: isStaleTerminal + ) } } } catch is CancellationError { @@ -416,36 +446,38 @@ 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 @@ -465,9 +497,9 @@ public final class RunSession { .toolApprovalGranted, .toolApprovalDenied, .planApprovalGranted, .planApprovalDenied, .runWaitingForUser: - return true + true default: - return false + false } } @@ -517,6 +549,7 @@ public final class RunSession { } 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 @@ -534,6 +567,11 @@ public final class RunSession { } private func clearActiveRuns() { + submissionGeneration &+= 1 + for task in submissionStreamTasks.values { + task.cancel() + } + submissionStreamTasks = [:] activeSubmission?.markDisplaced() activeSubmission = nil activeRunIDs = [] @@ -555,6 +593,12 @@ public final class RunSession { 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 ) { @@ -622,11 +666,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 @@ -638,21 +682,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 } @@ -717,7 +761,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 index 543f832d..ba3ce66d 100644 --- a/macapp/Sources/GoCodeUI/RunSubmission.swift +++ b/macapp/Sources/GoCodeUI/RunSubmission.swift @@ -41,14 +41,23 @@ public final class RunSubmission { 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? - private(set) public var isDisplaced = false + 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 var timeoutCancellationConsumed = false - public var runID: String? { resolvedRunID } + public var runID: String? { + resolvedRunID + } public var failure: String? { guard case .failed(let message) = lifecycle else { return nil } @@ -60,10 +69,30 @@ public final class RunSubmission { return false } - init(prompt: String) { + init(prompt: String, timeoutOwner: UUID, timeoutGeneration: UInt) { + self.timeoutOwner = timeoutOwner + self.timeoutGeneration = timeoutGeneration 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 + } + func markStarted(runID: String) { guard case .starting = lifecycle else { return } resolvedRunID = runID diff --git a/macapp/Sources/ToolWalk/Runner.swift b/macapp/Sources/ToolWalk/Runner.swift index f295ad1a..e2f6b6fc 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` @@ -41,7 +42,8 @@ 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 @@ -50,7 +52,8 @@ enum Runner { run.draft = spec.prompt guard let submission = project.submit() else { let result = ToolResult( - name: spec.name, verdict: "fail", reply: "submission was not accepted") + name: spec.name, verdict: "fail", reply: "submission was not accepted" + ) results.append(result) print(" FAIL (submission was not accepted)") continue @@ -64,7 +67,8 @@ enum Runner { guard started == .started else { if started == .terminal { let result = judge( - tool: spec.name, observed: observe(submission, timedOut: false)) + tool: spec.name, observed: observe(submission, timedOut: false) + ) results.append(result) print(" \(result.verdict.uppercased())") continue @@ -80,7 +84,7 @@ enum Runner { break case .timedOut: if shouldCancel(for: finished) { - run.cancelTimedOutRun(expectedRunID: submission.runID) + run.cancelTimedOutSubmission(submission) } // Give the cooperative cancel a moment to land before moving // on, or the next tool's newConversation() races its teardown. @@ -100,7 +104,8 @@ enum Runner { } let result = judge( - tool: spec.name, observed: observe(submission, timedOut: finished == .timedOut)) + tool: spec.name, observed: observe(submission, timedOut: finished == .timedOut) + ) results.append(result) print(" \(result.verdict.uppercased())") } @@ -112,8 +117,18 @@ enum Runner { ) async -> SubmissionWaitOutcome { let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) while ContinuousClock.now < deadline { - let outcome = outcome(for: submission) - if outcome != .started || submission.runID != nil { return outcome } + // 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 @@ -130,11 +145,23 @@ enum Runner { while ContinuousClock.now < deadline { let outcome = outcome(for: submission) switch outcome { - case .terminal, .failed, .displaced: return outcome - case .started: break + case .terminal, .failed: return outcome + case .started, .displaced: break case .timedOut: return .timedOut } - guard let runID = submission.runID, run.currentRunID == runID else { return .displaced } + guard let runID = submission.runID else { + try? await Task.sleep(for: config.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: config.pollInterval) + continue + } if let prompt = run.pendingQuestions { guard prompt.runID == runID else { return .displaced } var answers: [String: String] = [:] @@ -167,10 +194,10 @@ enum Runner { for lifecycle: RunSubmission.Lifecycle, isDisplaced: Bool ) -> SubmissionWaitOutcome { switch lifecycle { - case .terminal: return .terminal - case .failed(let message): return .failed(message) + case .terminal: .terminal + case .failed(let message): .failed(message) case .starting, .started: - return isDisplaced ? .displaced : .started + isDisplaced ? .displaced : .started } } @@ -207,23 +234,24 @@ enum Runner { runFailed: submission.transcript.runState == .failed, runCancelled: submission.transcript.runState == .cancelled, connectionError: submission.failure, - timedOut: timedOut) + timedOut: timedOut + ) } private static func failedResult(tool: String, outcome: SubmissionWaitOutcome) -> ToolResult { - let reply: String - switch outcome { - case .displaced: - reply = "submission was displaced by another run; no action was sent to that run" - case .failed(let message): - reply = "submission failed: \(message)" - case .terminal: - reply = "submission reached terminal state before ToolWalk could observe it" - case .timedOut: - reply = "submission timed out waiting for a terminal outcome" - case .started: - reply = "submission did not reach a controllable started state" - } + 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..699bddb2 --- /dev/null +++ b/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift @@ -0,0 +1,560 @@ +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 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") + } + + 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 run = session() + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit()) + try await displaceA(run, submission: submission) + let waitTask = Task { + await Runner.waitForTerminal( + run: run, submission: submission, + config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5))) + } + PassiveOutcomeProtocol.openGate("a-terminal") + #expect(await waitTask.value == .terminal) + #expect(run.currentRunID == "run_b") + assertNoAction(for: ["run_b"]) + 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 run = session() + run.load(messages: [], conversationID: "conversation") + run.draft = "A" + let submission = try #require(run.submit()) + try await displaceA(run, submission: submission) + let waitTask = Task { + await Runner.waitForTerminal( + run: run, submission: submission, + config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5))) + } + PassiveOutcomeProtocol.openGate("a-eof") + #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_b"]) + 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()) + try await displaceA(run, submission: submission) + let outcome = await Runner.waitForTerminal( + run: run, submission: submission, + config: .init(timeoutPerTool: .milliseconds(80), pollInterval: .milliseconds(5)) + ) + #expect(outcome == .timedOut) + #expect(run.cancelTimedOutSubmission(submission)) + 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()) + 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" } + // Timeout policy is already exercised above. This direct dispatch + // proves the more important authority condition deterministically: + // once B is terminal and C owns visible state, A's captured handle + // still authorizes exactly one A-only cancellation. + #expect(run.cancelTimedOutSubmission(submission)) + #expect(!run.cancelTimedOutSubmission(submission)) + 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()) + try await wait { submission.runID == "run_a" } + #expect(run.cancelTimedOutSubmission(submission)) + #expect(!run.cancelTimedOutSubmission(submission)) + 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()) + try await wait { terminal.runID == "run_a" } + PassiveOutcomeProtocol.openGate("a-terminal") + try await wait { terminal.isTerminal } + #expect(!run.cancelTimedOutSubmission(terminal)) + #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") } + #expect(!run.cancelTimedOutSubmission(reset)) + #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 + } + #expect(!run.cancelTimedOutSubmission(submission)) + #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 cc6dc9ee..be4548cf 100644 --- a/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift +++ b/macapp/Tests/GoCodeUITests/RunSessionExternalControlTests.swift @@ -5,31 +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":"\#(runID)","call_id":"call_\#(runID)","questions":[{"question":"Continue?","options":[]}]}"# - .utf8) + .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) @@ -47,7 +59,9 @@ 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( @@ -60,7 +74,8 @@ struct RunSessionExternalControlTests { id: id, event: type, data: #"{"id":"\#(id)","run_id":"\#(runID)","type":"\#(type)"\#(timestampField),"payload":\#(payload)}"# - )) + ) + ) } private func wait( @@ -84,13 +99,17 @@ 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") @@ -99,10 +118,13 @@ struct RunSessionExternalControlTests { try await session.applyConversationEvent( event( "run_b:1", "run_b", "tool.approval_required", - payload: #"{"call_id":"call_b_1","tool":"bash","arguments":"{}"}"#), - conversationID: "conversation_a") + 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" } @@ -115,24 +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") + 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:5", "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"]) @@ -165,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) @@ -185,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() @@ -200,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() @@ -216,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") @@ -232,10 +267,12 @@ 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() @@ -251,25 +288,30 @@ struct RunSessionExternalControlTests { 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") + 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") + 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") + 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" }) + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/approve" } + ) session.reset() } @@ -280,15 +322,19 @@ struct RunSessionExternalControlTests { 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") + 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") + 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") + 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) @@ -296,7 +342,8 @@ struct RunSessionExternalControlTests { try await session.applyConversationEvent( event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), - conversationID: "conversation_plan_input_owner") + conversationID: "conversation_plan_input_owner" + ) #expect(session.currentRunID == "run_b") #expect(session.transcript.pendingPlan == nil) @@ -315,20 +362,25 @@ struct RunSessionExternalControlTests { 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") + 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") + 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") + 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") + conversationID: "conversation_terminal_owner" + ) #expect(session.currentRunID == nil) #expect(session.transcript.pendingApproval == nil) @@ -343,22 +395,27 @@ struct RunSessionExternalControlTests { 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") + 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") + 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") + 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") + conversationID: "conversation_terminal_fallback_owner" + ) #expect(session.currentRunID == "run_b") #expect(session.transcript.pendingApproval == nil) @@ -373,18 +430,23 @@ struct RunSessionExternalControlTests { 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") + 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") + 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") + 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") + conversationID: "conversation_foreign_terminal" + ) #expect(session.currentRunID == "run_b") #expect(session.transcript.pendingApproval?.runID == "run_b") @@ -398,7 +460,8 @@ struct RunSessionExternalControlTests { 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") + 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 @@ -409,13 +472,15 @@ struct RunSessionExternalControlTests { } try await session.applyConversationEvent( event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), - conversationID: "conversation_stale_stop") + 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" }) + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/cancel" } + ) #expect(session.currentRunID == "run_b") #expect(session.transcript.runState != .cancelled) @@ -427,27 +492,6 @@ struct RunSessionExternalControlTests { session.reset() } - @Test("ToolWalk timeout captured for A cannot cancel newer B") - func toolWalkTimeoutDoesNotTargetNewerRun() async throws { - ExternalRunControlStub.reset() - let session = makeSession() - session.load(messages: [], conversationID: "conversation_toolwalk_timeout") - try await session.applyConversationEvent( - event("run_a:0", "run_a", "run.started", timestamp: "2026-08-03T18:00:01Z"), - conversationID: "conversation_toolwalk_timeout") - let timedOutRunID = try #require(session.currentRunID) - try await session.applyConversationEvent( - event("run_b:0", "run_b", "run.started", timestamp: "2026-08-03T18:00:02Z"), - conversationID: "conversation_toolwalk_timeout") - - session.cancelTimedOutRun(expectedRunID: timedOutRunID) - try await Task.sleep(for: .milliseconds(50)) - #expect( - !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/cancel" }) - #expect(session.currentRunID == "run_b") - session.reset() - } - @Test("a stale Composer steer captured for A leaves newer B and its draft untouched") func staleSteerDoesNotTargetNewerRun() async throws { ExternalRunControlStub.reset() @@ -455,18 +499,21 @@ struct RunSessionExternalControlTests { 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") + 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") + 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" }) + !ExternalRunControlStub.requests.contains { $0.url?.path == "/v1/runs/run_b/steer" } + ) #expect(session.draft == "keep watching") #expect(session.currentRunID == "run_b") diff --git a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift index ee4f5ca7..8225b09c 100644 --- a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift +++ b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift @@ -5,7 +5,7 @@ import Testing @testable import GoCodeUI private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { - struct Response: Sendable { + struct Response { var status = 200 var headers = ["Content-Type": "application/json"] var body = Data() @@ -17,12 +17,12 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { var waitForGate: String? } - nonisolated(unsafe) private static var handler: (@Sendable (URLRequest) -> Response)? - nonisolated(unsafe) private static var requests: [URLRequest] = [] - nonisolated(unsafe) private static var completedPaths: Set = [] + 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() - nonisolated(unsafe) private static var openGates: Set = [] + private nonisolated(unsafe) static var openGates: Set = [] static func set(_ handler: @escaping @Sendable (URLRequest) -> Response) { lock.withLock { self.handler = handler } @@ -38,7 +38,11 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { openGates = [] gateLock.unlock() } - static func paths() -> [String] { lock.withLock { requests.compactMap(\.url?.path) } } + + static func paths() -> [String] { + lock.withLock { requests.compactMap(\.url?.path) } + } + static func completed(_ path: String) -> Bool { lock.withLock { completedPaths.contains(path) } } @@ -50,11 +54,16 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { gateLock.unlock() } - override class func canInit(with request: URLRequest) -> Bool { true } - override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } + 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 let response = Self.lock.withLock { Self.requests.append(request) return Self.handler?(request) ?? Response() @@ -69,7 +78,8 @@ private final class SubmissionHandleStub: URLProtocol, @unchecked Sendable { private func deliver(_ response: Response, request: URLRequest) { let http = HTTPURLResponse( url: request.url!, statusCode: response.status, httpVersion: "HTTP/1.1", - headerFields: response.headers)! + headerFields: response.headers + )! client?.urlProtocol(self, didReceive: http, cacheStoragePolicy: .notAllowed) client?.urlProtocol(self, didLoad: response.body) if !response.neverFinishes { client?.urlProtocolDidFinishLoading(self) } @@ -95,7 +105,9 @@ struct RunSubmissionTests { return RunSession( client: HarnessClient( baseURL: URL(string: "http://127.0.0.1:8896")!, - session: URLSession(configuration: configuration))) + session: URLSession(configuration: configuration) + ) + ) } private func event(_ id: String, _ runID: String, _ type: String, payload: String = "{}") throws @@ -106,7 +118,8 @@ struct RunSubmissionTests { 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 { @@ -127,7 +140,8 @@ struct RunSubmissionTests { action.perform( canSubmit: true, steer: { steeredRunID = $0 }, - submit: { submitted = true }) + submit: { submitted = true } + ) #expect(steeredRunID == "run_a") #expect(!submitted) @@ -139,12 +153,13 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init( + .init( status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), - delay: 0.15) + delay: 0.15 + ) case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) - default: return .init() + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() } } let session = makeSession() @@ -155,7 +170,8 @@ struct RunSubmissionTests { try await wait { SubmissionHandleStub.paths().contains("/v1/runs") } try await session.applyConversationEvent( - event("run_b:0", "run_b", "run.started"), conversationID: "conversation") + event("run_b:0", "run_b", "run.started"), conversationID: "conversation" + ) #expect(session.currentRunID == "run_b") try await wait { submission.runID == "run_a" } @@ -175,10 +191,10 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) - default: return .init() + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() } } let session = makeSession() @@ -188,11 +204,14 @@ struct RunSubmissionTests { 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") + event("run_b:0", "run_b", "run.started"), conversationID: "conversation" + ) #expect( - session.currentRunID == "run_a", "timestamp-less B cannot displace provisional local A") + 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") + 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( @@ -200,7 +219,8 @@ struct RunSubmissionTests { 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") @@ -212,42 +232,48 @@ struct RunSubmissionTests { 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. - submission.apply(try event("run_a:late", "run_a", "run.completed")) + try submission.apply(event("run_a:late", "run_a", "run.completed")) #expect(submission.isDisplaced) #expect(submission.isTerminal) #expect(submission.state == .terminal("run_a")) - session.cancelTimedOutRun(expectedRunID: submission.runID) + session.cancelTimedOutSubmission(submission) #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() async throws { + func submissionRetainsItsOwnTerminalTranscript() throws { let submission = RunSubmission(prompt: "run A") submission.markStarted(runID: "run_a") - submission.apply( - try event( - "run_a:0", "run_a", "assistant.message", payload: #"{"content":"A replied"}"#)) - submission.apply(try event("run_a:1", "run_a", "run.completed")) + 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. - submission.apply( - try event( - "run_b:0", "run_b", "assistant.message", payload: #"{"content":"B replied"}"#)) + 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( @@ -256,7 +282,8 @@ struct RunSubmissionTests { return message.text == "A replied" } return false - }) + } + ) } @Test("start failure and reset leave no resurrected submission") @@ -278,11 +305,12 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init( + .init( status: 202, body: Data(#"{"run_id":"late_a","status":"queued"}"#.utf8), - delay: 0.15) + delay: 0.15 + ) default: - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) } } let resetSession = makeSession() @@ -301,12 +329,13 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init( + .init( status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), - waitForGate: "release_a_ack") + waitForGate: "release_a_ack" + ) case ("GET", "/v1/runs/run_a/events"), ("GET", "/v1/conversations/conversation/events"): - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) - default: return .init() + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() } } let session = makeSession() @@ -320,7 +349,8 @@ struct RunSubmissionTests { 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") @@ -342,7 +372,8 @@ struct RunSubmissionTests { return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) } return .init( - status: 503, body: Data(#"{"error":"A unavailable"}"#.utf8), waitForGate: "fail_a") + status: 503, body: Data(#"{"error":"A unavailable"}"#.utf8), waitForGate: "fail_a" + ) } let session = makeSession() session.load(messages: [], conversationID: "conversation") @@ -354,7 +385,8 @@ struct RunSubmissionTests { 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") @@ -374,12 +406,12 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) case ("GET", "/v1/runs/run_a/events"): - return .init(headers: ["Content-Type": "text/event-stream"], waitForGate: "eof_a") + .init(headers: ["Content-Type": "text/event-stream"], waitForGate: "eof_a") case ("GET", "/v1/conversations/conversation/events"): - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) - default: return .init() + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() } } let session = makeSession() @@ -392,14 +424,16 @@ struct RunSubmissionTests { 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") @@ -417,13 +451,14 @@ struct RunSubmissionTests { SubmissionHandleStub.set { request in switch (request.httpMethod, request.url?.path) { case ("POST", "/v1/runs"): - return .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) + .init(status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8)) case ("GET", "/v1/runs/run_a/events"): - return .init( - headers: ["Content-Type": "text/event-stream"], waitForGate: "owned_eof") + .init( + headers: ["Content-Type": "text/event-stream"], waitForGate: "owned_eof" + ) case ("GET", "/v1/conversations/run_a/events"): - return .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) - default: return .init() + .init(headers: ["Content-Type": "text/event-stream"], neverFinishes: true) + default: .init() } } let session = makeSession() @@ -448,7 +483,8 @@ struct RunSubmissionTests { } return .init( status: 202, body: Data(#"{"run_id":"run_a","status":"queued"}"#.utf8), - waitForGate: "late_a_after_load") + waitForGate: "late_a_after_load" + ) } let session = makeSession() session.load(messages: [], conversationID: "conversation_a") From 0a0bedffa7101985c750a443824920ec93e75709 Mon Sep 17 00:00:00 2001 From: Dennison Bertram Date: Tue, 4 Aug 2026 01:05:33 +0200 Subject: [PATCH 4/4] fix(toolwalk): gate timeout cancellation by deadline ticket (#1146) --- docs/logs/engineering-log.md | 27 +- docs/logs/long-term-thinking-log.md | 7 +- docs/logs/observational-log.md | 8 +- docs/logs/system-log.md | 17 +- ...issue-1136-timeout-authority-impact-map.md | 18 +- ...08-03-issue-1136-timeout-authority-plan.md | 41 ++- macapp/Sources/GoCodeUI/ProjectSession.swift | 9 +- .../GoCodeUI/RunSession+RunControls.swift | 73 +++++- macapp/Sources/GoCodeUI/RunSession.swift | 31 ++- macapp/Sources/GoCodeUI/RunSubmission.swift | 29 ++- macapp/Sources/ToolWalk/Runner.swift | 28 ++- ...iveSubmissionOutcomeIntegrationTests.swift | 233 +++++++++++++++--- .../GoCodeUITests/RunSubmissionTests.swift | 19 +- 13 files changed, 454 insertions(+), 86 deletions(-) diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index cba3bceb..bc418ce4 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -59,14 +59,29 @@ coverage and zero uncovered functions in 2m26s. ## 2026-08-03 (Issue #1136 immutable timeout authority) -- Replaced the provisional mutable-pointer authorization with a private - `RunSubmission` owner-token/generation capability. It atomically dispatches - only once while A is started; terminal, failure, reset, and load revoke it. +- 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. -- Deterministic gated evidence proves B -> C -> A emits exactly one A cancel, - zero B/C actions, and terminal/failure/reset have no dispatch. Full native - Swift passes 245 tests in 46 suites; repository regression awaits #1135. +- 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) diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 1d4e85ac..5cc895b5 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -43,9 +43,10 @@ - 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: one A handle can consume its 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. +- 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. diff --git a/docs/logs/observational-log.md b/docs/logs/observational-log.md index 10aee4b7..d3c0d54d 100644 --- a/docs/logs/observational-log.md +++ b/docs/logs/observational-log.md @@ -33,9 +33,11 @@ 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 an - authority proof. Direct synchronous capability consumption makes B -> C -> A - exact-one dispatch and terminal/failure/reset non-dispatch deterministic. +- 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. diff --git a/docs/logs/system-log.md b/docs/logs/system-log.md index 8f3a95e5..0aa91911 100644 --- a/docs/logs/system-log.md +++ b/docs/logs/system-log.md @@ -41,9 +41,20 @@ ## 2026-08-03 (Issue #1136 immutable timeout capability) - `RunSubmission` privately binds owner token, generation, lifecycle, and a - consumed bit. `RunSession` is the only authority that can consume it, and - `cancelTimedOutSubmission` dispatches a transport-only A cancel only on that - success. A handle-keyed task registry lets reset/load cancel all local streams. + 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) 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 index 8a62c242..41528407 100644 --- 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 @@ -10,9 +10,16 @@ ## Surfaces - Native model: private owner UUID plus reset/load generation and lifecycle - form an unforgeable, one-shot A cancellation capability. -- ToolWalk: invokes only the handle API. Its timeout is transport-only; it + 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. @@ -23,6 +30,9 @@ 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 - B -> C -> A sends exactly one A cancel, zero B/C actions, and reset stops - both concurrent A/C event streams. + 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 index 814f85d7..9f7d581a 100644 --- a/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md +++ b/docs/plans/2026-08-03-issue-1136-timeout-authority-plan.md @@ -16,9 +16,18 @@ - 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` atomically consumes a started-only capability once; - terminal, failure, reset, and load revoke it. Reset/load cancel every live - submission stream by immutable handle, including displaced A plus selected C. + 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 @@ -28,11 +37,23 @@ - [x] Write red and repair the authority model. - [x] Add deterministic capability/revocation/detachment tests. -- [x] Re-run strict format (0/7 touched Swift files) and full Swift (245 tests - / 46 suites) after the final proof update. -- [x] Focused `PassiveSubmissionOutcomeIntegrationTests`: 10/10 cases on the +- [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 #1135 baseline repair: - normal, race, and coverage passed (85.5% total; zero uncovered production - functions). -- [x] Publish the separate stacked draft PR with `Closes #1136`. +- [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/macapp/Sources/GoCodeUI/ProjectSession.swift b/macapp/Sources/GoCodeUI/ProjectSession.swift index 3986ed7f..e3446b8e 100644 --- a/macapp/Sources/GoCodeUI/ProjectSession.swift +++ b/macapp/Sources/GoCodeUI/ProjectSession.swift @@ -343,11 +343,18 @@ public final class ProjectSession { @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 - let submission = 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 diff --git a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift index 9b65624b..b6bd37ca 100644 --- a/macapp/Sources/GoCodeUI/RunSession+RunControls.swift +++ b/macapp/Sources/GoCodeUI/RunSession+RunControls.swift @@ -1,6 +1,57 @@ 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 @@ -18,17 +69,17 @@ extension RunSession { cancel(runID: runID) } - /// Consumes the exact submitted A timeout capability. Unlike a bare run - /// string, this cannot be redirected to selected B, replayed after reset, - /// or re-used after terminal/failure. The transport-only path deliberately - /// makes no shared UI state change. - @discardableResult - public func cancelTimedOutSubmission(_ submission: RunSubmission) -> Bool { - guard let runID = consumeTimeoutCancellation(for: submission) else { return false } - Task { [client] in - try? await client.cancel(runID: runID) - } - return true + /// 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 diff --git a/macapp/Sources/GoCodeUI/RunSession.swift b/macapp/Sources/GoCodeUI/RunSession.swift index 4387be9c..8d8621c7 100644 --- a/macapp/Sources/GoCodeUI/RunSession.swift +++ b/macapp/Sources/GoCodeUI/RunSession.swift @@ -90,13 +90,28 @@ public final class RunSession { private var activeSubmission: RunSubmission? /// Each submission captures this unforgeable owner token and the current /// generation. It remains independent from selected-run UI state. - private let submissionOwnerToken = UUID() + 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. - private var submissionGeneration: UInt = 0 + 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) { @@ -131,11 +146,20 @@ public final class RunSession { @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 nil } let submission = RunSubmission( prompt: prompt, timeoutOwner: submissionOwnerToken, - timeoutGeneration: submissionGeneration + timeoutGeneration: submissionGeneration, timeoutAfter: timeoutAfter, + timeoutNow: submissionTimeoutNow ) activeSubmission = submission draft = "" @@ -572,6 +596,7 @@ public final class RunSession { task.cancel() } submissionStreamTasks = [:] + submissionTimeoutGates = [:] activeSubmission?.markDisplaced() activeSubmission = nil activeRunIDs = [] diff --git a/macapp/Sources/GoCodeUI/RunSubmission.swift b/macapp/Sources/GoCodeUI/RunSubmission.swift index ba3ce66d..4fb372c3 100644 --- a/macapp/Sources/GoCodeUI/RunSubmission.swift +++ b/macapp/Sources/GoCodeUI/RunSubmission.swift @@ -53,6 +53,10 @@ public final class RunSubmission { /// 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? { @@ -69,9 +73,15 @@ public final class RunSubmission { return false } - init(prompt: String, timeoutOwner: UUID, timeoutGeneration: UInt) { + 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) } @@ -93,12 +103,29 @@ public final class RunSubmission { 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) diff --git a/macapp/Sources/ToolWalk/Runner.swift b/macapp/Sources/ToolWalk/Runner.swift index e2f6b6fc..242c874d 100644 --- a/macapp/Sources/ToolWalk/Runner.swift +++ b/macapp/Sources/ToolWalk/Runner.swift @@ -50,7 +50,7 @@ enum Runner { } run.draft = spec.prompt - guard let submission = project.submit() else { + guard let submission = project.submit(timeoutAfter: config.timeoutPerTool) else { let result = ToolResult( name: spec.name, verdict: "fail", reply: "submission was not accepted" ) @@ -78,14 +78,15 @@ enum Runner { print(" FAIL (\(result.reply))") continue } - let finished = await waitForTerminal(run: run, submission: submission, config: config) + let finished = await waitForTerminal( + run: run, submission: submission, pollInterval: config.pollInterval + ) { ticket in + _ = ticket.consume() + } switch finished { case .terminal: break case .timedOut: - if shouldCancel(for: finished) { - run.cancelTimedOutSubmission(submission) - } // 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)) @@ -139,18 +140,22 @@ enum Runner { /// Without this, AskUserQuestion (and any tool a permission rule gates) /// would simply hang every walk until the timeout. static func waitForTerminal( - run: RunSession, submission: RunSubmission, config: RunnerConfig + run: RunSession, submission: RunSubmission, pollInterval: Duration, + onTimeout: @escaping @MainActor (TimedOutSubmissionTicket) -> Void = { _ in } ) async -> SubmissionWaitOutcome { - let deadline = ContinuousClock.now.advanced(by: config.timeoutPerTool) - while ContinuousClock.now < deadline { + 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: config.pollInterval) + try? await Task.sleep(for: pollInterval) continue } // Once B owns visible state, A's handle remains an observation @@ -159,7 +164,7 @@ enum Runner { // 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: config.pollInterval) + try? await Task.sleep(for: pollInterval) continue } if let prompt = run.pendingQuestions { @@ -178,9 +183,8 @@ enum Runner { guard plan.runID == runID else { return .displaced } run.approve(expectedRunID: plan.runID, option: plan.options.first?.id) } - try? await Task.sleep(for: config.pollInterval) + try? await Task.sleep(for: pollInterval) } - return .timedOut } /// Lifecycle has priority over displacement. A selected B must prevent diff --git a/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift b/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift index 699bddb2..a8d6df7f 100644 --- a/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift +++ b/macapp/Tests/GoCodeUITests/PassiveSubmissionOutcomeIntegrationTests.swift @@ -116,6 +116,12 @@ struct PassiveSubmissionOutcomeIntegrationTests { 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] @@ -163,6 +169,20 @@ struct PassiveSubmissionOutcomeIntegrationTests { 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( @@ -202,20 +222,23 @@ struct PassiveSubmissionOutcomeIntegrationTests { default: .init() } } - let run = session() + let now = ContinuousClock.now + let run = session(submissionTimeoutNow: { now }) run.load(messages: [], conversationID: "conversation") run.draft = "A" - let submission = try #require(run.submit()) + 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, - config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5))) + 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_b"]) + assertNoAction(for: ["run_a", "run_b", "run_c"]) run.reset() } @@ -313,21 +336,27 @@ struct PassiveSubmissionOutcomeIntegrationTests { default: .init() } } - let run = session() + let now = ContinuousClock.now + let run = session(submissionTimeoutNow: { now }) run.load(messages: [], conversationID: "conversation") run.draft = "A" - let submission = try #require(run.submit()) + 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, - config: .init(timeoutPerTool: .seconds(1), pollInterval: .milliseconds(5))) + 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_b"]) + assertNoAction(for: ["run_a", "run_b", "run_c"]) run.reset() } @@ -348,14 +377,11 @@ struct PassiveSubmissionOutcomeIntegrationTests { let run = session() run.load(messages: [], conversationID: "conversation") run.draft = "A" - let submission = try #require(run.submit()) + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) try await displaceA(run, submission: submission) - let outcome = await Runner.waitForTerminal( - run: run, submission: submission, - config: .init(timeoutPerTool: .milliseconds(80), pollInterval: .milliseconds(5)) - ) + let (outcome, ticket) = await waitForTimeoutTicket(run, submission: submission) #expect(outcome == .timedOut) - #expect(run.cancelTimedOutSubmission(submission)) + #expect(ticket?.consume() == true) try await wait { PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel") } #expect(run.currentRunID == "run_b") assertNoAction(for: ["run_b"]) @@ -386,7 +412,7 @@ struct PassiveSubmissionOutcomeIntegrationTests { let run = session() run.load(messages: [], conversationID: "conversation") run.draft = "A" - let submission = try #require(run.submit()) + 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"), @@ -400,12 +426,12 @@ struct PassiveSubmissionOutcomeIntegrationTests { run.draft = "C" let c = try #require(run.submit()) try await wait { c.runID == "run_c" && run.currentRunID == "run_c" } - // Timeout policy is already exercised above. This direct dispatch - // proves the more important authority condition deterministically: - // once B is terminal and C owns visible state, A's captured handle - // still authorizes exactly one A-only cancellation. - #expect(run.cancelTimedOutSubmission(submission)) - #expect(!run.cancelTimedOutSubmission(submission)) + // 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") @@ -434,10 +460,12 @@ struct PassiveSubmissionOutcomeIntegrationTests { } let run = session() run.draft = "A" - let submission = try #require(run.submit()) + let submission = try #require(run.submit(timeoutAfter: .milliseconds(80))) try await wait { submission.runID == "run_a" } - #expect(run.cancelTimedOutSubmission(submission)) - #expect(!run.cancelTimedOutSubmission(submission)) + 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 } @@ -465,11 +493,11 @@ struct PassiveSubmissionOutcomeIntegrationTests { } let run = session() run.draft = "A" - let terminal = try #require(run.submit()) + 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 } - #expect(!run.cancelTimedOutSubmission(terminal)) + // A terminal run can never mint a deadline ticket. #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_a/cancel")) PassiveOutcomeProtocol.reset() @@ -490,7 +518,7 @@ struct PassiveSubmissionOutcomeIntegrationTests { try await wait { reset.runID == "run_b" } run.reset() try await wait { PassiveOutcomeProtocol.stopped("/v1/runs/run_b/events") } - #expect(!run.cancelTimedOutSubmission(reset)) + // Reset detaches the stream before another deadline can mint a ticket. #expect(!PassiveOutcomeProtocol.paths().contains("/v1/runs/run_b/cancel")) } @@ -518,7 +546,156 @@ struct PassiveSubmissionOutcomeIntegrationTests { if case .failed = submission.lifecycle { return true } return false } - #expect(!run.cancelTimedOutSubmission(submission)) + // 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")) } diff --git a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift index 8225b09c..6f251dd4 100644 --- a/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift +++ b/macapp/Tests/GoCodeUITests/RunSubmissionTests.swift @@ -253,7 +253,10 @@ struct RunSubmissionTests { #expect(submission.isTerminal) #expect(submission.state == .terminal("run_a")) - session.cancelTimedOutSubmission(submission) + // 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() } @@ -505,4 +508,18 @@ struct RunSubmissionTests { #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")) + } }