Daemon lifecycle management + PATH shim (desktop supervisor) - #542
Merged
alexeyzimarev merged 65 commits intoAug 12, 2026
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Post-install verification with rollback (name-lock TOCTOU), ownership- branched takeover handoff with stop verification, four-state service status JSON with install_binary_path provenance, valid-profile gate with --profile/KCAP_PROFILE pinning, login-shell PATH injection, incompatible- daemon skew trigger via hello version, lstat-checked non-forcing shim, serialized lifecycle gate + app-state store, IProcessRunner stdout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Startup-phase eligibility shared with the shim latch, ownership-verified post-mutation checks (job_pid==daemon_pid + fresh attach), hardened service status (launchd query without plist, job/daemon pids) and uninstall (bootout failure propagated, plist retained), gated kickstart with post-start verification, branch-specific failure recovery from re-queried evidence, env-overlay + kill-tree/abandon-wait process semantics, same-binary (not app-managed) classification with universal recapture disclosure, (reason,version) incompatible dedupe, -lic sentinel terminal-PATH probe gating silent mutations, dialoged reinstall affordance reusing takeover machinery, POSIX-escaped sudo fallback, atomic claim-before-show app state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shutdown defers through mutation verification/rollback (detach-on-cancel scoped to detached start), service stop/start hardened to bootout/ bootstrap (signal-only stop cannot stop a lock-losing KeepAlive job), uninstall distinguishes benign label absence, orphan-label rows keyed on job state before plist presence across startup/Start/takeover, daemon_pid live identity-validated via IsOurDaemon, unknown-PATH gate narrowed to unit-writing mutations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ownership verification and rollback move into the CLI as a --verify transaction on install/start (crash/force-quit safe), stopped units auto-start on relaunch via bootstrap (stop->relaunch contradiction), tri-state launchd classification with status --json failing on unknown, per-label cross-process flock closing the uninstall TOCTOU, install preconditions before any destructive step, no-live-owner branch skips daemon stop, unknown PATH no longer closes the startup phase, startup reconciliation query on every path surfaces crash residue. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verify predicate gains readiness (local-control hello + version + final ownership recheck) since the PID file lands before the host builds; takeover collapses into one CLI transaction (install --replace --verify) so app death between commands cannot strand a removed daemon; install's initial bootout is classifier-gated (no write on loaded/unknown); a durable txn marker + broadened startup reconciliation catch mutation- process death; command-layer lock scope with mixed-version residual documented; verified-safe failure states replace "exact pre-op state"; closed-stdio tolerance for the npm grandchild topology. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Decision 6 amended (user-approved): NameInUse exits 0 so a cross-machine name collision rests stopped instead of respinning past any verification window. Distinct fixed-namespace service flock + phase-recording durable marker with fingerprint-scoped recovery authority and txn_active liveness, fsync ordering, explicit --replace ownership matrix with an internal raw-kill helper below StopByName, two-phase deadline with a reserved rollback budget and bounded ServiceProcess, final on-disk fingerprint recheck, per-verb one-shot hello probe contract, viability scoped to install/replace only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Supervised local lock-refusal joins NameInUse in the exit-0 deliberate- refusal family (plain non-verify service verbs cannot spin), and read-only status probes the transaction flock non-blockingly so txn_active is actually observable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implement Task 1 of AI-1654 slice: add tri-state LabelProbe classification for launchd service discovery, rich ServiceQuery record with unit presence and job pid, and Query method across all three IServiceManager implementations. - Add LabelProbe enum (Loaded/Absent/Unknown) to classify launchctl results - Add ServiceQuery record with probe, unit presence, state, binary path, job pid - Add Query method to IServiceManager interface - Implement LaunchdUnit.ClassifyPrint: tri-state on exit code + stderr signature - Implement LaunchdUnit.PidFromPrint: extract job pid from launchctl print output - Implement Query in LaunchdServiceManager: full probe + optional plist read - Implement Query in SystemdServiceManager: minimal mapping of state to probe - Implement Query in WindowsScheduledTaskServiceManager: minimal mapping - Add LaunchdClassifyTests: 5 tests covering classify/pid parsing edge cases All LaunchdClassifyTests pass; systemd and Windows managers delegate to existing Status method for backward compatibility. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move DaemonCommands' private ReadPidFile/IsOurDaemon PID-file validation into Capacitor.Cli.Services.DaemonPidProbe and add the public ValidatedPid seam (spec §3.4 daemon_pid). daemon stop delegates its ownership+liveness check to it; start/status/doctor call the moved primitives directly so their finer-grained (absent vs. unparseable vs. stale) messaging and exit codes stay byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
StopByName routed its live/stale classification through DaemonPidProbe.ValidatedPid, which re-reads the PID file. A concurrent `daemon start` between the outer ReadPidFile check and that re-read could hand the kill branch a PID this call never validated from the same snapshot. Bind entry once from ReadPidFile and call IsOurDaemon(entry.Pid, entry.StartToken) directly for the live/stale branch and the kill target, matching the pre-refactor single-read behavior exactly. ValidatedPid stays as the seam for new callers (e.g. status --json), not for StopByName. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Call DaemonLockPaths.EnsureDirectory() in TryAcquire to prevent DirectoryNotFoundException masquerading as contention - Wrap lock acquire in try/finally to release on assertion failure - Add regression test for missing directory creation Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds a machine-readable status verb (spec §3.4) wiring the tri-state launchd Query, validated daemon PID, and service-txn lock probe added in Tasks 1-3. The renderer refuses to mask an Unknown launchd probe as not_installed, exiting non-zero with no JSON instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds ServiceTxnMarker (temp-file + rename + FlushToDisk before and after the rename) so a resumer can read the last recorded phase of an in-flight install/replace/start. Directory-entry durability is best-effort on .NET, which has no portable directory fsync; the marker content itself is torn-proof. Wires status --json's txn_marker through ServiceTxnMarker.Exists instead of a raw File.Exists check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Read and Delete used typed catch clauses that let an unexpected exception type propagate, breaking the "null on missing/corrupt, never throws" contract. Match the sibling DaemonRestartMarker.TryRead/Delete pattern with a bare catch. Also documents that ServiceTxnMarker does no locking itself (callers serialize per-serviceId writes via ServiceTxnLock), and adds a test that actually exercises Delete's swallow path (deleting a marker path that is a directory, which File.Delete cannot remove). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Spec §3.4: a non-zero launchctl bootout is not automatically a failure - re-query with launchctl print and treat LabelProbe.Absent as an idempotent success (delete the plist), while Loaded/Unknown retain it and fail. Changes IServiceManager.Uninstall to bool Uninstall(string, out string? error) across all three managers and callers; the daemon-service command case now wraps the call in ServiceTxnLock.TryAcquire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LaunchdServiceManager.Uid() P/Invokes libc's getuid, which has no Windows implementation; the new Uninstall tests would fault on the Windows CI leg. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Stop now issues `launchctl bootout` instead of `kill SIGTERM`, since a signal cannot stop a KeepAlive job caught between short-lived incarnations; the plist is never deleted. Start probes the label first (print + ClassifyPrint) and issues `bootstrap` when unloaded or `kickstart` when already loaded, failing without mutating on an ambiguous probe. IServiceManager.Start/Stop now return bool + out error; systemd/Windows managers wrap their existing behavior. Both CLI verbs acquire the per-service ServiceTxnLock, matching uninstall. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 8 of AI-1654: ServiceProcess.RunBounded tree-kills and awaits on timeout (draining stdout/stderr on background tasks to avoid a pipe-buffer deadlock), and HelloProbe does a single dial+Hello+reply against a daemon's local control socket without LocalControlClient's status/1 capability gate, since start-verify must accept a capability-incompatible hello. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ServiceVerify runs marker -> mutate -> ownership+readiness poll -> final recheck -> commit, or rollback to the verified-safe unloaded state, per spec §3.4. Injectable manager/pid-probe/hello/TimeProvider seams make every outcome drivable without launchctl. start --verify accepts any well-formed hello (no capability/version gate) and performs no viability check. Wires into `kcap daemon service start --verify`, which now runs the engine (the engine acquires ServiceTxnLock itself) instead of the plain start path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…art (review round 1) manager.Start's own error was silently discarded before — the readiness poll is still the source of truth for pass/fail, but the reason is worth Say-ing if it never recovers. The command layer's post-success print is now IOException-tolerant like the engine's own Say, so a closed stdout on an already-successful verified start can't crash the process. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…cation (review round 2) The confirmation recheck shared the same forward deadline as the primary readiness check, so a slow-but-genuine primary hello landing right at the deadline starved the recheck (remaining <= 0 => auto-fail without probing), rolling back a daemon just verified healthy. The recheck now gets a floor of one poll interval instead of being budget-starved by the same clock. Rollback's restore verification was a single-shot re-query after Stop, which can race a launchctl bootout that returns before the label is actually gone. It now polls (bounded by the previously-unused rollbackReserve) the same way the forward phase does. Stop's own error is now surfaced via Say, matching the existing treatment of Start's error. FakeServiceManager now records every verb in argv-order (List<string> Calls) per the brief, rather than bare counters, so rollback tests can assert Start precedes Stop and the restore Query keeps polling after Stop. Two new tests cover the recheck floor and a predicate-holds-once-then-fails-recheck case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements InstallVerifiedAsync for replace:false: lock -> leftover-marker self-heal (trivial committed case only; anything else surfaces as RestoreVerification, Task 11 owns full resumption) -> viability -> pre-query classify (Loaded -> Contended, Unknown -> BootoutUnknown, since a fresh install must never clear an existing label) -> marker through captured/written/bootstrapped -> WriteAndBootstrap -> poll for hello version match + ownership -> floored final recheck that also fingerprints the on-disk plist against the marker, so a foreign writer between bootstrap and recheck is detected and never deleted -> committed -> Ok. WriteAndBootstrap is new on IServiceManager: write + bootstrap with no leading bootout, since the engine (not the manager) now owns the bootout decision on the fresh path. Systemd/Windows delegate it mechanically to their existing Install; launchd's goes through the injectable _runProcess (unlike Install's static ServiceProcess calls) so it's testable. Also aligns Start's rollback reserve-expiry exit to RollbackBudget (26) instead of RestoreVerification (27), matching the pinned distinction that 26 is a timeout and 27 is an affirmatively-observed wrong state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d races, and non-launchd use (review round 1) Critical: manager.WriteAndBootstrap/GenerateFiles could throw (EPERM under MDM, launchctl I/O errors) and escape uncaught to Program.cs's top-level guard, leaving an orphaned marker and — when WriteAndBootstrap had already written the plist — an unregistered file on disk. Both mutation steps are now wrapped: a GenerateFiles throw is pure (nothing written) and just clears its own marker; a WriteAndBootstrap throw runs the same fingerprint-gated InstallRollback as every other failure path. Critical: the pre-mutation classifier keyed only on the label, so a stopped-but-installed service (Probe=Absent, UnitPresent=true — `service stop` retains the plist by design) was treated as a fresh Absent slot, overwritten, and then deleted by any later rollback. Absent-with-UnitPresent now joins Loaded under Contended, matching the README's stated contract. Important: threaded the manager-provided GeneratedFile.Path through the final recheck and both rollback paths instead of a hardcoded LaunchdUnit.PlistPath call, so the engine has no launchd-specific dependency; gated `--verify` to LaunchdServiceManager at the command-wiring layer (GenerateFiles().Single() and the recheck assume one generated file, which only launchd currently satisfies). Important: 26 vs 27 now turns on the LAST observation at reserve expiry, not just "reserve expired" — Unknown is a genuine timeout (26), but a still- Loaded (or file-still-present) last observation is an affirmatively wrong state (27) even though the reserve also ran out. Fixed both the start and install rollback loops and the start-path test whose own scenario (stays Loaded) was asserting the wrong code. Important: extended the closed-stdio try/catch to the whole `service install` success tail, not just its first line — a broken pipe on any of the trailing Log/Stop/Remove/reviewer-notice lines was still able to turn an already-successful install into a crash. Added coverage for all of the above plus the previously-untested RollbackBudget(26) install path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adds DaemonKill.KillValidatedOwner (a raw, seam-free kill of an already-validated owner PID, mirroring StopByName's exception handling) and wires it into ServiceVerify.InstallVerifiedAsync's --replace path: an ownership matrix that clears/takes over an existing label, stopped-but-installed unit, or manual (non-service) owner before writing the new one, gated by a stop-confirmation poll (StopUnconfirmed on timeout). Also replaces the placeholder entry-time marker recovery with full content-scoped recovery (fingerprint match cleans up the dead transaction's own residue; mismatch surfaces rather than paves over it). --replace is wired through DaemonCommands (requires --verify). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Self-review surfaced that the --replace matrix trusted manager.Uninstall's success unconditionally and never confirmed the label actually went Absent before writing a new unit over it (a slow/failed bootout could clobber a still-loaded service). ClearLabelAsync now polls for confirmed Absent (mirroring InstallRollback's own 26-vs-27 reasoning) and aborts with the marker retained on failure/timeout, used by both matrix branches and by entry-time marker recovery's own Uninstall call. Also: viability now checks before marker recovery (so its "nothing touched" doc comment stays true), DaemonKill.KillValidatedOwner no longer lets AggregateException/Win32Exception escape mid-transaction, and the shared tail no longer regresses a --replace marker's phase back to "captured" after the matrix already advanced it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…view round 2) Coordinator review of round 1 found four Important issues, all fixed: 1. Pid-staleness window: the matrix captured validatedPid ONCE before clearing the label, then used that pre-clear snapshot for the kill — but clearing (bootout) can itself terminate the true owner as a side effect, and the clear's own confirmation poll can run for up to _rollbackReserve. Now re-reads validatedDaemonPid AFTER the clear and skips the kill entirely if no live owner remains, so "kill the validated owner IF ONE REMAINS" is literally true rather than trusting a pid that may have been recycled in the interim. 2. RecoverLeftoverMarker treated an unreadable-but-present plist the same as "already gone" (readPlist returns null for both). Added a plistExists seam (defaults to File.Exists) to distinguish them: present-but-unreadable now surfaces RestoreVerification untouched instead of guessing it's safe to pave over. 3. Entry-time recovery's cleanup called the raw manager.Uninstall bool instead of the confirmed-Absent ClearLabelAsync the --replace matrix uses — a bootout that returns success without actually having unloaded yet would delete the marker on an unconfirmed clear. RecoverLeftoverMarker is now async and reuses ClearLabelAsync. 4. Two ServiceVerifyReplaceTests injected a plain small pid (4242) as the validated-owner seam in scenarios where DaemonKill's real, unmocked Process.Kill(entireProcessTree: true) could in principle be reached if the guard they exercise regressed. Switched both to the existing ManualOwnerPid sentinel (999_999_111, guaranteed not to resolve to a real process) so a regression fails loudly rather than risking a real kill. Also added the doc sentence pinning the 22 (pre-mutation abort) vs 26 (post-mutation undetermined-at-reserve-expiry) split on ClearLabelAsync, per the coordinator's ruling, plus three new regression tests (stale-pid re-read, unreadable-plist, entry-recovery clear-never-confirms). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Real-process integration coverage for spec §7 (parent death & stdio): kcap daemon service start --verify still exits with a coded VerifyExit when its stdio pipes close, and the service flock is released even when the CLI is orphaned from its parent — guarantees unit tests with fakes can't prove. No production code changed; Say() was already broken-pipe-safe.
Fix round 1 from task review (1 Critical + 5 Important): - Reconciliation now runs on the daemon_incompatible path too, not just Connected — a skewed daemon is exactly the prior-run-residue scenario a stale marker or orphan label shows up in. - The startup matrix no longer mutates while service_status reports txn_active: it waits out the one bounded requery (spec §6) and only proceeds once the flock is confirmed clear. - Event admission is now separate from startup auto-action eligibility: every AttachStatus transition still reaches OnAttachStatus's switch: the once-per-run arm only gates whether the startup branch/reconciliation actually runs, not whether the dispatcher observes the event at all — clearing the way for a later Task 20 skew hook on a non-first daemon_incompatible. - Stale-evidence handling now distinguishes a genuine query failure (surfaced honestly and logged) from evidence that raced a newer, meaningfully different attach outcome (re-evaluated once, so a racing Connected still gets its reconciliation) from a merely duplicate re-observation (no wasted query — keeps the once-per-run arm honest). - DisposeAsync is idempotent. Plus the trivial minors flagged alongside: the confirm-window timer is cancelled early once a fresh Connected arrives instead of leaking until its own deadline; the reconciliation-only txn-active requery is gated by the operation gate (not just the startup branch's own); RestartLoopAsync fires after any attempted mutation, not only a successful one; fire-and- forget entry points log unexpected exceptions instead of dropping them; the reconciliation ownership-mismatch checks are attached-only (was a latent double-report path with a startup-matrix row); test harness temp dirs are cleaned up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…econcile Fix round 2 from re-review (residual on finding 3): The startup branch's forced re-evaluation (a racing Connected mid-query) still reconciled with a hardcoded attached: false, so the run's only reconciliation pass silently ran in permanently-unattached mode for exactly the race window the round-1 fix targeted — the ownership-mismatch and coexistence checks were unreachable. IsCurrentlyAttached() now reads _lastObserved fresh at each Reconcile call site (the startup branch's own call, and both txn-active requery helpers, which had the same captured-parameter staleness risk across their own async gaps) instead of threading a boolean captured before the async work began. Also documents (comment only, no behavior change) why the no-unit install row doesn't pre-check DaemonPid: a racing/wedged manual daemon there is the install --verify transaction's own job to detect and roll back from, not a pre-flight guess by the app. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Extends DaemonLifecycleController (spec §4.3): on every Connected (paired with
the latest Snapshots-stream daemon version) and every daemon_incompatible
(paired with the hello DaemonVersion), a version mismatch against the cached
CLI version offers a takeover dialog — classified same-binary ("restart-update")
vs different-binary ("takeover") by canonical binary-path comparison, both
carrying the decision-3 disclosure. Accept is exactly one
ServiceInstallVerifiedAsync(replace:true) reusing Task 19's fresh-Connected
confirmation; decline persists the (daemonVersion, cliVersion) pair in
AppStateStore to suppress future offers of the same pair. At most one skew
dialog per run, and a version change between prompt and accept aborts with no
mutation.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix round 1 from review:
- Critical: RunSkewCheckAsync now awaits the held CacheVersionAsync task
(was fire-and-forget) before reading CliVersion — the attach cycle (a
sub-ms socket dial) could win the race against the version probe
(a process spawn), silently dropping the first-connect skew offer for
the whole run since nothing re-triggers it.
- Decline memory is now claim-before-show (spec §3.5): the pair is
persisted before ConfirmAsync is even called, so a crash mid-dialog
still suppresses a re-offer next run. Accept retracts the claim
(success or coded failure); a stale-consent abort retracts it too and
clears the once-per-run flag so the next trigger can re-offer — a
coded-failure accept also clears the flag, since the user got no
resolution.
- ClassifyTakeover treats a blank binary_path as different-binary instead
of letting Path.GetFullPath("") throw; CanonicalPath's try/catch now
wraps GetFullPath itself.
- The skew dialog gates on the same missing-binary/missing-profile
preconditions the silent-install row uses (not the PATH check —
decision 7 lets a dialoged install proceed on PathDegraded disclosure
instead of blocking).
- LifecyclePrompt.Kind* consts replace string literals; added a symlink
canonical-compare test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix round 2 from review: RetractDeclineAsync ran after awaiting RunVerifiedMutationAsync, which doesn't catch around the CLI call — an install that throws (shutdown mid-spawn, a process/IO fault) skipped the retract entirely, leaving an accepted pair mislabeled "declined" on disk. Since acceptance itself is the fact that invalidates the decline claim, the retract now runs immediately once the stale-consent check passes and before the mutation starts, so it can never be skipped by an exception the mutation raises. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 21 of AI-1654: StartActionAsync now implements the full §4.4 branch table (loaded/orphan/coexisting/bootstrap/nothing-at-all), routing any mismatch through a dialoged repair affordance that shares its accept path with skew's takeover (ConfirmAndTakeoverAsync). The Start command on the main window is repointed to it, and app shutdown now waits (capped at 60s) for QuiescedAsync before tearing the lifecycle controller down, so an internally-triggered mutation is never abandoned mid-flight. Composes the controller into App.axaml.cs for the first time, subscribing before the daemon client's attach pump starts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… dialog The interim lifecycle prompt dialog ignored its ConfirmAsync CancellationToken entirely - the tcs only resolved on a button click or the window's own Closed event. Since ConfirmAndTakeoverAsync holds the operation gate across the whole ConfirmAsync await, a dialog left open through a lifetime-cancel (app shutdown) held that gate forever, so QuiescedAsync - the very backstop shutdown relies on - never completed. WireDialogCancellation closes the dialog on cancellation (marshaled to the UI thread, since Cancel() can arrive from any thread), resolving false through the same Closed handler a manual Cancel click uses, and disposes its registration once the dialog resolves on its own. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Task 22 of the AI-1654 plan: production ILifecycleSurface replacing Task 21's interim NotifierLifecycleSurface. LifecyclePromptWindow/LifecyclePromptViewModel render Kind-specific dialogs (styled after ConsentPromptWindow); LifecycleSurface serializes ConfirmAsync calls with a SemaphoreSlim(1,1) and preserves Task 21's ct-cancellation contract. Status routes into MainWindowViewModel's start-message lane; Attention upgrades TrayViewModel's tray state, both via new optional constructor observables so every existing caller is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…text Task 22 fix round 1: TrayViewModel.Build judged lifecycleAttentionActive against the post-upgrade `state`, which already includes TrayState.Attention produced by rows 2/5/6/9/10 (daemon_incompatible, reconnecting, disconnected, malformed count, unrecognized values) on their own. Any co-occurring lifecycle Attention message then won the header over that row's own text, masking genuine connection trouble behind a stale/unrelated line. Judge both pendingAttention and lifecycleAttentionActive against baseState (the row's own pre-upgrade verdict) instead, so a lifecycle message only ever wins the header when the underlying row was genuinely fine (Idle/Running). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Preflight lstat taxonomy, argv-safe osascript install, post-install PATH probe, cancel/failure classification, and a POSIX-escaped sudo fallback for /usr/local/bin/kcap (spec §5). Mechanics only — the once-ever offer/tray wiring is a later task. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Match AppleScript's parenthesized "(-128)" cancellation marker instead of a bare "-128" substring — a genuine osascript/do-shell-script failure can embed the unescaped target path in its own error text (e.g. ".../app-128/kcap"), and a target under a PR/build-numbered directory containing "-128" would otherwise misclassify a real Failed as Cancelled, silently discarding the Detail and SudoFallback. Also cleans up the PathShimInstallerTests temp dirs in an [After(Test)] hook (best-effort delete). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
AI-1654 Task 24 (spec §5 offer surface): ShimOfferCoordinator waits for DaemonLifecycleController.PhaseClosed, then offers the once-ever PATH-shim dialog only when the CLI resolver has an absolute target and the login-shell probe positively finds kcap absent, claiming the AppState offered flag before showing the dialog. The offer reuses ILifecycleSurface.ConfirmAsync so it never stacks with the skew dialog. A new "Install command-line tool…" tray item stays visible while applicable-but-absent and lets the user retry the install manually regardless of the once-ever claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Documents the daemon service surface added over Tasks 1-13: status --json (machine-readable snapshot), install --verify/--replace (crash-safe transactional install with ownership+readiness verification, rollback, and takeover), and start --verify. Updates the ServiceUsage() inline usage text, kcap daemon --help (help-daemon.txt), and the README daemon-service section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The launchd unit was baking the GUI's inherited PATH, not the probed terminal PATH: KcapCli was constructed with terminalPath: null at the composition root and never reconstructed, so the PATH overlay was dead in production. KcapCli now takes a lazy Func<CancellationToken, Task<string?>> resolved from the shared LoginShellProbe and applies it only to ServiceInstallVerifiedAsync (the unit-writing mutation, per decision 7) — start-verify and read-only calls stay exempt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CLAUDE.md forbids Linear IDs in comments; replace the AI-1654/AI-1653 spec citations with neutral "spec §"/"the lifecycle slice" references, keeping the useful section pointers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![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: 8e94762379
ℹ️ 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".
| Func<string, string[], (int ExitCode, string StdOut, string StdErr)>? runProcess = null | ||
| ) : IServiceManager { | ||
| readonly UnitFileWriter _writeUnit = writeUnit ?? ((path, content, encoding) => ServiceFiles.WriteOwnerOnly(path, content, encoding)); | ||
| readonly Func<string, string[], (int ExitCode, string StdOut, string StdErr)> _runProcess = runProcess ?? ServiceProcess.Run; |
There was a problem hiding this comment.
Route verified service operations through the bounded process runner rather than ServiceProcess.Run. If any launchctl invocation hangs, the forward and rollback deadlines in ServiceVerify cannot run because every manager query/mutation blocks here indefinitely; terminal invocations never return, while the app eventually kills the entire CLI process and may interrupt it before rollback, leaving a transaction marker or partially mutated service.
Useful? React with 👍 / 👎.
| case "stop": manager.Stop(id); await Console.Out.WriteLineAsync($"Service '{id}' stopped (still installed)."); return 0; | ||
| case "status": return await ServiceStatus(manager, id); | ||
| case "uninstall": return await ServiceUninstall(manager, id); | ||
| case "start": return rest.Contains("--verify") ? await ServiceStartVerified(manager, id) : await ServiceStart(manager, id); |
There was a problem hiding this comment.
Reject start --verify on Linux and Windows, as is already done for verified installs, or implement those backends' ownership probes. Both SystemdServiceManager.Query and WindowsScheduledTaskServiceManager.Query always return a null JobPid, so ServiceVerify.IsReadyAsync can never succeed there; invoking this advertised command starts the service, waits through the full verification budget, then stops it during rollback and reports failure.
Useful? React with 👍 / 👎.
Comment on lines
+512
to
+514
| var h = await hello(serviceId, budget); | ||
| if (!h.WellFormed) return InstallReady.NotReady; | ||
| if (h.DaemonVersion != expectedVersion) return InstallReady.VersionMismatch; |
There was a problem hiding this comment.
Require the install hello's ProtocolVersion to be supported and its DaemonName to match serviceId, not merely that the frame deserialized and its version string matched. HelloProbeResult deliberately carries both fields, but this predicate ignores them, so an incompatible daemon or a daemon identifying as another name can satisfy the two readiness checks and cause install --verify to commit instead of rolling back.
Useful? React with 👍 / 👎.
Comment on lines
+27
to
+28
| case ShimPreflight.AlreadyInstalled: | ||
| return new ShimResult(ShimOutcome.Installed, null, null); |
There was a problem hiding this comment.
Run the login-shell PATH probe for AlreadyInstalled just as after a newly created link. This branch is reachable specifically when /usr/local/bin/kcap already points at the target but /usr/local/bin is absent from the user's PATH; it currently returns Installed, causing the UI to claim that kcap is now on the terminal PATH even though the coordinator's preceding probe established the opposite.
Useful? React with 👍 / 👎.
Comment on lines
+121
to
+123
| Task SurfaceResultAsync(ShimResult result) => result.Outcome switch { | ||
| ShimOutcome.Installed => Status("kcap is now on your terminal PATH."), | ||
| ShimOutcome.InstalledButNotOnPath => Status(result.Detail ?? "kcap was linked, but is not yet on your terminal PATH."), |
There was a problem hiding this comment.
Publish _offerable.OnNext(false) when installation succeeds and the probe confirms kcap is on PATH. Once RunAsync has emitted true, neither the automatic nor manual success path ever resets the BehaviorSubject, so the tray continues showing “Install command-line tool…” for the rest of the run and allows redundant repeated installs despite the documented applicable-but-absent visibility condition.
Useful? React with 👍 / 👎.
PR Summary by QodoCrash-safe daemon service lifecycle + PATH shim for desktop supervisor
AI Description
Diagram
High-Level Assessment
Files changed (76)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
- CLI: gate `start --verify` to launchd, matching install --verify's gate - CLI: install/replace hello validation now checks daemon name and protocol version, not just DaemonVersion; version check is skipped when expectedVersion is null (honors the nullable contract) - App: PathShimInstaller's AlreadyInstalled path re-runs the login-shell PATH probe instead of declaring success on the symlink alone - App: ShimOfferCoordinator resets Offerable to false after a confirmed on-PATH install, so the tray item disappears once the shim actually works - App: KcapCli degrades honestly (null / exit 127) instead of throwing when CliPath is null Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes 8 verified findings in the service install/start --verify transaction: - Plain `service install` now serializes on the per-label service lock like every other mutating verb (extracted ServiceInstallPlain). - Bound the transaction to one forward cutoff + a separate rollback reserve (30s advertised bound, exposed as ServiceVerify.AdvertisedBound), and route the verify path's launchctl calls through ServiceProcess.RunBounded so a hung tool maps to a bounded failure instead of blocking past the deadline. The legacy non-verify path is unchanged. - Prove viability (plist render + pinned-profile server-URL validity) before the first destructive step, in both fresh and replace paths. - ClearLabelAsync confirms the plist is gone (not just the label absent), re-uninstalling a bootout-retained orphan or failing coded. - Owning-label replace confirms the old daemon pid exited before write/bootstrap. - Readiness pins the observed job pid and the final recheck requires the same incarnation, so a KeepAlive respawn can't bless a crash-looping unit. - Start-verify records phase=committed before deleting the marker. - fsync the marker's containing directory (libc, Unix-guarded, best-effort). - Reject `--no-start` combined with `--verify`; help/usage/README updated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PathShimInstaller.ProbeOutcomeAsync reused ShimOfferCoordinator's cached pre-install "absent" answer, so every successful shim install reported InstalledButNotOnPath and the tray item never hid. LoginShellProbe now supports a forceRefresh probe that bypasses and repopulates the cache; the post-install/AlreadyInstalled verification uses it, while the offer-decision call keeps using the cached (correct) pre-install state. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ShimOfferCoordinator unconditionally offered the shim and spawned PathShimInstaller, which unconditionally targets osascript and /usr/local/bin/kcap — an operation the app could offer on Linux/Windows and then fail to spawn. The coordinator now forces its link target to null off macOS (an injectable OS-check seam for testability), reusing the existing "nothing to link" no-op path everywhere: no probe, no offer, Offerable stays false, no installer spawn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing (AI-1654) Three fixes plus one bound adjustment from hosted-reviewer findings on the daemon lifecycle controller: - Revalidate takeover evidence inside the gate after the confirmation dialog: a terminal replacing the plist/unit while the dialog is open produces no attach event, so the generation-token check alone misses it. ConfirmAndTakeoverAsync now re-queries fresh status and rejects a classification that no longer matches what was disclosed, aborting with a Status line exactly like the existing stale-consent path. - Never read a forced-kill's exit code as a verify outcome: when a mutation's ProcessResult.TimedOut is true, RunVerifiedMutationAsync re-queries service status and surfaces an Attention state from the evidence instead of treating the killed process's exit code as success or a coded failure. - Parse the wire service state into a ServiceState enum with an explicit Unknown arm, shared by the startup matrix, Start action, and reconciliation — an unrecognized value is no longer read as NotInstalled and can't fall into an auto-install/start path. - Raise KcapCli's mutation timeout from 45s to 60s: the CLI transaction's true worst case is ~50s (forward + rollback reserve + lock-wait + crash-recovery pre-phase + KillWait), above the prior bound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…llback plist Two crash-safety fixes in the service --verify transaction engine, plus a comment cleanup. Readiness observation now honors a single absolute forward deadline. IsReadyAsync and IsInstallReadyAsync take the forward DateTimeOffset and recompute Remaining() immediately before each bounded sub-call (hello, then Query), returning NotReady when no budget remains rather than starting a sub-call. Previously a late-but- well-formed hello could consume almost the whole budget and a hung launchctl print was then handed the SAME full budget, letting readiness spend ~2x the intended forward time and making AdvertisedBound (20s forward + 10s reserve) untrue. The final recheck now runs inside a small confirm slice reserved WITHIN the forward budget (the primary poll runs to deadline - reserve), so total forward work (poll + recheck) stays inside the one forward cutoff instead of overrunning it by the old max(remaining, PollInterval) floor. InstallRollback now fails closed on a plist it cannot verify is ours: a null read is absence OR a present-but-unreadable/foreign file, so it only uninstalls when the file is genuinely absent (read null AND !plistExists) or verified-ours (readable AND fingerprint matches). A lock-unaware writer that replaces the plist between bootstrap and rollback is now surfaced (restore_verification), never paved over — the same rule RecoverLeftoverMarker already applies at entry. Restated two comments to their invariant, dropping leaked review-narrative labels. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
State each invariant once at its home (the _confirmReserve field, the IsReadyAsync docs); trim the repeated restatements and drop the historical narrative from two tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alexeyzimarev
deleted the
alexeyzimarev/ai-1654-daemon-lifecycle-management-path-shim-desktop-supervisor
branch
August 12, 2026 14:15
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
1 participant
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.
Closes #541. AI-1654.
Slice 3 of the desktop supervisor app: the app manages the
kcapdaemon's launchd lifecycle, offers takeover on version skew, and installs a PATH shim — built on a crash-safe CLI transaction so no lifecycle path can silently kill or spin a user's daemon. macOS-first; Windows/Linux left open via the cross-platformIServiceManagerand the shim interface.Design (survived 8 rounds of Codex spec-review):
docs/superpowers/specs/2026-08-10-ai1654-daemon-lifecycle-path-shim-design.md.CLI (
src/Capacitor.Cli,src/Capacitor.Cli.Daemon)daemon service status --json— machine-readable status with tri-state launchd classification (loaded/absent/unknown—absentonly on the positive could-not-find signature;--jsonfails non-zero onunknownrather than masquerading asnot_installed), the validated PID-file owner, the launchd job PID, and transaction-marker/lock liveness.daemon service install/start --verify— one CLI process owns the mutation, verification, and rollback end-to-end. Success requires ownership AND readiness (a local-control hello, not just the PID file, which lands long before the daemon's host is up) plus a final on-disk plist-fingerprint recheck. A durable, phase-recording, fsync-ordered marker plus a fingerprint-scoped recovery authority make it crash- and power-loss-safe; a two-phase deadline reserves a rollback budget; the child completes even if the app (or the npm launcher's grandchild) is force-quit.install --replace(requires--verify, macOS/launchd-only) — the takeover ownership matrix: clears a non-owning/orphan label, stops-and-confirms a validated live owner via an internal raw-kill helper, then installs — all in the one transaction, so the app never orchestrates a multi-command destructive sequence.service stopnow unloads the label (bootout, plist retained) — aSIGTERMcan't stop a lock-losingKeepAlivejob between incarnations.service uninstalldistinguishes benign label-absence from a failed bootout.NameInUserefusal at initial connect, soKeepAlivecan't respin a name held elsewhere. The mid-run contest keeps exit 3 (one respawn settles it) — deliberately scoped.DaemonLockPaths.Directorynamespace, immune toKCAP_CONFIG_DIR.Core (
src/Capacitor.Cli.Core)DaemonVersionpropagatesCycleOutcome → LocalControlEvent.Unreachable → AttachStatus, deduped on(reason, version), so an incompatible old daemon — the takeover offer's primary audience — actually reaches the app. Zero wire-protocol changes.App (
src/Capacitor.App)DaemonLifecycleController— startup-phase state machine with a once-per-run auto-action arm, a connection-generation token discarding stale evidence, and a reconciliation query on every startup path (surfacing crash residue, orphan labels, and live transactions without ever mutating into a held flock).Connectedversion mismatch or an incompatible hello; same-binary vs different-binary classification (path equality is not provenance — both dialogs disclose the rewrite); accept is oneinstall --replace --verify; decline memory is claim-before-show with retract-on-accept/stale-abort, so a crash-at-dialog never re-nags and an accepted-but-failed install re-offers.PathShimInstaller(lstat taxonomy, argv-passed osascript target, POSIX-escaped sudo fallback,-128cancel detection), and the shim offer + tray item.IProcessRunnerv2 (stdout, env overlay, kill-tree vs abandon-wait),CliResolver+ typedIKcapCli, the interactive-login-shell PATH probe (so the launchd unit bakes the terminal PATH, not the GUI's minimal one — decision 7), and an atomic serialized app-state store.Testing
502 app unit tests; ~430 new/changed CLI unit tests; real-process integration tests for the transaction's parent-death and closed-stdio guarantees (macOS-manual). E2E stays a manual checklist (no signed bundle yet). Docs/help/README updated in this PR.
Built task-by-task with per-task spec+quality review, adversarial fix loops, and a final whole-branch review.
🤖 Generated with Claude Code