Repository navigation
Update script installs in place and add kcap refresh - #1340
Conversation
A daemon unit written from a script install pins its version directory, so service refresh moves it onto the current link; otherwise the daemon would never follow an update. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR Summary by QodoAdd verified self-updates for script installs and unify integration refresh
AI Description
Diagram
High-Level Assessment
Files changed (51)
|
![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: d98d5189ed
ℹ️ 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".
| // A leftover directory of this version is not current (only an older one is), so nothing runs from it. | ||
| var dest = layout.VersionDir(version); | ||
| try { | ||
| if (Directory.Exists(dest)) Directory.Delete(dest, recursive: true); |
There was a problem hiding this comment.
If an older versioned binary runs kcap update after current already points at the requested version—or two updates overlap—dest can be the active installation despite the comment above. This recursive deletion then removes files used by the current CLI, daemon, and integrations before activation completes; on Windows, locked executables can also leave the active directory partially deleted. Resolve and reject the active target or serialize activation before removing an existing version directory.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c847708. Requesting the active version now installs nothing and only rewrites the marker. Any other existing versions/<v> is renamed to versions/.replaced-<v>-<guid> and kept, never deleted in place, so a process still running from it keeps its files. A rename that fails (Windows, locked binary) installs nothing.
| if (binary is null || SystemdUnit.WithBinary(text, stabilize(binary)) is not { } rewritten) return false; | ||
|
|
||
| _writeUnit(path, rewritten, null); | ||
| _runProcess("systemctl", SystemdUnit.DaemonReloadArgs()); |
There was a problem hiding this comment.
When systemctl --user daemon-reload exits nonzero, _runProcess returns that failure but it is discarded, so refresh reports success even though systemd still has the old ExecStart cached. A later refresh sees the already-rewritten file and skips daemon-reload, allowing the next daemon start to keep using the old version indefinitely. The inspected systemctl --help describes daemon-reload as “Reload systemd manager configuration,” so its exit status must be checked and surfaced for retry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in c847708. Repoint throws when daemon-reload exits non-zero, and daemon service refresh reports that unit as failed. A unit already on the stable path is reloaded when systemctl show --property=NeedDaemonReload says yes, so the next refresh retries a reload that failed after the rewrite.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
The version being installed can already be on disk and running, so it is moved aside, and the active one is left alone. A script install's channel comes from install.json: the profile defaults to latest, so it cannot say whether stable was chosen. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Part of #1227 — AI-3354
What & why
A script install can update itself:
kcap updatereads the installer's channel manifest, checks the archive's sha256 against it (a mismatch installs nothing), extracts it intoversions/<v>and switchescurrent. Onekcap refreshreplaces the refresh lists the npm launcher and the install scripts each carried;refresh.jsnow just calls it.kcap-web follow-up: in
install.shandinstall.ps1, replace theTODO(#1227)plugin install --if-installedloop with"$kcap" refresh, then point the quickstart and setup-cli docs at the script installer.Where to look
versions/<v>/bin/kcap-daemon.daemon service refreshmoves it ontocurrent: launchd reloads only an idle daemon, systemd only rewrites the unit and runsdaemon-reload, so the new path applies on the next start.RestartCoordinatorwatches the binary throughcurrent, so a link switch triggers the idle restart.Verification
dotnet build Capacitor.slnx: 0 warnings.dotnet publish -c Releaseofkcapandkcap-daemon: no IL2026/IL3050.currentuntouched, and the launchd/systemd repoint.NudgeLeaseTests,RepositoryDetectionTrackedBranchTests, one Core test), all green alone.node npm/kcap/bin/{refresh,kcap,resolve}.test.js: ok.install.ps1on Windows.🤖 Generated with Claude Code