diff --git a/src/guard.mjs b/src/guard.mjs index 5c79a5e..140ecdd 100644 --- a/src/guard.mjs +++ b/src/guard.mjs @@ -236,6 +236,10 @@ export function scanProposedCommits(repo, commits, { rejectNote, limits = {} }) policyMigrationContexts: reviewContexts, limits, messageScope: authoredLocally ? 'commit' : 'changes-only', + // This guard runs over history the developer is receiving, not + // writing. A policy it cannot parse must not wedge every pull, + // merge and reset with no way forward. + tolerateUnreadablePolicy: true, }); } catch (error) { diff --git a/src/policy-resolver.mjs b/src/policy-resolver.mjs index 0fa0dc4..1884b5c 100644 --- a/src/policy-resolver.mjs +++ b/src/policy-resolver.mjs @@ -38,7 +38,7 @@ export function resolvePolicy(repo, options = {}) { if (!repo?.root || !repo?.stateDir) { throw new TypeError('repo must be an open repository'); } - const { target = 'worktree', revision, profile, strictFloor } = options; + const { target = 'worktree', revision, profile, strictFloor, tolerant = false } = options; let resolved; if (target === 'worktree') { resolved = resolveWorktreePolicy(repo); @@ -48,11 +48,26 @@ export function resolvePolicy(repo, options = {}) { if (typeof revision !== 'string' || !revision) { throw new PolicyTargetError('commit target needs a revision'); } - resolved = resolveGitPolicy( - repo, - readCommitPath(repo, revision, POLICY_PATH), - 'commit-policy', - ); + const result = readCommitPath(repo, revision, POLICY_PATH); + try { + resolved = resolveGitPolicy(repo, result, 'commit-policy'); + } catch (error) { + // History is not the caller's to fix. A policy a future version + // wrote cannot be read here, and throwing turns every pull, merge + // and reset that carries it into a refusal with no way forward. The + // caller asks for tolerance and decides the fallback; the worktree, + // the index, and anything the developer asked about directly still + // get the loud error. + if (!tolerant || !(error instanceof ProjectPolicyError)) throw error; + return { + profile: null, + source: 'commit-policy', + target: result.target, + policy_object_id: result.oid, + policy_mode: result.mode, + unparseable: error.message, + }; + } } else { throw new PolicyTargetError('must be worktree, staged, or commit'); } @@ -141,7 +156,7 @@ function resolveGitPolicy(repo, result, source) { }; } -function localFallback(repo, target) { +export function localFallback(repo, target) { const config = loadConfig(repo.stateDir); return { profile: config.profile, diff --git a/src/scan-target.mjs b/src/scan-target.mjs index bea655a..f85f517 100644 --- a/src/scan-target.mjs +++ b/src/scan-target.mjs @@ -1,6 +1,6 @@ import { Engine, newEngineWithDiagnostics } from './scan.mjs'; import { loadOverrides } from './state.mjs'; -import { applyExplicitProfile, applyStrictFloor, resolvePolicy } from './policy-resolver.mjs'; +import { applyExplicitProfile, applyStrictFloor, localFallback, resolvePolicy } from './policy-resolver.mjs'; import { GitRevisionError, stagedEntries, @@ -155,19 +155,41 @@ export function resolveStagedPolicy(repo, explicitProfile) { function scanCommit(repo, options) { if (!options.revision) throw new TypeError('commit scan needs a revision'); const snapshot = commitSnapshot(repo, options.revision); - const rawPolicy = resolvePolicy(repo, { target: 'commit', revision: snapshot.commit }); + // The final ref guard scans history the developer did not write, so a policy + // this version cannot parse must not turn every pull, merge and reset into a + // refusal. It asks for tolerance; the profile then comes from local config + // and the reason is reported. Direct questions (check --commit, audit) still + // get the loud error. + const tolerant = Boolean(options.tolerateUnreadablePolicy); + const unreadablePolicies = []; + const resolvedPolicy = resolvePolicy(repo, { target: 'commit', revision: snapshot.commit, tolerant }); + if (resolvedPolicy.unparseable) unreadablePolicies.push(resolvedPolicy.unparseable); + const rawPolicy = resolvedPolicy.unparseable + ? { + ...localFallback(repo, resolvedPolicy.target), + policy_object_id: resolvedPolicy.policy_object_id, + policy_mode: resolvedPolicy.policy_mode, + } + : resolvedPolicy; // Bind policy-migration acks to the repository's current HEAD — the state an // ack is recorded against (policy-review --head --transition X) — // matching scanRange and resolveStagedPolicy. Probing with snapshot.commit // almost never matched a real ack, so `check --commit X` spuriously blocked // transitions that `check --range` honored. Exact-match security on every // ack field is unchanged; a null head simply fails closed (no ack honored). - const headBaseline = headPolicy(repo); + const headBaseline = headPolicy(repo, tolerant); const head = headBaseline?.target?.startsWith('commit:') ? headBaseline.target.slice('commit:'.length) : null; - const strictParentPolicies = snapshot.parents - .map((parent) => resolvePolicy(repo, { target: 'commit', revision: parent })) - .filter(isVersionedStrict); + const parentPolicies = snapshot.parents + .map((parent) => resolvePolicy(repo, { target: 'commit', revision: parent, tolerant })); + for (const parent of parentPolicies) { + if (parent.unparseable) unreadablePolicies.push(parent.unparseable); + } + // A parent whose policy cannot be read might have been strict. Counting it + // toward the floor keeps the guard conservative; ignoring it would let an + // unreadable file switch strict enforcement off. + const unreadableParent = parentPolicies.some((parent) => parent.unparseable); + const strictParentPolicies = parentPolicies.filter(isVersionedStrict); const policyMigrationContexts = options.policyMigrationContexts ?? (head ? [{ head, transition: snapshot.commit, @@ -181,7 +203,8 @@ function scanCommit(repo, options) { newMode: rawPolicy.policy_mode, }) )); - const floor = strictParentPolicies.length > 0 && !acknowledged && !isVersionedStrict(rawPolicy) + const floor = (strictParentPolicies.length > 0 || unreadableParent) + && !acknowledged && !isVersionedStrict(rawPolicy) ? 'parent-strict-floor' : null; const policy = effectivePolicy( @@ -205,6 +228,12 @@ function scanCommit(repo, options) { const accumulator = createAccumulator(options.limits); accumulator.addSkipped(loaded.skipped); const diagnostics = [...loaded.diagnostics]; + for (const message of unreadablePolicies) { + diagnostics.push({ + level: 'warning', + message: `${message}; scanned ${snapshot.commit} under the ${policy.profile} profile instead`, + }); + } if (snapshot.shallowBoundary) { const message = 'shallow repository: commit scan cannot prove parent policy; fetch full history (e.g. fetch-depth: 0)'; @@ -550,9 +579,9 @@ function effectivePolicy(raw, explicitProfile, floorSource, enforcedObjectIds = return applyExplicitProfile(floored, explicitProfile); } -function headPolicy(repo) { +function headPolicy(repo, tolerant = false) { try { - return resolvePolicy(repo, { target: 'commit', revision: 'HEAD' }); + return resolvePolicy(repo, { target: 'commit', revision: 'HEAD', tolerant }); } catch (error) { if (error instanceof GitRevisionError) return null; throw error; diff --git a/tests/policy-resolver.test.mjs b/tests/policy-resolver.test.mjs index f35b322..c1779cb 100644 --- a/tests/policy-resolver.test.mjs +++ b/tests/policy-resolver.test.mjs @@ -12,6 +12,7 @@ import { resolvePolicy, } from '../src/policy-resolver.mjs'; import { loadProjectPolicy, ProjectPolicyError, saveConfig } from '../src/state.mjs'; +import { scanGitTarget } from '../src/scan-target.mjs'; function freshRepo() { const dir = mkdtempSync(join(tmpdir(), 'aim-policy-target-')); @@ -199,3 +200,59 @@ test('worktree policy resolution rejects valid and dangling symlinks', { rmSync(dir, { recursive: true, force: true }); } }); + +// A policy this version cannot parse, anywhere in fetched history, used to make +// the final ref guard refuse every update that would introduce it — pull, reset +// and merge alike, with no message pointing at a way out. The guard needs a +// resolution it can carry on from; the loud error stays for the worktree, the +// index, and anything the developer asked about directly. +test('a commit policy this version cannot parse resolves tolerantly instead of throwing', () => { + const dir = freshRepo(); + try { + const repo = openRepo(dir); + writeFileSync(join(dir, '.aimhooman.json'), JSON.stringify({ schema_version: 1, profile: 'clean', exclude: ['vendor/**'] })); + execFileSync('git', ['add', '.aimhooman.json'], { cwd: dir }); + execFileSync('git', ['commit', '-q', '-m', 'unreadable policy'], { cwd: dir }); + + assert.throws( + () => resolvePolicy(repo, { target: 'commit', revision: 'HEAD' }), + ProjectPolicyError, + 'the direct question still gets the loud answer', + ); + + const tolerant = resolvePolicy(repo, { target: 'commit', revision: 'HEAD', tolerant: true }); + assert.equal(tolerant.profile, null, 'no profile can be read from it'); + assert.match(tolerant.unparseable, /unsupported field/, 'and the reason travels with the result'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +// The other half: the guard that receives history must be able to finish a scan +// of a commit whose policy it cannot read, under a profile it can reach, and say +// which commit it fell back on. +test('a commit scan tolerates an unreadable policy and reports the fallback', () => { + const dir = freshRepo(); + try { + const repo = openRepo(dir); + writeFileSync(join(dir, '.aimhooman.json'), JSON.stringify({ schema_version: 1, profile: 'clean', exclude: ['vendor/**'] })); + execFileSync('git', ['add', '.aimhooman.json'], { cwd: dir }); + execFileSync('git', ['commit', '-q', '-m', 'unreadable policy'], { cwd: dir }); + const head = execFileSync('git', ['rev-parse', 'HEAD'], { cwd: dir, encoding: 'utf8' }).trim(); + + assert.throws( + () => scanGitTarget(repo, { kind: 'commit', revision: head }), + ProjectPolicyError, + 'a direct scan still refuses loudly', + ); + + const scan = scanGitTarget(repo, { kind: 'commit', revision: head, tolerateUnreadablePolicy: true }); + assert.ok(scan.profile, 'the scan runs under a profile it could reach'); + assert.ok( + scan.diagnostics.some((d) => d.level === 'warning' && d.message.includes(head)), + 'and names the commit it could not read the policy of', + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +});