feat: deepen shell command projection seam (#3445)
* feat: deepen shell command projection seam * chore: add changeset for shell projection seam * feat: centralize local and portable hook path projection * docs: avoid prompt-scan false positive in installer migrations * docs(adr): fix path-style token punctuation
This commit is contained in:
@@ -17,68 +17,33 @@
|
||||
|
||||
const { describe, test, before } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
|
||||
// All hooks that the installer registers for local installs
|
||||
const HOOKS = [
|
||||
'gsd-statusline.js',
|
||||
'gsd-check-update.js',
|
||||
'gsd-context-monitor.js',
|
||||
'gsd-prompt-guard.js',
|
||||
'gsd-read-guard.js',
|
||||
'gsd-workflow-guard.js',
|
||||
'gsd-validate-commit.sh',
|
||||
'gsd-session-state.sh',
|
||||
'gsd-phase-boundary.sh',
|
||||
];
|
||||
const projection = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'shell-command-projection.cjs'));
|
||||
const { projectLocalHookPrefix, projectShellCommandText } = projection;
|
||||
|
||||
describe('bug #1906: local hook commands use $CLAUDE_PROJECT_DIR', () => {
|
||||
let src;
|
||||
|
||||
before(() => {
|
||||
src = fs.readFileSync(INSTALL_SRC, 'utf-8');
|
||||
assert.equal(typeof projectLocalHookPrefix, 'function');
|
||||
assert.equal(typeof projectShellCommandText, 'function');
|
||||
});
|
||||
|
||||
test('localPrefix definition includes $CLAUDE_PROJECT_DIR for non-Gemini runtimes', () => {
|
||||
// localPrefix is now a ternary — Gemini/Antigravity use bare dirName (#2557),
|
||||
// all other runtimes use "$CLAUDE_PROJECT_DIR"/ to anchor hook paths.
|
||||
assert.match(src, /const localPrefix\s*=[\s\S]*?"\$CLAUDE_PROJECT_DIR"/,
|
||||
'localPrefix definition must include "$CLAUDE_PROJECT_DIR" branch for non-Gemini runtimes');
|
||||
test('non-Gemini runtimes get $CLAUDE_PROJECT_DIR anchored local prefix', () => {
|
||||
const prefix = projectLocalHookPrefix({ runtime: 'claude', dirName: '.claude' });
|
||||
assert.equal(prefix, '"$CLAUDE_PROJECT_DIR"/.claude');
|
||||
});
|
||||
|
||||
for (const hook of HOOKS) {
|
||||
test(`${hook} local command uses localPrefix (not bare dirName)`, () => {
|
||||
// Find all local command strings for this hook
|
||||
// The pattern is: `<runner> ' + localPrefix + '/hooks/<hook>'`
|
||||
// or the old broken pattern: `<runner> ' + dirName + '/hooks/<hook>'`
|
||||
const hookEscaped = hook.replace(/\./g, '\\.');
|
||||
const brokenPattern = new RegExp(
|
||||
`['"](?:node|bash)\\s['"]\\s*\\+\\s*dirName\\s*\\+\\s*['"]/hooks/${hookEscaped}['"]`
|
||||
);
|
||||
assert.ok(
|
||||
!brokenPattern.test(src),
|
||||
`${hook} must not use bare dirName — should use localPrefix for cwd-independent resolution`
|
||||
);
|
||||
test('local command projection for non-Gemini keeps $CLAUDE_PROJECT_DIR anchor', () => {
|
||||
const prefix = projectLocalHookPrefix({ runtime: 'claude', dirName: '.claude' });
|
||||
const command = projectShellCommandText({
|
||||
runnerToken: '"/usr/local/bin/node"',
|
||||
argTokens: [`${prefix}/hooks/gsd-context-monitor.js`],
|
||||
runtime: 'claude',
|
||||
platform: 'linux',
|
||||
});
|
||||
}
|
||||
|
||||
test('no local hook command uses bare dirName + /hooks/', () => {
|
||||
// Broader check: no local (non-global) hook path should use dirName directly
|
||||
// The pattern `': '<runner> ' + dirName + '/hooks/'` is the broken form
|
||||
const lines = src.split('\n');
|
||||
const offenders = [];
|
||||
for (let i = 0; i < lines.length; i++) {
|
||||
const line = lines[i];
|
||||
// Match lines that build local hook commands with bare dirName
|
||||
if (/['"](?:node|bash)\s['"][^;]*\+\s*dirName\s*\+\s*['"]\/hooks\//.test(line)) {
|
||||
offenders.push(`line ${i + 1}: ${line.trim()}`);
|
||||
}
|
||||
}
|
||||
assert.equal(offenders.length, 0,
|
||||
'Found local hook commands using bare dirName instead of localPrefix:\n' +
|
||||
offenders.join('\n'));
|
||||
assert.equal(
|
||||
command,
|
||||
'"/usr/local/bin/node" "$CLAUDE_PROJECT_DIR"/.claude/hooks/gsd-context-monitor.js',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -13,62 +13,40 @@
|
||||
* dir as cwd. Claude Code and others still use "$CLAUDE_PROJECT_DIR"/ (#1906).
|
||||
*/
|
||||
|
||||
const { describe, test, before } = require('node:test');
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
||||
const INSTALL_SRC = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
const projection = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'shell-command-projection.cjs'));
|
||||
const { projectLocalHookPrefix, projectShellCommandText } = projection;
|
||||
|
||||
describe('bug #2557: Gemini/Antigravity local hooks use relative paths (not $CLAUDE_PROJECT_DIR)', () => {
|
||||
let src;
|
||||
|
||||
before(() => {
|
||||
src = fs.readFileSync(INSTALL_SRC, 'utf-8');
|
||||
test('Gemini local prefix is bare dirName', () => {
|
||||
assert.equal(projectLocalHookPrefix({ runtime: 'gemini', dirName: '.gemini' }), '.gemini');
|
||||
});
|
||||
|
||||
test('localPrefix is a ternary that branches on Gemini/Antigravity', () => {
|
||||
assert.match(
|
||||
src,
|
||||
/const localPrefix\s*=\s*\(runtime\s*===\s*['"]gemini['"]/,
|
||||
'localPrefix must branch on runtime === "gemini"',
|
||||
test('Antigravity local prefix is bare dirName', () => {
|
||||
assert.equal(projectLocalHookPrefix({ runtime: 'antigravity', dirName: '.agent' }), '.agent');
|
||||
});
|
||||
|
||||
test('non-Gemini local prefix remains $CLAUDE_PROJECT_DIR anchored', () => {
|
||||
assert.equal(
|
||||
projectLocalHookPrefix({ runtime: 'claude', dirName: '.claude' }),
|
||||
'"$CLAUDE_PROJECT_DIR"/.claude',
|
||||
);
|
||||
});
|
||||
|
||||
test('Gemini/Antigravity branch of localPrefix uses bare dirName (relative path)', () => {
|
||||
// The ternary must assign `dirName` (not `"$CLAUDE_PROJECT_DIR"/` + dirName)
|
||||
// for the Gemini branch so hooks use a relative path on all platforms.
|
||||
assert.match(
|
||||
src,
|
||||
/const localPrefix\s*=\s*\(runtime\s*===\s*['"]gemini['"]\s*\|\|\s*runtime\s*===\s*['"]antigravity['"]\)\s*\n?\s*\?\s*dirName/,
|
||||
'Gemini/Antigravity branch must resolve to bare dirName',
|
||||
);
|
||||
});
|
||||
|
||||
test('non-Gemini branch of localPrefix uses "$CLAUDE_PROJECT_DIR"/', () => {
|
||||
// The else branch must still use "$CLAUDE_PROJECT_DIR"/ to fix #1906 for
|
||||
// Claude Code and other runtimes that do set the variable.
|
||||
assert.match(
|
||||
src,
|
||||
/:\s*['"]\"\$CLAUDE_PROJECT_DIR\"\//,
|
||||
'non-Gemini branch must use "$CLAUDE_PROJECT_DIR"/ prefix',
|
||||
);
|
||||
});
|
||||
|
||||
test('Gemini/Antigravity hook commands do not contain "$CLAUDE_PROJECT_DIR" literal', () => {
|
||||
// Since localPrefix is now dirName for Gemini/Antigravity, no command
|
||||
// string built via `localPrefix` should contain the variable literal.
|
||||
// We verify by checking that the only occurrence of $CLAUDE_PROJECT_DIR
|
||||
// in the localPrefix definition is in the non-Gemini (else) branch.
|
||||
const lines = src.split('\n');
|
||||
const prefixDefIdx = lines.findIndex(l => /const localPrefix\s*=/.test(l));
|
||||
assert.ok(prefixDefIdx >= 0, 'localPrefix definition not found');
|
||||
// The Gemini (truthy) branch is the line right after the ternary condition.
|
||||
// It must NOT contain $CLAUDE_PROJECT_DIR.
|
||||
const geminiLine = lines[prefixDefIdx + 1] || '';
|
||||
test('Gemini local command projection does not contain "$CLAUDE_PROJECT_DIR"', () => {
|
||||
const prefix = projectLocalHookPrefix({ runtime: 'gemini', dirName: '.gemini' });
|
||||
const command = projectShellCommandText({
|
||||
runnerToken: '"/usr/local/bin/node"',
|
||||
argTokens: [`${prefix}/hooks/gsd-check-update.js`],
|
||||
runtime: 'gemini',
|
||||
platform: 'linux',
|
||||
});
|
||||
assert.ok(
|
||||
!geminiLine.includes('$CLAUDE_PROJECT_DIR'),
|
||||
'Gemini branch of localPrefix must not reference $CLAUDE_PROJECT_DIR',
|
||||
!command.includes('$CLAUDE_PROJECT_DIR'),
|
||||
'Gemini local command must not include $CLAUDE_PROJECT_DIR',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,7 +9,14 @@ const path = require('node:path');
|
||||
const projection = require(path.join(__dirname, '..', 'get-shit-done', 'bin', 'lib', 'shell-command-projection.cjs'));
|
||||
const install = require(path.join(__dirname, '..', 'bin', 'install.js'));
|
||||
|
||||
const { hookCommandNeedsPowerShellCallOperator, formatHookCommandForRuntime } = projection;
|
||||
const {
|
||||
hookCommandNeedsPowerShellCallOperator,
|
||||
formatHookCommandForRuntime,
|
||||
isManagedHookBasename,
|
||||
projectLocalHookPrefix,
|
||||
projectLegacySettingsHookCommand,
|
||||
projectPortableHookBaseDir,
|
||||
} = projection;
|
||||
const { buildHookCommand, rewriteLegacyManagedNodeHookCommands } = install;
|
||||
|
||||
describe('bug #3413: Shell Command Projection Module uses runtime-aware hook policy', () => {
|
||||
@@ -71,3 +78,67 @@ describe('bug #3413: installer hook surfaces consume runtime-aware projection',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bug #3439: shell projection module owns managed-hook policy and legacy rewrite projection', () => {
|
||||
test('isManagedHookBasename is surface-aware', () => {
|
||||
assert.equal(isManagedHookBasename('/x/hooks/gsd-check-update.js', { surface: 'settings-json' }), true);
|
||||
assert.equal(isManagedHookBasename('/x/hooks/gsd-statusline.js', { surface: 'settings-json' }), true);
|
||||
assert.equal(isManagedHookBasename('/x/hooks/gsd-statusline.js', { surface: 'codex-toml' }), false);
|
||||
assert.equal(isManagedHookBasename('/x/hooks/custom-hook.js', { surface: 'settings-json' }), false);
|
||||
});
|
||||
|
||||
test('projectLegacySettingsHookCommand preserves non-Windows script token shape', () => {
|
||||
const command = projectLegacySettingsHookCommand({
|
||||
absoluteRunner: '"/usr/local/bin/node"',
|
||||
scriptPath: '/x/hooks/gsd-statusline.js',
|
||||
scriptToken: "'/x/hooks/gsd-statusline.js'",
|
||||
platform: 'linux',
|
||||
runtime: 'claude',
|
||||
});
|
||||
assert.equal(command, `"/usr/local/bin/node" '/x/hooks/gsd-statusline.js'`);
|
||||
});
|
||||
|
||||
test('projectLegacySettingsHookCommand normalizes Windows managed paths and runtime wrapper policy', () => {
|
||||
const command = projectLegacySettingsHookCommand({
|
||||
absoluteRunner: '"C:/nvm4w/nodejs/node.exe"',
|
||||
scriptPath: 'C:\\Users\\me\\.gemini\\hooks\\gsd-prompt-guard.js',
|
||||
scriptToken: "'C:\\Users\\me\\.gemini\\hooks\\gsd-prompt-guard.js'",
|
||||
platform: 'win32',
|
||||
runtime: 'gemini',
|
||||
});
|
||||
assert.equal(command, '& "C:/nvm4w/nodejs/node.exe" "C:/Users/me/.gemini/hooks/gsd-prompt-guard.js"');
|
||||
});
|
||||
|
||||
test('projectLocalHookPrefix centralizes runtime-specific project-dir interpolation policy', () => {
|
||||
assert.equal(projectLocalHookPrefix({ runtime: 'gemini', dirName: '.gemini' }), '.gemini');
|
||||
assert.equal(projectLocalHookPrefix({ runtime: 'antigravity', dirName: '.agent' }), '.agent');
|
||||
assert.equal(
|
||||
projectLocalHookPrefix({ runtime: 'claude', dirName: '.claude' }),
|
||||
'"$CLAUDE_PROJECT_DIR"/.claude',
|
||||
);
|
||||
});
|
||||
|
||||
test('projectPortableHookBaseDir centralizes $HOME interpolation policy', () => {
|
||||
assert.equal(
|
||||
projectPortableHookBaseDir({
|
||||
configDir: '/Users/me/.claude',
|
||||
homeDir: '/Users/me',
|
||||
}),
|
||||
'$HOME/.claude',
|
||||
);
|
||||
assert.equal(
|
||||
projectPortableHookBaseDir({
|
||||
configDir: 'C:\\Users\\me\\.claude',
|
||||
homeDir: 'C:\\Users\\me',
|
||||
}),
|
||||
'$HOME/.claude',
|
||||
);
|
||||
assert.equal(
|
||||
projectPortableHookBaseDir({
|
||||
configDir: '/opt/custom/.claude',
|
||||
homeDir: '/Users/me',
|
||||
}),
|
||||
'/opt/custom/.claude',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user