Sync fork main with upstream - #2
Merged
Merged
Conversation
…#653) * docs(plan): AI-2194 desktop shell Home implementation plan * feat(ipc): advertise supported vendors on the daemon status snapshot * feat(app): remember the chosen harness per repository Add HarnessByRepo member to AppState to persist the vendor harness choice per repository path. Null key means the choice was never made; empty string key ("") holds the choice for the scratch "No repository" target. Tests verify serialization round-trip and null default behavior. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(app): harness catalogue driven by the daemon's advertised vendors Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(app): rename HarnessCatalog to HostedHarnessCatalog, derive vendors from Core Removes duplicated source of truth by deriving the vendor list from Capacitor.Cli.Core.Setup.HarnessCatalog.All instead of maintaining a separate Known array. Transport family (pty/acp/rpc) is now kept in a private map as it's specific to the daemon's hosting strategy, separate from Core's vendor registration which handles installation flags and detection logic. This fixes the name collision that prevented using the new HostedHarnessCatalog class alongside Core's HarnessCatalog without explicit qualification. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * fix(app): remove unnecessary qualification of HarnessCatalog reference Now that the app's harness catalogue is renamed to HostedHarnessCatalog, the unqualified HarnessCatalog reference correctly resolves to Core's HarnessCatalog via the using statement. The workaround qualification is no longer needed. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat(app): launch sessions through the server hub Home needs every vendor, but the daemon's local Spawn frame only resolves claude/codex against its PTY launcher dictionary. The server's RequestLaunchAgentV2 reaches all nine vendors through the runtime factories, so the launch path goes through the hub instead of the local socket. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(app): send RequestLaunchAgentV2 payload keys in snake_case The server applies PropertyNamingPolicy = SnakeCaseLower to every hub payload (kcap-server JsonDefaults.ConfigureSignalRPayload); the client's payload was serializing camelCase keys, so DaemonName and RepoPath would bind null server-side and every launch would fail. Fix both the wire naming (explicit snake_case [JsonPropertyName] on every member, plus the same SnakeCaseLower policy the daemon's own ServerConnection/WatchCommand apply) and the test gap that missed it: LaunchRequestTests now serializes through LaunchHubJson.Configure, the exact JsonSerializerOptions ServerLaunchClient hands AddJsonProtocol, instead of a bare context that only proved the client's own idea of the format. Adds a test pinning the full twelve-key set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(app): Home view-model with per-repository harness memory Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(app): marshal HomeViewModel's daemon projections onto the UI thread Sessions/Harnesses were bound straight off IDaemonClientService's background thread, matching neither MainWindowViewModel nor ConsentPromptViewModel's ObserveOn-before-binding rule. Add RxSchedulers.MainThreadScheduler ObserveOn before SortAndBind/ToProperty, and bring HomeViewModelTests into the AvaloniaSession.WithImmediateRxScheduler / NotInParallel("AvaloniaSession") cohort those schedulers require. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(app): Home surface with repository and harness selection Adds Home as the first MainWindow tab, backed by a new HomeView bound to Task 5's HomeViewModel: goal input, repository/harness chips, remember toggle, Start, and the active-sessions grid. Introduces the design's dark palette as Application-level resources (App.axaml) instead of per-control hex literals. HomeViewModel now implements IDisposable via a CompositeDisposable, matching TrayViewModel/ActivityViewModel, since this task is what first constructs one. * fix(app): wire HomeViewModel through the composition root, make Home the default tab Task 6 fix round: HomeView was inert (no DataContext) and Agents was pinned as the default tab to dodge two smoke-test assumptions instead of fixing them. - App.BuildAndShowMainWindow now constructs HomeViewModel over the same IDaemonClientService instance MainWindowViewModel uses (never a second daemon connection), plus a fresh AppStateStore/ServerLaunchClient — the same cheap-construction pattern BuildLifecycleController already relies on. MainWindowViewModel exposes it as Home; MainWindow.axaml binds HomeView's DataContext to it. App reads Home back off the built window's own DataContext into a new _home field, so BuildAndShowMainWindow's signature (and therefore AppStartupTests' direct call to it) never changes; _home disposes through the same UI-disposables list as _activity/_trayVm/_pause, on both the normal shutdown and startup-failure paths. - Removed the IsSelected="True" pin on Agents so Home is genuinely the default tab, and updated the two MainWindowSmokeTests that assumed Agents opened first to select it explicitly before asserting on its content. * fix(app): correct false claims and thread/lifetime hazards on the Home surface The scratch target's comments claimed a "" repo path launches into a daemon-owned worktree; AgentOrchestrator rejects any repo path that fails Directory.Exists, so the key is storage-only until the daemon accepts a repo-less launch. The concept and its key handling stay. ServerLaunchClient leaked a HubConnection whenever StartAsync threw (the instance was never assigned to _hub) and disposed its gate out from under an in-flight launch. The client is now held by the composition root, shared across window rebuilds, and disposed after Home on both teardown paths. SessionCardViewModel built SolidColorBrushes on the daemon pump thread, which worked only by accident of per-instance dispatcher affinity; ImmutableSolidColorBrush is not an AvaloniaObject, so the four dots are shared rather than reallocated per card per revision. Also fixes the tab comments Home's arrival falsified, and the JSON naming comment in ILaunchClient: an explicit [JsonPropertyName] always beats a policy, and a policy on JsonSerializerOptions does reach source-generated metadata. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(app): cover the daemon-advertised picker, the UI-thread marshalling and the vendor map Three gaps the branch left unfalsifiable. A snapshot narrowing the harness picker was untested end to end (the fake's supportedVendors parameter had no caller). HomeViewModel's ObserveOn before SortAndBind could be deleted with every test still green: the suite pins the scheduler to Immediate and pushes from the UI thread. The new smoke test pushes from a background thread over the session's real scheduler and asserts the bound collection is mutated ON the UI thread — "does not throw" is not falsifiable here, since the push raises nothing and the container still realizes even unmarshalled (a bare VerifyAccess and a control property set from the same thread do throw, so the harness enforces affinity; this path defers its UI work). The transport-family map is hand-written while the vendor list comes from Core, so a tenth vendor would be labelled "chat" silently. The runtime fallback stays — an unknown advertised vendor must still be listed — but the gap is now a red suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(plan): correct the AOT constraint, hub payload shape, and Task 5 type name Three corrections made while executing, so the plan matches what was built: - Capacitor.Cli is the only AOT-published project; the publish gate never compiles Capacitor.App, so it cannot be evidence about app code. - The hub payload is a source-generated record, not an anonymous type. - Task 5 consumes HostedHarnessCatalog; HarnessCatalog is Core's own type. * fix(app): address PR review findings - Thread the shutdown token into HomeViewModel.StartAsync; a launch held CancellationToken.None and could not be cancelled against teardown. - Hold the connection gate across the hub invoke. GetConnectionAsync disposes and rebuilds a connection that is not Connected, so releasing before the invoke let a second launch dispose one still in use. - Compare repo keys the way the filesystem does: case-insensitive on Windows and macOS, case-sensitive on Linux. Applied on read, since System.Text.Json rebuilds the dictionary with an ordinal comparer. - Assert JSON null through JsonElementExtensions.IsNull rather than reading ValueKind directly. - Remove Linear issue IDs from source comments (CI gate). --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* Desktop shell: repository list on Home The repository chip now opens a menu assembled from the two sources AI-2194's plan named — remembered HarnessByRepo keys and distinct agent RepoPaths — plus the current selection, deduped under PathComparer. Each entry shows its remembered harness; "No repository" stays last and separated, and the folder picker survives as "Add repository…". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Merge the daemon's persisted known repos into the repository menu RepoPathStore (repos.json) is what DaemonConnect.RepoPaths feeds the server's launch dialog, so without it the app's menu missed every repo that has no live agent and no locally remembered harness. The source is a required constructor dependency so tests script it rather than read the developer's own config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Resolve linked worktrees to their main repository in user-facing repo lists Agent registrations persist the launch path, and review flows launch into the requester's worktree — so worktree checkouts became "known repos" in repos.json and spread to the server launch dialog, the web UI, kcap repos list, and the app menu (regression of AI-134's rule that repo lists show actual repositories). GitRepository.ResolveMainRepoRoot follows a linked worktree's gitdir to its main root (submodules untouched), falling back to stripping the .claude/.capacitor worktrees patterns for entries whose directory is gone. RepoPathStore applies it on write and collapses on read (newest last_used wins), so historical pollution disappears everywhere with no migration and no server change; the app menu's live-agent source runs through the same helper. Closes kurrent-io#655 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Repository menu: regular-weight entry names Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Fix Windows leg: assert the collapsed store path via GetFullPath On Windows a rootless "/gone/repo" normalizes to "<drive>:\gone\repo", so the raw-string match found nothing — same convention as every other assertion in the class. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…rent-io#640) * Create the first-run flow before opening the browser, and poll it The server's two rendezvous routes shipped with no caller at all, so nothing generated a flow id and the browser's claim-on-arrival was what established ownership - which is where it sat under the retired pairing, and the one property of the design the server half could not realise alone. The leg runs after login, since both routes are authenticated. It generates a 128-bit base64url id, creates the flow, opens {server}/setup?s=<id>, and polls until every step it knows has settled. - Refusals are handled apart: 404/401/403/405 on the create mean the tenant does not serve the flow, and say nothing; 429 reports the server's own Retry-After rather than sleeping through it; 409 retries with a fresh id, since it means the id is taken rather than the credentials wrong. - The poll's decision is extracted and unit-tested per branch. 410 is a dead link, 404 a flow that will never be ours, 401 a re-login rather than a new link, and 5xx or a transport blip is another tick. - Outcomes, never instructions. Step and status strings map onto closed local sets and an unrecognised member is dropped, because kcap setup writes Claude Code hooks and a hook entry is a command string Claude Code runs. Which steps are gates stays the server's to say, via can_finish. - The setup URL is composed locally, so unlike the pairing there is no server-supplied URL reaching a shell-executed open to validate. - Any key ends the wait. The 30-minute budget is the backstop for a terminal nobody is sitting at; a closed tab should not cost half an hour of dots. - Headless is deliberately not a skip - the link is printed as well as opened, which is what keeps the screens available to the device-path population. The leg reports and configures nothing: the screens that would push configuration are their own tickets, and the terminal steps remain what wires the machine up. * Honour caller cancellation and date-form Retry-After in the first-run flow client Qodo review findings on the first-run browser leg: - caller cancellation was swallowed as a transport blip, so Ctrl-C could not stop the poll before its 30-minute budget; rethrow OCE when the caller token is cancelled and degrade only HttpClient's own timeout (same exception type, token unsignalled) - Retry-After was read as delta-seconds only; a proxy rewriting it as an HTTP date was reported as no header at all. Read both forms, measured against the response's own Date header so server clock skew cannot turn the wait negative - trim the retired-pairing narrative from the new docblocks down to the non-obvious constraints, per the comment rule * Address peer-review findings on the first-run browser leg From Alexey's inline comments and Tony's kcap peer review: - the leg now builds its client through the ONE authenticated-client choke point: the bearer is resolved against this server (refreshing if expired, binding-checked) and a mid-poll 401 is recovered by refresh, so a short-lived WorkOS token cannot turn the back half of a thirty-minute wait into a dead sign-in. The token read moved inside the leg's guarded try (the "cannot crash setup" promise now covers it), and a non-Ok auth status gets one line telling the user to re-login - the poll verifies the echoed flow_id exactly as the create path does - the escape hatch stays responsive: the delay is slept in 200ms slices, a keypress during an in-flight poll is noticed right after it, and a keypress that preceded the wait is drained rather than taken as a dismiss - the poll backs off on every unhappy response (honouring the route's Retry-After) and snaps back to the 2s cadence on a good state - an unreadable 2xx create body is reported as unreadable, not as a rejection quoting the success status - the setup URL is reprinted every ~minute so the poll dots cannot scroll the one line a headless machine's user needs to read away - docs: the browser leg's skip list now includes auth provider None - fix the stale rationale on the poll 401 classification * Address the follow-up qodo review on the first-run browser leg - a server-provided Retry-After is honoured as-is, even beyond the 30s cap that still bounds the locally computed doubling; a rate-limited route that asks for 60s is not polled at 30s - the stale-input drain moved to before the "press any key" prompt renders: a key that preceded the leg is still drained, and a key pressed in response to the prompt is a real dismissal, not stale input - the poll loop re-checks the budget deadline after the interval wait, so a sleep crossing the deadline ends the wait instead of issuing one more poll - trim the choke-point comment in the leg down to the non-obvious why * Bound the poll wait by the remaining budget; render the skip line through Spectre From Tony's static review of the latest head: - a server Retry-After longer than what remains of the 30-minute budget no longer sleeps past the backstop: the interval is capped at deadline - now, so a route that asks for an hour cannot hold a keyboard-less host for one - the no-token skip line goes through AnsiConsole so its [dim] markup renders instead of printing literally
kurrent-io#656) * feat: kcap daemon service ensure — the flow's daemon-install ladder Adds 'kcap daemon service ensure': from a fresh status read, install when there is no unit or start when the unit is stopped, baking the born-prompt consent directive on install and gating the start exactly as an app-managed start is. A gate refusal exits with the verify transaction's coded exit plus one start_gate_reason= line, mapped machine-readably to recovery_surface=takeover|reinstall|attention via the pinned ReasonRouting table — never guessed at from prose. On non-launchd the ladder degrades to plain install/start; --json reports verified:false so the flow's copy can say so. Ambiguous states (unknown probe, active transaction, orphan label, stale marker) fail closed to attention with a coded reason. Moves ReasonRouting/RecoverySurface from the retiring Capacitor.App into Capacitor.Cli.Core so the CLI and the app share one pinned mapping (the same rescue shape as AI-2167). Adds --json output (ServiceEnsureJson), pure classifier + failure-map, and unit tests. * docs: record the Windows answer in the AI-2039 design doc * fix: address qodo findings on the ensure ladder - Default the profile to the resolved active one, so a bare 'ensure' on launchd still carries KCAP_PROFILE for the start gate's identity half (matches how Install resolves the pin). - Drift now carries verify_start_gate_drift as its reason alongside the attention surface — the JSON and the human line no longer read empty. - Mark the console-writing dispatch tests [NotInParallel]. * docs: pin the drift wire contract as a gate-family refusal * fix: address peer review on the ensure ladder (7 findings) * fix: make the launchd no-profile refusal a pure predicate (Windows-safe test)
* feat: rescue LoginShellProbe and PathShimInstaller into Capacitor.Cli.Core Move the process seam (IProcessRunner + records + the production ProcessRunner implementation) and the two Avalonia-only setup classes into Core so the flow can drive them after AI-2053 deletes the app. App consumers pick them up via using; the installer's destination-override seam becomes public (Core has no InternalsVisibleTo for the app). Tests move with the classes. * feat: kcap daemon shim ensure — the flow's PATH-fix capability The flow's Agents screen PATH warning offers 'fix it for me' and 'show me the line'. The lane carries values, not paths or commands (retirement spec 6.1), so the fix is a named capability the CLI composes itself: it resolves its own binary path (never a server-supplied one), probes the interactive login shell, and on a positive absence links /usr/local/bin/kcap to itself via the osascript admin prompt, then re-probes so success is never reported on the symlink alone. Unknown probe, filesystem conflict, and non-macOS all fail closed with a coded reason. --json emits the outcome the flow keys off. * fix: address review findings on the shim ensure verb and the Core rescue - Fail closed on a null post-install re-probe (was asserted as a definitive not-on-path diagnosis — the one guess in an otherwise fail-closed ladder). - Add an independent preflight seam so the conflict row is stubbable; the conflict refusal is now covered by a test instead of being untestable. - Reject unknown flags (--help, typos) before any probe or prompt. - Make the isMacOs seam nullable so the off-macOS arm can be forced on a macOS host (a bool default could not distinguish unspecified from false). - Reuse the probe instead of constructing a second one for the installer; make the classifier types internal. - Sanitize control bytes from human console output (the JSON arm is already escaped by System.Text.Json). - Fix stale doc references (ServiceProcess comment, ShimOfferCoordinator wording, installer class doc) and move the README shim section out of the middle of the service prose; design doc no longer cites the unmerged service-ensure sibling branch. * fix: address qodo findings on the shim ensure PR - Share FakeLoginShellProbe via Capacitor.Tests.Helpers instead of duplicating it in the Core and App test suites (the repo rule: cross-suite helpers live in Helpers with a public surface). - Off-macOS refusal (unsupported_platform) now beats an unknown probe in the classifier — the flow expects a stable platform row, not a probe-dependent one — and the daemon usage line lists the reviewer subcommand it dispatches. * fix: address review — preserve the npm launcher and the coded conflict row Two flow-contract corrections from review: - Link the shim to the npm launcher (kcap.js) when this CLI is part of an npm-global install, not to the native binary kcap.js spawned. The launcher is what intercepts 'kcap update' and runs npm; linking the native image would have made /usr/local/bin/kcap update a no-op. The launcher is a sibling package, so its path is derived from the running binary's own location with no environment lookup; a standalone binary links to itself. - Re-preflight after a failed install: the outer preflight and the installer's checks are not atomic, so an entry that appears mid-flight (or a non-forcing ln -s that loses the race) now still surfaces the coded refused/conflict row instead of a generic failed. * Merge main into ai-2167/let-the-flow-fix-a-broken-kcap-path AI-2039's ensure ladder (kurrent-io#656) merged to main after this branch was cut; both PRs added 'using Capacitor.Cli.Core;' to the same app test files, so the merge ref carried the directive twice (position-independent additions — git sees no textual conflict, the compiler does). Drop the duplicates. * fix: platform-neutral launcher-resolution test GetFullPath both sides of the npm-launcher assertion — on Windows a hardcoded POSIX path normalizes against the current drive's root, so the comparison failed on the Windows CI leg.
* Spec: session workspace with terminal
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-1 review findings
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-2 review findings (lifecycle/API seams)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-3 review findings (teardown bounds, outcome classification)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-4 review findings (pre-attach close, writer seam, shutdown latch)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-5 review findings (resolve gate, cause slot, fault mapping)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-6 review findings (intent vs cause, Resolving contract, observed stragglers)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-7 review findings (loser matrix, Detached UX, cancellable callbacks)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-8 review findings (lifetime CTS, diagnostic sink)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-9 review finding (diagnostic sink contract)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Spec: address round-10 review finding (sink guarantee narrowed to containment)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Plan: session workspace with terminal
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Wire: additive has_terminal on AgentStatusDto
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Daemon: stamp has_terminal from EmitsTerminalOutput
SnapshotAgentsForStatus now stamps AgentStatusDto.HasTerminal from
a.Runtime.EmitsTerminalOutput, so every status payload carries whether the
agent's runtime is attachable from a local terminal.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: terminal gate projection on HostedHarnessCatalog
Add three public static methods to HostedHarnessCatalog for projecting
terminal capability decisions:
- FamilyFor: maps vendor tokens to transport families
- ShowsTerminal: daemon's has_terminal flag with family guess fallback
- EffectiveFamily: corrects family when daemon denies terminal on a PTY vendor
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: AgentAttachClient skeleton — handshake, streaming, outcome slot
Adds AttachOutcome (Detached/Exited/Failed/ConnectionLost), the
AgentAttachClient core (dial -> Attach -> read loop -> Attached/
AttachedReadOnly snapshot callback with a resize nudge only for
read-write -> Stdout -> output callback -> Exited/Error -> outcome
via an atomic cause slot), and a scripted Unix-socket test harness
covering the four happy paths. SendInputAsync/ResizeAsync/DetachAsync
are NotImplementedException stubs for Tasks 5-6 to fill in.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: attach client termination semantics and cause slot
Implements DetachAsync/DisposeAsync and completes exception
classification for AgentAttachClient: detach records intent without
claiming the cause slot (except on write failure or no stream), the
pump's frame read is threaded on the internal lifetime token (not the
caller's) so both caller cancellation and Dispose unblock it without
needing to tear down the socket, and callback exceptions are
distinguished from expected cancellation caused by that same token.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: fix attach client CTS ordering and dispose leak from review
Two fixes from task review: (1) make the internal lifetime token an
unlinked CancellationTokenSource so claim-before-cancel on caller
cancellation is structural (one callback, program order) rather than
resting on an undocumented CancellationTokenSource callback-ordering
detail; (2) dispose the lifetime CTS and write-lock semaphore in
DisposeAsync, deferred until after the pump task completes, and guard
DisposeAsync against a second call — the prior version leaked one CTS
plus one registration on the caller's long-lived token per retired
attach.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: attach client outbound invariants
Implements AgentAttachClient.SendInputAsync/ResizeAsync: dropped silently
before a read-write Attached, after AttachedReadOnly, behind a queued
detach, or once _cause is terminal; dimensions outside 1..=ushort.MaxValue
are rejected locally without throwing or sending. A transport-write
failure claims ConnectionLost and closes the socket so the blocked pump
read completes, without the initiating call rethrowing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: attach client diagnostic sink and loser matrix
Fixes the known gap: an exception whose cause-slot claim lost to Exited/
ConnectionLost (not just Detached/CancelledSentinel) was silently dropped by
ReportIfLost's old narrow check. All loser call sites (pump classification,
callback catch, outbound write catch) now report on any genuine loss, gated
directly on the TryClaim result rather than re-deriving win/loss from _cause
afterward; a won claim is reported only when the outcome itself carries no
detail (ConnectionLost from an outbound write). Cooperative cancellation and
Detached-close artifacts remain excluded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Core: fix attach client loser matrix per task review (round 2)
Addresses six review findings on verification integrity:
- Pump's ClassifyPumpException now reports a WINNING transport exception too
(ConnectionLost carries no detail; without this, the daemon dying — the most
common real failure — yielded no errno anywhere).
- Added deterministic pump-side loss and win tests (mutation-verified; the
pump's sink path was previously dead code under test).
- Reworked Concurrent_losers_are_serialized_into_the_sink to force ordering
under proven lock contention instead of an unproven, non-concurrent race.
- Added the missing write-loses-to-already-claimed-Detached ordering test
(mutation-verified via a bounded retry loop, disclosed honestly since the
race can't be forced deterministically without touching the frozen API).
- Strengthened the throwing-sink test to an exact call count and context,
by gating the pump so the write is the only possible producer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Plan: fix merged Task 11 heading
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: terminal packages (direct-pinned), UTF-8 assembler, transcript gate
Adds SvcSystems.UI.Terminal 1.1.1 and XTerm.NET 1.0.16 as both
PackageVersion (Directory.Packages.props) and direct PackageReference
(Capacitor.App.csproj) — central management here has no transitive
pinning, so the direct reference is what actually pins XTerm.NET.
Utf8StreamDecoder (src/Capacitor.App/Services/) is a single incremental
UTF-8 Decoder spanning one attach attempt's snapshot + all live frames:
the terminal control's own byte[] Feed does a fresh GetString per call,
which corrupts a multibyte code point split across PTY frames, so
decoding happens externally and only Feed(string) is used downstream.
TerminalTranscriptTests feeds a recorded ANSI/TUI transcript (SGR color,
cursor addressing, alt-screen enter/leave) through the decoder into a
TerminalControlModel (ReflowOnResize=false, 80x24), chunked at ugly
byte boundaries, and asserts on the XTerm.NET engine buffer directly.
The full discovered API surface (decompiled via ilspycmd, cross-checked
against both packages' READMEs) is recorded as a comment block at the
top of that file for Tasks 10-12 to consume.
License note: both packages are MIT (nuspec license expression on both;
XTerm.NET's own transitive deps Unicode.net and Wcwidth are MIT too).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: pin alt-screen entry and isolation in the transcript test (review fix)
The original TerminalTranscriptTests only asserted IsAlternateBufferActive
at the very end (always false there regardless of whether the switch was
honored) and that "back" was present (fed unconditionally after where
\x1b[?1049l would sit) — both held identically whether the 1049 codes were
interpreted or silently no-op'd, so alt-screen support wasn't actually
exercised.
Splits the feed at the \x1b[?1049h boundary to pin alt-screen ENTRY
(IsAlternateBufferActive == true right after the prefix), keeps the EXIT
pin (== false after the suffix), and adds an isolation assertion that the
alt-buffer's own text ("alt") does not leak into the main buffer once
switched back — the actual proof that two distinct buffer objects are in
play, not just a flag. Red-verified by temporarily stripping the 1049
codes: both the entry and (independently, via a temporary assertion
reorder) the isolation assertion failed exactly as expected, reproducing
the reviewer's own empirical probe, then reverted.
Extends the API-discovery comment block with what these assertions do and
don't prove, so Tasks 11/12 don't inherit the blind spot.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: terminal attach seam and session states
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: TerminalTabViewModel — resolve gate, attempt lifecycle, outcome mapping
Resolve gate (Agents-cache watch + 10s TimeProvider timeout, CAS-linearized
pending/dto-won/timeout-won/disposed) gates NoTerminal/NotFound/Connecting;
attempt lifecycle (generation-checked completions, try-entered single-flight
attach/reattach) drives client swap, read-only suppression, and outcome
mapping (Exited/Failed/Detached/ConnectionLost); TeardownAsync bounds detach
to 1s and the whole teardown to 3s, abandoning rather than blocking on either.
Introduces ITerminalSurface (Task 11 will provide the production adapter) and
FakeTerminalAttachClient/FakeTerminalAttachClientFactory as the scripting
seam for TerminalTabViewModelTests.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: fix TerminalTabViewModel review findings — disposal races, bounded teardown, decoder flush
Closes a Critical: TryStartAttemptAsync now re-checks resolveState/generation
after every suspension point (disposing any client already built before
bailing), never re-reads a possibly-disposed CTS's .Token, and TeardownAsync
no longer disposes the attempt CTS at all — closing both a leaked live client
after teardown and an ObjectDisposedException from a straddling attempt.
Also fixes six Important findings: RetryResolveAsync is now a CAS from
timed-out (not read-then-write); the decoder is flushed at every terminal
outcome so a trailing partial code point isn't silently dropped; TeardownAsync's
detach/dispose/run-task steps all share one 3s remainder instead of an
unbounded DisposeAsync inside the budget; the fake client's DisposeAsync now
terminalizes Result with Detached and supports a gate for deterministic
never-completing-dispose tests; the resolve-driven attach's fault is observed
and rendered as a local Failed instead of leaving the tab stuck in Resolving;
and the NoTerminal note no longer leaks the internal "rpc" transport token.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: xterm surface adapter with terminal-reply lane
XtermTerminalSurface wraps TerminalControlModel (ReflowOnResize=false) and
fans InputProduced in from both TerminalControlModel.UserInput (keyboard)
and model.Terminal.Engine.DataReceived (terminal-generated protocol
replies, e.g. DSR/CPR) -- the latter reachable only through the raw
XTerm.NET engine object per Task 8's discovery, not the SvcSystems wrapper
or the model itself. Resized republishes SizeChanged's Cols/Rows.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: WorkspaceViewModel and WorkspaceView
Header (title/repo/vendor chip + Open-in-web/Stop) and tab strip (Terminal tab or a
family-aware no-terminal note) for a single agent's session workspace, hosting
SvcSystems.UI.Terminal's TerminalControl directly against XtermTerminalSurface's Model
(its own internal resize wiring already satisfies Task 11's obligation) with state
banners for Connecting/read-only/Detached-or-Failed/Exited/NotFound/SessionEnded.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: workspace teardown tracker
Registers and observes async workspace teardowns that the synchronous
disposal pass cannot express. DrainAsync seals atomically and bounds
the wait to 5s via the injected TimeProvider; a faulting teardown logs
once through the diagnostics callback and never poisons the drain or
a sibling teardown. Post-seal Track still executes and observes the
teardown immediately rather than refusing it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: workspace navigation, entry points, shutdown latch
One window, two surfaces: the tabbed shell while CurrentWorkspace is null,
that session's workspace while it is not. MainWindowViewModel owns the
workspace and starts the outgoing one's tracked teardown on every exit —
Back, opening another session, the coordinator's intercepted close-to-hide,
a real close, and the first shutdown pass.
NavigationGate is the app-lifetime piece the composition root owns and every
window shares: a generation that every navigation bumps, and a shutdown latch
that no later window can miss. A launch captures the generation before its
call, so a success landing after the user navigated away opens nothing rather
than attaching an invisible terminal; a Started outcome whose id is not
32 hex digits surfaces as launched-but-unopenable and opens nothing at all.
Shutdown latches navigation on every pass (BeginShutdownPass rule 1's twin),
registering the live workspace's teardown synchronously, and drains the
tracker before quiesce and any disposal — the terminal attach's clamp is
released for other viewers first, not last.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: pin the real-close workspace release
The Closed-handler release in MainWindowCoordinator had no mutation pressure —
deleting it left the suite green. Real close is its own exit path in the spec
(the coordinator discards the window and rebuilds on the next Show), so it now
has a test: real coordinator wiring over a real shown window, an attached
workspace, QuitInProgress set so the close is not intercepted, then Close() —
asserting the window is discarded, the workspace unhooked, exactly one tracked
teardown, and Detach+Dispose on the scripted client. Red-verified by deleting
the release line: the workspace, and its attach, outlive the window.
Also trims the shutdown-drain comment to what omitting ConfigureAwait(false)
actually buys (the drain's own continuation), since the quiesce that follows
can still leave the UI thread.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* App: workspace smoke tests
Headless smoke coverage for WorkspaceView (all nine named controls, the
Terminal-tab/no-terminal-note visibility flip, the Detached reattach banner)
plus a docs/CHANGES.md entry for AI-2195's session workspace terminal
mechanics.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Final fix wave: attach-test race + initial PTY size seam
Fixes two findings from the AI-2195 whole-branch review:
- ScriptedAttachServer.Received is drained by a background pump; two
AgentAttachClientTests asserted on it right after `await run` without
waiting for the pump or locking the read. WaitForReceivedAsync now
carries an internal ~10s deadline (a future regression fails loudly
instead of hanging), and a new SnapshotReceived() gives the two tests a
locked copy to enumerate.
- Every fresh terminal attach nudged the PTY to the phantom 80x24
constant because the real pane size was never known before RunAsync
started (the surface's own resize event fires before WireSurface
subscribes). ITerminalSurface now exposes CurrentSize; a read-write
Attached resends the surface's real size right after State=Attached,
guarded by the existing attempt-generation check.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Rework Item 2: seed RunAsync with the real pane size, not a post-attach resend
A scoped re-review found the previous fix structurally defeated: the
post-attach ResizeAsync fired from inside OnAttachedAsync, but
AgentAttachClient's own post-attach repaint nudge (fired at whatever
size RunAsync started with) writes to the wire right after that
callback returns -- observed order Attach | Resize(137x41) |
Resize(80x24), daemon applies last-write-wins, so the PTY still ended
at the phantom 80x24 on every attach.
Removes that resend entirely. TryStartAttemptAsync now reads the
fresh surface's CurrentSize right after the UI swap dispatch (by then
the view's Model binding and the control's own synchronous
Model-assignment resize have already run) and starts RunAsync at that
real size instead of the DefaultCols/DefaultRows constant, so there is
nothing left for Core's own nudge to race.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Workspace: attach banner (and Detach) visible in both PTY modes
The attach banner previously bound to a read-only-only visibility check,
so the only DetachButton in the view was unreachable from a normal
read-write session -- the in-place Detached/Reattach flow was dead in
the primary flow. Visibility now follows Terminal.State.Phase == Attached
alone; the banner's text/background/border switch on ReadOnly via
dedicated converters (neutral copy and surface/border brushes for
read-write, the existing warning copy and brushes for read-only).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* AgentAttachClient: close the detach/attach race, clamp the initial nudge
Three verified PR-bot findings on the attach lifecycle, plus a comment trim:
- The opening Attach write bypassed _writeLock while DetachAsync's write went
through it, so a concurrent detach could interleave with the multi-part
Attach write and corrupt framing. The Attach write now goes through the
same lock and re-checks _detachRequested under it, settling Detached
without ever writing Attach when intent was already recorded.
- DetachAsync's no-stream-yet path claimed Detached but left an in-flight
dial running; a daemon could still see a real attach after the caller
believed it had detached. DetachAsync now also cancels the internal
lifetime token, and the connect dials on that same token, so a detach
recorded before or during the dial reliably aborts it (or, if the dial
already won, the write-lock re-check above still stops the Attach).
- RunAsync's initial cols/rows were cast to ushort unchecked, silently
wrapping out-of-range values in the repaint nudge; they're now clamped
before use, matching ResizeAsync's own validation.
- Trimmed review-history/version-verification narration from two ViewModel
doc comments, keeping the invariants they document.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fix Windows-only AgentAttachClient test races on RST-discarded Attached reads
On Windows, closing a socket without a graceful shutdown sends RST, which
discards any receive-buffered data the peer hasn't read yet — unlike Unix's
FIN, delivered only after buffered data. Several tests sent the Attached
frame and then immediately (or after a probabilistic delay) severed or
truncated the connection, racing the client's read of that frame. When the
sever won, the client's _attachedAny stayed false and the classifier took
the pre-attach branch (Failed) instead of the intended post-attach branch
(ConnectionLost), failing only on the Windows CI leg.
Production code is correct; this is a test-synchronization fix. Adds an
AttachedObserved TaskCompletionSource (via Recorder or a local TCS for
inline-lambda tests) completed inside the client's OnAttached callback, and
awaits it before severing/truncating in every affected test. Tests whose
expected outcome is unaffected by the race (detach-intent tests that settle
Detached either way, or tests that never sever the connection) are left
unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Workspace QA fixes: normalize launch ids, hide read-write banner, monospace font
The server hub returns a dashed Guid while the daemon cache keys on the
32-hex form — every real launch read as unusable until normalized. The
attach banner overlaid the terminal, so a read-write session now shows
none (owner decision; read-only keeps it — it explains dead keystrokes).
The control's default font is Cascadia Mono, Windows-only: elsewhere it
fell back to estimated cell metrics and smeared styled runs off their
columns, so the host sets a cross-platform monospace stack.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Workspace: visible caret — accent brush and focus-on-attach
The control draws its filled caret only while focused, and the unfocused
fallback is a hairline in the default brush — invisible on this palette.
Nothing focused the control when a session opened, so no cursor showed
at all. Model assignment is the terminal-became-live moment.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Workspace: re-show the caret after snapshot; pass launch ids through verbatim
Two manual-QA findings, both probe-confirmed against the live daemon:
Claude/codex hide the hardware cursor once at stream start and draw
their caret as an inverse-video cell, which the control paints
black-on-black (upstream: both default-color sentinels resolve by the
draw call's isForeground position, not by sentinel value) — the engine
cursor still tracks the TUI caret, so re-showing it once after the
snapshot renders a correctly placed caret. And a production daemon keys
its status cache on SHORT 8-hex agent ids, so the launch guard's
Guid-only rule rejected real launches: Guids normalize to N, everything
else non-blank passes through verbatim, and only a blank id is unusable.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Post-review cleanup: hot-path allocations, wording seam, shared test fixtures
Four parallel cleanup reviews (reuse / simplification / efficiency /
altitude) over the branch diff; applying the surviving findings:
- AgentAttachClient: Stdout frames route through a dedicated
InvokeOutputAsync instead of a per-frame closure over the generic
callback helper — one heap allocation per frame off the hottest path.
- Utf8StreamDecoder: grow-only scratch buffer instead of a fresh char[]
per Decode (delivery is strictly sequential, one reader pump).
- No-terminal wording moved from TerminalTabViewModel.NoteFor into
HostedHarnessCatalog.NoTerminalNote: both VMs now depend downward on
the catalog seam instead of one VM reaching sideways into another.
- Test fixtures deduplicated: one shared FakeTerminalSurface (was five
copies, one under the name SilentTerminalSurface),
AvaloniaSession.RunOnUiAsync (was four copies), and WorkspaceFixtures
holding the AgentStatusDto builder, NewActions and WaitUntilAsync
(call sites unchanged via using static; suites with their own
defaults keep a thin wrapper delegating to the shared builder).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Reference the upstream caret-rendering issue beside the workaround
SvcSystems.UI.Terminal#69, filed today and already fixed upstream in
1.1.2 — the note marks EnsureCaretVisible as a retirement candidate
once a bump past 1.1.2 is verified visually.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Bumps TUnit from 1.65.31 to 1.65.38 Bumps TUnit.Core from 1.65.31 to 1.65.38 --- updated-dependencies: - dependency-name: TUnit dependency-version: 1.65.38 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: tunit - dependency-name: TUnit.Core dependency-version: 1.65.38 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: tunit ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ns (kurrent-io#659) * [AI-2214] Add authoring rules for comments, commits and PR descriptions Ported from kcap-server; the title rule puts [AI-123] in the PR title, which the reference bullet had forbidden. That bullet now governs the description's Linear and GitHub references only, matching what merged PRs already do. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * [AI-2214] Make the PR reference line explicit and distinct from the title prefix The template's placeholder is deliberately not a valid issue link: `#<issue>` closes nothing when a PR ships unedited, where a specimen `kurrent-io#123` would close an unrelated issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Reference the GitHub issue in commit subjects (kurrent-io#658) The PR title omits the reference: squash-merge appends the PR number to it, and a second `(#n)` beside that one reads as another PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urrent-io#664) * Bump SvcSystems.UI.Terminal to 1.1.2 and retire the caret workaround The upstream fix (SvcSystems.UI.Terminal#69, released in 1.1.2) makes inverse-of-default cells render correctly, so the TUI's own inverse-video caret is visible without forcing the engine cursor on. Retires EnsureCaretVisible end to end: the ITerminalSurface member, the XtermTerminalSurface implementation, the TerminalTabViewModel call, its test, and the CaretShown counter on the shared fake. XTerm.NET moves to 1.1.0, the new floor. A TUI that deliberately hides its cursor is no longer overridden once per attach — the principled state now that the control renders what the TUI actually paints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Note the pin move in the API-discovery header The IL-derived notes were taken against 1.1.1/1.0.16; the header now says so explicitly and states how the claims stay proven on the new pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* De-flake the attach detach/dial race test The test deadlocks against its own fixture when the dial wins the race. A graceful detach deliberately leaves the transport open so a terminal frame can still beat Detached, so the client parks in its read loop awaiting the peer -- but the fixture accepted only after awaiting the run, leaving each side waiting for the other until the 5s backstop fired. Accept concurrently instead, and close once an Attach has been observed: the EOF a daemon produces once it holds the frame. AgentAttachClient is unchanged. * Trim the accept comments to the constraint Keep why the accept runs concurrently and why it is bounded; drop the account of how it got that way. * Make the no-attach assertion read the wire The inbound pump ran on the caller's accept deadline and was never joined, so a snapshot could be taken while a frame already on the socket was still undrained -- reading "the daemon never saw an attach" off a pump that had not caught up, which is the one thing this assertion must not do. Bound the accept only, let the pump run to EOF, and expose its completion. The test half-closes so the client settles on its own outcome, then closes it so the pump ends at EOF, and joins before snapshotting.
|
Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters. If you still want a review, comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7383cda9-753b-418c-bdd8-fc5661a3c3d6) |
PR Summary by QodoSync upstream desktop, setup, and daemon enhancements
AI Description
Diagram
High-Level Assessment
Files changed (132)
|
There was a problem hiding this comment.
Pull request overview
This PR fast-forwards the fork to upstream kurrent-io/kcap-cli main (target 9b5f52f…), bringing in upstream feature work across the CLI, Core library, daemon status IPC, and the desktop app (plus substantial new/updated unit tests).
Changes:
- Add a “browser setup” leg to
kcap setupvia new CoreFirstRunflow models/client/polling logic and CLI rendering. - Add/extend machine-readable JSON surfaces and daemon/agent status DTOs (e.g.,
supported_vendors,has_terminal) and wire them through daemon snapshotting and app presentation. - Introduce desktop “workspace + terminal attach” support, plus new teardown/navigation infrastructure and expanded test coverage.
Reviewed changes
Copilot reviewed 133 out of 133 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Capacitor.Tests.Helpers/FakeLoginShellProbe.cs | Shared scripted login-shell probe for tests |
| test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyStartTests.cs | Tests for gate-reason lifecycle behavior |
| test/Capacitor.Cli.Tests.Unit/Services/ServiceVerifyInstallTests.cs | Tests for viability/refusal token surfacing |
| test/Capacitor.Cli.Tests.Unit/Commands/SetupCommandTests.cs | Tests for browser-setup outcome rendering |
| test/Capacitor.Cli.Tests.Unit/Commands/DaemonCommandsServiceEnsureTests.cs | Unit tests for daemon service ensure classification |
| test/Capacitor.Cli.Daemon.Tests.Unit/Services/AgentStatusSnapshotTests.cs | Tests for has_terminal snapshot + serialized payload |
| test/Capacitor.Cli.Core.Tests.Unit/StreamingRunnerTests.cs | Migrate streaming runner tests to Core runner |
| test/Capacitor.Cli.Core.Tests.Unit/ReasonRoutingTests.cs | Rename/relocate reason-routing tests into Core |
| test/Capacitor.Cli.Core.Tests.Unit/ProcessRunnerTests.cs | Migrate process runner tests to Core runner |
| test/Capacitor.Cli.Core.Tests.Unit/PathShimInstallerTests.cs | Update shim installer behavior & test expectations |
| test/Capacitor.Cli.Core.Tests.Unit/LoginShellProbeTests.cs | Move login-shell probe tests to Core namespace |
| test/Capacitor.Cli.Core.Tests.Unit/LocalIpc/StatusIpcJsonTests.cs | Pin new IPC JSON shape and backwards-compat |
| test/Capacitor.Cli.Core.Tests.Unit/LocalIpc/ScriptedAttachServer.cs | New scripted local IPC server for attach tests |
| test/Capacitor.Cli.Core.Tests.Unit/LocalIpc/DaemonStatusDtoTests.cs | Tests for supported_vendors round-tripping |
| test/Capacitor.Cli.Core.Tests.Unit/GitRepositoryTests.cs | Tests for linked-worktree main-repo resolution |
| test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowPollTests.cs | Tests for poll classification logic |
| test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowOutcomesTests.cs | Tests for step/outcome mapping and finish logic |
| test/Capacitor.Cli.Core.Tests.Unit/FirstRun/FirstRunFlowIdTests.cs | Tests for flow-id length/alphabet/uniqueness |
| test/Capacitor.Cli.Core.Tests.Unit/Config/RepoPathStoreTests.cs | Tests for worktree resolution & collapse-on-load |
| test/Capacitor.App.Tests.Unit/WorkspaceViewModelTests.cs | New workspace header/stop/terminal visibility tests |
| test/Capacitor.App.Tests.Unit/WorkspaceTeardownTrackerTests.cs | New teardown tracker tests |
| test/Capacitor.App.Tests.Unit/WorkspaceFixtures.cs | Shared workspace test fixtures/helpers |
| test/Capacitor.App.Tests.Unit/WizardSimpleStepsTests.cs | Update wizard tests for moved Core types |
| test/Capacitor.App.Tests.Unit/Utf8StreamDecoderTests.cs | Tests for incremental UTF-8 decoding |
| test/Capacitor.App.Tests.Unit/ShimOfferCoordinatorTests.cs | Update shim coordinator tests for moved types |
| test/Capacitor.App.Tests.Unit/MutationRequestFactoryTests.cs | Update mutation tests for moved routing types |
| test/Capacitor.App.Tests.Unit/MainWindowViewModelTests.cs | Add navigation/workspace-gate coverage |
| test/Capacitor.App.Tests.Unit/MainWindowSmokeTests.cs | Strengthen tab-selection + workspace swap tests |
| test/Capacitor.App.Tests.Unit/LaunchRequestTests.cs | Pin SignalR wire JSON payload shape/options |
| test/Capacitor.App.Tests.Unit/KcapCliTests.cs | Update app tests for moved Core types |
| test/Capacitor.App.Tests.Unit/HostedHarnessCatalogTests.cs | Tests for advertised vendors & transport mapping |
| test/Capacitor.App.Tests.Unit/FakeTerminalSurface.cs | New terminal surface fake for VM tests |
| test/Capacitor.App.Tests.Unit/FakeTerminalAttachClient.cs | New scripted attach client + factory for VM tests |
| test/Capacitor.App.Tests.Unit/FakeDaemonClientService.cs | Extend fake daemon snapshots for new DTO fields |
| test/Capacitor.App.Tests.Unit/DaemonStepViewModelTests.cs | Update tests for moved Core types |
| test/Capacitor.App.Tests.Unit/DaemonMutationLaneTests.cs | Update tests for moved setup/routing types |
| test/Capacitor.App.Tests.Unit/DaemonLifecycleControllerTests.cs | Remove in-file FakeLoginShellProbe; use shared helper |
| test/Capacitor.App.Tests.Unit/DaemonClientServiceTests.cs | Update tests for moved Core types |
| test/Capacitor.App.Tests.Unit/AvaloniaSession.cs | Add combined UI wrapper helper (dispatch + scheduler) |
| test/Capacitor.App.Tests.Unit/AppStateStoreTests.cs | Tests for per-repo harness preference persistence |
| test/Capacitor.App.Tests.Unit/AppStartupTests.cs | Update app startup tests for moved Core types |
| test/Capacitor.App.Tests.Unit/AppMutationLaneWiringTests.cs | Update wiring tests for moved Core types |
| test/Capacitor.App.Tests.Unit/AgentsImportStepsTests.cs | Update import step tests for moved Core types |
| src/Capacitor.Cli/Services/ServiceVerify.cs | Add in-process evidence fields + propagate refusal tokens |
| src/Capacitor.Cli/Services/ServiceProcess.cs | Update ProcessRunner reference in docs comment |
| src/Capacitor.Cli/Commands/ShimEnsureJson.cs | New JSON DTO + source-gen context for shim ensure |
| src/Capacitor.Cli/Commands/SetupCommand.cs | Add browser setup leg + outcome rendering |
| src/Capacitor.Cli/Commands/ServiceStatusJson.cs | Add ServiceEnsureJson + source-gen support |
| src/Capacitor.Cli/Commands/DaemonCommands.cs | Add daemon shim subcommand + usage update |
| src/Capacitor.Cli.Daemon/Services/DaemonStatusIpc.cs | Include supported_vendors in daemon status snapshot |
| src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.LocalIpc.cs | Stamp has_terminal from runtime capability |
| src/Capacitor.Cli.Core/Setup/PathShimInstaller.cs | Move to Core setup namespace; fail-closed on unknown reprobe |
| src/Capacitor.Cli.Core/Setup/LoginShellProbe.cs | Move to Core setup namespace |
| src/Capacitor.Cli.Core/Resources/help-setup.txt | Document browser setup leg behavior |
| src/Capacitor.Cli.Core/Resources/help-daemon.txt | Document daemon service ensure and daemon shim ensure |
| src/Capacitor.Cli.Core/RecoveryRouting.cs | Move recovery surface + routing into Core |
| src/Capacitor.Cli.Core/Models.cs | Add first-run flow models to source-gen JSON context |
| src/Capacitor.Cli.Core/LocalIpc/StatusIpc.cs | Add supported_vendors + has_terminal to status DTOs |
| src/Capacitor.Cli.Core/LocalIpc/AttachOutcome.cs | New attach outcome discriminated record |
| src/Capacitor.Cli.Core/HttpClientExtensions.cs | Add opt-in 401 refresh/retry for long-running legs |
| src/Capacitor.Cli.Core/GitRepository.cs | Add linked-worktree main-repo resolver utility |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowResult.cs | New result cases for browser setup leg |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowProgress.cs | New progress interface for browser setup leg |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowPoll.cs | New poll classification logic |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowOutcomes.cs | Map wire outcomes to closed local sets |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowModels.cs | Add flow create/poll request/response models |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowId.cs | New base64url CSPRNG flow-id generator |
| src/Capacitor.Cli.Core/FirstRun/FirstRunFlowClient.cs | New HTTP client channel for create/poll routes |
| src/Capacitor.Cli.Core/Config/RepoPathStore.cs | Resolve/collapse worktree paths on add/load |
| src/Capacitor.App/Views/WorkspaceView.axaml.cs | New workspace view code-behind + converters |
| src/Capacitor.App/Views/MainWindow.axaml.cs | Fix comment re: default-selected tab |
| src/Capacitor.App/Views/HomeView.axaml.cs | New Home tab view behavior (menus, click routing) |
| src/Capacitor.App/Views/HomeView.axaml | New Home tab UI markup |
| src/Capacitor.App/ViewModels/WorkspaceViewModel.cs | New workspace VM: header projections + stop routing |
| src/Capacitor.App/ViewModels/SessionCardViewModel.cs | New Home “active session” card model |
| src/Capacitor.App/ViewModels/Onboarding/ShimStepViewModel.cs | Update onboarding shim step for moved Core types |
| src/Capacitor.App/ViewModels/Onboarding/ImportStepViewModel.cs | Update import step for moved Core types |
| src/Capacitor.App/ViewModels/MainWindowViewModel.cs | Add workspace navigation + teardown tracking hooks |
| src/Capacitor.App/Services/XtermTerminalSurface.cs | New production terminal surface wrapper |
| src/Capacitor.App/Services/WorkspaceTeardownTracker.cs | New async teardown registry/drain facility |
| src/Capacitor.App/Services/Utf8StreamDecoder.cs | New incremental UTF-8 decoder for terminal frames |
| src/Capacitor.App/Services/TerminalAttach.cs | New app-side attach seam + session-state types |
| src/Capacitor.App/Services/ShimOfferCoordinator.cs | Update coordinator for moved Core setup types |
| src/Capacitor.App/Services/ServerLaunchClient.cs | New SignalR-based server launch client |
| src/Capacitor.App/Services/Onboarding/WizardLateBinding.cs | Update onboarding wiring for moved Core types |
| src/Capacitor.App/Services/NavigationGate.cs | New shared navigation staleness + shutdown latch |
| src/Capacitor.App/Services/Mutation/MutationRequestFactory.cs | Update mutation request factory for moved routing types |
| src/Capacitor.App/Services/Mutation/MutationModel.cs | Remove duplicated routing; use Core routing |
| src/Capacitor.App/Services/Mutation/DaemonMutationLane.cs | Update lane for moved Core setup types |
| src/Capacitor.App/Services/MainWindowCoordinator.cs | Release workspace on hide/close paths |
| src/Capacitor.App/Services/LaunchHubJson.cs | Shared SignalR JSON payload configuration |
| src/Capacitor.App/Services/KcapCli.cs | Update app CLI service for moved Core types |
| src/Capacitor.App/Services/ITerminalSurface.cs | New app-local terminal surface interface |
| src/Capacitor.App/Services/IProcessRunner.cs | Remove app-local process runner (moved to Core) |
| src/Capacitor.App/Services/ILaunchClient.cs | New server-launch interfaces/payload model |
| src/Capacitor.App/Services/HostedHarnessCatalog.cs | New harness catalog derived from daemon advertising |
| src/Capacitor.App/Services/DaemonLifecycleController.cs | Update lifecycle controller for moved Core setup types |
| src/Capacitor.App/Services/DaemonClientService.cs | Remove nested ProcessRunner implementation |
| src/Capacitor.App/Services/AppStateStore.cs | Add per-repo harness preference storage |
| src/Capacitor.App/Capacitor.App.csproj | Add SignalR + terminal UI dependencies |
| src/Capacitor.App/App.axaml | Add app resources for Home surface palette |
| docs/superpowers/specs/2026-08-24-ai2167-let-the-flow-fix-a-broken-kcap-path-design.md | Add spec doc for PATH shim flow |
| docs/superpowers/specs/2026-08-24-ai2039-daemon-service-ensure-design.md | Add spec doc for service ensure ladder |
| docs/CHANGES.md | Document new workspace terminal behavior |
| Directory.Packages.props | Bump TUnit, add terminal package versions |
| CLAUDE.md | Add/expand contributor guidance (comments/PR template) |
| .github/PULL_REQUEST_TEMPLATE.md | Add PR description template |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+18
to
+23
| public void Track(Func<Task> teardown) { | ||
| var task = ObserveAsync(teardown); | ||
| lock (_lock) { | ||
| if (!_sealed) _pending.Add(task); | ||
| } | ||
| } |
Comment on lines
+32
to
+37
| try { | ||
| while (await FrameCodec.ReadAsync(_stream, CancellationToken.None) is { } f) { | ||
| lock (Received) Received.Add(f); | ||
| FirstFrame.TrySetResult(f); | ||
| } | ||
| } catch { /* connection closed by client — fine for a script */ } |
Comment on lines
+11
to
+20
| /// TerminalHost hosts SvcSystems.UI.Terminal's own TerminalControl directly (no custom host/ | ||
| /// wrapper needed) -- decompile-verified (Task 12 discovery) that TerminalControl already calls | ||
| /// its Model's Resize(width, height, textWidth, textHeight) itself, from BOTH its ModelProperty | ||
| /// change handler (a reattach's fresh Model gets sized to the control's CURRENT bounds | ||
| /// immediately) and its inner surface's own OnSizeChanged (an actual window/pane resize). Task | ||
| /// 11's "the view must call Model.Resize(...) from its bounds-changed handling" obligation is | ||
| /// therefore satisfied by USING the real vendor control rather than by re-implementing what it | ||
| /// already does -- a hand-rolled bounds-changed handler here would only risk double-invoking | ||
| /// Resize with worse (font-metric-unaware) width/height than TerminalControl's own | ||
| /// _consoleTextSize-based computation. |
Code Review by Qodo
1. Queued launch crashes dispatcher
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| /// second launch dispose the very connection this one is still using. Launches are a per-click | ||
| /// UI action, so serializing them costs nothing worth having. | ||
| public async Task<LaunchOutcome> StartAsync(LaunchRequest request, CancellationToken ct) { | ||
| await _gate.WaitAsync(ct); |
There was a problem hiding this comment.
1. Queued launch crashes dispatcher 🐞 Bug ☼ Reliability
ServerLaunchClient.StartAsync awaits the cancellation-aware semaphore before entering its try, so shutdown cancellation of a queued launch escapes instead of returning a LaunchOutcome. HomeViewModel.StartCommand does not catch or subscribe to that exception, causing ReactiveUI's default handler to reschedule an unhandled exception on the still-running dispatcher during shutdown.
Agent Prompt
## Issue description
A launch waiting on `ServerLaunchClient`'s semaphore can be canceled during application shutdown before execution enters the method's exception handler. The resulting `OperationCanceledException` escapes `HomeViewModel.StartCommand` and reaches ReactiveUI's default unhandled-exception path.
## Issue Context
Track whether the semaphore was acquired so `finally` releases it only after successful acquisition. Ensure cancellation during both gate acquisition and launch execution is contained; also defensively keep cancellation from escaping the Home reactive command.
## Fix Focus Areas
- src/Capacitor.App/Services/ServerLaunchClient.cs[19-31]
- src/Capacitor.App/ViewModels/HomeViewModel.cs[225-244]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b5f52fc29
ℹ️ 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".
Comment on lines
+19
to
+21
| var task = ObserveAsync(teardown); | ||
| lock (_lock) { | ||
| if (!_sealed) _pending.Add(task); |
There was a problem hiding this comment.
When a workspace close races shutdown, ObserveAsync(teardown) can reach its first incomplete await before Track acquires _lock; DrainAsync can then seal the tracker and snapshot _pending, after which this task is omitted because _sealed is true. Shutdown may consequently dispose the daemon and launch dependencies without waiting for a teardown that began before the seal, leaving the detach/dispose sequence racing disposed infrastructure. Registration and sealing need a single atomic ordering point.
Useful? React with 👍 / 👎.
| // A linked worktree registers as its main repository: user-facing repo lists show actual | ||
| // repositories, and review flows launching into a requester's worktree must not mint a | ||
| // "known repo" out of it (GH #655). | ||
| var normalized = NormalizePath(GitRepository.ResolveMainRepoRoot(path)); |
There was a problem hiding this comment.
After this change, adding a linked worktree stores its main-repository path, but RemoveAsync still normalizes only the literal path it receives. Thus kcap repos add . followed by kcap repos remove . from that same linked worktree reports “Not found” and leaves the stored main repository behind. Apply the same ResolveMainRepoRoot normalization on removal so the add/remove operations address the same key.
Useful? React with 👍 / 👎.
Comment on lines
+144
to
+145
| preflight ??= target => PathShimInstaller.Preflight(PathShimInstaller.Destination, target); | ||
| if (preflight(target) == ShimPreflight.Conflict) |
There was a problem hiding this comment.
When inspecting /usr/local/bin/kcap fails because of permissions or another filesystem I/O error, PathShimInstaller.Preflight can throw from its FileInfo.LinkTarget/path inspection, and both calls here let that exception escape. The top-level CLI handler then emits a generic error instead of the promised ShimEnsureJson, so a browser-flow invocation using --json receives no machine-readable outcome at all. Contain preflight failures and report a nonzero coded refusal rather than bypassing the command's result contract.
Useful? React with 👍 / 👎.
Comment on lines
+476
to
+478
| var serverUrl = await ResolveServerUrlAsync(profileName); | ||
| if (serverUrl is null) | ||
| return await Report(new ServiceEnsureJson(id, state, "start", "refused", null, "no_server_configured"), 1, json); |
There was a problem hiding this comment.
On Linux and Windows, a stopped service can be started entirely from its already-installed unit, and the plain branch below never uses serverUrl; nevertheless this unconditional check refuses ensure when the current CLI profile/config is unavailable. For example, after the config file is removed or the active profile is switched, kcap daemon service start can still start the unit with its baked environment, while service ensure incorrectly returns no_server_configured. Restrict this prerequisite to the launchd verified branch that actually needs the URL for its identity gate.
Useful? React with 👍 / 👎.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| **Interfaces:** | ||
| - Produces: `DaemonInfoDto.SupportedVendors` — `string[]?`, trailing, default `null`. Vendor tokens exactly as the runtime factories key them (`claude`, `codex`, `cursor`, `copilot`, `gemini`, `kiro`, `opencode`, `antigravity`, `pi`). `null` means "an older daemon that never set it" — consumers must treat that as *unknown*, not *none*. | ||
|
|
||
| - [ ] **Step 1: Write the failing test** |
There was a problem hiding this comment.
WARNING: Stale plan — all 42 checkboxes remain unchecked, but the implementation already shipped in PR kurrent-io#653/kurrent-io#654.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| **Tech Stack:** .NET 10, Avalonia 11 + FluentTheme, ReactiveUI.Avalonia, DynamicData, `Microsoft.AspNetCore.SignalR.Client` (new to `Capacitor.App`, already in `Directory.Packages.props` at 10.0.11), TUnit. | ||
|
|
||
| **Spec:** The design record is the AI-2171 Linear comment (decisions, with the reasoning); the visual reference is the design canvas linked from it. AI-2194 is this slice. |
There was a problem hiding this comment.
WARNING: Spec reference points to an "AI-2171 Linear comment", but no corresponding docs/superpowers/specs/2026-08-23-ai2194-desktop-shell-home-design.md or AI-2171 spec file exists in the repo.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| - Consumes: existing `AgentStatusDto`, `StatusIpcJsonContext`. | ||
| - Produces: `AgentStatusDto.HasTerminal` — trailing `bool? HasTerminal = null`, serialized `has_terminal`, always emitted. Every later task reads this exact member name. | ||
|
|
||
| - [ ] **Step 1: Write the failing tests** (append to `StatusIpcJsonTests`; mirror the file's existing serialize/deserialize helpers — read the top of the file first for its DTO-builder helpers and reuse them): |
There was a problem hiding this comment.
WARNING: Stale plan — all 69 checkboxes remain unchecked, but the implementation is already documented in docs/CHANGES.md under "Session workspace terminal".
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| **Architecture:** One additive wire field (`has_terminal`) gives the app the authoritative Terminal gate; a new BCL-only Core client (`AgentAttachClient`) pumps the existing Attach/Stdout/Stdin/Resize frames with an atomic terminal-cause slot; app-side, a `WorkspaceViewModel` + `TerminalTabViewModel` pair owns the attach lifecycle behind a factory seam, `MainWindowViewModel` gains the first navigation seam (top-level surface swap), and a bounded teardown tracker guarantees socket close on every exit path. | ||
|
|
||
| **Tech Stack:** .NET 10, Avalonia 12.1.1, ReactiveUI/DynamicData, TUnit; `SvcSystems.UI.Terminal` 1.1.1 + `XTerm.NET` 1.0.16 (direct pinned). |
There was a problem hiding this comment.
WARNING: Version drift — plan pins SvcSystems.UI.Terminal 1.1.1 and XTerm.NET 1.0.16, but Directory.Packages.props was bumped to 1.1.2 and 1.1.0 in the same PR.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| ## Global Constraints | ||
|
|
||
| - Branch: `alexeyzimarev/ai-2195-desktop-shell-session-workspace-with-terminal` (already checked out in this worktree; spec committed). |
There was a problem hiding this comment.
SUGGESTION: Branch-specific execution detail (alexeyzimarev/ai-2195-desktop-shell-session-workspace-with-terminal) is frozen into a file that lives on main forever.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| ## Risks | ||
|
|
||
| - **Supply chain**: `SvcSystems.UI.Terminal` (1.1.1) + `XTerm.NET` (1.0.16) are |
There was a problem hiding this comment.
WARNING: Version drift — spec pins SvcSystems.UI.Terminal 1.1.1 and XTerm.NET 1.0.16, but the repo ships 1.1.2 and 1.1.0 in Directory.Packages.props.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| SSO discovery signs in through a `127.0.0.1` browser callback, which a browser on another machine can't reach. So it also offers a **device code**: kcap prints a URL and a short code, and you approve on whatever machine has a browser. Press `d` while the browser sign-in is waiting to switch to it, or pass `--device` up front to skip the browser entirely — the flag works the same way for org SSO and for GitHub. A run whose input is redirected has no key to press, so it goes straight to a device code. `--server-url <url>` remains the way to configure a workspace you already have, and `--github` still routes discovery to the legacy GitHub App path, which is being phased out. | ||
|
|
||
| Once you are signed in, on a server that offers browser setup kcap creates a setup link, opens your browser on it, and waits while you work through the screens there. The link is printed as well as opened, so a machine with no browser of its own can be finished from a browser on another, and any key stops the wait. Setup carries on in the terminal whatever happens in the browser, and is skipped entirely on servers that do not offer it, on servers that need no sign-in (auth provider `None`), and under `--no-prompt`. |
There was a problem hiding this comment.
WARNING: Duplicate browser-setup content — the same explanation appears as a detailed sub-bullet under step 2 (lines 102–113) and again here as a shorter standalone paragraph.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| first shutdown pass latches (which also bumps the generation), so `OpenSession` — card click or launch | ||
| auto-open alike — rejects from then on in every window, current or later-built. | ||
|
|
||
| ## Launch and stop command routing |
There was a problem hiding this comment.
WARNING: Omitted entries — AI-2039 (kcap daemon service ensure) and AI-2167 (PATH shim rescue into Core) were delivered in this PR but have no corresponding ## sections in CHANGES.md.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (8 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash:free · Input: 258.5K · Output: 41.8K · Cached: 6.2M |
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.
Fast-forward the personal fork to current kurrent-io/kcap-cli main.\n\n- Source: upstream/main\n- Target commit: 9b5f52f\n- No personal commits were rewritten\n- No source changes beyond upstream's existing commits