From cadc94478167e43d27909a564d8f1dd47de4ed98 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sat, 20 Jun 2026 22:29:15 -0400 Subject: [PATCH] refactor(#1510): relocate getDirName + processAttribution out of bin/install.js (#1512) Phase 1 of epic #1507 (ADR-1508): behavior-preserving relocation of the pure rewrite-engine helpers out of the hand-authored installer so the conversion module can own them without importing bin/install.js. - getDirName -> src/runtime-name-policy.cts (pure runtime->dir-name switch). - processAttribution -> src/runtime-artifact-conversion.cts (pure Co-Authored-By content transform). - bin/install.js imports both back via destructure/binding and re-exports getDirName unchanged (Hyrum: install.test.cjs + runtime install tests import getDirName from bin/install.js). Two refinements to ADR-1508's Phase 1 (verified against the source): - getCommitAttribution STAYS in bin/install.js: it is impure install-time config I/O (reads runtime settings.json, uses install-time config-dir state + attributionCache), not a content transform. Phase 2 will inject the resolved attribution into the engine rather than move this function. - The convertClaudeToAugmentMarkdown dedup is deferred to Phase 2's cleanup: the local copy is entangled with a converter cluster (convertSlashCommandsTo AugmentSkillMentions is used only by it; the family is partly dead-local via the ...runtimeArtifactConversion export spread), deduping only augment would be arbitrary, and it is not required to unblock Phase 2 (the engine will call the conversion module's own copy when it moves). New tests/enh-1510-*.test.cjs: getDirName at its new home (all runtimes + fallback + install.js re-export identity) and processAttribution (null/undefined/string/$-escape/CRLF/global). 487 affected-suite tests green. Claude-Session: https://claude.ai/code/session_0187qgypdy1wkWRpdaf2hRuD Co-authored-by: Claude Opus 4.8 (1M context) --- bin/install.js | 52 +++------ src/runtime-artifact-conversion.cts | 29 +++++ src/runtime-name-policy.cts | 29 +++++ ...-rewrite-engine-helper-relocation.test.cjs | 110 ++++++++++++++++++ 4 files changed, 183 insertions(+), 37 deletions(-) create mode 100644 tests/enh-1510-rewrite-engine-helper-relocation.test.cjs diff --git a/bin/install.js b/bin/install.js index 994d723ae..a3877657e 100755 --- a/bin/install.js +++ b/bin/install.js @@ -33,6 +33,11 @@ const { getGlobalConfigDir, getGlobalSkillsBase, } = require('../gsd-core/bin/lib/runtime-homes.cjs'); +// getDirName (runtime -> local config dir name) is relocated out of this +// installer to the runtime-name-policy leaf (ADR-1508 / #1510 Phase 1) so the +// conversion module's rewrite engine can consume it without importing +// bin/install.js. Re-exported below for back-compat consumers/tests. +const { getDirName } = require('../gsd-core/bin/lib/runtime-name-policy.cjs'); const { applyWorktreeBaseRef, readBaseRefFromSettings, @@ -461,25 +466,8 @@ Then re-run: npx ${pkg.name}@latest } } -// Helper to get directory name for a runtime (used for local/project installs) -function getDirName(runtime) { - if (runtime === 'copilot') return '.github'; - if (runtime === 'opencode') return '.opencode'; - if (runtime === 'gemini') return '.gemini'; - if (runtime === 'kilo') return '.kilo'; - if (runtime === 'codex') return '.codex'; - if (runtime === 'antigravity') return '.agents'; - if (runtime === 'cursor') return '.cursor'; - if (runtime === 'windsurf') return '.devin'; - if (runtime === 'augment') return '.augment'; - if (runtime === 'trae') return '.trae'; - if (runtime === 'qwen') return '.qwen'; - if (runtime === 'hermes') return '.hermes'; - if (runtime === 'kimi') return '.kimi-code'; - if (runtime === 'codebuddy') return '.codebuddy'; - if (runtime === 'cline') return '.cline'; - return '.claude'; -} +// getDirName (runtime -> local config dir name) now lives in +// runtime-name-policy.cjs (ADR-1508 / #1510 Phase 1); imported + re-exported. /** * Get the config directory path relative to home directory for a runtime @@ -643,6 +631,11 @@ const referencesHook = hooksSurface.referencesHook; // applySettingsJsonHooks: mutates settings.hooks.* in place with all GSD-managed // hook registrations for settings.json-surface runtimes (ADR-857 phase 5f-1b). const applySettingsJsonHooks = hooksSurface.applySettingsJsonHooks; +// processAttribution: pure Co-Authored-By content transform, relocated to the +// conversion module (ADR-1508 / #1510 Phase 1). Bound here so install.js +// callers continue to work and there is a single implementation. (All call +// sites are below this line, so the const binding has no TDZ hazard.) +const processAttribution = runtimeArtifactConversion.processAttribution; function rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts) { return hooksSurface.rewriteLegacyManagedNodeHookCommands(settings, absoluteRunner, opts); @@ -1417,24 +1410,9 @@ function getCommitAttribution(runtime) { return result; } -/** - * Process Co-Authored-By lines based on attribution setting - * @param {string} content - File content to process - * @param {null|undefined|string} attribution - null=remove, undefined=keep, string=replace - * @returns {string} Processed content - */ -function processAttribution(content, attribution) { - if (attribution === null) { - // Remove Co-Authored-By lines and the preceding blank line - return content.replace(/(\r?\n){2}Co-Authored-By:.*$/gim, ''); - } - if (attribution === undefined) { - return content; - } - // Replace with custom attribution (escape $ to prevent backreference injection) - const safeAttribution = attribution.replace(/\$/g, '$$$$'); - return content.replace(/Co-Authored-By:.*$/gim, `Co-Authored-By: ${safeAttribution}`); -} +// processAttribution (pure Co-Authored-By content transform) relocated to +// runtime-artifact-conversion.cjs (ADR-1508 / #1510 Phase 1); bound above. +// getCommitAttribution stays here — it is impure install-time config I/O. /** * Convert Claude Code frontmatter to opencode format diff --git a/src/runtime-artifact-conversion.cts b/src/runtime-artifact-conversion.cts index fd9a8072a..422cba86e 100644 --- a/src/runtime-artifact-conversion.cts +++ b/src/runtime-artifact-conversion.cts @@ -2095,7 +2095,36 @@ function convertClaudeCommandToKiloSkill(content, skillName) { } +/** + * Apply Co-Authored-By attribution policy to file content. + * - null -> remove the Co-Authored-By line and its preceding blank line + * - undefined -> leave content unchanged + * - string -> replace the value ($ escaped to block backreference injection) + * + * Pure content transform, relocated from bin/install.js per ADR-1508 + * (epic #1507, #1510 Phase 1). NOTE: getCommitAttribution stays in the + * installer — it is impure install-time config I/O (reads runtime + * settings.json, uses the install-time config-dir + cache), not a content + * transform, so it does not belong behind this content-conversion seam. + */ +function processAttribution( + content: string, + attribution: string | null | undefined, +): string { + if (attribution === null) { + // Remove Co-Authored-By lines and the preceding blank line + return content.replace(/(\r?\n){2}Co-Authored-By:.*$/gim, ''); + } + if (attribution === undefined) { + return content; + } + // Replace with custom attribution (escape $ to prevent backreference injection) + const safeAttribution = attribution.replace(/\$/g, '$$$$'); + return content.replace(/Co-Authored-By:.*$/gim, `Co-Authored-By: ${safeAttribution}`); +} + export = { + processAttribution, yamlIdentifier, yamlQuote, toSingleLine, diff --git a/src/runtime-name-policy.cts b/src/runtime-name-policy.cts index 8134ef5b8..07d0f50f2 100644 --- a/src/runtime-name-policy.cts +++ b/src/runtime-name-policy.cts @@ -88,3 +88,32 @@ export function resolveRuntimeNameFromCandidates(...candidates: unknown[]): stri } return null; } + +/** + * Map a canonical runtime id to its on-disk local config directory name + * (e.g. `cursor` -> `.cursor`, `windsurf` -> `.devin`). Unknown/empty inputs + * fall back to `.claude`. + * + * Pure runtime-identity projection. Relocated from `bin/install.js` per + * ADR-1508 (epic #1507, #1510 Phase 1) so the Runtime Artifact Conversion + * Module's rewrite engine can consume it without importing the installer. + * `bin/install.js` re-exports this same function for back-compat. + */ +export function getDirName(runtime: string): string { + if (runtime === 'copilot') return '.github'; + if (runtime === 'opencode') return '.opencode'; + if (runtime === 'gemini') return '.gemini'; + if (runtime === 'kilo') return '.kilo'; + if (runtime === 'codex') return '.codex'; + if (runtime === 'antigravity') return '.agents'; + if (runtime === 'cursor') return '.cursor'; + if (runtime === 'windsurf') return '.devin'; + if (runtime === 'augment') return '.augment'; + if (runtime === 'trae') return '.trae'; + if (runtime === 'qwen') return '.qwen'; + if (runtime === 'hermes') return '.hermes'; + if (runtime === 'kimi') return '.kimi-code'; + if (runtime === 'codebuddy') return '.codebuddy'; + if (runtime === 'cline') return '.cline'; + return '.claude'; +} diff --git a/tests/enh-1510-rewrite-engine-helper-relocation.test.cjs b/tests/enh-1510-rewrite-engine-helper-relocation.test.cjs new file mode 100644 index 000000000..47ad7cb00 --- /dev/null +++ b/tests/enh-1510-rewrite-engine-helper-relocation.test.cjs @@ -0,0 +1,110 @@ +'use strict'; + +// Enhancement #1510 (epic #1507, ADR-1508 Phase 1): behavior-preserving +// relocation of pure rewrite-engine helpers out of hand-authored bin/install.js. +// - getDirName -> gsd-core/bin/lib/runtime-name-policy.cjs +// - processAttribution -> gsd-core/bin/lib/runtime-artifact-conversion.cjs +// getCommitAttribution stays in install.js (impure install-time config I/O); the +// convertClaudeToAugmentMarkdown duplicate dedup is deferred to Phase 2's cleanup +// (entangled converter cluster; not required to unblock Phase 2). +// These tests exercise the REAL relocated functions at their new home (the +// generated .cjs) and assert install.js re-exports the SAME references +// (Hyrum: existing consumers import these names from bin/install.js). + +const { test, describe } = require('node:test'); +const assert = require('node:assert'); + +const runtimeNamePolicy = require('../gsd-core/bin/lib/runtime-name-policy.cjs'); +const conversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs'); +const installer = require('../bin/install.js'); + +// ── Slice A: getDirName relocated to runtime-name-policy ────────────────────── +describe('getDirName (relocated to runtime-name-policy)', () => { + const EXPECTED = { + claude: '.claude', + copilot: '.github', + opencode: '.opencode', + gemini: '.gemini', + kilo: '.kilo', + codex: '.codex', + antigravity: '.agents', + cursor: '.cursor', + windsurf: '.devin', + augment: '.augment', + trae: '.trae', + qwen: '.qwen', + hermes: '.hermes', + kimi: '.kimi-code', + codebuddy: '.codebuddy', + cline: '.cline', + }; + + for (const [runtime, dir] of Object.entries(EXPECTED)) { + test(`maps '${runtime}' to '${dir}'`, () => { + assert.strictEqual(runtimeNamePolicy.getDirName(runtime), dir); + }); + } + + test('falls back to .claude for an unknown runtime', () => { + assert.strictEqual(runtimeNamePolicy.getDirName('definitely-not-a-runtime'), '.claude'); + }); + + test('falls back to .claude for empty input', () => { + assert.strictEqual(runtimeNamePolicy.getDirName(''), '.claude'); + }); + + test('bin/install.js re-exports the SAME getDirName reference (no drift)', () => { + assert.strictEqual(installer.getDirName, runtimeNamePolicy.getDirName); + }); +}); + +// ── Slice B: processAttribution relocated to runtime-artifact-conversion ─────── +describe('processAttribution (relocated to runtime-artifact-conversion)', () => { + test('null removes the Co-Authored-By line and its preceding blank line', () => { + const input = 'Commit body line.\n\nCo-Authored-By: Someone '; + assert.strictEqual(conversion.processAttribution(input, null), 'Commit body line.'); + }); + + test('undefined leaves content unchanged', () => { + const input = 'Commit body.\n\nCo-Authored-By: Someone '; + assert.strictEqual(conversion.processAttribution(input, undefined), input); + }); + + test('a string replaces the attribution value', () => { + const input = 'Body\n\nCo-Authored-By: Old Name '; + assert.strictEqual( + conversion.processAttribution(input, 'New Name '), + 'Body\n\nCo-Authored-By: New Name ', + ); + }); + + test('escapes $ in the attribution to prevent backreference injection', () => { + const input = 'Body\n\nCo-Authored-By: x'; + // "$1" must survive literally, not be interpreted as a regex backreference. + assert.strictEqual( + conversion.processAttribution(input, 'A $1 B'), + 'Body\n\nCo-Authored-By: A $1 B', + ); + }); + + test('handles CRLF when removing (null)', () => { + const input = 'Body\r\n\r\nCo-Authored-By: Someone '; + assert.strictEqual(conversion.processAttribution(input, null), 'Body'); + }); + + test('replaces every Co-Authored-By line (global)', () => { + const input = 'Body\nCo-Authored-By: A \nCo-Authored-By: B '; + assert.strictEqual( + conversion.processAttribution(input, 'Z '), + 'Body\nCo-Authored-By: Z \nCo-Authored-By: Z ', + ); + }); + + test('bin/install.js re-exports the SAME processAttribution reference (no drift)', () => { + // processAttribution flows into install.js's exports via the + // ...runtimeArtifactConversion spread, so the installer's processAttribution + // must be the conversion module's single implementation (the local copy is + // deleted; install.js binds it for its internal callers). + assert.strictEqual(installer.processAttribution, conversion.processAttribution); + }); +});