Skip to content

[AI-2193] CLI: hand Claude's SessionEnd hook off to a detached continuation - #649

Merged
alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-2193-kcap-hook-claude-session-end-is-killed-by-claude-codes-15s
Aug 23, 2026
Merged

alexeyzimarev merged 2 commits into
mainfrom
alexeyzimarev/ai-2193-kcap-hook-claude-session-end-is-killed-by-claude-codes-15s

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

Fixes AI-2193. (No GitHub issue — the bug was filed in Linear directly.)

What & why

At the end of every interactive Claude Code session:

SessionEnd hook [kcap hook --claude --no-update-check] failed: Hook cancelled

and the session never receives /hooks/session-end — it sits active until the stale sweep abandons it ~30 min later.

Claude Code (2.1.241, verified against the installed binary) runs SessionEnd hooks on shutdown, /clear and resume under AbortSignal.timeout(getSessionEndHookTimeoutMs()). That function reads hook timeouts from settings.json and agent hooks only; plugin hooks.json entries are merged for matching but not for the grace computation, so kcap's "timeout": 15 is never seen and the grace is the 1.5 s floor. The session-end path was budgeted for 15 s: resolve server URL (git) → drain spools → kill the watcher → inline-drain the transcript tail → enrich → POST. On any real session that exceeds 1.5 s, and the ordering made it worse than a lost message: the hook killed the watcher — whose parent-exit watchdog would otherwise have posted session-end — and was then killed itself before its own POST.

One thing beyond the ticket: before ClaudeHookCommand even runs, Program.cs does ResolveServerUrl (two git calls, ≤1 s each) and AgentHookPoster.DrainSpoolsAsync (a 1.5 s network budget). Either alone can spend the grace, so the hand-off lives in Program.cs ahead of both.

Changes

  • Harness/Claude/ClaudeSessionEndHandoff.cs (new) — for a SessionEnd payload, the hook re-invokes itself as kcap hook --claude --no-update-check --detached, pipes the payload to the child's stdin (no inherited handles, same cwd, no KCAP_URL overlay — the continuation resolves the URL itself) and exits 0. The continuation redirects its output to the session log (logs/{sid}.log, where the watcher's already goes), setsid()s so a closing terminal can't SIGHUP it, and then runs the unchanged session-end path under the existing 15 s HookBudget — spool fallback and ended_at idempotency carry over untouched. A failed spawn falls back to the inline path.
  • Program.cs — reads hook --claude stdin once, before URL resolution and the global spool drain; detached → enter; SessionEnd → hand off; everything else replays the body to the dispatcher.
  • WatcherManager.StartProcess made internal so the hand-off shares the existing ProcessStarterForTesting seam; stale budget comments on PreHookDrainCap/HookBudget corrected; docs/CHANGES.md entry.

Only SessionEnd is handed off: SubagentStop is already async in hooks.json, and the other events honour their timeouts. Not in scope (per the ticket): writing the hook into settings.json, and the upstream Claude Code bug report.

Tests

  • ClaudeSessionEndHandoffTests (unit, 13) — hand-off decision per event name / malformed payload / --detached; spawn shape (this binary, hook args + --detached, all std streams redirected, no KCAP_URL contributed); payload reaches the child's stdin (via a /bin/sh stand-in); spawn failure is reported, not thrown.
  • ClaudeSessionEndHandoffTests (integration) — real binary against WireMock that stalls /hooks/session-end for 3 s: the hook must exit 0 in < 1 s with empty stdout, the POST must still arrive with session_id/reason/ended_at, and the continuation's output lands in the session log. Fails without the fix (hook took ~4 s).

Measured

AOT binary, 4 MB transcript, unreachable server: 37 ms warm (756 ms on the first cold exec of a freshly published binary). The continuation ran on its own — pre-drain cap elapsed, session-end spooled for replay — and no process was left behind.

