diff --git a/docs-ai/013-prowl-cli/contracts/agents-wait.md b/docs-ai/013-prowl-cli/contracts/agents-wait.md index bc7b1ba5..c01a7281 100644 --- a/docs-ai/013-prowl-cli/contracts/agents-wait.md +++ b/docs-ai/013-prowl-cli/contracts/agents-wait.md @@ -69,10 +69,11 @@ The pane is resolved once to a stable target. `changed` requires a post-baseline `exit` requires the surface to stop being live. Exact surface closure satisfies `exit`; for `idle`, `blocked`, or `changed` it returns structured condition-mode `AGENT_GONE` immediately. Exact/high current-epoch cooperative evidence wins. `auto` may fall back to a heuristic -idle/blocked/exit match only after the observed state and revision remain unchanged for two -seconds, and only while no `verified_live` channel holds a terminal signal for that condition -(a channel that has only reported `session-start`, or whose active level is another event, -does not suppress the fallback); `changed` never falls back while such a channel exists. +idle/blocked match only after the observed state and revision remain unchanged for two +seconds, and only while no covering `verified_live` channel holds a terminal signal (a channel +that has only reported `session-start` does not suppress the fallback; one holding an opposite +level does, so exact evidence is never overridden by the screen). `changed` and `exit` never +fall back while such a channel exists: `exit` then resolves on `session-end` or surface closure. Higher minimum-confidence settings reject weaker evidence rather than relabelling it. diff --git a/docs-ai/064-agent-completion-signals/013-idle-evidence-fallback.md b/docs-ai/064-agent-completion-signals/013-idle-evidence-fallback.md index ee54f5f6..1abae065 100644 --- a/docs-ai/064-agent-completion-signals/013-idle-evidence-fallback.md +++ b/docs-ai/064-agent-completion-signals/013-idle-evidence-fallback.md @@ -38,7 +38,7 @@ person's in-flight keystrokes land in the new shell. | # | Decision | Alternatives rejected | | --- | --- | --- | | C1 | `decodeClaude` accepts the same `Notification` types as the S3b decoder (`permission_prompt`, `elicitation_dialog`); `idle_prompt` throws `unsupportedEvent` and the hook ingress drops it. One shared `acceptedAttentionNotifications` set replaces the two lists. | Mapping `idle_prompt` to an idle-flavoured signal (a new event on the public `agents signal` schema for a runtime-specific timer); special-casing the detail string inside the wait handler. | -| C2 | Under `auto`, a `verified_live` channel suppresses the stabilized heuristic fallback only while the pane's active terminal signal *is* the condition's covered event (`turn-ended` for `idle`, `needs-input` for `blocked`, `session-end` for `exit`); `changed` stays suppressed whenever such a channel exists. A channel with no terminal level, or another level, cannot describe the current state, so the two-second detector view decides. | Keeping the advertised-event rule and documenting `--min-confidence heuristic` as the launch recipe (leaves `auto` — the default — wrong for the skill's own flow); dropping suppression entirely (would let a detector guess pre-empt an exact `changed` edge). | +| C2 | Under `auto`, a covering `verified_live` channel suppresses the stabilized heuristic fallback for `changed` and `exit` always, and for `idle`/`blocked` whenever the pane holds *any* active terminal signal: the condition's own event resolves through the exact path, and an opposite event (a `needs-input` during an idle wait, a `turn-ended` during a blocked wait) means the runtime disagrees with the screen, which the detector must not override. Only a channel with no terminal level — a freshly launched, unprompted Profile that has reported `session-start` alone — leaves the current state to the two-second detector view. | Keeping the advertised-event rule and documenting `--min-confidence heuristic` as the launch recipe (leaves `auto` — the default — wrong for the skill's own flow); allowing the fallback whenever the level is not the condition's own event (review P1: a fresh opposite exact signal could be out-voted by 1.8 s of stale detector state); dropping suppression entirely (would let a detector guess pre-empt an exact `changed` edge). | | C3 | Docs and the skill say `worktree.id` for automation, warn that plain creates take focus, name `.data.anchor.pane.id`, and note that a receipt may precede Codex's own `turn-ended`. Plain `--background` stays a product follow-up. | Implementing `--background` for plain shells in this PR (feature, not a fix). | Note that C2 alone is not enough for (1): with `idle_prompt` recorded, the active level is a @@ -51,8 +51,10 @@ path intact; C2 covers the no-level case that C1 cannot. - `AgentNativeHookDecoder`: `acceptedAttentionNotifications` (`elicitation_dialog`, `permission_prompt`) is the single accepted list for Claude Code and the S3b runtimes. - `AgentWaitCommandHandler.allowsHeuristic` takes the current `ConditionSnapshot`: with a - covering `verified_live` channel it returns `false` for `changed`, and otherwise `false` only - when `snapshot.signal?.event == coveredEvent`. + covering `verified_live` channel it returns `false` for `changed` and `exit`, and for + `idle`/`blocked` returns `true` only while `snapshot.signal == nil`. A signal arriving + mid-stabilization therefore also resets the `HeuristicStabilizer` (its candidate becomes + `nil`). - Docs: `docs/components/cli.md` (targeting model, wait fallback, dispatch/receipt timing, `create tab` focus note, anchor field), `docs/components/agent-detection.md` (Claude notification types), `skills/prowl-cli/SKILL.md`, and the `agents-wait` contract page. @@ -80,6 +82,22 @@ path intact; C2 covers the no-level case that C1 cannot. (`e2e/phase1*.log`, `dispatch-*.log`, `s*.log`, `burst*.log`) and in the independent validation run under `/tmp/prowl-independent-validation-20260828-235005`. +## Review + +One adversarial round on the PR, both findings fixed test-first: + +- P1 — the first cut allowed the fallback whenever the active level was not the condition's + own event, so 1.8 s of stale `idle` detector state plus a freshly arrived exact `needs-input` + resolved `idle` heuristically at the two-second mark (and `blocked` symmetrically on a fresh + `turn-ended`). Now any terminal level suppresses the fallback; + `autoIdleDoesNotOverrideFreshNeedsInputDuringStabilization` and + `autoIdleDoesNotOverridePreArmNeedsInputWhileLiveChannelIsPresent` pin it. A pre-arm opposite + level stays suppressed as before this change rather than letting the detector out-vote it. +- P2 — the same cut let `exit` fall back on a live surface whose agent merely left the detector, + contradicting the contract (`exit` needs `session-end` or surface closure) without a test. + `exit` now stays suppressed with a covering channel, as it was; + `autoExitWaitsForSessionEndWhileLiveChannelIsPresent` pins it. + ## Observed but not changed - A plain `create tab`/`create pane` has no `--background`; while a person types in the app, diff --git a/docs/components/cli.md b/docs/components/cli.md index 6136c0ed..778e5aed 100644 --- a/docs/components/cli.md +++ b/docs/components/cli.md @@ -334,13 +334,14 @@ or blocked), while a signal arriving after arming counts on its own. To wait for turn edge rather than the current state, use `--until changed`, which needs a post-baseline revision or a newer signal — under `auto` with a `verified_live` channel it returns at the next runtime signal, not at a screen change. `auto` may fall back to a heuristic result — the -detector's view after the pane has remained unchanged for two seconds — whenever no -`verified_live` channel holds a terminal signal for the condition: right after a Profile -launch, when the channel has only reported `session-start`, or when its active level is a -different event. While the channel holds the condition's own level (`turn-ended` for `idle`, -`needs-input` for `blocked`), that signal decides — with detector corroboration when it -predates the wait — or the next runtime signal does; `changed` never falls back while a -`verified_live` channel exists. When the pane hosts no detected agent yet +detector's view after the pane has remained unchanged for two seconds — only while no +covering `verified_live` channel holds a terminal signal: right after a Profile launch, when +the channel has only reported `session-start`. Once the channel holds a terminal level, that +level decides: the condition's own event (`turn-ended` for `idle`, `needs-input` for +`blocked`) resolves the wait, with detector corroboration when it predates the wait, and an +opposite event is never overridden by the screen — the wait then ends at the next runtime +signal. `changed` and `exit` never fall back while such a channel exists (`exit` resolves on +`session-end` or surface closure). When the pane hosts no detected agent yet (typically right after launching one), the wait keeps polling for up to ten seconds, bounded by `--timeout`, before failing with `AGENT_NOT_FOUND`. `--include-screen` samples the detection buffer until it is stable for 800 ms (or the two-second cap), then returns the diff --git a/skills/prowl-cli/SKILL.md b/skills/prowl-cli/SKILL.md index 0ef035c9..6a89321e 100644 --- a/skills/prowl-cli/SKILL.md +++ b/skills/prowl-cli/SKILL.md @@ -189,7 +189,7 @@ result="$(prowl agents wait "$pane" --until idle --include-screen 40 --timeout 6 printf '%s\n' "$result" | jq '.data.observation, .data.screen' ``` - `--until idle|blocked` observe the current state: a signal that already existed when the wait was armed counts only if the screen detector agrees, a signal arriving afterwards counts on its own, and an already-idle agent with such a signal returns immediately. Detection-only evidence (no hook or cooperative signal, the usual case for a manually launched agent) resolves only after the state has stayed unchanged for two seconds. The same stabilized fallback applies to a Profile agent whose `verified_live` channel holds no signal for the condition yet — a freshly launched, unprompted Profile has only reported `session-start` — so `--until idle` before the first prompt resolves with `confidence: heuristic`. Give those waits a `--timeout` of at least a few seconds. To wait for the *next* turn edge (for example after `send`ing a new prompt), use `--until changed`; with a `verified_live` hook channel it returns at the next runtime signal, not at a screen change. + `--until idle|blocked` observe the current state: a signal that already existed when the wait was armed counts only if the screen detector agrees, a signal arriving afterwards counts on its own, and an already-idle agent with such a signal returns immediately. Detection-only evidence (no hook or cooperative signal, the usual case for a manually launched agent) resolves only after the state has stayed unchanged for two seconds. The same stabilized fallback applies to a Profile agent whose `verified_live` channel holds no terminal signal yet — a freshly launched, unprompted Profile has only reported `session-start` — so `--until idle` before the first prompt resolves with `confidence: heuristic`; once the channel holds a `turn-ended` or `needs-input`, that runtime evidence decides and the screen never overrides it. Give those waits a `--timeout` of at least a few seconds. To wait for the *next* turn edge (for example after `send`ing a new prompt), use `--until changed`; with a `verified_live` hook channel it returns at the next runtime signal, not at a screen change. Exact/high evidence can establish the requested observable condition. If `jq -e '.data.observation.confidence == "heuristic"'` matches, inspect the included stable diff --git a/supacode/CLIService/AgentWaitCommandHandler.swift b/supacode/CLIService/AgentWaitCommandHandler.swift index d64ad3de..94e2bdc2 100644 --- a/supacode/CLIService/AgentWaitCommandHandler.swift +++ b/supacode/CLIService/AgentWaitCommandHandler.swift @@ -620,11 +620,14 @@ final class AgentWaitCommandHandler: CommandHandler { } } - /// Whether `auto` may fall back to the stabilized screen detector. A `verified_live` channel - /// that covers the condition's event is authoritative for `changed` (the runtime reports the - /// next edge) and, for a level condition, only while the pane's active terminal signal is that - /// event: a channel that has reported nothing terminal yet — a freshly launched, unprompted - /// Profile — or whose level is another event cannot describe the current state. + /// Whether `auto` may fall back to the stabilized screen detector. A covering `verified_live` + /// channel reports the next edge itself (`changed`) and its own `session-end` (`exit`), so + /// those never fall back. For `idle` and `blocked` the channel is authoritative while it holds + /// any terminal level: the condition's own event resolves through the exact path, and an + /// opposite event means the runtime disagrees with the screen, which a stabilized detector + /// view must not override. Only a channel with no terminal level yet — a freshly launched, + /// unprompted Profile that has reported `session-start` alone — leaves the current state to + /// the detector. private func allowsHeuristic( _ minimum: AgentWaitMinimumConfidence, condition: AgentWaitCondition, @@ -647,7 +650,12 @@ final class AgentWaitCommandHandler: CommandHandler { $0.state == .verifiedLive && (condition == .changed || $0.events.contains(coveredEvent)) } guard liveChannelCovers else { return true } - return condition != .changed && snapshot.signal?.event != coveredEvent + switch condition { + case .changed, .exit: + return false + case .idle, .blocked: + return snapshot.signal == nil + } } } diff --git a/supacodeTests/AgentWaitCommandHandlerTests.swift b/supacodeTests/AgentWaitCommandHandlerTests.swift index 41b6dfc1..dee7e04e 100644 --- a/supacodeTests/AgentWaitCommandHandlerTests.swift +++ b/supacodeTests/AgentWaitCommandHandlerTests.swift @@ -723,9 +723,9 @@ struct AgentWaitCommandHandlerTests { #expect(condition.observation.source == "detection") } - @Test func autoIdleFallsBackToDetectorWhenLiveChannelHoldsAnotherLevel() async throws { - // The channel's active terminal signal is `needs-input`, which says nothing about idleness; - // the detector's stable idle view must still be able to end the wait. + @Test func autoIdleDoesNotOverridePreArmNeedsInputWhileLiveChannelIsPresent() async { + // The channel's active terminal level is an exact `needs-input`: the runtime disagrees with + // the screen, so the stabilized detector view must not end the wait on its own. let clock = TestClock() let target = resolvedTarget() let idle = agentEntry(surfaceID: UUID(uuidString: target.paneID)!, status: .idle) @@ -743,20 +743,79 @@ struct AgentWaitCommandHandlerTests { await handler.handle(envelope: conditionWait(target, .idle, timeout: 3)) } - // Advance past the timeout so a regression surfaces as WAIT_TIMEOUT instead of a hang. for _ in 0..<16 { await Task.yield() await clock.advance(by: .milliseconds(200)) } let response = await task.value - #expect(response.ok, "\(String(describing: response.error))") - let payload = try #require(response.data).decode(as: AgentWaitCommandPayload.self) - guard case .condition(let condition) = payload else { - Issue.record("Expected condition payload") - return + #expect(response.error?.code == CLIErrorCode.waitTimeout) + } + + @Test func autoIdleDoesNotOverrideFreshNeedsInputDuringStabilization() async { + // The detector has shown idle for 1.8 s when an exact `needs-input` lands before the screen + // catches up: exact evidence wins, so the stabilizer resets instead of resolving idle at the + // two-second mark. + let clock = TestClock() + let target = resolvedTarget() + let idle = agentEntry(surfaceID: UUID(uuidString: target.paneID)!, status: .idle) + let fresh = signal(.needsInput, at: Self.start.addingTimeInterval(1.8)) + var snapshotReads = 0 + let handler = AgentWaitCommandHandler( + observeDispatch: { _ in .failure(.notFound) }, + resolveConditionTarget: { _ in .success(target) }, + conditionSnapshot: { _ in + snapshotReads += 1 + // Read 1 is the arm-time baseline and poll read k happens at (k - 2) * 200 ms, so the + // signal lands at 1.8 s — inside the stabilizer's window. + let arrived = snapshotReads > 10 + return .init( + agent: idle, + signal: arrived ? fresh : nil, + revision: arrived ? 3 : 2, + isLive: true, + signals: Self.liveClaudeSignals + ) + }, + clock: clock, + now: { Self.start } + ) + let task = Task { + await handler.handle(envelope: conditionWait(target, .idle, timeout: 3)) } - #expect(condition.waitedMilliseconds == 2_000) - #expect(condition.observation.confidence == "heuristic") + + for _ in 0..<16 { + await Task.yield() + await clock.advance(by: .milliseconds(200)) + } + let response = await task.value + #expect(response.error?.code == CLIErrorCode.waitTimeout) + #expect(snapshotReads > 10) + } + + @Test func autoExitWaitsForSessionEndWhileLiveChannelIsPresent() async { + // A verified_live channel reports its own `session-end`; the detector losing sight of the + // agent on a still-live surface is not an exit under `auto`. + let clock = TestClock() + let target = resolvedTarget() + let handler = AgentWaitCommandHandler( + observeDispatch: { _ in .failure(.notFound) }, + resolveConditionTarget: { _ in .success(target) }, + conditionSnapshot: { _ in + .init(agent: nil, signal: nil, revision: 2, isLive: true, signals: Self.liveClaudeSignals) + }, + clock: clock, + now: { Self.start } + ) + let task = Task { + await handler.handle(envelope: conditionWait(target, .exit, timeout: 3)) + } + + for _ in 0..<16 { + await Task.yield() + await clock.advance(by: .milliseconds(200)) + } + let response = await task.value + #expect(response.error?.code == CLIErrorCode.waitTimeout) } @Test func autoChangedIgnoresDetectorChangesWhileLiveChannelIsPresent() async {