Skip to content

Show a hosted session waiting for input in the desktop app - #790

Merged
alexeyzimarev merged 5 commits into
mainfrom
desktop-awaiting-input
Sep 6, 2026
Merged

alexeyzimarev merged 5 commits into
mainfrom
desktop-awaiting-input

Conversation

@alexeyzimarev

Copy link
Copy Markdown
Member

No tracker reference: neither a GitHub issue nor a Linear id exists for this report.

What & why

The web marks a session whose turn is over; the desktop app lit its needs-you pip only for a failed status or a pending ask, because the daemon's local status payload carried no turn-boundary fact. The daemon now owns an awaiting_input verdict on the agent's activity clock: a turn's falling edge sets it, a rising edge or a delivered input clears it, and the flag rides the local status payload without touching the activity evidence the reaper and the server's idle episodes read. Hosted Claude and Codex, whose PTY silence attests nothing, relay their Stop, prompt-submit and tool-call hooks to the daemon's loopback bridge on a new input-wait route beside the permission one. The app folds the flag into the rail pip for an answerable agent and labels the card and chat footer "Waiting for input".

Where to look

AgentActivityClock.SetTurnInFlight: only a genuine falling edge sets the flag, so a runtime clearing its gate on the way to terminal does not read as waiting. The hook relay runs ahead of every server gate in the Claude and Codex hook commands, on a one-second cap, and only when both KCAP_AGENT_ID and a loopback KCAP_DAEMON_URL are present.

Verification

Suite Result
Core unit 2913 run, 0 failed
CLI unit 4009 run, 0 failed
App unit 1418 run, 0 failed
Daemon unit 3010 run, 1 failed: Installed_codex_schema_matches_the_vendored_pin, the local newer-Codex environmental check

dotnet build Capacitor.slnx: 14 projects, 0 warnings. dotnet publish -c Release of the CLI: 0 IL warnings.

🤖 Generated with Claude Code

The flag rides the agent's activity clock but never advances it: a display hint must not
delay a reap or seed a server-side idle marker. PTY vendors relay Stop and prompt-submit
hooks to the daemon's loopback bridge, since PTY silence attests no turn boundary.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-06T09:00:40.118429Z ba9640e PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Show hosted sessions waiting for input in the desktop app

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Tracks completed turns as daemon-owned waiting state without changing idle or reaping evidence.
• Relays Claude and Codex hook boundaries through an authenticated loopback endpoint.
• Shows answerable waiting sessions in desktop labels, tooltips, and attention pips.
Diagram

sequenceDiagram
    actor User
    participant Hooks as Vendor Hooks
    participant Relay as CLI Relay
    participant Bridge as Loopback Bridge
    participant Daemon as Daemon
    participant Clock as Activity Clock
    participant App as Desktop App
    Hooks->>Relay: Turn boundary
    Relay->>Bridge: POST input-wait
    Bridge->>Daemon: Attributed verdict
    Daemon->>Clock: Set awaiting
    Clock-->>Daemon: Pulse status
    Daemon-->>App: awaiting_input
    App-->>User: Waiting indicator
    User->>Daemon: Send input
    Daemon->>Clock: Clear awaiting
Loading
High-Level Assessment

The daemon-owned verdict is the strongest approach because it unifies runtime-attested and PTY-hook turn boundaries while keeping display state independent from reaping evidence. Inferring waits from PTY silence would be unreliable, and deriving them from server state would fail offline and bypass the desktop's local status path. The nullable trailing IPC field also provides appropriate backward compatibility.

Files changed (26) +640 / -21

Enhancement (13) +206 / -14
ChatTabViewModel.csShow waiting state in the chat footer +1/-1

Show waiting state in the chat footer

• Uses the shared DTO-aware status label so a live session awaiting input displays “Waiting for input” while retaining its running dot.

src/Capacitor.App/ViewModels/ChatTabViewModel.cs

