Skip to content

[AI-3419] Run the macOS daemon as a Standard launchd process - #1269

Merged
realtonyyoung merged 7 commits into
mainfrom
claude-tyoung/daemon-processtype-standard
Oct 1, 2026
Merged

realtonyyoung merged 7 commits into
mainfrom
claude-tyoung/daemon-processtype-standard

Conversation

@realtonyyoung

Copy link
Copy Markdown
Collaborator

AI-3419 (no GitHub issue)

What & why

With ProcessType=Adaptive, launchd runs the daemon at priority 4, and under load its hub heartbeat starves into reconnect storms. New plists are now written as Standard. launchd reads that key only when a job loads, so daemon service refresh runs from the post-update refresh list. It rewrites the plist and reloads the job, but only after the daemon accepts an idle-only restart (Restart "now", a frame older daemons already handle). A busy daemon is left for the next update.

Where to look

  • LaunchdServiceManager.RefreshProcessType: the bootout runs while the accepted restart is still exiting, so launchd can't relaunch from its cached definition. A failed bootstrap restores the old plist and bootstraps again.
  • Daemons the desktop app installed share these labels, so they also get the idle-only reload when an npm install updates.

Verification

  • Hand-editing the plist on one Mac moved the daemon from priority 4 to 20. Over the next 24h, 22 heavy-load windows (load up to 95 on 12 cores, 0% idle) had 0 forced reconnects; at priority 4 the day before there were 230.
  • LaunchdProcessTypeRefreshTests 7/7, Launchd* 50/50, DaemonRestartCommandTests 4/4. Removing the idle gate fails Busy_daemon_is_not_reloaded_but_its_plist_is_rewritten.
  • node npm/kcap/bin/refresh.test.js passes.

🤖 Generated with Claude Code

launchd reads ProcessType only when a job loads, so an installed daemon
is reloaded after an update, and only once it accepts an idle-only
restart: the reload kills whatever it hosts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Oct 1, 2026

Copy link
Copy Markdown

AI-3419

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Run macOS daemons at standard priority with idle-only migration

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Write new launchd jobs as Standard to prevent heartbeat starvation under load.
• Refresh installed jobs after updates, reloading only daemons that accept an idle-only restart.
• Restore the previous plist if the upgraded job fails to bootstrap.
Diagram

graph TD
  A["Package update"] --> B["Service refresh"] --> C["Check and rewrite plist"] --> D{"Adaptive loaded?"} -->|yes| E{"Idle restart accepted?"} -->|yes| F["Reload launchd"] -->|bootstrap fails| G["Restore old plist"]
  D -->|no| H["No reload"]
  E -->|no| H
Loading
High-Level Assessment

The targeted plist migration preserves existing service settings, while checking launchd's loaded state handles jobs deferred by an earlier update. An unconditional reload would interrupt hosted agents; replacing whole plists would risk overwriting existing configuration.

Files changed (9) +294 / -11

Enhancement (1) +19 / -0
ProcessTypeRefresh.csDefine per-job refresh outcomes +19/-0

Define per-job refresh outcomes

• Adds distinct outcomes for unchanged, rewritten, deferred, reloaded, and failed migrations.

src/Capacitor.Cli/Services/ProcessTypeRefresh.cs

Bug fix (4) +106 / -3
refresh.jsRun daemon service refresh after package updates +3/-1

Run daemon service refresh after package updates

• Adds the service refresh command to the existing post-update list. It operates on installed units without requiring a plugin installation marker.

npm/kcap/bin/refresh.js

DaemonServiceCommands.csAdd an idle-gated service refresh command +46/-1

Add an idle-gated service refresh command

• Enumerates installed launchd jobs, requests a timed idle-only restart when needed, and reports reloads, deferrals, and failures. Other service managers return without action.

src/Capacitor.Cli/Commands/DaemonServiceCommands.cs

LaunchdServiceManager.csMigrate and safely reload Adaptive launchd jobs +37/-0

