Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 170 additions & 0 deletions cli/__tests__/stray-gate-calls.test.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import { describe, expect, it } from 'vitest';
import { strayGateCalls } from '../lib/doctor/stray-gate-calls.mts';

// A repo that hand-rolled its gates before devkit absorbed them keeps the old lines below the
// managed block, so every commit runs each gate twice — two model bills for the LLM judges, while
// .devkit/config.json still describes one run. This finds those, and (just as importantly) does not
// cry wolf about lines that only look like calls.

const hook = (
below: string,
inBlock = 'bunx guard-decisions detect --gate\nbunx guard-review --gate',
) =>
['#!/bin/bash', '# >>> devkit-guards >>>', inBlock, '# <<< devkit-guards <<<', below].join('\n');

describe('strayGateCalls', () => {
it('reports a gate invoked outside the block that the block also runs', () => {
const found = strayGateCalls(hook('guard-review --gate || rrc=$?'));
expect(found).toHaveLength(1);
// Signature is bin + SUBCOMMAND; `--gate` is a flag, so it stops at the bin.
expect(found[0].bin).toBe('guard-review');
expect(found[0].line).toBe(6); // shebang, open marker, 2 block lines, close marker, then this
});

it('ignores calls INSIDE the managed block — that is where they belong', () => {
expect(strayGateCalls(hook('echo done'))).toHaveLength(0);
});

it('ignores a subcommand the block does not run', () => {
// devkit emits `guard-decisions detect`; it ships no fragment for check-alignment, so this is
// the consumer's ONLY invocation. Flagging it would advise deleting a live gate.
expect(strayGateCalls(hook('guard-decisions check-alignment --gate'))).toHaveLength(0);
});

it('still catches the duplicated subcommand alongside a non-duplicated one', () => {
const found = strayGateCalls(
hook('guard-decisions check-alignment --gate\nguard-decisions detect --gate'),
);
expect(found.map((f) => f.bin)).toEqual(['guard-decisions detect']);
});

it('ignores bins named inside echo/printf remedy strings', () => {
// A remedy line naturally names the very bin it tells you to run.
const below = 'echo " run: guard-review --gate to retry"\nprintf "guard-decisions detect\\n"';
expect(strayGateCalls(hook(below))).toHaveLength(0);
});

it('ignores comments', () => {
expect(strayGateCalls(hook('# guard-review --gate runs in the block above'))).toHaveLength(0);
});

it('returns nothing when the hook has no managed block (nothing to duplicate)', () => {
expect(strayGateCalls('#!/bin/bash\nguard-review --gate\n')).toHaveLength(0);
});

// A monorepo hook holds one block per package. A sibling's block is devkit-written and valid, so
// reporting it would tell the consumer to delete devkit's own output.
it('does not report a SIBLING package block in a monorepo hook', () => {
const monorepo = [
'#!/bin/bash',
'# >>> devkit-guards: services/api >>>',
'bunx guard-deterministic --hook "$0" || exit 1',
'# <<< devkit-guards: services/api <<<',
'# >>> devkit-guards: services/web >>>',
'bunx guard-deterministic --hook "$0" || exit 1',
'# <<< devkit-guards: services/web <<<',
].join('\n');
expect(strayGateCalls(monorepo, 'services/web')).toHaveLength(0);
});

// guard-deterministic is an orchestrator: the block never names the gates it runs, so matching on
// literal block text alone could never flag a stray copy of one of them.
it('reports a stray sub-gate that guard-deterministic already orchestrates', () => {
const found = strayGateCalls(
hook('guard-dup scan --new --changed --gate', 'bunx guard-deterministic --hook "$0"'),
);
expect(found.map((f) => f.bin)).toEqual(['guard-dup scan']);
});

it('reports EVERY gate on a chained line, not just the first', () => {
const found = strayGateCalls(
hook(
'guard-dup scan ; guard-review --gate',
'bunx guard-deterministic --hook "$0"\nbunx guard-review --gate',
),
);
expect(found.map((f) => f.bin)).toEqual(['guard-dup scan', 'guard-review']);
});

// A guarded call spans two lines: `if command -v X; then` / ` X --check`. Only the second runs
// the gate — counting the probe double-reports and points at a line that invokes nothing.
it('ignores a `command -v` existence probe, but still reports the guarded call', () => {
const below = [
'if command -v guard-dup >/dev/null 2>&1; then',
' guard-dup scan --gate || true',
'fi',
].join('\n');
const found = strayGateCalls(hook(below, 'bunx guard-deterministic --hook "$0"'));
expect(found).toHaveLength(1);
expect(found[0].line).toBe(6); // the invocation, not the probe on line 5
});

it('catches the real call when a probe and the invocation share ONE line', () => {
// Same bin twice on one line: tracking only its first occurrence filters the whole bin away as
// "just a probe" and misses the duplicate.
const found = strayGateCalls(
hook('command -v guard-dup && guard-dup scan --gate', 'bunx guard-deterministic --hook "$0"'),
);
expect(found.map((f) => f.bin)).toEqual(['guard-dup scan']);
});

it('does not read guard-dup-allowlist as a guard-dup call', () => {
const found = strayGateCalls(
hook('guard-dup-allowlist add a b c d', 'bunx guard-deterministic --hook "$0"'),
);
expect(found).toHaveLength(0);
});

it('ignores a bin named in a NON-leading echo, or an inline comment', () => {
// A bin name is far more often mentioned than run — in a remedy message or a trailing note.
const below = [
'[ -n "$V" ] && echo "next up: guard-review --gate"',
'true # guard-review runs in the block above',
].join('\n');
expect(strayGateCalls(hook(below))).toHaveLength(0);
});

it('ignores a DIFFERENT subcommand of a bin the block runs bare', () => {
// The block runs `guard-review --gate`; `guard-review transcript` is a different command, not a
// second run of the gate. Only the gates guard-deterministic orchestrates get a bare-bin match.
expect(strayGateCalls(hook('guard-review transcript'))).toHaveLength(0);
expect(strayGateCalls(hook('guard-review clear-cache'))).toHaveLength(0);
});

it('ignores which/type/hash probes too', () => {
const below = 'which guard-review\ntype guard-review\nhash guard-review';
expect(strayGateCalls(hook(below))).toHaveLength(0);
});

// Self-host rewrites `bunx guard-review` to `node gate-engine/review/cli.mts` before writing the
// hook, so matching bin NAMES alone leaves blockSignatures empty and the check silently does
// nothing — in the very repo that dogfoods it. The alias comes from the repo's own bin map.
it('matches a self-hosted block (node gate-engine/...) against a bin-name stray', () => {
const selfHosted = [
'#!/bin/bash',
'# >>> devkit-guards >>>',
'node gate-engine/review/cli.mts --gate',
'# <<< devkit-guards <<<',
'guard-review --gate || rrc=$?',
].join('\n');
// cwd = the devkit repo, whose package.json bin map supplies the alias.
const found = strayGateCalls(selfHosted, '', process.cwd());
expect(found.map((f) => f.bin)).toEqual(['guard-review']);
});

it('still reports a genuine stray in a monorepo hook', () => {
const monorepo = [
'#!/bin/bash',
'# >>> devkit-guards: services/api >>>',
'bunx guard-deterministic --hook "$0" || exit 1',
'# <<< devkit-guards: services/api <<<',
'# >>> devkit-guards: services/web >>>',
'bunx guard-deterministic --hook "$0" || exit 1',
'# <<< devkit-guards: services/web <<<',
'guard-deterministic --hook "$0" || exit 1', // hand-written, outside every block
].join('\n');
const found = strayGateCalls(monorepo, 'services/web');
expect(found).toHaveLength(1);
expect(found[0].line).toBe(8);
});
});
64 changes: 6 additions & 58 deletions cli/commands/doctor.mts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,10 @@ import {
} from '../lib/doctor/asset-checks.mts';
import { type CheckResult, check } from '../lib/doctor/check-result.mts';
import { checkHookRunner, checkHusky } from '../lib/doctor/hook-checks.mts';
import { runSelfHostDoctor } from '../lib/doctor/self-host-doctor.mts';
import { packageDir, readJson } from '../lib/fs-helpers.mts';
import { checkCommitMsgHook, commitMsgGuards } from '../lib/husky/commit-msg-block.mts';
import { extractGuardBlock, QAVIS_ADVISORY_ID } from '../lib/husky/husky-block.mts';
import {
buildSelfHostBlock,
installSelfHostHook,
SELF_HOST_EXTRAS,
SELF_HOST_STRUCTURE_CMD,
selfHostSelection,
} from '../lib/husky/self-host.mts';
import { checkHookRegistrations } from '../lib/install/install-hooks.mts';
import { HEAL_ALIAS_NAME, isHealAlias, syncOverlayHook } from '../lib/overlay.mts';
import { globalHookInstalled, globalInitPath } from '../lib/overlay-global-hook.mts';
Expand Down Expand Up @@ -367,8 +361,11 @@ function applyFix(
// The guard blocks (pre-commit + commit-msg) AND the structure-lint `--structure` arg are all
// rebuilt by init from the recorded selection — so a drifted result on any of them takes the
// same init repair path (each flags itself fixable, else --fix would no-op it).
// `r.fixable` is part of the condition, not just the name: a hook check can now report a problem
// init CANNOT repair (a hand-written gate call OUTSIDE the managed block — regenerating the block
// leaves it untouched). Without this, --fix re-inits on every run and the warning never clears.
const HOOK_CHECKS = new Set(['.husky/pre-commit', '.husky/commit-msg', 'structure-lint']);
const huskyDrift = results.some((r) => HOOK_CHECKS.has(r.name) && r.status !== 'OK');
const huskyDrift = results.some((r) => HOOK_CHECKS.has(r.name) && r.status !== 'OK' && r.fixable);
if (needsInit || huskyDrift) {
const args = ['init', '--stack', stack, ...selectionFlags(sel)];
// Preserve the recorded install mode: a standalone repo re-inits standalone (no package dep).
Expand Down Expand Up @@ -407,7 +404,7 @@ function applyFix(
* git ROOT because that's the cwd the husky fragment shells the gate from — doctor should report
* what the hook would actually see, not what this cwd sees.
*/
function printQavisAdvisoryHealth(cwd: string, guards: string[]): void {
export function printQavisAdvisoryHealth(cwd: string, guards: string[]): void {
if (!guards.includes(QAVIS_ADVISORY_ID)) return;
const { gitRoot } = detectGitRoot(cwd);
if (!existsSync(join(gitRoot, QAVIS_RECIPE))) {
Expand Down Expand Up @@ -544,55 +541,6 @@ async function runOverlayDoctor(cwd: string, cfg: DevkitConfig, fix: boolean): P
// generator changed without a regen, or the hook was hand-edited. `--fix` regenerates it. Skills/
// agents are advisory (a re-sync heals them). Pin/extends/structure/version checks don't apply —
// the configs are hand-owned local files, not `@norvalbv/devkit/*` extends, and there is no dep.
async function runSelfHostDoctor(cwd: string, cfg: DevkitConfig, fix: boolean): Promise<number> {
const { gitRoot, pkgRel } = detectGitRoot(cwd);
const hookPath = join(gitRoot, '.husky', 'pre-commit');
console.log('devkit doctor — self-host (source-mode dogfood)\n');

let hookOk = false;
if (!existsSync(hookPath)) {
console.log(' ✗ .husky/pre-commit MISSING — run `devkit init` (self-host)');
} else {
const currentBlock = extractGuardBlock(readFileSync(hookPath, 'utf8'), pkgRel);
const expectedBlock = buildSelfHostBlock(
{ ...selfHostSelection(), structureCmd: SELF_HOST_STRUCTURE_CMD, extras: SELF_HOST_EXTRAS },
pkgRel,
cwd,
);
if (currentBlock !== null && currentBlock.trim() === expectedBlock.trim()) {
hookOk = true;
console.log(' ✓ .husky/pre-commit in sync with the generator');
} else if (fix) {
installSelfHostHook(gitRoot, pkgRel, selfHostSelection(), false, cwd);
hookOk = true;
console.log(
' ✓ .husky/pre-commit regenerated (was stale — refreshed to the current generator)',
);
} else {
console.log(
' ⚠ .husky/pre-commit is STALE (generator changed or the hook was hand-edited) — run `devkit doctor --fix`',
);
}
}

// Agent assets — advisory (never gate the exit code; a re-run re-syncs them).
const sel: Partial<Selection> = cfg.components ?? {};
const surfaces = sel.agentTargets ?? ['claude', 'cursor'];
const primary = surfaces.includes('claude') ? 'claude' : surfaces[0];
const advise = (r: CheckResult) =>
console.log(` ${r.status === 'OK' ? '✓' : '·'} ${r.name}: ${r.detail}`);
if (sel.skills && primary) advise(await checkSkills(cwd, primary));
if (sel.agents && primary) advise(await checkAgents(cwd, primary));
printQavisAdvisoryHealth(cwd, sel.guards ?? []);

// The dogfood repo is gated by the same mechanism devkit ships to consumers, so it owes itself the
// same worktree-safety verdict — a self-host repo whose runner is unreachable gates nothing either.
const runner = checkHookRunner(cwd);
console.log(` ${runner.status === 'OK' ? '✓' : '⚠'} ${runner.name}: ${runner.detail}`);
if (runner.status !== 'OK') console.log(` → ${runner.remediation}`);

return hookOk && runner.status === 'OK' ? 0 : 1;
}

/**
* Build the doctor result list for a package/standalone install from its recorded config — a pure
Expand Down
18 changes: 18 additions & 0 deletions cli/lib/doctor/hook-checks.mts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { detectGitRoot } from '../detect-git-root.mts';
import { markEnd, markStart } from '../husky/husky.mts';
import { extractGuardBlock, QAVIS_ADVISORY_ID } from '../husky/husky-block.mts';
import { type CheckResult, check } from './check-result.mts';
import { strayGateCalls } from './stray-gate-calls.mts';

// Selection-aware: only the SELECTED guards must be present in the block (a deselected
// guard being absent is correct, not drift). Monorepo: the hook lives at the git root and the
Expand Down Expand Up @@ -59,6 +60,23 @@ export function checkHusky(cwd: string, selectedGuards: string[]): CheckResult {
true,
);
}
// A gate devkit emits, ALSO invoked outside the block, runs twice per commit — and for the LLM
// judges that is a second model bill on every single commit, while .devkit/config.json still
// describes one run. Report it; never rewrite the consumer's own lines (see strayGateCalls).
const stray = strayGateCalls(content, pkgRel, gitRoot);
if (stray.length) {
const where = stray.map((s) => `${s.bin} (line ${s.line})`).join(', ');
return check(
'.husky/pre-commit',
'DRIFT',
`${stray.length} devkit gate call(s) OUTSIDE the managed block — these run a second time every commit: ${where}`,
'each is a hand-written copy of a gate devkit now owns. Review, then delete the block around it so only the devkit-guards block runs it — or keep it deliberately if it differs (different flags/ordering)',
// NOT fixable: `--fix` re-runs `devkit init`, which regenerates the managed block and cannot
// touch a hand-written line outside it. Claiming fixable would loop with no effect, and this
// check is report-only by design (see strayGateCalls) — the consumer decides.
false,
);
}
return check(
'.husky/pre-commit',
'OK',
Expand Down
85 changes: 85 additions & 0 deletions cli/lib/doctor/self-host-doctor.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/**
* `devkit doctor` for a SELF-HOSTED repo (devkit itself): the hook is generated from source paths
* rather than `bunx guard-*`, so it is compared against the generator directly instead of going
* through the CheckResult pipeline. Split out of doctor.mts, which is at its line budget.
*/

import { existsSync, readFileSync } from 'node:fs';
import { join } from 'node:path';
import { printQavisAdvisoryHealth } from '../../commands/doctor.mts';
import type { Selection } from '../components.mts';
import { detectGitRoot } from '../detect-git-root.mts';
import { extractGuardBlock } from '../husky/husky-block.mts';
import {
buildSelfHostBlock,
installSelfHostHook,
SELF_HOST_EXTRAS,
SELF_HOST_STRUCTURE_CMD,
selfHostSelection,
} from '../husky/self-host.mts';
import { checkAgents, checkSkills } from './asset-checks.mts';
import type { CheckResult } from './check-result.mts';
import { checkHookRunner } from './hook-checks.mts';
import { printStrayGateCalls } from './stray-gate-calls.mts';

/** The recorded config fields this path consults. */
interface SelfHostConfig {
components?: Partial<Selection>;
}

export async function runSelfHostDoctor(
cwd: string,
cfg: SelfHostConfig,
fix: boolean,
): Promise<number> {
const { gitRoot, pkgRel } = detectGitRoot(cwd);
const hookPath = join(gitRoot, '.husky', 'pre-commit');
console.log('devkit doctor — self-host (source-mode dogfood)\n');

let hookOk = false;
if (!existsSync(hookPath)) {
console.log(' ✗ .husky/pre-commit MISSING — run `devkit init` (self-host)');
} else {
const currentBlock = extractGuardBlock(readFileSync(hookPath, 'utf8'), pkgRel);
const expectedBlock = buildSelfHostBlock(
{ ...selfHostSelection(), structureCmd: SELF_HOST_STRUCTURE_CMD, extras: SELF_HOST_EXTRAS },
pkgRel,
cwd,
);
if (currentBlock !== null && currentBlock.trim() === expectedBlock.trim()) {
hookOk = true;
console.log(' ✓ .husky/pre-commit in sync with the generator');
} else if (fix) {
installSelfHostHook(gitRoot, pkgRel, selfHostSelection(), false, cwd);
hookOk = true;
console.log(
' ✓ .husky/pre-commit regenerated (was stale — refreshed to the current generator)',
);
} else {
console.log(
' ⚠ .husky/pre-commit is STALE (generator changed or the hook was hand-edited) — run `devkit doctor --fix`',
);
}
// Self-host never runs checkHusky, so without this the duplicate-gate warning is unreachable in
// exactly the repo that dogfoods devkit — the one most likely to grow a hand-written gate copy.
printStrayGateCalls(readFileSync(hookPath, 'utf8'), pkgRel, cwd);
}

// Agent assets — advisory (never gate the exit code; a re-run re-syncs them).
const sel: Partial<Selection> = cfg.components ?? {};
const surfaces = sel.agentTargets ?? ['claude', 'cursor'];
const primary = surfaces.includes('claude') ? 'claude' : surfaces[0];
const advise = (r: CheckResult) =>
console.log(` ${r.status === 'OK' ? '✓' : '·'} ${r.name}: ${r.detail}`);
if (sel.skills && primary) advise(await checkSkills(cwd, primary));
if (sel.agents && primary) advise(await checkAgents(cwd, primary));
printQavisAdvisoryHealth(cwd, sel.guards ?? []);

// The dogfood repo is gated by the same mechanism devkit ships to consumers, so it owes itself the
// same worktree-safety verdict — a self-host repo whose runner is unreachable gates nothing either.
const runner = checkHookRunner(cwd);
console.log(` ${runner.status === 'OK' ? '✓' : '⚠'} ${runner.name}: ${runner.detail}`);
if (runner.status !== 'OK') console.log(` → ${runner.remediation}`);

return hookOk && runner.status === 'OK' ? 0 : 1;
}
Loading
Loading