Files
msd-core/docs/adr/0010-file-operation-engine-module.md
Tom Boucher 1e091d2bcb refactor(shell-projection): remove deprecated wrappers + finalize ADRs (Phase 4, #3468) (#3484)
* refactor(shell-projection): remove deprecated wrappers + finalize ADRs (Phase 4, #3468)

Final phase of the shell-command-projection expansion. Removes the legacy
core.cjs wrappers (`atomicWriteFileSync`, `safeReadFile`, `normalizeMd`)
now that every call site lives behind the seam, plus three Phase-3
stragglers (`graphify.cjs`, `template.cjs`, dead import in
`profile-pipeline.cjs`).

Documentation:
- ADR-0009: addendum noting Phase 1–4 scope expansion (subprocess +
  file I/O ownership), supersession of "does not execute" constraint,
  and resolution of open Q4.
- ADR-0010: status changed to Superseded by ADR-0009 with explanation.
- CONTEXT.md "Shell Command Projection Module" entry already current
  from Phase 1 — no edit needed.

Tests:
- `tests/atomic-write.test.cjs` deleted — wrapper it tested is gone;
  `atomic-write-coverage.test.cjs` (Phase 3) covers platformWriteSync.
- `tests/core.test.cjs::safeReadFile` + `::normalizeMd` describes
  deleted — wrappers are gone.
- `tests/concurrency-safety.test.cjs` normalizeMd suite (behavioral /
  perf / snapshot) repointed via 2-line shim at the seam's
  `normalizeContent` — full regression coverage preserved.

Test result: 9059/9041/18 — exact pre-Phase-4 baseline. All 18
failures are pre-existing path-with-spaces local-env issues.

Closes #3468

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(shell-projection): migrate remaining raw fs.writeFileSync sites (Phase 4, #3468)

Sweeps the 7 raw fs.writeFileSync call sites that bypassed the seam through Phase 3,
folding them into platformWriteSync. Net -14 lines: deletes the local writeFileAtomicSync
helper in installer-migrations.cjs and collapses surface.cjs's manual tmp+rename into a
single seam call.

Sites migrated:
- drift.cjs (1) — frontmatter write
- learnings.cjs (1) — learning record JSON write
- install-profiles.cjs (1) — profile marker write (collapsed redundant mkdir)
- gsd2-import.cjs (1) — imported file write (collapsed redundant mkdir)
- surface.cjs (1) — surface state write (replaced manual tmp+rename block)
- installer-migrations.cjs (3) — journal init/finalize + rewrite-json action;
  deleted private writeFileAtomicSync helper and its three call sites

Two sites intentionally retained outside the seam:
- planning-workspace.cjs:241 — workspace lock (wx-flag atomic-create; previously excluded by Phase 3)
- installer-migrations.cjs:220 — install migration lock (fd write into wx-opened handle)
- writeInstallState (installer-migrations.cjs) — strict atomic contract for install state;
  the seam's fallback-to-direct-write on rename failure would silently violate the
  invariant that install state must never be left half-written. Inline tmp+rename with
  rethrow keeps the original guarantee.

Tests: 9059 / 9041 / 18 — exactly the pre-Phase-4 baseline; 18 failures are the
pre-existing path-with-spaces local-env issues, identical files as before.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix(installer-migrations): use strict atomic write for rollback install-state restore

The rollback path was restoring INSTALL_STATE via platformWriteSync, which falls
back to a direct write on rename failure and would silently violate the
half-written invariant that the install-state contract guarantees elsewhere.

Extracts the strict tmp+rename logic from writeInstallState into a shared
atomicWriteInstallState(configDir, content) helper and routes both
writeInstallState and rollbackAppliedMigrationResult through it. Preserves the
existing null-handling (rmSync when previousInstallStateBytes === null) and
existing failure-collection (failures.push on caught errors).

Byte-faithful restore: previousInstallStateBytes is written as-is (no JSON
parse round-trip), preserving the exact prior file contents on restore.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-05-13 20:46:02 -04:00

6.3 KiB
Raw Blame History

File Operation Engine Module owns safe runtime/config file mutations

  • Status: Superseded by ADR-0009 (Shell Command Projection Module expansion, Phases 3–4, #3467–#3468)
  • Date: 2026-05-12
  • Superseded: 2026-05-13

Supersession note. Rather than build a separate File Operation Engine, the file-mutation safety policy this ADR proposed was absorbed into the Shell Command Projection Module (ADR-0009). Phase 3 (#3467) added platformWriteSync / platformReadSync / platformEnsureDir / normalizeContent to that seam, owning atomic write (tmp+rename), .md normalization, and directory creation as a single platform-conditional surface. Phase 4 (#3468) removed the duplicated atomicWriteFileSync / safeReadFile / normalizeMd wrappers from core.cjs. The applyFileMutationPlan / typed plan IR design proposed below was not built — the simpler per-call seam proved sufficient for the actual drift sites. Lock-file lifecycle (Track B item 3) remains owned by withPlanningLock in planning-workspace.cjs because its { flag: 'wx' } exclusive-create semantics differ from atomic-write rename semantics.


We propose introducing a File Operation Engine Module that owns policy for managed file reads, writes, deletes, locks, backups, and rollbacks across installer, migration, and planning surfaces. Today, file mutation behavior is duplicated across bin/install.js, get-shit-done/bin/lib/installer-migrations.cjs, and multiple planning modules, with drift in atomic-write guarantees, path safety checks, and ownership classification.

This ADR also captures where Shell Command Projection Module policy should be consumed or expanded for hook-command-specific file mutations, so shell command drift and file mutation drift do not evolve as separate bug classes.

Decision

  • Add a File Operation Engine Module under get-shit-done/bin/lib/ as the single seam for file mutation safety policy.
  • Keep command-text projection in the Shell Command Projection Module (ADR-0009), but route projection-adjacent hook file mutations through shared managed-hook ownership policy.
  • Move file operation adapters to the new seam in two tracks:
    • Track A (projection-adjacent): runtime config hook-command detection/rewrite/delete paths consume shared managed-hook policy from the projection seam.
    • Track B (solution-wide): shared file operation engine owns atomic write, path containment, lock behavior, rollback bookkeeping, and best-effort cleanup policy.
  • Keep internal subprocess execution out of this seam (same boundary as ADR-0009): this is a file operation seam, not a command runner.

Initial Scope

  1. Unify managed-hook ownership classification used by install/uninstall/migration hook config rewrites.
  2. Unify atomic write behavior currently duplicated in installer/core/migration paths.
  3. Unify lock-file lifecycle policy used by planning workspace and installer migration journal flows.
  4. Expose typed file mutation plan IR for tests (rewrite-json, rewrite-text with format (toml/markdown/plain), delete-file, backup-file, restore-file, ensure-dir).

Migration Inventory

Projection-adjacent file mutation drift (Track A)

  • bin/install.js
    • hook cleanup command detection (isGsdHookCommand)
    • stale Codex hook strip basenames (STALE_HOOK_BASENAMES)
    • settings/config hook entry prune/rewrite paths
  • get-shit-done/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs
    • isManagedCodexHookCommand regex/path detection duplicated from installer-owned hook policy
  • get-shit-done/bin/lib/shell-command-projection.cjs
    • isManagedHookBasename already owns part of this policy and should become the canonical owner

Solution-wide file operation drift (Track B)

  • bin/install.js
    • local atomicWriteFileSync and temp cleanup registry
    • large inlined read/modify/write + backup/rollback logic for runtime config and hooks
  • get-shit-done/bin/lib/core.cjs
    • atomicWriteFileSync helper diverges in fallback behavior from installer/migration variants
  • get-shit-done/bin/lib/installer-migrations.cjs
    • separate writeFileAtomicSync, rollback journaling, lock handling, and containment checks
  • get-shit-done/bin/lib/planning-workspace.cjs and get-shit-done/bin/lib/state.cjs
    • duplicated lock-file create/release/remove patterns and best-effort cleanup semantics
  • get-shit-done/bin/lib/roadmap.cjs, phase.cjs, milestone.cjs, frontmatter.cjs, drift.cjs
    • direct read/modify/write flows with inconsistent atomicity and normalization policy application

Interface sketch

The File Operation Engine Module should expose typed mutation planning and execution helpers:

planFileMutations({
  rootDir,
  operations: [
    { type: 'rewrite-json', relPath, mutate },
    { type: 'rewrite-text', relPath, mutate, format: 'toml' | 'markdown' | 'plain' },
    { type: 'delete-file', relPath },
    { type: 'ensure-dir', relPath },
  ],
  ownership: { mode: 'managed-only' | 'allow-user', classifier },
})
applyFileMutationPlan({
  plan,
  atomic: true,
  rollback: true,
  lock: { scope: 'config' | 'planning', id: '...' },
})

For projection-adjacent paths, adapters should consume projection policy:

isManagedHookCommand(commandText, { surface, configDir })

Consequences

  • File mutation safety policy becomes local to one module, reducing drift across installer/migration/planning paths.
  • Shell command projection and hook ownership classification stay aligned at one seam family.
  • Tests can assert typed mutation IR and reason codes instead of source-grep and duplicated predicate mirrors.
  • Initial migration is broad; sequencing should prioritize projection-adjacent hook config paths first, then converge atomic-write and lock semantics.

Open questions

  • Whether lock semantics should be one shared policy for installer + planning, or two adapters over one lock primitive.
  • Whether SDK query write paths should consume the same engine in the first pass or follow after CJS convergence.
  • Whether file mutation telemetry (per-op reason codes and rollback events) should be required for all engine adapters.

References

  • ADR-0008: 0008-installer-migration-module.md
  • ADR-0009: 0009-shell-command-projection-module.md
  • Related bug history: #1755, #2866, #2979, #3002, #3017, #3439