Migrate and safely reload Adaptive launchd jobs

• Rewrites older plists, checks launchd's loaded spawn type, and reloads only after restart acceptance. If bootstrap fails, it restores the original plist and attempts to bootstrap the job again.

src/Capacitor.Cli/Services/LaunchdServiceManager.cs

LaunchdUnit.csWrite Standard plists and identify stale jobs +20/-1

Write Standard plists and identify stale jobs

• New plists specify Standard instead of Adaptive. Helpers replace the exact legacy line and detect an Adaptive spawn type in launchctl output.

src/Capacitor.Cli/Services/LaunchdUnit.cs

Refactor (2) +23 / -8
DaemonCommands.csShare restart socket handling with service refresh +2/-8

Share restart socket handling with service refresh

• Routes the existing daemon restart command through the extracted restart client while retaining its reply handling.

src/Capacitor.Cli/Commands/DaemonCommands.cs

DaemonRestartClient.csExtract daemon restart IPC client +21/-0

Extract daemon restart IPC client

• Provides a cancellable control-socket request used by both explicit restarts and idle-only service refresh.

src/Capacitor.Cli/Commands/DaemonRestartClient.cs

Tests (2) +146 / -0
LaunchdProcessTypeRefreshTests.csCover launchd ProcessType migration and rollback +138/-0

Cover launchd ProcessType migration and rollback

• Tests plist conversion, spawn-type detection, idle and busy jobs, a previously deferred job, an unchanged job, and failed-bootstrap restoration.

test/Capacitor.Cli.Tests.Unit/Services/LaunchdProcessTypeRefreshTests.cs

LaunchdUnitTests.csAssert new plists use Standard ProcessType +8/-0

Assert new plists use Standard ProcessType

• Adds an XML-level assertion that generated launchd plists specify Standard.

test/Capacitor.Cli.Tests.Unit/Services/LaunchdUnitTests.cs

@qodo-code-review

qodo-code-review Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. A failed reload can leave the daemon offline ✓ Resolved
Description
RefreshProcessType ignores the result of its rollback bootstrap and returns Failed as though
restoring the original plist also restored the job. If the first bootstrap fails after a successful
bootout and the second fails too, the label remains unloaded; the caller reports only the first
error.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R143-145]

+        if (upgraded is not null) _writeUnit(path, original!, null);
+        RunCtl(null, LaunchdUnit.BootstrapArgs(Uid(), path));
+
Relevance

●●● Strong

Ignoring rollback failure can leave launchd state inconsistent; historical service verification
findings favor explicit failure handling.

PR-#556
PR-#938

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Bootout and bootstrap are separate launchctl operations. The new code discards the rollback result,
whereas the existing uninstall path re-queries a failed bootout rather than assuming the label
state.

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[138-146]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[192-214]
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[114-117]

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

## Issue description
A failed upgrade bootstrap can be followed by a failed rollback bootstrap, leaving the daemon unloaded while refresh reports only the first failure.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[138-146]
## Recommended Fix
Check the rollback bootstrap result and the final label state. Include recovery failure in the returned error, and avoid claiming that the job was restored when it was not.

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


2. New work can be cut off by the reload ✗ Dismissed
Description
RequestIdleRestart relies on a momentary IsBusy result, but accepting a restart does not prevent
the orchestrator from admitting work before RefreshProcessType runs bootout. If an agent starts in
that interval, the new reload terminates a daemon that is now hosting it despite the idle-only gate.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R134-138]

+        if (!requestIdleRestart()) return ProcessTypeRefresh.Deferred;
+
+        // The accepted restart is already exiting the daemon. Booting out now unloads the job before
+        // launchd can relaunch it from the definition it cached at load.
+        RunCtl(null, LaunchdUnit.BootoutArgs(Uid(), serviceId));
Relevance

●● Moderate

This is a plausible concurrency race, but historical evidence does not directly establish acceptance
for this restart window.

