Skip to content

Commit 9a14756

Browse files
committed
Restore CI coverage for hook argv expansion and preflight blocking
The hook escaping regression came from asserting a literal `node` prefix even though installed hooks may be patched to an absolute node binary. The test now captures argv after shell expansion instead of accidentally invoking the real runner, and the pre-tool enforcer now sanitizes malformed threshold env input so agent-heavy exhausted-context preflight keeps blocking deterministically. The remaining CI flakes came from host config leakage in the pre-tool harness and a tmux-availability assumption in the duplicate-worker dispatch test, so those assertions are now hermetic and environment-agnostic. Constraint: Installed hooks may start with an absolute quoted node path after plugin setup Constraint: Pre-tool tests must not inherit CLAUDE config from the host, CI runner, or repo-root cwd Constraint: Duplicate-worker dispatch coverage must not require tmux to be available Rejected: Keep assuming a literal `node` prefix in hook tests | no longer matches shipped hook shape Rejected: Leave malformed threshold env unchecked | comparison against NaN silently disables blocking Rejected: Assert immediate `notified` dispatch state in the duplicate-pane test | depends on tmux availability outside the test contract Confidence: high Scope-risk: narrow Directive: Keep hook argv assertions focused on post-shell tokenization, not a specific node binary spelling Directive: Keep shell-spawned hook tests pinned to isolated HOME/CLAUDE_CONFIG_DIR values and an isolated child cwd so host routing config cannot leak in Tested: npm test -- --run src/__tests__/pre-tool-enforcer.test.ts src/team/__tests__/api-interop.dispatch.test.ts src/__tests__/hooks-command-escaping.test.ts Tested: npm test -- --run Tested: npm run build Tested: npm run lint Not-tested: PR CI rerun on GitHub after push
1 parent 85b12bd commit 9a14756

File tree

4 files changed

+61
-20
lines changed

4 files changed

+61
-20
lines changed

scripts/pre-tool-enforcer.mjs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,16 @@ const MODE_STATE_FILES = [
9191
'omc-teams-state.json',
9292
];
9393
const AGENT_HEAVY_TOOLS = new Set(['Task', 'TaskCreate', 'TaskUpdate']);
94-
const PREFLIGHT_CONTEXT_THRESHOLD = parseInt(process.env.OMC_AGENT_PREFLIGHT_CONTEXT_THRESHOLD || '72', 10);
9594
const QUIET_LEVEL = getQuietLevel();
9695