RailSessionViewModel.csAdd waiting sessions to rail attention state +3/-2

Add waiting sessions to rail attention state

• Includes awaiting-input context in tooltips and uses DTO-aware attention logic for the session pip.

src/Capacitor.App/ViewModels/RailSessionViewModel.cs

RailWorktreeViewModel.csAggregate waiting verdicts into worktree attention +1/-1

Aggregate waiting verdicts into worktree attention

• Evaluates complete status DTOs when determining whether any session should light the worktree needs-you pip.

src/Capacitor.App/ViewModels/RailWorktreeViewModel.cs

SessionCardViewModel.csLabel waiting sessions on cards +1/-1

Label waiting sessions on cards

• Uses the shared status label to distinguish an awaiting live process from an actively working one.

src/Capacitor.App/ViewModels/SessionCardViewModel.cs

SessionStatusDots.csCentralize waiting labels and attention rules +11/-3

Centralize waiting labels and attention rules

• Adds DTO-aware status labeling and attention detection. Awaiting input lights the pip only for agent kinds the user can answer, while failures remain attention-worthy.

src/Capacitor.App/ViewModels/SessionStatusDots.cs

StatusIpc.csAdd awaiting_input to local agent status +5/-1

Add awaiting_input to local agent status

• Appends a nullable waiting verdict to AgentStatusDto, preserving compatibility with older daemons that omit the field.

src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs

AgentActivityClock.csTrack awaiting input independently of activity +32/-1

Track awaiting input independently of activity

• Sets waiting on genuine turn falling edges and clears it on rising edges or explicit signals. Changes notify status consumers without advancing activity sequence or idle timing.

src/Capacitor.Cli.Daemon/Services/AgentActivityClock.cs

AgentOrchestrator.LocalIpc.csPublish live agents’ waiting verdicts +4/-1

Publish live agents’ waiting verdicts

• Adds the clock’s awaiting-input state to local status snapshots only while an agent is running.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs

AgentOrchestrator.csWire waiting transitions through the orchestrator +14/-2

Wire waiting transitions through the orchestrator

• Routes attributed bridge reports to agent clocks, pulses local status on state changes, and clears waiting after successful input delivery.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs

LocalPermissionBridge.csAccept authenticated input-wait hook reports +89/-1

Accept authenticated input-wait hook reports

• Adds the Claude and Codex /input-wait route with token validation, bounded body parsing, session validation, attribution, and best-effort delivery to the orchestrator.

src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs

DaemonInputWaitRelay.csRelay hosted hook boundaries to the daemon +35/-0

Relay hosted hook boundaries to the daemon

• Introduces a best-effort, one-second loopback POST carrying agent, session, directory, and waiting state. It no-ops unless daemon URL and agent identity are available.

src/Capacitor.Cli/Commands/DaemonInputWaitRelay.cs

ClaudeHookCommand.csRelay Claude turn-boundary hooks +5/-0

Relay Claude turn-boundary hooks

• Reports Stop as waiting and prompt-submit or pre-tool-use as working before server authentication and recording gates.

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs

CodexHookCommand.csRelay Codex turn-boundary hooks +5/-0

Relay Codex turn-boundary hooks

• Reports Stop, prompt-submit, and pre-tool-use boundaries to the local daemon before disabled and exclusion gates.

src/Capacitor.Cli/Commands/Harness/CodexHookCommand.cs

Tests (12) +406 / -7
ChatComposerTests.csTest chat waiting status presentation +5/-0

Test chat waiting status presentation

• Verifies awaiting input changes the chat text while preserving the running status dot.

test/Capacitor.App.Tests.Unit/ChatComposerTests.cs

RailSessionViewModelTests.csTest session rail waiting attention +28/-0

Test session rail waiting attention

• Covers waiting, working, older-daemon null values, tooltip text, and suppression for protected flow participants.

test/Capacitor.App.Tests.Unit/RailSessionViewModelTests.cs

