From 2a122b23425b3f587e36219ec7d4fb7e3d50b408 Mon Sep 17 00:00:00 2001 From: Stephen Tung Date: Tue, 7 Jul 2026 01:40:43 +0800 Subject: [PATCH 1/2] Daemon fallback: discover hosted Claude agent's session id from its transcript A hosted agent's web chat can only attach once the server registry entry has a SessionId, and the only source of that link was the spawned Claude's session-start hook POSTing agent_host_id to /hooks/session-start. When that hook fails (production case: expired kcap token, so every /hooks POST 401s), the link is never made: the daemon always sent SessionId = null, and the server's recovery path reads an AgentRun heartbeat that only the same hook handler writes. The agent page then shows "Waiting for session to start..." forever while the terminal works. Make the daemon discover the session id itself: after spawning a claude-vendor agent, poll the Claude project dir for the worktree (every 2 s, up to 3 min) for a *.jsonl transcript written at/after the spawn time. Because the daemon symlinks the worktree's project dir to the SOURCE repo's (shared with the user's own sessions), each candidate is verified by reading its first lines (FileShare.ReadWrite, tolerating partial/invalid lines) and requiring the JSON "cwd" to equal the agent's per-agent worktree path. On a match, the id (dashless-normalized, as in NormalizeGuidField) is set on the agent and reported over the daemon's own authenticated connection via AgentStatusChanged AND an AgentRunHeartbeat, so the server's restart-recovery path (FindAgentSessionIdAsync) works too. Once set, the existing 30 s heartbeat loop and reconnect re-registration keep re-sending it, so a transient report failure self-heals. Best-effort throughout; never breaks the launch; cancelled with the agent. The pure decision logic lives in SessionTranscriptLocator (cwd matching with Windows case-insensitivity and separator tolerance, filename -> session id parsing, spawn-time filtering) and is unit-tested without a filesystem. Co-Authored-By: Claude Fable 5 --- .../Services/AgentOrchestrator.cs | 82 +++++++ .../Services/SessionTranscriptLocator.cs | 165 ++++++++++++++ .../SessionTranscriptLocatorTests.cs | 214 ++++++++++++++++++ 3 files changed, 461 insertions(+) create mode 100644 src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs create mode 100644 test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs diff --git a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs index ea85c4f4..aa8bddab 100644 --- a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs +++ b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs @@ -401,6 +401,11 @@ async Task HandleLaunchAgent(LaunchAgentCommand cmd) { HostedRuntimeStart start; + // Captured BEFORE the spawn so the transcript-based session-id fallback + // (DetectClaudeSessionIdAsync) can filter the shared project dir to files + // written by THIS agent's process, not the user's earlier sessions. + var spawnedAtUtc = DateTime.UtcNow; + try { start = await runtimeFactory.StartAsync(runtimeCtx, _shutdownCts.Token); } catch (CodexHooksNotInstalledException ex) { @@ -448,6 +453,18 @@ async Task HandleLaunchAgent(LaunchAgentCommand cmd) { // Start reading output _ = ReadAgentOutputAsync(agent); + // Fallback session-id discovery: the primary link (the spawned Claude's + // session-start hook POSTing agent_host_id to /hooks/session-start) silently + // breaks when the hook can't authenticate (e.g. expired kcap token → 401), + // leaving the agent's web chat stuck on "Waiting for session to start..." + // forever while the terminal works. The daemon can discover the session id + // itself from the transcript file Claude writes and report it over its own + // authenticated connection. Claude-vendor only — other vendors have different + // transcript layouts. Best-effort background task, cancelled with the agent. + if (string.Equals(cmd.Vendor, "claude", StringComparison.OrdinalIgnoreCase)) { + _ = DetectClaudeSessionIdAsync(agent, spawnedAtUtc); + } + // Report the resolved model so the server can display the real model the agent // is running (the dispatched `model` may be the "default" no-override sentinel, // in which case Codex picks the model from ~/.codex/config.toml). The hub contract @@ -1310,6 +1327,62 @@ internal static bool IsStartupFailure(DateTime createdAt, DateTime lastOutputAt, static readonly HashSet ValidEffortLevels = ["low", "medium", "high", "max"]; + static readonly TimeSpan SessionIdPollInterval = TimeSpan.FromSeconds(2); + static readonly TimeSpan SessionIdPollTimeout = TimeSpan.FromMinutes(3); + + /// + /// Background fallback that discovers the spawned Claude's session id from its transcript + /// file and reports it to the server, for when the session-start hook (the primary source + /// of the agent↔session link) fails — e.g. an expired kcap token means every /hooks POST + /// 401s, and the server would otherwise never learn the session id (its recovery path reads + /// an AgentRun heartbeat that only the same hook handler writes). + /// + /// Polls the Claude project dir for the agent's worktree (symlinked to the SOURCE repo's + /// project dir, so it's shared with the user's own sessions — see + /// for how candidates are disambiguated by cwd). + /// On a match it sets and best-effort reports via + /// AgentStatusChanged (live registry link) AND an (so the + /// server's restart-recovery FindAgentSessionIdAsync path works too). Once SessionId is + /// set, the regular 30 s heartbeat loop and reconnect re-registration keep re-sending it, + /// so a transient report failure here self-heals. Stops when the id is known by other + /// means, the agent exits (ReadCts), or the daemon shuts down. Never breaks the launch. + /// + async Task DetectClaudeSessionIdAsync(AgentInstance agent, DateTime spawnedAtUtc) { + try { + var projectDir = ClaudePaths.ProjectDir(agent.Worktree.Path); + var deadline = DateTime.UtcNow + SessionIdPollTimeout; + + using var cts = CancellationTokenSource.CreateLinkedTokenSource(agent.ReadCts.Token, _shutdownCts.Token); + + while (DateTime.UtcNow < deadline) { + if (agent.SessionId is not null) return; // linked by other means (hook succeeded) + + if (SessionTranscriptLocator.TryLocate(projectDir, agent.Worktree.Path, spawnedAtUtc) is { } sessionId) { + agent.SessionId ??= sessionId; + + LogSessionIdDetected(agent.Id, sessionId); + + if (!agent.IsPrivate) { + await _server.AgentStatusChangedAsync(agent.Id, agent.Status, sessionId); + await _server.AppendAgentRunEventAsync(agent.Id, new AgentRunHeartbeat(sessionId)); + } + + return; + } + + await Task.Delay(SessionIdPollInterval, cts.Token); + } + + LogSessionIdNotDetected(agent.Id, SessionIdPollTimeout.TotalSeconds); + } catch (OperationCanceledException) { + // Agent stopped or daemon shutting down — nothing to report. + } catch (Exception ex) { + // Best-effort by design: if the immediate report failed after SessionId was set, + // the heartbeat loop / reconnect re-registration will re-send it. + LogSessionIdDetectFailed(ex, agent.Id); + } + } + async Task RunHeartbeatLoopAsync(CancellationToken ct) { while (await _heartbeatTimer.WaitForNextTickAsync(ct)) { // PrivateLocal agents get no heartbeats and no stuck-Starting auto-stop (deny-all; @@ -1540,6 +1613,15 @@ static string ExtractTerminalText(TerminalOutputBuffer buffer) { [LoggerMessage(Level = LogLevel.Debug, Message = "Failed to persist repo path for agent {AgentId}")] partial void LogRepoPathPersistFailed(Exception ex, string agentId); + [LoggerMessage(Level = LogLevel.Information, Message = "Agent {AgentId} linked to session {SessionId} via its transcript file (session-start hook fallback)")] + partial void LogSessionIdDetected(string agentId, string sessionId); + + [LoggerMessage(Level = LogLevel.Information, Message = "No transcript matched agent {AgentId}'s worktree within {Seconds:F0}s; session id stays hook-provided (or unknown if the hook failed)")] + partial void LogSessionIdNotDetected(string agentId, double seconds); + + [LoggerMessage(Level = LogLevel.Warning, Message = "Transcript-based session-id detection failed for agent {AgentId} (continuing; the session-start hook remains the primary link)")] + partial void LogSessionIdDetectFailed(Exception ex, string agentId); + [GeneratedRegex(@"\x1B\[[0-9;]*[A-Za-z]|\x1B\].*?\x07|\x1B[()][AB012]|\x1B\[[\?]?[0-9;]*[hlm]")] private static partial Regex StripAnsiRegex(); diff --git a/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs b/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs new file mode 100644 index 00000000..a29a307e --- /dev/null +++ b/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs @@ -0,0 +1,165 @@ +using System.Text.Json.Nodes; + +namespace Capacitor.Cli.Daemon.Services; + +/// +/// Discovers the session id of a freshly spawned hosted Claude agent by scanning the +/// Claude Code project directory for the transcript file the agent writes. +/// +/// Why this exists: the web chat can only attach to a hosted agent once the server's +/// registry entry has a SessionId, and until now the ONLY source of that link was the +/// spawned Claude's session-start hook POSTing to /hooks/session-start. When that +/// hook fails (e.g. an expired kcap token → 401), the link is never made and the agent +/// page shows "Waiting for session to start..." forever even though the terminal works. +/// The daemon itself can make the link instead: Claude Code writes its transcript to +/// {ClaudePaths.Projects}/{project-dir-hash}/{session-id}.jsonl, so polling that +/// directory and reporting the discovered id over the daemon's own (authenticated) +/// SignalR connection is a hook-independent fallback. +/// +/// Disambiguation: the daemon symlinks the worktree's project dir to the SOURCE repo's +/// project dir (see ClaudeLauncher.SymlinkClaudeProjectDir), which is shared with +/// the user's own sessions in that repo — so a new .jsonl there is not necessarily +/// this agent's. Candidates are verified by reading the first lines of the file and +/// requiring the JSON "cwd" field to equal the agent's worktree path (the worktree +/// is created per-agent, so a cwd match is definitive). +/// +/// The decision logic (cwd matching, filename → session id parsing, timestamp filtering) +/// is pure and unit-tested without a filesystem; only touches disk. +/// +internal static class SessionTranscriptLocator { + /// + /// How many non-blank transcript lines to inspect for a cwd match before + /// giving up on a candidate file. The cwd appears on the very first lines of a + /// transcript, so 50 is generous while still bounding the read. + /// + public const int MaxLinesToInspect = 50; + + /// + /// Slack applied to the spawn-time filter to absorb filesystem timestamp + /// granularity (e.g. FAT's 2 s) and small clock quirks. Safe to be generous: + /// the cwd check — against a worktree path created for this specific agent — + /// is the real disambiguator; the time filter only avoids re-reading old files. + /// + static readonly TimeSpan FileTimeSkewTolerance = TimeSpan.FromSeconds(5); + + /// + /// Scans for a transcript written at/after + /// whose early lines report cwd equal to + /// . Returns the normalized (dashless, lowercase) + /// session id, or null when no candidate matches yet. Best-effort: unreadable or + /// malformed candidates are skipped, never thrown. + /// + public static string? TryLocate(string projectDir, string worktreePath, DateTime spawnedAtUtc) { + if (!Directory.Exists(projectDir)) return null; + + foreach (var file in Directory.EnumerateFiles(projectDir, "*.jsonl")) { + try { + if (SessionIdFromFileName(file) is not { } sessionId) continue; + + if (!IsNewEnough(File.GetCreationTimeUtc(file), File.GetLastWriteTimeUtc(file), spawnedAtUtc)) continue; + + if (TryMatchTranscript(ReadFirstLines(file), worktreePath)) return sessionId; + } catch { + // Candidate vanished mid-scan, is locked, or is otherwise unreadable — skip it; + // the caller polls, so a transiently unreadable match is retried next tick. + } + } + + return null; + } + + /// + /// True when any of the first non-blank lines is a JSON + /// object whose cwd equals . Tolerates partial or + /// invalid JSON lines (skipped — the file is being written concurrently) and normalizes + /// both paths (separator style, trailing separators) before comparing. + /// + /// + /// Path comparison; defaults to case-insensitive on Windows, case-sensitive elsewhere. + /// Injectable so tests can pin either behaviour regardless of the OS they run on. + /// + public static bool TryMatchTranscript(IEnumerable lines, string worktreePath, StringComparison? comparison = null) { + var cmp = comparison ?? DefaultPathComparison; + var target = NormalizePath(worktreePath); + + var inspected = 0; + + foreach (var line in lines) { + if (inspected >= MaxLinesToInspect) break; + if (string.IsNullOrWhiteSpace(line)) continue; + + inspected++; + + string? cwd; + + try { + cwd = JsonNode.Parse(line)?["cwd"]?.GetValue(); + } catch { + continue; // partial/invalid JSONL line — tolerate and move on + } + + if (cwd is not null && string.Equals(NormalizePath(cwd), target, cmp)) return true; + } + + return false; + } + + /// + /// Derives the session id from a transcript file name: strips .jsonl, removes + /// dashes and lowercases — the same canonical form the CLI's NormalizeGuidField + /// produces for hook session ids. Returns null unless the stem is a GUID (32 hex chars + /// once dashes are removed), so stray non-session .jsonl files never match. + /// + public static string? SessionIdFromFileName(string fileNameOrPath) { + var name = Path.GetFileName(fileNameOrPath); + + if (!name.EndsWith(".jsonl", StringComparison.OrdinalIgnoreCase)) return null; + + var stem = name[..^".jsonl".Length]; + var normalized = stem.Replace("-", "").ToLowerInvariant(); + + if (normalized.Length != 32) return null; + + foreach (var c in normalized) { + if (!Uri.IsHexDigit(c)) return null; + } + + return normalized; + } + + /// + /// True when the file's newest timestamp (creation or last write) is at/after the + /// agent's spawn time, minus . Filters out the + /// user's own pre-existing sessions in the (shared, symlinked) project dir. + /// + public static bool IsNewEnough(DateTime creationTimeUtc, DateTime lastWriteTimeUtc, DateTime spawnedAtUtc) { + var newest = creationTimeUtc > lastWriteTimeUtc ? creationTimeUtc : lastWriteTimeUtc; + + return newest >= spawnedAtUtc - FileTimeSkewTolerance; + } + + static StringComparison DefaultPathComparison + => OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + + /// Unifies separator style and strips trailing separators so + /// C:\w\t\, C:/w/t and C:\w\t all compare equal. + static string NormalizePath(string path) => path.Replace('\\', '/').TrimEnd('/'); + + /// + /// Reads up to lines with + /// (Claude is appending to the file while we read) into memory, so the pure matcher never + /// holds a file handle across JSON parsing. + /// + static List ReadFirstLines(string path) { + var lines = new List(capacity: 16); + + using var fs = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete); + using var reader = new StreamReader(fs); + + while (lines.Count < MaxLinesToInspect && reader.ReadLine() is { } line) { + lines.Add(line); + } + + return lines; + } +} diff --git a/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs new file mode 100644 index 00000000..f57e65bd --- /dev/null +++ b/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs @@ -0,0 +1,214 @@ +using Capacitor.Cli.Daemon.Services; + +namespace Capacitor.Cli.Tests.Unit; + +/// +/// Tests the pure decision logic of — the daemon's +/// fallback that links a hosted Claude agent to its session when the session-start hook +/// fails (e.g. expired kcap token → every /hooks POST 401s and the agent page shows +/// "Waiting for session to start..." forever). +/// +/// The daemon symlinks the worktree's Claude project dir to the SOURCE repo's, which is +/// shared with the user's own sessions — so a candidate transcript must be verified by its +/// cwd matching the agent's per-agent worktree path, and only files written at/after +/// the spawn time are considered. +/// +public class SessionTranscriptLocatorTests { + const string Worktree = "/home/user/.kcap/worktrees/agent-1"; + + static string Line(string cwd) => $$"""{"type":"user","cwd":"{{cwd}}","sessionId":"abc"}"""; + + // ── TryMatchTranscript: cwd match / mismatch ───────────────────────── + + [Test] + public async Task CwdMatch_ReturnsTrue() { + var lines = new[] { Line(Worktree) }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsTrue(); + } + + [Test] + public async Task CwdMismatch_ReturnsFalse() { + // The user's own session in the source repo — same project dir (symlinked), + // different cwd. Must NOT be claimed as the agent's session. + var lines = new[] { Line("/home/user/dev/source-repo") }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsFalse(); + } + + [Test] + public async Task CwdOnLaterLine_StillMatches() { + var lines = new[] { + """{"type":"summary","summary":"hello"}""", + Line(Worktree) + }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsTrue(); + } + + // ── Windows semantics: case-insensitivity + separator tolerance ───── + + [Test] + public async Task WindowsCaseInsensitive_DifferentCase_Matches() { + var lines = new[] { Line(@"C:\\Users\\Dev\\.kcap\\Worktrees\\Agent-1") }; + + var result = SessionTranscriptLocator.TryMatchTranscript( + lines, + @"c:\users\dev\.kcap\worktrees\agent-1", + StringComparison.OrdinalIgnoreCase + ); + + await Assert.That(result).IsTrue(); + } + + [Test] + public async Task CaseSensitiveComparison_DifferentCase_DoesNotMatch() { + var lines = new[] { Line("/HOME/user/.kcap/worktrees/agent-1") }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsFalse(); + } + + [Test] + public async Task TrailingSeparator_IsTolerated() { + var lines = new[] { Line(Worktree + "/") }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsTrue(); + } + + [Test] + public async Task MixedSeparatorStyles_AreTolerated() { + // cwd recorded with backslashes, worktree passed with forward slashes (or vice + // versa) — both normalize to the same path. + var lines = new[] { Line(@"C:\\w\\agent-1") }; + + var result = SessionTranscriptLocator.TryMatchTranscript(lines, "C:/w/agent-1", StringComparison.OrdinalIgnoreCase); + + await Assert.That(result).IsTrue(); + } + + // ── Robustness against a concurrently-written file ─────────────────── + + [Test] + public async Task InvalidJsonLines_AreSkipped() { + var lines = new[] { + "not json at all", + """{"type":"user","cwd":123}""", // cwd not a string + """{"truncated":"partial-write""", // torn tail — file written concurrently + Line(Worktree) + }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsTrue(); + } + + [Test] + public async Task BlankLines_AreSkippedAndNotCounted() { + var lines = new List { "", " " }; + lines.Add(Line(Worktree)); + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsTrue(); + } + + [Test] + public async Task NoCwdAnywhere_ReturnsFalse() { + var lines = new[] { """{"type":"summary"}""", """{"type":"queue"}""" }; + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsFalse(); + } + + [Test] + public async Task MatchBeyondLineCap_IsNotFound() { + // The cwd appears on early lines in real transcripts; the cap bounds how much of + // a (possibly huge) foreign transcript is parsed per poll tick. + var lines = Enumerable + .Repeat("""{"type":"assistant"}""", SessionTranscriptLocator.MaxLinesToInspect) + .Append(Line(Worktree)); + + await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsFalse(); + } + + // ── SessionIdFromFileName: dashed → dashless normalization ────────── + + [Test] + public async Task DashedGuidFileName_NormalizesToDashlessLowercase() { + var id = SessionTranscriptLocator.SessionIdFromFileName("A1B2C3D4-E5F6-7890-ABCD-EF0123456789.jsonl"); + + await Assert.That(id).IsEqualTo("a1b2c3d4e5f67890abcdef0123456789"); + } + + [Test] + public async Task DashlessGuidFileName_IsAccepted() { + var id = SessionTranscriptLocator.SessionIdFromFileName("a1b2c3d4e5f67890abcdef0123456789.jsonl"); + + await Assert.That(id).IsEqualTo("a1b2c3d4e5f67890abcdef0123456789"); + } + + [Test] + public async Task FullPath_UsesBasenameOnly() { + var id = SessionTranscriptLocator.SessionIdFromFileName( + "/home/user/.claude/projects/-home-user-repo/a1b2c3d4-e5f6-7890-abcd-ef0123456789.jsonl" + ); + + await Assert.That(id).IsEqualTo("a1b2c3d4e5f67890abcdef0123456789"); + } + + [Test] + public async Task NonGuidFileName_ReturnsNull() { + await Assert.That(SessionTranscriptLocator.SessionIdFromFileName("notes.jsonl")).IsNull(); + } + + [Test] + public async Task NonHexStemOfGuidLength_ReturnsNull() { + // 32 chars once dashes are removed, but not hex. + await Assert.That(SessionTranscriptLocator.SessionIdFromFileName("zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz.jsonl")).IsNull(); + } + + [Test] + public async Task WrongExtension_ReturnsNull() { + await Assert.That(SessionTranscriptLocator.SessionIdFromFileName("a1b2c3d4-e5f6-7890-abcd-ef0123456789.json")).IsNull(); + } + + // ── IsNewEnough: spawn-time filter ─────────────────────────────────── + + static readonly DateTime SpawnedAt = new(2026, 7, 1, 12, 0, 0, DateTimeKind.Utc); + + [Test] + public async Task FileOlderThanSpawn_IsExcluded() { + // A pre-existing user session in the shared (symlinked) project dir. + var old = SpawnedAt.AddMinutes(-30); + + await Assert.That(SessionTranscriptLocator.IsNewEnough(old, old, SpawnedAt)).IsFalse(); + } + + [Test] + public async Task FileCreatedAfterSpawn_IsIncluded() { + var created = SpawnedAt.AddSeconds(4); + + await Assert.That(SessionTranscriptLocator.IsNewEnough(created, created, SpawnedAt)).IsTrue(); + } + + [Test] + public async Task OldCreationButRecentWrite_IsIncluded() { + // Newest of the two timestamps counts: e.g. a resumed transcript file whose + // creation predates the spawn but which is actively being appended to. + var creation = SpawnedAt.AddHours(-1); + var lastWrite = SpawnedAt.AddSeconds(10); + + await Assert.That(SessionTranscriptLocator.IsNewEnough(creation, lastWrite, SpawnedAt)).IsTrue(); + } + + [Test] + public async Task FileWithinSkewToleranceBeforeSpawn_IsIncluded() { + // Filesystem timestamp granularity / small clock skew must not exclude the + // agent's own transcript. The cwd check is the real disambiguator. + var justBefore = SpawnedAt.AddSeconds(-2); + + await Assert.That(SessionTranscriptLocator.IsNewEnough(justBefore, justBefore, SpawnedAt)).IsTrue(); + } + + [Test] + public async Task FileWellBeforeSkewTolerance_IsExcluded() { + var tooOld = SpawnedAt.AddSeconds(-6); + + await Assert.That(SessionTranscriptLocator.IsNewEnough(tooOld, tooOld, SpawnedAt)).IsFalse(); + } +} From e00d3060e7ccd7672920abf03e4ad8fff43735cd Mon Sep 17 00:00:00 2001 From: Stephen Tung Date: Thu, 16 Jul 2026 10:59:56 +0800 Subject: [PATCH 2/2] Address Qodo review: report ordering + transcript-scan negative cache Finding 1 (reliability): in the session-id fallback, enqueue the AgentRunHeartbeat run-event before the SignalR status update. The SignalR call can throw during a reconnect while the enqueue is non-throwing, so ordering it first stops a transient hiccup from skipping the durable report. Finding 2 (performance): avoid re-opening the same foreign transcripts on every 2s poll tick over the shared project dir. TryLocate now takes an optional negative-cache set; only definitive non-matches are cached (non-session filename, or a present-but-foreign cwd). Files with no cwd yet return the new CwdMatch.Unknown and are never cached, so the agent's own freshly-created transcript is always re-checked. TryMatchTranscript now delegates to a tri-state MatchTranscript, leaving its behaviour (and the existing tests) unchanged; 4 new tests pin the Yes/No/Unknown classification the cache relies on. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Services/AgentOrchestrator.cs | 14 ++++- .../Services/SessionTranscriptLocator.cs | 59 ++++++++++++++++--- .../SessionTranscriptLocatorTests.cs | 39 ++++++++++++ 3 files changed, 102 insertions(+), 10 deletions(-) diff --git a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs index aa8bddab..d0ae9e17 100644 --- a/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs +++ b/src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs @@ -1352,19 +1352,29 @@ async Task DetectClaudeSessionIdAsync(AgentInstance agent, DateTime spawnedAtUtc var projectDir = ClaudePaths.ProjectDir(agent.Worktree.Path); var deadline = DateTime.UtcNow + SessionIdPollTimeout; + // Transcripts confirmed to belong to another session (a foreign cwd) are remembered + // here so we don't re-open them every 2 s tick. A session's cwd never changes, so a + // definitive non-match is permanent; files still being written (no cwd yet) are NOT + // added, so the agent's own freshly-created transcript is always re-checked. + var ruledOut = new HashSet(); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(agent.ReadCts.Token, _shutdownCts.Token); while (DateTime.UtcNow < deadline) { if (agent.SessionId is not null) return; // linked by other means (hook succeeded) - if (SessionTranscriptLocator.TryLocate(projectDir, agent.Worktree.Path, spawnedAtUtc) is { } sessionId) { + if (SessionTranscriptLocator.TryLocate(projectDir, agent.Worktree.Path, spawnedAtUtc, ruledOut) is { } sessionId) { agent.SessionId ??= sessionId; LogSessionIdDetected(agent.Id, sessionId); if (!agent.IsPrivate) { - await _server.AgentStatusChangedAsync(agent.Id, agent.Status, sessionId); + // Enqueue the run-event first: it's a non-throwing local enqueue, whereas + // the SignalR status update can throw during a reconnect. Ordering it first + // ensures a transient connection hiccup can't skip the durable report — the + // status update then re-sends via the heartbeat loop regardless. await _server.AppendAgentRunEventAsync(agent.Id, new AgentRunHeartbeat(sessionId)); + await _server.AgentStatusChangedAsync(agent.Id, agent.Status, sessionId); } return; diff --git a/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs b/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs index a29a307e..6c61ee9c 100644 --- a/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs +++ b/src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs @@ -49,16 +49,34 @@ internal static class SessionTranscriptLocator { /// session id, or null when no candidate matches yet. Best-effort: unreadable or /// malformed candidates are skipped, never thrown. /// - public static string? TryLocate(string projectDir, string worktreePath, DateTime spawnedAtUtc) { + /// + /// Optional set of file paths already confirmed to belong to another session (or to not be a + /// transcript at all). Since the caller polls every few seconds over a shared project dir, + /// caching definitive non-matches here avoids re-opening the user's own concurrently-written + /// sessions on every tick. Only definitive non-matches are added: a file whose name + /// isn't a session id, or one whose cwd is present but foreign (a cwd never changes). + /// Files with no cwd yet (still being written) are left out so the agent's own + /// freshly-created transcript is always re-checked. + /// + public static string? TryLocate(string projectDir, string worktreePath, DateTime spawnedAtUtc, ISet? ruledOut = null) { if (!Directory.Exists(projectDir)) return null; foreach (var file in Directory.EnumerateFiles(projectDir, "*.jsonl")) { + if (ruledOut?.Contains(file) == true) continue; + try { - if (SessionIdFromFileName(file) is not { } sessionId) continue; + if (SessionIdFromFileName(file) is not { } sessionId) { + ruledOut?.Add(file); // not a session transcript — its name won't change + continue; + } if (!IsNewEnough(File.GetCreationTimeUtc(file), File.GetLastWriteTimeUtc(file), spawnedAtUtc)) continue; - if (TryMatchTranscript(ReadFirstLines(file), worktreePath)) return sessionId; + switch (MatchTranscript(ReadFirstLines(file), worktreePath, DefaultPathComparison)) { + case CwdMatch.Yes: return sessionId; + case CwdMatch.No: ruledOut?.Add(file); break; // another session's transcript — cwd is fixed + // CwdMatch.Unknown: no cwd yet (file still being written) — leave uncached, re-check next tick. + } } catch { // Candidate vanished mid-scan, is locked, or is otherwise unreadable — skip it; // the caller polls, so a transiently unreadable match is retried next tick. @@ -78,11 +96,32 @@ internal static class SessionTranscriptLocator { /// Path comparison; defaults to case-insensitive on Windows, case-sensitive elsewhere. /// Injectable so tests can pin either behaviour regardless of the OS they run on. /// - public static bool TryMatchTranscript(IEnumerable lines, string worktreePath, StringComparison? comparison = null) { - var cmp = comparison ?? DefaultPathComparison; + public static bool TryMatchTranscript(IEnumerable lines, string worktreePath, StringComparison? comparison = null) + => MatchTranscript(lines, worktreePath, comparison ?? DefaultPathComparison) == CwdMatch.Yes; + + /// Outcome of inspecting a transcript's early lines for a cwd. + internal enum CwdMatch { + /// A cwd equal to the worktree path was found — this is the agent's transcript. + Yes, + /// A cwd was found but it belongs to a different session — a definitive, permanent non-match. + No, + /// No parseable cwd in the inspected lines — the file may still be being written; re-check later. + Unknown, + } + + /// + /// Classifies a candidate transcript by scanning up to non-blank + /// lines for a JSON cwd. Returns on the first line whose cwd + /// equals , if a cwd was seen but none + /// matched, or if no cwd was seen at all. Partial or invalid + /// JSON lines are tolerated (skipped — the file is being written concurrently) and both paths are + /// normalized (separator style, trailing separators) before comparing. + /// + internal static CwdMatch MatchTranscript(IEnumerable lines, string worktreePath, StringComparison comparison) { var target = NormalizePath(worktreePath); var inspected = 0; + var sawCwd = false; foreach (var line in lines) { if (inspected >= MaxLinesToInspect) break; @@ -95,13 +134,17 @@ public static bool TryMatchTranscript(IEnumerable lines, string worktree try { cwd = JsonNode.Parse(line)?["cwd"]?.GetValue(); } catch { - continue; // partial/invalid JSONL line — tolerate and move on + continue; // partial/invalid JSONL line (or non-string cwd) — tolerate and move on } - if (cwd is not null && string.Equals(NormalizePath(cwd), target, cmp)) return true; + if (cwd is null) continue; + + sawCwd = true; + + if (string.Equals(NormalizePath(cwd), target, comparison)) return CwdMatch.Yes; } - return false; + return sawCwd ? CwdMatch.No : CwdMatch.Unknown; } /// diff --git a/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs b/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs index f57e65bd..ad8bee46 100644 --- a/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/SessionTranscriptLocatorTests.cs @@ -126,6 +126,45 @@ public async Task MatchBeyondLineCap_IsNotFound() { await Assert.That(SessionTranscriptLocator.TryMatchTranscript(lines, Worktree, StringComparison.Ordinal)).IsFalse(); } + // ── MatchTranscript tri-state: drives the poll-loop's negative cache ─ + // + // A definitive foreign cwd (No) is cached and never re-read; a file with no cwd yet + // (Unknown) must NOT be cached, so the agent's own transcript is re-checked once Claude + // writes its cwd line. + + [Test] + public async Task MatchTranscript_MatchingCwd_ReturnsYes() { + var lines = new[] { Line(Worktree) }; + + await Assert.That(SessionTranscriptLocator.MatchTranscript(lines, Worktree, StringComparison.Ordinal)) + .IsEqualTo(SessionTranscriptLocator.CwdMatch.Yes); + } + + [Test] + public async Task MatchTranscript_ForeignCwd_ReturnsNo() { + // Seen a cwd, it's someone else's — permanent non-match, safe to cache. + var lines = new[] { Line("/home/user/dev/source-repo") }; + + await Assert.That(SessionTranscriptLocator.MatchTranscript(lines, Worktree, StringComparison.Ordinal)) + .IsEqualTo(SessionTranscriptLocator.CwdMatch.No); + } + + [Test] + public async Task MatchTranscript_NoCwdYet_ReturnsUnknown() { + // A freshly-created transcript whose cwd line hasn't been written yet. Must be + // Unknown (not No) so the poll loop keeps re-checking it — it could be ours. + var lines = new[] { """{"type":"summary"}""", """{"truncated":"partial""" }; + + await Assert.That(SessionTranscriptLocator.MatchTranscript(lines, Worktree, StringComparison.Ordinal)) + .IsEqualTo(SessionTranscriptLocator.CwdMatch.Unknown); + } + + [Test] + public async Task MatchTranscript_EmptyFile_ReturnsUnknown() { + await Assert.That(SessionTranscriptLocator.MatchTranscript([], Worktree, StringComparison.Ordinal)) + .IsEqualTo(SessionTranscriptLocator.CwdMatch.Unknown); + } + // ── SessionIdFromFileName: dashed → dashless normalization ────────── [Test]