[AI-1005] Durable subagent-stop via bounded POST + spool

[alexeyzimarev]

AI-1005 (CLI side): durable subagent-stop

Companion to the kcap-server fix (kcap-server#829). That PR makes the read model recover from a missing SubagentCompleted (cascade + reaper). This PR reduces the rate at which those completions are dropped in the first place.

Problem

The merged durable-hook-spool work (#187) gave session-start and session-end dedicated bounded-POST + spool-on-transient paths, but subagent-stop was left out — it still fell through to the shared PostWithRetryAsync path, which on failure writes stderr and return 1 with no spool. Under hook-budget pressure / fan-out, a dropped subagent-stop POST permanently loses that subagent's SubagentCompleted, leaving a stuck spinner in the UI.

Fix

A dedicated subagent-stop branch in ClaudeHookCommand.HandleCore (mirrors the session-end block), gated on agent_id being present:

• single bounded PostOnceAsync clamped to HookBudget.Remaining(processStart, "subagent-stop") (5s ceiling);
• on transient failure (unreachable / timeout / 5xx / 408 / 429) → spool.Append(sessionId, "subagent-stop", body) and return 0;
• on permanent 4xx (≠ 408/429) → no spool;
• without agent_id → unchanged (falls through to the shared path).

The existing cross-session drainer replays spooled entries to /hooks/subagent-stop; server-side SubagentCompleted ids are deterministic (UUIDv5 over session+agent), so replay is idempotent — no server changes required.

Pre-drain clamp (also fixed here)

subagent-stop's pre-drain (KillWatcher + InlineDrain) was capped at PreHookDrainCap (8s) while the hook ceiling is only 5s — against a hung server the drain alone could blow the budget before the bounded POST, so the spool path was never reached before Claude's kill. It is now clamped to the remaining budget.

⚠️ Do not "simplify" the clamp to match session-end. The subagent-stop clamp is Math.Max(0, Math.Min(PreHookDrainCap, remaining − HookBudget.Safety)). The extra − Safety is load-bearing for the tight 5s ceiling (session-end's 15s ceiling doesn't need it): without it the drain can consume the whole ~3.5s budget, the POST is skipped, and a 4xx would spuriously spool. This is pinned by subagent_stop_on_4xx_is_not_spooled.

Tests

TDD; mirrors the session_end_* tests. New: subagent_stop_on_5xx_is_spooled_and_returns_zero, subagent_stop_against_hung_server_is_spooled_within_budget (elapsed < 5s), subagent_stop_on_4xx_is_not_spooled, subagent_stop_without_agent_id_is_not_spooled, spooled_subagent_stop_is_replayed_on_next_hook. Full unit suite 1806/1806, AOT-clean (no IL2026/IL3050).

🤖 Generated with Claude Code

[linear-code]

AI-1005

[qodo-code-review]

PR Summary by Qodo

Durable subagent-stop: bounded POST + spool with budget-safe pre-drain
🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

Description

• Add bounded subagent-stop POST that spools transient failures for replay.
• Clamp subagent-stop pre-drain to remaining hook budget to preserve POST time.
• Add unit tests for spooling, budget adherence, and replay behavior.

Diagram

graph TD
  A["ClaudeHookCommand"] --> C["HookBudget.Remaining"] --> D["WatcherManager pre-drain"] --> E["PostOnceAsync (bounded)"] --> F["kcap-server hooks"]
  E -- "timeout/5xx/408/429" --> B[("HookSpool")]
  A -- "next hook" --> B --> E

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Extract a shared durable-hook helper (table-driven routes)

• ➕ Reduces duplicated bounded-POST + transient/permanent classification logic across session-end/subagent-stop.
• ➕ Makes it easier to add durability to other hook routes consistently.
• ➖ Adds abstraction/indirection to a timing-sensitive path; harder to reason about per-route budget nuances (e.g., subagent-stop safety reservation).
• ➖ May obscure route-specific behaviors like agent_id gating and different ceilings.

2. Always spool subagent-stop regardless of agent_id presence

• ➕ Simpler control flow; eliminates the agent_id parsing gate.
• ➖ Spools events that do not correspond to a SubagentCompleted, increasing disk churn/noise.
• ➖ Changes existing behavior and may replay non-idempotent/no-op events unexpectedly.

Recommendation: Keep the PR’s route-specific implementation: subagent-stop has unique constraints (5s ceiling + safety reservation + agent_id gating) that benefit from explicit, readable logic. If additional hook routes need durability later, consider a small shared helper for the POST/spool decision only (not for budgeting), to avoid over-abstracting the pre-drain timing rules.

Files changed (2) +97 / -2

Bug fix (1) +48 / -2

ClaudeHookCommand.csAdd durable subagent-stop path with budget-safe pre-drain and spooling +48/-2

Add durable subagent-stop path with budget-safe pre-drain and spooling

• Clamps subagent-stop pre-drain to the remaining hook budget minus safety so the bounded POST still has time to run. Adds a dedicated subagent-stop handling block that performs a single PostOnceAsync bounded by HookBudget.Remaining and spools the payload on transient failures (timeout/unreachable/5xx/408/429), while avoiding spooling on permanent 4xx errors. Leaves behavior unchanged when agent_id is missing by falling through to the shared retry path.

src/Capacitor.Cli/Commands/ClaudeHookCommand.cs

Tests (1) +49 / -0

ClaudeHookCommandTests.csAdd unit tests for subagent-stop spooling, budget clamp, and replay +49/-0

Add unit tests for subagent-stop spooling, budget clamp, and replay

• Introduces tests verifying subagent-stop spools on 5xx and hung server scenarios while returning exit code 0 within the 5s ceiling. Adds negative coverage ensuring 4xx responses and missing agent_id do not spool, and validates that spooled subagent-stop entries are replayed (drained) on the next hook.

test/Capacitor.Cli.Tests.Unit/ClaudeHookCommandTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. README missing subagent-stop spool ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The PR makes subagent-stop durable by spooling transient failures (and returning 0), which is a
user-facing CLI behavior change. README.md still documents durable delivery only for
SessionStart/SessionEnd, so documentation is now inaccurate/out-of-sync.

Code

src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[R577-610]

+        // Dedicated bounded POST for the per-agent subagent-stop: a single attempt clamped to the
+        // remaining hook budget that spools on transient failure, so a dropped SubagentCompleted is
+        // replayed on the next hook (AI-1005). Only the stop carrying agent_id maps to a completion;
+        // without it, fall through to the shared best-effort path (behavior unchanged).
+        if (command == "subagent-stop") {
+            string? sessionId = null, agentId = null;
+            try {
+                var node  = JsonNode.Parse(body);
+                sessionId = node?["session_id"]?.GetValue<string>();
+                agentId   = node?["agent_id"]?.GetValue<string>();
+            } catch { }
+
+            if (sessionId is not null && agentId is not null) {
+                var remaining = HookBudget.Remaining(processStart, command);
+                HttpResponseMessage? resp = null;
+                try {
+                    if (remaining > TimeSpan.Zero) {
+                        using var content = new StringContent(body, Encoding.UTF8, "application/json");
+                        resp = await client.PostOnceAsync($"{baseUrl}/hooks/subagent-stop", content, remaining, CancellationToken.None);
+                    }
+                } catch { resp = null; }
+
+                if (resp is null || !resp.IsSuccessStatusCode) {
+                    var permanent = resp is not null && (int)resp.StatusCode is < 500 and not 408 and not 429;
+                    resp?.Dispose();
+                    if (!permanent) {
+                        spool.Append(sessionId, "subagent-stop", body);
+                        await Console.Error.WriteLineAsync($"[kcap] subagent-stop spooled; will retry on the next kcap hook ({sessionId}/{agentId})");
+                    }
+                    return 0;
+                }
+
+                resp.Dispose();
+                return 0;

Evidence

The new subagent-stop path explicitly spools transient failures (`spool.Append(sessionId,
"subagent-stop", body)) and returns 0`, changing observable CLI behavior. However, the README's
"Durable lifecycle delivery" section describes spooling only for SessionStart/SessionEnd, so it
is not updated to reflect the new durable subagent-stop behavior.

CLAUDE.md: Update README.md in the same PR when user-facing CLI behavior changes
src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[577-610]
README.md[120-126]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR changes user-facing CLI hook behavior: `subagent-stop` is now sent via a bounded POST and spooled on transient failure (durable delivery). The root `README.md` still states durable delivery applies only to `SessionStart`/`SessionEnd`, so docs are now incorrect.

## Issue Context
Compliance requires updating `README.md` in the same PR for user-facing CLI behavior changes.

## Fix Focus Areas
- README.md[120-133]
- src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[577-610]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Subagent-stop dropped on auth hang ✓ Resolved 🐞 Bug ☼ Reliability

Description

The new durable subagent-stop POST+spool branch only runs inside HandleCore after an
HttpClient is created, but HandleWithDeps returns early when client creation exceeds the hook
budget without spooling subagent-stop. This can still permanently lose SubagentCompleted during
auth/config hangs or outages (exactly when the hook budget is tight).

Code

src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[R581-611]

+        if (command == "subagent-stop") {
+            string? sessionId = null, agentId = null;
+            try {
+                var node  = JsonNode.Parse(body);
+                sessionId = node?["session_id"]?.GetValue<string>();
+                agentId   = node?["agent_id"]?.GetValue<string>();
+            } catch { }
+
+            if (sessionId is not null && agentId is not null) {
+                var remaining = HookBudget.Remaining(processStart, command);
+                HttpResponseMessage? resp = null;
+                try {
+                    if (remaining > TimeSpan.Zero) {
+                        using var content = new StringContent(body, Encoding.UTF8, "application/json");
+                        resp = await client.PostOnceAsync($"{baseUrl}/hooks/subagent-stop", content, remaining, CancellationToken.None);
+                    }
+                } catch { resp = null; }
+
+                if (resp is null || !resp.IsSuccessStatusCode) {
+                    var permanent = resp is not null && (int)resp.StatusCode is < 500 and not 408 and not 429;
+                    resp?.Dispose();
+                    if (!permanent) {
+                        spool.Append(sessionId, "subagent-stop", body);
+                        await Console.Error.WriteLineAsync($"[kcap] subagent-stop spooled; will retry on the next kcap hook ({sessionId}/{agentId})");
+                    }
+                    return 0;
+                }
+
+                resp.Dispose();
+                return 0;
+            }

Evidence

HandleWithDeps explicitly spools only session-start/session-end when client creation exceeds
budget, so subagent-stop never reaches the new durable branch in HandleCore in that scenario.
Given the 5s ceiling for non-session-end hooks, the client-creation budget can be small and time out
during outages, causing subagent-stop drops despite the new bounded POST logic.

src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[55-76]
src/Capacitor.Cli/Commands/HookBudget.cs[10-21]
src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[577-612]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`subagent-stop` durability is implemented only after `HttpClient` creation succeeds (inside `HandleCore`). When `CreateClientWithinBudgetAsync(...)` times out, `HandleWithDeps` currently spools only `session-start` / `session-end`, so `subagent-stop` events are dropped in this failure mode.

## Issue Context
This can happen under the same outage conditions the durability work is meant to mitigate (hung `/auth/config`, token refresh hang, etc.). Since `subagent-stop` has a 5s ceiling (and `HookBudget.Remaining` subtracts 1.5s safety), the client-creation cap can be small and realistically exceeded.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[55-76]
- src/Capacitor.Cli/Commands/ClaudeHookCommand.cs[577-612]
- src/Capacitor.Cli/Commands/HookBudget.cs[10-21]

## What to change
1. In the minimal pre-client parse in `HandleWithDeps`, also extract `agent_id` when `command == "subagent-stop"`.
2. In the `created is null` early-return branch, add a spool path for `subagent-stop` when `sessionId` and `agentId` are present (mirroring the existing `session-start/session-end` behavior).
3. Add a unit test similar to `session_end_spooled_when_client_creation_exceeds_budget`, but for `subagent-stop` (ensuring a spooled entry is written when client creation exceeds budget and `agent_id` is present).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---