PR-#526
PR-#656

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The busy predicate counts current agents and evaluations. The coordinator's lock protects restart
flags, not agent admission, and refresh performs bootout after receiving the acknowledgement.

src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs[39-60]
src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs[130-158]
src/Capacitor.Cli.Daemon/Services/AgentOrchestrator.cs[970-970]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[134-139]

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

## Issue description
An idle acknowledgement is not a guarantee that the daemon remains idle until launchctl bootout.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[134-139]
- src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[124-130]
## Recommended Fix
Coordinate the restart decision with work admission so an accepted idle-only restart prevents new agents and evaluations from starting before shutdown. Reload only after that shutdown commitment is established.

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


3. An update can stop midway through a reload ✓ Resolved
Description
RefreshProcessType passes null to every RunCtl call, selecting synchronous launchctl execution
without a timeout. If launchctl stalls after bootout, npm's refresh wrapper kills the child at 60
seconds before it can bootstrap the job; a direct refresh can instead remain blocked indefinitely.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R138-139]

+        RunCtl(null, LaunchdUnit.BootoutArgs(Uid(), serviceId));
+        var (bootstrapExit, _, bootstrapErr, _) = RunCtl(null, LaunchdUnit.BootstrapArgs(Uid(), path));
Relevance

●● Moderate

Timeout concerns are credible, though similar unbounded process-execution concerns were previously
rejected.

PR-#137
PR-#556

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
A null timeout selects ServiceProcess.Run, whose reads and wait are unbounded. The npm wrapper
imposes an independent 60-second SIGKILL deadline on the entire refresh child.

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[16-22]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-145]
src/Capacitor.Cli/Services/ServiceProcess.cs[13-25]
npm/kcap/bin/refresh.js[43-62]

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

## Issue description
Unbounded launchctl calls can outlive the npm refresh child's deadline, including between bootout and bootstrap.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-145]
- npm/kcap/bin/refresh.js[43-62]
## Recommended Fix
Use bounded launchctl calls with an overall deadline that reserves time for rollback. Handle timeouts as uncertain label states and attempt verified recovery before the wrapper's deadline.

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



Remediation recommended

4. Later services miss the priority upgrade ✗ Dismissed
Description
Refresh() keeps iterating after its 55-second deadline, and each RefreshProcessType() call runs
a launchctl probe before checking the remaining time. With enough installed services whose probes
take up to seven seconds, npm kills the refresh process at 60 seconds before it reaches the
remaining services.
Code

src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[R104-106]

+        var failed  = 0;
+        var started = time.GetTimestamp();
+        TimeSpan TimeLeft() => RefreshDeadline - time.GetElapsedTime(started);
Relevance

●●● Strong

Recent precedents accept enforcing aggregate deadlines across unbounded work and preventing later
operations from overrunning process budgets.

PR-#187
PR-#173

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The command uses one timer for an unbounded list of installed plists, while each service performs a
seven-second probe before consulting that timer; the npm wrapper kills the child at 60 seconds.

src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[94-109]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[43-51]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[112-114]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[138-150]
npm/kcap/bin/refresh.js[39-57]

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

## Issue description
The global refresh deadline does not bound the per-service probes, so npm can kill the process before later installed services are checked.
## Fix Focus Areas
- src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[104-109]
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[138-150]
## Recommended Fix
Check the remaining deadline before starting each service and bound the initial probe by the time available. Defer services that cannot be checked within the deadline rather than starting another full-length probe.

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


5. A failure comment promises a rollback ✗ Dismissed
Description
The Failed enum comment says the original plist is restored and bootstrapped again, but
RefreshProcessType returns Failed immediately when bootout leaves the job loaded. On that path,
the method has already rewritten the plist and performs neither the restoration nor the second
bootstrap, so a reader relying on the comment would misunderstand the failure state.
Code

src/Capacitor.Cli/Services/ProcessTypeRefresh.cs[R21-22]

