Repository navigation
Support a typeable agent terminal and local fork bases in the daemon - #1314
Conversation
Teardown keeps the agent registered while it disposes the runtime, so the PTY master's close is deferred behind a SafeHandle refcount: a write racing dispose can no longer land on a reused fd.
PR Summary by QodoSupport hosted terminal input and local fork base refs
AI Description
Diagram
High-Level Assessment
Files changed (13)
|
Code Review by Qodo
1. Dropped keystrokes appear delivered
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| // Advertises the SendRawInput handler. False from a daemon predating it; the server then | ||
| // never sends raw input. | ||
| bool SupportsRawInput = false, | ||
| // A launch's base ref may name a local branch, which the daemon resolves on its own machine | ||
| // before falling back to a fetch. False from a daemon predating it, whose fetch-only path | ||
| // cannot see a branch that exists only here. |
There was a problem hiding this comment.
3. Daemon capability notes retain release history 📘 Rule violation ⚙ Maintainability
The DaemonConnect comments describe compatibility as coming from daemons that “predate” the capability fields instead of stating only the current wire behavior. These historical qualifiers are attached to the newly added capability documentation and will become stale as supported daemon versions change.
Agent Prompt
## Issue description
The new `DaemonConnect` comments use historical wording about daemons predating the fields, which violates the requirement to keep comments focused on current behavior rather than change history.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Models.cs[2195-2200]
## Recommended Fix
Rewrite the comments to describe the observable protocol behavior directly: unsupported daemons omit or send false capability flags, and the server does not send raw input or local branch-base requests when the capability is unavailable. Remove the historical “predating” wording.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
An older daemon still connects without this field, so naming what its absence means is a live compatibility constraint, which the comment rules keep.
| public readonly record struct SendRawInputCommand( | ||
| string AgentId, | ||
| string Data, | ||
| Guid DispatchId | ||
| ); |
There was a problem hiding this comment.
5. Pr description omits the required issue link 📘 Rule violation § Compliance
The description's first reference line says AI-3173 — no GitHub issue for this work. and contains neither a GitHub closing keyword with an issue number nor both references on one line. Reviewers therefore cannot use the description to identify a linked GitHub issue for this change.
Agent Prompt
## Issue description
The PR description does not contain a single reference line combining a GitHub closing keyword, a GitHub issue number, and the Linear issue key.
## Fix Focus Areas
- src/Capacitor.Cli.Core/Models.cs[2099-2103]
## Recommended Fix
Add a dedicated first-line reference such as `Closes #1234 AI-3173`, using the actual GitHub issue number and retaining the Linear key on the same line. Do not put the two references on separate lines or in the PR title.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
There is no GitHub issue for this work; the reference line names the Linear key only, as the template allows when one half does not exist.
| if (!_agents.TryGetValue(cmd.AgentId, out var agent)) { | ||
| LogSendRawInputUnknownAgent(cmd.AgentId, _agents.Count); | ||
| return; | ||
| } |
There was a problem hiding this comment.
1. Dropped keystrokes appear delivered 🐞 Bug ≡ Correctness
HandleSendRawInput returns on rejected payloads or unavailable agents and only logs unsupported runtimes, without reporting the command's DispatchId through SendInputRejectedAsync. When any of those conditions drops a dispatch, the server has no rejection report, unlike the existing structured-input path.
Agent Prompt
## Issue description
Raw-input commands carry dispatch IDs but are silently dropped for unknown, private, closing, malformed, empty, or unsupported destinations.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4489-4518]
- src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[4087-4121]
## Recommended Fix
Report each failed raw-input dispatch using the existing bounded rejection mechanism and an appropriate drop reason; cover the drop paths in tests.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Raw input is fire-and-forget like special keys: the server keeps no pending state for a raw dispatch that a rejection could retire, so a report would change nothing until the server's burst recording learns to take one back.
| VendorModels: _config.VendorModels); | ||
| VendorModels: _config.VendorModels, | ||
| SupportsRawInput: AdvertisesRawInput, | ||
| SupportsBranchBaseRef: true); |
There was a problem hiding this comment.
11. Forks cannot use local branches 🔗 Cross-repo conflict ≡ Correctness
BuildDaemonConnect advertises SupportsBranchBaseRef, and CreateAsync can resolve a supplied local branch, but kcap-server has no corresponding capability field and its ordinary launch construction never supplies BaseRef. Until the server selects a capable daemon and sends the source branch as the launch base, browser-initiated forks cannot exercise this new path.
Agent Prompt
## Issue description
The daemon can start from a local branch, but the pinned server does not send one for hosted-agent launches.
## Fix Focus Areas
- src/Capacitor.Cli.Daemon/Services/ServerConnection.cs[675-677]
- /cross_repos/kcap-server/src/Capacitor.Api.Public.Abstractions/Agents/Daemon/DaemonConnectArgs.cs[75-83]
- /cross_repos/kcap-server/src/Capacitor.Server/Sessions/AgentStoreDataService.cs[1490-1515]
## Recommended Fix
Update kcap-server to retain the branch-base capability and, for a fork, pass its source branch through `LaunchAgentCommand.BaseRef` only when the selected daemon supports it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
The paired kcap-server PR #2244 keeps supports_branch_base_ref and sends a fork's source branch as BaseRef only to a daemon that advertises it, refusing the fork otherwise.
The server's flow-participant check fails open once a run ends, so the daemon refuses keys and raw input to review flows itself; PR-review agents still take typing. A failed PTY start disowns the handle before closing the fd, or its finalizer would close a reused number.
|
NO FINDINGS — reviewed the raw-input daemon path, PTY lifetime handling, and local fork-base selection on the current head. I did not build or run tests. |
2 participants
AI-3173 — no GitHub issue for this work.
What & why
The server's redesigned agent page lets a viewer with Full access type straight into a hosted agent's terminal, and fork a session from any turn. The daemon now takes raw keystrokes through a new single-record
SendRawInputhub method, advertised by a trailingsupports_raw_inputon connect. It also starts a fork's worktree from a local commit or a local branch before falling back to fetching origin, advertised bysupports_branch_base_ref.Where to look
WorktreeManager.CreateAsync: a hub-supplied base ref is checked withgit check-ref-format --branch(exact echo, no leading-) before any git call or directory is created; that also closes an option injection into the origin fetch.UnixPtyProcess: the master fd moved behind a SafeHandle, so input that arrives while an agent is being torn down is dropped instead of written to a closed, possibly reused fd.Verification
Reverting the PTY disposal guard makes both new disposal tests fail: 4 and 38 bytes land in the file that reused the closed fd. Ran live against a dev server with the daemon in Docker: typed into a hosted Claude agent's terminal from the browser, and a branch-head fork started from the source agent's local branch head rather than the default branch.