Repository navigation
Add plugin install/remove command and fix marketplace discovery - #3
Merged
Merged
Conversation
- Add `kapacitor plugin install [--project]` and `kapacitor plugin remove [--project]` commands for standalone plugin management without running full setup - Add marketplace.json to plugin directory — required by Claude Code to discover plugins within a directory-type marketplace - Rename plugin directory from `plugin/` to `kapacitor/` to match the plugin name (single-plugin marketplace convention) - Change marketplace registration from `kapacitor@kurrent` to `kapacitor@kapacitor` matching how other single-plugin marketplaces (impeccable, dotnet-skills) work - Clean up stale `kurrent` marketplace entries during install Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review Summary by QodoAdd plugin install/remove commands and fix marketplace discovery
WalkthroughsDescription• Add kapacitor plugin install/remove commands for standalone plugin management • Fix marketplace discovery by renaming plugin directory to kapacitor/ • Add marketplace.json for Claude Code directory-type marketplace support • Update plugin registration from kapacitor@kurrent to kapacitor@kapacitor • Clean up stale marketplace entries during plugin installation Diagramflowchart LR
A["User runs plugin command"] --> B{"install or remove?"}
B -->|install| C["Resolve plugin path"]
C --> D["Update settings file"]
D --> E["Register kapacitor@kapacitor"]
B -->|remove| F["Parse settings file"]
F --> G["Remove plugin entries"]
G --> H["Clean stale entries"]
E --> I["Plugin ready in Claude Code"]
H --> I
File Changes1. src/kapacitor/Commands/PluginCommand.cs
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
Apr 13, 2026
Three fixes from PR #11 review (Finding #3 on invalid verdicts was already addressed in 31cd70d): 1. Flag value becomes sessionId (Action required) — ResolveSessionId picked the first non-"--" token, so `kapacitor eval --model sonnet <id>` ended up calling the server with "sonnet" as the session id. Extracted ResolveSessionId into a testable ArgParsing helper that accepts a list of value-bearing flags and correctly skips their values. The eval dispatcher declares ["--model", "--threshold"] as such. 8 unit tests cover positional-before-flags, flags-before-positional, mixed value and boolean flags, and the env-var fallback (serialized via NotInParallel since tests mutate process-level env). 2. Prompt exceeds argv limits (Action required) — eval embeds the full compacted trace in the prompt; for larger sessions that easily exceeds Windows' 32K argv limit (and stresses macOS/Linux limits too). Added a `promptViaStdin` flag to ClaudeCliRunner; when true the prompt is written to the child process's stdin after start instead of as a positional argv, and `claude -p` reads it from stdin. EvalCommand opts in; TitleGenerator / WhatsDone keep their existing argv behaviour. 3. Eval not in main help — added an entry under the Session section of help-usage.txt so `kapacitor --help` surfaces the command. AOT publish clean (0 IL warnings), full suite 195/195 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
Apr 13, 2026
…11) * [DEV-1433] add kapacitor eval command (sequential, transcript-based) kapacitor eval <sessionId> runs an LLM-as-judge evaluation over a stored session. 13 questions across 4 categories (safety, plan adherence, quality, efficiency) are sent to Claude in sequence — each in its own headless invocation with no tools — with the server's compacted eval-context (DEV-1432) embedded in the prompt. Verdicts are aggregated into per-category + overall scores, rendered to the terminal, and POSTed to the server's eval results endpoint (DEV-1433a) for persistence as a SessionEvalCompleted event on the session's stream. Key decisions vs. the superseded PR #223: - Transcript is embedded directly, not searched via MCP. No Kontext, no submodule, no embeddings. Judges read the full (compacted) trace. - Question taxonomy and category structure salvaged verbatim from #223; prompt template rewritten for the full-trace approach and stored as an embedded resource. - ClaudeCliRunner gained optional `model` and `maxTurns` parameters; default (haiku, 1) preserves behaviour for TitleGenerator / WhatsDone. Eval defaults to sonnet — overridable via `--model`. Verdict parsing is tolerant of markdown code fences (judges sometimes wrap their JSON despite the prompt's instruction otherwise) and overrides category/question_id mismatches against the asked-about question to defend against mild hallucination. Unit tests cover verdict parsing (clean JSON, fenced JSON, malformed, category/id override), aggregation (per-category averaging, canonical category ordering, fail-verdict derivation), and prompt-template placeholder substitution — 8/8 passing, full suite 184/184. `dotnet publish -c Release` produces zero IL3050/IL2026 warnings. Server endpoint (POST /api/sessions/{id}/evals) and eval-context endpoint are in separate PRs on the kapacitor-server repo (#474 and already-merged #473). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [DEV-1433] harden verdict parsing against judge hallucinations Defense-in-depth layer to complement the server-side validator (#474): - Reject verdicts whose score is outside 1..5 (null return, judge output skipped from the aggregate) - Always derive verdict from score, ignoring whatever string the judge placed in the verdict field. The prompt documents the mapping (pass=4-5, warn=2-3, fail=1); trusting the score over the judge's verdict string canonicalizes away mild hallucinations (verdict="banana", score=5/verdict="fail", etc.) without discarding otherwise-useful scores. Means the CLI never sends contract-violating payloads to the server in the common case, while the server's validator remains authoritative. Three new tests: - Score out of range → null - score/verdict mismatch → verdict derived from score - Garbage verdict string → sanitized via derivation 11 EvalCommandTests pass, full suite 187/187, AOT publish clean. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * [DEV-1433] address review feedback on eval command Three fixes from PR #11 review (Finding #3 on invalid verdicts was already addressed in 31cd70d): 1. Flag value becomes sessionId (Action required) — ResolveSessionId picked the first non-"--" token, so `kapacitor eval --model sonnet <id>` ended up calling the server with "sonnet" as the session id. Extracted ResolveSessionId into a testable ArgParsing helper that accepts a list of value-bearing flags and correctly skips their values. The eval dispatcher declares ["--model", "--threshold"] as such. 8 unit tests cover positional-before-flags, flags-before-positional, mixed value and boolean flags, and the env-var fallback (serialized via NotInParallel since tests mutate process-level env). 2. Prompt exceeds argv limits (Action required) — eval embeds the full compacted trace in the prompt; for larger sessions that easily exceeds Windows' 32K argv limit (and stresses macOS/Linux limits too). Added a `promptViaStdin` flag to ClaudeCliRunner; when true the prompt is written to the child process's stdin after start instead of as a positional argv, and `claude -p` reads it from stdin. EvalCommand opts in; TitleGenerator / WhatsDone keep their existing argv behaviour. 3. Eval not in main help — added an entry under the Session section of help-usage.txt so `kapacitor --help` surfaces the command. AOT publish clean (0 IL warnings), full suite 195/195 pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: document kapacitor eval in user-facing README New "Session evaluation (LLM-as-judge)" section under post-session analysis commands. Explains the no-tools judge pattern, lists example flags, and notes that results persist to the session stream as SessionEvalCompleted events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 19, 2026
alexeyzimarev
added a commit
that referenced
this pull request
Apr 24, 2026
…ogout - Fix #2: Apply AppConfig.NormalizeUrl to tenant origins in MergeProfiles, HandleDiscoverLoginAsync, and SetupCommand.RunDiscoveryAsync so trailing slashes never produce double-slash URLs in subsequent requests - Fix #3: Add `using` to HttpResponseMessage in GetGitHubClientIdAsync and DiscoverTenantsAsync to ensure connection resources are always released - Fix #4: DeleteAsync now deletes all *.json files under tokens/ (not just the active profile), so multi-profile logout is complete; add test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
Apr 24, 2026
* Add hard-coded auth proxy URL with env override Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: defensive env var cleanup and internal override comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Refactor OAuthLoginFlow to expose RunDeviceFlowAsync and ExchangeAndSaveAsync Extract Device Flow polling and token exchange/save steps into composable public static helpers so upcoming tenant-discovery flow can run them independently. HandleGitHubLogin is now a thin composition of the two helpers; behavior unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: document Device Flow side effects and add AuthProvider constants Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add AuthProxyClient for /config and /discover-tenants Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: rename DiscoveryError.UpstreamError and move DTOs to AuthModels - Rename GitHubError → UpstreamError (accurate for both proxy 5xx and upstream failures) - Remove redundant TaskCanceledException from exception filters (it inherits OperationCanceledException) - Move ProxyConfigResponse, DiscoveredTenant, DiscoveryError, DiscoveryResult from AuthProxyClient.cs to AuthModels.cs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add per-profile TokenStore with legacy fallback and migration Introduces profile-aware LoadAsync(profile)/SaveAsync(profile, tokens)/Delete(profile) overloads that write to tokens/<profile>.json. The existing no-arg overloads delegate to the active profile and fall back to the legacy tokens.json for pre-upgrade installs. On first per-profile write, the legacy single-file is deleted (idempotent migration). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: make TokenStore.DeleteAsync async, add Delete(profile) test, remove dead TempDir Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add TenantDiscovery orchestrator with profile merging Extract IAuthProxyClient interface from AuthProxyClient, add TenantDiscovery class with RunAsync (0/1/many tenant handling, error mapping) and static MergeProfiles helper. Add NSubstitute to test project; 7 new tests all pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add tests for UpstreamError arm and picker-returns-null path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: MergeProfiles inherits template from active profile When merging discovered tenants into existing profiles, prefer the user's active profile as the inheritance template before falling back to "default", then to a blank profile. Adds a test covering the case where ActiveProfile is not "default". Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Wire tenant discovery into kapacitor setup When --server-url is not passed, kapacitor setup now runs GitHub Device Flow against the shared auth proxy, discovers the user's Capacitor tenants, creates a profile per tenant, and uses the selected tenant for the rest of setup — skipping the redundant login step in Step 2 since the GitHub token can be exchanged directly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: extract RunDiscoveryAsync, fix re-run check, unify login confirmation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add kapacitor login --discover Adds a --discover flag to kapacitor login that runs tenant discovery across all the user's GitHub org memberships, exchanges tokens for each discovered tenant (saving them as named profiles), and sets the picked tenant as the active profile — no pre-existing configuration required. Also adds a named-profile overload of ExchangeAndSaveAsync and 5 unit tests covering the new overload. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address review: parallel token exchange, document GitHubApp assumption, tighten tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use auth.kapacitor.ai as the production proxy URL * Address Qodo review: share HttpClient across parallel tenant token exchanges Add a 5-param ExchangeAndSaveAsync overload that accepts a caller-supplied HttpClient, and refactor the 4-param overload to delegate to it. In HandleDiscoverLoginAsync, pass the already-open http instance into the Task.WhenAll fan-out so the whole discovery run reuses one HttpClient instead of creating one per tenant, preventing socket/port exhaustion with many tenants. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address Qodo review: normalize tenant URLs, dispose responses, full logout - Fix #2: Apply AppConfig.NormalizeUrl to tenant origins in MergeProfiles, HandleDiscoverLoginAsync, and SetupCommand.RunDiscoveryAsync so trailing slashes never produce double-slash URLs in subsequent requests - Fix #3: Add `using` to HttpResponseMessage in GetGitHubClientIdAsync and DiscoverTenantsAsync to ensure connection resources are always released - Fix #4: DeleteAsync now deletes all *.json files under tokens/ (not just the active profile), so multi-profile logout is complete; add test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Address Copilot review: null-safe ActiveProfile, validate profile names, harden DeleteAsync, status-agnostic error message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
May 11, 2026
- PluginCommand.InstallCodexHooks/RemoveCodexHooks and StatusCommand.IsCodexHooksInstalled: use type-safe pattern matching instead of JsonNode indexers + GetValue<string>(). A malformed user-edited hooks.json no longer turns into a silent install/remove/detection failure. Factored out internal helper EntryReferencesKapacitorCodexHook shared by both commands. - CodexHookCommand.Handle: extract a TryGetString helper and route every payload-field read through it, so a schema drift (e.g., non-string hook_event_name) returns 0 instead of crashing the host process. - WatcherManager.BuildSpawnArgs: quote --vendor like other args. Addresses Qodo findings #2, #3, #4 on PR #55. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
May 11, 2026
…--codex) (#55) * [AI-70] CodexPaths: expose Home and UserHooksJson * [AI-70] kapacitor watch: thread --vendor through SendTranscriptBatch * [AI-70] WatcherManager: thread vendor through SpawnWatcher * [AI-70] InlineDrainAsync: thread vendor onto TranscriptBatch * [AI-70] kapacitor codex-hook: dispatcher for SessionStart/Stop/PermissionRequest * [AI-70] CodexHookCommand tests: restore Console.Out in finally * [AI-70] kapacitor plugin install --codex writes ~/.codex/hooks.json * [AI-70] kapacitor status: report Claude and Codex hook installation * [AI-70] docs: surface --codex flags in help output * [AI-70] Remove accidentally-committed .mcp.json The .mcp.json was leaked in from local session config and configures the Linear MCP server globally. The daemon's AgentOrchestrator reads repo-committed .mcp.json to enable MCP servers, so committing this file would silently turn on third-party network access for everyone using the repo. Unrelated to AI-70. * [AI-70] Qodo fixes: type-safe JSON parsing + quoted vendor arg - PluginCommand.InstallCodexHooks/RemoveCodexHooks and StatusCommand.IsCodexHooksInstalled: use type-safe pattern matching instead of JsonNode indexers + GetValue<string>(). A malformed user-edited hooks.json no longer turns into a silent install/remove/detection failure. Factored out internal helper EntryReferencesKapacitorCodexHook shared by both commands. - CodexHookCommand.Handle: extract a TryGetString helper and route every payload-field read through it, so a schema drift (e.g., non-string hook_event_name) returns 0 instead of crashing the host process. - WatcherManager.BuildSpawnArgs: quote --vendor like other args. Addresses Qodo findings #2, #3, #4 on PR #55. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
3 of 6 tasks
alexeyzimarev
added a commit
that referenced
this pull request
May 15, 2026
…eSystemOverlay against symlinks Qodo review findings #2 (empty HOME breaks paths) and #3 (overlay follows symlinks): - PathHelpers.HomeDirectory now treats HOME as missing when it's empty, whitespace, or a non-rooted relative path — falling back to Environment.SpecialFolder.UserProfile in all three cases. Previously HOME="" or HOME="foo" would silently produce a relative HomeDirectory, causing daemon/CLI config reads/writes to land in CWD instead of the user profile. - FileSystemOverlay.OverlayDirectory passes EnumerationOptions { AttributesToSkip = FileAttributes.ReparsePoint } to both GetFiles and GetDirectories. Symlinks (files and dirs) and other reparse points are now skipped during overlay, preventing symlink-loop recursion and accidental copy of out-of-repo content into the worktree. Both ClaudeLauncher and CodexLauncher use this helper to merge .claude/.codex from the source repo into the spawned worktree. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
May 15, 2026
* [AI-68] docs: design for hosted Codex agents on macOS and Linux
Captures the strategy-pattern split of AgentOrchestrator into per-vendor
IHostedAgentLauncher impls, the Tomlyn-based ~/.codex/config.toml pre-trust
writer, LocalPermissionBridge per-vendor URL routing, and the CLI codex-hook
PermissionRequest bridge bounce. Includes the confirmed Codex pre-trust
spike findings. Out-of-scope follow-ups tracked in AI-632 (PR review) and
AI-633 (sandbox/approval selection).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: address review findings on hosted-Codex design
Six follow-ups from review on /Users/alexey/.../design.md (file:line refs):
- Add §3.5 covering the kapacitor-server wire-contract changes (DaemonCommands,
CapacitorHub, LaunchAgentDialog, AgentRunStarted read-model) so AI-68 is
scoped as a single cross-repo issue with a paired-PR rollout ordering.
- §4.3 CodexLauncher.Prepare gains a fail-fast hook preflight that throws
CodexHooksNotInstalledException when ~/.codex/hooks.json doesn't reference
kapacitor codex-hook for SessionStart / Stop / PermissionRequest — keeps
hosted Codex from silently running without session linking, watcher, or
permission round-trip.
- §4.4 adds AgentInstance.Vendor and routes the failed-launch cleanup path
(orchestrator line 361) through launcher.Cleanup instead of inlined
Claude-specific calls.
- §5.2 switches CodexPaths.Home from init-once to a computed property so
HOME overrides take effect mid-test, and creates the parent .codex
directory before atomic write so first-time Codex users get pre-trusted.
- §7.3 Codex CLI bridge bounce becomes fail-closed (deny + nonzero exit on
any HTTP failure), with loopback validation extracted from
PermissionRequestCommand.cs:70 as a shared DaemonBridgeUrl helper.
- §8 test list updated for the new fail-closed semantics, the directory
creation case, the hook preflight, and the failed-after-prepare cleanup
regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: address second-round review on hosted-Codex design
Five follow-ups + the open question from the second review pass:
- §7.0 (new) explicitly updates PermissionRequestCommand.cs:63 to post to
/claude/permission-request when the bridge URL split lands. Adds the
hosted-Claude regression test under §8.3 so the URL migration cannot
break Claude silently.
- §3.5 server rollout: RequestPermission gains an OPTIONAL fifth `vendor`
parameter (defaulted to "claude") instead of a required one. SignalR's
positional-arg dispatch lets old daemons (four-arg call sites) keep
working against the new server. Server-first ordering is now safe.
LaunchAgentCommand.Vendor on the server side also defaults to "claude"
for old-daemon JSON-deserialisation compatibility.
- §4.3 hook preflight accepts either user-scope (~/.codex/hooks.json) OR
project-scope (worktree/.codex/hooks.json after overlay) installs.
Overlay runs BEFORE preflight so project hooks copied from the source
repo satisfy the check. Splits hooks across the two scopes are also
accepted in aggregate.
- §4.3 + §10 move EntryReferencesKapacitorCodexHook + CodexHookEvents +
hooks.json parsing helpers to new src/Kapacitor.Core/CodexHooksParser.cs
so the daemon can use them (Kapacitor.Daemon only references Core, not
the CLI project).
- §8.8 AOT publish covers BOTH src/kapacitor/kapacitor.csproj AND
src/Kapacitor.Daemon/Kapacitor.Daemon.csproj. Daemon is where Tomlyn
lands so its publish has to be the one that's grepped for IL3050/IL2026.
- Open question resolved: §3.5 + §10.2 pull permission-request UI vendor
awareness into AI-68 scope. PendingPermission carries Vendor;
PermissionRequestCard hides Always Allow + Update Input for Codex.
Server tests added.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: address third-round review on hosted-Codex design
Four follow-ups from third review:
- §3.5 + §6.2 + §10.2 switch from "optional fifth parameter on
RequestPermission" to "new RequestPermissionV2 hub method with the same
four args plus vendor". This repo's existing SignalR regression notes
(SendTranscriptBatchWireTests.cs:15, RemoteAgentService.cs:142) document
that HubConnection.InvokeAsync dispatches by exact arity and does NOT
backfill optional values. The named-method approach is the only safe
pattern. Old daemons keep calling four-arg RequestPermission; new
daemons invoke V2. Wire-level tests added.
- §3.5 expands permission-UI vendor awareness beyond PermissionRequestCard
to cover AgentChatView.razor:62 ("Yes, always" inline option) and
ElicitationOption.cs:13 (Claude-only "always" variant). Vendor threads
through InterruptIssued, PendingPermission, SignalR PermissionRequested
push, AgentChatContext, and AgentChatView. Server-side tests added for
each surface.
- §6.2 updates the daemon's ServerConnection.RequestPermissionAsync
signature to take string vendor and to invoke "RequestPermissionV2".
New LocalPermissionBridgeTests cover that POST /{token}/codex/...
reaches the server with vendor = "codex" and the claude path with
vendor = "claude".
- §5.4 reconciles the "best-effort vs fail-fast" semantics that previously
contradicted §4.3. Scopes the swallow-and-continue policy to filesystem
/ TOML errors only; CodexHooksNotInstalledException stays fail-fast and
produces LaunchFailed. Documents the wrapper shape with explicit catch
ordering so future fail-fast preconditions follow the same pattern.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: address fourth-round review on hosted-Codex design
Four follow-ups from fourth review:
- PermissionRequested SignalR broadcast hits the same arity trap as
RequestPermission. Spec switches to a dual-broadcast: keep the existing
five-arg PermissionRequested unchanged for old clients, add a sibling
six-arg PermissionRequestedV2 that includes vendor. RemoteAgentService
subscribes to both. New wire-regression tests cover both shapes firing
on the same trigger.
- InterruptIssued comes from the external Kurrent.Agent.Schema package
and is not editable here. Vendor is carried on Capacitor-owned
ClaudeCodePermissionExtension instead (ClaudeCodeExtensions.cs:107).
§3.5 + §10.2 updated to reflect the extension as the carrier.
- IHostedAgentLauncher.Prepare XML doc rewritten to acknowledge both
failure modes — filesystem/parse errors swallowed inside the launcher,
typed preflight exceptions allowed to propagate to LaunchFailed —
pointing to §5.4's wrapper shape for the orchestrator-side catch
ordering.
- Permission-elicitation UI surfaces corrected to match the actual code
shape: PendingPermission lives in ElicitationOption.cs (add Vendor
field), AgentChatView's "Yes, always" is hardcoded markup (Razor-render
conditional), AgentChatContext exposes scalar fields (add
PermissionVendor, not a nested CurrentPermission object). Dropped the
misnamed ElicitationOption_factory test.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: drop back-compat layers; vendor is a required field
Reverts the V2 hub method, dual SignalR broadcast, and nullable-default
workarounds added in prior amendments. Staging is the only environment
and the deployment is single-controlled — server, daemon, and clients
upgrade together as part of this Linear issue. The compatibility layers
were carrying rollout-window complexity for a window that does not exist
in practice.
Changes:
- §3.1 LaunchAgentCommand.Vendor is a required positional parameter
(no C# default). Required positionals moved before optional ones to
satisfy C# language rules.
- §3.5 RequestPermission bumped to five args (adds vendor); no V2 alias.
- §3.5 PermissionRequested broadcast bumped to six args (adds vendor);
no V2 alias, no dual-broadcast.
- §3.5 DaemonCommands.LaunchAgentCommand.Vendor required (no
"claude" default). Old-daemon JSON deserialisation compat language
removed.
- §6.2 daemon-side ServerConnection.RequestPermissionAsync invokes plain
RequestPermission (not RequestPermissionV2).
- §10.2 server file list and tests pruned: dropped Old_daemon_simulator,
PermissionRequested_old_event_still_fires, Server_dual_broadcasts,
LaunchAgentCommand_deserialises_with_default_vendor. Added
LaunchAgentCommand_round_trips_required_vendor_field and
LaunchAgentCommand_rejects_unknown_vendor_at_controller_boundary.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] docs: implementation plan for hosted Codex agents (CLI/daemon repo)
21-task TDD plan covering the §10.1 surface — Tomlyn dep + AOT gate,
CodexHooksParser move to Core, CodexPaths computed properties, DaemonBridgeUrl
loopback helper, CodexHooksNotInstalledException, CodexConfigWriter,
LaunchAgentCommand/AgentRunStarted Vendor fields, ServerConnection vendor
plumbing, Claude CLI hook URL migration, IHostedAgentLauncher abstraction,
ClaudeLauncher extraction, AgentInstance.Vendor, orchestrator vendor routing,
DaemonConfig.CodexPath, CodexLauncher (overlay + preflight + pre-trust +
BuildArgs), LocalPermissionBridge per-vendor routing, Codex CLI hook bridge
bounce (fail-closed), DI registration, README updates, final AOT gate.
Cross-repo §10.2 server-side work is explicitly scoped out — paired PR
plan lives in the kapacitor-server worktree.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] deps: add Tomlyn for Codex config.toml pre-trust
* [AI-68] deps: pin Spectre.Console + Microsoft.* to versions matching prior float ranges
* [AI-68] core: extract CodexHooksParser for daemon-side preflight reuse
Move EntryReferencesKapacitorCodexHook and CodexHookEvents from PluginCommand
into a new CodexHooksParser in Kapacitor.Core so Kapacitor.Daemon can reference
the predicate without taking a dependency on the CLI project. Update all call
sites (PluginCommand, StatusCommand) to delegate to CodexHooksParser.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] test: enforce CodexHookEvents order positionally
* [AI-68] core: make CodexPaths computed so tests can scope HOME
* [AI-68] cli: extract DaemonBridgeUrl loopback helper for Codex reuse
* [AI-68] cli: collapse TryGetLoopbackDaemonUrl to single delegation
* [AI-68] daemon: add CodexHooksNotInstalledException for preflight fail-fast
* [AI-68] daemon: CodexConfigWriter pre-trusts worktrees in ~/.codex/config.toml
Uses Tomlyn 2.4.0 TomlTable model with AOT-safe GetBuiltInTypeInfo<TomlTable>
via a thin TomlSerializerContext subclass — avoids RequiresDynamicCode/
RequiresUnreferencedCode on the generic Deserialize<T> overload.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] test: serialise HOME-mutating tests via shared NotInParallel key
* [AI-68] models: required Vendor field on LaunchAgentCommand
Add required string Vendor positional to LaunchAgentCommand (between
AttachmentIds and Kind to satisfy C# required-before-optional rule).
Daemon validates at top of HandleLaunchAgent; rejects anything other than
"claude" or "codex" via LaunchFailedAsync. Wire-format test added to
confirm round-trip serialization.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] models: required Vendor field on AgentRunStarted
* [AI-68] daemon: ServerConnection.RequestPermissionAsync gains vendor param
* [AI-68] cli: Claude permission-request posts to /claude/permission-request
* [AI-68] daemon: define IHostedAgentLauncher strategy interface
* [AI-68] daemon: WIP extract ClaudeLauncher (next: wire through orchestrator)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] daemon: AgentInstance carries Vendor for cleanup dispatch
* [AI-68] daemon: orchestrator dispatches launches through IHostedAgentLauncher
HandleLaunchAgent now looks up the vendor-specific launcher from a
constructor-injected dictionary, invokes Prepare/BuildArgs/Cleanup,
and surfaces CodexHooksNotInstalledException as an actionable
LaunchFailed. CleanupAgentAsync and the failed-launch catch block
dispatch teardown through the same dictionary, with a NoopPtyProcess
stand-in so the launcher can run without a live PTY. A few
ServerConnection methods become virtual so tests can override them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-68] daemon: add CodexPath config field
* [AI-68] daemon: CodexLauncher with overlay + hook preflight + pre-trust
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] daemon: LocalPermissionBridge per-vendor URL routing + response shapes
Switch from flat /{token}/permission-request to /{token}/{vendor}/permission-request.
Claude responses carry the full shape (applyPermissions, updatedInput); Codex responses
emit behavior only. Vendor is forwarded to RequestPermissionAsync replacing the "claude"
stub. Legacy URL now returns 404. FakeServerConnection captures vendor for assertion.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] cli: codex-hook PermissionRequest bridges through daemon (fail-closed)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] daemon: DI registers ClaudeLauncher + CodexLauncher
* [AI-68] docs: README + help text for hosted Codex agents
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] verify: AOT clean + all tests green
* [AI-68] cli: long PermissionRequest hook timeout (24h) so dashboard decisions aren't killed at 30s
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] config: wire claude_path + codex_path through daemon profile
Add claude_path and codex_path as settable daemon profile fields so
users can override the binary paths via `kapacitor config set
daemon.claude_path` / `daemon.codex_path` or via
KAPACITOR_CLAUDE_PATH / KAPACITOR_CODEX_PATH env vars. Previously
DaemonConfig carried these fields but nothing populated them from the
profile — the daemon always resolved "claude" / "codex" via PATH.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] fix: harden HomeDirectory against empty/relative HOME and FileSystemOverlay against symlinks
Qodo review findings #2 (empty HOME breaks paths) and #3 (overlay follows
symlinks):
- PathHelpers.HomeDirectory now treats HOME as missing when it's empty,
whitespace, or a non-rooted relative path — falling back to
Environment.SpecialFolder.UserProfile in all three cases. Previously
HOME="" or HOME="foo" would silently produce a relative
HomeDirectory, causing daemon/CLI config reads/writes to land in CWD
instead of the user profile.
- FileSystemOverlay.OverlayDirectory passes EnumerationOptions
{ AttributesToSkip = FileAttributes.ReparsePoint } to both GetFiles
and GetDirectories. Symlinks (files and dirs) and other reparse
points are now skipped during overlay, preventing symlink-loop
recursion and accidental copy of out-of-repo content into the
worktree. Both ClaudeLauncher and CodexLauncher use this helper to
merge .claude/.codex from the source repo into the spawned worktree.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-68] test: globally serialize Stop + PermissionRequest_records to fix in-suite Console.SetOut race
Both tests use Console.SetOut to capture stdout. Stop_maps_to_session_end_codex_route
takes ~3m20s (real WatcherManager path through InlineDrainAsync + WhatsDoneGenerator
spawn against an absent transcript), holding Console.Out captured for the full
duration. With the suite at 727 tests, the long capture window plus TUnit's
parallel scheduling drops captured output for the other ConsoleSerialGroup test
intermittently — both pass in isolation, fail together in suite.
Tightening from NotInParallel(ConsoleSerialGroup) (group-scoped) to plain
NotInParallel (globally sequential) for these two tests guarantees they don't
overlap with any other test, removing the race surface. Other ConsoleSerialGroup
tests in the file (the new PermissionRequest_with_daemon_url_* set from AI-68)
keep their group-scoped attribute — they're fast and serializing only against
each other is sufficient.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks done
alexeyzimarev
added a commit
that referenced
this pull request
May 21, 2026
…er header metadata Finding P1 #3: CLI's ListBubblesAsync returns rows in SQLite storage order (undefined). BuildPayload now builds a position map from fullConversationHeadersOnly and sorts assembledBubbles by that map; orphaned bubbles not present in the headers list are dropped. Fallback to SQLite order when orderMap is empty (parse failure). Finding P2 #5: RawComposerHeader only carried 5 fields; subtitle, totalLinesAdded, totalLinesRemoved, filesChangedCount, and trackedGitRepos were silently dropped, leaving the server's CursorSessionExtension always empty. GetComposerHeaderAsync now parses all fields (all optional/defaultable) and BuildPayload populates the wire CursorHeader from the richer record. Tests: CursorStateReaderTests gains two new cases (extended fields present / absent). CursorCommandTests gains Import_orders_bubbles_by_fullConversationHeadersOnly, which inserts three bubbles in shuffled SQLite order and asserts the POST body reflects the fullConversationHeadersOnly order. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
alexeyzimarev
added a commit
that referenced
this pull request
May 21, 2026
* [AI-661] CursorPaths OS resolution
* [AI-661] CursorStateReader (Microsoft.Data.Sqlite, ReadOnly)
* [AI-661] CursorPayloadAssembler: allowlist + redaction + size caps
* [AI-661] kapacitor cursor import command
Wire up end-to-end Cursor IDE session ingestion: CursorCommand.RunAsync
resolves workspaces by walking workspaceStorage, filters to agent-mode
composers, checks server-side watermark, builds the import payload via
CursorStateReader + CursorPayloadAssembler, enforces 10 MB hard cap,
and POSTs to /hooks/cursor-import. Adds help-cursor.txt and wires the
"cursor" subcommand in Program.cs and top-level usage text.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-661] README: document kapacitor cursor import
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-661] Fix CursorPayloadAssembler to handle object-typed fields
Real Cursor databases store toolFormerData.params and toolFormerData.result
as JSON objects (not JSON-encoded strings) for some bubble types, causing
InvalidOperationException ("element of type 'Object'") during payload assembly.
- Add GetStringOrSerialize helper: returns GetString() for String kind,
GetRawText() for Object/Array/Number/True/False, null for Null/Undefined.
- Apply GetStringOrSerialize to all toolFormerData string fields (params,
result, rawArgs, toolCallId, name, status) and top-level text/thinking.
- Add GetCreatedAtString to tolerate numeric Unix-ms timestamps for createdAt.
- Defensive ValueKind guards in BuildPayload for modelName, selectedModels,
fullConversationHeadersOnly, generatingBubbleIds, and status fields.
- Add regression test: Handles_object_typed_params_and_result.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-661] Address Qodo review: busy-session skip, shared auth helper, non-zero exit on POST failure
- Skip composers with non-empty generatingBubbleIds (read composerData up-front, pass to BuildPayload to avoid double read)
- Replace manual HttpClient + token with HttpClientExtensions.CreateAuthenticatedClientAsync; use GetWithRetryAsync / PostWithRetryAsync
- Track failed POSTs; return exit code 1 when any upload fails
- Drop token parameter from RunAsync signature and corresponding TokenStore.LoadAsync block in Program.cs
- Add tests: Import_skips_composer_with_generating_bubbles_in_flight, Import_returns_nonzero_when_post_fails
* [AI-661] Order bubbles by fullConversationHeadersOnly; surface composer header metadata
Finding P1 #3: CLI's ListBubblesAsync returns rows in SQLite storage order (undefined).
BuildPayload now builds a position map from fullConversationHeadersOnly and sorts
assembledBubbles by that map; orphaned bubbles not present in the headers list are
dropped. Fallback to SQLite order when orderMap is empty (parse failure).
Finding P2 #5: RawComposerHeader only carried 5 fields; subtitle, totalLinesAdded,
totalLinesRemoved, filesChangedCount, and trackedGitRepos were silently dropped,
leaving the server's CursorSessionExtension always empty. GetComposerHeaderAsync
now parses all fields (all optional/defaultable) and BuildPayload populates the
wire CursorHeader from the richer record.
Tests: CursorStateReaderTests gains two new cases (extended fields present / absent).
CursorCommandTests gains Import_orders_bubbles_by_fullConversationHeadersOnly, which
inserts three bubbles in shuffled SQLite order and asserts the POST body reflects
the fullConversationHeadersOnly order.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-661] Mark over-cap composers as failures so CLI exits non-zero
When a composer payload exceeds the hard cap, increment `failed` so the
command returns exit code 1. Adds a `payloadHardCapBytes` parameter
(default = real 10 MB constant) to make the threshold injectable in tests
— avoids needing a 10 MB fixture file while keeping the production cap
unchanged. New test: `Import_returns_nonzero_when_payload_exceeds_hard_cap`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* [AI-661] Filter content blobs to ordered bubbles; parse Cursor branch objects
- CursorCommand.BuildPayload: moved blob-key collection AFTER orderedBubbles
filter so orphan bubbles (not in fullConversationHeadersOnly) can never
contribute blob keys — prevents uploading blobs no session event references.
- CursorStateReader.GetComposerHeaderAsync: parse branches as object array
(real Cursor shape: [{branchName,lastInteractionAt}]) with defensive fallback
for legacy string-array shape (branchNames: ["main","feature/x"]).
- Tests: Import_content_blobs_excludes_orphan_bubble_blobs verifies the blob
filter; Reads_extended_composer_header_fields updated to real object shape;
Reads_branches_from_legacy_string_array_shape added for backward compat.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
7 tasks done
alexeyzimarev
added a commit
that referenced
this pull request
May 22, 2026
* [AI-665] Normalize workspaceFolder before Cursor path redaction
CursorPayloadAssembler.RedactPaths previously did a single exact-string
Replace(repoPath, "/repo"). On Windows that misses any path that doesn't
exactly match repoPath's normalisation — different separator, different
case, trailing-slash variant, JSON-escaped backslashes — and absolute
paths leak to the server.
Two coordinated fixes:
* CursorCommand.ResolveWorkspaces normalises folderPath once (Path.GetFullPath
+ TrimEnd) and yields the normalised value to all call sites, so the
redaction repoPath always arrives in canonical native form.
* CursorPayloadAssembler.RedactPaths now tries three variants of the
trimmed repoPath against the bubble JSON: forward-slash, native
backslash, and the JSON-escaped backslash form ("\\\\"). Comparison is
case-insensitive on Windows, case-sensitive on POSIX. The escaped form
is replaced before the raw backslash form so we don't consume half of
an escape sequence mid-string.
Follow-up from Qodo review on #85 (bug #3 — Windows correctness).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* [AI-665] Log workspace skips when folder path can't be normalized
Previous catch silently continued when NormalizePath threw, leaving users
with only the generic "No matching Cursor workspace found." message and
no clue which workspace.json was malformed. Now writes a one-line
diagnostic to stderr including the workspace storage subdir, the
problematic folder path, and the exception message before continuing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 of 5 tasks
realtonyyoung
added a commit
that referenced
this pull request
Jun 27, 2026
…ations #2: replace the ledger's parent line-count key with a SHA-256 content fingerprint over the parent transcript + children, so a same-line-count mutation (tool part completing, in-place edit, changed/added child) invalidates the skip and re-imports. Fingerprint computed at classify, carried on SourceMeta, recorded after session-end. Document #1 (server returns 200 on swallowed per-event write failure) and #3 (subagent lifecycle hooks return OK on write failure) as known limitations with server-repo follow-ups. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jun 27, 2026
…20) (#192) * Surface OpenCode in `kcap status` hooks line AI-921 wired OpenCode end-to-end (installer, plugin, hook dispatcher, import-less live ingest) but left it off the `kcap status` Hooks line and the `kcap status --help` text — the same vendor-surface gap that hit Gemini and Kiro before (PR #169). The vendor fully works; it just wasn't reported, so nothing failed and the miss was invisible to build/tests. - StatusCommand.BuildHooksStatusLine: add `opencode` param + `OpenCode ✓/✗` entry (canonical order, last — like Pi it ships a live-ingest plugin file, not shell hooks), detected via OpenCodeExtensionInstaller.IsInstalled(OpenCodePaths.KcapPlugin()). - help-status.txt: Hooks line now reads "Pi / OpenCode live-ingest extensions". - StatusCommandHooksTests: cover OpenCode in both BuildHooksStatusLine tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * README: list OpenCode in the setup-wizard detection paragraph The `## CLI commands` setup section enumerated detected agents only through Pi, omitting SST OpenCode — the quick-start paragraph already lists it, and CLAUDE.md requires both stay in sync. Mirrors the existing phrasing and notes that, like Pi, OpenCode has no shell hooks so the wizard installs a live-ingest plugin. Flagged by PR review on #178. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Design spec: kcap import --opencode (historical OpenCode import from SQLite) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revise OpenCode import spec per Codex design review - Fix subagent lifecycle order (children before parent session-end) - Resolve subagent watermark via Gemini import precedent (startLine 0 + idempotency) - Soften byte-match claim to final-state/normalizer-compatible; document watermark caveat - Pin part ordering (time_created,id) with empirical validation note - Move SQLite dependency to CLI project (keep daemon/Core AOT-clean) - Use /hooks/set-title for native title; ms epoch conversion - Add edge-cases section; accept shared send-failure behavior (idempotency) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revise OpenCode import spec per Codex 2nd-pass review (rev3) - Replace line-number resume with binary New/AlreadyLoaded classification (live snapshot vs import final-state line spaces are incompatible) - Add importable-line predicate (Pi IsImportRelevantLine analog) for MinLines - Fix synthesis query: order by message chronology, not lexical message_id; LEFT JOIN so empty messages aren't dropped - Strengthen part-ordering pre-merge verification requirements - Document OpenCode-specific send-failure consequence (summary/model recompute) - Make parent/child lifecycle sequencing explicit; deterministic child order - Fix Architecture/Core-vs-CLI contradiction; expand edge cases (no-canonical-event messages, grandchildren, mixed live/historical) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Implementation plan: kcap import --opencode (TDD, 10 tasks) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revise OpenCode import plan per Codex plan review - Add opt-in strict transcript sender (failOnError) — OpenCode aborts before session-end/subagent-stop on batch failure (binary policy has no resume) - Make IsImportRelevantLine role-aware + hidden-aware to match server normalizer - Subagent start/stop use the real temp transcript path; stronger ordering test (agent_id, agent_type, vendor, full POST order) - Order-sensitive assertions (string.Join+IsEqualTo) instead of IsEquivalentTo - Harden AOT gate: explicit RIDs + pipefail, no masked publish failures - Add WAL-writer read test; null-dir / zero-message / timestamp-magnitude cases - Cleaner fixture JSON via JsonObject - Reconcile spec (strict sender, role-aware predicate) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revise plan+spec per Codex 2nd plan review: completeness-gated repair Blocker fix: binary AlreadyLoaded couldn't repair a partial multi-batch import (server HWM advances on transcript ingest, not session-end). Replace with completeness-gated classification: - New / AlreadyLoaded(ended) / Partial-repair(watermark, not ended) / TooShort - repair replays full transcript with line numbers offset above HWM (lineNumberOffset param added to SendTranscriptBatches); dedup by canonical prt_ id - strict sender's role clarified: keeps session not-ended on failure so re-run repairs - per-subsession gating via ?agentId= (skip if SubagentCompleted, else repair) - new Task 0: confirm server contract (ended signal, dedup-by-id, HWM filter) + fallback Also from review: align IsImportRelevantLine with server normalizer (Length>0, assistant id requirement, tool fields null-checked); AOT gate exit 1 not break; WAL test asserts sidecars + settles Cache=Private; timestamp seconds test; tighten subagent test (child transcript between start/stop); repair integration test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revise plan+spec per Codex 3rd plan review: CLI-side import ledger Codex confirmed no data-loss blocker remains, but the current server exposes no ended signal — so the completeness-gated design would run in always-replay mode (re-send every session each run). Per decision, add a client-side import ledger as the completeness signal instead: - OpenCodeImportLedger (Task 4b): per-machine, per-server record of fully-imported sessions (keyed by server URL + reconstructed line count), AOT-safe source-gen JSON - classification: ledger hit -> AlreadyLoaded (skip); else New / Partial-repair - ledger written only after session-end succeeds; strict sender keeps partials unrecorded - drop the speculative server ended-signal plumbing (ServerState); Task 0 now confirms only dedup-by-id + HWM filter - children: no per-child gate (complete parent skipped wholesale via ledger) - tests isolated via fixture LedgerPath; add ledger round-trip, second-run-skip, and batch2-failure-then-rerun-repairs (WireMock scenario, 150 lines) tests Also from review: HasField -> string-kind (server Str parity); checked offset arithmetic for overflow; fix stale no-watermark-left comment; Task 0 reframed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Task 0 confirmed: kcap-server repair contract verified against source Verified against kurrent-io/kcap-server@main: - dedup by canonical EventId (HashGuid of prt_/message id), line-number-independent - HWM filter drops line_number <= currentHwm before normalization - last-line returns last_line_number only (no ended field -> ledger is required); reads max lineNumber over last 50 events backward (under-report caveat noted) - HWM + dedup keyed sessionId|agentId; last-line accepts ?agentId= Repair design inherits the live watcher reconnect-resend idempotency profile. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build: add Microsoft.Data.Sqlite to CLI for OpenCode import Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OpenCodeDb read-only reader, line reconstruction, importable predicate Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OpenCode import ledger (client-side completeness record) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: OpenCodeImportSource discovery + ledger-gated classify + import + subagents Includes opt-in failOnError/lineNumberOffset on SessionImporter.SendTranscriptBatches (defaulted; peers unchanged). Parent + child import with strict send + repair-above-HWM. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: wire --opencode import filter + register OpenCodeImportSource Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: OpenCode import integration tests (lifecycle, subagents, repair, ledger) Also fix IL2026/IL3050 in OpenCodeDb: cast to JsonNode so Add(JsonNode?) is chosen over the AOT-unsafe generic Add<T> (per CLAUDE.md). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: document kcap import --opencode (help + README) Corrects the README claim that OpenCode capture is live-only / has no import. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * harden OpenCode import per code review: discovery guard, cancellation, overflow - DiscoverAsync: guard db open/query so a corrupt/schema-drifted opencode.db warns and skips OpenCode instead of crashing the whole import run (other vendors) - QuerySessions: skip malformed/null rows instead of aborting the scan - propagate OperationCanceledException out of the import catches (was swallowed as Failed) - checked() on repair line-number offsets (parent + child) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat: ledger keyed by content fingerprint; document server-side limitations #2: replace the ledger's parent line-count key with a SHA-256 content fingerprint over the parent transcript + children, so a same-line-count mutation (tool part completing, in-place edit, changed/added child) invalidates the skip and re-imports. Fingerprint computed at classify, carried on SourceMeta, recorded after session-end. Document #1 (server returns 200 on swallowed per-event write failure) and #3 (subagent lifecycle hooks return OK on write failure) as known limitations with server-repo follow-ups. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review: fix NUL byte in source, propagate cancellation in helpers, cite AI-1023 Codex final pre-merge review (no blockers): - replace the literal NUL fingerprint separator with a backslash-u0000 escape so the .cs file is text (rg/tools no longer treat it as binary); runtime unchanged - rethrow OperationCanceledException in the watermark-probe and PostHookAsync catches so cancellation is not masked as ProbeError/hook-failure - name AI-1023 in the spec server-side limitation notes Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 3, 2026
…ailure (review round 2) Addresses follow-up review on the proactive-refresh change: - Legacy fallback (Qodo #2): RefreshIfExpiringAsync read only the per-profile token file, so a pre-upgrade install with only tokens.json got no proactive refresh. Extract LoadWithLegacyFallbackAsync (shared with LoadAsync) and use it, so the legacy token is refreshed — and migrated into the per-profile store when the refresh persists under the lock. - Lock contention (Qodo #3): a 15s cross-process-lock acquisition timeout returned null, which the daemon reported as a refresh Failure (misleading "run kcap login" warning + backoff) even though no endpoint call was made. Add an onLockContended callback (proactive path only; reactive GetValidTokensAsync is unaffected — default null) and a ProactiveRefreshOutcome.Contended that the loop logs at Debug with no warning and no backoff (contention is transient). Qodo #1 (profile-switch miswrite) was already resolved in 783b65b. Adds a TokenRefreshLoop test for the Contended path; legacy fallback is covered via the shared LoadWithLegacyFallbackAsync (LoadAsync's existing fallback tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 17, 2026
… it actually sends Three related TOCTOU-closing fixes to the D0 rewrite guard, all stemming from the same theme: the guard's hash must be bound to the exact bytes actually processed/sent, and its baseline must be real from the start. 1. Bind-to-sent-bytes (finding #1): ReadNewCompleteLinesAsync now optionally (captureRawBytes: true, wired for vendor=="cursor") captures the raw byte buffer of the SAME capped read that decodes the batch's lines (NewTranscriptLines.SnapshotBytes). DrainNewLines's guard block now hashes directly from that snapshot instead of reopening the file separately to record the new-range/prior-zone hashes — closing the window where a rewrite landing between the decode read and the old reopen produced a hash for bytes the batch never actually came from. The post-ack checkpoint's trailing hash is now derived from the same already-verified snapshot instead of reopening the file after the RPC returns (a rewrite in flight during the ack could otherwise be blessed as the new baseline). 2. Reconnect rewind atomicity (finding #2): a reconnect discovering the server is behind the client now rewinds state.CursorByteOffset and the guard's checkpoint ATOMICALLY with the line cursor, via the extracted ApplyReconnectRewindAsync (testable without a live SignalR reconnect). The true byte offset of the rewound line is resolved by scanning the transcript (ResolveByteOffsetForLineAsync) rather than leaving the byte checkpoint at the later, too-far-ahead offset (which left the replayed line gap's new-range verification starting past the bytes it actually occupies). 3. Real full-prefix baseline (finding #3): the periodic full-prefix re-hash now seeds on the REAL first poll (not just lazily on the guard's own first VerifyFullPrefix call, which previously never happened before poll N) — a same-length rewrite of an already-checkpointed middle region landing in polls 1..N-1 now has a real baseline to be caught against at poll N, instead of poll N seeding the already-rewritten file as if it were the original. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 17, 2026
…h the historical import's close-and-fail path Finding #2 - SendSubagentLifecycleAsync's catch-all swallowed the typed TranscriptDeliveryAbortedException into a bare `false`, so a quarantine tripped during a CHILD's own transcript delivery made ImportSessionAsync return Failed WITHOUT ever posting the parent's best-effort session-end - even though the child's subagent-start had already landed, leaving the parent/subsession stuck Active forever (the quarantine marker makes the next run Skip at preflight instead of repairing it). The child lifecycle helper now (a) checks quarantine BEFORE posting a new child's subagent-start, so a family found quarantined never starts another child, and (b) rethrows TranscriptDeliveryAbortedException instead of catching it, so the parent's child loop (now wrapped the same way the parent's own transcript delivery already was) routes it through the shared CloseAndFailAsync best-effort session-end + Failed contract. Finding #3 - SendTranscriptBatches only re-checked abortDelivery BEFORE each batch POST. A transcript that fits in a single (or final) batch - including every transcript of <=100 lines - has no "next" batch to gate, so a quarantine marker written while that one-and-only POST was in flight was never observed anywhere; the method returned normally and the caller proceeded into child lifecycle / normal completion. abortDelivery is now also re-checked immediately AFTER every POST (mid-loop and the trailing batch), closing that window. Regression tests: a quarantine trip during a child's own transcript delivery closes the parent via best-effort session-end + Failed; a later child is never started once the family is found quarantined (even when an earlier child's own delivery was clean); a quarantine marker appearing during a single-batch (<=100 line) transcript's only POST is observed and routed through close-and-fail, at both the SessionImporter. SendTranscriptBatches level and the CursorImportSource.ImportSessionAsync level. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 17, 2026
…thread cancellation into the same-workspace scan Qodo finding #2 (Action required): ClassifyAsync unioned every same-workspace transcript path across ALL workspaces touched by a classify call and ran CursorSubagentCorrelator.Correlate over the combined set, so a child session in workspace A could be linked to a parent in workspace B whenever their canonical prompt hashes happened to match — corrupting nesting/attribution. Same-workspace discovery is meant only to give a filtered/scoped import visibility into a parent; it must also constrain correlation. Fixed by building one path map per workspace (keyed by SanitizedDir, or an isolated per-session bucket when a session carries none) and running Correlate() independently per workspace, then merging the resulting subagentLinks. Qodo finding #3 (Review recommended): DiscoverSameWorkspaceSessionPaths did a synchronous Directory.EnumerateDirectories scan without consulting the CancellationToken ClassifyAsync receives, so Ctrl-C could hang until a large/ slow-filesystem scan finished. Threaded the token through, calling ct.ThrowIfCancellationRequested() once per directory iteration (letting OperationCanceledException propagate past the existing hostile-subtree catch-alls). Adds regression tests for both: a cross-workspace prompt-collision case asserting no cross-workspace link is made, and a pre-cancelled-token case asserting ClassifyAsync throws promptly during the scan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 17, 2026
…ts in OpenCode import Same comment cleanup as the Gemini side: removes every AI-#### token and condenses multi-paragraph review-round narrative (fix #2/#3/#4/#5/#6/#7 etc.) down to the essential contract each comment is documenting. No behavior change here — OpenCode's descendant walk is SQL-based, not filesystem-based, so it has no directly analogous silent directory-enumeration catch (a query failure already propagates and fails the whole import loudly, rather than being swallowed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alexeyzimarev
pushed a commit
that referenced
this pull request
Jul 18, 2026
* Fix Kiro parent-PID watchdog name match (kiro-cli)
Adds a bounded `-cli` suffix tolerance to MatchesAgentName so the by-name
ancestry walk identifies the durable `kiro-cli` process for vendor `kiro`,
instead of falling back to the fragile getpgrp/getppid heuristic.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Freeze the watcher idle clock while disconnected
Subtracts SignalR-outage time (accrued since last activity) from the idle
measure so a transient disconnect can't false-idle-end a Codex/Antigravity
session, while repeated reconnects with no new lines still idle-end after
the connected budget.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add staged parent-dead / wedged-watcher recovery
On ParentAlreadyDead the watcher now periodically re-resolves + re-arms the
parent-exit watchdog (using the vendor process-name alias); it ends the
session only after a long, configurable ceiling with no transcript progress
and continued resolution failure, and any new progress resets the window.
This is the only end path for a wedged, alive-but-connected watcher that the
server stale sweep can't see. New env var KCAP_PARENT_DEAD_CEILING_MINUTES.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: AgentHookPoster.Spooled + PostOrSpoolAsync spool-on-lapse
Adds a Spooled outcome and PostOrSpoolAsync: on lapsed-auth or transient
(5xx/408/429/unreachable) failure the lifecycle payload is durably spooled
(HookSpool) for a later drain pass, so live capture can start regardless of
hook-POST delivery (spawn-before-post). Refs AI-1357.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: TranscriptSpool — bounded, no-drop, needs-import marker
AI-1357 Task 2: dedicated on-disk spool for undelivered transcript-tail
batches captured during a watcher outage. Unlike HookSpool (1 MB,
drop-oldest — fine for small lifecycle POSTs), this is bounded at 8 MB
per session with NO SILENT DROP: on cap exhaustion it stops appending
and writes a needs-import marker instead of truncating history, so the
session surfaces as requiring `kcap import` rather than silently losing
transcript content.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: TranscriptSpool no silent drop on I/O failure + Ignored result
Review fixes for AI-1357 Task 2:
- Append now marks needs-import (not phantom Appended) when the live
write throws — the silent-drop this class exists to prevent.
- MarkNeedsImport returns bool and logs to stderr on failure so callers
don't trust an unpersisted marker.
- Add AppendResult.Ignored for the malformed-session-id drop so it can't
be mistaken for a real append.
- Tests for the I/O-failure path and the malformed-id path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: LifecycleSpoolDrain — global cross-spool ordered drain pass
Adds a session-agnostic drain pass to run at the start of every kcap
invocation (and periodically by the daemon), enforcing per-session
ordering across the lifecycle (HookSpool) and transcript (TranscriptSpool)
spools: spooled session-start -> transcript tail (+ needs-import marker,
delivered even over cap) -> spooled session-end. Adds the route-filtered
HookSpool.DrainRoutesAsync/SessionIdsWithBacklog and
TranscriptSpool.SessionIdsWithBacklog helpers this needs; HookSpool's
existing route-agnostic DrainAllAsync is untouched (still used by
Claude/Cursor).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: spawn-before-post + Spooled for Kiro/OpenCode/Pi/Copilot hooks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: throttle spool drain per-prompt; ShouldSpawnAfter drops AuthLapsed
AI-1357 review: (1) DrainSpoolsAsync now self-throttles via an on-disk
.last-drain stamp (30s) so Kiro agentSpawn / OpenCode idle re-fires can't
attempt a network drain every prompt during an outage; reaps moved inside.
(2) ShouldSpawnAfter spawns only on Posted/Spooled — AuthLapsed spools
nothing, so spawning would orphan a session with a dropped SessionStarted.
(3) Documented the fresh-client deviation (no reusable vendor client exists
at the pre-POST drain point).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Codex spawn-before-post with stdout-first carve-out
Auth lapse now spools + spawns via PostOrSpoolAsync/ShouldSpawnAfter
(matching Kiro/OpenCode/Pi/Copilot) instead of dropping the session and
skipping the watcher. The global lifecycle/transcript spool drain runs
fire-and-forget AFTER Codex's blocking `{"continue":true}` stdout
handshake — never before, never gating it — so a large/unreachable spool
backlog can't stall the parent process. Extracted WriteSessionScopedOutput
and RunSessionStartHandshakeForTest as the seam CodexStdoutContractTests
uses to prove the ordering without a live server.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Antigravity spawn-before-post + Gemini awaited EnsureWatcherRunning
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Gemini session-start spawn-before-post + gated spool drain
Migrate Gemini's session-start off the POST-only PostAsync path to
AgentHookPoster.PostOrSpoolAsync + ShouldSpawnAfter, so an auth lapse or
transient outage spools the SessionStarted payload and STILL spawns the
watcher (was: returned without reaching EnsureWatcherRunning on anything
but Posted). Wire the throttled DrainSpoolsAsync gated to the two lifecycle
events (SessionStart/SessionEnd) so the per-turn Notification path adds no
network cost. Fix a dangling "PostHookAsync (below)" comment in
AntigravityHookCommand left by the Task 6 deletion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: shutdown final-line completion signal (newline/parseable-JSON)
The shutdown-during-outage final drain (idle-timeout / parent-exit) used
to disable the half-written-line holdback unconditionally, which could
consume a line the agent was still mid-write on, or drop a complete
newline-less final line. Add a pure IsFinalLineComplete signal (empty /
newline-terminated / last line parses as JSON — length-stability alone is
NOT proof, since a large write can pause mid-record past any bounded
window) plus a bounded (<=2s) WaitForFinalLineCompletionAsync wait in the
final-drain path. Complete -> send the newline-less final line as before;
incomplete -> keep the holdback on (never send-and-advance a truncated
line) and flag the session needs-import via TranscriptSpool so `kcap
import` can recover it later.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: close final-drain completeness TOCTOU; re-validate at consume time
Review found the completeness decision and the consuming read were two
separate reads: WaitForFinalLineCompletionAsync probed once, then
DrainNewLines consumed with holdback disabled — so a final line that
resumed growing in the gap was sent-and-advanced anyway, violating "never
consume a still-growing line."
Move the decision into the consuming read via an IncompleteFinalLinePolicy
(Hold / ConsumeIfComplete). Under ConsumeIfComplete the unterminated final
line is consumed ONLY IF the exact bytes read parse as a complete JSON
record; otherwise it is held and NewTranscriptLines.HeldIncompleteFinalLine
is set. The parse check runs on the same bytes being consumed, so there is
no TOCTOU with the bounded pre-wait (now purely advisory — gives the writer
time). RunWatch flags needs-import off the actual consume-time held result
via state.FinalDrainHeldIncompleteLine.
Also updates the stale ApplyPartialLineHoldback comment that claimed the
final drain opts out of holdback because "the file is static then" — the
exact wrong assumption this task fixes.
Tests: SplitNewCompleteLines + ReadNewCompleteLinesAsync gain parseable-
consume / unparseable-held cases, HeldIncompleteFinalLine parity, and a
grew-after-the-completeness-check TOCTOU guard asserting the resumed
partial is held, not sent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: spool undelivered transcript tail on shutdown-during-outage
At final drain during idle-timeout/parent-exit shutdown, DrainNewLines only
advances LinesProcessed past lines the hub confirmed sent. If the hub is
down (or still reconnecting) at that point, any transcript lines from
LinesProcessed to EOF were never delivered and the process exits right
after — silently dropping the tail.
Adds BuildTranscriptSpoolBatch (pure TranscriptBatch JSON builder) and
SpoolUndeliveredTranscriptTailAsync, which re-reads the undelivered tail
using the same ConsumeIfComplete completion decision as the final drain
and spools it into the dedicated TranscriptSpool (task 2) so the global
drain (task 3) replays it after recovery, without a manual `kcap import`.
Cap exhaustion still marks the session needs-import rather than dropping.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: spool tail unconditionally + redact secrets (task-8 review)
Two Critical review findings on the shutdown-during-outage tail spool:
1. Insufficient trigger → silent drop. Gating the spool on
hubConnection.State != Connected missed a HubException thrown by
SendTranscriptBatch2 while the connection stayed Connected (the generic
catch does not change connection state) — the tail was undelivered but
never spooled. Call SpoolUndeliveredTranscriptTailAsync unconditionally
on the shutdown path; it is already a no-op when position == EOF.
2. SECURITY: spooled tail skipped secret redaction. The live/inline drains
run lines through SecretRedactor.RedactLine, but the spooled tail was
written raw → secrets on disk and POSTed unredacted on replay. Redact
each spooled line exactly as the live drain does before writing.
Tests: undelivered_tail_spooled_even_though_connection_stayed_up (state-
agnostic spooling) and spooled_tail_is_secret_redacted (ghp_ token →
[REDACTED], raw secret absent from disk).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: watcher heartbeat + lock-guarded staleness reap+respawn
AI-1357 task 9. WatcherManager.IsWatcherAlive only checked the PID, so a
wedged (hung-but-alive) watcher was never restarted. The watcher now
touches a per-key heartbeat file every main-loop iteration — including
no-content drains and while disconnected/reconnecting — via the pure
WatcherHeartbeat helper (Core). IsWatcherAlive requires both a live PID
and a non-stale heartbeat (past a 30s startup grace, 20s threshold).
EnsureWatcherRunning reaps a wedged watcher (kill + respawn) under a
cross-platform spawn lock — same FileShare.None/flock primitive as
DaemonLock — so concurrent hooks racing the same key can't double-spawn.
The whole decide-and-spawn sequence is locked, not just the reap: killing
the old watcher deletes its pid file before the respawn's new one lands,
so guarding only the reap step would leave a window where a second hook
sees "no pid" and spawns unguarded (caught by the new concurrency test).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: keep heartbeat fresh during connect-retry + purge watcher sidecar files
AI-1357 task 9 review. Two Important issues.
1. Startup/reconnect outage looked "wedged" → false reap. The SignalR
connect-retry loop never touched the heartbeat, so a server outage
>=~50s at startup was indistinguishable from a wedged loop and the
hook probe would repeatedly reap+respawn a healthy-but-reconnecting
watcher. RunWatch now touches the heartbeat at the top of every
connect-retry iteration and waits via DelayWithHeartbeatAsync, which
chunks the backoff (grows to 30s > the 20s threshold) into <=5s slices
through the new pure HeartbeatSlices helper, touching before each — so
no wait window can ever cross the staleness threshold. The automatic-
reconnect path was already covered by the main loop's per-iteration
touch in the disconnected branch.
2. Sidecar file leak. Only .pid was deleted. KillWatcher now removes the
{key}.heartbeat and {key}.started markers, but deliberately NOT the
{key}.spawnlock: KillWatcher runs from inside WithSpawnLock on the reap
path, and unlinking a held lock file on POSIX lets a racing hook open a
fresh non-conflicting flock (the DaemonLock unlink-race). Spawn locks
are swept by the new WatcherManager.PurgeAuxiliaryFiles(), called by
kcap cleanup (holds no lock; also mops up orphans). CleanupCommand now
uses GetWatcherDir() so it honors KCAP_WATCHER_DIR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Kiro live usage-backfill synthetic lines (credits/context%)
AI-1357 task 10: the live kcap watch path never emitted Kiro's per-turn
credits/context% (the AI-1196 hedge only fires on import, which reads the
sibling {id}.json up front). A live drain has usually already sent the
turn's anchor AssistantMessage line by the time Kiro flushes that sidecar,
so inline enrichment can't reach it there.
Add a synthetic KiroUsageBackfilled line per turn anchor instead, mirroring
the Antigravity synthetic-USAGE-line pattern: WatchCommand.
BuildKiroUsageBackfillLine builds the JSONL; AppendKiroUsageBackfillLines
reads the sidecar via KiroUsage.AnchorMap and appends one line per anchor
not yet in the new WatchState.KiroUsageEmittedAnchors, staging them on
KiroUsagePendingAnchors for the caller to commit only after a successful
send (so a failed batch re-reads and re-stages instead of losing them).
Server-side event fold is a separate follow-up (task 13).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Cursor live subagent split via hook/backfill path (AI-1151)
CursorSubagentCorrelator only ran on the historical import path, so a live
Cursor Task/Agent subagent was ingested as its own top-level session instead
of nesting under its parent. Cursor is not watcher-backed (CursorHookCommand
backfills each session's transcript over HTTP as hooks arrive), so the
correlation now runs inline in that per-hook dispatcher via a new thin
wrapper, CursorLiveSubagentLinker.
At a child's sessionStart, ResolveParent (reusing
CursorSubagentCorrelator.Correlate) checks the sibling transcripts under the
same agent-transcripts workspace dir; a match is persisted to a small
on-disk marker (the CLI is a fresh process per hook) and diverts subsequent
hooks for that session: subagent-start replaces the top-level sessionStart,
transcript backfill is routed under the parent with agent_id=child (mirrors
CursorImportSource.SendSubagentLifecycleAsync's watermark + POST shape), and
subagent-stop replaces sessionEnd. Mid-lifecycle hooks
(beforeSubmitPrompt/afterAgentThought/telemetry) are not forwarded for a
linked child, matching the import path (which has no side channel for them
either) instead of writing to a phantom AgentSession stream that never got a
SessionStarted.
Dashless ids are used throughout so a live-then-import of the same session
converges on the same AgentSubsession-{parent}-{child} stream rather than
duplicating it (ties to AI-1358 A1). Known eventual-consistency gap
(documented in code): if the parent's Task tool_use isn't yet flushed to
disk at the child's first hook, the child is temporarily ingested top-level
until a later `kcap import --cursor` converges it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: enforce start-before-stop spool ordering on Cursor live subagent path (AI-1151)
HandleSubagentChildEventAsync posted subagent-stop (and ran the sessionStart
backfill) with no spool.HasBacklog check, unlike the sibling top-level path.
Under a transient failure that left a subagent-start undelivered in the spool,
a later subagent-stop could be POSTed ahead of its own subagent-start, leaving
the AgentSubsession stream mis-ordered / never opened.
Mirror the top-level guard: after the HandleCore drain, if the child still has
spool backlog (drain hit a transient failure), spool the fresh lifecycle event
behind it and skip the agent-routed backfill so the next hook re-drains
start-first. Also gate the sessionStart backfill on the subagent-start POST
succeeding, so the transcript can't be routed to an AgentSubsession the
now-spooled start hasn't opened yet.
Adds a regression test: a child whose subagent-start is spooled (transient
failure) does not get subagent-stop delivered ahead of it, and a later
recovered drain delivers start before stop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: wire global spool-drain pass into hook entry + daemon-periodic
AI-1357 Task 12. Runs LifecycleSpoolDrain centrally in Program.cs's
`case "hook":` (before dispatch, non-Codex; throttled via the existing
AgentHookPoster.DrainSpoolsAsync) instead of per-vendor, and adds a 60s
daemon-periodic sweep (SpoolDrainLoop) for backlogs no later hook process
ever touches (Kiro/OpenCode watcher-owned session-end, GUI idle/parent-exit).
Moves HookSpool/TranscriptSpool/LifecycleSpoolDrain to Capacitor.Cli.Core so
the daemon can share them without referencing the CLI's exe project.
Resolves three ordering hazards surfaced by this wiring (each covered by a
new test): the ordered drain's withheld temp files now use a distinct
".ordered-*" namespace so Claude/Cursor's unrelated FIFO drain can never
cross-consume them (BLOCKER-1); the generic drain now fires the
generate_whats_done side effect on any vendor's session-end, not just
Claude's own poster (BLOCKER-2); and a session whose session-end was already
delivered is durably marked ended so a later straggler entry is dropped
instead of replayed out of order (BLOCKER-3).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Claude ordering guard must see the .ordered-* backlog namespace
AI-1357 Task 12 review fix. ClaudeHookCommand.CurrentSessionHasBacklog was a
stale private duplicate that only checked {sid}.jsonl + {sid}.*.draining, not
the .ordered-* namespace this task introduced. Since the centralized ordered
drain now runs on every non-Codex invocation (incl. --claude), a Claude
session-end withheld in .ordered-* pending the transcript tail was invisible
to the guard, so a later Claude subagent-stop for the same session posted
directly — ahead of the still-withheld session-end (the exact BLOCKER-1/3
cross-spool ordering violation).
Delegate CurrentSessionHasBacklog to the public HookSpool.HasBacklog (which
covers all three namespaces), matching CursorHookCommand. Add a test proving a
subagent-stop spools behind a session-end withheld in .ordered-*.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: AI-1357 CLI acceptance tests (spawn-before-post, drain order, heartbeat)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: add import origin marker constant (CLI)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: shared guarded discovery helper (A4)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: adopt guarded discovery in all import sources (A4)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: restore flat discovery scope for Kiro/Gemini (A4 fix)
GuardedDiscovery.EnumerateFiles gains a recursive flag (default true); the
two originally-TopDirectoryOnly call sites pass recursive:false so A4 adds
symlink/cycle/inaccessible/per-entry safety without widening discovery scope.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: OpenCode ended_at null/0 not 1970 (A3)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: per-vendor ended_at resolvers (A3)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: confirm/extend Codex ExtractLastTimestamp shape (A3, open Q2)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Claude/Codex resume end-only reassert + fail-closed tail (A2)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: routed sources send historical-import origin marker (A1)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Cursor import no stale PR + per-cwd repo cache (item 5)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Antigravity import usage pass, USAGE-before-end, all classifications (item 7)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 7: CLI phase-0 append-only harness + runtime rewrite guard + quarantine (D0)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 8: CLI side-effect barrier + hook heartbeat (D1)
Adds CursorMarkers.CreateBarrier/BarrierPending/ClearBarrier (thin wrappers
over WatcherHeartbeat's atomic timestamp read/write) and TouchHeartbeat.
CursorHookCommand.HandleCore now touches the per-session heartbeat on every
invocation carrying a session id (including telemetry-only hooks), creates
the barrier before beforeSubmitPrompt's own POST (clearing it on a 2xx from
either that live POST or a later hook-spool drain delivery of the same
spooled entry), and relies on the existing spool-drain-before-transcript-
drain ordering for sessionEnd (already correct — no reorder needed).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 9: CLI per-session Cursor watcher spawn, precedence-ordered (D1)
Adds "cursor" to WatchCommand.KnownVendors and the watch --vendor usage
string. CursorHookCommand gains the pure ShouldSpawnWatcher(eventName,
isSubagentChild) precedence predicate (terminal hooks never spawn;
a correlated subagent child never spawns a top-level watcher — routed via
the gated parent-child key in a later task) and MaybeSpawnWatcherAsync,
which additionally gates on the quarantine marker and a non-empty
transcript path before calling WatcherManager.EnsureWatcherRunning(vendor:
"cursor"). Wired into HandleCore: sessionStart spawns before its POST
(reusing the workspace_roots-derived cwd already computed for repository
enrichment); every other non-terminal hook spawns only after its own
lifecycle POST has succeeded (recovery spawn).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 10: CLI hook-backfill hardening — shared reader, Hold/ConsumeIfComplete, barrier+quarantine-aware (D2)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 11: CLI watcher exit conditions + acked-batch cursor (D1/D3)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 12: CLI child subagent watchers gated on acked subagent-start
HandleSubagentChildEventAsync now spawns the child (subagent) Cursor
watcher — key `{parentSessionId}-{childSessionId}`, tailing the child's
own transcript file, agentId=childSessionId/sessionIdOverride=parentSessionId
— only once the diverted subagent-start POST is acknowledged (2xx).
A spooled start (POST failure) defers the spawn entirely: no code path
spawns a child watcher for an unacked start, preserving the invariant
that no child transcript line reaches the server before SubagentStarted
is appended.
The deferred half is wired into HandleCore's generic top-of-method spool
drain (which runs before the isSubagentChild divert, keyed on the same
childSessionId): when it redelivers a previously-spooled subagent-start
entry and gets a 2xx, it parses the parent/child/transcript-path triple
back out of the entry's own payload and performs the spawn then — the
"later invocation whose spool drain delivers the start" the design calls
for. Quarantine is checked on the parent session id, matching how
CursorRewriteGuard/WatchCommand.RunWatch resolve their own guard identity
for a child watcher process (sessionId = sessionIdOverride ?? key).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Task 13: end-to-end acceptance tests + docs (CLI half)
Adds CursorTailingWatcherTests.cs covering the plan's four acceptance
scenarios from the CLI side: force-quit mid-turn (a transcript's final
line, complete but not yet newline-terminated, is held by every live
drain and only consumed by the shutdown final drain); idle-ceiling exit
without Cursor synthesizing its own session-end (extracts the pure
CursorSuppressesEndPost(vendor, idleExit) helper out of RunWatch's
inline check so the suppression decision is independently testable,
contrasted against Codex which does NOT suppress); and reactivation via
BOTH a sessionStart resume and a non-sessionStart resume hook, driven
through the real CursorHookCommand.HandleCore dispatcher rather than
the bare ShouldSpawnWatcher predicate. "Sweep closes" is server-side
(already proven by Task 5's StaleActiveSessionReaperTests) and is out
of this repo's visibility — noted in the test file's doc comment rather
than re-derived.
Also documents the watcher-backed Cursor capture (hooks retained as
belt-and-braces, child-watcher acked-start gating, force-quit/idle-
ceiling/reactivation behavior, the runtime rewrite guard) and the
KCAP_CURSOR_IDLE_CEILING_MINUTES knob in README.md, plus
cursor-verify-appendonly as the phase-0 diagnostic.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix: rewrite-guard shrink/full-prefix wiring, acked-byte checkpoint, heartbeat-aware idle ceiling, delivery-boundary re-checks
Addresses findings #2, #3, #6, and the watcher-side half of #8/#1 from
kcap-cli PR #324 review:
- #2: CursorRewriteGuard.VerifyFullPrefix existed but was never called from
the poll loop; wire it in on a periodic cadence (CursorFullPrefixVerifyEveryNPolls).
Also add VerifyNotShrunk — the guard's zone checks were previously gated
entirely behind "did the file grow", so a shrink or in-place same-length
rewrite slipped through undetected.
- #3: checkpoint the Cursor byte cursor using the SAME capped snapshot length
ReadNewCompleteLinesAsync sampled (NewTranscriptLines.SnapshotByteLength),
not a fresh FileInfo.Length re-sample racing a concurrent append. Advance
the checkpoint only as far as the server's acked LINE count actually
covers (ByteOffsetForAckedLines), not the full capped range, so a
partially-disposed D3 batch never has its unacked tail checkpointed as
delivered.
- #6: the Cursor idle clock (ShouldEndOnIdle) is now the later of transcript
activity and the hook heartbeat mtime (ResolveCursorIdleClock), and child
(subagent) watchers are idle-ceiling eligible without requiring
ThresholdReached, which they never set.
- #8 (watcher half): re-check the quarantine/barrier markers immediately
before SendTranscriptBatchAcked, not only at the top of the poll.
- #1 (backfill half): CursorTranscriptBackfill re-checks quarantine/barrier
immediately before the POST, closing the same race window on that path.
DrainNewLines is now internal (was private) so the guard wiring is directly
regression-testable without a live SignalR server — every path exercised
trips before ever touching the HubConnection argument.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix: recovery spawn withheld on spool backlog; dropped subagent-start permanently gates child transcript delivery
Addresses findings #4 and #5 from kcap-cli PR #324 review:
- #4: capture whether the session still has spool backlog AFTER the
generic top-of-method drain attempt, and require no remaining backlog
before the recovery-spawn watcher call — a telemetry-only mapping
(SpoolOnFailure=false) previously reached the spawn regardless of
whether an earlier canonical event (e.g. sessionStart) was still stuck
undelivered.
- #5: add a durable per-child subagent-start-acknowledgement marker
(CursorMarkers.MarkSubagentStartAcked/HasSubagentStartAck), written the
moment a 2xx is observed (live POST or a later spool-drain delivery).
HandleSubagentChildEventAsync now gates ALL non-start hooks (content-less
backfill and the child's own subagent-stop) on this marker instead of on
"no spool backlog" — a subagent-start that hits a non-transient 4xx on
retry is permanently Dropped from the spool (HasBacklog goes false) even
though no AgentSubsession stream was ever opened server-side; without the
marker that emptied backlog let child transcript content flow ungated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix: spool-replay enforces Cursor quarantine; historical import skips quarantined sessions (incl. correlated children)
Addresses the replay half of finding #1 and finding #7 from kcap-cli PR #324
review:
- #1 (replay half): LifecycleSpoolDrain's transcript poster now parses the
batch's vendor/session_id and drops (permanently discards) a Cursor batch
whose session is quarantined, and treats a pending side-effect barrier as
transient (retry later). This is the only delivery-time check the
shutdown-spool-replay path has — a batch queued before a runtime
rewrite-guard trip could otherwise still be replayed later.
- #7: CursorImportSource.ClassifyAsync now skips (ProbeError) any session
whose quarantine IDENTITY is marked — resolved via the already-computed
subagentLinks map so a correlated child is filtered under its PARENT's
quarantine marker (CursorRewriteGuard is always keyed on the family/parent
id for a spawned child watcher), not its own id. Previously `kcap import`
had no awareness of the marker at all and could feed the exact corrupted
line-number source the guard exists to shut off.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r2): Cursor top-level watcher skips the below-threshold buffer
Cursor's own sessionStart hook posts (and spawns this very watcher) before any
transcript line is ever read, exactly like Antigravity's pre-spawn POST — so the
generic 10-line below-threshold buffer must not apply either. Before this fix a
top-level Cursor watcher re-added its still-unread lines to BufferedLines every
poll (the line cursor never advances while buffering) until they eventually
flushed as duplicates, and a watcher that force-quit before crossing the
artificial threshold skipped its final drain and shutdown spool, and was
permanently ineligible for the Cursor idle ceiling. Extracted into a pure,
testable SkipsThresholdBuffering predicate; child watchers are unaffected
(they never buffer regardless).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r2): bind the Cursor rewrite guard to the bytes it actually sends
Three related TOCTOU-closing fixes to the D0 rewrite guard, all stemming from the
same theme: the guard's hash must be bound to the exact bytes actually
processed/sent, and its baseline must be real from the start.
1. Bind-to-sent-bytes (finding #1): ReadNewCompleteLinesAsync now optionally
(captureRawBytes: true, wired for vendor=="cursor") captures the raw byte
buffer of the SAME capped read that decodes the batch's lines
(NewTranscriptLines.SnapshotBytes). DrainNewLines's guard block now hashes
directly from that snapshot instead of reopening the file separately to
record the new-range/prior-zone hashes — closing the window where a rewrite
landing between the decode read and the old reopen produced a hash for bytes
the batch never actually came from. The post-ack checkpoint's trailing hash
is now derived from the same already-verified snapshot instead of reopening
the file after the RPC returns (a rewrite in flight during the ack could
otherwise be blessed as the new baseline).
2. Reconnect rewind atomicity (finding #2): a reconnect discovering the server
is behind the client now rewinds state.CursorByteOffset and the guard's
checkpoint ATOMICALLY with the line cursor, via the extracted
ApplyReconnectRewindAsync (testable without a live SignalR reconnect). The
true byte offset of the rewound line is resolved by scanning the transcript
(ResolveByteOffsetForLineAsync) rather than leaving the byte checkpoint at
the later, too-far-ahead offset (which left the replayed line gap's
new-range verification starting past the bytes it actually occupies).
3. Real full-prefix baseline (finding #3): the periodic full-prefix re-hash now
seeds on the REAL first poll (not just lazily on the guard's own first
VerifyFullPrefix call, which previously never happened before poll N) — a
same-length rewrite of an already-checkpointed middle region landing in
polls 1..N-1 now has a real baseline to be caught against at poll N, instead
of poll N seeding the already-rewritten file as if it were the original.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r2): later nonterminal child hooks self-heal a dead watcher
Once a child's subagent-start is durably acked, every LATER NONTERMINAL Cursor
hook now also attempts to (re)spawn its child watcher, not just the child's own
sessionStart. Before this fix, only sessionStart ever called
MaybeSpawnChildWatcherAsync, so a child watcher that later exited (the
newly-enabled idle ceiling), crashed, or never actually spawned (e.g. its acked
sessionStart carried no transcript path) was never restarted. Retains the
terminal no-spawn rule for sessionEnd. EnsureWatcherRunning is idempotent
(PID+heartbeat check), so this is a cheap no-op once the watcher is alive.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r2): historical import re-checks quarantine at delivery, resolves family via the live marker
1. Delivery-boundary quarantine check (finding #6): ImportSessionAsync now
re-checks CursorMarkers.IsQuarantined FRESH, before ANY lifecycle/transcript
delivery — previously it had no quarantine check at all, so a session
quarantined by the live watcher's runtime rewrite guard AFTER
ClassifyAsync (during repo probing, an interactive confirmation prompt, or
simply queueing behind other sessions in the same import run) would still
post session-start, every corrupted line, children, and session-end. A
second check right before the transcript boundary catches a guard trip
during the sessionStart POST itself: no transcript content is sent, but the
already-server-side-created session is still best-effort closed with
session-end so it doesn't hang open forever.
2. Family-identity fallback via the live marker (finding #7): subagentLinks is
only ever computed from the sessions ONE import batch discovered — a
`--session <child>` filter (or an inaccessible/omitted parent transcript)
excludes the parent entirely, so the in-batch correlator can't produce the
family link, and the CHILD's own (unquarantined) id was checked instead of
its family's. ResolveQuarantineIdentity now falls back to the persisted
CursorLiveSubagentLinker marker (written independently by the live hook
dispatcher), resolved once at classify time and stamped onto SourceMeta as
QuarantineIdentity so ImportSessionAsync's fresh re-check (above) uses the
same family identity.
Regression test note: several existing tests in CursorImportSourceTests share
hardcoded session ids across ClassifyAsync/ImportSessionAsync calls, backed by
CursorMarkers' real (non-injectable) on-disk quarantine marker; ImportSessionAsync
now also reading that marker widened a latent parallel-execution race, so the
class is marked [NotInParallel] (mirrors the existing MachineIdFileTests pattern).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r3): serialize reconnect rewind with drain, seed initial-resume byte offset, bound rewrite-guard capture to the new range
Three P1 follow-on findings from the round-2 guard-byte-binding rework (db65d93):
1. WatchCommand.RunWatch's Reconnected handler and main polling loop both mutated
WatchState/CursorRewriteGuard once the hub reconnected, with no synchronization between
them — a reconnect rewind's three writes (byte offset, checkpoint reset, line cursor)
could interleave with a concurrently-running drain, recreating the byte/line-frontier
divergence the r2 fix was meant to remove. Serialized both under a single
SemaphoreSlim(1, 1) (cursorRewindGate, Cursor-only) via two new directly-testable helpers,
GatedApplyReconnectRewindAsync/GatedDrainNewLinesAsync.
2. The INITIAL WatcherConnect registration only ever assigned state.LinesProcessed on
resume; CursorByteOffset stayed at its default (0), so a watcher resuming at server line N
mapped acked-line counts relative to N but their bytes from 0 — a permanent, silent
line/byte-frontier misalignment. Extracted the reconnect-rewind's own byte-seeding logic
into a shared SeedCursorByteOffsetAsync helper, called from both the reconnect path and
the initial registration.
3. The Cursor watcher's captureRawBytes read materialized a buffer the size of the WHOLE
file on every poll, including idle one-second polls with nothing new — unbounded LOH churn
for large transcripts. ReadNewCompleteLinesAsync now accepts rawBytesReadFrom/
newRangeByteOffset so DrainNewLines can request a BOUNDED read (the guard's own small
trailing-tail zone plus whatever's actually new) on every poll except the rare periodic
full-prefix cadence, which still needs — and gets — the whole file. CursorRewriteGuard's
HashPriorZone gained an explicit snapshot-start-offset parameter so its window clips
correctly against a non-zero-based buffer.
Regression tests: gate-composition tests proving a drain/rewind cannot observe a half-applied
counterpart while the gate is held; SeedCursorByteOffsetAsync unit + composition tests proving
a full ack of M resumed lines checkpoints at N+M, not M; bounded-capture tests proving the
buffer is sized to the new range (not file length), an idle poll allocates ~0 bytes, and a
large-file rewrite is still caught via the bounded path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r3): re-check quarantine during multi-batch import delivery
SessionImporter.SendTranscriptBatches streamed the mutable transcript file and posted one
request per 100 lines with no quarantine check at all inside the loop — the round-2 fix
only re-checked immediately before calling it. A quarantine written by the live watcher's
runtime rewrite guard AFTER the first transcript POST still let every remaining batch post.
SendTranscriptBatches now accepts an optional abortDelivery predicate, checked immediately
before every batch POST (including the first); a trip throws the new
TranscriptDeliveryAbortedException without posting the pending batch. CursorImportSource
threads its already-resolved quarantineIdentity into both the parent's own transcript send
and each subagent child's, so a quarantine trip aborts remaining batches for either — no
extra correlator work per batch. The parent's catch block reacts to the abort the same way
as the two existing quarantine boundary checks: best-effort session-end so the session
doesn't hang open "active" forever, then Failed so a re-run hits the pre-flight check and
cleanly Skips from then on.
Regression test: a 150-line transcript (two 100/50-line batches) with the quarantine marker
written the instant the first batch lands proves the second batch is never posted.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r4): keep byte/line frontier in lockstep across polls, cadence, and resume
Finding #1 - a poll that consumed only blank/whitespace lines advanced
state.LinesProcessed without advancing state.CursorByteOffset (or the
rewrite guard's checkpoint) to match. The next poll's bounded decoder then
re-read the same already-"processed" blank bytes but seeded its line
numbering from the already-advanced LinesProcessed, inflating the line
count on every idle poll and mis-mapping the next real batch's ack byte
frontier. DrainNewLines' "no content, no repo change" branch now maps the
same blank-line count to its true byte offset (ByteOffsetForAckedLines,
mirroring the ack path) and re-checkpoints the guard, so a truly idle
follow-up poll decodes nothing new.
Finding #4 - CursorGuardPollCount (the periodic full-prefix cadence
counter) incremented unconditionally before the guarded file was even
opened. A transient IOException on that read still consumed the cadence
slot, so the next successful poll silently skipped its due full-prefix
baseline. The cadence decision is now peeked without mutating state, and
only committed once the guarded read actually completes without throwing.
Finding #5 - ResolveByteOffsetForLineAsync silently clamped to EOF when
the server's acknowledged resume line exceeded the local transcript's
line count (a transcript truncated/replaced while the watcher was
offline), and SeedCursorByteOffsetAsync/ApplyReconnectRewindAsync treated
that clamp as a valid baseline — seeding the rewrite guard against the
wrong (truncated) file while the line cursor advanced past it. Both
methods now return a typed "could not resolve exactly" result; the seed
path quarantines the session instead of seeding a bogus baseline, and
both the initial WatcherConnect resume and reconnect-rewind call sites
exit (cts.Cancel()) rather than proceed on an unresolved rewind.
Regression tests (CursorGuardWiringTests, CursorReconnectRewindTests): a
blank-only poll advances both frontiers together and a subsequent
genuinely-idle poll re-decodes nothing; a guarded read that throws
IOException never consumes the full-prefix cadence; a resume/reconnect
frontier beyond the local transcript's line count quarantines instead of
seeding a clamped baseline.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r4): propagate delivery-abort quarantine through the historical import's close-and-fail path
Finding #2 - SendSubagentLifecycleAsync's catch-all swallowed the typed
TranscriptDeliveryAbortedException into a bare `false`, so a quarantine
tripped during a CHILD's own transcript delivery made ImportSessionAsync
return Failed WITHOUT ever posting the parent's best-effort session-end -
even though the child's subagent-start had already landed, leaving the
parent/subsession stuck Active forever (the quarantine marker makes the
next run Skip at preflight instead of repairing it). The child lifecycle
helper now (a) checks quarantine BEFORE posting a new child's
subagent-start, so a family found quarantined never starts another child,
and (b) rethrows TranscriptDeliveryAbortedException instead of catching
it, so the parent's child loop (now wrapped the same way the parent's own
transcript delivery already was) routes it through the shared
CloseAndFailAsync best-effort session-end + Failed contract.
Finding #3 - SendTranscriptBatches only re-checked abortDelivery BEFORE
each batch POST. A transcript that fits in a single (or final) batch -
including every transcript of <=100 lines - has no "next" batch to gate,
so a quarantine marker written while that one-and-only POST was in flight
was never observed anywhere; the method returned normally and the caller
proceeded into child lifecycle / normal completion. abortDelivery is now
also re-checked immediately AFTER every POST (mid-loop and the trailing
batch), closing that window.
Regression tests: a quarantine trip during a child's own transcript
delivery closes the parent via best-effort session-end + Failed; a later
child is never started once the family is found quarantined (even when
an earlier child's own delivery was clean); a quarantine marker appearing
during a single-batch (<=100 line) transcript's only POST is observed and
routed through close-and-fail, at both the SessionImporter.
SendTranscriptBatches level and the CursorImportSource.ImportSessionAsync
level.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r4): lockstep byte frontier in the repo-only-failed drain branch too
The "repo-only batch failed" catch branch in DrainNewLines advanced
state.LinesProcessed past blank/whitespace-only lines without advancing
state.CursorByteOffset (or the rewrite guard's checkpoint) to match -
the same byte/line-frontier drift finding #1 fixed for the "no content,
no repo change" early return, on the other path that moves the line
cursor without an acked send. This branch is reached only when a repo
change was pending (repoToSend != null) and the repo-only RPC threw with
newLines.Count == 0 (any lines read were blank); left unfixed, the next
poll's bounded decoder re-read those same blank bytes under an inflated
line number.
Extracted the finding-#1 lockstep logic into a shared local function
(AdvanceCursorBlankByteFrontierInLockstep) so both call sites - the early
return and this catch branch - map the consumed blank-line count to its
true byte offset via ByteOffsetForAckedLines (fed from the already-captured
cursorGuardSnapshot) and re-checkpoint the guard byte-for-byte identically.
Regression test: a blank-only poll with a pending repo change whose
repo-only send fails advances byte AND line frontier together, and a
subsequent idle poll (repo change still pending, still failing) re-decodes
nothing / does not inflate the line number.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r5): resolve a complete unterminated final line at EOF instead of quarantining
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Review fix (r6): rewind unterminated-final-line resume so an appended terminator keeps line numbering
The r5 fix seeded CursorByteOffset at EOF while leaving LinesProcessed at
the already-acked final record's own line count. When Cursor later appended
that record's terminating newline before its next record (Cursor's normal
write order), the bounded reader started reading exactly at that EOF and
misread the leading '\n' as closing a phantom empty line — because it seeds
its own line index from LinesProcessed, already past that record. Every
following line then landed one number too high, permanently: the server,
still waiting at the true frontier, saw a persistent gap while the watcher
kept resending from the stale offset.
ResolveByteOffsetForLineAsync now returns a (ByteOffset, LineNumber) pair;
the complete-unterminated-final-record case rewinds to the record's own
start paired with LineNumber - 1 instead of EOF paired with LineNumber
unchanged, so the record is re-read/re-sent next poll — harmless, since
Cursor's normalizer emits deterministic event ids and the server's
source-ack frontier dedupes a resend at/behind it. SeedCursorByteOffsetAsync
now assigns both halves of the pair together (and, for every vendor, owns
WatchState.LinesProcessed directly rather than leaving it to two separate
call sites), keeping the byte/line frontier in lockstep from one source of
truth.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Qodo fix: make CursorMarkers.Quarantine fail-open on write failure
Quarantine() did unguarded filesystem writes/moves with no exception handling,
but it's called from CursorRewriteGuard.Reject deep in the watcher's drain
loop, which has no broad exception handler above it (DrainNewLines only
catches IOException/OperationCanceledException). A non-IOException failure
(e.g. UnauthorizedAccessException) would escape and crash the watcher instead
of letting the caller's Verify* return value cleanly stop delivery and exit.
Wraps the write in a try/catch, logs to stderr, and never throws — matching
IsQuarantined/ReadMarker's existing fail-open contract. Also trims the
class-level doc comment per repo comment-verbosity guidelines.
Adds a regression test that occupies the marker's own file path with a
directory (so the final rename fails) and asserts Quarantine doesn't throw.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1382] Qodo fix: trim over-verbose Cursor watcher comments
CursorRewriteGuard's class doc and a few method docs, plus one
CursorHookCommand block, ran to multi-paragraph design-doc-style prose.
Condenses them to the essential invariant/precondition, keeping the subtle
correctness notes (TOCTOU binding, checkpoint-reset rationale) but dropping
narrative. Also strips AI-#### Linear identifiers from code comments per repo
guideline (CLAUDE.md: "DO NOT use Linear issue numbers in comments").
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
alexeyzimarev
pushed a commit
that referenced
this pull request
Jul 18, 2026
…CLI) (#326) * Fix Kiro parent-PID watchdog name match (kiro-cli) Adds a bounded `-cli` suffix tolerance to MatchesAgentName so the by-name ancestry walk identifies the durable `kiro-cli` process for vendor `kiro`, instead of falling back to the fragile getpgrp/getppid heuristic. Refs AI-1359. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Freeze the watcher idle clock while disconnected Subtracts SignalR-outage time (accrued since last activity) from the idle measure so a transient disconnect can't false-idle-end a Codex/Antigravity session, while repeated reconnects with no new lines still idle-end after the connected budget. Refs AI-1359. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add staged parent-dead / wedged-watcher recovery On ParentAlreadyDead the watcher now periodically re-resolves + re-arms the parent-exit watchdog (using the vendor process-name alias); it ends the session only after a long, configurable ceiling with no transcript progress and continued resolution failure, and any new progress resets the window. This is the only end path for a wedged, alive-but-connected watcher that the server stale sweep can't see. New env var KCAP_PARENT_DEAD_CEILING_MINUTES. Refs AI-1359. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: AgentHookPoster.Spooled + PostOrSpoolAsync spool-on-lapse Adds a Spooled outcome and PostOrSpoolAsync: on lapsed-auth or transient (5xx/408/429/unreachable) failure the lifecycle payload is durably spooled (HookSpool) for a later drain pass, so live capture can start regardless of hook-POST delivery (spawn-before-post). Refs AI-1357. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: TranscriptSpool — bounded, no-drop, needs-import marker AI-1357 Task 2: dedicated on-disk spool for undelivered transcript-tail batches captured during a watcher outage. Unlike HookSpool (1 MB, drop-oldest — fine for small lifecycle POSTs), this is bounded at 8 MB per session with NO SILENT DROP: on cap exhaustion it stops appending and writes a needs-import marker instead of truncating history, so the session surfaces as requiring `kcap import` rather than silently losing transcript content. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: TranscriptSpool no silent drop on I/O failure + Ignored result Review fixes for AI-1357 Task 2: - Append now marks needs-import (not phantom Appended) when the live write throws — the silent-drop this class exists to prevent. - MarkNeedsImport returns bool and logs to stderr on failure so callers don't trust an unpersisted marker. - Add AppendResult.Ignored for the malformed-session-id drop so it can't be mistaken for a real append. - Tests for the I/O-failure path and the malformed-id path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: LifecycleSpoolDrain — global cross-spool ordered drain pass Adds a session-agnostic drain pass to run at the start of every kcap invocation (and periodically by the daemon), enforcing per-session ordering across the lifecycle (HookSpool) and transcript (TranscriptSpool) spools: spooled session-start -> transcript tail (+ needs-import marker, delivered even over cap) -> spooled session-end. Adds the route-filtered HookSpool.DrainRoutesAsync/SessionIdsWithBacklog and TranscriptSpool.SessionIdsWithBacklog helpers this needs; HookSpool's existing route-agnostic DrainAllAsync is untouched (still used by Claude/Cursor). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: spawn-before-post + Spooled for Kiro/OpenCode/Pi/Copilot hooks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: throttle spool drain per-prompt; ShouldSpawnAfter drops AuthLapsed AI-1357 review: (1) DrainSpoolsAsync now self-throttles via an on-disk .last-drain stamp (30s) so Kiro agentSpawn / OpenCode idle re-fires can't attempt a network drain every prompt during an outage; reaps moved inside. (2) ShouldSpawnAfter spawns only on Posted/Spooled — AuthLapsed spools nothing, so spawning would orphan a session with a dropped SessionStarted. (3) Documented the fresh-client deviation (no reusable vendor client exists at the pre-POST drain point). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Codex spawn-before-post with stdout-first carve-out Auth lapse now spools + spawns via PostOrSpoolAsync/ShouldSpawnAfter (matching Kiro/OpenCode/Pi/Copilot) instead of dropping the session and skipping the watcher. The global lifecycle/transcript spool drain runs fire-and-forget AFTER Codex's blocking `{"continue":true}` stdout handshake — never before, never gating it — so a large/unreachable spool backlog can't stall the parent process. Extracted WriteSessionScopedOutput and RunSessionStartHandshakeForTest as the seam CodexStdoutContractTests uses to prove the ordering without a live server. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Antigravity spawn-before-post + Gemini awaited EnsureWatcherRunning Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Gemini session-start spawn-before-post + gated spool drain Migrate Gemini's session-start off the POST-only PostAsync path to AgentHookPoster.PostOrSpoolAsync + ShouldSpawnAfter, so an auth lapse or transient outage spools the SessionStarted payload and STILL spawns the watcher (was: returned without reaching EnsureWatcherRunning on anything but Posted). Wire the throttled DrainSpoolsAsync gated to the two lifecycle events (SessionStart/SessionEnd) so the per-turn Notification path adds no network cost. Fix a dangling "PostHookAsync (below)" comment in AntigravityHookCommand left by the Task 6 deletion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: shutdown final-line completion signal (newline/parseable-JSON) The shutdown-during-outage final drain (idle-timeout / parent-exit) used to disable the half-written-line holdback unconditionally, which could consume a line the agent was still mid-write on, or drop a complete newline-less final line. Add a pure IsFinalLineComplete signal (empty / newline-terminated / last line parses as JSON — length-stability alone is NOT proof, since a large write can pause mid-record past any bounded window) plus a bounded (<=2s) WaitForFinalLineCompletionAsync wait in the final-drain path. Complete -> send the newline-less final line as before; incomplete -> keep the holdback on (never send-and-advance a truncated line) and flag the session needs-import via TranscriptSpool so `kcap import` can recover it later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: close final-drain completeness TOCTOU; re-validate at consume time Review found the completeness decision and the consuming read were two separate reads: WaitForFinalLineCompletionAsync probed once, then DrainNewLines consumed with holdback disabled — so a final line that resumed growing in the gap was sent-and-advanced anyway, violating "never consume a still-growing line." Move the decision into the consuming read via an IncompleteFinalLinePolicy (Hold / ConsumeIfComplete). Under ConsumeIfComplete the unterminated final line is consumed ONLY IF the exact bytes read parse as a complete JSON record; otherwise it is held and NewTranscriptLines.HeldIncompleteFinalLine is set. The parse check runs on the same bytes being consumed, so there is no TOCTOU with the bounded pre-wait (now purely advisory — gives the writer time). RunWatch flags needs-import off the actual consume-time held result via state.FinalDrainHeldIncompleteLine. Also updates the stale ApplyPartialLineHoldback comment that claimed the final drain opts out of holdback because "the file is static then" — the exact wrong assumption this task fixes. Tests: SplitNewCompleteLines + ReadNewCompleteLinesAsync gain parseable- consume / unparseable-held cases, HeldIncompleteFinalLine parity, and a grew-after-the-completeness-check TOCTOU guard asserting the resumed partial is held, not sent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: spool undelivered transcript tail on shutdown-during-outage At final drain during idle-timeout/parent-exit shutdown, DrainNewLines only advances LinesProcessed past lines the hub confirmed sent. If the hub is down (or still reconnecting) at that point, any transcript lines from LinesProcessed to EOF were never delivered and the process exits right after — silently dropping the tail. Adds BuildTranscriptSpoolBatch (pure TranscriptBatch JSON builder) and SpoolUndeliveredTranscriptTailAsync, which re-reads the undelivered tail using the same ConsumeIfComplete completion decision as the final drain and spools it into the dedicated TranscriptSpool (task 2) so the global drain (task 3) replays it after recovery, without a manual `kcap import`. Cap exhaustion still marks the session needs-import rather than dropping. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: spool tail unconditionally + redact secrets (task-8 review) Two Critical review findings on the shutdown-during-outage tail spool: 1. Insufficient trigger → silent drop. Gating the spool on hubConnection.State != Connected missed a HubException thrown by SendTranscriptBatch2 while the connection stayed Connected (the generic catch does not change connection state) — the tail was undelivered but never spooled. Call SpoolUndeliveredTranscriptTailAsync unconditionally on the shutdown path; it is already a no-op when position == EOF. 2. SECURITY: spooled tail skipped secret redaction. The live/inline drains run lines through SecretRedactor.RedactLine, but the spooled tail was written raw → secrets on disk and POSTed unredacted on replay. Redact each spooled line exactly as the live drain does before writing. Tests: undelivered_tail_spooled_even_though_connection_stayed_up (state- agnostic spooling) and spooled_tail_is_secret_redacted (ghp_ token → [REDACTED], raw secret absent from disk). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: watcher heartbeat + lock-guarded staleness reap+respawn AI-1357 task 9. WatcherManager.IsWatcherAlive only checked the PID, so a wedged (hung-but-alive) watcher was never restarted. The watcher now touches a per-key heartbeat file every main-loop iteration — including no-content drains and while disconnected/reconnecting — via the pure WatcherHeartbeat helper (Core). IsWatcherAlive requires both a live PID and a non-stale heartbeat (past a 30s startup grace, 20s threshold). EnsureWatcherRunning reaps a wedged watcher (kill + respawn) under a cross-platform spawn lock — same FileShare.None/flock primitive as DaemonLock — so concurrent hooks racing the same key can't double-spawn. The whole decide-and-spawn sequence is locked, not just the reap: killing the old watcher deletes its pid file before the respawn's new one lands, so guarding only the reap step would leave a window where a second hook sees "no pid" and spawns unguarded (caught by the new concurrency test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: keep heartbeat fresh during connect-retry + purge watcher sidecar files AI-1357 task 9 review. Two Important issues. 1. Startup/reconnect outage looked "wedged" → false reap. The SignalR connect-retry loop never touched the heartbeat, so a server outage >=~50s at startup was indistinguishable from a wedged loop and the hook probe would repeatedly reap+respawn a healthy-but-reconnecting watcher. RunWatch now touches the heartbeat at the top of every connect-retry iteration and waits via DelayWithHeartbeatAsync, which chunks the backoff (grows to 30s > the 20s threshold) into <=5s slices through the new pure HeartbeatSlices helper, touching before each — so no wait window can ever cross the staleness threshold. The automatic- reconnect path was already covered by the main loop's per-iteration touch in the disconnected branch. 2. Sidecar file leak. Only .pid was deleted. KillWatcher now removes the {key}.heartbeat and {key}.started markers, but deliberately NOT the {key}.spawnlock: KillWatcher runs from inside WithSpawnLock on the reap path, and unlinking a held lock file on POSIX lets a racing hook open a fresh non-conflicting flock (the DaemonLock unlink-race). Spawn locks are swept by the new WatcherManager.PurgeAuxiliaryFiles(), called by kcap cleanup (holds no lock; also mops up orphans). CleanupCommand now uses GetWatcherDir() so it honors KCAP_WATCHER_DIR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Kiro live usage-backfill synthetic lines (credits/context%) AI-1357 task 10: the live kcap watch path never emitted Kiro's per-turn credits/context% (the AI-1196 hedge only fires on import, which reads the sibling {id}.json up front). A live drain has usually already sent the turn's anchor AssistantMessage line by the time Kiro flushes that sidecar, so inline enrichment can't reach it there. Add a synthetic KiroUsageBackfilled line per turn anchor instead, mirroring the Antigravity synthetic-USAGE-line pattern: WatchCommand. BuildKiroUsageBackfillLine builds the JSONL; AppendKiroUsageBackfillLines reads the sidecar via KiroUsage.AnchorMap and appends one line per anchor not yet in the new WatchState.KiroUsageEmittedAnchors, staging them on KiroUsagePendingAnchors for the caller to commit only after a successful send (so a failed batch re-reads and re-stages instead of losing them). Server-side event fold is a separate follow-up (task 13). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: Cursor live subagent split via hook/backfill path (AI-1151) CursorSubagentCorrelator only ran on the historical import path, so a live Cursor Task/Agent subagent was ingested as its own top-level session instead of nesting under its parent. Cursor is not watcher-backed (CursorHookCommand backfills each session's transcript over HTTP as hooks arrive), so the correlation now runs inline in that per-hook dispatcher via a new thin wrapper, CursorLiveSubagentLinker. At a child's sessionStart, ResolveParent (reusing CursorSubagentCorrelator.Correlate) checks the sibling transcripts under the same agent-transcripts workspace dir; a match is persisted to a small on-disk marker (the CLI is a fresh process per hook) and diverts subsequent hooks for that session: subagent-start replaces the top-level sessionStart, transcript backfill is routed under the parent with agent_id=child (mirrors CursorImportSource.SendSubagentLifecycleAsync's watermark + POST shape), and subagent-stop replaces sessionEnd. Mid-lifecycle hooks (beforeSubmitPrompt/afterAgentThought/telemetry) are not forwarded for a linked child, matching the import path (which has no side channel for them either) instead of writing to a phantom AgentSession stream that never got a SessionStarted. Dashless ids are used throughout so a live-then-import of the same session converges on the same AgentSubsession-{parent}-{child} stream rather than duplicating it (ties to AI-1358 A1). Known eventual-consistency gap (documented in code): if the parent's Task tool_use isn't yet flushed to disk at the child's first hook, the child is temporarily ingested top-level until a later `kcap import --cursor` converges it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: enforce start-before-stop spool ordering on Cursor live subagent path (AI-1151) HandleSubagentChildEventAsync posted subagent-stop (and ran the sessionStart backfill) with no spool.HasBacklog check, unlike the sibling top-level path. Under a transient failure that left a subagent-start undelivered in the spool, a later subagent-stop could be POSTed ahead of its own subagent-start, leaving the AgentSubsession stream mis-ordered / never opened. Mirror the top-level guard: after the HandleCore drain, if the child still has spool backlog (drain hit a transient failure), spool the fresh lifecycle event behind it and skip the agent-routed backfill so the next hook re-drains start-first. Also gate the sessionStart backfill on the subagent-start POST succeeding, so the transcript can't be routed to an AgentSubsession the now-spooled start hasn't opened yet. Adds a regression test: a child whose subagent-start is spooled (transient failure) does not get subagent-stop delivered ahead of it, and a later recovered drain delivers start before stop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat: wire global spool-drain pass into hook entry + daemon-periodic AI-1357 Task 12. Runs LifecycleSpoolDrain centrally in Program.cs's `case "hook":` (before dispatch, non-Codex; throttled via the existing AgentHookPoster.DrainSpoolsAsync) instead of per-vendor, and adds a 60s daemon-periodic sweep (SpoolDrainLoop) for backlogs no later hook process ever touches (Kiro/OpenCode watcher-owned session-end, GUI idle/parent-exit). Moves HookSpool/TranscriptSpool/LifecycleSpoolDrain to Capacitor.Cli.Core so the daemon can share them without referencing the CLI's exe project. Resolves three ordering hazards surfaced by this wiring (each covered by a new test): the ordered drain's withheld temp files now use a distinct ".ordered-*" namespace so Claude/Cursor's unrelated FIFO drain can never cross-consume them (BLOCKER-1); the generic drain now fires the generate_whats_done side effect on any vendor's session-end, not just Claude's own poster (BLOCKER-2); and a session whose session-end was already delivered is durably marked ended so a later straggler entry is dropped instead of replayed out of order (BLOCKER-3). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: Claude ordering guard must see the .ordered-* backlog namespace AI-1357 Task 12 review fix. ClaudeHookCommand.CurrentSessionHasBacklog was a stale private duplicate that only checked {sid}.jsonl + {sid}.*.draining, not the .ordered-* namespace this task introduced. Since the centralized ordered drain now runs on every non-Codex invocation (incl. --claude), a Claude session-end withheld in .ordered-* pending the transcript tail was invisible to the guard, so a later Claude subagent-stop for the same session posted directly — ahead of the still-withheld session-end (the exact BLOCKER-1/3 cross-spool ordering violation). Delegate CurrentSessionHasBacklog to the public HookSpool.HasBacklog (which covers all three namespaces), matching CursorHookCommand. Add a test proving a subagent-stop spools behind a session-end withheld in .ordered-*. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: AI-1357 CLI acceptance tests (spawn-before-post, drain order, heartbeat) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: add import origin marker constant (CLI) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: shared guarded discovery helper (A4) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: adopt guarded discovery in all import sources (A4) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: restore flat discovery scope for Kiro/Gemini (A4 fix) GuardedDiscovery.EnumerateFiles gains a recursive flag (default true); the two originally-TopDirectoryOnly call sites pass recursive:false so A4 adds symlink/cycle/inaccessible/per-entry safety without widening discovery scope. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: OpenCode ended_at null/0 not 1970 (A3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: per-vendor ended_at resolvers (A3) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: confirm/extend Codex ExtractLastTimestamp shape (A3, open Q2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: Claude/Codex resume end-only reassert + fail-closed tail (A2) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: routed sources send historical-import origin marker (A1) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: Cursor import no stale PR + per-cwd repo cache (item 5) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AI-1358: Antigravity import usage pass, USAGE-before-end, all classifications (item 7) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3: OpenCode recursive descendant discovery + ledger invalidation + finally re-close OpenCodeDb.QueryChildren was single-level (WHERE parent_id = ?), so a subagent's own subagents (grandchildren) were silently dropped on import - never discovered, never posted. Adds OpenCodeDb.QueryDescendants: a BFS walk with a visited-id cycle guard and a depth cap of 8; nodes beyond the cap are neither imported nor promoted and are surfaced via a per-root descendants_omitted stderr diagnostic (never silent). OpenCodeImportSource.ImportDescendantsAsync (renamed from ImportChildrenAsync) now imports every transitive descendant as a direct subagent of the top-level root - the existing flatten shape, since Model A's stream key can't express deeper nesting. ComputeClassificationInfo's content fingerprint covered only the parent transcript + direct QueryChildren, so a previously-completed (AlreadyLoaded) root would keep its grandchildren dropped forever. The fingerprint is now recursive over every descendant (including an empty descendant set) and carries an explicit schema-version marker, cache- busting every pre-upgrade ledger entry so the first post-upgrade import re-classifies and picks up what the old single-level fingerprint couldn't see. Finally-style re-close: a subagent-start posted against an already-Ended root can reactivate it (Model A always routes SubagentStarted through EnsureSessionExists(isReactivation:true)). ImportSessionAsync previously returned Failed as soon as a descendant's lifecycle POST failed, bailing out before session-end and leaving a reactivated root stuck Active. The session-end re-assertion now always runs after the descendants step, regardless of whether it threw; the overall outcome is still Failed so a rerun retries, but the root is never left dangling Active. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3: Gemini recursive descendant discovery with context-carrying walker GeminiSubagentDiscovery.EnumerateSubagentFiles only scanned one level of chats/<parent>/*.jsonl, so a subagent's own subagents (grandchildren) were never imported at all. Adds EnumerateDescendantFiles: a BFS walk (visited-id cycle guard, depth cap 8) that recurses into each discovered subagent's own chats/<subId>/ directory - always derived from the ROOT's chats/ dir, never relative to a nested dir. Critically, the walker carries CONTEXT, not just files: each result carries its own dashed id, depth, and its IMMEDIATE parent's transcript path, because a grandchild's agent_name/invocation-type mapping lives in its immediate parent's invoke_agent tool call - a flat file list would mis-resolve every grandchild to the "subagent" fallback. Descendants beyond the depth cap are neither discovered nor recursed into and are surfaced via a per-root descendants_omitted stderr diagnostic (never silent). GeminiImportSource.ImportSubagentsAsync now imports every transitive descendant as a direct subagent of the top-level root (unchanged flatten shape), resolving each one's agent_type from its own immediate parent transcript (per-parent-transcript cache to avoid re-scanning for siblings). Gemini already reached session-end regardless of descendant failures, so no finally-contract change was needed on this side. EnumerateSubagentFiles (single-level) is left untouched - it's shared with the live watcher and teardown paths, which are out of scope here (AI-1383 D3 is import-only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: re-close the OpenCode root on cancellation-after-reactivation A cancellation arriving after ImportDescendantsAsync had already posted a descendant lifecycle (which can reactivate an already-Ended root) was rethrown immediately, bypassing the session-end re-close and leaving the root stuck Active. The cancellation is now recorded, the terminal session-end is always re-asserted (via CancellationToken.None, since the whole point is to run despite the caller's token already being cancelled), and only then is the original OperationCanceledException rethrown. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: count and fingerprint the whole omitted descendant subtree QueryDescendants (OpenCode) and EnumerateDescendantFiles (Gemini) used to stop AT the depth-cap boundary child, so a chain continuing below it (e.g. depths 9 and 10) was undercounted as a single omitted descendant instead of the true total, and OpenCode's completeness fingerprint — hashing only the in-cap descendant list — never changed when a capped descendant was added, leaving an AlreadyLoaded ledger hit stuck forever with no descendants_omitted diagnostic ever surfacing. Both walkers now continue traversing below the import cap (same visited-set cycle guard, never importing) to produce an accurate total omitted count and the omitted ids themselves, bounded by a new MaxCountingNodes hard ceiling (10,000) as a safety valve against a pathologically huge/wide capped subtree. OpenCode's fingerprint now feeds those omitted ids too (schema version bumped to cache-bust pre-fix ledger entries), so a newly-reachable capped descendant invalidates AlreadyLoaded and reaches the re-import + descendants_omitted diagnostic path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: scope MaxCountingNodes to the below-cap counting walk only The counting ceiling gated the whole unified traversal via the shared visited-id set's total size, so a root with a wide/deep IN-CAP subtree (within MaxDescendantDepth) could itself get silently truncated once 10,000 total ids had been discovered — corrupting the import set, not merely the below-cap counting walk it was meant to bound. A parent whose fingerprint went stale this way would also get stuck AlreadyLoaded forever, since the ledger trusted an incomplete signature. QueryDescendants/EnumerateDescendantFiles now discover every in-cap descendant unconditionally (never bounded by MaxCountingNodes), and apply the ceiling only to the below-cap counting/signature walk. If that below-cap walk hits the ceiling, DescendantDiscoveryResult.CountTruncated is set: the omitted count/ids become an explicit lower bound (surfaced via a "(lower bound — counting ceiling hit)" stderr diagnostic) and OpenCodeImportSource.ClassifyAsync skips the completeness ledger for that session instead of trusting a possibly-stale fingerprint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: propagate cancellation that arrives during title/cleanup The round-1 fix only recorded a cancellation THROWN BY ImportDescendantsAsync. A cancellation arriving AFTER descendants finish — most notably while step 4's PostSetTitleAsync is awaited, which swallows OperationCanceledException internally as "best effort" — was never observed: session-end always runs with CancellationToken.None (by design, to re-assert the terminal state), and nothing downstream re-checked `ct`. The import could report success and mark the ledger complete even though the caller's cancellation was never honored. ImportSessionAsync now checks ct.IsCancellationRequested once more right after the uncancellable session-end re-close, preferring the already-captured cancellation exception (with its original throw site) when step 3 threw one, and constructing a fresh OperationCanceledException otherwise. This runs before evaluating endOk or writing ledger completeness, so a run cancelled at any point — during descendants or during title/cleanup — always re-closes the root, always propagates the cancellation, and never marks the ledger complete. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: stop expanding the below-cap frontier once truncated The MaxCountingNodes ceiling bounded the RETURNED omitted count/ids, but not the actual work behind them: every below-cap descendant already enqueued before the ceiling was hit (up to MaxCountingNodes of them) still got individually dequeued and queried/directory-enumerated for its own children even after CountTruncated was already known true. Split QueryDescendants (OpenCodeDb) and EnumerateDescendantFiles (GeminiSubagentDiscovery) into two independent frontiers — in-cap (always run to completion, unbounded) and below-cap (drained only while !countTruncated). A below-cap node's own descendants are always below-cap too, so the split can't affect in-cap completeness. The moment truncation is established, the below-cap frontier is abandoned instead of drained node-by-node. Adds internal-only instrumentation (OpenCodeDb.QueryChildrenCallCount, an internal EnumerateDescendantFiles overload with a below-cap-dequeue probe) so a regression test can prove the bounded work directly: a below-cap fan-out of MaxCountingNodes+50 nodes now drives ~9 DB queries / 0 directory probes instead of ~10,009 / 10,000. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: cap-bound the per-parent below-cap child fetch A single below-cap parent (depth == MaxDescendantDepth) still queried/ enumerated its ENTIRE child set before CountTruncated could be detected — OpenCodeDb.QueryChildren materialized every row from SQLite, and Gemini's EnumerateChildFiles OrderBy-sorted the whole directory. A pathologically wide below-cap fan-out could force reading/allocating (and, for Gemini, sorting) millions of rows/files in one call. Add QueryChildrenBounded (SQL LIMIT) and EnumerateChildFilesBounded (lazy Take()) for the branch where every child is necessarily below the cap (parent depth >= MaxDescendantDepth), capped to remaining counting capacity + 1 sentinel. In-cap parents (depth < MaxDescendantDepth) keep the unbounded query/enumerator, since the round-2 P1 unbounded-in-cap- discovery contract must not regress. Add regression tests measuring rows/files actually materialized inside a single below-cap fetch (5x MaxCountingNodes children/files), proving the fetch is capped rather than draining the true dataset size. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] D3 review fix: sentinel must count eligible rows, not raw rows A bounded below-cap fetch caps RAW rows/files via a SQL LIMIT / lazy Take(), but GUID validation, row-mapping failure, and visited-set dedup all happen AFTER that fetch. So a malformed or already-visited entry landing inside the sentinel window could silently consume the sole sentinel slot: 10,000 valid descendants plus one non-GUID *.jsonl (or one malformed SQL row) within the bounded window filled omittedIds to exactly MaxCountingNodes while CountTruncated stayed false — a false "complete" report, even though the parent's true child count could be larger. Fix: QueryChildrenBounded/EnumerateChildFilesBounded now also report whether the RAW fetch actually reached its limit (HitLimit), computed from the raw row/file count before mapping-time filtering. When it did, truncation is asserted regardless of how many of the fetched rows turned out valid — the only case that can prove otherwise is a raw count strictly below the limit (definite EOF). This can over-report truncation when a directory/parent holds exactly `limit` raw entries and no more, which is safe: once truncated, the omitted count/ids were already documented as a lower bound, never a claimed- complete enumeration. Regression tests (Gemini + OpenCode) reproduce the exact-boundary + junk/duplicate scenario and confirm they fail against the pre-fix code (a malformed row/file at the boundary silently suppresses CountTruncated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] Qodo fix: surface unreadable Gemini descendant dirs; strip Linear IDs from comments DescendantDiscoveryResult gains UnreadableDescendantDirs: an existing-but-unreadable descendant directory (permissions, I/O error) previously hit a bare catch that returned null and silently dropped that whole subtree with no counter and no diagnostic. It's now counted and surfaced via a stderr diagnostic in GeminiImportSource, alongside the existing descendants_omitted note. Adds two regression tests covering the in-cap and below-cap code paths. Also strips every AI-#### token from code comments in these files and condenses the most verbose review-round narrative down to the essential contract/invariant (ticket history belongs in commit messages, not comments). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] Qodo fix: strip Linear IDs and trim review-narrative comments in OpenCode import Same comment cleanup as the Gemini side: removes every AI-#### token and condenses multi-paragraph review-round narrative (fix #2/#3/#4/#5/#6/#7 etc.) down to the essential contract each comment is documenting. No behavior change here — OpenCode's descendant walk is SQL-based, not filesystem-based, so it has no directly analogous silent directory-enumeration catch (a query failure already propagates and fails the whole import loudly, rather than being swallowed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [AI-1383] Qodo fix: document recursive descendant import in README The OpenCode and Gemini historical-import sections still described child-only subagent import. Updated both to explain: recursive transitive-descendant import up to a depth cap of 8, the descendants_omitted stderr diagnostic, and the "(lower bound — counting ceiling hit)" note for the rare case where counting the omitted subtree itself hits an internal safety cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
alexeyzimarev
pushed a commit
that referenced
this pull request
Jul 18, 2026
* Fix Kiro parent-PID watchdog name match (kiro-cli)
Adds a bounded `-cli` suffix tolerance to MatchesAgentName so the by-name
ancestry walk identifies the durable `kiro-cli` process for vendor `kiro`,
instead of falling back to the fragile getpgrp/getppid heuristic.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Freeze the watcher idle clock while disconnected
Subtracts SignalR-outage time (accrued since last activity) from the idle
measure so a transient disconnect can't false-idle-end a Codex/Antigravity
session, while repeated reconnects with no new lines still idle-end after
the connected budget.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add staged parent-dead / wedged-watcher recovery
On ParentAlreadyDead the watcher now periodically re-resolves + re-arms the
parent-exit watchdog (using the vendor process-name alias); it ends the
session only after a long, configurable ceiling with no transcript progress
and continued resolution failure, and any new progress resets the window.
This is the only end path for a wedged, alive-but-connected watcher that the
server stale sweep can't see. New env var KCAP_PARENT_DEAD_CEILING_MINUTES.
Refs AI-1359.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: AgentHookPoster.Spooled + PostOrSpoolAsync spool-on-lapse
Adds a Spooled outcome and PostOrSpoolAsync: on lapsed-auth or transient
(5xx/408/429/unreachable) failure the lifecycle payload is durably spooled
(HookSpool) for a later drain pass, so live capture can start regardless of
hook-POST delivery (spawn-before-post). Refs AI-1357.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: TranscriptSpool — bounded, no-drop, needs-import marker
AI-1357 Task 2: dedicated on-disk spool for undelivered transcript-tail
batches captured during a watcher outage. Unlike HookSpool (1 MB,
drop-oldest — fine for small lifecycle POSTs), this is bounded at 8 MB
per session with NO SILENT DROP: on cap exhaustion it stops appending
and writes a needs-import marker instead of truncating history, so the
session surfaces as requiring `kcap import` rather than silently losing
transcript content.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: TranscriptSpool no silent drop on I/O failure + Ignored result
Review fixes for AI-1357 Task 2:
- Append now marks needs-import (not phantom Appended) when the live
write throws — the silent-drop this class exists to prevent.
- MarkNeedsImport returns bool and logs to stderr on failure so callers
don't trust an unpersisted marker.
- Add AppendResult.Ignored for the malformed-session-id drop so it can't
be mistaken for a real append.
- Tests for the I/O-failure path and the malformed-id path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: LifecycleSpoolDrain — global cross-spool ordered drain pass
Adds a session-agnostic drain pass to run at the start of every kcap
invocation (and periodically by the daemon), enforcing per-session
ordering across the lifecycle (HookSpool) and transcript (TranscriptSpool)
spools: spooled session-start -> transcript tail (+ needs-import marker,
delivered even over cap) -> spooled session-end. Adds the route-filtered
HookSpool.DrainRoutesAsync/SessionIdsWithBacklog and
TranscriptSpool.SessionIdsWithBacklog helpers this needs; HookSpool's
existing route-agnostic DrainAllAsync is untouched (still used by
Claude/Cursor).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: spawn-before-post + Spooled for Kiro/OpenCode/Pi/Copilot hooks
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: throttle spool drain per-prompt; ShouldSpawnAfter drops AuthLapsed
AI-1357 review: (1) DrainSpoolsAsync now self-throttles via an on-disk
.last-drain stamp (30s) so Kiro agentSpawn / OpenCode idle re-fires can't
attempt a network drain every prompt during an outage; reaps moved inside.
(2) ShouldSpawnAfter spawns only on Posted/Spooled — AuthLapsed spools
nothing, so spawning would orphan a session with a dropped SessionStarted.
(3) Documented the fresh-client deviation (no reusable vendor client exists
at the pre-POST drain point).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Codex spawn-before-post with stdout-first carve-out
Auth lapse now spools + spawns via PostOrSpoolAsync/ShouldSpawnAfter
(matching Kiro/OpenCode/Pi/Copilot) instead of dropping the session and
skipping the watcher. The global lifecycle/transcript spool drain runs
fire-and-forget AFTER Codex's blocking `{"continue":true}` stdout
handshake — never before, never gating it — so a large/unreachable spool
backlog can't stall the parent process. Extracted WriteSessionScopedOutput
and RunSessionStartHandshakeForTest as the seam CodexStdoutContractTests
uses to prove the ordering without a live server.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Antigravity spawn-before-post + Gemini awaited EnsureWatcherRunning
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Gemini session-start spawn-before-post + gated spool drain
Migrate Gemini's session-start off the POST-only PostAsync path to
AgentHookPoster.PostOrSpoolAsync + ShouldSpawnAfter, so an auth lapse or
transient outage spools the SessionStarted payload and STILL spawns the
watcher (was: returned without reaching EnsureWatcherRunning on anything
but Posted). Wire the throttled DrainSpoolsAsync gated to the two lifecycle
events (SessionStart/SessionEnd) so the per-turn Notification path adds no
network cost. Fix a dangling "PostHookAsync (below)" comment in
AntigravityHookCommand left by the Task 6 deletion.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: shutdown final-line completion signal (newline/parseable-JSON)
The shutdown-during-outage final drain (idle-timeout / parent-exit) used
to disable the half-written-line holdback unconditionally, which could
consume a line the agent was still mid-write on, or drop a complete
newline-less final line. Add a pure IsFinalLineComplete signal (empty /
newline-terminated / last line parses as JSON — length-stability alone is
NOT proof, since a large write can pause mid-record past any bounded
window) plus a bounded (<=2s) WaitForFinalLineCompletionAsync wait in the
final-drain path. Complete -> send the newline-less final line as before;
incomplete -> keep the holdback on (never send-and-advance a truncated
line) and flag the session needs-import via TranscriptSpool so `kcap
import` can recover it later.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: close final-drain completeness TOCTOU; re-validate at consume time
Review found the completeness decision and the consuming read were two
separate reads: WaitForFinalLineCompletionAsync probed once, then
DrainNewLines consumed with holdback disabled — so a final line that
resumed growing in the gap was sent-and-advanced anyway, violating "never
consume a still-growing line."
Move the decision into the consuming read via an IncompleteFinalLinePolicy
(Hold / ConsumeIfComplete). Under ConsumeIfComplete the unterminated final
line is consumed ONLY IF the exact bytes read parse as a complete JSON
record; otherwise it is held and NewTranscriptLines.HeldIncompleteFinalLine
is set. The parse check runs on the same bytes being consumed, so there is
no TOCTOU with the bounded pre-wait (now purely advisory — gives the writer
time). RunWatch flags needs-import off the actual consume-time held result
via state.FinalDrainHeldIncompleteLine.
Also updates the stale ApplyPartialLineHoldback comment that claimed the
final drain opts out of holdback because "the file is static then" — the
exact wrong assumption this task fixes.
Tests: SplitNewCompleteLines + ReadNewCompleteLinesAsync gain parseable-
consume / unparseable-held cases, HeldIncompleteFinalLine parity, and a
grew-after-the-completeness-check TOCTOU guard asserting the resumed
partial is held, not sent.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: spool undelivered transcript tail on shutdown-during-outage
At final drain during idle-timeout/parent-exit shutdown, DrainNewLines only
advances LinesProcessed past lines the hub confirmed sent. If the hub is
down (or still reconnecting) at that point, any transcript lines from
LinesProcessed to EOF were never delivered and the process exits right
after — silently dropping the tail.
Adds BuildTranscriptSpoolBatch (pure TranscriptBatch JSON builder) and
SpoolUndeliveredTranscriptTailAsync, which re-reads the undelivered tail
using the same ConsumeIfComplete completion decision as the final drain
and spools it into the dedicated TranscriptSpool (task 2) so the global
drain (task 3) replays it after recovery, without a manual `kcap import`.
Cap exhaustion still marks the session needs-import rather than dropping.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: spool tail unconditionally + redact secrets (task-8 review)
Two Critical review findings on the shutdown-during-outage tail spool:
1. Insufficient trigger → silent drop. Gating the spool on
hubConnection.State != Connected missed a HubException thrown by
SendTranscriptBatch2 while the connection stayed Connected (the generic
catch does not change connection state) — the tail was undelivered but
never spooled. Call SpoolUndeliveredTranscriptTailAsync unconditionally
on the shutdown path; it is already a no-op when position == EOF.
2. SECURITY: spooled tail skipped secret redaction. The live/inline drains
run lines through SecretRedactor.RedactLine, but the spooled tail was
written raw → secrets on disk and POSTed unredacted on replay. Redact
each spooled line exactly as the live drain does before writing.
Tests: undelivered_tail_spooled_even_though_connection_stayed_up (state-
agnostic spooling) and spooled_tail_is_secret_redacted (ghp_ token →
[REDACTED], raw secret absent from disk).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: watcher heartbeat + lock-guarded staleness reap+respawn
AI-1357 task 9. WatcherManager.IsWatcherAlive only checked the PID, so a
wedged (hung-but-alive) watcher was never restarted. The watcher now
touches a per-key heartbeat file every main-loop iteration — including
no-content drains and while disconnected/reconnecting — via the pure
WatcherHeartbeat helper (Core). IsWatcherAlive requires both a live PID
and a non-stale heartbeat (past a 30s startup grace, 20s threshold).
EnsureWatcherRunning reaps a wedged watcher (kill + respawn) under a
cross-platform spawn lock — same FileShare.None/flock primitive as
DaemonLock — so concurrent hooks racing the same key can't double-spawn.
The whole decide-and-spawn sequence is locked, not just the reap: killing
the old watcher deletes its pid file before the respawn's new one lands,
so guarding only the reap step would leave a window where a second hook
sees "no pid" and spawns unguarded (caught by the new concurrency test).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: keep heartbeat fresh during connect-retry + purge watcher sidecar files
AI-1357 task 9 review. Two Important issues.
1. Startup/reconnect outage looked "wedged" → false reap. The SignalR
connect-retry loop never touched the heartbeat, so a server outage
>=~50s at startup was indistinguishable from a wedged loop and the
hook probe would repeatedly reap+respawn a healthy-but-reconnecting
watcher. RunWatch now touches the heartbeat at the top of every
connect-retry iteration and waits via DelayWithHeartbeatAsync, which
chunks the backoff (grows to 30s > the 20s threshold) into <=5s slices
through the new pure HeartbeatSlices helper, touching before each — so
no wait window can ever cross the staleness threshold. The automatic-
reconnect path was already covered by the main loop's per-iteration
touch in the disconnected branch.
2. Sidecar file leak. Only .pid was deleted. KillWatcher now removes the
{key}.heartbeat and {key}.started markers, but deliberately NOT the
{key}.spawnlock: KillWatcher runs from inside WithSpawnLock on the reap
path, and unlinking a held lock file on POSIX lets a racing hook open a
fresh non-conflicting flock (the DaemonLock unlink-race). Spawn locks
are swept by the new WatcherManager.PurgeAuxiliaryFiles(), called by
kcap cleanup (holds no lock; also mops up orphans). CleanupCommand now
uses GetWatcherDir() so it honors KCAP_WATCHER_DIR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Kiro live usage-backfill synthetic lines (credits/context%)
AI-1357 task 10: the live kcap watch path never emitted Kiro's per-turn
credits/context% (the AI-1196 hedge only fires on import, which reads the
sibling {id}.json up front). A live drain has usually already sent the
turn's anchor AssistantMessage line by the time Kiro flushes that sidecar,
so inline enrichment can't reach it there.
Add a synthetic KiroUsageBackfilled line per turn anchor instead, mirroring
the Antigravity synthetic-USAGE-line pattern: WatchCommand.
BuildKiroUsageBackfillLine builds the JSONL; AppendKiroUsageBackfillLines
reads the sidecar via KiroUsage.AnchorMap and appends one line per anchor
not yet in the new WatchState.KiroUsageEmittedAnchors, staging them on
KiroUsagePendingAnchors for the caller to commit only after a successful
send (so a failed batch re-reads and re-stages instead of losing them).
Server-side event fold is a separate follow-up (task 13).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: Cursor live subagent split via hook/backfill path (AI-1151)
CursorSubagentCorrelator only ran on the historical import path, so a live
Cursor Task/Agent subagent was ingested as its own top-level session instead
of nesting under its parent. Cursor is not watcher-backed (CursorHookCommand
backfills each session's transcript over HTTP as hooks arrive), so the
correlation now runs inline in that per-hook dispatcher via a new thin
wrapper, CursorLiveSubagentLinker.
At a child's sessionStart, ResolveParent (reusing
CursorSubagentCorrelator.Correlate) checks the sibling transcripts under the
same agent-transcripts workspace dir; a match is persisted to a small
on-disk marker (the CLI is a fresh process per hook) and diverts subsequent
hooks for that session: subagent-start replaces the top-level sessionStart,
transcript backfill is routed under the parent with agent_id=child (mirrors
CursorImportSource.SendSubagentLifecycleAsync's watermark + POST shape), and
subagent-stop replaces sessionEnd. Mid-lifecycle hooks
(beforeSubmitPrompt/afterAgentThought/telemetry) are not forwarded for a
linked child, matching the import path (which has no side channel for them
either) instead of writing to a phantom AgentSession stream that never got a
SessionStarted.
Dashless ids are used throughout so a live-then-import of the same session
converges on the same AgentSubsession-{parent}-{child} stream rather than
duplicating it (ties to AI-1358 A1). Known eventual-consistency gap
(documented in code): if the parent's Task tool_use isn't yet flushed to
disk at the child's first hook, the child is temporarily ingested top-level
until a later `kcap import --cursor` converges it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: enforce start-before-stop spool ordering on Cursor live subagent path (AI-1151)
HandleSubagentChildEventAsync posted subagent-stop (and ran the sessionStart
backfill) with no spool.HasBacklog check, unlike the sibling top-level path.
Under a transient failure that left a subagent-start undelivered in the spool,
a later subagent-stop could be POSTed ahead of its own subagent-start, leaving
the AgentSubsession stream mis-ordered / never opened.
Mirror the top-level guard: after the HandleCore drain, if the child still has
spool backlog (drain hit a transient failure), spool the fresh lifecycle event
behind it and skip the agent-routed backfill so the next hook re-drains
start-first. Also gate the sessionStart backfill on the subagent-start POST
succeeding, so the transcript can't be routed to an AgentSubsession the
now-spooled start hasn't opened yet.
Adds a regression test: a child whose subagent-start is spooled (transient
failure) does not get subagent-stop delivered ahead of it, and a later
recovered drain delivers start before stop.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* feat: wire global spool-drain pass into hook entry + daemon-periodic
AI-1357 Task 12. Runs LifecycleSpoolDrain centrally in Program.cs's
`case "hook":` (before dispatch, non-Codex; throttled via the existing
AgentHookPoster.DrainSpoolsAsync) instead of per-vendor, and adds a 60s
daemon-periodic sweep (SpoolDrainLoop) for backlogs no later hook process
ever touches (Kiro/OpenCode watcher-owned session-end, GUI idle/parent-exit).
Moves HookSpool/TranscriptSpool/LifecycleSpoolDrain to Capacitor.Cli.Core so
the daemon can share them without referencing the CLI's exe project.
Resolves three ordering hazards surfaced by this wiring (each covered by a
new test): the ordered drain's withheld temp files now use a distinct
".ordered-*" namespace so Claude/Cursor's unrelated FIFO drain can never
cross-consume them (BLOCKER-1); the generic drain now fires the
generate_whats_done side effect on any vendor's session-end, not just
Claude's own poster (BLOCKER-2); and a session whose session-end was already
delivered is durably marked ended so a later straggler entry is dropped
instead of replayed out of order (BLOCKER-3).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix: Claude ordering guard must see the .ordered-* backlog namespace
AI-1357 Task 12 review fix. ClaudeHookCommand.CurrentSessionHasBacklog was a
stale private duplicate that only checked {sid}.jsonl + {sid}.*.draining, not
the .ordered-* namespace this task introduced. Since the centralized ordered
drain now runs on every non-Codex invocation (incl. --claude), a Claude
session-end withheld in .ordered-* pending the transcript tail was invisible
to the guard, so a later Claude subagent-stop for the same session posted
directly — ahead of the still-withheld session-end (the exact BLOCKER-1/3
cross-spool ordering violation).
Delegate CurrentSessionHasBacklog to the public HookSpool.HasBacklog (which
covers all three namespaces), matching CursorHookCommand. Add a test proving a
subagent-stop spools behind a session-end withheld in .ordered-*.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: AI-1357 CLI acceptance tests (spawn-before-post, drain order, heartbeat)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: add import origin marker constant (CLI)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: shared guarded discovery helper (A4)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: adopt guarded discovery in all import sources (A4)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: restore flat discovery scope for Kiro/Gemini (A4 fix)
GuardedDiscovery.EnumerateFiles gains a recursive flag (default true); the
two originally-TopDirectoryOnly call sites pass recursive:false so A4 adds
symlink/cycle/inaccessible/per-entry safety without widening discovery scope.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: OpenCode ended_at null/0 not 1970 (A3)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: per-vendor ended_at resolvers (A3)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: confirm/extend Codex ExtractLastTimestamp shape (A3, open Q2)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Claude/Codex resume end-only reassert + fail-closed tail (A2)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: routed sources send historical-import origin marker (A1)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Cursor import no stale PR + per-cwd repo cache (item 5)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AI-1358: Antigravity import usage pass, USAGE-before-end, all classifications (item 7)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Task A1 (CLI): shared Cursor prompt canonicalization + parity corpus
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Task C2: suppressed Cursor re-import still records repository (D1, CLI)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Task F1: widen Cursor correlator input to same-workspace discovery (D5)
CursorImportSource.ClassifyAsync built its subagent correlator input strictly
from the scope-filtered `sessions` slice, so a child whose parent falls
outside --session/--cwd/--since/scope (e.g. `--session <child>`) could never
be correlated -- the correlator never even saw the parent's Task prompt.
Widen ONLY the correlator's input to every session under the same
sanitized-workspace agent-transcripts/ dir (cheap local file reads via a new
DiscoverSameWorkspaceSessionPaths helper mirroring DiscoverAsync's walk minus
the filters). Repo detection continues to run only on the filtered slice.
StampSubagentMeta additionally carries ParentSessionId on a correlated child
so Task F2 can reconcile an orphaned child (parent not in this run's plan) to
a standalone import.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Task F2: import an orphaned Cursor child standalone (D5)
CursorImportSource.ImportSessionAsync unconditionally skips (drops) any
session stamped IsSubagentChild, assuming its parent will import it inline.
That assumption breaks once F1 widened the correlator's input beyond the
scope-filtered slice: a child's parent can now be correlated even when the
parent itself isn't part of this run's plan (never classified, or classified
but excluded for another reason) -- silently dropping the child instead of
importing it, and risking a subagent-start against an un-planned/possibly
ended parent.
Add ImportCommand.ReconcileOrphanedCursorSubagentChildren, run right after
`routed` is built: for each routed classification still flagged
IsSubagentChild, clear IsSubagentChild/ParentSessionId when its
ParentSessionId isn't itself among `routed`'s session ids. The child then
falls through CursorImportSource's existing (now correctly conditional)
nested-child skip into the ordinary standalone session-start/transcript/
session-end path. The server-side CursorSubagentAdoptionSweep (AI-1156 D4)
adopts it under its real parent later, once the parent exists server-side.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: prune out-of-plan Cursor subagent children before attach (P1)
CursorImportSource.ClassifyAsync widens its correlator input to same-workspace
discovery (D5/F1) so it can find parent<->child links even when the filtered
run only sees one side. That widened set was previously also used to decide
which children to ATTACH: a routed parent's SubagentChildren could list a
child that fell outside this run's routed/filtered plan (e.g. `--session
<parent>`, or a --since/scope filter excluding the child), so
ImportSessionAsync would send that child's subagent-start/transcript/stop
anyway -- silently widening the import plan past what the user asked for.
ReconcileOrphanedCursorSubagentChildren now prunes SubagentChildren down to
children that are actually part of `routed` for this run, in addition to its
existing orphaned-child-side reconciliation. A child left out of the plan
still imports standalone on its own run, or gets adopted later by the
server-side CursorSubagentAdoptionSweep.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: don't double-count AlreadyLoaded repo-backfill replays (P2)
The routed-phase loop (Cursor) treats any Loaded/Resumed ImportOutcome as a
newly imported session, incrementing routedLoaded and joining
importedSessionIds. But an AlreadyLoaded classification's ImportSessionAsync
call exists purely to re-assert lifecycle hooks and backfill the repository
node (the C2 suppressed-repo-import contract) -- there's nothing past the
watermark to send, yet it still returns Resumed (no zero-line signal to
distinguish "nothing sent" from "brand new"). That folded one session into
both the classify-time AlreadyLoaded bucket and the routed Loaded counter, and
added it to importedSessionIds so a later --private pass would wrongly
re-private a session this run never actually (re)imported.
IsLifecycleOnlyRoutedReplay(status, outcome) gates both loop branches (TTY and
non-TTY): an AlreadyLoaded + Loaded/Resumed replay is now surfaced with its
own "Refreshed" message instead of counting toward Loaded or
importedSessionIds. A genuine New/Partial import, or an AlreadyLoaded replay
that fails, is unaffected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: thread a real sent-child-content signal out of ImportSessionAsync (P1)
IsLifecycleOnlyRoutedReplay inferred "no new work happened" purely from the
parent classification/outcome pair, which is wrong when an AlreadyLoaded
Cursor parent attaches a previously-unloaded nested subagent child inline —
that IS real new work, even though the parent's own transcript has nothing
past its watermark.
Add ImportSessionResult (ImportOutcome + SentChildContent, with an implicit
conversion from ImportOutcome so every non-Cursor source is unaffected) as
the new IImportSource.ImportSessionAsync return type. CursorImportSource
tracks whether SendSubagentLifecycleAsync actually POSTed new transcript
bytes for any correlated child and surfaces that via SentChildContent,
independent of its own sent/startLine outcome.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: consume the sent-child signal, don't double-count AlreadyLoaded+Skipped children (P1+P2)
Two follow-on findings on IsLifecycleOnlyRoutedReplay:
- P1: the routed-phase loop now passes ImportSessionResult.SentChildContent
into IsLifecycleOnlyRoutedReplay. An AlreadyLoaded parent that attached a
brand-new nested child is no longer suppressed — it joins routedLoaded /
importedSessionIds so a --private run correctly re-privates it.
- P2: broaden IsLifecycleOnlyRoutedReplay to also recognize
AlreadyLoaded+Skipped (a correlated nested child whose own routed call
short-circuits to Skipped because its parent imports it inline). Previously
this rolled into routedExcluded on top of the classify-time AlreadyLoaded
bucket, double-counting the same child as both Already-loaded and Excluded.
A Failed outcome is untouched by this check and still surfaces as an error.
Extends ImportDoneBreakdownTests with regression coverage for both findings.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: fail-open child watermark probe must not assert new content (P2)
SendSubagentLifecycleAsync fails open when the child subsession watermark
probe throws (transient 5xx): startLine resets to 0 and the whole child
transcript is reposted. Those events are server-side idempotent duplicates
when the child was already complete, but SendTranscriptBatches returns the
count of lines POSTED, not "new" - so the SentContent signal was wrongly
true, letting an AlreadyLoaded parent be miscounted as newly Loaded and
re-privatized.
Track whether the probe itself succeeded (probeFailed) and require it be
false in addition to childSent > 0 before asserting SentContent. A genuine
resend - the watermark was actually known, whether it came back empty or
with a real value - still reports true.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: scope the lifecycle-only-replay gate to Cursor (P2)
IsLifecycleOnlyRoutedReplay applied to every routed vendor, but
SentChildContent is populated only by Cursor. Antigravity's AlreadyLoaded
repair path deliberately routes through ImportChildrenAsync to attach a
previously-missing child - which can POST new transcript content - then
returns Skipped via the implicit ImportOutcome conversion (SentChildContent
defaults false). The shared gate then suppressed that real import as
lifecycle-only, wrongly excluding it from Done accounting.
Scope the gate to vendor == "cursor": the behavior it models (repo-backfill
AlreadyLoaded replays with no new content) is Cursor-specific, so this is
the minimal fix that can't suppress a real import on any other vendor.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix: probe-failure resend defaults to privacy-safe (counts + privatizes) (P1)
A prior review fix (a12c254) forced SentContent=false whenever the child
subsession watermark probe threw, treating a fail-open full resend as proof
of "no new content". That's wrong: a failed probe is INDETERMINATE, not
proof of a duplicate. When the child is genuinely new and the watermark
endpoint merely 500s transiently, the fail-open resend really does post new
content, yet the caller was told nothing happened — so an AlreadyLoaded
parent was excluded from importedSessionIds and never privatized under
--private, leaking the newly-attached child's content on a public session.
Fix: drop the probeFailed suppression. SentContent is now
`stopOk && childSent > 0` regardless of whether the probe succeeded or
failed open — a probe failure is never treated as "definitely no new
content", only as "unknown", and posted content is conservatively counted
so the parent is privatized. Accepted trade-off (separately tracked): an
already-complete child that gets fail-open-resent will also report
SentContent=true, which can cosmetically double-count its AlreadyLoaded
parent across the Loaded/AlreadyLoaded buckets — privacy correctness wins
over count precision.
Inverts the round-3 regression test
(already_loaded_parent_with_failing_child_watermark_probe_does_not_report_sent_child_content)
to assert the privacy-safe behavior, renamed to
already_loaded_parent_with_failing_child_watermark_probe_conservatively_reports_sent_child_content_to_preserve_privacy.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Review fix (r6): privatize --private sessions independently of import lifecycle success (P1)
A Cursor session's routed import only joined importedSessionIds — the sole
input to the end-of-run --private PUT /visibility pass — when its own
ImportOutcome was Loaded/Resumed and wasn't a lifecycle-only replay. A
subagent-stop/session-end POST failing AFTER a child's transcript content had
already persisted made ImportSessionAsync return Failed (excluded from
privatization), and a later retry that read the session as
AlreadyLoaded-with-nothing-new was excluded too (lifecycle-only replay) —
either way a --private run's newly-attached content stayed public forever,
even across repeated re-runs.
Fix: a separate, outcome-independent tracker (privateScopeSessionIds) now
captures every Cursor routed classification touched under --private,
regardless of ImportOutcome/ClassificationStatus/SentChildContent, and is
unioned into the privatize set at the end of the run. importedSessionIds and
the Done-grid counting it feeds are untouched (the AI-1389 cosmetic
double-count concern stays deferred, as intended).
Also considered privatize-first (stamping default_visibility on Cursor's
session-start payload, as Antigravity/OpenCode/Pi already do) — server-side
CursorSessionStartHook has no DefaultVisibility field, so that mechanism
would be silently inert for Cursor until the server adds support. The union
fix instead reuses the already-trusted PUT /api/sessions/{id}/visibility path
end-of-run, so it works with the currently deployed server.
Regression tests (new CursorPrivatizeLifecycleFailureTests, integration):
verified red without the fix, green with it.
1. lifecycle POST fails after content persisted -> still privatized
2. AlreadyLoaded retry with no new content -> still privatized (self-heals)
3. non---private run -> never calls PUT visibility
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Qodo fix: scope Cursor subagent correlation per workspace, thread cancellation into the same-workspace scan
Qodo finding #2 (Action required): ClassifyAsync unioned every same-workspace
transcript path across ALL workspaces touched by a classify call and ran
CursorSubagentCorrelator.Correlate over the combined set, so a child session
in workspace A could be linked to a parent in workspace B whenever their
canonical prompt hashes happened to match — corrupting nesting/attribution.
Same-workspace discovery is meant only to give a filtered/scoped import
visibility into a parent; it must also constrain correlation. Fixed by
building one path map per workspace (keyed by SanitizedDir, or an isolated
per-session bucket when a session carries none) and running Correlate()
independently per workspace, then merging the resulting subagentLinks.
Qodo finding #3 (Review recommended): DiscoverSameWorkspaceSessionPaths did a
synchronous Directory.EnumerateDirectories scan without consulting the
CancellationToken ClassifyAsync receives, so Ctrl-C could hang until a large/
slow-filesystem scan finished. Threaded the token through, calling
ct.ThrowIfCancellationRequested() once per directory iteration (letting
OperationCanceledException propagate past the existing hostile-subtree
catch-alls).
Adds regression tests for both: a cross-workspace prompt-collision case
asserting no cross-workspace link is made, and a pre-cancelled-token case
asserting ClassifyAsync throws promptly during the scan.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* [AI-1154] Qodo fix: trim verbose Cursor subagent reconciliation doc comment
Qodo finding #1 (Review recommended): the doc comment above
ReconcileOrphanedCursorSubagentChildren carried narrative history (two
separate review-round call-outs, a <para> aside) well past the repo's
keep-comments-concise convention. Trimmed to the essential contract — what
counts as an orphan, why it must import standalone, and why SubagentChildren
is pruned — and dropped the embedded Linear identifiers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 21, 2026
…(Qodo) - Security (Qodo #3): AcpReviewFlowMcp allowlist now resolves via KcapMcpRegistry.TryResolveReviewFlowAllowlist — the authoritative read-only reviewer policy (ReviewFlowAutoApprovableServers) the orchestrator already enforces for Codex — so a write server like kcap-memory can never reach an auto-approving reviewer. - Observability (Qodo #2): an unknown/flow-starting/non-auto-approvable entry now fails the launch fast with the offending name instead of being silently dropped. Build takes the validated canonical ids. - Maintainability (Qodo #1): trimmed verbose spec-prose comments to concise intent (rationale lives in the design spec). Tests updated: split recursion/dedup into a dedup test + a parameterized fail-fast test (kcap-flows/kcap-memory/kcap-workitems/unknown). 67 tests pass; AOT clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 21, 2026
…P + owned-worktree-gated auto-approve (#337) * [AI-1407] ACP unattended reviewer foundation: flow-result MCP over ACP + owned-worktree-gated auto-approve Wires the vendor-neutral plumbing so a per-vendor child can flip SupportsUnattended and become an unattended review-flow reviewer over ACP. Flips nothing itself — every real vendor stays SupportsUnattended:false, so these paths are unreachable in production (Cursor pinned byte-for-byte). - AcpReviewFlowMcp.Build: the AcpMcpServerSpec analogue of ClaudeLauncher's PTY BuildReviewFlowMcpConfig — kcap-flow-result (KCAP_URL + KCAP_FLOW_AGENT_ID) plus the flow's MCP allowlist resolved via KcapMcpRegistry (flow-starting servers stripped, unknown skipped, deduped by canonical id to match ClaudeLauncher's JsonObject keying). - AcpHostedAgentRuntimeFactory: ValidateAndBuildReviewFlowMcp runs as the first statement of StartAsync, BEFORE the connectionSource spawns, and fails closed for a review-flow launch that isn't unattended-capable, isn't an owned worktree, has no ACP mcpServers support, or can't build a deliverable result channel (nonblank url/path/agent id). BuildProcessStartInfo gains a matching owned-worktree refusal as defense-in-depth. - AcpInteractionBridge: autoApproveUnattended selects the least-privilege allow option by Kind (nonblank + unique OptionId) without routing to a human, and declines elicitations; fails closed otherwise. Audit log pins agentId + kind + untrusted tool title, never a path. Owned-worktree is a launch precondition, NOT filesystem confinement — real containment is a per-vendor live-verification gate for each reviewer child. 62 new/covered unit tests; AOT-clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * AI-1407: enforce read-only reviewer allowlist policy + trim comments (Qodo) - Security (Qodo #3): AcpReviewFlowMcp allowlist now resolves via KcapMcpRegistry.TryResolveReviewFlowAllowlist — the authoritative read-only reviewer policy (ReviewFlowAutoApprovableServers) the orchestrator already enforces for Codex — so a write server like kcap-memory can never reach an auto-approving reviewer. - Observability (Qodo #2): an unknown/flow-starting/non-auto-approvable entry now fails the launch fast with the offending name instead of being silently dropped. Build takes the validated canonical ids. - Maintainability (Qodo #1): trimmed verbose spec-prose comments to concise intent (rationale lives in the design spec). Tests updated: split recursion/dedup into a dedup test + a parameterized fail-fast test (kcap-flows/kcap-memory/kcap-workitems/unknown). 67 tests pass; AOT clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * AI-1407: treat reserved kcap-flow-result allowlist id as a no-op (codex review) The result channel is always injected by AcpReviewFlowMcp.Build and is not a KcapMcpRegistry entry, but the server's DynamicFlowPolicy legitimately lists kcap-flow-result in McpAllowlist. Strip it (case-insensitive) before the strict read-only validation so it is a redundant no-op, not a fail-fast rejection — while still rejecting unknown/flow-starting/write-server entries. Single-sourced as AcpReviewFlowMcp.ResultChannelId. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * AI-1407: normalize reserved kcap-flow-result no-op in TryResolveReviewFlowAllowlist (codex review r2) Move the reserved result-channel normalization from a daemon-side pre-filter into KcapMcpRegistry.TryResolveReviewFlowAllowlist so the ACP reviewer path and the Codex orchestrator path share ONE consistent contract: kcap-flow-result (always launcher-injected, not a registry entry, legitimately listed by the server's dynamic-flow policy) is a satisfied no-op, never re-emitted, never a rejection. Single-sourced as KcapMcpRegistry.ReservedResultChannelId. Core test added; ACP end-to-end no-op test retained. 9+31+37+67 tests pass; AOT clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Jul 28, 2026
…e as a swap Codex review round 1 on this PR. Three findings, all real. P1 #1 -- the retry budget was right for registration and wrong on the launch path: three 10s waits plus backoff is ~30.75s of a thread-pool thread held before a launch can even be rejected, multiplied by concurrent launches. Registration keeps the three attempts (its result is cached for the daemon's lifetime, so a miss there is durable and destructive); the launch path now uses a single attempt. It does not need the retry: a miss there is no longer misclassified, produces a retryable rejection, and the caller can try again. P1 #2 -- the same bug I fixed at registration was still live at LAUNCH. With a version advertised at registration and the launch probe timing out, the swap arm fired and told the operator the CLI had changed and to restart -- when nothing had changed and restarting merely repeats the transient failure. A failed probe is now its own arm, classified BEFORE the swap comparison, with a retry remedy. Still fails closed; only the diagnosis and the remedy differ. P2 #3 -- the swap arm hid the out-of-range diagnosis. A CLI genuinely replaced with an out-of-range version was told only to restart the daemon, and restarting re-advertises the same out-of-range version, so the remedy could never work. The range is now checked before the swap arm, so the operator is told the thing they can act on. Final arm order: vendor -> connection -> policy -> probe-failed -> range -> swap. Mutation-verified rather than asserted: - remove the probe-failed arm -> 3 tests fail - move the swap arm before the range arm -> 1 test fails 12 tests, check-linear-ids clean.
realtonyyoung
added a commit
that referenced
this pull request
Jul 28, 2026
…ewer (#385) * fix: a transient version-probe timeout must not disable the reviewer Surfaced by three cross-driver conformance attempts that each failed differently. One defect explains all of them. ProbeCliVersion shelled out to `<cli> --version` with a 3s timeout, no retry, returning null on failure -- and that null was computed once at daemon registration and advertised as ExpectedCliVersion for the daemon's whole lifetime. `claude` is a Node CLI whose cold start routinely exceeds 3s on a loaded host. One transient failure then produced either: - the vendor dropping out of the unattended set entirely (reviewer_vendor_unavailable), because the server cannot range-check a null; - or EVERY launch being rejected, because the launch-time arm demanded exact equality between a freshly probed version and the advertised null. The second is worse than it sounds: it survives the remedy the error text gives. Restarting the daemon on a still-loaded host simply re-poisons the advertisement, which is exactly what happened between two of the attempts. Three changes: 1. The probe retries (3 attempts, 250ms/500ms backoff) with a 10s budget. A cold Node start under load is not an error condition. 2. The equality arm no longer treats "null advertised" as a mismatch. It exists to catch a CLI SWAP between advertisement and launch; null-vs-value is not a swap. A null or empty advertised version now falls through to the range check, which is the real gate -- proven by a test that an out-of-range CLI is still rejected when the advertisement is null. 3. The rejection NAMES the failing arm. Four distinguishable conditions collapsed into one message that reported the certification revision -- which matches on every one of these paths -- and told the operator to update a CLI that was correct and in range. This was already recommended when the same message was hit from a different arm (empty AllowedCliRanges), but that fix shipped server-side only and the daemon message never changed. Each arm now states the cause AND the remedy that applies to it, and a test asserts no rejection mentions the revision. The arm logic is extracted to an internal EvaluateReviewerCertification so it is testable at all -- it was previously a single inline boolean expression. 9 new tests, 29/29 in the neighbouring daemon suite, check-linear-ids clean. * fix: split the probe budget, and stop misreading a failed launch probe as a swap Codex review round 1 on this PR. Three findings, all real. P1 #1 -- the retry budget was right for registration and wrong on the launch path: three 10s waits plus backoff is ~30.75s of a thread-pool thread held before a launch can even be rejected, multiplied by concurrent launches. Registration keeps the three attempts (its result is cached for the daemon's lifetime, so a miss there is durable and destructive); the launch path now uses a single attempt. It does not need the retry: a miss there is no longer misclassified, produces a retryable rejection, and the caller can try again. P1 #2 -- the same bug I fixed at registration was still live at LAUNCH. With a version advertised at registration and the launch probe timing out, the swap arm fired and told the operator the CLI had changed and to restart -- when nothing had changed and restarting merely repeats the transient failure. A failed probe is now its own arm, classified BEFORE the swap comparison, with a retry remedy. Still fails closed; only the diagnosis and the remedy differ. P2 #3 -- the swap arm hid the out-of-range diagnosis. A CLI genuinely replaced with an out-of-range version was told only to restart the daemon, and restarting re-advertises the same out-of-range version, so the remedy could never work. The range is now checked before the swap arm, so the operator is told the thing they can act on. Final arm order: vendor -> connection -> policy -> probe-failed -> range -> swap. Mutation-verified rather than asserted: - remove the probe-failed arm -> 3 tests fail - move the swap arm before the range arm -> 1 test fails 12 tests, check-linear-ids clean. * fix: notify the caller before the self-heal, not after it Codex review round 2. My round-1 claim that the launch path was bounded to one 10s attempt was FALSE, and the reviewer was right to check the wiring rather than take it. The rejection branch called ComputeUnattendedVendorCapabilities BEFORE LaunchFailedAsync -- and that recompute re-runs the REGISTRATION probe, three 10s attempts plus 750ms of backoff. So a failed launch probe cost ~10s in the single-attempt probe, entered the new transient arm, and then blocked ~30.75s more before the caller heard anything. Splitting the probe budget did nothing while this sat in front of the notification. LaunchFailedAsync now goes first. The self-heal (recompute + re-advertise) still runs -- a certification mismatch usually does mean the advertisement is stale -- but off the response path and contained, so a throw there cannot surface as a second, different failure for a launch that has already been rejected. Not unit-tested, and saying so rather than implying otherwise: this is call-site wiring inside a large launch method, and the arm tests deliberately cover only EvaluateReviewerCertification. The reviewer made the same point. Verified by reading the ordering, not by a test. * fix: single-flight the background capability refresh Codex review round 3. My round-2 fire-and-forget REINTRODUCED the defect this PR removes, and the reviewer described the interleaving exactly: refresh A starts on a loaded host and spends ~30s timing out refresh B starts later, probes successfully, publishes, re-registers A completes LAST, overwrites the valid snapshot with its failed-probe null, and re-registers that poisoned advertisement Atomic reference assignment prevents a torn pointer, not stale completion order. I had reasoned about the former and asserted the latter was fine. SingleFlightRefresh serialises publication and coalesces: a request arriving mid-flight sets a rerun flag rather than starting its own pass, so a burst of rejections collapses to at most ONE extra pass, and that pass starts AFTER the request that asked for it -- so the last write is always the newest computation. The rerun flag is cleared BEFORE the work, not after, or a request arriving during a pass would be swallowed. Extracted as its own type for the same reason the certification arms were: the coordination is the risky part and it was untestable inline. 5 tests, including the reviewer's exact scenario (slow-failing overlapping fast-success) and a burst collapsing to one rerun. Mutation-verified: remove the gate so passes can run concurrently and ALL 5 fail. 12 certification-arm tests still green. * fix: Trigger must schedule, not run the delegate inline Codex review round 4. A DISCARDED task is not an asynchronous boundary -- `_ = RequestAsync(...)` still runs the method's synchronous prefix on the caller's stack, up to its first incomplete await. The refresh delegate computes capabilities SYNCHRONOUSLY (it shells out to probe CLI versions) before awaiting anything, and SemaphoreSlim.WaitAsync(0) completes synchronously when the gate is free -- so nothing yielded, and the launch path still ate the whole probe budget. My round-3 claim that the work was off the response path was wrong for the second time in this PR, in the same way: I asserted a boundary existed instead of checking where the first real await was. RequestAsync becomes a non-async void Trigger that wins the gate with the synchronous Wait(0) and hands the pass to Task.Run. The caller cannot inherit the work. That also removes the API asymmetry I raised last round rather than papering over it with a doc comment: there is no task to await, so a future caller cannot be misled by one whose meaning depends on whether they won the gate. An internal Current property exists purely so tests can await quiescence. New test: a delegate that BLOCKS SYNCHRONOUSLY must not delay Trigger. Mutation-verified -- replace Task.Run with an inline call and it fails. 6 single-flight tests, 12 certification-arm tests, check-linear-ids clean. * fix: serialize daemon registration so a stale snapshot cannot land last Codex review round 5. I argued this was a harmless duplicate registration and that reasoning was wrong -- checking the DTO settled it. DaemonConnectAsync reads _config.UnattendedVendorCapabilities INLINE at DTO construction, which happens AFTER an `await MergeRepoPathsAsync()` yield, and nothing serialized two concurrent registrations. So: heartbeat (slot displaced) constructs its DTO with the OLD capabilities the certification self-heal assigns the NEW capabilities and sends its own if the heartbeat's frame is processed last, the server ends up advertising the STALE set while the daemon's local config says otherwise That silently undoes the self-heal. It matters more now than it did before, because this change makes the self-heal the thing that restores a missing advertisement -- so its reliability is load-bearing rather than incidental. DaemonConnect construction AND invocation now run under a registration lock. Held across the invoke deliberately: releasing after construction would let a DTO built from fresher config overtake an in-flight older one, which is the same bug with a smaller window. The race predates this PR. I am fixing it here anyway because this PR is what makes the self-heal load-bearing, and because the fix is contained to one class rather than the cross-subsystem change I assumed it would be when I declined it last round. NOT unit-tested, stated plainly: exercising it needs a hub fake that can pause a registration mid-invoke, and there is no ServerConnection harness in this suite. Verified by reading the ordering. If a harness is wanted, it should be its own change rather than grown inside this one. 6 single-flight tests, 12 certification-arm tests, check-linear-ids clean. * fix: keep the launch-lane probe on its original short budget Qodo finding 4 caught a regression I introduced. The launch-time probe runs on the SEQUENCED COMMAND LANE -- a single serial consumer -- so every later launch and stop queues behind it. Reducing it to one attempt was right, but I also raised the per-attempt timeout from 3s to 10s for BOTH call sites, so the lane stall went from 3s to 10s. The change meant to shorten it tripled it. Registration keeps 3 x 10s (cached for the daemon's lifetime, so a miss there is durable). The launch path is back to a single 3s attempt: a miss there is now correctly classified as transient and retryable, so it never needed the bigger budget. Qodo's 61.5s figure assumed the capability recompute still ran inline on the launch path; it moved to a scheduled single-flight pass earlier in this PR, so the second probe no longer holds the lane at all. Findings 1-3 (comment length): trimmed the probe-budget, ExpectedCliVersion and test-class comments to the non-obvious invariant. Incident history belongs in the PR, which is where it now lives. 12 certification-arm tests, 6 single-flight tests, check-linear-ids clean.
realtonyyoung
added a commit
that referenced
this pull request
Jul 29, 2026
Qodo #3. The two bundled static configs were covered by an [Arguments] test the tripwire could not see, and both reduce to `--codex` / `--claude` there — so deleting the Codex-plugin arm left `--codex` green while one of two INDEPENDENT Codex registration mechanisms went untested. The bundled configs are now a list the coverage assertion can read, compared against what is actually shipped in kcap/. A third bundled config, or a deleted arm, fails. Mutation-tested by removing the .codex-mcp.json entry. 42/42.
realtonyyoung
added a commit
that referenced
this pull request
Jul 29, 2026
…jection (#388) * test(mcp): pin the vendor-capable flows schema across every driver projection Reviewer choice is meant to be a property of the request, not of whichever harness is driving. Nothing enforced that: registration is FOUR mechanisms, not one. Six harnesses (Cursor, Copilot, Gemini, Kiro, OpenCode, Antigravity) converge on one JSON writer and differ only by a shape; Codex writes TOML through a separate engine with its own ownership ledger; Claude Code loads a hand-maintained static kcap/.mcp.json; and Pi gets a hard-coded server list inside an embedded TypeScript bridge. The existing per-harness tests each assert Contains("kcap-flows") in isolation — that a server by that name was written, not that it resolves to the same executable and therefore the same schema. A harness whose registration drifts to a different command leaves a caller believing it named a reviewer when it sent nothing. Adds a conformance suite covering: - vendor is an optional string on both START tools, and on neither of the six follow-up tools (the applied vendor is pinned at start; a vendor there would be ignored or an incoherent mid-run switch); - the vendor/model DESCRIPTIONS carry what the schema cannot — that omitting vendor takes the server default, that the token is canonical lowercase, that there is no silent fallback, and that model requires vendor. This is the only mechanism by which a driver LLM learns to pass the parameter, so a correct schema with a silent description produces exactly the failure the contract exists to prevent; - all nine driver projections resolve to the same `kcap mcp flows`, driven through the real writers rather than asserted against the descriptor; - Pi's bridge still lists flows in its literal — it discovers tools at runtime, so dropping it there is silent; - the two independent copies of the server list (KcapMcpServers vs KcapMcpRegistry) agree, and every canonical server resolves as an allowlist entry. Nothing kept these in sync. Also pins the hand-written harness table against VendorSelection.KnownVendorFlags (now internal), so a tenth installable target fails here instead of quietly being uncovered — no enumeration of supported harnesses exists in production code, the list is spread across four separate string arrays. Mutation-tested: dropping vendor from start_flow, drifting KcapMcpRegistry's args, removing flows from Pi's literal, adding a tenth vendor flag, and changing the canonical flows args each fail their assertion. The last one is instructive — all seven generated arms fail while the two static hand-maintained files pass, which is exactly the generated-vs-static drift this is for. Scratch dirs live under the assembly output, not the system temp root: on macOS /var is a symlink and CodexConfigToml's path guard rejects any symlinked component, so a temp-rooted Codex registration silently returns Failed. (That is also why the pre-existing CodexConfigTomlTests fail locally on macOS.) 26/26. Full Tests.Unit: 42 failures, all pre-existing on macOS and none in this suite. * fix(test): drive the real installers, not a reconstructed projection table Codex review round 1, and the P1 defeated the suite's central claim. The table hard-coded KcapMcpServers.ForCursor and the expected McpConfigShape, while the real choices are wired independently in SetupCommand and each PluginCommand installer. So changing a real arm to omit kcap-flows, use the wrong subset, or use the wrong shape left every projection test green — the test kept invoking its own correct reconstruction. The Codex arm had the same hole, calling RegisterKcapMcpServers directly rather than the install path. Each arm now runs PluginCommand.HandleAsync against a FakeUserHome, seeding the same installed-but-stale state its own PluginCommand*Tests use so `--if-installed` takes the refresh branch. Codex is flagged BareInstall — it installs unconditionally and needs a planted plugin root. Proof it now bites: dropping kcap-flows from the production ForCursor subset fails six arms, where before it failed none. Also from round 1: - argv comparisons were UNORDERED, so ["flows","mcp"] passed while launching nothing. Ordered in both the projection assertion and the two-list check. - the drift check only ran canonical -> registry, so a registry-only server stayed allowlistable-but-never-registered with everything green — one of the exact failure modes the suite claims to prevent. Now compares both name sets (KcapMcpRegistry.AllIds is new) and every server's args. - `model` was pinned by prose only, so retyping it to boolean or promoting it into Required — which would break every caller relying on the vendor's default model — still passed. Type and non-requiredness now pinned, mirroring vendor. The coverage tripwire also stops matching on a display name split on whitespace (which could match by accident) and matches on the flag each arm actually drives. Every fix mutation-tested: production dropping flows, a registry-only server, reversed argv order, and model promoted to Required each fail. 27/27. Full Tests.Unit unchanged at 42 pre-existing macOS failures, none mine. * fix: one definition of each harness's MCP projection, and real path containment Codex review round 2. P1 — the SetupCommand route was outside the gate. `kcap setup` builds its own six Register*Mcp delegates, duplicating the (subset, shape, marker) tuple that PluginCommand also spells out. Mutating SetupCommand.RegisterCopilotMcp to drop flows or use a divergent shape left every installer-driven arm green: a user could get a different tool surface depending on whether they ran `kcap plugin install` or `kcap setup`. Fixed structurally rather than by testing both routes. The tuple now lives once, in HarnessMcpProjections, and both call sites consume it — there is no longer a second definition to diverge. Dropping flows from the shared Copilot projection now fails 2 tests; giving it the wrong shape fails 1. P1 — path containment was broken in three of seven arms, and the test could therefore read and rewrite a developer's REAL harness config, or pass against a pre-existing entry. The Gemini arm cleared GEMINI_HOME, a name GeminiPaths does not read (it honours GEMINI_CLI_HOME); the Codex arm cleared nothing while CodexPaths still gives ambient CODEX_HOME precedence; OpenCode cleared OPENCODE_CONFIG_DIR but left its XDG_CONFIG_HOME fallback live. Every known override is now cleared for every arm — a per-arm list is exactly what was wrong, so there is no per-arm list. Codex also now passes --skip-codex-network-access; a schema test has no business rewriting profile network config. P2 — the status assertion was satisfied by the fallback. FormatStatusResponse catches formatter exceptions and returns the raw JSON body, so "contains claude" passed even if formatting failed entirely. Now asserts the rendered labels and that no raw JSON survives. Mutation-testing that fix found something else: the audit rendering is TRIPLICATED across FormatRoundResponse, FormatStatusResponse and FormatPolledRoundResult, and my first mutant hit the wrong copy. The polled path is the one an agent reads on nearly every flow, and it had no coverage at all — added, and both formatter mutants now fail. 34/34. Full Tests.Unit unchanged at 42 pre-existing macOS failures, none mine. * fix: route removal and ownership through the projection too Codex review round 3. I added HarnessMcpProjection.Unregister and then left it unused — the six PluginCommand remove paths still hard-coded shape and marker, and Kiro's "is the MCP half already installed?" probe constructed `new McpMarker("kiro")` directly. So the single-source claim was only half true: changing a projection made new installs write under one ownership tuple while uninstall looked under the old one (stranding owned entries kcap could no longer see) and Kiro's refresh read an existing install as absent. All six removals now go through the projection, and OwnsAnything moves the probe there for the same reason the marker name is derived rather than passed: a probe reading a different tuple than the writer is the same bug in a third place. `new McpMarker(` no longer appears in PluginCommand at all. Pinned by a per-harness register -> probe -> unregister round-trip asserting the config is left with no kcap entries. Mutation-tested by making Unregister use a different marker name: all six fail. 41/41. Full Tests.Unit failure set byte-identical to the 42-item pre-existing macOS baseline. * test(mcp): make deleting a bundled-config arm fail the coverage tripwire Qodo #3. The two bundled static configs were covered by an [Arguments] test the tripwire could not see, and both reduce to `--codex` / `--claude` there — so deleting the Codex-plugin arm left `--codex` green while one of two INDEPENDENT Codex registration mechanisms went untested. The bundled configs are now a list the coverage assertion can read, compared against what is actually shipped in kcap/. A third bundled config, or a deleted arm, fails. Mutation-tested by removing the .codex-mcp.json entry. 42/42. * docs(test): drop the review narrative from the conformance comments Qodo #1, partially taken. The comments explain why each assertion is shaped the way it is — which is load-bearing here, since three of this PR's review findings were tests that passed vacuously and the rationale is what stops the next reader simplifying them back. That stays, and it matches the surrounding code (SingleFlightRefresh, McpConfigShape, KcapMcpRegistry all document intent at length). What was genuinely historical rather than explanatory is gone: which review round found what, what my earlier attempts did, and past-tense accounts of defects that no longer exist. Rewritten as present-tense reasons. 42/42.
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
…reason (qodo) - FailEnvelopeSourcedLaunchAsync now SetAgentStatus(Failed) BEFORE CleanupAgentAsync disposes the runtime — disposal ends the read loop, whose FinalizeAgentRunAsync would otherwise classify by exit code and emit a Completed/Failed status that races and could MASK the coded launch failure. A pre-set Failed makes that classification a no-op (the same guard the launch-verdict path uses). qodo #3 (finalizer overwrites claim failure). - The rejected-claim reason is now vendor-neutral (no 'codex app-server:' prefix in shared orchestration code). qodo #1 (vendor/driver separation). - qodo #2 (first-turn leak) was already addressed in the prior commit (tear down on a first-turn dispatch failure). Tests 4/4 + 8 forwarding regression still green.
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
…nce (#612) * Codex app-server: orchestrator deferred-first-turn source-claim sequence The daemon driver that wires the hosted-Codex source-claim handshake into the launch lifecycle. Dormant until the activation slice sets deferFirstTurn on the factory (the production factory is unchanged; the fake test runtime exercises the whole sequence). - Factor StartAcpForwardingAsync's post-bind body (liveness/TOCTOU guards, local binding registration, forwarder build+run) into a shared StartForwarderAfterBind, so the source-claim path reuses it WITHOUT re-binding (the claim supersedes AcpSessionStarted). The daemon forwarder ignores the claim's AcceptedSeq — resume is server-driven via the AcpBatchAck, exactly as before. - StartEnvelopeSourcedSessionAsync (fired fire-and-forget after RegisterAgentAsync for a runtime whose RequiresSourceClaimBeforeFirstTurn is true): AcpSessionSourceClaim → (Rejected/method-not-found: coded launch failure + teardown | Bound: start forwarder without re-binding → BeginFirstTurnAsync → confirm). Every step gated on the agent's AcpCts so a finalize aborts cleanly; contains all its own faults. - FailEnvelopeSourcedLaunchAsync = CleanupAgentAsync (single-flight: disposes the runtime, releases the slot, unregisters) + LaunchFailedAsync — the proven outer-catch composition, never FinalizeAgentRunAsync (which classifies a still-live process by exit code). - ConfirmSessionLaunchLoopAsync: background confirm, retrying only transient failures on a paced cadence for the agent's lifetime and NEVER tearing it down (Confirmed/AlreadyConfirmed done; Superseded/NotFound stop; a lost confirm leaves the row provisional for the server's own recovery to settle). - HandleLaunchAgent branches on RequiresSourceClaimBeforeFirstTurn; Cursor keeps the single-phase bind-then-forward path. Tests: AgentOrchestratorSourceClaimTests (3 — happy path claims-then-forwards-without-rebind- then-first-turns-then-confirms, Rejected teardown, method-not-found teardown), asserting through the deterministic AcpCallOrder/signal seams. The 8 existing forwarding tests still green (the factoring is behaviour-preserving). CaptureServerConnection/FakeAcpRuntime/ SpyAcpHostedAgentRuntimeFactory gain the source-claim/confirm/deferred-first-turn doubles. * Codex app-server: contain FailEnvelopeSourcedLaunchAsync teardown faults (Copilot review) It is awaited from the fire-and-forget source-claim task outside any try, so a teardown fault (a cleanup or a LaunchFailed hub error) would escape as an unobserved task exception. Wrap its body in try/catch + log; document the no-throw invariant. * Codex app-server: tear down on a first-turn dispatch failure (Kiro review) A BeginFirstTurnAsync failure after a successful claim previously only logged, leaving a zombie agent holding a slot while the server's Rule-2 expiry closed the provisional row. Tear the agent down instead (symmetric with the claim-failure path, immediate slot release); the provisional row is still closed by Rule 2. New test: first-turn failure → LaunchFailed + unregister + dispose, no confirm. * Codex app-server: mark agent Failed before teardown + vendor-neutral reason (qodo) - FailEnvelopeSourcedLaunchAsync now SetAgentStatus(Failed) BEFORE CleanupAgentAsync disposes the runtime — disposal ends the read loop, whose FinalizeAgentRunAsync would otherwise classify by exit code and emit a Completed/Failed status that races and could MASK the coded launch failure. A pre-set Failed makes that classification a no-op (the same guard the launch-verdict path uses). qodo #3 (finalizer overwrites claim failure). - The rejected-claim reason is now vendor-neutral (no 'codex app-server:' prefix in shared orchestration code). qodo #1 (vendor/driver separation). - qodo #2 (first-turn leak) was already addressed in the prior commit (tear down on a first-turn dispatch failure). Tests 4/4 + 8 forwarding regression still green.
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
- Fix (qodo #4): the coordination-notices capability was injected into `body`, which the transient-POST-failure path also spools — so a replay would make the server mark notices delivered that the replay never renders. Inject into a separate POST-only `postBody`; the spool now keeps the capability-free `body`. New test pins that a spooled session-start body carries no coordination_notices while the live POST still advertises it. - Docs (qodo #1, #5): add a README SessionStart coordination-notices bullet (opt-out + capability-gating) next to the sibling injections, and list disable_coordination_notices in `kcap config set` usage. - Comments (qodo #2): trim narration in ProfileConfig / ClaudeHookCommand / CoordinationNoticesEmitter, keeping the non-obvious rationale. - qodo #3 (move emitter under Harness/Claude/): declined — every sibling SessionStart-fragment emitter lives at src/Capacitor.Cli/ root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 19, 2026
…lane (#609) * [AI-2095] Render SessionStart coordination-notices terminal-delivery lane Client half of the kcap-server U3 coordination-notices lane (server shipped first, capability-gated and inert until this lands). On a live Claude/generic SessionStart the CLI now: - advertises the `coordination_notices: "v1"` capability on the POST body, only on the live path (injected after the ordering-guard/backlog return, so a spooled body never carries it; `kcap import` uses the vendor routes with origin=historical and never reaches here); - renders a returned `coordination_notices: [{text}]` list into the SessionStart additionalContext envelope, next to the team-memory index, via a new CoordinationNoticesEmitter (modelled on SessionGuidelinesEmitter); - honours a new `disable_coordination_notices` profile opt-out that mirrors `disable_memory_index` (Profile field, ConfigCommand set arm, help-config), read from the effective profile so it holds for KCAP_URL users too; when set, the capability is not sent at all (notices stay in the bell / Slack). Additive and fail-open throughout: any parse/build error leaves the hook unaffected. AOT-safe (JsonNode + the existing source-gen Profile context; no reflection). Unit + integration tests cover capability send/omit, render, opt-out suppression and malformed-field fail-open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [AI-2095] Address qodo review: capability-free replay spool + docs - Fix (qodo #4): the coordination-notices capability was injected into `body`, which the transient-POST-failure path also spools — so a replay would make the server mark notices delivered that the replay never renders. Inject into a separate POST-only `postBody`; the spool now keeps the capability-free `body`. New test pins that a spooled session-start body carries no coordination_notices while the live POST still advertises it. - Docs (qodo #1, #5): add a README SessionStart coordination-notices bullet (opt-out + capability-gating) next to the sibling injections, and list disable_coordination_notices in `kcap config set` usage. - Comments (qodo #2): trim narration in ProfileConfig / ClaudeHookCommand / CoordinationNoticesEmitter, keeping the non-obvious rationale. - qodo #3 (move emitter under Harness/Claude/): declined — every sibling SessionStart-fragment emitter lives at src/Capacitor.Cli/ root. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [AI-2095] Drop redundant null-forgiving in CoordinationNoticesEmitter (kiro nit) string.IsNullOrWhiteSpace is [NotNullWhen(false)], so after the guard the compiler already flows `text` as non-null — the `!` was redundant. Still builds warning-free without it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 20, 2026
…ness, catalog delegate - Offer ledger updates now serialize across processes via ConfigFileLock and write through a per-process-unique temp file, so a concurrent hook/setup/ command can no longer overwrite another's change (notably lose a dismissal). (qodo #4) - Load normalizes a null `vendors` member to an empty dictionary, honoring the corrupt-to-empty contract so management commands can't null-deref. (qodo #5) - Claude/Codex wired-checks moved into their own installers (ClaudePluginInstaller.IsPluginEnabled / CodexHooksInstaller.ReferencesKcapHook) and read via a new Core SharedFileText (FileShare.ReadWrite), so probing never blocks an agent writing its own settings.json/hooks.json on Windows. (qodo #2) - Wired-check folded into a per-entry delegate on HarnessCatalog (the single Core registration site); HarnessIntegrationProbe is now thin generic dispatch, no per-vendor switch in shared code. (qodo #1) - `harness` added to Program.cs offline-command allowlist, so `kcap harness list|dismiss|reset` works with no server configured. (qodo #7) - Trimmed predicate comments that restated the code. (qodo #3) - Added null-vendors ledger test. (qodo #6 — "setup stamping fails compilation" — is a false positive: `detected` is CodingAgentsStep.DetectedAgents whose members are bool, and CI's build passed.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 20, 2026
…osed throttle, honest dismiss, snapshot-consistent wiring - Offer-ledger Update takes a lock timeout; the hook/best-effort StampOffered path uses a short 1s wait so a contended ledger can't stall a SessionStart hook past a host's ~5s budget (management commands keep the 5s wait). (Codex P1 #1) - SharedFileText opens with FileShare.Delete too, so a concurrent read can't make the ledger's atomic File.Move-overwrite fail with a sharing violation on Windows (which would silently drop a dismissal). (Codex P1 #2) - TryClaimCheck now fails CLOSED: if the throttle stamp can't be written the same failure blocks the ledger stamp, so returning true would nudge on every hook — suppress instead. (Codex P1 #3) - Update returns whether the change persisted; `kcap harness dismiss|reset` surface a false as exit 1 instead of falsely reporting success (a lost dismissal would otherwise silently revive the nudge). (Codex P1 #4) - Kiro/Pi/OpenCode wired-probes now thread the injected KiroHome/PiAgentDir/ OpenCodeConfigDir overrides (new optional params on the *Paths helpers) so detection and wiring consume the same AgentDetectionInputs snapshot, per the Core API contract. (Codex P2 #5) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 20, 2026
…ck, pure snapshot wiring - On lock-acquire failure Update no longer does a lockless read-modify-write (which could recreate the lost-dismissal race); it returns false without mutating. Hook callers ignore it (best-effort); dismiss/reset already report false as exit 1. (Codex R2 P1 #1) - Hook-path stamping uses a zero-wait (non-blocking) lock try, so a SessionStart hook spends none of its exit-budget safety reserve waiting on the mutex; on contention it simply skips stamping. (Codex R2 P1 #2) - Kiro/Pi/OpenCode wired-probes resolve paths via new pure helpers (KcapAgentJsonPure / KcapExtensionPure / KcapPluginPure) built from the existing *Pure roots, so a null injected override means "unset → home default" and never re-reads ambient env — honoring AgentDetectionInputs' null-as-unset contract and keeping detection and wiring on the same snapshot. Reverted the round-1 non-pure override params (superseded). Added a Kiro override-threading test. (Codex R2 P2 #3) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 20, 2026
* Spec: new-harness detection and setup nudges (#625) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Spec: address Copilot spec-review round 1 findings (#625) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Spec: address Copilot spec-review round 2 findings (#625) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Spec: address Kiro spec-review round 1 findings (#625) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Spec: post-signoff consistency pass, mark reviewed (#625) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * New-harness detection and setup nudges (surfaces 1–2) (#625) Detects, after initial setup, that a supported coding harness is installed but kcap is not wired into it, and prompts to set it up. Implements the spec's PR 1 (independently shippable client surfaces); surfaces 3–4 (daemon/ingest inventory → server notification, and the server-side machine-went-quiet backstop) follow in PRs 2–3. Core substrate: - HarnessCatalog: single Core table binding vendor id + label + install flag + detection selector; the App's AgentVendors re-derives from it, and a conformance test pins it against VendorSelection.KnownVendorFlags. - HarnessIntegrationProbe.IsWired: "is kcap wired into vendor X?", the single source of truth now shared by the kcap status Hooks line (Claude's enabled-plugin and Codex's hooks-reference checks moved to Core; status delegates to them, behavior-preserving). - HarnessOfferLedger/Store: per-machine harness-offers-v1.json (atomic write, corrupt→empty) plus the shared 6h evaluation throttle stamp. - HarnessNudge.Nudgeable: pure predicate (detected ∧ !wired ∧ !declined ∧ past the 7-day re-offer floor). Surfaces: - 1: HarnessNudgeEmitter fragment wired into all 9 hook commands' SessionStart additionalContext, next to the work-items nudge. - 2: HarnessSetupNotice — exit-time interactive stderr notice (TTY-gated, human-facing commands only), mirroring the update-available notice and sharing surface 1's throttle. - kcap status: passive "installed but kcap not configured" line (ledger- independent — always tells the truth). - kcap harness list|dismiss|reset: inspect/silence/re-enable the nudges; all bypass the throttle. - kcap setup stamps offered harnesses so it never re-nudges a vendor just seen at setup; never writes/overwrites a dismissal. - Profile.DisableHarnessNudge (config key disable_harness_nudge) opts out surfaces 1–2 entirely. Tests: catalog conformance, predicate per-vendor, ledger/store (incl. dismissal-preservation), IsWired, emitter (fragment/notice/throttle/fold/ opt-out/exception→null). README + help + config help updated. Closes #625 AI-2118 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address qodo review: locking, shared reads, null-vendors, offline harness, catalog delegate - Offer ledger updates now serialize across processes via ConfigFileLock and write through a per-process-unique temp file, so a concurrent hook/setup/ command can no longer overwrite another's change (notably lose a dismissal). (qodo #4) - Load normalizes a null `vendors` member to an empty dictionary, honoring the corrupt-to-empty contract so management commands can't null-deref. (qodo #5) - Claude/Codex wired-checks moved into their own installers (ClaudePluginInstaller.IsPluginEnabled / CodexHooksInstaller.ReferencesKcapHook) and read via a new Core SharedFileText (FileShare.ReadWrite), so probing never blocks an agent writing its own settings.json/hooks.json on Windows. (qodo #2) - Wired-check folded into a per-entry delegate on HarnessCatalog (the single Core registration site); HarnessIntegrationProbe is now thin generic dispatch, no per-vendor switch in shared code. (qodo #1) - `harness` added to Program.cs offline-command allowlist, so `kcap harness list|dismiss|reset` works with no server configured. (qodo #7) - Trimmed predicate comments that restated the code. (qodo #3) - Added null-vendors ledger test. (qodo #6 — "setup stamping fails compilation" — is a false positive: `detected` is CodingAgentsStep.DetectedAgents whose members are bool, and CI's build passed.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address Codex review: hook-path lock timeout, delete-sharing, fail-closed throttle, honest dismiss, snapshot-consistent wiring - Offer-ledger Update takes a lock timeout; the hook/best-effort StampOffered path uses a short 1s wait so a contended ledger can't stall a SessionStart hook past a host's ~5s budget (management commands keep the 5s wait). (Codex P1 #1) - SharedFileText opens with FileShare.Delete too, so a concurrent read can't make the ledger's atomic File.Move-overwrite fail with a sharing violation on Windows (which would silently drop a dismissal). (Codex P1 #2) - TryClaimCheck now fails CLOSED: if the throttle stamp can't be written the same failure blocks the ledger stamp, so returning true would nudge on every hook — suppress instead. (Codex P1 #3) - Update returns whether the change persisted; `kcap harness dismiss|reset` surface a false as exit 1 instead of falsely reporting success (a lost dismissal would otherwise silently revive the nudge). (Codex P1 #4) - Kiro/Pi/OpenCode wired-probes now thread the injected KiroHome/PiAgentDir/ OpenCodeConfigDir overrides (new optional params on the *Paths helpers) so detection and wiring consume the same AgentDetectionInputs snapshot, per the Core API contract. (Codex P2 #5) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address Codex review round 2: no lockless fallback, zero-wait hook lock, pure snapshot wiring - On lock-acquire failure Update no longer does a lockless read-modify-write (which could recreate the lost-dismissal race); it returns false without mutating. Hook callers ignore it (best-effort); dismiss/reset already report false as exit 1. (Codex R2 P1 #1) - Hook-path stamping uses a zero-wait (non-blocking) lock try, so a SessionStart hook spends none of its exit-budget safety reserve waiting on the mutex; on contention it simply skips stamping. (Codex R2 P1 #2) - Kiro/Pi/OpenCode wired-probes resolve paths via new pure helpers (KcapAgentJsonPure / KcapExtensionPure / KcapPluginPure) built from the existing *Pure roots, so a null injected override means "unset → home default" and never re-reads ambient env — honoring AgentDetectionInputs' null-as-unset contract and keeping detection and wiring on the same snapshot. Reverted the round-1 non-pure override params (superseded). Added a Kiro override-threading test. (Codex R2 P2 #3) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Address Codex review round 3: setup stamping wait, full snapshot purity - StampOffered takes a lockTimeout; the hook path passes TimeSpan.Zero while setup omits it and gets the normal serialized 5s wait, so a momentary concurrent writer no longer makes setup silently skip stamping the offered vendors (which would drop the 7-day floor). (Codex R3 P2 #1) - Copilot/Gemini/Antigravity wired-probes now use pure path helpers (KcapHooksJsonPure / SettingsJsonPure / GlobalHooksJsonPure) built from the existing *Pure roots, so every override the snapshot carries is resolved without an ambient env re-read. Claude/Codex necessarily read CLAUDE_CONFIG_DIR/CODEX_HOME from ambient because those roots aren't part of AgentDetectionInputs (detection is PATH-only for them) — documented as the sole, production-coincident exception on HarnessCatalog. (Codex R3 P2 #2) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fix Antigravity pure hooks path: GUI config root, not data root (#626) GlobalHooksJsonPure built from RootPure (<gemini>/antigravity, the data root), but production GlobalHooksJson resolves through GuiConfigRoot (<gemini>/config), so a normally-wired Antigravity install was probed at the wrong path and reported unwired — spurious nudges while `kcap status` said configured. Add GuiConfigRootPure (<gemini>/config) and build the pure hooks path from it, mirroring production. Add HarnessWiredPathParityTests pinning every pure wiring path to its production layout so a wrong root/segment fails a test. (Codex R4 P1) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
realtonyyoung
added a commit
that referenced
this pull request
Aug 22, 2026
… too Windows CI + qodo review on PR #648: - qodo #3 / Windows CI failure: SafeDirectory used Path.GetDirectoryName, which rewrites/misreads the OTHER OS's separator style (a Unix path run through Windows' Path becomes garbage, and vice versa) instead of the transcript-producing OS's own convention. Replaced with a lexical last-separator split. ExtractClaudePaths' path.StartsWith('/') gate likewise dropped every Windows-absolute path; replaced with IsLexicallyAbsolute, mirroring RepoAttributionMatcher.IsLexicallyAbsolute from the server repo (reimplemented locally, no dependency taken) — Unix-rooted, Windows drive-rooted, or UNC, purely lexical either way. - qodo #2: a tool_use block can sit at an assistant event's top-level content, not only nested under message.content; ExtractClaudePaths now checks both.
realtonyyoung
added a commit
that referenced
this pull request
Aug 22, 2026
…utside a repo (#648) * feat: evidence scanner deriving a git root from tool-input paths * feat: watcher evidence-based repo detection for outside-repo sessions * fix: genericize evidence scanner on completeness + deliver read fallback via final-drain batch Spec review findings on the watcher-integration work: - Scanner latched Done the moment FindRoot found a .git dir, before checking whether the root resolved to a usable owner/repo. A no-remote local repo edited first permanently blocked a later real GitHub repo from attributing. RepoEvidenceScanner is now generic over TRepo with an injected async resolver + completeness predicate; a root is only latched once resolved AND complete, and each distinct root is resolved at most once (cached). - The read-fallback promotion for a clean session end (StopWatcher, no parent-exit) was applied after the final drain's batch had already been sent, so a correctly-computed fallback was never transmitted. Promotion now happens inside DrainNewLines's isFinalDrain branch, before repoToSend is computed, so it rides the same live-hub batch that goes out on every exit that runs a final drain. * feat: import applies evidence-based repo detection to outside-repo transcripts * test: guard slot-B fallback delivery on the final drain Part A whole-branch review, item 1: the final-drain promote-and-deliver path (Finding 1's fix) was verified only by inspection. Extract it into WatchCommand.ApplyEvidenceScanAsync — a small seam over WatchState + the generic scanner, no HubConnection needed — and unit-test with a fake scanner that a read-only line's fallback lands on state.Repository when isFinalDrain is true, and stays untouched when it's false. * fix: catch File.ReadLines' eager path validation in the import evidence scan Part A whole-branch review, item 2: File.ReadLines(session.FilePath) was passed as a call argument to TryBuildEvidenceRepositoryNodeAsync, i.e. evaluated outside that helper's own try/catch. File.ReadLines validates its path argument eagerly (confirmed: an empty path throws ArgumentException synchronously, before the lazy file read), so an invalid path would mark an otherwise-importable session Errored instead of importing without evidence. Add a path-based overload that reads inside its own try and delegates to the existing line-based one; the call site now passes session.FilePath directly instead of pre-evaluating File.ReadLines. * fix: make the evidence scanner OS-independent; read top-level content too Windows CI + qodo review on PR #648: - qodo #3 / Windows CI failure: SafeDirectory used Path.GetDirectoryName, which rewrites/misreads the OTHER OS's separator style (a Unix path run through Windows' Path becomes garbage, and vice versa) instead of the transcript-producing OS's own convention. Replaced with a lexical last-separator split. ExtractClaudePaths' path.StartsWith('/') gate likewise dropped every Windows-absolute path; replaced with IsLexicallyAbsolute, mirroring RepoAttributionMatcher.IsLexicallyAbsolute from the server repo (reimplemented locally, no dependency taken) — Unix-rooted, Windows drive-rooted, or UNC, purely lexical either way. - qodo #2: a tool_use block can sit at an assistant event's top-level content, not only nested under message.content; ExtractClaudePaths now checks both. * fix: read transcripts with FileShare.ReadWrite, not File.ReadLines qodo #1 (High) on PR #648: both new transcript reads (the watcher's one-shot prefix scan, and ImportCommand's path-based evidence overload) used File.ReadLines, which opens FileShare.Read — mandatory-exclusive on Windows, so it can deny the write handle the agent itself still holds on that same transcript mid-flush. Added WatchCommand.ReadLinesShared: a line-yielding sibling of the existing ReadAllTextShared/ReadAllTextSharedAsync helpers (same FileShare.ReadWrite, same rationale, doc-commented in place already), streamed rather than materialized so a scan that attributes early doesn't pay for the whole file. Both call sites now use it; ImportCommand reuses it directly since it's in the same project/namespace.
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
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.
Summary
kapacitor plugin install [--project]andkapacitor plugin remove [--project]commands for standalone plugin managementmarketplace.json(required by Claude Code for directory-type marketplaces) and switching to single-plugin marketplace convention (kapacitor@kapacitor)plugin/tokapacitor/to match plugin nameTest plan
kapacitor plugin installregisters plugin in~/.claude/settings.jsonkapacitor plugin removeremoves plugin entrieskapacitor plugin removeis idempotent (says "not installed" on second run)kapacitor plugin install --projectwrites to.claude/settings.local.jsonkapacitor setupstill installs plugin correctly during step 4/pluginwithin Claude Code🤖 Generated with Claude Code