+    /// <summary>The reload failed. The original plist is restored and bootstrapped again; the error says
+    /// whether that rollback loaded.</summary>
Relevance

●●● Strong

The comment inaccurately describes an early-return path; accepted precedents favor concise,
behavior-accurate documentation of failure states.

PR-#556
PR-#421

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new enum comment promises restoration for every failure, while the failed-bootout branch returns
before the restoration and rollback-bootstrap calls.

Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
src/Capacitor.Cli/Services/ProcessTypeRefresh.cs[21-22]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[158-167]

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

## Issue description
The `Failed` comment promises restoration and a second bootstrap even when a failed bootout returns before either operation.

## Fix Focus Areas
- src/Capacitor.Cli/Services/ProcessTypeRefresh.cs[21-22]

## Recommended Fix
Describe `Failed` without promising a rollback on every path, and leave path-specific recovery details to the returned error.

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


6. Probe timeouts silently skip upgrades ✗ Dismissed
Description
RefreshProcessType returns Unchanged as soon as its bounded launchctl print times out, without
distinguishing that failure from an already-current job. When a direct refresh encounters a stalled
probe, Refresh emits no message for Unchanged and returns success even though it has not checked
or migrated that installed service.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R130-131]

+        var (printExit, printOut, printErr, printTimedOut) = RunCtl(RefreshCtlTimeout, LaunchdUnit.PrintArgs(Uid(), serviceId));
+        if (printTimedOut) return ProcessTypeRefresh.Unchanged;
Relevance

●●● Strong

Concrete timeout failure is silently reported as success, directly undermining the refresh command’s
migration purpose.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The newly bounded print returns Unchanged on timeout before inspecting the plist's process type.
The command only prints Reloaded, Deferred, and Failed, and counts only Failed toward its
exit status.

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[123-139]
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[98-121]

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

## Issue description
A timed-out print probe is presented as an unchanged service, so the refresh command reports success without indicating that an installed job was not checked.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[130-134]
- src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[103-121]
## Recommended Fix
Return a distinct failed or deferred result for a timed-out probe and make the command surface that outcome rather than treating it as `Unchanged`.

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


View medium (7)
7. Timed-out unloads trigger unsafe reloads ✓ Resolved
Description
RefreshProcessType discards the bounded bootout result and attempts bootstrap even when the
unload timed out and its process was terminated. If the old label remains loaded, both bootstrap
attempts can fail, and the rollback message can incorrectly say the daemon is not loaded.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R149-150]

+        RunCtl(RefreshCtlTimeout, LaunchdUnit.BootoutArgs(Uid(), serviceId));
+        if (TryBootstrap(path, out var bootstrapError)) return ProcessTypeRefresh.Reloaded;
Relevance

●●● Strong

Ignoring bootout timeout creates an unsafe bootstrap race and inaccurate rollback reporting in the
refresh workflow.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
RunBounded terminates an over-time process and returns TimedOut, but refresh ignores the bootout
tuple and proceeds directly to bootstrap. Its rollback error asserts that the daemon is not loaded
without checking the label; the manager's uninstall path instead checks the timeout and re-queries
after a failed bootout.

src/Capacitor.Cli/Services/ServiceProcess.cs[43-64]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[149-166]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[175-195]

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

## Issue description
A bounded bootout can time out without unloading the job, yet refresh proceeds to bootstrap and may misreport the rollback state.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[149-155]
## Recommended Fix
Inspect the bootout timeout and result, then confirm label absence with a bounded print before bootstrapping; report an unresolved unload without asserting that the daemon is offline.

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


8. A bootstrap timeout can undo migration ✗ Dismissed
Description
TryBootstrap treats a timeout as failure without checking whether launchd already loaded the
upgraded job, and RefreshProcessType then restores the original Adaptive plist. If the first
bootstrap took effect before its command timed out, the rollback bootstrap can fail against the
loaded label while the on-disk priority change has been undone.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R160-163]