RailWorktreeViewModelTests.csTest worktree waiting pip transitions +14/-0

Test worktree waiting pip transitions

• Verifies a session’s waiting verdict lights the worktree pip and answering clears it.

test/Capacitor.App.Tests.Unit/RailWorktreeViewModelTests.cs

SessionCardViewModelTests.csTest session card waiting labels +18/-0

Test session card waiting labels

• Verifies waiting cards show the new label with a running dot while ordinary live sessions remain labeled Running.

test/Capacitor.App.Tests.Unit/SessionCardViewModelTests.cs

StatusIpcJsonTests.csPin awaiting_input wire compatibility +36/-7

Pin awaiting_input wire compatibility

• Updates exact JSON expectations and verifies true, false, and omitted-field deserialization behavior for the trailing nullable property.

test/Capacitor.Cli.Core.Tests.Unit/LocalIpc/StatusIpcJsonTests.cs

AgentActivityClockTests.csTest activity-clock waiting transitions +47/-0

Test activity-clock waiting transitions

• Covers falling and rising edges, false terminal-like clears, explicit updates, notification deduplication, and isolation from activity evidence.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentActivityClockTests.cs

AgentOrchestratorInputWaitTests.csTest orchestrator relay handling +31/-0

Test orchestrator relay handling

• Verifies attributed verdicts update the matching agent clock and unknown agent identifiers are dropped.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentOrchestratorInputWaitTests.cs

AgentStatusSnapshotTests.csTest waiting status snapshots and notifications +38/-0

Test waiting status snapshots and notifications

• Ensures only running agents publish true waiting verdicts and turn boundaries immediately advance local status generation.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentStatusSnapshotTests.cs

LocalPermissionBridgeInputWaitTests.csTest the input-wait bridge endpoint +105/-0

Test the input-wait bridge endpoint

• Covers accepted Claude and Codex reports, unattributed events, malformed payloads, unsupported vendors, and invalid tokens.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/LocalPermissionBridgeInputWaitTests.cs

SendInputActivityClockTests.csTest input delivery clears waiting +14/-0

Test input delivery clears waiting

• Verifies successfully handing a message to an agent clears its awaiting-input verdict.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/SendInputActivityClockTests.cs

ClaudeHookCommandTests.csTest Claude hook relay payloads +44/-0

Test Claude hook relay payloads

• Verifies hosted turn events relay waiting state and attribution fields, while unhosted sessions send nothing.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs

CodexHookCommandTests.csTest Codex hook relay payloads +26/-0

Test Codex hook relay payloads

• Verifies hosted Stop and prompt-submit events post the expected waiting verdict and normalized session identity to the daemon bridge.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CodexHookCommandTests.cs

Documentation (1) +28 / -0
CHANGES.mdDocument the hosted-session waiting state +28/-0

Document the hosted-session waiting state

• Explains the daemon-owned verdict, PTY hook relay, activity-clock invariants, compatibility behavior, and desktop presentation rules.

docs/CHANGES.md

@qodo-code-review

qodo-code-review Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Terminal prompts leave sessions waiting ✓ Resolved 🐞 Bug ≡ Correctness
Description
ClaudeHookCommand clears the verdict for user-prompt-submit, but the installed Claude
configuration routes that event only to set-title-prompt.sh rather than the command containing
this relay. When a user submits directly through the hosted terminal, the preceding Stop remains
recorded during the next turn and indefinitely for text-only turns that never invoke PreToolUse.
Code

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[R81-82]

+        if (command switch { "stop" => true, "user-prompt-submit" or "pre-tool-use" => false, _ => (bool?) null } is { } waiting)
+            await DaemonInputWaitRelay.NotifyAsync("claude", sessionId, cwd, waiting);
Relevance

●●● Strong

Installed hook configuration must invoke every relay branch; otherwise the advertised prompt-submit
clearing behavior is ineffective.

