Skip to content

Commit 738892d

Browse files
committed
Fix interview.mjs CLI deadlocking on its own import cycle
`interview.mjs check` exited 13 ("unsettled top-level await") in every repo, canon included, so the pending-adoption note could never print anywhere. Refs #580. The cycle closes only when this module is the entry point: its top-level `await check(...)` holds evaluation open, check() calls loadPacks(), discovery dynamically imports every pack's skills/<skill>/checks.mjs, and adopt-claudinite/checks.mjs imports interviewState straight back from the module still being evaluated. That import can never resolve, so the await never settles. Not gated on the pack being declared — scanSkillChecks runs at discovery, before activation is consulted. Starting the work after evaluation completes leaves the same cycle harmless: the re-import resolves against a finished module. Kept fail-soft, since the orchestrator treats a failed step as a missing note rather than a crash. The bug shipped because every existing test imports the pure helpers directly, and the cycle only closes in the CLI. The regression tests spawn it as a child process and assert the note reaches stdout, not merely that the exit code is 0 — a process exiting clean having printed nothing is this bug's own signature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RVkfwBVJWV7tdk25KjNpdL
1 parent e80b672 commit 738892d

2 files changed

Lines changed: 60 additions & 1 deletion

File tree

packs-tests/grow_with_claudinite/skills/adopt-claudinite/interview.test.mjs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import { test } from 'node:test';
22
import assert from 'node:assert/strict';
3+
import { spawnSync } from 'node:child_process';
4+
import { fileURLToPath } from 'node:url';
35
import { makeRepo, cleanup } from '../../../../engine-tests/helpers.mjs';
46
import { loadConfig } from '../../../../engine/checks/helpers/repo-context.mjs';
57
import { loadPacks } from '../../../../engine/pack_loader/pack-registry.mjs';
68
import { packQuestions, interviewState, renderPending } from '../../../../packs/grow_with_claudinite/skills/adopt-claudinite/interview.mjs';
79

10+
const CLI = fileURLToPath(new URL('../../../../packs/grow_with_claudinite/skills/adopt-claudinite/interview.mjs', import.meta.url));
11+
812
const pack = (over = {}) => ({
913
id: 'p',
1014
questions: [{ id: 'q1', prompt: 'P1?' }, { id: 'q2', prompt: 'P2?', distill: 'D2' }],
@@ -98,3 +102,44 @@ test('integration: declaring barriers pends its goals question until the entry a
98102
assert.deepEqual(interviewState(packs, loadConfig(answered)).pending, []);
99103
} finally { cleanup(unanswered); cleanup(answered); }
100104
});
105+
106+
// The CLI, run as a CHILD PROCESS — the only context where this module's import
107+
// cycle closes, and so the only shape that can catch it. Every test above
108+
// imports the pure helpers directly, which is exactly why the cycle shipped: it
109+
// deadlocked `interview.mjs check` in every repo (Node exits 13, "unsettled
110+
// top-level await") while the pure-function coverage stayed green. Asserting the
111+
// note reaches stdout, not merely that the exit code is 0, is deliberate — a
112+
// process that exits clean having printed nothing is the bug's own signature.
113+
test('CLI: `check` prints the pending note and exits clean (its import cycle must not deadlock)', () => {
114+
const repo = makeRepo({ base: { '.claudinite-checks.json': JSON.stringify({ packs: ['barriers'] }) } });
115+
try {
116+
const r = spawnSync(process.execPath, [CLI, 'check'], {
117+
encoding: 'utf8',
118+
env: { ...process.env, CLAUDE_PROJECT_DIR: repo },
119+
});
120+
assert.equal(r.status, 0, `exited ${r.status}\nstderr: ${r.stderr}`);
121+
assert.doesNotMatch(r.stderr, /unsettled top-level await/, 'the CLI deadlocked on its own import cycle');
122+
assert.match(r.stdout, /Pack adoption interview pending/);
123+
assert.match(r.stdout, /- barriers \/ goals:/);
124+
} finally { cleanup(repo); }
125+
});
126+
127+
test('CLI: a repo with nothing pending prints nothing and exits clean', () => {
128+
const repo = makeRepo({ base: { '.claudinite-checks.json': JSON.stringify({
129+
packs: [{ id: 'barriers', answers: { goals: 'n/a — none wanted' } }],
130+
}) } });
131+
try {
132+
const r = spawnSync(process.execPath, [CLI, 'check'], {
133+
encoding: 'utf8',
134+
env: { ...process.env, CLAUDE_PROJECT_DIR: repo },
135+
});
136+
assert.equal(r.status, 0, `exited ${r.status}\nstderr: ${r.stderr}`);
137+
assert.equal(r.stdout, '');
138+
} finally { cleanup(repo); }
139+
});
140+
141+
test('CLI: an unknown subcommand still reports usage and exits 2', () => {
142+
const r = spawnSync(process.execPath, [CLI, 'nope'], { encoding: 'utf8' });
143+
assert.equal(r.status, 2);
144+
assert.match(r.stderr, /usage: interview\.mjs check/);
145+
});

packs/grow_with_claudinite/skills/adopt-claudinite/interview.mjs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,25 @@ async function check(projectRoot) {
107107
}
108108

109109
// CLI — but importable (the tests and the runner import the pure helpers above).
110+
//
111+
// NEVER `await check(...)` here. This module sits in a cycle that closes only
112+
// when it is the ENTRY point: check() calls loadPacks(), whose discovery
113+
// dynamically imports every pack's skills/<skill>/checks.mjs, and both
114+
// adopt-claudinite/checks.mjs and adopt-pack/checks.mjs import `interviewState`
115+
// from this file. A top-level await holds this module's evaluation open, so that
116+
// re-import can never resolve, the await never settles, and Node exits 13
117+
// without running the check at all — silently, since the orchestrator treats a
118+
// failed step as fail-soft. Starting the work AFTER evaluation completes leaves
119+
// the same cycle harmless: the re-import resolves against a finished module.
110120
if (import.meta.url === pathToFileURL(process.argv[1] || '').href) {
111121
const cmd = process.argv[2];
112122
const projectRoot = process.env.CLAUDE_PROJECT_DIR || process.cwd();
113123
if (cmd === 'check') {
114-
await check(projectRoot); // fails soft upstream — the orchestrator tolerates a step failure
124+
// Fails soft upstream — the orchestrator tolerates a step failure, so a
125+
// broken check must degrade to a missing note, never to a crash.
126+
check(projectRoot).catch((e) => {
127+
process.stderr.write(`interview check failed: ${e.message}\n`);
128+
});
115129
} else {
116130
process.stderr.write('usage: interview.mjs check\n');
117131
process.exit(2);

0 commit comments

Comments
 (0)