+    bool TryBootstrap(string plistPath, out string? error) {
+        var (exit, _, err, timedOut) = RunCtl(RefreshCtlTimeout, LaunchdUnit.BootstrapArgs(Uid(), plistPath));
+        error = timedOut ? "launchctl bootstrap timed out and was terminated"
+              : exit != 0 ? $"launchctl bootstrap failed (exit {exit}): {err.Trim()}"
Relevance

●●● Strong

Bootstrap timeout is ambiguous; unconditional rollback can undo a migration that launchd already
applied.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
RunBounded reports whether the launchctl process exited by the deadline, not whether launchd
applied its request. TryBootstrap converts that timeout directly into an error, after which
refresh rewrites the original plist and retries bootstrap without a label query or unload.

src/Capacitor.Cli/Services/ServiceProcess.cs[43-64]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[150-166]

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

## Issue description
A timed-out bootstrap does not establish that the job failed to load, so immediately restoring its old plist can leave the file and loaded definition inconsistent.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[150-166]
## Recommended Fix
After a bootstrap timeout, use a bounded label probe to establish whether the upgraded job loaded before deciding to restore the plist. If rollback is required, ensure the label is unloaded before bootstrapping the original unit and report any state that cannot be established.

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


9. Failed probes leave jobs at old priority ✗ Dismissed
Description
RefreshProcessType treats a non-timeout launchctl print result classified as Unknown as though
the label were absent. If an installed Adaptive job is still loaded when that probe fails, the
method rewrites its plist and returns Rewritten without reloading the cached job.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R133-134]

+        var loaded   = LaunchdUnit.ClassifyPrint(printExit, printOut, printErr) == LabelProbe.Loaded;
+        var stale    = loaded && LaunchdUnit.LoadedAsAdaptive(printOut);
Relevance

●●● Strong

Unknown probe results can cause an installed Adaptive job to be rewritten without reload, a concrete
migration correctness bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
ClassifyPrint explicitly returns Unknown for nonzero exits other than a confirmed absent
service. Refresh reduces every result other than Loaded to loaded = false, while an upgradeable
plist is written and returned as Rewritten when stale is false.

src/Capacitor.Cli/Services/LaunchdUnit.cs[217-223]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[130-145]

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

## Issue description
An unknown print result does not establish that an installed job is unloaded, but refresh currently treats it that way and can skip a necessary reload.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[130-145]
## Recommended Fix
Retain the full `LabelProbe` classification and handle `Unknown` as a failed or deferred refresh before rewriting or claiming the loaded job needs no reload.

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


10. Service refresh is absent from CLI docs ✓ Resolved
Description
DaemonServiceCommands.DispatchAsync adds daemon service refresh, but the README's
service-command list does not include it. Users consulting that list after an update cannot discover
the command or learn that it reloads only idle daemons.
Code

src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[45]

+            "refresh"   => await verbs.Refresh(),
Relevance

●●● Strong

README synchronization findings for new user-facing CLI commands are consistently accepted.

PR-#191
PR-#240
PR-#629

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2270057 requires README updates for user-facing CLI changes. The changed dispatch adds
refresh, while the README's existing daemon-service command list omits it.

Rule 2270057: Keep CLI documentation in README.md in sync with user-facing CLI changes
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[42-45]
README.md[1063-1075]

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

## Issue description
The new service refresh command is missing from the README's daemon-service command list.

## Fix Focus Areas
- README.md[1057-1075]

## Recommended Fix
Add `kcap daemon service refresh` to the service-command list and explain that it updates installed macOS units while deferring reloads for busy daemons.

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


11. Some old jobs reload at the old priority ✓ Resolved
Description
RefreshProcessType proceeds to bootout and returns Reloaded when the loaded job is Adaptive even
if UpgradeProcessType found no exact Adaptive line to change on disk. An installed plist with
different whitespace or line endings retains its Adaptive value, so an idle daemon is restarted from
the unchanged plist and the command incorrectly announces standard priority.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R131-134]

+        if (upgraded is null && !stale) return ProcessTypeRefresh.Unchanged;
+        if (upgraded is not null) _writeUnit(path, upgraded, null);
+        if (!stale) return ProcessTypeRefresh.Rewritten;
+        if (!requestIdleRestart()) return ProcessTypeRefresh.Deferred;
Relevance

●●● Strong

Launchd plist parsing and process-type correctness issues have been accepted, especially around
strict state validation.

PR-#556
PR-#938

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The replacement requires the writer's exact LF-terminated line. The refresh path still reloads a
stale loaded label when replacement returns null, and treats a successful bootstrap as a Standard
reload without checking the plist value.

src/Capacitor.Cli/Services/LaunchdUnit.cs[16-24]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-140]
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[106-109]

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

## Issue description
A loaded Adaptive job is reloaded even when its on-disk Adaptive value was not converted, so refresh can claim an upgrade that did not occur.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdUnit.cs[16-24]
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-140]
## Recommended Fix
Inspect or update the plist's ProcessType independently of exact formatting. Before requesting a restart, establish that the on-disk job will load as Standard; otherwise return a reported failure without restarting it.

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


12. Stopped daemons miss the priority upgrade ✓ Resolved
Description
RefreshProcessType requires RequestIdleRestart to succeed before reloading every loaded Adaptive
label, even when that label has no running daemon to contact. An older job stopped after `install
--no-start` can retain its loaded Adaptive definition, so each refresh rewrites its plist but defers
the reload because its socket is unreachable.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R133-134]

+        if (!stale) return ProcessTypeRefresh.Rewritten;
+        if (!requestIdleRestart()) return ProcessTypeRefresh.Deferred;
Relevance

●●● Strong

Stopped-versus-loaded service state is a concrete lifecycle correctness issue consistent with
accepted service findings.

PR-#137
PR-#656

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Install's --no-start path bootstraps and then kills the job, while the plist uses KeepAlive only
for unsuccessful exits. The refresh gates even a loaded, non-running job on a socket
acknowledgement.

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[94-102]
src/Capacitor.Cli/Services/LaunchdUnit.cs[70-72]
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-139]
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[124-133]

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

## Issue description
A loaded but non-running Adaptive job cannot acknowledge a restart, so refresh never reloads its updated plist.
## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[126-140]
## Recommended Fix
Use the launchctl probe to distinguish an inactive loaded job from a running one. Reload only a verified inactive job without requesting daemon IPC; keep the idle acknowledgement requirement for running jobs.

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


13. An accepted restart can be mistaken for busy ✗ Dismissed
Description
RequestIdleRestart treats both a missing restarting acknowledgement and a queued reply as
refusals, even though StopApplication can run before the acknowledgement is written and
RequestRestart can queue the request when work becomes active between two busy checks. In either
case refresh reports a deferral and skips bootout, while the accepted or pending restart can later
relaunch the daemon from launchd’s cached Adaptive definition.
Code

src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[R128-132]

+            var reply = DaemonRestartClient.RequestAsync(store, serviceId, "now", cts.Token).GetAwaiter().GetResult();
+
+            return reply is { Type: Core.LocalIpc.FrameType.RestartAck, Text: "restarting" };
+        } catch (Exception ex) when (ex is SocketException or IOException or OperationCanceledException) {
+            return false;
Relevance

●● Moderate

The race is technically specific, but no close restart-acknowledgement precedent establishes team
acceptance.

PR-#526
PR-#656

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The supervised restart strategy calls StopApplication synchronously, and the control server writes
its reply only afterward using a host shutdown token that may already be canceled. Separately, the
server checks IsBusy before calling RequestRestart, but the coordinator checks it again after
setting _pending; if work becomes active between those checks, it returns a queued reply that
fails the client’s restarting predicate even though the restart remains pending.

src/Capacitor.Cli.Daemon/Services/SupervisedExitStrategy.cs[10-16]
src/Capacitor.Cli.Daemon/Services/LocalControlServer.cs[22-34]
src/Capacitor.Cli.Daemon/Services/LocalControlServer.cs[99-109]
src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[125-133]
src/Capacitor.Cli.Daemon/Services/LocalControlServer.cs[91-109]
src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs[101-114]
src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs[130-149]

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

## Issue description
Refresh can treat an accepted idle restart as a refusal when shutdown prevents its acknowledgement from arriving, or when work becomes active between the daemon’s busy checks and the request is queued. In either case, refresh skips bootout even though the daemon may later restart from launchd’s cached Adaptive definition.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/DaemonServiceCommands.cs[124-133]
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[134-139]

## Recommended Fix
Make the idle-only decision atomic with queuing: if the restart cannot begin immediately, return a busy refusal without leaving a pending restart. For accepted restarts, deliver a durable acknowledgement before initiating shutdown, or give refresh a safe way to distinguish an accepted restart from a genuine busy refusal if the connection closes; update refresh to handle those outcomes distinctly.

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



Informational

14. Refresh timing depends on an external kill limit 📘 Rule violation ⚙ Maintainability ⭐ New
Description
RefreshCtlTimeout is documented using the npm wrapper's 60-second termination behavior rather than
a self-contained service invariant. Changes to that wrapper's timeout can invalidate the
seven-second budget without any indication in this code, affecting whether daemon reloads finish
safely.
Code

src/Capacitor.Cli/Services/LaunchdServiceManager.cs[R112-114]

+    /// <summary>Per launchctl call, which normally answers in milliseconds. Refresh runs under the npm
+    /// refresh wrapper's 60s kill.</summary>
+    public static readonly TimeSpan RefreshCtlTimeout = TimeSpan.FromSeconds(7);
Relevance

● Weak

Recent precedent rejected removing external timeout metadata when it documented a real operational
invariant.

PR-#978

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added comment explicitly ties the timeout rationale to an external process artifact, namely the
npm refresh wrapper's 60-second kill. The compliance checklist prohibits process-reference metadata
in code comments.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
src/Capacitor.Cli/Services/LaunchdServiceManager.cs[112-114]

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

## Issue description
The comment for `RefreshCtlTimeout` relies on the external npm refresh wrapper's 60-second kill limit, which is process-reference metadata and can become stale when the wrapper changes.

## Fix Focus Areas
- src/Capacitor.Cli/Services/LaunchdServiceManager.cs[112-114]

## Recommended Fix
Rewrite the comment to explain the current internal timeout invariant and why each bounded launchctl call must complete within the refresh budget, without referring to the npm wrapper or its kill behavior.

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

Dismiss ↗ | View ↗


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
Review mode: 🚀 Fast: The push makes a small, localized timeout guard and related visibility/documentation adjustments in the daemon refresh path, without broad or high-risk behavioral changes.

Grey Divider

Tip of the day
💡 Did you know, you can keep summaries lean with Findings visible per group, which tucks the rest behind a View link

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/DaemonServiceCommands.cs
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Commands/DaemonServiceCommands.cs
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
The refresh runs under the npm wrapper's 60s kill, so each launchctl call
is bounded, and a job left unloaded says so instead of claiming a restore.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

/agentic_review

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
Comment thread src/Capacitor.Cli/Services/LaunchdServiceManager.cs Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 3b636ee

realtonyyoung and others added 2 commits October 1, 2026 11:18
The npm refresh wrapper SIGKILLs at 60s, and a kill between bootout and
bootstrap leaves the job unloaded with no rollback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tstrap

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

/agentic_review

Comment thread src/Capacitor.Cli/Services/ProcessTypeRefresh.cs Outdated
Comment thread src/Capacitor.Cli/Commands/DaemonServiceCommands.cs
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6c8d80b

…dard

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 9e3b069

… time

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d8f15e0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant