Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ Not release notes. Each entry is written as of the change that produced it and i
code moves on; where an entry disagrees with the code, the code wins.


## Claude SessionEnd hand-off

Claude Code computes the grace it gives SessionEnd hooks from `settings.json` 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 — after it has already killed the watcher whose
parent-exit watchdog would otherwise have ended the session. The hook therefore reads its payload,
re-invokes itself with `--detached`, pipes the payload to that child and exits, all before the
server-URL git probes and the global spool drain that `Program.cs` runs ahead of every hook. 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 can reach it. Only
SessionEnd is handed off: SubagentStop is already `async` in `hooks.json`, and the others honour
their timeouts.

## Review flows and reviewer selection

Review flows use a vendor-neutral catalog-start v2 protocol: reserved `spec-review`/`code-review`
Expand Down
11 changes: 6 additions & 5 deletions src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ namespace Capacitor.Cli.Commands.Harness;
/// </summary>
public static class ClaudeHookCommand {
// Hard ceiling on the best-effort pre-POST drain (watcher kill + inline transcript
// drain) for session-end / subagent-stop. Claude kills the SessionEnd hook at its
// configured timeout (15s); a slow or retrying remote call in the drain could consume
// all of it (the HTTP retry helper alone allows up to 30s) and the session-end POST
// would never be sent, leaving the session stuck "Active". 8s leaves ample headroom to
// send the POST; the server's StopAndDrain + the "kcap import" hint recover the rest.
// drain) for session-end / subagent-stop. Session-end runs in the detached continuation
// (ClaudeSessionEndHandoff) under HookBudget's 15s, subagent-stop inside the hook; either
// way a slow or retrying remote call in the drain could consume the whole budget (the HTTP
// retry helper alone allows up to 30s) and the lifecycle POST would never be sent, leaving
// the session stuck "Active". 8s leaves ample headroom to send the POST; the server's
// StopAndDrain + the "kcap import" hint recover the rest.
static readonly TimeSpan PreHookDrainCap = TimeSpan.FromSeconds(8);

public static Task<int> Handle(string baseUrl, TextReader stdin, long processStart = 0,
Expand Down
5 changes: 5 additions & 0 deletions src/Capacitor.Cli/Commands/HookBudget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ namespace Capacitor.Cli.Commands;
/// Per-event hook-timeout ceilings (mirror kcap/hooks/hooks.json) and a
/// safety-adjusted "remaining" computed from a process-start timestamp, so the
/// hook always leaves time to spool + exit before Claude's kill.
///
/// <para>Claude honours the hooks.json timeout for every event except SessionEnd, which it
/// caps at 1.5 s for plugin hooks; that hook hands off to a detached continuation
/// (<see cref="Cli.Harness.Claude.ClaudeSessionEndHandoff"/>), and the 15 s here is the
/// continuation's budget, not the hook's.</para>
/// </summary>
public static class HookBudget {
public static readonly TimeSpan Safety = TimeSpan.FromMilliseconds(1500);
Expand Down
113 changes: 113 additions & 0 deletions src/Capacitor.Cli/Harness/Claude/ClaudeSessionEndHandoff.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using System.Diagnostics;
using System.Text.Json.Nodes;
using Capacitor.Cli.Core;

namespace Capacitor.Cli.Harness.Claude;

/// <summary>
/// Hands the Claude <c>SessionEnd</c> event to a detached re-invocation of this hook so the hook
/// itself returns at once: Claude Code caps a plugin's SessionEnd hook at 1.5 s regardless of
/// the <c>hooks.json</c> timeout, and the session-end path cannot fit. The continuation runs that
/// path unchanged under its 15 s budget. Reasoning in <c>docs/CHANGES.md</c>.
/// </summary>
static class ClaudeSessionEndHandoff {
public const string DetachedFlag = "--detached";

public static bool IsDetached(string[] args) => args.Contains(DetachedFlag);

public static bool ShouldHandOff(string[] args, string body) => !IsDetached(args) && IsSessionEnd(body);

static bool IsSessionEnd(string body) {
try {
var name = JsonNode.Parse(body)?["hook_event_name"]?.GetValue<string>();

return name is not null
&& name.Replace("-", "").Replace("_", "").Equals("sessionend", StringComparison.OrdinalIgnoreCase);
} catch {
return false;
}
}

/// <summary>
/// Starts the detached continuation and feeds it <paramref name="body"/>. False when that did
/// not fully happen — the caller then runs the event inline, as before.
/// </summary>
public static bool TrySpawn(string[] args, string body) {
Process? process = null;

try {
var psi = new ProcessStartInfo(Environment.ProcessPath ?? "kcap") {
RedirectStandardInput = true,
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
CreateNoWindow = true,
};

foreach (var arg in args) psi.ArgumentList.Add(arg);
psi.ArgumentList.Add(DetachedFlag);

// Same pipe-leak hazard as the watcher spawn: the child must not hold Claude's hook
// pipes open, or Claude waits on them past the hook's own exit.
ProcessHelpers.PreventInheritedHandles();

process = WatcherManager.StartProcess(psi);

if (process is null) {
Console.Error.WriteLine("[kcap] session-end hand-off: failed to start the detached continuation; running inline");

return false;
}

process.StandardInput.Write(body);
process.StandardInput.Close();
process.StandardOutput.Close();
process.StandardError.Close();
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved

return true;
} catch (Exception ex) {
Console.Error.WriteLine($"[kcap] session-end hand-off failed: {ex.Message}; running inline");

// A child that started but never got the full payload must not outlive this failure:
// the inline path is about to do the work, and two owners would double-post.
try { process?.Kill(entireProcessTree: true); } catch { }

return false;
} finally {
process?.Dispose();
}
}

/// <summary>
/// The continuation's own setup: output to the session log (its pipes are already closed) and
/// out of the terminal's session so a closing window cannot SIGHUP it mid-drain.
/// </summary>
public static void EnterDetached(string body) {
TextWriter writer;

try {
var logDir = PathHelpers.ConfigPath("logs");
Directory.CreateDirectory(logDir);
writer = new StreamWriter(Path.Combine(logDir, $"{LogName(body)}.log"), append: true) { AutoFlush = true };
} catch {
writer = TextWriter.Null;
}

Console.SetOut(writer);
Console.SetError(writer);

ProcessHelpers.DetachFromControllingTerminal();
}

// The watcher's log name for a well-formed id, so the two interleave in one file; anything
// else (the id is payload data) stays inside the logs directory under a fixed name.
internal static string LogName(string body) {
try {
var sessionId = JsonNode.Parse(body)?["session_id"]?.GetValue<string>()?.Replace("-", "");

if (sessionId is { Length: > 0 } && sessionId.All(char.IsAsciiLetterOrDigit)) return sessionId;
} catch { }

return "claude-session-end";
}
}
15 changes: 14 additions & 1 deletion src/Capacitor.Cli/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,19 @@
var hookProcessStart = System.Diagnostics.Stopwatch.GetTimestamp();
var isHook = command == "hook";

// Claude kills a SessionEnd hook after 1.5 s (ClaudeSessionEndHandoff), so the hand-off sits
// ahead of ResolveServerUrl's git probes and the global spool drain, each of which can spend it.
string? claudeHookBody = null;
if (isHook && args.Contains("--claude")) {
try { claudeHookBody = await Console.In.ReadToEndAsync(); } catch { claudeHookBody = ""; }

if (ClaudeSessionEndHandoff.IsDetached(args)) {
ClaudeSessionEndHandoff.EnterDetached(claudeHookBody);
} else if (ClaudeSessionEndHandoff.ShouldHandOff(args, claudeHookBody) && ClaudeSessionEndHandoff.TrySpawn(args, claudeHookBody)) {
return 0;
}
}

// Agent-spawned commands owe an output contract, or must leave no orphaned child, so an unusable
// server URL must not kill them mid-contract — EnsureAbsolute throws for them instead of exiting.
// Interactive commands keep exiting 2 with the actionable hint, which is the right UX with a user
Expand Down Expand Up @@ -798,7 +811,7 @@ await AgentHookPoster.DrainSpoolsAsync(
sessionId: null); // current session unknown here — reading stdin now would consume it
}
if (args.Contains("--claude")) {
return await ClaudeHookCommand.Handle(baseUrl!, Console.In, processStart: hookProcessStart);
return await ClaudeHookCommand.Handle(baseUrl!, new StringReader(claudeHookBody!), processStart: hookProcessStart);
}
if (args.Contains("--codex")) {
return await CodexHookCommand.Handle(baseUrl!, Console.In, hookProcessStart);
Expand Down
6 changes: 3 additions & 3 deletions src/Capacitor.Cli/WatcherManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ internal static string GetWatcherDir() {
internal static Func<string, Task>? SpawnOverrideForTesting;

/// <summary>
/// Test seam for the ACTUAL <c>Process.Start</c> call inside <see cref="SpawnWatcher"/> and
/// <see cref="SpawnCopilotFinalizeDrain"/>. Distinct from <see cref="SpawnOverrideForTesting"/>,
/// Test seam for the ACTUAL <c>Process.Start</c> call inside <see cref="SpawnWatcher"/>,
/// <see cref="SpawnCopilotFinalizeDrain"/> and <c>ClaudeSessionEndHandoff.TrySpawn</c>. Distinct from <see cref="SpawnOverrideForTesting"/>,
/// which only <c>SpawnForKeyAsync</c> consults and so cannot observe those methods at all.
///
/// <para>Needed because both call static <c>Process.Start</c> inside a catch-all, and the finalize
Expand All @@ -63,7 +63,7 @@ internal static string GetWatcherDir() {
/// </summary>
internal static Func<ProcessStartInfo, Process?>? ProcessStarterForTesting;

static Process? StartProcess(ProcessStartInfo psi) =>
internal static Process? StartProcess(ProcessStartInfo psi) =>
ProcessStarterForTesting is { } fake ? fake(psi) : Process.Start(psi);

internal static string BuildSpawnArgs(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
using System.Diagnostics;
using System.Text.Json.Nodes;
using Capacitor.Cli.Core;
using WireMock.RequestBuilders;
using WireMock.ResponseBuilders;
using WireMock.Server;

namespace Capacitor.Cli.Tests.Integration;

/// <summary>
/// Real-binary proof that the Claude SessionEnd hook returns inside Claude Code's 1.5 s grace
/// while the session-end still reaches the server. The server stalls <c>/hooks/session-end</c>
/// for longer than the hook is allowed to live, so the only way the POST can arrive is from a
/// process that outlived the hook.
/// </summary>
public class ClaudeSessionEndHandoffTests : IDisposable {
static readonly TimeSpan ServerStall = TimeSpan.FromSeconds(3);

[TempDir] public required TempDir Tmp { get; init; }
[TempDaemonPaths] public required TempDaemonStore Daemons { get; init; }

readonly WireMockServer _server = WireMockServer.Start();

public void Dispose() => _server.Stop();

[Test]
public async Task Hook_returns_at_once_and_the_detached_continuation_posts_session_end() {
var sid = Guid.NewGuid().ToString("N");
var transcript = Tmp.CreateFile("t.jsonl", """{"type":"user","uuid":"u1","message":{"role":"user","content":"hi"}}""" + "\n");
var configDir = Tmp.CreateDir("cfg");

_server.Given(Request.Create().WithPath("/hooks/session-end").UsingPost())
.RespondWith(Response.Create().WithStatusCode(200).WithBody("{}").WithDelay(ServerStall));
_server.Given(Request.Create().WithPath("/hooks/transcript").UsingPost())
.RespondWith(Response.Create().WithStatusCode(200).WithBody("""{"processed":0}"""));

var psi = KcapProcess.StartInfo(Daemons.Store, "hook", "--claude", "--no-update-check");
psi.WorkingDirectory = Tmp.Path;
psi.Environment["KCAP_CONFIG_DIR"] = configDir;
psi.Environment["KCAP_URL"] = _server.Url!;

var payload = new JsonObject {
["hook_event_name"] = "SessionEnd",
["session_id"] = sid,
["transcript_path"] = transcript,
["cwd"] = Tmp.Path,
["reason"] = "exit",
}.ToJsonString();

var clock = Stopwatch.StartNew();
using var hook = Process.Start(psi) ?? throw new InvalidOperationException("Failed to start kcap");
await hook.StandardInput.WriteAsync(payload);
hook.StandardInput.Close();
var stdout = hook.StandardOutput.ReadToEndAsync();
var stderr = hook.StandardError.ReadToEndAsync();
await hook.WaitForExitAsync();
clock.Stop();

await Assert.That(hook.ExitCode).IsEqualTo(0).Because(await stderr);
await Assert.That(await stdout).IsEmpty();
// The ticket's bar: well inside the 1.5 s grace, on a session with a transcript to drain.
await Assert.That(clock.Elapsed).IsLessThan(TimeSpan.FromSeconds(1)).Because($"stderr: {await stderr}");

// The hook is gone; the continuation has today's 15 s budget and must wait out the stall.
var posted = await WaitForSessionEndAsync(TimeSpan.FromSeconds(20));

await Assert.That(posted).IsNotNull().Because("no /hooks/session-end reached the server");
await Assert.That(posted!["session_id"]?.GetValue<string>()).IsEqualTo(sid);
await Assert.That(posted["reason"]?.GetValue<string>()).IsEqualTo("exit");
await Assert.That(posted["ended_at"]?.GetValue<string>()).IsNotNull();

// The continuation's output lands in the session log, where the watcher's already goes.
var log = Path.Combine(configDir, "logs", $"{sid}.log");
await Assert.That(File.Exists(log)).IsTrue();
await Assert.That(SharedFileText.ReadAllText(log)).Contains($"Inline drain for {sid}");
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Resolved

async Task<JsonNode?> WaitForSessionEndAsync(TimeSpan budget) {
var deadline = DateTime.UtcNow + budget;

while (DateTime.UtcNow < deadline) {
var entries = _server.FindLogEntries(Request.Create().WithPath("/hooks/session-end").UsingPost());
if (entries.Count > 0) return JsonNode.Parse(entries[0].RequestMessage.Body!);
await Task.Delay(100);
}

return null;
}
}
Loading