96+
function getPreflightContextThreshold() {
97+
const parsed = Number.parseInt(process.env.OMC_AGENT_PREFLIGHT_CONTEXT_THRESHOLD || '72', 10);
98+
if (Number.isNaN(parsed)) return 72;
99+
return Math.max(1, Math.min(100, parsed));
100+
}
101+
102+
const PREFLIGHT_CONTEXT_THRESHOLD = getPreflightContextThreshold();
103+
97104
function getQuietLevel() {
98105
const parsed = Number.parseInt(process.env.OMC_QUIET || '0', 10);
99106
if (Number.isNaN(parsed)) return 0;

src/__tests__/hooks-command-escaping.test.ts

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,23 @@ interface HooksConfig {
99

1010
const hooksJsonPath = join(__dirname, '..', '..', 'hooks', 'hooks.json');
1111

12+
function expandHookCommandArgv(command: string, pluginRoot: string): string[] {
13+
const shellScript =
14+
`eval "set -- $HOOK_COMMAND"; ` +
15+
`node -e 'console.log(JSON.stringify(process.argv.slice(1)))' -- "$@"`;
16+
17+
return JSON.parse(
18+
execFileSync('bash', ['-lc', shellScript], {
19+
encoding: 'utf-8',
20+
env: {
21+
...process.env,
22+
HOOK_COMMAND: command,
23+
CLAUDE_PLUGIN_ROOT: pluginRoot,
24+
},
25+
}).trim()
26+
) as string[];
27+
}
28+
1229
function getHookCommands(): string[] {
1330
const raw = JSON.parse(readFileSync(hooksJsonPath, 'utf-8')) as HooksConfig;
1431
return Object.values(raw.hooks ?? {})
@@ -31,24 +48,13 @@ describe('hooks.json command escaping', () => {
3148
const pluginRoot = '/c/Users/First Last/.claude/plugins/cache/omc/oh-my-claudecode/4.7.10';
3249

3350
for (const command of getHookCommands()) {
34-
const argv = JSON.parse(
35-
execFileSync(
36-
'bash',
37-
['-lc', command.replace(/^node\b/, `node -e "console.log(JSON.stringify(process.argv.slice(1)))"`)],
38-
{
39-
encoding: 'utf-8',
40-
env: {
41-
...process.env,
42-
CLAUDE_PLUGIN_ROOT: pluginRoot,
43-
},
44-
}
45-
).trim()
46-
) as string[];
47-
48-
expect(argv[0]).toBe(`${pluginRoot}/scripts/run.cjs`);
49-
expect(argv[1]).toContain(`${pluginRoot}/scripts/`);
50-
expect(argv[0]).toContain('First Last');
51+
const argv = expandHookCommandArgv(command, pluginRoot);
52+
53+
expect(argv[0]).toMatch(/(^|[/\\])node(?:\.exe)?$/);
54+
expect(argv[1]).toBe(`${pluginRoot}/scripts/run.cjs`);
55+
expect(argv[2]).toContain(`${pluginRoot}/scripts/`);
5156
expect(argv[1]).toContain('First Last');
57+
expect(argv[2]).toContain('First Last');
5258
expect(argv).not.toContain('/c/Users/First');
5359
expect(argv).not.toContain('Last/.claude/plugins/cache/omc/oh-my-claudecode/4.7.10/scripts/run.cjs');
5460
}

src/__tests__/pre-tool-enforcer.test.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,21 @@ function runPreToolEnforcerWithEnv(
1414
input: Record<string, unknown>,
1515
env: Record<string, string> = {},
1616
): Record<string, unknown> {
17+
const cwd = typeof input.cwd === 'string' ? input.cwd : process.cwd();
18+
const homeDir = join(cwd, '.test-home');
1719
const stdout = execSync(`node "${SCRIPT_PATH}"`, {
20+
cwd,
1821
input: JSON.stringify(input),
1922
encoding: 'utf-8',
2023
timeout: 5000,
2124
env: {
2225
...process.env,
26+
HOME: homeDir,
27+
CLAUDE_CONFIG_DIR: join(homeDir, '.claude'),
2328
NODE_ENV: 'test',
2429
// Reset Bedrock/routing env vars so tests are isolated from the host environment.
2530
// Tests that exercise Bedrock model-routing behaviour set these explicitly via `env`.
31+
OMC_AGENT_PREFLIGHT_CONTEXT_THRESHOLD: '',
2632
OMC_ROUTING_FORCE_INHERIT: '',
2733
OMC_SUBAGENT_MODEL: '',
2834
CLAUDE_MODEL: '',
@@ -377,6 +383,28 @@ describe('pre-tool-enforcer fallback gating (issue #970)', () => {
377383
expect(String(output.reason)).toContain('Safe recovery');
378384
});
379385

386+
it('falls back to the default preflight threshold when the env value is invalid', () => {
387+
const transcriptPath = join(tempDir, 'transcript.jsonl');
388+
writeTranscriptWithContext(transcriptPath, 1000, 800); // 80%
389+
390+
const output = runPreToolEnforcerWithEnv(
391+
{
392+
tool_name: 'Task',
393+
toolInput: {
394+
subagent_type: 'oh-my-claudecode:executor',
395+
description: 'High fan-out execution',
396+
},
397+
cwd: tempDir,
398+
transcript_path: transcriptPath,
399+
session_id: 'session-1373-invalid-threshold',
400+
},
401+
{ OMC_AGENT_PREFLIGHT_CONTEXT_THRESHOLD: 'abc' },
402+
);
403+
404+
expect(output.decision).toBe('block');
405+
expect(String(output.reason)).toContain('threshold: 72%');
406+
});
407+
380408
it('allows non-agent-heavy tools even when transcript context is high', () => {
381409
const transcriptPath = join(tempDir, 'transcript.jsonl');
382410
writeTranscriptWithContext(transcriptPath, 1000, 900); // 90%
@@ -392,7 +420,6 @@ describe('pre-tool-enforcer fallback gating (issue #970)', () => {
392420
expect(output.decision).toBeUndefined();
393421
});
394422

395-
396423
it('clears awaiting confirmation from session-scoped mode state when a skill is invoked', () => {
397424
const sessionId = 'session-confirm';
398425
const sessionStateDir = join(tempDir, '.omc', 'state', 'sessions', sessionId);

src/team/__tests__/api-interop.dispatch.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ describe('team api dispatch-aware messaging', () => {
206206
const requests = await listDispatchRequests(teamName, cwd, { kind: 'mailbox', to_worker: 'worker-1' });
207207
expect(requests).toHaveLength(1);
208208
expect(requests[0]?.message_id).toBe(messageId);
209-
expect(requests[0]?.status).toBe('pending');
209+
expect(requests[0]?.pane_id).toBe('%9');
210+
expect(['pending', 'notified']).toContain(requests[0]?.status);
210211
});
211212
});

0 commit comments

Comments
 (0)