[alexeyzimarev]

Summary

The PermissionRequest hook previously only recorded the permission event (or long-polled for the decision in rendered-agent mode) and never checked whether the transcript watcher was alive. Since permission requests fire frequently mid-session, this PR adds a best-effort, idempotent watcher self-heal there — the most frequent mid-session recovery point, complementing the existing notification/stop re-ensure.

This recovers sessions that get stuck active (because session-end is never POSTed) when an abruptly-killed agent orphans its watcher or the watcher self-kills.

Changes

• PermissionRequestCommand.TryEnsureWatcher(...) calls the idempotent WatcherManager.EnsureWatcherRunning (PID-file liveness check, spawn only if dead). Wired into Handle before the rendered/non-rendered split so both the interactive and daemon-hosted long-poll paths benefit.
• Main session only: a present agent_id marks a subagent tool call, whose watcher uses a distinct key ({sessionId}-{agentId}) + transcript and is ensured at subagent-start. Mirrors how notification/stop only re-ensure the main watcher.
• Best-effort: wrapped in try/catch; never throws into the hook path and does not delay the permission decision (spawn is a sub-second Process.Start).

Tests

New integration tests (mirror WatcherLifecycleTests: own KCAP_WATCHER_DIR, no Console capture):

• spawns the watcher when a main-session transcript is present
• skips when agent_id is present
• skips when transcript_path is missing

All pass; existing permission/watcher suites pass; AOT publish clean (no IL3050/IL2026 warnings).

No README change — internal self-heal, no user-facing CLI surface change.

Closes #209
AI-1085

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Self-heal transcript watcher on permission-request hook

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Ensure the main-session transcript watcher is running on each permission request.
• Skip self-heal for subagent tool calls (agent_id) and when transcript_path is missing.
• Add integration coverage for spawn vs skip scenarios using KCAP_WATCHER_DIR.

Diagram

