refactor(#1511): move content-rewrite engine to conversion module, delete the install.js relay (#1513)

* refactor(#1511): move content-rewrite engine to conversion module, delete the install.js relay

Phase 2 of epic #1507 (ADR-1508). Behavior-preserving: makes the Runtime
Artifact Conversion Module the single owner of per-runtime content rewriting and
removes the last upward .cts -> bin/install.js dependency.

- src/runtime-artifact-conversion.cts now owns the engine (_applyRuntimeRewrites,
  5-arg with INJECTED attribution), the staged-content walkers
  (applyRuntimeContentRewritesInPlace / ...ForCommandsInPlace), computePathPrefix
  (private, exported as _computePathPrefix for tests), and the deep public seam
  rewriteStagedSkillBodies / rewriteStagedCommandBodies({runtime, configDir,
  scope, homedir?, platform?, resolveAttribution?}).
- src/surface.cts:applySurface calls rewriteStagedSkillBodies directly (no
  resolveAttribution -> undefined). Co-Authored-By is absent from ALL rewritten
  content, so processAttribution is vacuous there and undefined is provably
  behavior-identical. surface no longer imports getInstallExports.
- src/runtime-artifact-layout.cts: deleted getInstallExports / loadInstallExports
  / InstallExports + the GSD_TEST_MODE require('bin/install.js') relay.
- bin/install.js: binds computePathPrefix / the two walkers / _applyRuntimeRewrites
  from the conversion module (single implementation, exports preserved for Hyrum);
  install callsites pass getCommitAttribution(runtime) as the injected attribution.
  getCommitAttribution stays here (impure install-time config I/O).
- DEFECT.GENERATIVE-FIX guard: tests assert install.X === conversion.X reference
  identity for computePathPrefix + both walkers (no drift).

New tests/enh-1511-*.test.cjs (engine, attribution injection, deep seam, prefix,
layout-no-relay guard, reference-identity). 316 affected-suite tests green; lint clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD

* test(#1511): make rewrite-engine path assertions Windows-robust

The deep seam normalizes paths as path.resolve(configDir).replace(/\\/g,'/')
and compares homedir().replace(/\\/g,'/'). Three assertions in the new test
rebuilt expected paths without that normalization, so they passed on Mac/Linux
but failed on Windows CI (PR #1513):

- two absolute-branch asserts rebuilt resolvedTarget via path.resolve(configDir)
  without the backslash→slash replace → mismatch on Windows.
- the $HOME-branch test fed a POSIX-literal /home/testuser, which Windows
  path.resolve re-roots onto the cwd drive (D:/home/...), so the
  resolvedTarget.startsWith(homeDir) check failed and the $HOME shorthand was
  never produced.

Fix is test-only (engine unchanged, still behavior-preserving): mirror the
engine's .replace(/\\/g,'/') in the two absolute-branch asserts, and use a real
absolute path (path.resolve(os.tmpdir(), ...)) + platform: process.platform for
the $HOME-branch test so the comparison holds on all platforms.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-20 23:59:50 -04:00
committed by GitHub
parent cadc944781
commit eb81faaeab
6 changed files with 688 additions and 314 deletions

View File

@@ -34,39 +34,10 @@ const conversionExports = runtimeArtifactConversion as Record<string, unknown> &
// In .cts (CommonJS output) files, `require` is available as a global.
const _require: NodeRequire = require;
// ---------------------------------------------------------------------------
// Lazy installer exports (avoids GSD_TEST_MODE env mutation at module load)
// ---------------------------------------------------------------------------
interface InstallExports {
computePathPrefix: (opts: { isGlobal: boolean; isOpencode: boolean; isWindowsHost: boolean; resolvedTarget: string; homeDir: string }) => string;
applyRuntimeContentRewritesInPlace: (stagedDir: string, runtime: string, pathPrefix: string) => void;
[converterName: string]: unknown;
}
/**
* Load bin/install.js exports in a test-safe way.
* Sets GSD_TEST_MODE only for the duration of the require() call and only if
* it was not already set, restoring the original value in a finally block so
* the module-level environment is never permanently mutated.
*/
function loadInstallExports(): InstallExports {
const savedTestMode = process.env['GSD_TEST_MODE'];
if (savedTestMode === undefined) process.env['GSD_TEST_MODE'] = '1';
try {
return _require('../../../bin/install.js') as InstallExports;
} finally {
if (savedTestMode === undefined) delete process.env['GSD_TEST_MODE'];
else process.env['GSD_TEST_MODE'] = savedTestMode;
}
}
/** Cache after first successful load. */
let _installExports: InstallExports | null = null;
function getInstallExports(): InstallExports {
if (!_installExports) _installExports = loadInstallExports();
return _installExports;
}
// loadInstallExports / getInstallExports / InstallExports removed in ADR-1508
// / #1511 Phase 2 — removed this module's upward dependency on bin/install.js
// (the getInstallExports relay). surface.cts now calls
// runtimeArtifactConversion.rewriteStagedSkillBodies directly.
// ---------------------------------------------------------------------------
// Types
@@ -494,4 +465,5 @@ function resolveRuntimeArtifactLayoutFromRegistry(
return { runtime, configDir, scope, kinds };
}
export = { resolveRuntimeArtifactLayout, resolveRuntimeArtifactLayoutFromRegistry, findInstallSourceRoot, getInstallExports };
// getInstallExports removed in ADR-1508 / #1511 Phase 2 (last upward .cts→install.js dep).
export = { resolveRuntimeArtifactLayout, resolveRuntimeArtifactLayoutFromRegistry, findInstallSourceRoot };