mirror of
https://github.com/Yeachan-Heo/oh-my-claudecode.git
synced 2026-09-03 06:25:33 +08:00
fix(hooks): robust path resolution and graceful fallback for stale CLAUDE_PLUGIN_ROOT (#1011)
When a plugin update replaces an old version directory, sessions still referencing the old CLAUDE_PLUGIN_ROOT fail with MODULE_NOT_FOUND, stopping execution. This was intermittent and depended on timing of cache cleanup vs hook invocation. Changes: - run.cjs: validate target exists before spawning; fall back to latest available plugin cache version with the same script; exit 0 gracefully when no valid target is found (prevents hook errors from blocking execution) - session-start.mjs: use atomic temp-symlink + rename for symlink updates to eliminate the race window between rmSync and symlinkSync - Add regression tests for run.cjs graceful fallback scenarios Fixes #1007 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,8 @@
|
||||
*/
|
||||
|
||||
const { spawnSync } = require('child_process');
|
||||
const { existsSync, realpathSync } = require('fs');
|
||||
const { join, basename, dirname } = require('path');
|
||||
|
||||
const target = process.argv[2];
|
||||
if (!target) {
|
||||
@@ -26,9 +28,81 @@ if (!target) {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the hook script target path, handling stale CLAUDE_PLUGIN_ROOT.
|
||||
*
|
||||
* When a plugin update replaces an old version directory with a symlink (or
|
||||
* deletes it entirely), sessions that still reference the old version via
|
||||
* CLAUDE_PLUGIN_ROOT will fail with MODULE_NOT_FOUND.
|
||||
*
|
||||
* Resolution strategy:
|
||||
* 1. Use the target as-is if it exists.
|
||||
* 2. Try resolving through realpathSync (follows symlinks).
|
||||
* 3. Scan the plugin cache for the latest available version that has the
|
||||
* same script name and use that instead.
|
||||
* 4. If all else fails, return null (caller exits cleanly).
|
||||
*
|
||||
* See: https://github.com/Yeachan-Heo/oh-my-claudecode/issues/1007
|
||||
*/
|
||||
function resolveTarget(targetPath) {
|
||||
// Fast path: target exists (common case)
|
||||
if (existsSync(targetPath)) return targetPath;
|
||||
|
||||
// Try realpath resolution (handles broken symlinks that resolve elsewhere)
|
||||
try {
|
||||
const resolved = realpathSync(targetPath);
|
||||
if (existsSync(resolved)) return resolved;
|
||||
} catch {
|
||||
// realpathSync throws if the path doesn't exist at all — expected
|
||||
}
|
||||
|
||||
// Fallback: scan plugin cache for the same script in the latest version.
|
||||
// CLAUDE_PLUGIN_ROOT is e.g. ~/.claude/plugins/cache/omc/oh-my-claudecode/4.2.14
|
||||
// We look one level up for sibling version directories.
|
||||
try {
|
||||
const pluginRoot = process.env.CLAUDE_PLUGIN_ROOT;
|
||||
if (!pluginRoot) return null;
|
||||
|
||||
const cacheBase = dirname(pluginRoot); // .../oh-my-claudecode/
|
||||
const scriptRelative = targetPath.slice(pluginRoot.length); // /scripts/persistent-mode.cjs
|
||||
|
||||
if (!scriptRelative || !existsSync(cacheBase)) return null;
|
||||
|
||||
// Find version directories (real dirs or valid symlinks), pick latest
|
||||
const { readdirSync, lstatSync, readlinkSync } = require('fs');
|
||||
const entries = readdirSync(cacheBase).filter(v => /^\d+\.\d+\.\d+/.test(v));
|
||||
|
||||
// Sort descending by semver
|
||||
entries.sort((a, b) => {
|
||||
const pa = a.split('.').map(Number);
|
||||
const pb = b.split('.').map(Number);
|
||||
for (let i = 0; i < 3; i++) {
|
||||
if ((pa[i] || 0) !== (pb[i] || 0)) return (pb[i] || 0) - (pa[i] || 0);
|
||||
}
|
||||
return 0;
|
||||
});
|
||||
|
||||
for (const version of entries) {
|
||||
const candidate = join(cacheBase, version) + scriptRelative;
|
||||
if (existsSync(candidate)) return candidate;
|
||||
}
|
||||
} catch {
|
||||
// Any error in fallback scan — give up gracefully
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
const resolved = resolveTarget(target);
|
||||
if (!resolved) {
|
||||
// Target not found anywhere — exit cleanly so hooks are never blocked.
|
||||
// This is the graceful fallback for stale CLAUDE_PLUGIN_ROOT paths.
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[target, ...process.argv.slice(3)],
|
||||
[resolved, ...process.argv.slice(3)],
|
||||
{
|
||||
stdio: 'inherit',
|
||||
env: process.env,
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
* Cross-platform: Windows, macOS, Linux
|
||||
*/
|
||||
|
||||
import { existsSync, readFileSync, readdirSync, rmSync, mkdirSync, writeFileSync, symlinkSync, lstatSync, readlinkSync, unlinkSync } from 'fs';
|
||||
import { existsSync, readFileSync, readdirSync, rmSync, mkdirSync, writeFileSync, symlinkSync, lstatSync, readlinkSync, unlinkSync, renameSync } from 'fs';
|
||||
import { join, dirname } from 'path';
|
||||
import { homedir } from 'os';
|
||||
import { fileURLToPath, pathToFileURL } from 'url';
|
||||
@@ -434,26 +434,42 @@ ${cleanContent}
|
||||
const versionPath = join(cacheBase, version);
|
||||
const stat = lstatSync(versionPath);
|
||||
|
||||
if (stat.isSymbolicLink()) {
|
||||
// Already a symlink — update only if pointing to wrong target
|
||||
const target = readlinkSync(versionPath);
|
||||
if (target === latest) continue;
|
||||
unlinkSync(versionPath);
|
||||
} else if (stat.isDirectory()) {
|
||||
rmSync(versionPath, { recursive: true, force: true });
|
||||
}
|
||||
const isWin = process.platform === 'win32';
|
||||
const symlinkTarget = isWin ? join(cacheBase, latest) : latest;
|
||||
|
||||
// Create symlink: e.g. 4.4.1 -> 4.4.3
|
||||
// On Windows, use 'junction' type which works without Developer Mode
|
||||
// or admin privileges. On Unix, use default (relative symlink).
|
||||
try {
|
||||
const isWin = process.platform === 'win32';
|
||||
const symlinkTarget = isWin ? join(cacheBase, latest) : latest;
|
||||
symlinkSync(symlinkTarget, versionPath, isWin ? 'junction' : undefined);
|
||||
} catch (symlinkErr) {
|
||||
// EEXIST: another session raced us and created the symlink — safe to ignore.
|
||||
if (symlinkErr?.code !== 'EEXIST') {
|
||||
// Symlink genuinely failed. Leave the path as-is rather than losing it.
|
||||
if (stat.isSymbolicLink()) {
|
||||
// Already a symlink — update only if pointing to wrong target.
|
||||
// Use atomic temp-symlink + rename to avoid a window where
|
||||
// the path doesn't exist (fixes race in issue #1007).
|
||||
const target = readlinkSync(versionPath);
|
||||
if (target === latest || target === join(cacheBase, latest)) continue;
|
||||
try {
|
||||
const tmpLink = versionPath + '.tmp.' + process.pid;
|
||||
symlinkSync(symlinkTarget, tmpLink, isWin ? 'junction' : undefined);
|
||||
try {
|
||||
renameSync(tmpLink, versionPath);
|
||||
} catch {
|
||||
// rename failed (e.g. cross-device) — fall back to unlink+symlink
|
||||
try { unlinkSync(tmpLink); } catch {}
|
||||
unlinkSync(versionPath);
|
||||
symlinkSync(symlinkTarget, versionPath, isWin ? 'junction' : undefined);
|
||||
}
|
||||
} catch (swapErr) {
|
||||
if (swapErr?.code !== 'EEXIST') {
|
||||
// Leave as-is rather than losing it
|
||||
}
|
||||
}
|
||||
} else if (stat.isDirectory()) {
|
||||
// Directory → symlink: cannot be atomic, but run.cjs now
|
||||
// handles missing targets gracefully (issue #1007).
|
||||
rmSync(versionPath, { recursive: true, force: true });
|
||||
try {
|
||||
symlinkSync(symlinkTarget, versionPath, isWin ? 'junction' : undefined);
|
||||
} catch (symlinkErr) {
|
||||
// EEXIST: another session raced us — safe to ignore.
|
||||
if (symlinkErr?.code !== 'EEXIST') {
|
||||
// Symlink genuinely failed. Leave the path as-is.
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
|
||||
200
src/__tests__/run-cjs-graceful-fallback.test.ts
Normal file
200
src/__tests__/run-cjs-graceful-fallback.test.ts
Normal file
@@ -0,0 +1,200 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { mkdtempSync, mkdirSync, writeFileSync, rmSync, existsSync, symlinkSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
import { execFileSync } from 'child_process';
|
||||
|
||||
const RUN_CJS_PATH = join(__dirname, '..', '..', 'scripts', 'run.cjs');
|
||||
const NODE = process.execPath;
|
||||
|
||||
/**
|
||||
* Regression tests for run.cjs graceful fallback when CLAUDE_PLUGIN_ROOT
|
||||
* points to a stale/deleted/broken plugin cache directory.
|
||||
*
|
||||
* See: https://github.com/Yeachan-Heo/oh-my-claudecode/issues/1007
|
||||
*/
|
||||
describe('run.cjs — graceful fallback for stale plugin paths', () => {
|
||||
let tmpDir: string;
|
||||
let fakeCacheBase: string;
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDir = mkdtempSync(join(tmpdir(), 'omc-run-cjs-test-'));
|
||||
fakeCacheBase = join(tmpDir, 'plugins', 'cache', 'omc', 'oh-my-claudecode');
|
||||
mkdirSync(fakeCacheBase, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
rmSync(tmpDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function createFakeVersion(version: string, scripts: Record<string, string> = {}) {
|
||||
const versionDir = join(fakeCacheBase, version);
|
||||
const scriptsDir = join(versionDir, 'scripts');
|
||||
mkdirSync(scriptsDir, { recursive: true });
|
||||
for (const [name, content] of Object.entries(scripts)) {
|
||||
writeFileSync(join(scriptsDir, name), content);
|
||||
}
|
||||
return versionDir;
|
||||
}
|
||||
|
||||
function runCjs(target: string, env: Record<string, string> = {}): { status: number; stdout: string; stderr: string } {
|
||||
try {
|
||||
const stdout = execFileSync(NODE, [RUN_CJS_PATH, target], {
|
||||
encoding: 'utf-8',
|
||||
env: {
|
||||
...process.env,
|
||||
...env,
|
||||
},
|
||||
timeout: 10000,
|
||||
input: '{}',
|
||||
});
|
||||
return { status: 0, stdout: stdout || '', stderr: '' };
|
||||
} catch (err: any) {
|
||||
return {
|
||||
status: err.status ?? 1,
|
||||
stdout: err.stdout || '',
|
||||
stderr: err.stderr || '',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
it('exits 0 when no target argument is provided', () => {
|
||||
try {
|
||||
execFileSync(NODE, [RUN_CJS_PATH], {
|
||||
encoding: 'utf-8',
|
||||
timeout: 5000,
|
||||
});
|
||||
// If it exits 0, this succeeds
|
||||
} catch (err: any) {
|
||||
// Should not throw — exit 0 expected
|
||||
expect(err.status).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('exits 0 when target script does not exist (stale CLAUDE_PLUGIN_ROOT)', () => {
|
||||
const staleVersion = join(fakeCacheBase, '4.2.14');
|
||||
const staleTarget = join(staleVersion, 'scripts', 'persistent-mode.cjs');
|
||||
|
||||
// Do NOT create the version directory — simulates deleted cache
|
||||
const result = runCjs(staleTarget, {
|
||||
CLAUDE_PLUGIN_ROOT: staleVersion,
|
||||
});
|
||||
|
||||
// Must exit 0, not propagate MODULE_NOT_FOUND
|
||||
expect(result.status).toBe(0);
|
||||
});
|
||||
|
||||
it('falls back to latest version when target version is missing', () => {
|
||||
// Create a valid latest version with the target script
|
||||
const latestDir = createFakeVersion('4.4.5', {
|
||||
'test-hook.cjs': '#!/usr/bin/env node\nconsole.log("hook-ok"); process.exit(0);',
|
||||
});
|
||||
|
||||
// Target points to a non-existent old version
|
||||
const staleVersion = join(fakeCacheBase, '4.2.14');
|
||||
const staleTarget = join(staleVersion, 'scripts', 'test-hook.cjs');
|
||||
|
||||
const result = runCjs(staleTarget, {
|
||||
CLAUDE_PLUGIN_ROOT: staleVersion,
|
||||
});
|
||||
|
||||
// Should find the script in 4.4.5 and run it successfully
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stdout).toContain('hook-ok');
|
||||
});
|
||||
|
||||
it('falls back to latest version when multiple versions exist', () => {
|
||||
// Create two valid versions
|
||||
createFakeVersion('4.4.3', {
|
||||
'test-hook.cjs': '#!/usr/bin/env node\nconsole.log("from-4.4.3"); process.exit(0);',
|
||||
});
|
||||
createFakeVersion('4.4.5', {
|
||||
'test-hook.cjs': '#!/usr/bin/env node\nconsole.log("from-4.4.5"); process.exit(0);',
|
||||
});
|
||||
|
||||
// Target points to a deleted old version
|
||||
const staleVersion = join(fakeCacheBase, '4.2.14');
|
||||
const staleTarget = join(staleVersion, 'scripts', 'test-hook.cjs');
|
||||
|
||||
const result = runCjs(staleTarget, {
|
||||
CLAUDE_PLUGIN_ROOT: staleVersion,
|
||||
});
|
||||
|
||||
// Should pick the highest version (4.4.5)
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stdout).toContain('from-4.4.5');
|
||||
});
|
||||
|
||||
it('resolves target through symlinked version directory', () => {
|
||||
// Create a real latest version
|
||||
const latestDir = createFakeVersion('4.4.5', {
|
||||
'test-hook.cjs': '#!/usr/bin/env node\nconsole.log("via-symlink"); process.exit(0);',
|
||||
});
|
||||
|
||||
// Create a symlink from old version to latest
|
||||
const symlinkVersion = join(fakeCacheBase, '4.4.3');
|
||||
symlinkSync('4.4.5', symlinkVersion);
|
||||
|
||||
// Target uses the symlinked version
|
||||
const target = join(symlinkVersion, 'scripts', 'test-hook.cjs');
|
||||
|
||||
const result = runCjs(target, {
|
||||
CLAUDE_PLUGIN_ROOT: symlinkVersion,
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stdout).toContain('via-symlink');
|
||||
});
|
||||
|
||||
it('runs target normally when path is valid (fast path)', () => {
|
||||
const versionDir = createFakeVersion('4.4.5', {
|
||||
'test-hook.cjs': '#!/usr/bin/env node\nconsole.log("direct-ok"); process.exit(0);',
|
||||
});
|
||||
|
||||
const target = join(versionDir, 'scripts', 'test-hook.cjs');
|
||||
|
||||
const result = runCjs(target, {
|
||||
CLAUDE_PLUGIN_ROOT: versionDir,
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
expect(result.stdout).toContain('direct-ok');
|
||||
});
|
||||
|
||||
it('exits 0 when no CLAUDE_PLUGIN_ROOT is set and target is missing', () => {
|
||||
const result = runCjs('/nonexistent/path/to/hook.mjs', {
|
||||
CLAUDE_PLUGIN_ROOT: '',
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
});
|
||||
|
||||
it('exits 0 when cache base has no valid version directories', () => {
|
||||
const staleVersion = join(fakeCacheBase, '4.2.14');
|
||||
const staleTarget = join(staleVersion, 'scripts', 'test-hook.cjs');
|
||||
|
||||
// Cache base exists but has no version directories
|
||||
const result = runCjs(staleTarget, {
|
||||
CLAUDE_PLUGIN_ROOT: staleVersion,
|
||||
});
|
||||
|
||||
expect(result.status).toBe(0);
|
||||
});
|
||||
|
||||
it('exits 0 when fallback versions exist but lack the specific script', () => {
|
||||
// Create a version that does NOT have the target script
|
||||
createFakeVersion('4.4.5', {
|
||||
'other-hook.cjs': '#!/usr/bin/env node\nprocess.exit(0);',
|
||||
});
|
||||
|
||||
const staleVersion = join(fakeCacheBase, '4.2.14');
|
||||
const staleTarget = join(staleVersion, 'scripts', 'test-hook.cjs');
|
||||
|
||||
const result = runCjs(staleTarget, {
|
||||
CLAUDE_PLUGIN_ROOT: staleVersion,
|
||||
});
|
||||
|
||||
// No version has test-hook.cjs, so exit 0 gracefully
|
||||
expect(result.status).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user