AOT publish clean (no IL warnings). Full CLI unit + integration suites: the only failures (7 + 1, all work-items-nudge session-start output tests) fail identically on untouched main on this machine.

🤖 Generated with Claude Code

Claude Code computes the grace it gives SessionEnd hooks from settings.json
hook timeouts only; a plugin's hooks.json timeout is used for matching but
never for that computation, so kcap's SessionEnd hook gets the 1.5 s floor
and is killed ("Hook cancelled") — after it has already killed the watcher
whose parent-exit watchdog would otherwise have posted session-end. Sessions
then sat active until the stale sweep abandoned them.

`kcap hook --claude` now reads its payload, re-invokes itself with
`--detached`, pipes the payload to that child and exits — before the
server-URL git probes and the global spool drain that Program.cs runs ahead
of every hook, either of which alone can spend the grace. The continuation
runs the unchanged session-end path (spool fallback and ended_at idempotency
included) under the 15 s HookBudget that used to be the hook's, with its
output in the session log and its own session so neither Claude's abort nor
a closing terminal reaches it.

Measured with the AOT binary on a 4 MB transcript: 37 ms warm. The
integration test stalls /hooks/session-end for 3 s and asserts the hook
returns in under 1 s while the POST still arrives.

AI-2193

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Aug 23, 2026

Copy link
Copy Markdown

AI-2193

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix Claude SessionEnd hook cancellation via detached continuation hand-off

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Hand off Claude SessionEnd hook to a detached kcap continuation to outlive Claude’s 1.5s grace.
• Read and replay Claude hook stdin before URL resolution and global spool draining.
• Add unit/integration coverage plus changelog note for reliable /hooks/session-end delivery.
Diagram

graph TD
  A["Claude Code"] --> B["kcap hook (parent)"] -->|"spawn --detached"| C["kcap hook (detached)"] --> D["Server: /hooks/session-end"]
  B -->|"stdin payload"| C
  C --> E["logs/{sid}.log"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Move SessionEnd POST responsibility entirely to the watcher
  • ➕ Avoids spawning an extra short-lived continuation process
  • ➕ Watcher already persists across hook lifecycle events
  • ➖ Requires larger behavioral change and careful ordering/idempotency work
  • ➖ Watcher may still be terminated earlier than desired in some shutdown paths
2. Out-of-process daemon queue for lifecycle events (durable background worker)
  • ➕ Makes lifecycle delivery resilient to all hook timeouts/aborts
  • ➕ Could unify spool draining and retries across vendors
  • ➖ Significant infrastructure/operational complexity compared to targeted fix
  • ➖ More moving parts for a single vendor-specific edge case
3. Try to rely on Claude hook timeout configuration only
  • ➕ No code changes; simplest in theory
  • ➖ Not viable here: Claude ignores plugin hooks.json timeout for SessionEnd grace (root cause)

Recommendation: Proceed with the detached continuation hand-off. It is the smallest change that directly addresses Claude’s unconfigurable 1.5s SessionEnd grace while preserving the existing session-end logic (spool fallback + ended_at idempotency) and adding strong real-binary integration coverage.

Files changed (8) +350 / -9

Bug fix (2) +121 / -1
ClaudeSessionEndHandoff.csAdd detached continuation hand-off for Claude SessionEnd +105/-0

Add detached continuation hand-off for Claude SessionEnd

• Implements detection of SessionEnd payloads, spawning of a detached re-invocation of the hook with stdin-piped payload, and continuation setup (log redirection + terminal detachment). Falls back to inline execution if spawn fails.

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs

Program.csRead Claude hook stdin early and hand off SessionEnd before pre-hook work +16/-1

Read Claude hook stdin early and hand off SessionEnd before pre-hook work

• Reads stdin once for 'hook --claude' before server URL resolution and global spool drain, performs detached entry/hand-off decisions, and replays the body to the Claude dispatcher via StringReader.

src/Capacitor.Cli/Program.cs

Refactor (2) +9 / -8
ClaudeHookCommand.csUpdate PreHookDrainCap commentary for detached SessionEnd path +6/-5

Update PreHookDrainCap commentary for detached SessionEnd path

• Refreshes the rationale comments around the pre-POST drain cap to reflect that SessionEnd now runs under the detached continuation budget, while SubagentStop remains inline.

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs

WatcherManager.csExpose StartProcess internally for shared process-spawn seam +3/-3

Expose StartProcess internally for shared process-spawn seam

• Makes StartProcess internal and updates comments so ClaudeSessionEndHandoff can reuse the existing ProcessStarterForTesting seam for deterministic spawn testing.

src/Capacitor.Cli/WatcherManager.cs

Tests (2) +201 / -0
ClaudeSessionEndHandoffTests.csAdd real-binary integration test for SessionEnd hand-off timing +88/-0

Add real-binary integration test for SessionEnd hand-off timing

• Uses WireMock to stall '/hooks/session-end' beyond the hook’s grace, asserting the hook returns quickly while the detached continuation still delivers the POST and writes to the session log.

test/Capacitor.Cli.Tests.Integration/ClaudeSessionEndHandoffTests.cs

ClaudeSessionEndHandoffTests.csAdd unit tests for hand-off decision and spawn behavior +113/-0

Add unit tests for hand-off decision and spawn behavior

• Covers SessionEnd detection, detached recursion prevention, spawn argument/stdio configuration, stdin piping, and fail-open behavior on spawn errors using the WatcherManager process-start test seam.

test/Capacitor.Cli.Tests.Unit/Harness/Claude/ClaudeSessionEndHandoffTests.cs

Documentation (2) +19 / -0
CHANGES.mdDocument Claude SessionEnd detached hand-off rationale +14/-0

Document Claude SessionEnd detached hand-off rationale

• Adds a changelog entry explaining why Claude SessionEnd hooks are killed early and how kcap now hands off the work to a detached continuation to ensure the session-end POST completes.

docs/CHANGES.md

HookBudget.csClarify HookBudget semantics for Claude SessionEnd +5/-0

Clarify HookBudget semantics for Claude SessionEnd

• Extends documentation to note that the 15s SessionEnd ceiling now applies to the detached continuation rather than the original Claude hook process.

src/Capacitor.Cli/Commands/HookBudget.cs

@qodo-code-review

qodo-code-review Bot commented Aug 23, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Spawn failure leaves orphan ✓ Resolved 🐞 Bug ☼ Reliability
Description
ClaudeSessionEndHandoff.TrySpawn can start the detached continuation and then throw while
writing/closing stdin/stdout/stderr, returning false and causing the caller to run the inline path
while the detached process may still be running. This can orphan a background process and/or
double-send lifecycle events.
Code

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[R71-74]

+            process.StandardInput.Write(body);
+            process.StandardInput.Close();
+            process.StandardOutput.Close();
+            process.StandardError.Close();
Evidence
The code starts a process and then performs multiple operations that can throw; any exception
returns false without killing the already-started process, leading to the inline fallback running
concurrently with the detached process.

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[63-81]

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

### Issue description
`TrySpawn` wraps start + pipe writes in a single try/catch and returns `false` on any exception. If the process was already started and an exception occurs while writing/closing stdin (or closing the parent’s read ends), the spawned child can remain running while the hook falls back to inline handling, creating duplicate side effects and orphan risk.

### Issue Context
The parent hook returns quickly and the continuation is intended to be the only executor of SessionEnd work.

### Fix Focus Areas
- src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[46-81]

### Suggested fix
- After `StartProcess`, wrap the stdin write/close in a nested try; on failure, attempt `process.Kill(entireProcessTree: true)` (best-effort) and dispose the Process object.
- Consider returning `true` only after successfully writing and closing stdin.

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


2. File.ReadAllText reads session log ✓ Resolved 📘 Rule violation ☼ Reliability Common mistakes to avoid
Description
The new integration test reads the session log via File.ReadAllText, which opens with restrictive
sharing on Windows and can block the still-running continuation/watcher from writing. This can cause
flaky or deadlocking Windows CI runs when the log is still being appended.
Code

test/Capacitor.Cli.Tests.Integration/ClaudeSessionEndHandoffTests.cs[R72-75]

+        var log = Path.Combine(configDir, "logs", $"{sid}.log");
+        await Assert.That(File.Exists(log)).IsTrue();
+        await Assert.That(File.ReadAllText(log)).Contains($"Inline drain for {sid}");
+    }
Evidence
PR Compliance ID 23 requires agent-written files be read with write-sharing enabled (not
File.ReadAllText*). The new test reads the session log using File.ReadAllText(log), which can
deny concurrent writers on Windows while the detached continuation may still be writing.

