Skip to content

Bundle installer engine cleanup: tryExec extraction + MEMORY migration (#121, #107)#136

Merged
virtualian merged 2 commits intomainfrom
fix/121-107-installer-engine-cleanup
Apr 15, 2026
Merged

Bundle installer engine cleanup: tryExec extraction + MEMORY migration (#121, #107)#136
virtualian merged 2 commits intomainfrom
fix/121-107-installer-engine-cleanup

Conversation

@virtualian
Copy link
Copy Markdown
Owner

Bundles two installer-engine issues on a single branch with one commit each for bisectability.

#121 — Promote tryExec into shared engine/exec.ts

Extract the triplicated safe-subprocess helper into a leaf utility module with zero imports from the installer tree. Replaces three copies:

  • actions.ts — delete local tryExec, import from ./exec
  • detect.ts — delete local tryExec, import from ./exec, pass explicit 5000ms timeout at every call site to preserve the historical detect-probe bound (tryExec default is 30000ms, matching actions.ts)
  • repo-url.ts — rewrite readOriginRemote around tryExec, preserving the .git existsSync short-circuit

Zero behavioral change — purely structural consolidation. Originally flagged by the /simplify review in PR #120 (#115 fix) as pre-existing debt and deferred out of that scope.

Commit: cc0fcc5

#107 — Automate MEMORY migration for v4.0.2 → v4.0.3+ upgraders

Ships engine/memory-migration.ts patterned off command-migration.ts (PR #135). Automates the ~/.claude/MEMORY/~/.pai/MEMORY/ relocation that PR #106 documented but left manual.

Semantics:

  • Idempotent via marker file at <dest>/STATE/migration.json. Second run sees the marker and noops regardless of source state.
  • Refuses ambiguity: if both source and dest contain meaningful content without a marker, emits a diagnostic listing both paths, file counts, and latest mtime, then stops. Merging is a data-integrity decision outside installer scope.
  • Atomic move: renameSync first, cpSync + rmSync fallback on EXDEV. Preserves mode and timestamps. Marker written inside the new location after the move.
  • Meaningful-content filter: ignores .DS_Store and dotfile cruft so empty stub directories don't trip ambiguity detection.
  • Dry-run mode (options.dryRun=true) reports the intended action without touching the filesystem and without writing telemetry.
  • Telemetry logged to <paiDir>/MEMORY/LEARNING/SYSTEM/memory-migration-*.json so the outcome surfaces in the learning digest, not stdout where users miss it.

Wiring: Called from runRepository immediately after migrateUserContext and before migratePerPackSymlinks / migratePerPackCommands. Runs on every install path; the idempotent + noop-on-absent-source semantics make this safe for fresh installs.

Tests (engine/memory-migration.test.ts, 8 cases — all passing):

  • fresh install → noop-nothing-to-migrate
  • source has only .DS_Store → noop
  • upgrade → renames source into dest, writes marker, preserves nested files, method: "rename"
  • second run → noop-already-migrated regardless of source state
  • ambiguity → refused-ambiguous, neither side mutated, telemetry written
  • dry-run would-migrate → filesystem untouched
  • dry-run would-refuse-ambiguous → no telemetry written
  • post-migration telemetry entry exists in LEARNING/SYSTEM/

Commit: dad8df5

Verification

  • engine/exec.ts typechecks via bun build --target=bun
  • engine/actions.ts, engine/detect.ts, engine/repo-url.ts typecheck clean after refactor
  • engine/memory-migration.ts + engine/memory-migration.test.ts typecheck
  • bun test engine/memory-migration.test.ts — 8/8 pass
  • bun test engine/skill-migration.test.ts — 17/17 pass (regression check)
  • bash -n Releases/v4.0.3+/.claude/install.sh
  • bash -n Releases/v4.0.3+/.claude/PAI-Install/install.sh

Test plan

Issues closed

🤖 Generated with Claude Code

virtualian and others added 2 commits April 15, 2026 20:49
Extract the triplicated safe-subprocess helper into a leaf utility module
with zero imports from the installer tree, so any consumer (current or
future) can depend on it without circular-import risk. Replace the three
hand-rolled copies:

- actions.ts: delete local tryExec, import from ./exec
- detect.ts: delete local tryExec, import from ./exec, pass explicit
  5000ms timeout at every call site to preserve the historical detect
  probe bound (tryExec default is 30000ms, matching actions.ts)
- repo-url.ts: rewrite readOriginRemote around tryExec, preserving the
  .git existsSync short-circuit

Zero behavioral change — purely structural consolidation. Flagged by the
/simplify review in PR #120 (#115 fix) as pre-existing debt.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Patterned off command-migration.ts (PR #135). Ships engine/memory-migration.ts
with a single entry point migrateMemoryDirectory(claudeConfigDir, paiDir, emit,
options?), wired into runRepository immediately after user-context migration
and before the skill/command migrators.

Semantics:
- Idempotent via marker file at <dest>/STATE/migration.json. Second run
  sees the marker and noops regardless of source state.
- Refuses with a diagnostic (both paths + file counts + latest mtime) if
  both source and dest contain meaningful content without a marker.
  Merging is a data-integrity decision outside installer scope.
- renameSync first, cpSync+rmSync fallback on EXDEV. Preserves mode and
  timestamps. Marker written inside the new location after the move.
- Meaningful-content filter ignores .DS_Store and dotfile cruft so empty
  stub directories don't trip ambiguity detection.
- Dry-run mode (options.dryRun=true) reports the intended action without
  touching the filesystem and without writing telemetry.
- Telemetry logged to <paiDir>/MEMORY/LEARNING/SYSTEM/memory-migration-*.json
  so the outcome surfaces in the learning digest, not stdout where users
  miss it.

Tests (engine/memory-migration.test.ts, 8 cases):
- fresh install: noop-nothing-to-migrate
- source has only .DS_Store: noop
- upgrade: renames source into dest, writes marker, preserves nested files
- second run: noop-already-migrated regardless of source state
- ambiguity: refused-ambiguous, neither side mutated, telemetry written
- dry-run would-migrate: filesystem untouched
- dry-run would-refuse-ambiguous: no telemetry written
- post-migration telemetry entry exists in LEARNING/SYSTEM/

All 8 pass. Regression-checked skill-migration.test.ts (17/17 pass).
install.sh and PAI-Install/install.sh pass bash -n.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@virtualian virtualian merged commit 4aad21a into main Apr 15, 2026
@virtualian virtualian deleted the fix/121-107-installer-engine-cleanup branch April 15, 2026 19:56
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.

Promote tryExec to shared engine/exec.ts — currently triplicated across installer Automated MEMORY migration shim for v4.0.2 → v4.0.3+ upgraders

1 participant