Skip to content

feat(installer): committable repo-local (--local) install mode#428

Merged
filip131311 merged 39 commits into
mainfrom
feat/committable-install
Jul 8, 2026
Merged

feat(installer): committable repo-local (--local) install mode#428
filip131311 merged 39 commits into
mainfrom
feat/committable-install

Conversation

@filip131311

Copy link
Copy Markdown
Collaborator

Summary

Adds an opt-in committable / repo-local install mode so a team can version Argent with the repo. argent init --local installs @swmansion/argent as a project devDependency and writes MCP configs that launch the project-local copy — so after git clone && npm install, every teammate has the same Argent version and MCP wiring with no global install and no per-developer argent init.

Global mode stays the default and is unchanged; everything here is additive and backward-compatible.

Motivation: the workflow some users already do by hand (#207). Supersedes the earlier, since-closed #232 — this version is Windows-safe (node, not the .bin shim), handles Yarn PnP, and closes the cross-install tool-server hazards rather than leaving them.

What changed

init

  • --local / --global flags + an interactive "How should argent be installed?" prompt. Mutually exclusive; non-interactive (--yes) defaults to global.
  • Local mode installs the devDependency with the package manager detected from the project's lockfile (run with cwd = projectRoot).
  • MCP entry points at the local copy: node node_modules/@swmansion/argent/dist/cli.js mcp (the bin path is derived from the installed package.json bin and existence-checked — forward slashes, committable, Windows-safe). Yarn PnP → yarn argent mcp; an npx --no-install argent mcp fallback is used only when the bin can't be resolved.
  • Records the mode in a committed <root>/.argent/install.json.
  • Drops global-only adapters (Windsurf/Hermes) in local mode; prints team-setup / commit guidance.

update — detects the project's mode; in local mode reads the installed version from node_modules (not the global binary / npx cache) and bumps the devDependency + lockfile, then re-emits the local MCP command.

uninstall — scoped to the project's mode: a local-mode uninstall removes the local devDependency only and never the shared global install, and prunes .argent/install.json. Global-mode behavior is unchanged.

Cross-install hardening (the gate that makes per-project pinning safe)

  • launcher: a tool-server is reused only when its bundlePath matches the caller's — a repo-local pin never silently runs against (or kills) a different version's server.
  • postinstall: only stops a tool-server spawned from this package's own bundle, so a devDependency install can't tear down an unrelated editor session.

Telemetry — adds an install_mode dimension to init/update/uninstall events.

Testing

  • New unit tests: install-mode helpers + record (utils), mode-aware getMcpEntry/resolveLocalCommandMode incl. the opencode array form (mcp-configs), local-only uninstall guard (uninstall), and bundlePath-gated tool-server reuse (launcher).
  • All existing suites green (installer 346, tools-client 115, telemetry 191, registry 83, argent 13, tool-server 1621). Build + lint clean.
  • Live end-to-end in an isolated sandbox: init --local → real MCP stdio handshake serves 69 tools from node_modules; update --local reads the local version; uninstall --local removes the local install and leaves the global install intact; init --global unchanged; flag conflict rejected.

Notes / known limitations

  • Recommended on mainstream platforms (macOS, Linux x64, Windows x64) with prebuilt native binaries; Linux arm64 / Windows arm need a C/C++ toolchain to build tree-sitter on npm install (documented in the README).
  • Concurrent sessions of two different Argent versions on one machine each spawn their own tool-server; an orphaned one idle-times-out (no session is ever broken).

🤖 Generated with Claude Code

`argent init --local` installs @swmansion/argent as a project devDependency
and writes MCP configs that launch the project-local copy, so a committed
setup gives every teammate the same argent on `npm install` — no global
install, no per-developer `argent init`. Global mode stays the default.

- init: --local/--global flags + interactive prompt (non-interactive → global);
  installs the devDep (package manager detected from the project lockfile),
  writes a node + project-relative-path MCP command (Yarn PnP → `yarn argent`;
  `npx --no-install` fallback when the bin can't be resolved), records the mode
  in .argent/install.json, drops global-only adapters, prints team-setup guidance.
- update / uninstall: detect the project's mode. update bumps the devDependency
  (+ lockfile) and reads the version from node_modules, not the global binary;
  uninstall removes the local devDependency only — NEVER the shared global
  install — and prunes .argent/install.json.
- launcher: reuse a tool-server only when its bundlePath matches, so a repo-local
  pin never silently runs against a different version's server (and never kills
  a healthy server from another install).
- postinstall: only stop a tool-server spawned from this package's own bundle, so
  a devDependency install can't tear down an unrelated editor session.
- telemetry: add an install_mode dimension to init/update/uninstall events.

New unit tests (utils / mcp-configs / uninstall / launcher) plus end-to-end
verification: `init --local` → MCP handshake serves tools from node_modules;
update/uninstall act on the local install only; global mode unchanged.
…l-independent)

The committable feature must not depend on the postinstall script, which is
frequently disabled (--ignore-scripts, pnpm onlyBuiltDependencies, Yarn PnP, CI).
A local devDependency bump rewrites tool-server.cjs in place, so the bundlePath
string is unchanged and a still-running old-version server would otherwise be
reused. Record the package version in the tool-server state (provided via
BUNDLED_RUNTIME_PATHS) and refuse to reuse a server tracked under a different
version — the next MCP call respawns the new one automatically. Legacy state with
no recorded version is still reused (no respawn churn on the upgrade boundary).

Covered by a new launcher test (same bundle, different version → respawn).
@filip131311

Copy link
Copy Markdown
Collaborator Author

Follow-up: the feature does not depend on the postinstall script

postinstall is frequently disabled (--ignore-scripts, pnpm onlyBuiltDependencies, Yarn PnP, locked-down CI), so the committable flow is designed to work without it:

  • After npm install, the committed MCP config + the binary in node_modules are all that's needed — the editor spawns node node_modules/@swmansion/argent/dist/cli.js mcp directly. The postinstall change here is purely a defensive guard on the pre-existing tool-server kill (don't tear down a server from a different bundle), not new reliance.
  • The one thing postinstall used to provide — killing a stale tool-server after an in-place version bump so the new binary takes effect — is now handled in the launcher instead (commit e217e6e): the tool-server state records the installed version, and a server tracked under a different version is not reused. So a teammate who pulls a lockfile bump and runs plain npm install self-heals on their next MCP call, with postinstall disabled or not.

Net: postinstall is now a best-effort optimization (faster cleanup when it runs), never a correctness dependency.

@filip131311 filip131311 marked this pull request as draft June 30, 2026 13:20
…itecture)

Behavior-preserving decomposition of the monolithic init.ts (1094 lines) into
the focused-module layout from #242, keeping every bit of this branch's
robustness intact (Windows-safe `node` MCP command, Yarn PnP fallback, launcher
bundlePath+version-aware reuse, postinstall guard, uninstall mode-scoping, the
.argent/install.json record, and the install_mode telemetry dimension).

New modules (single responsibility each):
  package-manager.ts  PM type/detection + global/local install/uninstall commands
  preflight.ts        hasProjectPackageJson, isYarnPnp
  shell.ts            runShellCommand
  topology.ts         GLOBAL/LOCAL abstraction + install-detection probes
  install-record.ts   .argent/install.json + resolveInstallMode / …FromFlags
  init-args.ts        parseInitArgs + the InitCancelled cancel sentinel
  init-telemetry.ts   failure signals + InitTelemetry (centralized finalize/track)
  init-mode-prompt.ts install-mode selector
  install-runner.ts   Step-0 install for both modes
  init-adapters.ts    editor multiselect
  init-scope.ts       scope prompt
  init-mcp-write.ts   per-adapter MCP writes + local-mode adapter filtering
  init-allowlist.ts   tool auto-approval step
  init-skills.ts      skills step + runNpxSkills

init.ts 1094 -> 316 (thin orchestrator that reads like a recipe). utils.ts keeps
the path/skills/version helpers and re-exports every moved symbol, so all
existing import sites and tests resolve unchanged — zero test files edited.
Cancel telemetry is preserved by throwing InitCancelled(step) from the step
modules and emitting cli_init_cancel in the orchestrator's catch.

Verified: build + lint clean; argent-installer 346, argent-tools-client 116,
telemetry 191, registry 83, argent 13 all pass; and live end-to-end —
init --local writes the node-based MCP command + install.json and the committed
command serves 69 tools; init --global is unchanged; uninstall --local removes
only the local devDependency (global install survives); --local/--global conflict
is rejected.
@filip131311

Copy link
Copy Markdown
Collaborator Author

Adopted #242's clean module architecture (commit c2f83ae)

Per discussion, rather than carry forward the stale #242 branch (112 commits behind main, on the closed #232 base), I brought #242's focused-module architecture onto this branch — so #428 now has both the robustness and the clean structure, on live main.

init.ts went 1094 → 316 lines (a thin orchestrator that reads like a recipe). New single-responsibility modules mirror #242: package-manager.ts, preflight.ts, shell.ts, topology.ts (GLOBAL/LOCAL abstraction), install-record.ts, init-args.ts, init-telemetry.ts, init-mode-prompt.ts, install-runner.ts, init-adapters.ts, init-scope.ts, init-mcp-write.ts, init-allowlist.ts, init-skills.ts. utils.ts re-exports every moved symbol so all import sites/tests resolve unchanged (zero test files edited).

Behaviour-preserving: build + lint clean, all suites green (installer 346 / tools-client 116 / telemetry 191 / registry 83 / argent 13), and live-verified end-to-end (init --local serves 69 tools from node_modules; init --global unchanged; uninstall --local leaves the global install intact). Credit to #242 for the structure.

…er-bundle registry, PnP/workspace probes

Review fixes for the committable-install PR:

- launcher: replace the single-slot ~/.argent/tool-server.json with a
  per-bundle registry (tool-server-<hash>.json). Each install tracks its
  own server, so a foreign-bundle spawn no longer clobbers another
  install's record and orphans a server nothing can reach — including
  never-expiring `argent server start` (cli-managed) servers. The legacy
  file is still read (and cleared when it records the caller's bundle)
  for pre-registry compat; dead-pid records are swept opportunistically.
- launcher: add killToolServerForInstallDir(dir); `argent update` /
  `argent uninstall` now stop only the server(s) spawned from the install
  they are mutating, instead of whatever pid the shared slot tracked —
  the same invariant the postinstall sameBundle gate enforces.
- server CLI: status/stop are scoped to this binary's bundle and hint at
  servers belonging to other installs instead of touching them.
- postinstall: scan all state records (legacy + per-bundle), still gated
  on sameBundle.
- topology: resolve the local install via Node module resolution from the
  project root (hoisted workspaces, pnpm symlinks) instead of a hardcoded
  <root>/node_modules path, and treat a declared dep under Yarn PnP as
  installed. probeLocalInstall returns {installed, version, packageDir};
  update/uninstall use it so PnP projects no longer reinstall on every
  `update --yes` and can actually be uninstalled.
- install-record: infer local mode only from a dependency the project's
  package.json declares — mere node_modules presence (hoisted transitive
  dep, workspace symlink) no longer flips update/uninstall into local
  mode and rewrites a manifest the user never opted into.
- install-record lifecycle: uninstall removes .argent/install.json when
  the devDependency is removed (not only under prune); `init --global`
  clears a stale local-mode record.
- init: local mode excludes global-only editors from the selection prompt
  (they were offered with a "global config" hint and then silently
  dropped at write time) and warns loudly when zero MCP configs were
  written instead of printing an empty success summary.
- package-manager: honor the corepack packageManager field and walk up to
  a workspace-root lockfile (stopping at .git) before falling back to the
  npm_config_user_agent heuristic.
- mcp-configs: extract getMcpEntryForScope so init and update share one
  mode-and-scope → MCP command decision; drop the unused Topology
  abstraction from topology.ts.
Correctness:
- init: honor a committed .argent/install.json when resolving mode
  non-interactively and seed the interactive prompt from it, so `argent
  init -y` in a local-mode repo no longer silently reverts it to global.
- topology(getLocalArgentBinRelPath): commit the stable
  node_modules/<pkg> path instead of Node's realpath, which under pnpm is
  the version-pinned .pnpm store dir that dies on the next version bump.
- topology(getGloballyInstalledPackageRoot): validate the walked-up
  package.json is actually argent's, so a stray manifest (e.g. a Windows
  .cmd wrapper resolving to a ~/package.json) can't scope tool-server
  teardown to unrelated installs.
- update: in local mode with nothing in node_modules (fresh clone, or a
  manifest-less root), stop instead of running `<pm> install @latest` —
  which rewrites the team's committed pin or walks up and mutates an
  ancestor project — and tell the user to install first.
- install-runner: gate the "already a devDependency" skip on a manifest
  declaration (isDeclaredLocally), not mere resolvability, so a hoisted
  workspace/transitive copy no longer skips the add.
- launcher: only refuse reuse (forcing a kill+respawn) of a same-bundle
  server when the caller is strictly NEWER; reuse a newer tracked server
  instead of killing it, ending the version ping-pong between two live
  sessions across an in-place bump.
- launcher(sweepDeadStateFiles): re-read each state file immediately
  before unlinking so a concurrently-written live `--detach` record isn't
  deleted.
- cli(server): pass the recorded token to the health probe in
  ensureNoExistingServer so a live auth-enabled server isn't misread as
  unhealthy and orphaned on `server start`.
- tool-server(update-argent): spawn the running install's own dist/cli.js
  (not bare `argent`, absent from PATH in local mode) and add an 'error'
  listener so a failed spawn can't crash the tool-server via
  uncaughtException.
- update: disclose which install (local vs global) `update` targets
  before it mutates anything.

Telemetry:
- classify the `--local`/`--global` conflict as a new
  INSTALL_MODE_FLAG_CONFLICT signal instead of mislabeling it as a
  local-install precondition failure.

Tests: mode-record honoring, pnpm stable-path, non-argent kill-root
rejection, and newer-server reuse. All affected suites green.
@filip131311 filip131311 marked this pull request as ready for review July 3, 2026 14:53
@latekvo latekvo self-requested a review July 3, 2026 14:54

@latekvo latekvo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the committable repo-local install mode closely, including a full end-to-end run on the built binaries (v0.14.0 packed and installed as a local devDependency):

  • init --local commits node node_modules/@swmansion/argent/dist/cli.js mcp plus an .argent/install.json, and that command serves the full tool set (71 tools) over a real MCP stdio handshake, spawning a tool-server whose per-bundle state is correctly scoped to the local install (never touching a coexisting global one).
  • update --local reads the project-local version rather than the global/npx one; uninstall --local removes only the local devDependency and its marker and leaves a global install intact.
  • Build, all affected suites (installer / tools-client / telemetry / registry / tool-server), and both typecheck:tests gates are green; the default global install/update/uninstall flow is preserved.

The cross-install hardening (per-bundle state registry, bundlePath-gated reuse, scoped teardown, token-aware health probe, PnP/workspace-aware probes) is coherent and well reasoned.

The inline notes below are all minor and non-blocking — a couple of edges in the new version-aware server self-heal, one high-risk flow (update()) without a driving test, and a rare Windows path case. None of them affect the common paths, which work correctly. Approving; worth a glance at the inline points before merge.

wantVersion !== undefined &&
state.version !== undefined &&
state.version !== wantVersion &&
isVersionNewer(wantVersion, state.version)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isVersionNewer compares only the numeric major.minor.patch and ignores any prerelease suffix, so this gate treats two versions that share a base but differ only in a prerelease identifier as equal. The next canary channel publishes <patch>-next.<counter> on every push to main, so consecutive canaries — and the final -next.N build vs the stable release at the same base — all compare as not-newer here. When a project pins such a version through a committed lockfile and it is bumped in place at a stable bundlePath (npm-local, or npm/yarn-classic global; pnpm and Yarn PnP encode the version in the path and are unaffected) while a server from the previous build is still alive and install scripts are disabled (--ignore-scripts / locked-down CI — the case this version field exists to cover), the launcher keeps reusing the old server and serves stale tool-server code until the idle timeout. The version-aware self-heal silently does not fire for canary-channel installs.

* with state files written by older versions (treated as "unknown" — reused
* rather than forcing a respawn). See reusableHandle.
*/
version?: string;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This field is recorded only by the auto-spawn path; argent server start (runDetached and runForeground in packages/argent-cli/src/server.ts) writes its state with bundlePath and managed: "cli" but no version, though paths.version is available there. Because the reuse gate only compares versions when state.version !== undefined, a CLI-started server always falls into the version-unknown branch and is reused regardless of the caller's version. So if a long-lived CLI server is running when the install is bumped in place (without argent update or the postinstall kill), the MCP auto-spawn path reuses that stale server instead of spawning a fresh one — and this affects any version bump, not only the prerelease case.

Comment thread packages/argent-installer/src/update.ts Outdated
// package.json the package manager walks up and mutates an unrelated
// ancestor project. Never auto-mutate; tell the user to materialize the
// install themselves.
if (installMode === "local" && localProbe && !localProbe.installed) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The local-mode branches added to update() — this "declared but not materialized → stop rather than run <pm> install @latest" guard, and the PnP versionUnknown / needsInstall logic below it — are only exercised by calling update(args), but update.test.ts covers only the pure helpers (getUpdateTriggerFromEnv, resolveUpdatePackageAction); nothing drives update() itself. The sibling uninstall() has a direct test for its analogous local-only guard, so this is the one new high-risk flow (it can rewrite a team's committed version pin) left without behavioral coverage.

// committable, just less resilient to an in-place version bump.
const abs = path.join(pkgDir, binSub);
if (!fs.existsSync(abs)) return null;
return path.relative(root, abs).split(path.sep).join("/");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback runs when the package isn't under the project's own node_modules (a hoisted workspace). On Windows, path.relative(root, abs) returns an absolute path when root and the resolved package dir are on different drives, and that absolute path is then committed into the MCP config — machine-specific, so it breaks for any teammate who clones the repo. Narrow (node_modules on a separate volume from the project), but the committed command would be silently non-portable.

update/uninstall inferred a single install mode from the project
(resolveInstallMode), so a developer with argent installed globally AND
as a project devDependency was silently routed to the local one —
`update` left their global stale and `uninstall` removed the project's
devDep instead of the global install.

Add explicit target selection shared by both commands (install-targets.ts):
  - --global / --local flags win and are additive (pass both = both)
  - exactly one install present -> act on it (unchanged behavior)
  - a global install and a project-local install coexisting ->
    interactive multiselect (both preselected); non-interactively fall
    back to the local install (interim; a fuller default is planned)

Selection scopes only the package action; config/skill refresh and the
workspace cleanup are unchanged, and the historical single-install paths
are byte-for-byte preserved. decideInstallTargets is a pure resolver with
full unit coverage.
…bsent-target flags

update-argent (the MCP tool the agent calls) spawned its own cli.js, but
`update` then re-resolved the target from cwd — so a running GLOBAL server
in a repo that declares argent locally updated the wrong (local) install.
Classify the running install (its bundle dir vs the project's node_modules,
walking up from cwd for hoisted/pnpm layouts) and pass an explicit
--global/--local so the update hits exactly the install serving this
session. Add a `target: auto|global|local|both` param so the agent can
choose, and hint at the other install when auto updated only one.

Also soften the new flags: --global/--local on update/uninstall no longer
hard-error when the named install is absent. `update --global` installs a
missing global (as it already did), `update --local` on a non-dependency
points at `argent init --local`, and `uninstall` reports nothing to remove.
This mirrors how update already treated a missing global.
… scripts

pnpm 10+ exits non-zero (ERR_PNPM_IGNORED_BUILDS) when a freshly-added
dependency has build/postinstall scripts it blocks by default. argent has
several (its own postinstall + tree-sitter/node-pty/webtransport), so
`argent init --local` and `argent update` aborted before writing any config
-- leaving a half-configured repo -- even though the package was fully
installed and works (the committed node-path MCP command serves tools
regardless of those scripts).

Decide success from the DISK, not the exit code: after the add/update,
verify the package actually landed (init: resolvable in node_modules;
update: the target version is on disk) and treat a non-zero exit as fatal
only when it did not. A note points pnpm users at the optional
`pnpm approve-builds`. Genuine failures (bad spec, nonexistent version,
network) still abort with nothing written.

The update verify reads the version cache-free
(readLocalPackageVersionUncached) because Node's module-resolution realpath
cache otherwise reports the pre-update version after an in-process bump.
Giving update-argent a zodSchema typed its execute() params as
{ target?: ... }, so the existing tests passing `undefined` for params no
longer typecheck (the runtime handles it via `params?.target`, which is why
the vitest run stayed green while the CI typecheck:tests step failed). Pass
`{}` instead, matching the convention for tools with a params schema.
@latekvo latekvo self-requested a review July 6, 2026 16:12
…ck on update

In a fresh clone (or behind a pruned pnpm store dir masked by Node's
realpath cache) resolveLocalCommandMode resolves local-npx, and the
config refresh replaced the team's committed node-path command with
'npx --no-install argent mcp'. Leave existing project entries alone in
that state and say so; init still writes the fallback (with its own
warning) when configuring from scratch.

Also report 'update --version <older>' as a not-newer no-op instead of
'Already on the latest version.'
…hosen target

Package removal honored --local/--global, but MCP-entry, allowlist, and
skills/rules/agents cleanup stayed workspace-wide — 'uninstall --local'
deleted the argent entry from every global editor config, unwiring the
global install the user chose to keep (and --global dirtied the
committed project files). When the target was explicitly narrowed
(flags or the coexistence prompt), clean only the scopes that run the
removed install: local devDep -> project entries; global binary ->
global entries, plus project entries when the project is in global
mode. The no-flag single-install path keeps the historical
workspace-wide behavior, and .argent/install.json now survives a
--global-only uninstall of a local-mode project.
resolveInstallMode treats a devDependency the project's own manifest
declares as local intent, but init's recordedMode read only
.argent/install.json — so 'init -y' in a repo whose .argent/ was never
committed silently rewrote the committed local-command MCP config back
to the bare global 'argent' command. Seed init from record -> manifest
declaration, matching update/uninstall (re-running init also re-heals
the missing record).

Also clear a stale local-mode marker at the resolved project root —
where update/uninstall will look for it — not only at a custom scope
root.
… teardown

killToolServerForInstallDir signalled whatever pid a tracked record
named — records for retired installs are swept only opportunistically,
so a stale record's pid can be recycled onto an unrelated process by
the time update/uninstall runs. Re-read each state file before acting
(the snapshot can be republished concurrently, same reasoning as
sweepDeadStateFiles) and gate the kill on processCommandMatches with
the stillOurs re-check, mirroring ensureToolsServer's wedged-server
kill. Windows keeps the historical unguarded kill: ps is unavailable
there, so the guard would silently turn teardown into a no-op.
…e acts on both

A committed mode:'local' record whose devDependency isn't materialized
(fresh clone) used to shadow a present global install: 'update -y' /
'uninstall -y' targeted the missing local dep, warned, and left a
possibly outdated global binary untouched — a behavior change vs main
in that directory. Default the target to the install that is actually
present (local if materialized, else global if on PATH, else the
recorded mode), with a one-line hint when the declared local dep isn't
installed yet.

When both installs coexist, non-interactive 'update' now updates both —
updating is safe and matches the interactive prompt's both-preselected
default (the agent-triggered update still pins explicit flags).
'uninstall -y' keeps removing only the local devDep: removal is
destructive and the global install is shared with other projects, so
nuking it still requires an explicit --global.
…m the server's cwd

update-argent's 'auto' target walked up from the tool-server's inherited
cwd to decide global vs local. A detached server's cwd is whatever the
editor set for the MCP process — '/' or $HOME misclassifies a repo-local
install as global, and an auto-update the user consented to for their
local install would fresh-install a global package they never opted
into.

Classify in the argent package at process start instead — the moment
cwd is trustworthy, since a committed local MCP command
('node node_modules/.../cli.js mcp') only resolves with cwd at the
project root — and forward the result through ToolsServerPaths as
ARGENT_INSTALL_KIND in the spawned server's env. update-argent prefers
the env signal and keeps the cwd walk only as a fallback for servers
spawned by older argent versions.

@latekvo latekvo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two inline notes from going through the local-install flow — both on specific lines below.

// non-PnP layout with no node_modules entry means the add really failed (a bad
// spec, ERR_PNPM_ADDING_TO_ROOT, a network error) — don't write a config that
// runs a missing binary.
if (!isLocallyInstalled(projectRoot) && !isYarnPnp(projectRoot)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This post-install check treats mere resolvability as proof the add succeeded. When the project's own package.json does not declare @swmansion/argent (so the reuse guard at line 90 is skipped and an add is attempted) but a resolvable copy already exists in node_modules — a hoisted sibling/transitive dependency in a monorepo, or a stale copy left from a prior run — and the npm/yarn/pnpm add actually fails (network error, ERR_PNPM_ADDING_TO_ROOT, a bad spec), isLocallyInstalled(projectRoot) still returns true, so this fatal branch is skipped and the failure is swallowed.

init then prints "Added @swmansion/argent to devDependencies", attributes the non-zero exit to pnpm's blocked build scripts, writes .argent/install.json (with writtenBy set to the stale version), writes the MCP config, and exits 0 with is_success: true telemetry — while package.json was never modified. The committed "team-share" setup is therefore silently unbacked: a teammate's or CI's npm install won't install argent, and if the stale copy is ever pruned the committed node node_modules/…/cli.js mcp command points at a missing file. This is the exact hazard the reuse-guard comment at lines 84-89 was written to avoid, reintroduced through the disk-resolvability check.

Reproduced: a project package.json without argent + a stale node_modules/@swmansion/argent (v0.9.9-STALE) + a failing add (dead registry) → "Added…" printed, .argent/install.json written with writtenBy: "0.9.9-STALE", package.json unchanged, exit 0.

* major.minor.patch; ignores prerelease tags). Anything unparseable compares as
* "not newer" so the caller reuses rather than kills — the conservative choice.
*/
function isVersionNewer(a: string, b: string): boolean {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isVersionNewer compares only the numeric major.minor.patch and ignores the prerelease tag, so the version-based reuse gate never refuses reuse when the numeric triple is unchanged. That gate is the sole in-place-bump self-heal for a tracked server whose bundlePath does not change across the bump — i.e. same-path layouts such as the global npm install or a flat (non-pnpm) local node_modules, where tool-server.cjs is rewritten in place. For the next prerelease channel this repo publishes on every push to main, in-place bumps are …-next.N → …-next.N+1 (same triple), and a …-next.N → stable bump lands on the same triple too. In all of those, reusableHandle treats the running server as current.

So after such a bump — with the postinstall kill disabled (--ignore-scripts / pnpm onlyBuiltDependencies) and without an explicit argent update — a long-lived autospawn tool-server keeps executing the previous bundle's code until it idle-times-out (and a managed:"cli" server, indefinitely), even though the on-disk bundle was replaced. The ToolsServerPaths.version / bundled-paths.ts comments frame this same-path in-place rewrite as exactly the case the version field is meant to self-heal, which is where the prerelease gap bites. (pnpm is not affected: its versioned store path changes the bundlePath, so the state.bundlePath !== wantBundlePath branch respawns independently of this gate.)

Verified against the built algorithm: isVersionNewer("0.14.0-next.6","0.14.0-next.5") and isVersionNewer("0.14.0","0.14.0-next.9") both return false (while isVersionNewer("0.14.1","0.14.0") is true), so reuse is not refused across a same-triple prerelease bump.

…owed uninstalls

The scoped cleanup only engaged for explicit --local/--global or a prompt
pick, so the two implicit narrowings still cleaned workspace-wide:
'uninstall -y' with coexisting installs removed the local devDep but
stripped the KEPT global install's editor configs everywhere, and in a
fresh clone (record committed, dep not materialized) it deleted the
team's committed project entries and .argent/install.json while removing
the present global. Derive the protected scopes from what is RETAINED
instead of how the targets were chosen: a present global install not
being removed keeps its global-scope (and, in global mode, project-scope)
entries; a local-mode project keeps its committed project entries unless
the local install itself is being removed. A single install with nothing
retained still cleans both scopes — the historical behavior.

Covers all three scenarios with new uninstall tests.
…roject install

init -y in a fresh clone (argent declared, node_modules empty — the
manifest-seeded local mode from 1b9c282) ran the package manager's ADD
with a bare package name, silently rewriting the team's committed version
pin to @latest — the exact mutation update refuses on principle. When the
manifest already declares argent, run the plain 'npm install' (pnpm/yarn/
bun install) instead: it honors the committed pin and lockfile. The add
form remains for undeclared projects and the --from tarball dev flow.
…an an unverified live server

processCommandMatches split the ps command line on whitespace, so a
bundle path containing a space ('~/My App/…') could never match its own
process — killToolServerForInstallDir then skipped the kill but still
deleted the record, leaving the old server running, untracked, and
unreachable by 'argent server stop'. Match the raw command string at an
argument boundary ('<bundlePath> start') instead, and when a LIVE pid
still cannot be positively identified, keep its record rather than
unlinking — a truly stale record is swept once the pid dies.

Adds a test that a live-but-unidentifiable pid is neither signalled nor
untracked (skipped on win32, where the guard is deliberately off).
classifyInstallKind read process.cwd() outside its try block, at module
import time — before any fatal handler is installed — so running any
argent command from a directory deleted after the shell entered it threw
an unhandled ENOENT (uv_cwd). Classify as global in that case; there is
no project to be local to.
…nfigured

The dual-root cleanup removed the cwd project's committed
.argent/install.json even when init --global targeted a DIFFERENT custom
scope root — deleting another project's committed marker the run never
configured. Clear at the effective root only; for local/global scope that
IS the resolved project root where update/uninstall look.
…geting semantics

update/uninstall now default to the PRESENT install, 'update -y' acts on
both coexisting installs, and --global/--local scope config cleanup too —
the CLI help, README table, local-mode section, DecideTargetsContext docs,
and resolveInstallModeFromFlags doc still described the earlier interim
semantics. Also gate update's 'Left project MCP entries unchanged' note on
a project-scope entry actually existing, and mention the out-of-band-bump
cause alongside the fresh-checkout one.
topology.ts imported resolvePackageRoot from utils.ts — the barrel that
re-exports topology — a genuine ESM cycle that only worked because the
import was called lazily while utils.ts computes PACKAGE_ROOT at module
init; one eager reference away from a TDZ error. Move resolvePackageRoot
to a leaf package-root.ts module; utils re-exports it so existing import
sites keep resolving.
…the skill-refresh report

Two drift-prone blocks existed twice each, in install-runner.ts and
update.ts:

- The pnpm 10+ 'decide success from the disk, not the exit code' policy
  (ERR_PNPM_IGNORED_BUILDS) — now shell.ts runTrustingDisk(execute,
  landedOnDisk); callers keep their own probes, messaging, and telemetry.
- The 13-line skill-refresh + 'Skills Updated' note +
  skill_refresh_result tracking — now skills.ts reportSkillRefresh(root,
  stage). This also collapses the two INSTALL_SKILLS_REFRESH_FAILED
  definitions (same code and kind, drifted stage strings) into one; the
  per-flow failure_stage values are preserved via the stage parameter, so
  emitted telemetry is unchanged.
…fresh install

A leftover entry from a previous install can silently defeat the one init
just wrote: Claude Code's local scope (projects[root].mcpServers in
~/.claude.json, the default target of `claude mcp add`) outranks every
scope init can write; a recorded .mcp.json rejection
(disabledMcpjsonServers) blocks the project entry outright; Cursor and
VS Code leave same-name precedence undocumented; Codex and opencode
deep-merge per key, so fields of a stale global entry (env, enabled,
timeouts) leak into the project entry; Zed runs only the global entry
until the worktree is trusted. Net effect after a global→local
migration: tools silently absent, no error anywhere.

Add a stale-config sweep (init step 1d, also run after update's config
refresh):

- McpConfigAdapter.getArgentEntry() on every adapter (hasArgentEntry is
  now derived) so shared policy can judge an entry's command shape
  without format knowledge.
- Optional findShadowingConfigs() hook for hidden scopes: Claude Code's
  local scope + disabled lists (project keys matched through symlink and
  case variants via realpath; removal re-reads and refuses to write an
  unparseable ~/.claude.json, which also holds OAuth state), VS Code's
  user-profile mcp.json (stable + Insiders, all platforms).
- Policy in init-stale-config.ts: auto-remove findings confined to the
  project; remove bare-`argent` entries anywhere only when argent is no
  longer on PATH (provably dead for every project, so this cannot break
  another workspace); warn — never touch — anything else (custom
  commands, committed project files, working global installs).
- Every action is printed in a "Stale Config Cleanup" note; new
  installation:stale_config_cleanup telemetry event records counts.
…project

The dead-global-entry sweep judges "dead" by probing PATH in the shell
running init. A version-manager split (argent globally installed under
nvm's default node while init runs under another version) can fail that
probe while editors launched elsewhere still resolve the binary —
auto-removing would then silently break argent in other workspaces.

Queue every removal that touches a global (cross-project) config file
and ask once, listing each entry, before executing (default yes).
Project-confined removals — the Claude Code local-scope entry and
recorded .mcp.json rejections that motivated the sweep — still run
unprompted; they cannot affect other projects. Non-interactive runs
(-y, and both callers when the flag is set) proceed without the prompt,
and a declined or cancelled prompt keeps the entries with a per-entry
warning line instead of aborting init.
…rride

The stale-config sweep marked ANY argent entry in ~/.claude.json's
project-local scope autoRemove, so a deliberate hand-tuned override (a
dev-checkout command, extra args, env vars) — which outranks the
committed entry by design — was deleted silently, including by the
agent-triggered non-interactive `argent update --yes`.

Auto-removal is now limited to the stock `argent mcp` shape (no env);
anything customized goes through the shared warn / dead-confirm policy,
matching how custom-command global entries are already treated.
…mpt is declined

The multi-target rework turned the decline path from cancel-and-exit
into a plain return, so answering "no" still ran the config refresh —
entry rewrites, allowlist refresh, the stale-config sweep's removals,
rules/agents, and the skills refresh — mutating files the user had just
refused to have touched.

applyUpdateForTarget now reports updated/declined/noop; when at least
one target was declined and none was updated, the run completes
telemetry, prints the cancel notice, and exits 0 before the refresh —
the pre-multi-target behavior. A partial run (one target declined,
another updated) still refreshes, since the applied update needs its
configuration re-emitted.
Review findings on the committable-install branch, destructive-config cluster:

- The stale-config sweep never removes cross-project entries without an
  explicit confirmation: non-interactive runs (--yes, incl. the
  agent-triggered update) now report them with guidance instead of
  deleting on a fallible PATH probe. Entries carrying env vars are never
  'provably dead' (the env can make the command resolvable in the
  client), and a working env-tuned stock entry stays as quiet as an
  env-less one.
- Removing a recorded .mcp.json rejection writes the user's Claude
  settings file back verbatim instead of routing it through the
  prune-and-delete path that could erase the whole settings.json (and an
  emptied .claude dir) when only empty scaffolding remained.
- normalizeServerEntry surfaces env vars (opencode: environment) so
  classification can see hand-tuned entries, and a new
  isArgentManagedEntry names the exact shapes argent itself writes -
  including the hoisted-workspace and pnpm-store fallback node paths.
- The uninstall coexistence multiselect preselects only the local
  devDependency, matching the non-interactive default; Enter-through-
  defaults no longer removes the shared global package.
- init --global keeps a committed .argent/install.json while the local
  install is still declared (coexistence add), with explicit conversion
  guidance, instead of deleting the team's marker unconditionally.
…d multi-target runs

- update's config refresh classifies every configured entry BEFORE
  writing: customized entries (dev-checkout commands, extra args, env)
  are left untouched and reported, so the old rewrite-to-stock can no
  longer launder them into a bare shape the stale sweep would then
  delete. Managed and corrupted (unparseable-sentinel) entries are still
  rewritten unconditionally - the write also repairs state the
  normalized view can't see (opencode's enabled:false).
- The landed-on-disk probe now cuts both ways: a zero-exit install whose
  target version did not land (npm prefix/PATH split) fails the target
  instead of reporting 'Update complete'; an unreadable landed version
  (Yarn PnP) still defers to the exit code.
- A hard failure on one target no longer process.exit(1)s mid-loop: the
  remaining targets still run, a partially-updated run still refreshes,
  and the failure resurfaces in the terminal telemetry and exit code.
  --version is validated once, before the loop.
- Local-target intent rule shared by update and uninstall: a resolvable
  copy with neither a committed .argent record nor a manifest
  declaration (hoisted transitive dep, workspace symlink) is never acted
  on - no pm run that would rewrite an unopted manifest/lockfile.
  A record-only project (declaration hoisted to the workspace root)
  stays updatable: the record wins, as in resolveInstallMode.
- update accepts an explicit --project-root pin (used by the
  agent-triggered update-argent tool) so a monorepo marker-walk can't
  re-derive a different ancestor than the manifest-proven root.
- Package-manager runs in update/uninstall go through a shared
  Windows-safe spawn: bare name through a shell on win32, resolving both
  .cmd shims (npm/yarn) and native .exe managers (bun) via PATHEXT.
- init's already-installed update offer compares the registry against
  the GLOBAL install's version (never the npx cache's), warns and skips
  when that version can't be read, and records the global version after
  installs.
- The telemetry identity is erased when the global package is removed or
  when a removal leaves no global install behind (last-install case) -
  but no longer on a local-only removal that keeps a global install in
  active use.
… update targets

Launcher:
- The tool-server reuse gate compares the tracked server's version
  against the bundle's CURRENT on-disk package version (read fresh every
  call, and once more right before a spawn records state), so
  downgrades and prerelease bumps retire a stale server in BOTH
  directions - while two long-lived sessions frozen at different
  versions still agree on the one server that may live (no SIGTERM
  ping-pong). The frozen caller version remains only as the fallback
  when the disk is unreadable, and isVersionNewer now implements real
  semver precedence (prerelease identifiers included) instead of
  truncating tags.

update-argent tool:
- The running install's up-to-date/installability gates apply only when
  the (post-degradation) target IS the running install; explicit
  cross-install targets reach the installer, which re-checks each target
  against the registry.
- A local-targeted update pins WHERE explicitly: the spawn passes
  --project-root (never a cwd that may have vanished), the recorded
  ARGENT_PROJECT_ROOT is re-validated on disk, and when no project can
  be proven the tool refuses (local) or degrades with a note (both)
  instead of claiming an update it can't deliver. The declaration walk
  accepts optionalDependencies, matching the installer's probe.
- A no-op updater exit resets the in-progress flag so later calls
  aren't blocked forever.
- bundled-paths records the DECLARING project root (manifest or .argent
  record, walking up from the spawn-time cwd) rather than the physical
  hoist root, so hoisted npm-workspace members update the member that
  actually declares the pin; exported as ARGENT_PROJECT_ROOT.
# Conflicts:
#	packages/argent-installer/src/init.ts
#	packages/argent-installer/src/uninstall.ts
#	packages/argent-installer/src/update.ts
Comment-only pass over the PR's new code: every comment introduced on
this branch was reviewed and the verbose ones rewritten to be concise
while keeping the load-bearing content — invariants, cross-file
contracts, platform gotchas, and the safety rationale behind the
destructive-path guards. No code changes (verified by comparing
compiled output before and after).
The install-mode prompt ('How should argent be installed?' — global vs
committable local) already captures consent: a user who just answered
'Globally' (or passed --global) was then asked 'Argent is not installed
globally. Would you like to install it?' — the pre-install-mode consent
gate surviving from the days when it was init's only question. The
missing global package now installs directly after the mode choice, with
an info line instead of a duplicate prompt. Non-interactive behavior is
unchanged, and the global_install_decision telemetry still emits
(decision: install); the cancel path for this step is gone with the
prompt, and the event schema keeps the old labels for older clients.
…eview round)

Fifteen confirmed findings from the final pre-merge review, clustered:

Update chain:
- refreshArgentSkills bails when the bundled skills dir is unreadable (a
  pruned pnpm store mid-update) instead of classifying every installed
  skill as orphaned and mass-pruning both scopes, and pins its skills-CLI
  runs to the project root rather than the detached updater's cwd.
- Local installs derive their runtime paths from the version-stable
  node_modules symlink instead of the realpathed (pnpm version-pinned)
  store dir, so a still-running MCP session can respawn — and the
  launcher's disk gate can read the NEW version — after an in-place
  update prunes the old store; a missing bundle now fails with restart
  guidance instead of a cryptic ready-timeout. postinstall also retires
  servers whose recorded bundle no longer exists (pnpm/yarn global
  upgrades change dirs, so the same-bundle check never matched).
- The global update's pre-install server stop falls back to the legacy
  single-slot kill when the global package root is unresolvable (Windows
  .cmd-wrapper layouts) — main's behavior, scoped safely.
- Yarn PnP local installs (running from .yarn/) classify as local, so
  the agent-triggered auto update no longer targets a global install the
  user may not have.

Agent-triggered updates:
- The mcp_update trigger never installs a MISSING global package — an
  update consent is not an install consent.
- A global-serving (shared, frozen-cwd) tool-server refuses local
  targets with CLI guidance instead of pinning whatever project spawned
  it first; the cwd-walk fallback applies only to local-serving servers.
- Cross-install responses no longer promise a restart of a server the
  update does not touch, and the auto-hint routes the global-to-local
  direction through the CLI.

Consent and classification:
- The machine-wide ~/.claude/settings.json disabled-list rejection is
  never auto-removed (project-confined ones still are); ~/.claude.json
  edits write back verbatim instead of running the prune-and-delete
  path over the user's primary Claude config.
- Legacy argent-authored entries carrying only ARGENT_MCP_LOG (<= 0.9.x)
  classify as managed again, so the refresh keeps repairing them to the
  clean env-less shape (#238's rollout vehicle).
- Cursor/Windsurf allowlist removal honors the cleanup scope: their
  machine-global files survive a local-only uninstall that retains the
  global install.
- init --global keeps a committed local-mode project entry instead of
  clobbering the team's node-path command while simultaneously reporting
  the project stays in local mode; runShellCommand drops the hardcoded
  Windows .cmd suffix (bare name + shell resolves .exe managers too).
…te on init --global

Two edges from the final QA pass:

- bundled-paths' stable-symlink probe only checked node_modules at the
  level where the .pnpm store matched (the workspace root), so in a pnpm
  WORKSPACE — where the version-stable node_modules/<pkg> symlink lives in
  the DECLARING member, a level below the store — it fell back to the
  version-pinned .pnpm store path. An agent-triggered `update --local`
  then left the running session pointing at the pruned store dir, unable
  to respawn (the very breakage the stable-path fix was meant to prevent,
  just uncovered for workspace members). New findStablePackageDir scans
  every level from cwd up for the conventional symlink resolving to the
  running package; single-package pnpm, npm, hoisted, PnP, and global
  layouts are unaffected (it only ever returns an alias of the same real
  dir, and can't regress the fallback since it checks a superset of levels).

- init-mcp-write's keepsCommittedLocalEntry kept a committed node-path MCP
  entry purely from its command shape, so a project that abandoned local
  mode (devDependency removed) kept a DEAD entry through `init --global`
  even as the same run removed the .argent marker and declared conversion
  to global. Now gated on actual local intent (committed record or manifest
  declaration) — the coexistence case (dep still present) still keeps the
  entry; a genuinely-converted project has it rewritten to bare `argent`.

Tests: findStablePackageDir unit coverage (single-package, workspace
member, deepest-alias, no-match) + init-mcp-write still-local vs abandoned
cases. Live-verified a staged pnpm workspace member now yields a
.pnpm-free stable bundlePath.
@filip131311 filip131311 merged commit 7bd017a into main Jul 8, 2026
6 checks passed
@filip131311 filip131311 deleted the feat/committable-install branch July 8, 2026 10:24
latekvo added a commit that referenced this pull request Jul 9, 2026
Reconciles the help text with main's installer changes (#428): the top-level
table keeps its per-command detail lines but now renders them from
INSTALLER_COMMAND_META (single source), and the per-command option lists gain
the --global/--local flags the installers now parse.
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.

2 participants