Skip to content

Commit 87d1e1d

Browse files
committed
fix(desktop): harden the renderer architecture ratchet base comparison
Add a --strict-base flag that turns the silent fallback to the committed base ledger (the pre-#4249 behaviour that wedged CI in #4250) into a hard failure, and pass it from the CI step that supplies --base. When the checker script differs from the base commit, also measure both trees with the base commit's checker and ratchet those results, so a change cannot weaken a rule and lower both sides of the comparison at once. Cover the base-tree derivation with git-fixture integration tests. Generated-by: Claude Code
1 parent 68cda0b commit 87d1e1d

5 files changed

Lines changed: 503 additions & 36 deletions

File tree

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ jobs:
196196
BASE_SHA: ${{ github.event_name == 'push' && github.event.before || github.event.pull_request.base.sha }}
197197
run: |
198198
if [[ -n "$BASE_SHA" && ! "$BASE_SHA" =~ ^0+$ ]]; then
199-
npm run check:renderer-architecture -- --base "$BASE_SHA"
199+
npm run check:renderer-architecture -- --base "$BASE_SHA" --strict-base
200200
else
201201
npm run check:renderer-architecture
202202
fi

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ apps/desktop/resources/bin/
3535
# Rebuilt from experiments/windows-sandbox by scripts/package-windows-x64.mjs.
3636
apps/desktop/resources/windows-sandbox/
3737
apps/desktop/bundled-git.json
38+
# Scratch copy of the base commit's renderer architecture checker, written next
39+
# to the live script so its imports resolve; the check removes it after each run.
40+
apps/desktop/scripts/.tmp-base-checker-*.mjs
3841

3942
# Generated desktop release inputs and outputs.
4043
apps/desktop/resources/tools/

apps/desktop/scripts/check-renderer-architecture.mjs

Lines changed: 168 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2675,18 +2675,10 @@ export function checkRendererArchitecture({
26752675
// wedge the ledger permanently. We materialize the base tree and re-derive its
26762676
// debt, keeping the base ledger only as the source of policy fields (hook
26772677
// transitions, growth directories, root-debt key set, ownership).
2678-
function deriveBaseTreeConfig(repoRoot, desktopRoot, base, baseCommittedConfig) {
2678+
function materializeBaseTree(repoRoot, base) {
26792679
const scratch = mkdtempSync(join(tmpdir(), 'renderer-arch-base-'));
26802680
const worktreePath = join(scratch, 'tree');
2681-
try {
2682-
execFileSync('git', ['worktree', 'add', '--detach', worktreePath, base], {
2683-
cwd: repoRoot,
2684-
encoding: 'utf8',
2685-
stdio: ['ignore', 'pipe', 'pipe'],
2686-
});
2687-
const baseDesktopRoot = resolve(worktreePath, relative(repoRoot, desktopRoot));
2688-
return generateArchitectureConfig(baseDesktopRoot, baseCommittedConfig);
2689-
} finally {
2681+
const remove = () => {
26902682
try {
26912683
execFileSync('git', ['worktree', 'remove', '--force', worktreePath], {
26922684
cwd: repoRoot,
@@ -2704,11 +2696,131 @@ function deriveBaseTreeConfig(repoRoot, desktopRoot, base, baseCommittedConfig)
27042696
} catch {
27052697
// Best-effort cleanup of the scratch directory.
27062698
}
2699+
};
2700+
try {
2701+
execFileSync('git', ['worktree', 'add', '--detach', worktreePath, base], {
2702+
cwd: repoRoot,
2703+
encoding: 'utf8',
2704+
stdio: ['ignore', 'pipe', 'pipe'],
2705+
});
2706+
} catch (error) {
2707+
remove();
2708+
throw error;
2709+
}
2710+
return { remove, worktreePath };
2711+
}
2712+
2713+
function deriveBaseTreeConfig(baseDesktopRoot, baseCommittedConfig) {
2714+
return generateArchitectureConfig(baseDesktopRoot, baseCommittedConfig);
2715+
}
2716+
2717+
// If the base tree cannot be materialized or analyzed (e.g. git worktree is
2718+
// unavailable), fall back to the committed base ledger so the ratchet still
2719+
// runs. That silent fallback is exactly what wedged CI in #4250, so
2720+
// `--strict-base` turns it into a hard failure instead.
2721+
function baseTreeFallback({ base, baseCommittedConfig, error, strictBase }) {
2722+
const reason = error instanceof Error ? error.message : String(error);
2723+
if (strictBase) {
2724+
throw new Error(
2725+
`could not derive base tree debt at ${base}, and --strict-base forbids falling back to the committed base ledger (${reason})`,
2726+
);
2727+
}
2728+
console.warn(
2729+
`Renderer architecture check: could not derive base tree debt at ${base}; ` +
2730+
`falling back to the committed base ledger. (${reason})`,
2731+
);
2732+
return baseCommittedConfig;
2733+
}
2734+
2735+
// A change can weaken a rule in this checker and thereby lower both sides of
2736+
// the ratchet at once: the current tree and the re-derived base tree are then
2737+
// measured with the same relaxed rule, so the debt that rule used to flag
2738+
// vanishes from the comparison. Whenever the checker itself differs from the
2739+
// base commit, we therefore also measure both trees with the BASE commit's
2740+
// checker and ratchet those two measurements with the current comparison
2741+
// logic, so debt the base rules would have caught still fails.
2742+
async function crossCheckUnderBaseChecker({
2743+
base,
2744+
baseCommittedConfig,
2745+
baseDesktopRoot,
2746+
desktopRoot,
2747+
repoRoot,
2748+
strictBase,
2749+
}) {
2750+
const scriptPath = fileURLToPath(import.meta.url);
2751+
const relativeScript = normalizePath(relative(repoRoot, scriptPath));
2752+
let baseSource;
2753+
try {
2754+
baseSource = execFileSync('git', ['show', `${base}:${relativeScript}`], {
2755+
cwd: repoRoot,
2756+
encoding: 'utf8',
2757+
stdio: ['ignore', 'pipe', 'ignore'],
2758+
});
2759+
} catch {
2760+
console.log(
2761+
`Renderer architecture check: ${base} has no ${relativeScript}; skipping the base-checker cross-check.`,
2762+
);
2763+
return [];
2764+
}
2765+
if (baseSource === readFileSync(scriptPath, 'utf8')) return [];
2766+
2767+
const unavailable = (stage, error) => {
2768+
const reason = error instanceof Error ? error.message : String(error);
2769+
if (strictBase) {
2770+
throw new Error(
2771+
`base-checker cross-check: ${stage} at ${base}, and --strict-base forbids skipping the cross-check (${reason})`,
2772+
);
2773+
}
2774+
console.warn(`Renderer architecture check: base-checker cross-check skipped; ${stage} at ${base}. (${reason})`);
2775+
return [];
2776+
};
2777+
const skip = (reason) => {
2778+
console.log(`Renderer architecture check: ${reason}; skipping the base-checker cross-check.`);
2779+
return [];
2780+
};
2781+
2782+
// The copy lives next to this script so its bare imports resolve exactly as
2783+
// ours do; the name is gitignored and the copy is removed even on failure.
2784+
const tempPath = join(dirname(scriptPath), `.tmp-base-checker-${process.pid}.mjs`);
2785+
try {
2786+
writeFileSync(tempPath, baseSource);
2787+
let baseChecker;
2788+
try {
2789+
baseChecker = await import(pathToFileURL(tempPath).href);
2790+
} catch (error) {
2791+
return unavailable('the base checker could not be imported', error);
2792+
}
2793+
if (typeof baseChecker.generateArchitectureConfig !== 'function') {
2794+
return skip(`the checker at ${base} does not export generateArchitectureConfig`);
2795+
}
2796+
let baseUnderBaseRules;
2797+
let currentUnderBaseRules;
2798+
try {
2799+
baseUnderBaseRules = baseChecker.generateArchitectureConfig(baseDesktopRoot, baseCommittedConfig);
2800+
currentUnderBaseRules = baseChecker.generateArchitectureConfig(desktopRoot, baseCommittedConfig);
2801+
} catch (error) {
2802+
return unavailable('the base checker could not measure the base and current trees', error);
2803+
}
2804+
const shapeViolations = [];
2805+
if (
2806+
!validateArchitectureConfig(baseUnderBaseRules, 'base-checker base', shapeViolations) ||
2807+
!validateArchitectureConfig(currentUnderBaseRules, 'base-checker current', shapeViolations)
2808+
) {
2809+
return skip(`the checker at ${base} does not produce the current ledger shape (${shapeViolations.join('; ')})`);
2810+
}
2811+
const violations = [];
2812+
validateMonotonicDebt(currentUnderBaseRules, baseUnderBaseRules, desktopRoot, violations);
2813+
console.log(
2814+
`Renderer architecture check: ${relativeScript} differs from ${base}; cross-checked debt under the base checker.`,
2815+
);
2816+
return violations.sort().map((violation) => `base-checker cross-check: ${violation}`);
2817+
} finally {
2818+
rmSync(tempPath, { force: true });
27072819
}
27082820
}
27092821

2710-
function loadBaseConfig(repoRoot, desktopRoot, base) {
2711-
if (!base) return { baseConfig: undefined, introducedLedger: false };
2822+
async function loadBaseConfig(repoRoot, desktopRoot, base, { strictBase = false } = {}) {
2823+
if (!base) return { baseConfig: undefined, crossCheckViolations: [], introducedLedger: false };
27122824
const relativeConfig = normalizePath(relative(repoRoot, join(desktopRoot, 'renderer-architecture.json')));
27132825
try {
27142826
execFileSync('git', ['rev-parse', '--verify', `${base}^{commit}`], {
@@ -2743,7 +2855,7 @@ function loadBaseConfig(repoRoot, desktopRoot, base) {
27432855
},
27442856
).trim();
27452857
if (diffStatus === `A\t${relativeConfig}` || worktreeStatus === `?? ${relativeConfig}`) {
2746-
return { baseConfig: undefined, introducedLedger: true };
2858+
return { baseConfig: undefined, crossCheckViolations: [], introducedLedger: true };
27472859
}
27482860
throw new Error(`base ledger is missing at ${base}:${relativeConfig}`);
27492861
}
@@ -2757,57 +2869,79 @@ function loadBaseConfig(repoRoot, desktopRoot, base) {
27572869
);
27582870
}
27592871

2872+
let baseTree;
27602873
try {
2874+
baseTree = materializeBaseTree(repoRoot, base);
2875+
} catch (error) {
27612876
return {
2762-
baseConfig: deriveBaseTreeConfig(repoRoot, desktopRoot, base, baseCommittedConfig),
2877+
baseConfig: baseTreeFallback({ base, baseCommittedConfig, error, strictBase }),
2878+
crossCheckViolations: [],
27632879
introducedLedger: false,
27642880
};
2765-
} catch (error) {
2766-
// If the base tree cannot be materialized or analyzed (e.g. git worktree is
2767-
// unavailable), fall back to the committed base ledger so the ratchet still
2768-
// runs. This restores the pre-fix behavior rather than crashing the check.
2769-
console.warn(
2770-
`Renderer architecture check: could not derive base tree debt at ${base}; ` +
2771-
`falling back to the committed base ledger. (${error instanceof Error ? error.message : String(error)})`,
2772-
);
2773-
return { baseConfig: baseCommittedConfig, introducedLedger: false };
2881+
}
2882+
try {
2883+
const baseDesktopRoot = resolve(baseTree.worktreePath, relative(repoRoot, desktopRoot));
2884+
let baseConfig;
2885+
try {
2886+
baseConfig = deriveBaseTreeConfig(baseDesktopRoot, baseCommittedConfig);
2887+
} catch (error) {
2888+
baseConfig = baseTreeFallback({ base, baseCommittedConfig, error, strictBase });
2889+
}
2890+
const crossCheckViolations = await crossCheckUnderBaseChecker({
2891+
base,
2892+
baseCommittedConfig,
2893+
baseDesktopRoot,
2894+
desktopRoot,
2895+
repoRoot,
2896+
strictBase,
2897+
});
2898+
return { baseConfig, crossCheckViolations, introducedLedger: false };
2899+
} finally {
2900+
baseTree.remove();
27742901
}
27752902
}
27762903

2904+
const CLI_USAGE = 'usage: check-renderer-architecture.mjs [--write] [--base <commit> [--strict-base]]';
2905+
27772906
function parseCliArguments(args) {
27782907
let base;
2908+
let strictBase = false;
27792909
let write = false;
27802910
for (let index = 0; index < args.length; index += 1) {
27812911
const argument = args[index];
27822912
if (argument === '--write' && !write) {
27832913
write = true;
27842914
continue;
27852915
}
2916+
if (argument === '--strict-base' && !strictBase) {
2917+
strictBase = true;
2918+
continue;
2919+
}
27862920
if (argument === '--base' && base === undefined) {
27872921
const value = args[index + 1];
2788-
if (!value || value.startsWith('--')) {
2789-
throw new Error('usage: check-renderer-architecture.mjs [--write] [--base <commit>]');
2790-
}
2922+
if (!value || value.startsWith('--')) throw new Error(CLI_USAGE);
27912923
base = value;
27922924
index += 1;
27932925
continue;
27942926
}
2795-
throw new Error('usage: check-renderer-architecture.mjs [--write] [--base <commit>]');
2927+
throw new Error(CLI_USAGE);
27962928
}
2797-
return { base, write };
2929+
if (strictBase && base === undefined) throw new Error(`--strict-base requires --base <commit>\n${CLI_USAGE}`);
2930+
return { base, strictBase, write };
27982931
}
27992932

2800-
function runCli() {
2933+
async function runCli() {
28012934
const desktopRoot = resolve(fileURLToPath(new URL('..', import.meta.url)));
28022935
const repoRoot = resolve(desktopRoot, '../..');
28032936
let base;
28042937
let config;
28052938
let loadedBase;
2939+
let strictBase;
28062940
let write;
28072941
try {
2808-
({ base, write } = parseCliArguments(process.argv.slice(2)));
2942+
({ base, strictBase, write } = parseCliArguments(process.argv.slice(2)));
28092943
config = JSON.parse(readFileSync(join(desktopRoot, 'renderer-architecture.json'), 'utf8'));
2810-
loadedBase = loadBaseConfig(repoRoot, desktopRoot, base);
2944+
loadedBase = await loadBaseConfig(repoRoot, desktopRoot, base, { strictBase });
28112945
if (write) {
28122946
config = generateArchitectureConfig(desktopRoot, config);
28132947
writeFileSync(join(desktopRoot, 'renderer-architecture.json'), `${JSON.stringify(config, null, 2)}\n`);
@@ -2818,8 +2952,8 @@ function runCli() {
28182952
process.exitCode = 1;
28192953
return;
28202954
}
2821-
const { baseConfig, introducedLedger } = loadedBase;
2822-
const violations = checkRendererArchitecture({ baseConfig, config, desktopRoot });
2955+
const { baseConfig, crossCheckViolations, introducedLedger } = loadedBase;
2956+
const violations = [...checkRendererArchitecture({ baseConfig, config, desktopRoot }), ...crossCheckViolations];
28232957
if (violations.length > 0) {
28242958
console.error('Renderer architecture check failed:');
28252959
for (const violation of violations) console.error(`- ${violation}`);
@@ -2833,4 +2967,4 @@ function runCli() {
28332967
console.log(`Renderer architecture check passed${baseConfig ? ` against ${base}` : ''}.`);
28342968
}
28352969

2836-
if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) runCli();
2970+
if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) await runCli();

0 commit comments

Comments
 (0)