diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 7f8c890e4..d41a2588b 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -25,7 +25,8 @@ import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { runScriptSync } from '../lib/script-invocation'; import { collectCommit, collectLive } from '../lib/review-collect'; import { excludeLines, reviewExclude, type Exclusion } from '../lib/review-exclude'; -import { loadPolicyAt, isUnavailable } from '../lib/ship-policy'; +import { isUnavailable } from '../lib/ship-policy'; +import { loadPolicyAt } from '../lib/ship-policy-at'; import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth'; import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted'; import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check'; diff --git a/lib/review-exclude.ts b/lib/review-exclude.ts index 282594953..cc461a7c1 100644 --- a/lib/review-exclude.ts +++ b/lib/review-exclude.ts @@ -26,7 +26,8 @@ import { collectLive } from './review-collect'; import { DATA_PATH, classifyPath, isAgentProse, isDataConfig, isGatePath, type PathLines } from './review-depth'; import { secretLine } from './review-targeted'; import { cmpVersion, parseVersion } from './version-source'; -import { REVIEW_EXCLUDE_KEY_VERSION, SHIP_POLICY_PATH, isUnavailable, loadPolicyAt, policyUnavailableLine } from './ship-policy'; +import { REVIEW_EXCLUDE_KEY_VERSION, SHIP_POLICY_PATH, isUnavailable, policyUnavailableLine } from './ship-policy'; +import { loadPolicyAt } from './ship-policy-at'; /** The most diff bytes one reader is handed (about 500K tokens); above it the pass stops instead of truncating. */ export const REVIEW_DIFF_MAX_BYTES = 2_000_000; diff --git a/lib/ship-policy-at.ts b/lib/ship-policy-at.ts new file mode 100644 index 000000000..e32b92197 --- /dev/null +++ b/lib/ship-policy-at.ts @@ -0,0 +1,34 @@ +/** + * ship-policy-at — the ship policy committed at a commit the caller already + * resolved (docs/ship-policy.md#review-exclude-paths). Unlike + * loadPolicyFromBase, any git failure is `unavailable`, never a silent + * default. Callers: lib/review-exclude.ts, lib/ship-receipt-depth.ts, + * bin/gstack-review-plan. + */ +import { spawnSync } from 'node:child_process'; +import { SHIP_POLICY_PATH, finish, type PolicyLoad, type PolicySource } from './ship-policy'; + +function git(repoRoot: string, args: string[]): { ok: boolean; out: string; err: string } { + const r = spawnSync('git', args, { cwd: repoRoot, encoding: 'utf8', timeout: 30_000 }); + return { ok: r.status === 0, out: (r.stdout ?? '').trim(), err: (r.stderr ?? '').trim() }; +} + +/** + * The policy committed at `sha` (a commit the caller already resolved, labeled `ref`): `cat-file -e` first, and only an + * `ls-tree` that succeeds with no entry is a missing file (defaults); any git failure, including a `show` failure after + * the file was found, is `unavailable`, never a silent default. + */ +export function loadPolicyAt(repoRoot: string, sha: string, ref: string): PolicyLoad { + const label = `${ref}@${sha.slice(0, 12)}`; + const source: PolicySource = { kind: 'base', ref, sha, label: `${label}:${SHIP_POLICY_PATH}` }; + const spec = `${sha}:${SHIP_POLICY_PATH}`; + const base = ref.replace(/^origin\//, ''); + if (!git(repoRoot, ['cat-file', '-e', spec]).ok) { + const tree = git(repoRoot, ['ls-tree', sha, '--', SHIP_POLICY_PATH]); + if (!tree.ok) return { unavailable: true, base, detail: `git ls-tree ${sha.slice(0, 12)} failed: ${tree.err.split('\n')[0] || 'no detail'}` }; + if (!tree.out) return finish(null, { ...source, kind: 'none', label: `${label} (no ${SHIP_POLICY_PATH})` }); + } + const show = git(repoRoot, ['show', spec]); + if (!show.ok) return { unavailable: true, base, detail: `git show ${label}:${SHIP_POLICY_PATH} failed: ${show.err.split('\n')[0] || 'no detail'}` }; + return finish(show.out, source); +} diff --git a/lib/ship-policy.ts b/lib/ship-policy.ts index b9d199a79..a53ef5774 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -659,7 +659,7 @@ export function resolveBaseBranch(repoRoot: string, explicit?: string): string { return 'main'; } -function finish(rawText: string | null, source: PolicySource): LoadedPolicy { +export function finish(rawText: string | null, source: PolicySource): LoadedPolicy { if (rawText === null) return { policy: mergePolicy(null, source.label).policy, present: false, source, errors: [], values: mergePolicy(null, source.label).values }; let raw: unknown; try { raw = JSON.parse(rawText); } @@ -685,26 +685,6 @@ export function loadPolicyFromBase(repoRoot: string, base: string): PolicyLoad { return finish(show.ok ? show.out : null, source); } -/** - * The policy committed at `sha` (a commit the caller already resolved, labeled `ref`): `cat-file -e` first, and only an - * `ls-tree` that succeeds with no entry is a missing file (defaults); any git failure, including a `show` failure after - * the file was found, is `unavailable`, never a silent default. - */ -export function loadPolicyAt(repoRoot: string, sha: string, ref: string): PolicyLoad { - const label = `${ref}@${sha.slice(0, 12)}`; - const source: PolicySource = { kind: 'base', ref, sha, label: `${label}:${SHIP_POLICY_PATH}` }; - const spec = `${sha}:${SHIP_POLICY_PATH}`; - const base = ref.replace(/^origin\//, ''); - if (!git(repoRoot, ['cat-file', '-e', spec]).ok) { - const tree = git(repoRoot, ['ls-tree', sha, '--', SHIP_POLICY_PATH]); - if (!tree.ok) return { unavailable: true, base, detail: `git ls-tree ${sha.slice(0, 12)} failed: ${tree.err.split('\n')[0] || 'no detail'}` }; - if (!tree.out) return finish(null, { ...source, kind: 'none', label: `${label} (no ${SHIP_POLICY_PATH})` }); - } - const show = git(repoRoot, ['show', spec]); - if (!show.ok) return { unavailable: true, base, detail: `git show ${label}:${SHIP_POLICY_PATH} failed: ${show.err.split('\n')[0] || 'no detail'}` }; - return finish(show.out, source); -} - /** The working-tree policy: unreviewed, labeled so in every receipt. */ export function loadPolicyFromHead(repoRoot: string): LoadedPolicy { const file = path.join(repoRoot, SHIP_POLICY_PATH); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index dfba4e05c..80047c47e 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -22,7 +22,8 @@ import * as path from 'node:path'; import type { ShipReceipt } from './headless-artifacts'; import { collectCommit } from './review-collect'; import { reviewExclude, type Exclusion } from './review-exclude'; -import { isUnavailable, loadPolicyAt } from './ship-policy'; +import { isUnavailable } from './ship-policy'; +import { loadPolicyAt } from './ship-policy-at'; import { depthOfCommitInput, gateScanNeeded, type Depth } from './review-depth'; import { TARGETED_CLASSES, classList, targetedClasses, type TargetedClass } from './review-targeted'; import { runScriptSync } from './script-invocation'; diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index c50fdc3b1..b3cce7010 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -12,7 +12,8 @@ import { KEEP_ARG_MAX_BYTES, readSelection, reviewExclude, tooLargeLine, unreada import { collectCommit, collectLive } from '../lib/review-collect'; import { extractReceipt } from '../lib/ship-receipt'; import { bindShippedReceipt } from '../lib/ship-receipt-depth'; -import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable, loadPolicyAt } from '../lib/ship-policy'; +import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable } from '../lib/ship-policy'; +import { loadPolicyAt } from '../lib/ship-policy-at'; import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; const repos: FixtureRepo[] = [];