Rule Common mistakes to avoid: Agent-Owned Files Must Be Read With Write-Sharing Enabled (No File.ReadAllText on Agent-Written Files)
test/Capacitor.Cli.Tests.Integration/ClaudeSessionEndHandoffTests.cs[71-75]
src/Capacitor.Cli.Core/SharedFileText.cs[10-18]

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

## Issue description
`ClaudeSessionEndHandoffTests` reads a kcap-written log file using `File.ReadAllText`, which can deny write-sharing on Windows and cause CI flakiness/deadlocks.

## Issue Context
The repo has a dedicated helper (`Capacitor.Cli.Core.SharedFileText.ReadAllText`) that reads with `FileShare.ReadWrite | FileShare.Delete` for agent-/tool-written files.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Integration/ClaudeSessionEndHandoffTests.cs[71-75]

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


3. Broken pipes can crash ✓ Resolved 🐞 Bug ☼ Reliability
Description
The parent closes its read ends of the detached continuation’s stdout/stderr pipes immediately, and
EnterDetached swallows log setup failures without ensuring output is redirected or suppressed. If
log redirection fails (permissions, disk issues) or anything writes before redirection, the
continuation may throw on broken stdout/stderr and abort before posting session-end.
Code

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[R96-99]

+            var logWriter = new StreamWriter(Path.Combine(logDir, $"{sessionId ?? "claude-session-end"}.log"), append: true) { AutoFlush = true };
+            Console.SetOut(logWriter);
+            Console.SetError(logWriter);
+        } catch {
Evidence
The parent closes the detached process’s stdout/stderr pipes, while EnterDetached intentionally
ignores failures to redirect output. The Claude hook implementation later performs unguarded console
error writes, which can fail if still pointed at broken pipes.

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[71-75]
src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[89-103]
src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[107-110]

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

### Issue description
The detached continuation is started with redirected stdout/stderr (private pipes), and the parent closes its read ends right away. That means any write by the child to stdout/stderr becomes a broken-pipe scenario unless the child redirects output elsewhere immediately. However, `EnterDetached` treats log setup as best-effort and swallows errors; if it fails, later `Console.Error.WriteLineAsync(...)` calls can throw and terminate the continuation.

### Issue Context
The continuation is expected to reliably finish session-end even under degraded local conditions.

### Fix Focus Areas
- src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[71-75]
- src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[89-103]

### Suggested fix
- If log setup fails, explicitly set `Console.SetOut(TextWriter.Null)` and `Console.SetError(TextWriter.Null)` to avoid writes to the dead pipes.
- Optionally, don’t close the parent’s `StandardOutput`/`StandardError` until after the child has acknowledged it has redirected output (more complex), or redirect to a file/null at process start (if platform/runtime supports).
- Consider adding a very early `EnterDetached` path (before any other code that might emit output) remains true.

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



Remediation recommended

4. Unsanitized log filename ✓ Resolved 🐞 Bug ⛨ Security
Description
ClaudeSessionEndHandoff.EnterDetached uses session_id from the hook payload directly in a log file
name, allowing path separators to escape the logs directory if session_id is malformed or
attacker-controlled. This can overwrite arbitrary files accessible to the user running kcap.
Code

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[R94-97]

+            var logDir = PathHelpers.ConfigPath("logs");
+            Directory.CreateDirectory(logDir);
+            var logWriter = new StreamWriter(Path.Combine(logDir, $"{sessionId ?? "claude-session-end"}.log"), append: true) { AutoFlush = true };
+            Console.SetOut(logWriter);
Evidence
The continuation extracts session_id from JSON and uses it directly as part of the log filename,
without stripping path separators. This is newly introduced behavior in the hand-off code path.

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[90-98]

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

### Issue description
`EnterDetached` builds `logs/{sessionId}.log` using `session_id` from JSON with only `-` removed. If `session_id` contains path separators (e.g. `../`), `Path.Combine` can escape the intended logs directory.

### Issue Context
The session-end continuation writes to a per-session log under the config directory.

### Fix Focus Areas
- src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[90-99]

### Suggested fix
- Normalize `sessionId` to a safe filename component (e.g. allow only `[A-Za-z0-9]` and replace others with `_`), or use `Path.GetFileName(sessionId)` plus a strict allowlist.
- Consider a defensive fallback to a fixed filename if sanitization produces empty output.

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


5. Overlong narrative comments added ✓ Resolved 📘 Rule violation ⚙ Maintainability Dos and donts
Description
New multi-paragraph comments (including XML remarks) are long and narrate behavior and history
beyond concise rationale, increasing maintenance cost. This conflicts with the guideline to keep
comments short and focused on “why”.
Code

src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[R11-16]

+/// <remarks>
+/// Claude Code runs SessionEnd hooks on shutdown, <c>/clear</c> and resume under a grace it
+/// computes from <c>settings.json</c> hook timeouts only — a plugin's <c>hooks.json</c> timeout is
+/// matched but never read — so a plugin-sourced hook gets the 1.5 s floor, then is killed. The
+/// session-end path (server-URL git probe, spool drain, auth, watcher kill, transcript drain,
+/// POST) cannot fit, and killing the watcher before the POST left nothing to end the session.
Evidence
PR Compliance ID 25 requires comments to be short and explain why rather than narrating what. The
newly added comment blocks are multi-paragraph and restate the end-to-end flow in detail instead of
keeping to a brief rationale.

Rule Dos and donts: Comments Must Be Sparse, Short, and Explain Why (Not What)
src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[11-22]
src/Capacitor.Cli/Program.cs[80-84]

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

## Issue description
Several newly added comments are lengthy and narrate mechanics/history; per the checklist they should be sparse and primarily capture non-obvious rationale.

## Issue Context
This PR introduces a detached SessionEnd continuation; the key invariant can be preserved with a shorter comment pointing to the external behavior/constraint (Claude’s 1.5s grace) without restating the whole flow.

## Fix Focus Areas
- src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs[11-22]
- src/Capacitor.Cli/Program.cs[80-84]

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


Grey Divider

Tip of the day
💡 Did you know, you can turn these tips off under Display preferences

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs Outdated
Comment thread src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs Outdated
Comment thread src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs
Comment thread src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs Outdated
… null writer fallback

- TrySpawn kills (best-effort) and disposes a child that started but never
  received the full payload, so the inline fallback is the only owner.
- The continuation's log name is the watcher key only for an alphanumeric
  session id; anything else uses a fixed name inside the logs directory.
- A failed log open falls back to TextWriter.Null rather than leaving the
  console on the closed pipes.
- Shorter comments; the integration test reads the log through
  SharedFileText.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alexeyzimarev
alexeyzimarev merged commit 70c0257 into main Aug 23, 2026
6 checks passed
@alexeyzimarev
alexeyzimarev deleted the alexeyzimarev/ai-2193-kcap-hook-claude-session-end-is-killed-by-claude-codes-15s branch August 23, 2026 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant