Skip to content

refactor(installer): split init.ts into focused modules#242

Draft
filip131311 wants to merge 1 commit into
filip/argent-init-devdepfrom
filip/argent-init-devdep-refactor
Draft

refactor(installer): split init.ts into focused modules#242
filip131311 wants to merge 1 commit into
filip/argent-init-devdepfrom
filip/argent-init-devdep-refactor

Conversation

@filip131311

Copy link
Copy Markdown
Collaborator

Summary

Same feature as #232, restructured. The original PR's init.ts had grown to 822 lines mixing flag parsing, topology decisions, interactive prompts, install commands, adapter filtering, MCP config writes, allowlist, skills install, and the summary into one long function with deep branching on install mode. This refactor splits it into a thin orchestrator (~345 lines) plus focused modules.

No behavior changes. All 314 installer tests + 691 tool-server tests still pass. The visible CLI surface is unchanged — argent init, argent init --devdep, argent update, argent uninstall all behave exactly as before.

What changed

New modules (each 19–192 lines, single responsibility):

Module Responsibility
package-manager.ts PackageManager type, lockfile + user-agent detection, install/uninstall command builders for both topologies
topology.ts Topology interface + GLOBAL / LOCAL objects with probe, installCommand, uninstallCommand, spawnCwd. TOPOLOGIES table that update/uninstall iterate.
preflight.ts hasPackageJson, isYarnPnp
shell.ts runShellCommand
install-error.ts Manifest-error pattern matching + reportLocalInstallFailure
init-args.ts parseInitArgs + validateInitArgs (cross-flag invariants like --devdep--scope global)
init-mode-prompt.ts Step-0 interactive install-mode selector (with the Local-confirm loop)
install-runner.ts Shared install execution for both topologies, including preflight refusals + error reporting
init-adapters.ts Step-1 adapter universe (filtered for local), multiselect, dropped-adapter info line
init-scope.ts Scope prompt (local/global/custom-path)
init-mcp-write.ts Per-adapter MCP entry writes with the local/global fallback dance
init-allowlist.ts Tool auto-approval step
init-skills.ts Step-2 skills install flow (default / interactive / manual)

Refactors:

  • init.ts: 822 → 345 lines. Reads top-to-bottom like a recipe — parse args → decide topology → install → MCP config → allowlist → skills → rules → summary. Each phase is one call into a dedicated module.
  • update.ts: iterates TOPOLOGIES instead of bespoke per-topology helpers. The "needs update" loop is now four lines.
  • uninstall.ts: collapses two duplicated topology branches into a single loop over TOPOLOGIES. Tool-server kill happens once via a makeOnce helper.
  • mcp-configs.ts: drops adapter.name === \"Claude Code\" string match for ${CLAUDE_PROJECT_DIR} substitution; replaced with a typed expandsProjectDirVariable adapter flag (set on claudeAdapter). This was flagged as fragile in the original review.
  • utils.ts: 646 → 425 lines. PM + topology helpers re-exported from their new homes so external callers keep working.

How to review

Start at init.ts — the diff against #232 is large, but the new file reads sequentially and each function call points at the module that owns the work.

Then jump into topology.ts to see how the GLOBAL/LOCAL split is encoded once and consumed by all three commands. Compare update.ts and uninstall.ts to confirm the duplication is gone.

The remaining init-*.ts modules are each small and self-contained — review individually.

Tests

All existing tests pass unchanged:

  • @argent/installer: 314 passed (7 files)
  • @argent/tool-server: 691 passed (73 files)
  • Workspace totals: 1129 passed across 6 packages

Existing tests cover:

  • Flag parsing (--devdep / --local-install / --scope global rejection)
  • PM detection (lockfile takes precedence over user-agent under npx)
  • Install/uninstall command shapes for npm/pnpm/yarn/bun
  • isLocallyInstalled two-part check (declaration + files)
  • getLocallyInstalledVersion reads disk, not running module
  • getMcpEntry for both modes, Claude vs non-Claude adapters
  • npx-cache cannot fool topology detection
  • Existing-manifest error pattern matching
  • Topology-aware update command construction

Follow-up ideas (not in this PR)

  • Add focused unit tests for the new modules (topology.test.ts, init-args.test.ts, etc.). The current test surface lives mostly in utils.test.ts / init.test.ts and still passes because the re-exports keep the API stable, but the new modules deserve their own test files for clearer ownership.
  • Consider making getMcpEntry's adapter parameter required in local mode via overloads — currently optional with a fallback to the non-Claude path. Type system can enforce the contract.

Why this is a draft

Wanted to get the structure in front of reviewers before adding new tests for each module — happy to mark ready-for-review once the architecture has a 👍.

Replaces the spaghetti in init/update/uninstall with a topology table +
phase modules.

New modules:
  - package-manager.ts  PM detection, install/uninstall commands
  - topology.ts         GLOBAL/LOCAL with probe + install/uninstall/spawnCwd
  - preflight.ts        hasPackageJson / isYarnPnp
  - shell.ts            runShellCommand
  - install-error.ts    manifest-error pattern matching + reporting
  - init-args.ts        flag parsing + cross-flag validation
  - init-mode-prompt.ts Step-0 install-mode selector
  - install-runner.ts   shared install execution for both topologies
  - init-adapters.ts    Step-1 adapter universe + multiselect
  - init-scope.ts       scope prompt (local/global/custom)
  - init-mcp-write.ts   per-adapter MCP entry writes (with fallbacks)
  - init-allowlist.ts   tool auto-approval step
  - init-skills.ts      Step-2 skills install flow

Refactors:
  - init.ts: 822 → 345 lines, reads top-to-bottom like a recipe
  - update.ts: iterates TOPOLOGIES instead of duplicating per-topology
    helpers
  - uninstall.ts: collapses the two duplicate topology branches into a
    single loop over TOPOLOGIES
  - mcp-configs.ts: replaces `adapter.name === "Claude Code"` string
    match with a typed `expandsProjectDirVariable` adapter flag
  - utils.ts: 646 → 425 lines, PM and topology helpers re-exported from
    their new homes for backwards compatibility

All 314 installer tests + 691 tool-server tests still pass.
@latekvo latekvo force-pushed the filip/argent-init-devdep-refactor branch from e18ff5c to 08245e4 Compare June 3, 2026 14:48
@latekvo latekvo changed the base branch from filip/argent-init-devdep to main June 3, 2026 14:48
@latekvo latekvo force-pushed the filip/argent-init-devdep-refactor branch from 08245e4 to e18ff5c Compare June 3, 2026 15:01
@latekvo latekvo changed the base branch from main to filip/argent-init-devdep June 3, 2026 15:01
@latekvo

latekvo commented Jun 3, 2026

Copy link
Copy Markdown
Member

Force pushed, apologies. I reverted my changes.

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