Daemon-side session-id fallback for hosted Claude agents#283
Open
stktung wants to merge 1 commit into
Open
Conversation
…ranscript 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 <noreply@anthropic.com>
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Member
|
/agentic_review |
Code Review by Qodo
1. Fallback reports not independent
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+1365
to
+1368
| if (!agent.IsPrivate) { | ||
| await _server.AgentStatusChangedAsync(agent.Id, agent.Status, sessionId); | ||
| await _server.AppendAgentRunEventAsync(agent.Id, new AgentRunHeartbeat(sessionId)); | ||
| } |
There was a problem hiding this comment.
1. Fallback reports not independent 🐞 Bug ☼ Reliability
In DetectClaudeSessionIdAsync, an exception from the awaited SignalR call AgentStatusChangedAsync will skip the subsequent AppendAgentRunEventAsync(AgentRunHeartbeat), delaying the server’s recovery signal until the normal heartbeat/reconnect paths run. This weakens the “report both ways” behavior specifically during transient connection instability (one of the main scenarios this fallback should tolerate).
Agent Prompt
### Issue description
`DetectClaudeSessionIdAsync` awaits `_server.AgentStatusChangedAsync(...)` and then awaits `_server.AppendAgentRunEventAsync(...)` in the same block. If the first call throws (e.g., reconnect in progress), the second call is skipped and the method exits via the outer catch, which delays/weakens the fallback’s intended “report both ways” behavior.
### Issue Context
- `AgentStatusChangedAsync` is a SignalR `InvokeAsync` and can throw on disconnect / reconnect windows.
- `AppendAgentRunEventAsync` is a local enqueue into a bounded channel and is effectively non-throwing.
### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1360-1368]
Suggested implementation approach:
- Ensure the HTTP-backed run-event path is executed even if the SignalR call fails:
- Either enqueue `AppendAgentRunEventAsync(... new AgentRunHeartbeat(sessionId))` *before* calling `AgentStatusChangedAsync`, and/or
- Wrap each call in its own try/catch so one failure doesn’t prevent the other.
- Keep the method best-effort (no rethrow), but avoid losing the run-event append when the status update fails.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+55
to
+66
| 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. | ||
| } | ||
| } |
There was a problem hiding this comment.
2. Repeated transcript directory scans 🐞 Bug ➹ Performance
DetectClaudeSessionIdAsync polls every 2 seconds for up to 3 minutes, and each tick SessionTranscriptLocator.TryLocate enumerates all *.jsonl files and may open/read up to 50 lines per candidate, which can create avoidable sustained IO/CPU overhead in large shared Claude project directories. This can degrade daemon responsiveness during the polling window on machines with many transcript files.
Agent Prompt
### Issue description
The fallback performs repeated full-directory scans of `*.jsonl` transcripts (and potentially reads up to 50 lines from many files) every 2 seconds for up to 3 minutes. On large project dirs, this can cause unnecessary repeated work and noticeable IO/CPU churn.
### Issue Context
The logic is time-bounded (3 minutes) and per-file line-bounded (50), but it is not bounded by number of files inspected per tick, and it does not avoid re-checking files already known to be non-matches.
### Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[1330-1374]
- src/Capacitor.Cli.Daemon/Services/SessionTranscriptLocator.cs[52-66]
Suggested implementation approach (pick one or combine):
- Prefer inspecting newest files first and cap per tick (e.g., order by `LastWriteTimeUtc` desc and `Take(N)`), since the target transcript should be among the newest after spawn.
- Cache negative candidates across ticks (e.g., keep a `HashSet<string>` of inspected filenames or track `lastSeenWriteTime`) to avoid re-opening the same files repeatedly.
- If you keep a time filter, apply it before opening files, and short-circuit scanning as soon as you hit sufficiently old files when iterating in descending timestamp order.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A hosted agent's web chat can only attach once the server's registry entry for the agent has a SessionId. Today the only source of that link is the spawned Claude's session-start hook POSTing
agent_host_idto/hooks/session-start. If that hook fails — the production case is an expired kcap token, so every/hooksPOST 401s (/hooksrequires auth) — the link is never made:AgentStatusChangedAsynccalls always sendSessionId = null(agent.SessionIdwas never assigned anywhere in the daemon), andFindAgentSessionIdAsync) reads anAgentRunheartbeat that only the same hook handler writes.Result: the agent page shows "Waiting for session to start..." forever while the terminal works fine, and there is no fallback.
Design
Make the daemon itself discover the spawned Claude's session id and report it over its own authenticated SignalR connection — hook-independent:
claude-vendor agent spawns, a per-agent background task (cancelled by the agent's existing CTS) polls{ClaudePaths.Projects}/{project-dir-hash(worktree)}every 2 s for up to 3 min for*.jsonltranscripts written at/after the spawn time.cwd— the daemon symlinks the worktree's project dir to the SOURCE repo's (ClaudeLauncher.SymlinkClaudeProjectDir), which is shared with the user's own sessions in that repo, so a new.jsonlis not necessarily this agent's. Each candidate's first lines (read withFileShare.ReadWrite, partial/invalid JSONL lines tolerated, capped at 50 lines) must have a JSON"cwd"equal to the agent's worktree path (case-insensitive on Windows, separator/trailing-slash tolerant). The worktree is created per-agent, so a cwd match is definitive.NormalizeGuidField) is set on the agent and sent viaAgentStatusChanged(agentId, status, sessionId)(live registry link — the server'sCapacitorHub.AgentStatusChangedin kcap-server already accepts a sessionId) andAppendAgentRunEvent(new AgentRunHeartbeat(sessionId)), so the server's restart-recovery path works too. Onceagent.SessionIdis set, the existing 30 s heartbeat loop and reconnect re-registration keep re-sending it, so a transient report failure self-heals.The pure decision logic (cwd matching, filename → session-id parsing, spawn-time filtering) is extracted into
SessionTranscriptLocatorand unit-tested without a filesystem.This complements server-side handling in kcap-server (
CapacitorHub.AgentStatusChangedalready accepts and stores a sessionId); no server change is required for the fallback to take effect.Testing
Capacitor.Cli.Tests.Unit: 2489 passed, 0 failed (includes 21 newSessionTranscriptLocatorTestscovering cwd match/mismatch, Windows case-insensitivity, trailing-separator and mixed-separator tolerance, invalid/partial JSON lines skipped, line cap, dashed→dashless id normalization, and spawn-time filtering).vswhere.exe), so CI's AOT publish checks must confirm the publish end-to-end.No GitHub or Linear issue exists for this; this description is self-contained.
🤖 Generated with Claude Code