fix(update): update and verify active install#942
Conversation
781dddb to
6e23a7d
Compare
|
The core of this is good. The native update path with SHA-256 verification, the executable version check at the active path, and the structured error codes are a real improvement over the old spawn-npm-regardless behavior. Most of the PR is correct and should land. A few correctness items worth addressing before merge. One. Two. The new Three. The Rust daemon's Four. Five. The docs, install.sh hardening, and release workflow improvements all look correct. No issues there. |
Assisted-by: OpenAI Codex:gpt-5
6e23a7d to
bf5c196
Compare
|
Thanks for the detailed review — I addressed the five points.
After exhaustion, the result remains Installer and verifier subprocess environments now remove
Package-manager ownership inference is now centralized in one internal core helper used by both primary package-manager resolution and installation detection. The shared inference uses the apparent path, resolved path, platform, home directory, and explicit npm, Bun, pnpm, and Yarn roots. Broad substring checks such as Bun's configurable
The Rust daemon remains intentionally fail-closed with
The parser remains strict and now follows SemVer identifier rules, including prerelease and build metadata while rejecting leading zeros, empty identifiers, and prefixed output such as The same validation is shared with target-version normalization. The build path sets the embedded package version, and Commander prints that value directly followed by a newline. A real rebuilt-native check also confirmed that verification ignores a stale inherited
The documentation now distinguishes the exact guarantees:
It also states that these checks establish consistency with the manifest, not independent authenticity. Assets and manifests are not currently signed; the present trust anchor is the GitHub Actions release workflow, repository release permissions, and HTTPS delivery. Signatures and attestations remain separate work. Validation completed with 62 focused tests, full typecheck, targeted Biome checks, the complete build, a native rebuild, and a real stale-version runtime verification. |
|
Thanks Rémi, all five points look good. The centralized path inference in package-manager-path.ts is the right call. Catching the SIGNET_VERSION env leak on top of point four was a good get. Good to merge from my end. |
Summary
Fix
signet update installso it updates the installation owning thecurrently active executable, verifies the exact installed version at its
canonical path, and only then requests a daemon restart.
Package-manager installations remain supported. When a native installation
is active alongside an inactive npm, Bun, pnpm, or Yarn installation, Signet
reports the conflict without removing anything automatically.
Changes
package-manager installations.
manifest, platform, size, SHA-256, timeout, and atomic replacement checks.
--versionbefore reporting successor setting
pendingRestartVersion.unsupported_runtime_updateuntilverifiable native replacement is packaged for that runtime.
signet doctordiagnostics.opt-in, and always pass
install --force.to stable.
Type
Packages affected
Screenshots
N/A — CLI, daemon API, installer, and release workflow changes only.
PR Readiness (MANDATORY)
INDEX.md+dependencies.yaml); noself-update feature dependency changes are required.
duplicate installation warnings, integrity failures, and stable/nightly
installation.
baseline limitations are documented below.
Migration Notes
previous native release explicitly.
Testing
bun test— the full suite was started but not completed after unrelatedbaseline/sandbox failures involving protected home directories, local
listeners, and stale generated parity/content indexes.
bun run typecheckbun run lint— the repository-wide command currently reports theexisting baseline; Biome passes on every touched TypeScript file.
intentionally left unchanged.
Additional successful validation:
bun run buildbun run build:native-buncargo clippy -p signet-daemon --all-targetsrustfmtbash -n web/marketing/public/install.shgit diff --checkcargo fmt --all -- --checkremains blocked by pre-existing formatting inthe untouched Rust
main.rs.AI disclosure
validation, and documentation. The commit contains the required
Assisted-bytrailer.Notes
The production LOC increase comes from the shared installation detector and
the integrity-checked native installer. These replace the silent npm fallback
and false-success runtime path, while the regression tests account for a
substantial part of the overall diff.
No duplicate installation is ever removed automatically.