PR-#760

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The command contains the new prompt-submit clear branch, but the installed hook table proves that
Claude never invokes it for that event; only Stop and PreToolUse run the command.

src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[66-82]
kcap/hooks/hooks.json[59-68]
kcap/hooks/hooks.json[82-91]
kcap/hooks/hooks.json[104-113]

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

## Issue description
Claude's `UserPromptSubmit` event does not invoke `ClaudeHookCommand`, so terminal-submitted prompts cannot clear `AwaitingInput`.

## Issue Context
The current plugin configuration assigns `UserPromptSubmit` only to the title script, while Stop and PreToolUse invoke `kcap hook --claude`.

## Fix Focus Areas
- kcap/hooks/hooks.json[104-113]
- src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs[79-82]

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


2. Agents can spoof peers' waiting state ✗ Dismissed 🐞 Bug ⛨ Security
Description
HandleInputWaitAsync accepts the daemon-wide token and gives the body-provided agent_id to an
attribution ladder that prioritizes it over session and working-directory evidence. Any hosted agent
possessing the shared bridge URL can name another live agent and set or clear that peer's displayed
waiting verdict.
Code

src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[R963-964]

+        var attributed = AttributeHandler?.Invoke(new PermissionAttribution(Str(node, "agent_id"), sessionId, Str(node, "cwd")));
+        if (attributed is { } agent) InputWaitHandler?.Invoke(agent.AgentId, waiting.Value);
Relevance

●● Moderate

Potential shared-token attribution vulnerability is serious, but no closely matching accepted or
rejected precedent was found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The bridge generates one shared URL and supplies it to hosted agents, while the new endpoint accepts
that token, trusts caller-controlled attribution fields, and mutates whichever live agent the
existing ID-first ladder selects.

src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[146-158]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs[195-202]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[872-900]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[903-907]

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 input-wait route allows a shared-token caller to select any live target through the request body's `agent_id`.

## Issue Context
Hosted agents receive the same daemon bridge token, while attribution trusts a matching raw agent ID before checking session or working-directory evidence. Bind credentials to an agent or require all supplied attribution evidence to agree before applying the verdict.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[920-964]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[872-907]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs[195-202]

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



Remediation recommended

3. Flow sessions show a user-facing wait ✓ Resolved 🐞 Bug ≡ Correctness
Description
SessionStatusDots.Label returns Waiting for input for every DTO carrying the flag even though
NeedsAttention explicitly excludes protected kinds that cannot be answered by the user. When a
running review or flow participant ends a round, its card and rail tooltip describe a
user-actionable wait despite the participant actually waiting on the flow.
Code

src/Capacitor.App/ViewModels/SessionStatusDots.cs[34]

+    public static string Label(AgentStatusDto dto) => dto.AwaitingInput == true ? "Waiting for input" : dto.Status;
Relevance

●●● Strong

Directly contradicts the PR’s stated flow-participant protection requirement and causes incorrect
user-facing status text.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The daemon emits the flag for all running kinds, the application defines every non-agent kind as
protected, and only the pip—not the newly added labels—uses that protection rule.

src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs[62-65]
src/Capacitor.App/Services/AgentActionService.cs[57-61]
src/Capacitor.App/ViewModels/SessionStatusDots.cs[26-34]
src/Capacitor.App/ViewModels/RailSessionViewModel.cs[46-48]
src/Capacitor.App/ViewModels/SessionCardViewModel.cs[22-29]

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

## Issue description
Protected flow participants display `Waiting for input` even though users cannot answer them and their attention pip is deliberately suppressed.

## Issue Context
Apply the same protected-kind rule consistently to status labels and rail tooltips, or use distinct wording for flow-owned waits.

## Fix Focus Areas
- src/Capacitor.App/ViewModels/SessionStatusDots.cs[26-34]
- src/Capacitor.App/ViewModels/RailSessionViewModel.cs[46-48]

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


4. Malformed hook calls return server errors ✓ Resolved 🐞 Bug ☼ Reliability
Description
HandleInputWaitAsync accepts any successful JsonNode.Parse result and then indexes it as an
object when reading session_id and waiting. A hook body such as [], true, or a JSON string
throws during that indexing and reaches the outer exception handler, which returns 500 instead of
rejecting the caller with the route's intended 400 response.
Code

src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[R954-955]

+        var sessionId = PermissionWire.Canonical(Str(node, "session_id"));
+        var waiting   = node?["waiting"] is JsonValue verdict && verdict.TryGetValue<bool>(out var w) ? w : (bool?) null;
Relevance

●●● Strong

Malformed JSON shapes can throw before validation; repository precedents favor explicit fail-closed
request validation.

PR-#304

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new handler catches only parse failures, then accesses named members on the parsed node without
first requiring an object. Exceptions from that code are caught by the enclosing request handler,
which explicitly emits HTTP 500.

src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[946-966]
src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[712-720]

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 input-wait route parses scalar and array JSON successfully, but then treats every parsed node as an object. Validate that the parsed value is a `JsonObject` before reading named fields, and return HTTP 400 for any other JSON shape.

## Issue Context
This route is documented to refuse bodies from which it cannot read a session and verdict. Non-object JSON currently instead throws and is converted by the outer handler into a 500 response.

## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[946-960]
- src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs[712-720]

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



Informational

5. Two hook tests get conflicting locks ⊘ Outdated 📘 Rule violation ≡ Correctness
Description
hosted_turn_boundaries_relay_to_the_daemon_bridge, an_unhosted_stop_relays_nothing, and
Hosted_turn_boundaries_relay_to_the_daemon_bridge add bare [NotInParallel] metadata inside
classes already marked [NotInParallel("AuthProviderDiscoveryCache")]. When these cases run, they
inherit the keyed class constraint alongside the bare method constraint, leaving the scheduler with
contradictory isolation declarations while the tests capture console output or mutate process-wide
environment variables.
Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[105]

+    [Test, NotInParallel]
Relevance

● Weak

Recent repository precedents reject bare method constraints alongside keyed class-level
NotInParallel attributes.

PR-#768
PR-#721

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 2821213 prohibits applying [NotInParallel] at both class and method level.
ClaudeHookCommandTests.cs has the keyed class-level attribute at line 20 and newly added bare
method-level attributes at lines 105 and 133, while CodexHookCommandTests.cs has the keyed
class-level attribute at line 13 and the newly added bare method-level attribute at line 576.

Rule 2821213: Avoid conflicting [NotInParallel] attributes at method and class level
test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[20-21]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[105-105]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[133-133]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CodexHookCommandTests.cs[13-14]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CodexHookCommandTests.cs[576-576]

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

## Issue description
Three newly added hook tests have bare method-level `[NotInParallel]` attributes while their containing classes already have keyed `[NotInParallel("AuthProviderDiscoveryCache")]` attributes, creating conflicting class- and method-level isolation declarations.

## Issue Context
Compliance rule 2821213 prohibits applying `[NotInParallel]` at both class and method level. The bare isolation is needed because the tests capture console output or mutate process-wide environment variables, so preserve that method-level isolation while removing the class-level conflict through suitable test organization or metadata changes rather than weakening isolation.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[20-21]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[105-105]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/ClaudeHookCommandTests.cs[133-133]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CodexHookCommandTests.cs[13-14]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CodexHookCommandTests.cs[576-576]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 58 rules
Review mode: 🧠 Deep: This is a broad, behavior-changing cross-layer feature spanning daemon state transitions, IPC/security-sensitive loopback routing, CLI hooks, and desktop UI, with many independent logic sites where redundant review could catch subtle defects.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/Harness/ClaudeHookCommand.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs
Comment thread src/Capacitor.App/ViewModels/SessionStatusDots.cs Outdated
Comment thread src/Capacitor.Cli.Daemon/Services/LocalPermissionBridge.cs

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba9640e80f

ℹ️ 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".

// residual: a full ACP _pendingTurns queue drops input silently without throwing, so this
// can advance on a delivery that was actually dropped — kill-delaying only, accepted.
agent.ActivityClock.Advance();
agent.ActivityClock.SetAwaitingInput(false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Clear the wait flag for locally attached input

When the desktop composer or terminal answers a hosted Claude session, its bytes travel through AttachClientLoopAsync to SendRawInputAsync (AgentOrchestrator.LocalIpc.cs:340-345), not through this server-command path, so this clear never executes. The fallback hook does not cover that path either: the bundled Claude UserPromptSubmit entry runs only set-title-prompt.sh (kcap/hooks/hooks.json:104-110), not ClaudeHookCommand. Consequently, after a Stop has set the flag, the desktop continues to show NEEDS YOU while Claude processes the next prompt—indefinitely for a text-only response, or until the first PreToolUse for a tool-using response. Clear the flag after successful raw-input delivery or register the relay for Claude's prompt-submit hook.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8b98172: AttachClientLoopAsync clears the flag after a successful raw-input delivery whose bytes carry the submit key. Both the composer (paste envelope, then \r) and a terminal keystroke end up there, so this is the clearing edge for every locally driven PTY session regardless of which hooks are installed. A keystroke that submits nothing leaves the flag alone; both cases have tests.

Comment on lines +81 to +82
if (command switch { "stop" => true, "user-prompt-submit" or "pre-tool-use" => false, _ => (bool?) null } is { } waiting)
await DaemonInputWaitRelay.NotifyAsync("claude", sessionId, cwd, waiting);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Start the Claude hook budget before awaiting the relay

For a hosted Claude hook whose loopback bridge accepts a connection but stalls, this await can consume the relay's full one-second timeout before the five-second HookBudget is created on line 84. The installed PreToolUse hook itself has a hard five-second timeout (kcap/hooks/hooks.json:59-67), while subsequent exclusion work is allowed to consume the newly started full five-second budget, so Claude can terminate the process before a deny/ask decision is emitted. Include the relay in the existing hook budget (or reserve its time from that budget) so the new display hint cannot shorten the policy-decision window.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The budget was already counting: HookBudget.Remaining is the ceiling less HookClock.Elapsed, and the clock starts when the process does, so the relay's second was never in addition to the five. The window it could narrow is real, though, so in 8b98172 the relay is capped at the lesser of one second and the budget's remaining time and skipped once that is gone; An_exhausted_hook_budget_skips_the_relay pins it.

A local client's input reaches a PTY agent as raw bytes on the attach socket, never through
the server's send path, and the plugin's Claude prompt-submit hook does not run kcap, so the
submit key is the one clearing edge the desktop composer and terminal have.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@alexeyzimarev

Copy link
Copy Markdown
Member Author

Review round addressed in 8b98172. The relay tests moved into their own classes (ClaudeHookInputWaitRelayTests, CodexHookInputWaitRelayTests) so each carries a bare NotInParallel without shadowing the main suites' keyed one. Suites after the round: core 2913, CLI 4010, app 1418, all with 0 failed; daemon 3013 with the one local newer-Codex vendored-pin failure.

alexeyzimarev and others added 3 commits September 6, 2026 19:26
…sktop-awaiting-input

# Conflicts:
#	docs/CHANGES.md
#	src/Capacitor.App/ViewModels/SessionCardViewModel.cs
#	test/Capacitor.App.Tests.Unit/SessionCardViewModelTests.cs
Codex can complete a turn before the send that started it returns, and a PTY submit spray
runs for seconds, so the clear after a delivery must yield to a wait that began during it.
A subagent's tool hook inherits the parent's agent id but is not the parent's turn.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A PTY vendor relays only Stop, so a wait nothing cleared sees the next turn end as a second
Stop with the flag already set; it must still outrank the delivery in flight.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant