* fix(#2831): expand HOME in OpenCode skill/template paths OpenCode does not shell-expand $HOME in @file references on any platform — the literal `@$HOME/...` path is resolved relative to the config command/ dir, producing `command/$HOME/...` (file not found). The previous fix for #2376 only guarded Windows; extend to all platforms. Closes #2831 * test(#2831): assert behavior via exported computePathPrefix, not source grep Addresses CodeRabbit review on PR #2842: - Extracts pathPrefix logic into a named, test-exported computePathPrefix helper in bin/install.js (no behavior change at the call site). - Rewrites bug-2376 and bug-2831 regression tests to call the exported function directly instead of regex-matching install.js source text, per the repo's no-source-grep testing standard. - Wraps temp-dir test setup in try/finally so cleanup runs on assertion failures (no leaked tmp dirs).
This commit is contained in:
@@ -483,6 +483,31 @@ function expandTilde(filePath) {
|
||||
return filePath;
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the path prefix used for `@file` references in installed command/skill
|
||||
* markdown. For global installs into a runtime config dir under $HOME, we
|
||||
* normally substitute the home prefix with `$HOME` so paths expand correctly
|
||||
* inside double-quoted shell commands. OpenCode is exempt on every platform:
|
||||
* its `@file` include syntax does NOT shell-expand `$HOME`, so a literal
|
||||
* `@$HOME/...` is treated as a path relative to the config command/ dir, which
|
||||
* resolves to `command/$HOME/...` (file not found). For OpenCode we always emit
|
||||
* the absolute resolved path. (#2376 Windows, #2831 macOS/Linux.)
|
||||
*
|
||||
* @param {object} args
|
||||
* @param {boolean} args.isGlobal - Global runtime install vs local project
|
||||
* @param {boolean} args.isOpencode - Whether the runtime is OpenCode
|
||||
* @param {boolean} args.isWindowsHost - process.platform === 'win32'
|
||||
* @param {string} args.resolvedTarget - Absolute target dir, forward-slashed
|
||||
* @param {string} args.homeDir - User home dir, forward-slashed
|
||||
* @returns {string} pathPrefix ending with '/'
|
||||
*/
|
||||
function computePathPrefix({ isGlobal, isOpencode, isWindowsHost: _isWindowsHost, resolvedTarget, homeDir }) {
|
||||
if (isGlobal && resolvedTarget.startsWith(homeDir) && !isOpencode) {
|
||||
return '$HOME' + resolvedTarget.slice(homeDir.length) + '/';
|
||||
}
|
||||
return `${resolvedTarget}/`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a hook command path using forward slashes for cross-platform compatibility.
|
||||
* On Windows, $HOME is not expanded by cmd.exe/PowerShell, so we use the actual path.
|
||||
@@ -6566,14 +6591,20 @@ function install(isGlobal, runtime = 'claude') {
|
||||
// For global installs: use $HOME/ so paths expand correctly inside double-quoted
|
||||
// shell commands (~ does NOT expand inside double quotes, causing MODULE_NOT_FOUND).
|
||||
// For local installs: use resolved absolute path (may be outside $HOME).
|
||||
// Exception: OpenCode on Windows does not expand $HOME in @file references —
|
||||
// use the absolute path instead so @$HOME/... references resolve correctly (#2376).
|
||||
// Exception: OpenCode does not expand $HOME in @file references on any platform —
|
||||
// `@$HOME/...` is treated as a literal path relative to the config dir, producing
|
||||
// `command/$HOME/...` (file not found). Use the absolute path for OpenCode so
|
||||
// @-references resolve correctly (#2376 Windows, #2831 macOS/Linux).
|
||||
const resolvedTarget = path.resolve(targetDir).replace(/\\/g, '/');
|
||||
const homeDir = os.homedir().replace(/\\/g, '/');
|
||||
const isWindowsHost = process.platform === 'win32';
|
||||
const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
|
||||
? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
|
||||
: `${resolvedTarget}/`;
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal,
|
||||
isOpencode,
|
||||
isWindowsHost,
|
||||
resolvedTarget,
|
||||
homeDir,
|
||||
});
|
||||
|
||||
let runtimeLabel = 'Claude Code';
|
||||
if (isOpencode) runtimeLabel = 'OpenCode';
|
||||
@@ -8417,6 +8448,7 @@ function installAllRuntimes(runtimes, isGlobal, isInteractive) {
|
||||
if (process.env.GSD_TEST_MODE) {
|
||||
module.exports = {
|
||||
yamlIdentifier,
|
||||
computePathPrefix,
|
||||
getCodexSkillAdapterHeader,
|
||||
convertClaudeCommandToCursorSkill,
|
||||
convertClaudeAgentToCursorAgent,
|
||||
|
||||
@@ -5,114 +5,53 @@
|
||||
* resolve @$HOME/... file references in installed command files.
|
||||
*
|
||||
* Fix: install.js must use the absolute path (not $HOME-relative) when installing
|
||||
* for OpenCode on Windows.
|
||||
* for OpenCode. (Generalized to all platforms in #2831 — OpenCode `@file`
|
||||
* references are not shell-expanded on any platform.)
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const { describe, test, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const INSTALL_JS_PATH = path.join(__dirname, '..', 'bin', 'install.js');
|
||||
let computePathPrefix;
|
||||
|
||||
before(() => {
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
// Re-require fresh in case other tests already loaded it.
|
||||
delete require.cache[require.resolve('../bin/install.js')];
|
||||
({ computePathPrefix } = require('../bin/install.js'));
|
||||
});
|
||||
|
||||
after(() => {
|
||||
delete process.env.GSD_TEST_MODE;
|
||||
});
|
||||
|
||||
describe('bug-2376: OpenCode on Windows must use absolute path, not $HOME', () => {
|
||||
test('install.js exists', () => {
|
||||
assert.ok(fs.existsSync(INSTALL_JS_PATH), 'bin/install.js should exist');
|
||||
test('computePathPrefix is exported by install.js', () => {
|
||||
assert.equal(typeof computePathPrefix, 'function');
|
||||
});
|
||||
|
||||
test('install.js pathPrefix computation skips $HOME for OpenCode on Windows', () => {
|
||||
const content = fs.readFileSync(INSTALL_JS_PATH, 'utf-8');
|
||||
|
||||
// The fix must include a Windows detection condition for OpenCode
|
||||
const hasWindowsOpenCodeGuard = (
|
||||
content.includes('isWindowsHost') ||
|
||||
(content.includes("'win32'") && content.includes('isOpencode') && content.includes('pathPrefix'))
|
||||
);
|
||||
assert.ok(
|
||||
hasWindowsOpenCodeGuard,
|
||||
'install.js must include a Windows platform guard for OpenCode pathPrefix computation'
|
||||
);
|
||||
test('OpenCode on Windows: pathPrefix is absolute (no $HOME substitution)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: true,
|
||||
isWindowsHost: true,
|
||||
resolvedTarget: 'C:/Users/user/.config/opencode',
|
||||
homeDir: 'C:/Users/user',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, 'C:/Users/user/.config/opencode/');
|
||||
assert.ok(!pathPrefix.includes('$HOME'));
|
||||
});
|
||||
|
||||
test('install.js pathPrefix guard excludes $HOME for OpenCode+Windows combination', () => {
|
||||
const content = fs.readFileSync(INSTALL_JS_PATH, 'utf-8');
|
||||
|
||||
// The pathPrefix assignment must include a guard that prevents $HOME substitution
|
||||
// for OpenCode on Windows (process.platform === 'win32').
|
||||
const pathPrefixBlock = content.match(/const pathPrefix[\s\S]{0,500}resolvedTarget/);
|
||||
assert.ok(pathPrefixBlock, 'pathPrefix assignment block should be present');
|
||||
|
||||
const block = pathPrefixBlock[0];
|
||||
const excludesOpenCodeWindows = (
|
||||
block.includes('isWindowsHost') ||
|
||||
(block.includes('isOpencode') && block.includes('win32'))
|
||||
);
|
||||
assert.ok(
|
||||
excludesOpenCodeWindows,
|
||||
'pathPrefix computation must exclude $HOME substitution for OpenCode on Windows'
|
||||
);
|
||||
});
|
||||
|
||||
test('pathPrefix simulation: OpenCode on Windows uses absolute path', () => {
|
||||
// Simulate the fixed pathPrefix computation for Windows+OpenCode
|
||||
const isGlobal = true;
|
||||
const isOpencode = true;
|
||||
const isWindowsHost = true; // simulated Windows
|
||||
const resolvedTarget = 'C:/Users/user/.config/opencode';
|
||||
const homeDir = 'C:/Users/user';
|
||||
|
||||
const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
|
||||
? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
|
||||
: `${resolvedTarget}/`;
|
||||
|
||||
assert.strictEqual(
|
||||
pathPrefix,
|
||||
'C:/Users/user/.config/opencode/',
|
||||
'OpenCode on Windows should use absolute path, not $HOME-relative'
|
||||
);
|
||||
assert.ok(
|
||||
!pathPrefix.includes('$HOME'),
|
||||
'OpenCode on Windows pathPrefix must not contain $HOME'
|
||||
);
|
||||
});
|
||||
|
||||
test('pathPrefix simulation: OpenCode on Linux/macOS still uses $HOME', () => {
|
||||
// Non-Windows OpenCode should still use $HOME (POSIX shells expand it)
|
||||
const isGlobal = true;
|
||||
const isOpencode = true;
|
||||
const isWindowsHost = false; // simulated Linux/macOS
|
||||
const homeDir = '/home/user';
|
||||
const resolvedTarget = '/home/user/.config/opencode';
|
||||
|
||||
const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
|
||||
? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
|
||||
: `${resolvedTarget}/`;
|
||||
|
||||
assert.strictEqual(
|
||||
pathPrefix,
|
||||
'$HOME/.config/opencode/',
|
||||
'OpenCode on Linux/macOS should still use $HOME-relative path'
|
||||
);
|
||||
});
|
||||
|
||||
test('pathPrefix simulation: Claude Code on Windows still uses $HOME (unaffected)', () => {
|
||||
// Claude Code on Windows is handled by Claude Code's own shell, which expands $HOME
|
||||
const isGlobal = true;
|
||||
const isOpencode = false; // Claude Code, not OpenCode
|
||||
const isWindowsHost = true;
|
||||
const homeDir = 'C:/Users/user';
|
||||
const resolvedTarget = 'C:/Users/user/.claude';
|
||||
|
||||
const pathPrefix = isGlobal && resolvedTarget.startsWith(homeDir) && !(isOpencode && isWindowsHost)
|
||||
? '$HOME' + resolvedTarget.slice(homeDir.length) + '/'
|
||||
: `${resolvedTarget}/`;
|
||||
|
||||
assert.strictEqual(
|
||||
pathPrefix,
|
||||
'$HOME/.claude/',
|
||||
'Claude Code on Windows should still use $HOME-relative path (Claude Code handles this)'
|
||||
);
|
||||
test('Claude Code on Windows: pathPrefix still uses $HOME (unaffected)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: false,
|
||||
isWindowsHost: true,
|
||||
resolvedTarget: 'C:/Users/user/.claude',
|
||||
homeDir: 'C:/Users/user',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, '$HOME/.claude/');
|
||||
});
|
||||
});
|
||||
|
||||
149
tests/bug-2831-opencode-home-path-prefix.test.cjs
Normal file
149
tests/bug-2831-opencode-home-path-prefix.test.cjs
Normal file
@@ -0,0 +1,149 @@
|
||||
/**
|
||||
* Regression test for #2831: OpenCode @file references contain literal `$HOME`
|
||||
* which OpenCode does not expand — `@$HOME/.config/opencode/...` is resolved
|
||||
* as a path relative to the config command/ dir, producing
|
||||
* `command/$HOME/.config/opencode/...` (file not found).
|
||||
*
|
||||
* Root cause: install.js pathPrefix used `$HOME`-relative paths for OpenCode on
|
||||
* non-Windows hosts (only Windows was guarded by #2376). OpenCode's `@file`
|
||||
* include syntax does NOT shell-expand `$HOME` on any platform.
|
||||
*
|
||||
* Fix: pathPrefix must use the absolute path for OpenCode on all platforms.
|
||||
*
|
||||
* Tests exercise install.js's exported `computePathPrefix` directly (no source
|
||||
* grepping) and additionally simulate the `copyFlattenedCommands` substitution
|
||||
* pipeline on a temp tree to verify no `$HOME` literal leaks into emitted files.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const { describe, test, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
|
||||
let computePathPrefix;
|
||||
|
||||
before(() => {
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
delete require.cache[require.resolve('../bin/install.js')];
|
||||
({ computePathPrefix } = require('../bin/install.js'));
|
||||
});
|
||||
|
||||
after(() => {
|
||||
delete process.env.GSD_TEST_MODE;
|
||||
});
|
||||
|
||||
describe('bug-2831: OpenCode pathPrefix uses absolute path on all platforms', () => {
|
||||
test('computePathPrefix is exported by install.js', () => {
|
||||
assert.equal(typeof computePathPrefix, 'function');
|
||||
});
|
||||
|
||||
test('OpenCode on macOS: pathPrefix is absolute (no $HOME)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: true,
|
||||
isWindowsHost: false,
|
||||
homeDir: '/Users/alice',
|
||||
resolvedTarget: '/Users/alice/.config/opencode',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, '/Users/alice/.config/opencode/');
|
||||
assert.ok(!pathPrefix.includes('$HOME'));
|
||||
});
|
||||
|
||||
test('OpenCode on Linux: pathPrefix is absolute (no $HOME)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: true,
|
||||
isWindowsHost: false,
|
||||
homeDir: '/home/bob',
|
||||
resolvedTarget: '/home/bob/.config/opencode',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, '/home/bob/.config/opencode/');
|
||||
assert.ok(!pathPrefix.includes('$HOME'));
|
||||
});
|
||||
|
||||
test('OpenCode on Windows: pathPrefix is absolute (preserves #2376)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: true,
|
||||
isWindowsHost: true,
|
||||
homeDir: 'C:/Users/carol',
|
||||
resolvedTarget: 'C:/Users/carol/.config/opencode',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, 'C:/Users/carol/.config/opencode/');
|
||||
});
|
||||
|
||||
test('Claude Code on macOS: pathPrefix still uses $HOME (unaffected)', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: false,
|
||||
isWindowsHost: false,
|
||||
homeDir: '/Users/alice',
|
||||
resolvedTarget: '/Users/alice/.claude',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, '$HOME/.claude/');
|
||||
});
|
||||
|
||||
test('Local install (non-global): pathPrefix uses absolute path regardless of runtime', () => {
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: false,
|
||||
isOpencode: false,
|
||||
isWindowsHost: false,
|
||||
homeDir: '/Users/alice',
|
||||
resolvedTarget: '/Users/alice/projects/foo/.claude',
|
||||
});
|
||||
assert.strictEqual(pathPrefix, '/Users/alice/projects/foo/.claude/');
|
||||
});
|
||||
|
||||
test('Substitution pipeline simulation: OpenCode emits no @$HOME literal', () => {
|
||||
// This validates the same regex substitution pipeline used by
|
||||
// copyFlattenedCommands when writing OpenCode command files. We invoke the
|
||||
// real exported computePathPrefix; the regex passes mirror the install.js
|
||||
// call sites (globalClaudeRegex / globalClaudeHomeRegex).
|
||||
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2831-'));
|
||||
try {
|
||||
const srcRoot = path.join(tmp, 'src');
|
||||
const targetRoot = path.join(tmp, 'home', '.config', 'opencode');
|
||||
const srcCmdDir = path.join(srcRoot, 'commands', 'gsd');
|
||||
fs.mkdirSync(srcCmdDir, { recursive: true });
|
||||
fs.mkdirSync(targetRoot, { recursive: true });
|
||||
|
||||
const srcFile = path.join(srcCmdDir, 'autonomous.md');
|
||||
fs.writeFileSync(
|
||||
srcFile,
|
||||
'---\nname: autonomous\n---\n<execution_context>\n@~/.claude/get-shit-done/workflows/autonomous.md\n@$HOME/.claude/get-shit-done/references/ui-brand.md\n</execution_context>\n'
|
||||
);
|
||||
|
||||
const homeDir = path.join(tmp, 'home').replace(/\\/g, '/');
|
||||
const resolvedTarget = targetRoot.replace(/\\/g, '/');
|
||||
const pathPrefix = computePathPrefix({
|
||||
isGlobal: true,
|
||||
isOpencode: true,
|
||||
isWindowsHost: false,
|
||||
homeDir,
|
||||
resolvedTarget,
|
||||
});
|
||||
|
||||
let content = fs.readFileSync(srcFile, 'utf8');
|
||||
content = content.replace(/~\/\.claude\//g, pathPrefix);
|
||||
content = content.replace(/\$HOME\/\.claude\//g, pathPrefix);
|
||||
|
||||
assert.ok(
|
||||
!/@\$HOME\b/.test(content),
|
||||
`output must not contain @$HOME literal; got:\n${content}`
|
||||
);
|
||||
assert.ok(
|
||||
!/\$HOME\b/.test(content),
|
||||
`output must not contain $HOME literal; got:\n${content}`
|
||||
);
|
||||
assert.ok(
|
||||
content.includes(`@${resolvedTarget}/`),
|
||||
`output should include absolute path with @ prefix; got:\n${content}`
|
||||
);
|
||||
} finally {
|
||||
fs.rmSync(tmp, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user