mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-20 01:56:37 +03:00
Enforce the engines floor in the probe instead of a capability check
The probe asked whether node could load ESM, while the notice promised a Node 22 floor and package.json engines declares >=22.12.0. Reviewers kept flagging the gap, and they were right to: a 14.18-to-21 runtime passed the probe on the strength of one import while the hook and its detector bundle are only ever exercised on the engines floor, so "can load our code" was a weaker claim than the one being made for it. Check the floor directly: parseInt(process.versions.node) >= 22, in ES5-only syntax that parses on any node old enough to fail it. Probe and notice now derive from one NODE_MAJOR_FLOOR constant, so they cannot disagree, and the archaeology about node: scheme support and pre-15 unhandled-rejection semantics goes with the import it explained. Add the missing contract test: every generated hook command carries the probe, the notice appears exactly where a harness can render it (Claude and Codex, project and plugin), and the expected floor is read from package.json engines rather than repeated by hand. Verified against a fake pre-22 node, no node, and a real node: one notice then the marker holds it silent, exit 0 in every failure shape, and the hook's own exit code still passes through on a supported runtime. Co-Authored-By: Claude Fable 5 (via Cursor) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
co-authored by
Claude Fable 5
Cursor
parent
86cdf528c5
commit
fd9076f4f0
@@ -13,6 +13,7 @@ import {
|
||||
buildClaudeSettingsManifest,
|
||||
buildClaudePluginHooksManifest,
|
||||
buildCodexHooksManifest,
|
||||
buildCodexPluginHooksManifest,
|
||||
buildCursorHooksManifest,
|
||||
buildGitHubHooksManifest,
|
||||
buildGrokHooksManifest,
|
||||
@@ -25,14 +26,25 @@ function readJson(rel) {
|
||||
return JSON.parse(fs.readFileSync(path.join(REPO_ROOT, rel), 'utf8'));
|
||||
}
|
||||
|
||||
// The runtime probe every hook command must carry (issue #410): a node below
|
||||
// the engines floor exits the command at 0 instead of dying on ESM parse. The
|
||||
// expected floor comes from package.json engines, so probe and contract cannot
|
||||
// drift apart.
|
||||
const ENGINES_NODE_MAJOR = parseInt(
|
||||
JSON.parse(fs.readFileSync(path.join(REPO_ROOT, 'package.json'), 'utf8')).engines.node.replace(/[^\d.]/g, ''),
|
||||
10,
|
||||
);
|
||||
const NODE_PROBE = `process.exit(parseInt(process.versions.node,10)>=${ENGINES_NODE_MAJOR}?0:1)`;
|
||||
|
||||
function expectCommand(command, expectedPath) {
|
||||
assert.equal(typeof command, 'string');
|
||||
// node-command providers carry the missing-file guard (issue #399: exits 0
|
||||
// when absent, preserves node's exit code when present) plus the runtime probe
|
||||
// (issue #410: exits 0 when node cannot load ESM). GitHub's portable
|
||||
// `$(git rev-parse)` form is guarded too, so it lands in the same branch.
|
||||
// when absent, preserves node's exit code when present) plus the runtime
|
||||
// probe. GitHub's portable `$(git rev-parse)` form is guarded too, so it
|
||||
// lands in the same branch.
|
||||
if (command.startsWith('[ ! -f "')) {
|
||||
assert.match(command, /\|\| node "/);
|
||||
assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`);
|
||||
} else {
|
||||
assert.match(command, /^node "|^bash -c|\$\(git rev-parse/);
|
||||
}
|
||||
@@ -40,6 +52,20 @@ function expectCommand(command, expectedPath) {
|
||||
assert.ok(!command.includes('hook-probe.mjs'), `probe hook still referenced in ${command}`);
|
||||
}
|
||||
|
||||
function manifestCommands(manifest) {
|
||||
const commands = [];
|
||||
const walk = (value) => {
|
||||
if (Array.isArray(value)) { value.forEach(walk); return; }
|
||||
if (value && typeof value === 'object') {
|
||||
if (typeof value.command === 'string') commands.push(value.command);
|
||||
if (typeof value.bash === 'string') commands.push(value.bash);
|
||||
Object.values(value).forEach(walk);
|
||||
}
|
||||
};
|
||||
walk(manifest.hooks);
|
||||
return commands;
|
||||
}
|
||||
|
||||
describe('hook manifest builders', () => {
|
||||
it('builds Claude project settings for the real detector hook', () => {
|
||||
const manifest = buildClaudeSettingsManifest();
|
||||
@@ -166,6 +192,40 @@ describe('hook manifest builders', () => {
|
||||
expectCommand(stop.command, '.grok/skills/impeccable/scripts/hook.mjs');
|
||||
});
|
||||
|
||||
it('probes the node runtime everywhere, and notices only where a channel exists', () => {
|
||||
// Claude Code and Codex render a `systemMessage` from hook stdout, so their
|
||||
// manifests carry the one-time unsupported-runtime notice. Cursor (output is
|
||||
// permission-shaped; a message would block the edit), Grok (stdout ignored),
|
||||
// and Copilot (contract unconfirmed) get the silent probe only.
|
||||
const withNotice = [
|
||||
buildClaudeSettingsManifest(),
|
||||
buildClaudePluginHooksManifest(),
|
||||
buildCodexHooksManifest(),
|
||||
buildCodexPluginHooksManifest(),
|
||||
];
|
||||
const probeOnly = [
|
||||
buildCursorHooksManifest(),
|
||||
buildGitHubHooksManifest(),
|
||||
buildGrokHooksManifest(),
|
||||
];
|
||||
for (const manifest of [...withNotice, ...probeOnly]) {
|
||||
for (const command of manifestCommands(manifest)) {
|
||||
assert.ok(command.includes(NODE_PROBE), `missing runtime probe in ${command}`);
|
||||
}
|
||||
}
|
||||
for (const manifest of withNotice) {
|
||||
for (const command of manifestCommands(manifest)) {
|
||||
assert.ok(command.includes('systemMessage'), `missing notice in ${command}`);
|
||||
assert.ok(command.includes('node-unsupported'), `missing once-only marker in ${command}`);
|
||||
}
|
||||
}
|
||||
for (const manifest of probeOnly) {
|
||||
for (const command of manifestCommands(manifest)) {
|
||||
assert.ok(!command.includes('systemMessage'), `unexpected notice in ${command}`);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it('routes supported hook builders and leaves other providers alone', () => {
|
||||
assert.ok(hooksJsonFor('claude'));
|
||||
assert.ok(hooksJsonFor('codex'));
|
||||
|
||||
Reference in New Issue
Block a user