-
Notifications
You must be signed in to change notification settings - Fork 1
fix(toolwalk): retain immutable timeout authority #1139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # 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] 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`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
| } | ||
|
Comment on lines
+28
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the cancellation POST fails because of a transient transport or server error, this task suppresses the error and the method has already returned Useful? React with 👍 / 👎. |
||
| return true | ||
| } | ||
|
|
||
| /// Compatibility entry point for programmatic callers that do not retain | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This current-status entry says the repository regression still awaits the #1135 repair, while the newly added issue #1136 plan marks that repair and the complete normal/race/coverage regression as passed. Agents using the active plan cannot tell whether the mandatory gate remains blocked or has completed, so update these records to report one verified state.
AGENTS.md reference: AGENTS.md:L31-L35
Useful? React with 👍 / 👎.