graph TD
  A["PermissionRequestCommand.Handle"] --> B["TryEnsureWatcher"] --> C{"agent_id present\nOR transcript_path missing?"}
  C -- "Yes: skip" --> D["Continue hook flow\n(rendered/non-rendered)"]
  C -- "No: ensure" --> E["WatcherManager.EnsureWatcherRunning"] --> F[("{sessionId}.pid file")] --> G["Watcher process"]
  subgraph Legend
    direction LR
    _cmd["Command/Hook"] ~~~ _dec{"Decision"} ~~~ _fs[("PID file")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Server-side session liveness/expiry

• ➕ Recovers stuck "active" sessions even if no further hooks fire
• ➕ Avoids spawning logic in hook handlers
• ➖ Requires server changes and careful tuning to avoid prematurely ending active sessions
• ➖ Does not restart local watcher; only mitigates symptoms

2. Periodic local heartbeat/monitor for watcher

• ➕ Centralizes self-heal logic outside individual hooks
• ➕ Can detect failures without depending on permission-request frequency
• ➖ Adds a new always-on background component with its own lifecycle issues
• ➖ Harder to integrate with diverse agent execution environments

Recommendation: Current approach is a pragmatic, low-risk recovery point: permission requests are frequent mid-session, EnsureWatcherRunning is already idempotent, and the change is best-effort (won’t block or fail the hook). The main-session-only guard avoids interfering with subagent watcher keys. Server-side expiry could complement this later but is broader in scope.

Files changed (2) +136 / -0

Bug fix (1) +36 / -0

PermissionRequestCommand.csEnsure main-session watcher runs during permission-request handling +36/-0

Ensure main-session watcher runs during permission-request handling

• Invokes a new best-effort TryEnsureWatcher() before the rendered vs non-rendered split. The helper skips subagent calls (agent_id present) and missing transcript_path, otherwise calls WatcherManager.EnsureWatcherRunning and swallows errors to avoid impacting permission decisions.

src/Capacitor.Cli/Commands/PermissionRequestCommand.cs

Tests (1) +100 / -0

PermissionRequestWatcherSelfHealTests.csAdd integration tests for permission-request watcher self-heal +100/-0

Add integration tests for permission-request watcher self-heal

• Adds NotInParallel integration tests using KCAP_WATCHER_DIR to verify watcher spawning for main sessions and skipping for subagent payloads or missing transcript_path. Cleans up spawned watchers and temporary transcript files.

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccb4bbff24

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Respect exclusions before spawning the watcher

When a Claude session is in a configured excluded_paths/excluded_repos scope, ClaudeHookCommand dispatches permission-request to this handler before it reaches the profile exclusion checks later in that file, so this new self-heal can start a transcript watcher for a session that session-start intentionally skipped. That means the next permission prompt in an excluded project can begin uploading transcript data despite the user's exclusion settings; the self-heal needs to run only after applying the same exclusion/disabled-session gates or be guarded by them here.

Useful? React with 👍 / 👎.

[alexeyzimarev]

Good catch — fixed in b6ed302. The permission-request dispatch runs before the excluded_repos/excluded_paths gates, so the self-heal now applies the same exclusion check first and only spawns the watcher when the session isn't excluded (selfHeal = !IsSessionExcludedAsync(...) passed into PermissionRequestCommand.Handle). The permission record/long-poll itself is left unchanged, since hosted agents need the decision regardless of exclusion. Extracted the gate into a shared ClaudeHookCommand.IsSessionExcludedAsync so both call sites can't drift, and added unit tests for it.

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Self-heal the transcript watcher from the permission-request hook

1. Watcher ensure adds latency ✗ Dismissed 🐞 Bug ➹ Performance

Description

PermissionRequestCommand.Handle now awaits TryEnsureWatcher, so when the watcher is dead the
permission-request hook will block on process/PID resolution + Process.Start + PID-file IO before it
can return/long-poll. This increases hook latency compared to existing non-blocking watcher ensures
in other hook flows.

Code

src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[R37-42]

+        // Self-heal the transcript watcher (see TryEnsureWatcher). Done before the
+        // rendered/non-rendered split so both the interactive path and the daemon-hosted
+        // long-poll recover a dead or never-started watcher; idempotent, so a no-op when
+        // one is already running.
+        await TryEnsureWatcher(baseUrl, sessionId, node);
+

Evidence

The permission-request handler awaits the watcher ensure before selecting the rendered/non-rendered
path. Other commands (e.g., Gemini) explicitly fire-and-forget EnsureWatcherRunning to avoid
blocking hook handling.

src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[37-50]
src/Capacitor.Cli/Commands/GeminiHookCommand.cs[222-236]

Agent prompt

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

### Issue description
`PermissionRequestCommand.Handle` awaits `TryEnsureWatcher(...)`, which can add avoidable latency to the permission-request hook when the watcher needs to be spawned.

### Issue Context
Other hook handlers start/ensure watchers in a non-blocking way (fire-and-forget) so hook responses aren’t delayed by best-effort watcher work.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[37-42]
- src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[91-109]

### Suggested fix
Start the self-heal without awaiting it (and ensure exceptions are observed). For example:
- `_ = TryEnsureWatcher(baseUrl, sessionId, node);`
Optionally wrap with a short timeout inside `TryEnsureWatcher` (or pass a CancellationToken) so a pathological spawn/path lookup cannot hang the hook path.

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

---

2. Fragile JSON field parsing ✓ Resolved 🐞 Bug ☼ Reliability

Description

TryEnsureWatcher uses GetValue<string>() on optional fields (agent_id, transcript_path, cwd); if any
of these are present-but-null or non-string, it throws and the catch logs an error and skips
self-heal. This reduces recovery effectiveness on slightly malformed/variant payloads and can spam
stderr in a frequently firing hook.

Code

src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[R91-105]

+    internal static async Task TryEnsureWatcher(string baseUrl, string sessionId, JsonNode node) {
+        try {
+            if (node["agent_id"]?.GetValue<string>() is { Length: > 0 }) {
+                return;
+            }
+
+            var transcriptPath = node["transcript_path"]?.GetValue<string>();
+
+            if (string.IsNullOrEmpty(transcriptPath)) {
+                return;
+            }
+
+            var cwd = node["cwd"]?.GetValue<string>();
+
+            await WatcherManager.EnsureWatcherRunning(baseUrl, sessionId, transcriptPath, agentId: null, cwd: cwd);

Evidence

TryEnsureWatcher currently uses GetValue<string>() for optional fields. Other hook code includes a
TryGetString helper that safely returns null instead of throwing when fields are
absent/null/not-a-string.

src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[91-108]
src/Capacitor.Cli/Commands/CursorHookCommand.cs[251-259]

Agent prompt

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

### Issue description
`TryEnsureWatcher` reads `agent_id`, `transcript_path`, and `cwd` using `GetValue<string>()`, which throws when the JSON node is null or not a string.

### Issue Context
Elsewhere in the CLI, optional string extraction is done via `JsonValue.TryGetValue<string>(...)` helpers to avoid exceptions and preserve fail-open behavior without noisy logs.

### Fix Focus Areas
- src/Capacitor.Cli/Commands/PermissionRequestCommand.cs[91-105]

### Suggested fix
Replace `GetValue<string>()` calls with safe parsing:
- `static string? TryGetString(JsonNode? node, string field) { if (node?[field] is JsonValue v && v.TryGetValue<string>(out var s)) return s; return null; }`
Then use it for `agent_id`, `transcript_path`, and `cwd`, so null/non-string values cleanly act like "missing" without throwing/log-spam.

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

---

3. Test env var not restored ✓ Resolved 🐞 Bug ☼ Reliability

Description

PermissionRequestWatcherSelfHealTests sets KCAP_WATCHER_DIR in SetUp but TearDown clears it to null
instead of restoring any preexisting value. This can break test isolation when the test process
starts with KCAP_WATCHER_DIR already set.

Code

test/Capacitor.Cli.Tests.Integration/PermissionRequestWatcherSelfHealTests.cs[R18-31]

+    [Before(Class)]
+    public static void SetUp() {
+        Directory.CreateDirectory(TempDir);
+        Environment.SetEnvironmentVariable("KCAP_WATCHER_DIR", TempDir);
+    }
+
+    [After(Class)]
+    public static void TearDown() {
+        Environment.SetEnvironmentVariable("KCAP_WATCHER_DIR", null);
+
+        try { Directory.Delete(TempDir, recursive: true); } catch {
+            /* best effort */
+        }
+    }

Evidence

The new integration test sets KCAP_WATCHER_DIR to TempDir and later sets it to null. The unit tests
demonstrate an existing pattern that preserves and restores the previous env var value.

test/Capacitor.Cli.Tests.Integration/PermissionRequestWatcherSelfHealTests.cs[18-31]
test/Capacitor.Cli.Tests.Unit/PermissionRequestCommandTests.cs[82-93]

Agent prompt

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

### Issue description
The integration test fixture overwrites `KCAP_WATCHER_DIR` and later unsets it, losing any prior value.

### Issue Context
Some tests in the repo use a small scope helper pattern that saves the previous env var value and restores it on dispose.

### Fix Focus Areas
- test/Capacitor.Cli.Tests.Integration/PermissionRequestWatcherSelfHealTests.cs[18-31]

### Suggested fix
Store the previous value in a static field during `SetUp()` and restore it in `TearDown()` (instead of setting null), or introduce/reuse an `EnvVarScope`-style helper so the fixture is self-contained and doesn’t leak env var changes across tests.

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

---