fix(#2665): budget the scan per target, so order stops deciding the verdict
MAX_ENTRIES was a single running budget threaded across every watch target. One large early target exhausted it, and every target scanned afterwards reported truncated -> `unverified` -- which under GSD_STRICT_LIVE_CONFIG_GUARD=1 is a failed run. The guard's verdict therefore depended on directory iteration order and on unrelated local state, neither of which says anything about whether the suite leaked. Each target now draws a fresh allotment, so a pathological tree truncates itself and nothing else. MAX_TOTAL_ENTRIES keeps the aggregate bounded -- which is what the single budget was actually for -- and when that ceiling engages, the targets it curtails are still reported `unverified` rather than attested clean. The limits are injectable so the boundary is testable without materialising 20000 entries, matching the `deps` seam the resolvers already use. Two of the three new tests fail when the shared budget is restored; the third asserts the retained global ceiling, which is deliberately unchanged behaviour. Addresses review finding: Major 6.
This commit is contained in:
@@ -92,8 +92,23 @@ const GSD_ARTIFACT_PREFIX = 'gsd-';
|
||||
*/
|
||||
const NON_REGISTRY_OWNED_FILE = 'config.toml';
|
||||
|
||||
/** Bounds on the recursive walk, so a pathological tree cannot stall the suite. */
|
||||
/**
|
||||
* Bounds on the recursive walk, so a pathological tree cannot stall the suite.
|
||||
*
|
||||
* MAX_ENTRIES is PER WATCH TARGET, not per snapshot. It was a single running
|
||||
* budget threaded across every target, which made the guard's verdict depend on
|
||||
* directory ORDER and on unrelated local state: one large early target exhausted
|
||||
* it, and every target scanned afterwards reported `truncated` -> `unverified`,
|
||||
* which under GSD_STRICT_LIVE_CONFIG_GUARD=1 is a failed run. Per-target means a
|
||||
* pathological tree truncates ITSELF and nothing else.
|
||||
*
|
||||
* MAX_TOTAL_ENTRIES keeps the aggregate bounded, which is what the single budget
|
||||
* was really for. It engages only when the per-target bounds together exceed it;
|
||||
* when it does, the targets it curtails are reported `unverified` -- never
|
||||
* silently clean.
|
||||
*/
|
||||
const MAX_ENTRIES = 20000;
|
||||
const MAX_TOTAL_ENTRIES = 200000;
|
||||
const MAX_DEPTH = 12;
|
||||
|
||||
/**
|
||||
@@ -292,8 +307,9 @@ function newestMtime(target, budget) {
|
||||
* @returns {Record<string, {exists: boolean, newest: number, truncated: boolean}>}
|
||||
* keyed by absolute entry path.
|
||||
*/
|
||||
function snapshotLiveConfig(roots, extraTargets = []) {
|
||||
const budget = { remaining: MAX_ENTRIES };
|
||||
function snapshotLiveConfig(roots, extraTargets = [], limits = {}) {
|
||||
const perTarget = limits.perTarget ?? MAX_ENTRIES;
|
||||
const total = { remaining: limits.total ?? MAX_TOTAL_ENTRIES };
|
||||
const snap = {};
|
||||
|
||||
const record = (target) => {
|
||||
@@ -301,7 +317,14 @@ function snapshotLiveConfig(roots, extraTargets = []) {
|
||||
snap[target] = { exists: false, newest: 0, truncated: false };
|
||||
return;
|
||||
}
|
||||
// A FRESH budget per target, drawn against the global ceiling. See the
|
||||
// MAX_ENTRIES docblock: a shared running budget let one large early target
|
||||
// cascade `unverified` over every target scanned after it, so the verdict
|
||||
// depended on iteration order rather than on what the run actually touched.
|
||||
const budget = { remaining: Math.min(perTarget, total.remaining) };
|
||||
const allotted = budget.remaining;
|
||||
const { newest, truncated } = newestMtime(target, budget);
|
||||
total.remaining -= allotted - budget.remaining;
|
||||
snap[target] = { exists: true, newest, truncated };
|
||||
};
|
||||
|
||||
@@ -426,6 +449,7 @@ module.exports = {
|
||||
GSD_PREFIXED_PARENTS,
|
||||
GSD_ARTIFACT_PREFIX,
|
||||
MAX_ENTRIES,
|
||||
MAX_TOTAL_ENTRIES,
|
||||
MAX_DEPTH,
|
||||
resolveLiveConfigRoots,
|
||||
resolveExtraWatchTargets,
|
||||
|
||||
@@ -230,6 +230,61 @@ describe('#2665: live-config hermeticity guard', () => {
|
||||
}
|
||||
});
|
||||
|
||||
test('the scan budget is PER TARGET, so one big tree cannot cascade unverified', () => {
|
||||
// #2665 round 5: with a single running budget, target A exhausting it made
|
||||
// target B report `truncated` -> `unverified` -- a strict-mode FAILURE caused
|
||||
// by an unrelated directory. Each target now gets its own allotment.
|
||||
const [big] = treeWithEntries(8);
|
||||
const [small] = treeWithEntries(2);
|
||||
try {
|
||||
const snap = snapshotLiveConfig([], [big, small], { perTarget: 9, total: 1000 });
|
||||
assert.strictEqual(snap[path.resolve(big)].truncated, false, 'big target should fit its own budget');
|
||||
assert.strictEqual(
|
||||
snap[path.resolve(small)].truncated,
|
||||
false,
|
||||
'small target must NOT inherit exhaustion from a target scanned before it',
|
||||
);
|
||||
} finally {
|
||||
cleanup(big);
|
||||
cleanup(small);
|
||||
}
|
||||
});
|
||||
|
||||
test('the truncation verdict does not depend on target ORDER', () => {
|
||||
const [big] = treeWithEntries(8);
|
||||
const [small] = treeWithEntries(2);
|
||||
try {
|
||||
const limits = { perTarget: 9, total: 1000 };
|
||||
const a = snapshotLiveConfig([], [big, small], limits);
|
||||
const b = snapshotLiveConfig([], [small, big], limits);
|
||||
assert.strictEqual(a[path.resolve(small)].truncated, b[path.resolve(small)].truncated);
|
||||
assert.strictEqual(a[path.resolve(big)].truncated, b[path.resolve(big)].truncated);
|
||||
} finally {
|
||||
cleanup(big);
|
||||
cleanup(small);
|
||||
}
|
||||
});
|
||||
|
||||
test('the GLOBAL ceiling still bounds the aggregate, and reports unverified', () => {
|
||||
// The bound the single budget was really for is kept -- but when it engages,
|
||||
// the curtailed target is reported rather than silently attested clean.
|
||||
const [a] = treeWithEntries(5);
|
||||
const [b] = treeWithEntries(5);
|
||||
try {
|
||||
const snap = snapshotLiveConfig([], [a, b], { perTarget: 6, total: 6 });
|
||||
assert.strictEqual(snap[path.resolve(a)].truncated, false);
|
||||
assert.strictEqual(snap[path.resolve(b)].truncated, true, 'ceiling-curtailed target must be truncated');
|
||||
const violations = diffLiveConfig(snap, snap);
|
||||
assert.ok(
|
||||
violations.some((v) => v.kind === 'unverified' && v.path === path.resolve(b)),
|
||||
`expected an unverified violation for the curtailed target: ${JSON.stringify(violations)}`,
|
||||
);
|
||||
} finally {
|
||||
cleanup(a);
|
||||
cleanup(b);
|
||||
}
|
||||
});
|
||||
|
||||
test('the report names the path and the remedy', () => {
|
||||
const out = formatViolations([{ path: '/live/.claude/gsd-core', kind: 'created' }]);
|
||||
assert.match(out, /HERMETICITY WARNING/);
|
||||
|
||||
Reference in New Issue
Block a user