diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 255999778..10ef467c5 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1858,8 +1858,8 @@ reader, when all of these hold: canonical (no leading `a` digit unless it is `a`), at most 13 digits and at most 2^63−1 (`d4n4ub6itg400`); - neither it nor its sibling matches the base policy's - [`review_exclude_paths`](#review-exclude-paths), since no reader saw an - excluded script. + [`review_exclude_paths`](#review-exclude-paths) or `readers.forbid_paths`, + since no reader saw an excluded script. Each proven sidecar prints `UID_SIDECAR: (beside )`. Any other `.uid` keeps its old route, usually behavior, and `finish` prints diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index dcac95c98..4d4c1fbdf 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3209,7 +3209,7 @@ range added or modified or the review-loop exception accepted`, `has no sibling at the current tree`, `existed at the reviewed tree or the current base`, `is not a regular non-executable file (mode )`, `is not exactly one Godot uid:// line`, or `or its sibling matches the base -policy's review_exclude_paths`. +policy's review_exclude_paths or readers.forbid_paths`. **Fix.** Commit the sidecar exactly as Godot wrote it, beside a script this PR added or changed: one `uid://` line, a regular file, no symlink. Keep a diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 1f8f458d4..092d86d43 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -22,6 +22,7 @@ import { mechanicalPaths, writeMechanicalRecord } from './ship-mechanical'; import { uidSidecars } from './ship-sidecar'; import type { MechanicalConstant } from './ship-policy-mechanical'; import { reviewFreshness } from './review-evidence'; +import { forbidSpecs } from './review-fetch'; import { readRuledNotes, ruledNoteText } from './ruled-notes'; import { parseReviewLoopException, type ReviewLoopException } from './ship-receipt-depth'; import { parseReceiptDocs, type ReceiptDocs } from './ship-receipt-docs'; @@ -375,7 +376,7 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: Review * version, each against the audited bytes; GST-906), and --docs-requested fixes the audit's own result asked for do not * stale the audit (GST-391). */ -function docsFresh(cwd: string, base: string, postFile: string, moved: readonly string[], requested: string[], resultFile: string | undefined, loopAccepted: readonly string[] | undefined, parent: string | undefined, r: Report): boolean { +function docsFresh(cwd: string, base: string, postFile: string, moved: readonly string[], requested: string[], resultFile: string | undefined, loopAccepted: readonly string[] | undefined, parent: string | undefined, testCode: readonly string[], r: Report): boolean { const cmp = bin(cwd, 'gstack-docs-candidate', ['compare', postFile], 120_000); let stale: string[] | null = null; try { stale = JSON.parse(cmp.stdout).content_changed; } catch { stale = null; } @@ -428,7 +429,7 @@ function docsFresh(cwd: string, base: string, postFile: string, moved: readonly const merged = mergeChangelogs(audited.stdout, atNowBase.stdout, atAuditBase.stdout); return kept !== null && merged !== null && normalizeChangelog(kept) === normalizeChangelog(audited.stdout) && normalizeChangelog(merged) === normalizeChangelog(now); }); - const carried = { tests: stale.filter(p => isTestCode(p)), base: stale.filter(p => !isTestCode(p) && (moved.includes(p) || restamped.includes(p) || (!!auditBase && !!nowBase && !branchAtAudit.has(p) && !branchNow.has(p)) + const carried = { tests: stale.filter(p => isTestCode(p, testCode)), base: stale.filter(p => !isTestCode(p, testCode) && (moved.includes(p) || restamped.includes(p) || (!!auditBase && !!nowBase && !branchAtAudit.has(p) && !branchNow.has(p)) || (!!auditBase && !!nowBase && /^[0-9a-f]{40,64}$/.test(nowTree) && ![...record.staged, ...record.unstaged, ...record.untracked].includes(p) && comparePath(cwd, { reviewedBase: auditBase, reviewed: record.head, currentBase: nowBase, current: nowTree }, p) === 'same'))), requested: [] as string[], accepted: [] as string[] }; carried.accepted = stale.filter(p => loopAccepted?.includes(p) && !carried.tests.includes(p) && !carried.base.includes(p)); let audit: any = null; @@ -495,15 +496,16 @@ async function finishStages(argv: readonly string[], cwd: string): Promise Array.isArray(policy?.[key]) ? policy![key].filter((g: unknown): g is string => typeof g === 'string') : []; const bookkeeping = globs('bookkeeping'); + const forbidden = forbidSpecs({ forbid_paths: Array.isArray(policy?.readers?.forbid_paths) ? policy!.readers.forbid_paths.filter((g: unknown): g is string => typeof g === 'string') : [] }).map(s => s.slice(':(top,exclude,glob)'.length)); const mechanical: MechanicalConstant[] = Array.isArray(policy?.mechanical_retake) ? policy!.mechanical_retake : []; const reviewedBaseArg = flag('--reviewed-base'); const fragments = typeof policy?.changelog_fragments?.dir === 'string' ? [`:(top,exclude,glob)${policy.changelog_fragments.dir.replace(/\/+$/, '')}/*.md`] : []; const reviewedBase = reviewedBaseArg === undefined ? undefined : reviewedBaseCommit(cwd, `origin/${base.replace(/^origin\//, '')}`, reviewedBaseArg, [...releasePaths(policy, cwd, flag('--reviewed-tree')!, base, true).map(p => `:(top,exclude,literal)${p}`), ...fragments], r); if (reviewedBase === null) return r; const baseRelease = reviewedBase?.parent ? releasePaths(policy, cwd, git(cwd, ['merge-base', `origin/${base.replace(/^origin\//, '')}`, 'HEAD']).stdout.trim(), base, true) : undefined; - const routed = route(cwd, base, flag('--reviewed-tree')!, reviewedBase, release, { docsRechecked: has('--docs-rechecked'), contextReviewed: flag('--context-reviewed'), loop, audited, bookkeeping, mechanical, testCode: globs('test_code'), excluded: globs('review_exclude_paths'), baseRelease }, r); + const routed = route(cwd, base, flag('--reviewed-tree')!, reviewedBase, release, { docsRechecked: has('--docs-rechecked'), contextReviewed: flag('--context-reviewed'), loop, audited, bookkeeping, mechanical, testCode: globs('test_code'), excluded: [...globs('review_exclude_paths'), ...forbidden], baseRelease }, r); if (!routed) return r; - if (!docsFresh(cwd, base, flag('--docs-post')!, [...routed.base, ...routed.context, ...routed.mechanical, ...stampedFiles(policy, cwd, flag('--reviewed-tree')!, base)], all('--docs-requested'), flag('--docs-result'), loop ? routed.accepted : undefined, reviewedBase?.parent ? reviewedBase.commit : undefined, r)) return r; + if (!docsFresh(cwd, base, flag('--docs-post')!, [...routed.base, ...routed.context, ...routed.mechanical, ...stampedFiles(policy, cwd, flag('--reviewed-tree')!, base)], all('--docs-requested'), flag('--docs-result'), loop ? routed.accepted : undefined, reviewedBase?.parent ? reviewedBase.commit : undefined, globs('test_code'), r)) return r; if (!list.length && !has('--no-lanes-approved')) return r.stop('no test lanes: require Step 5\'s explicit untested-scope approval (rerun with --no-lanes-approved) or run Steps 5-15'); const allow = evidenceExempt(release).join(','); const check = (l: { label: string; command: string }) => bin(cwd, 'gstack-evidence', ['check', '--label', l.label, '--expect-cmd', l.command, '--max-age', '24', '--allow-paths', allow], 60_000).stdout; diff --git a/lib/ship-sidecar.ts b/lib/ship-sidecar.ts index b479237dc..1dffd1f2a 100644 --- a/lib/ship-sidecar.ts +++ b/lib/ship-sidecar.ts @@ -10,7 +10,8 @@ * - its sibling (the path without `.uid`) is a regular file now, and one the reviewed range added or modified, or one * the review-loop exception accepted; * - it is a regular, non-executable file whose whole content is exactly what Godot writes for one id; - * - neither it nor its sibling matches the base policy's review_exclude_paths (a sibling no reader saw is not reviewed). + * - neither it nor its sibling matches the base policy's review_exclude_paths or readers.forbid_paths (a sibling no reader + * saw is not reviewed). * Any other `.uid` is refused with the reason and stays behavior; a `.import` file is never a candidate. */ import { spawnSync } from 'node:child_process'; @@ -22,7 +23,7 @@ export interface SidecarInput { candidates: readonly string[]; /** Paths the review-loop exception accepted. */ accepted: readonly string[]; - /** The base policy's review_exclude_paths (git glob pathspecs). */ + /** The git glob pathspecs no reader saw: the base policy's review_exclude_paths and readers.forbid_paths. */ excluded: readonly string[]; } export interface SidecarRefusal { path: string; reason: string } @@ -43,7 +44,7 @@ export function isGodotUidText(text: string): boolean { export const sidecarLine = (p: string, sibling: string) => `Godot UID sidecar: ${p} (beside ${sibling})`; function git(cwd: string, args: string[]): string | null { - const r = spawnSync('git', ['-c', 'core.quotepath=off', ...args], { cwd, encoding: 'latin1', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); + const r = spawnSync('git', ['-c', 'core.quotepath=off', ...args], { cwd, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; } @@ -78,7 +79,7 @@ export function uidSidecars(cwd: string, i: SidecarInput): { proven: Record line'); continue; } - if (excluded.has(p) || excluded.has(sibling)) { refuse('or its sibling matches the base policy\'s review_exclude_paths, so no reader saw the script'); continue; } + if (excluded.has(p) || excluded.has(sibling)) { refuse('or its sibling matches the base policy\'s review_exclude_paths or readers.forbid_paths, so no reader saw the script'); continue; } proven[p] = sibling; } return { proven, refused }; diff --git a/test/ship-uid-sidecar.test.ts b/test/ship-uid-sidecar.test.ts index 7bd95e3a6..7febc4c3a 100644 --- a/test/ship-uid-sidecar.test.ts +++ b/test/ship-uid-sidecar.test.ts @@ -13,6 +13,7 @@ import * as path from 'node:path'; import { readMechanicalLines } from '../lib/ship-mechanical'; import { isTestCode } from '../lib/ship-patch'; import { REVIEW_EXCLUDE_KEY_VERSION, TEST_CODE_KEY_VERSION, validatePolicy } from '../lib/ship-policy'; +import { READERS_KEY_VERSION } from '../lib/ship-policy-keys'; import { isGodotUidText } from '../lib/ship-sidecar'; import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; @@ -120,6 +121,14 @@ describe('a Godot UID sidecar added after the fix-check (GST-1604: founderstar#6 expect([imported.status, lineOf(imported.stdout, 'STOP')]).toEqual([3, behaviorStop('tests/icon.png.import')]); expect(imported.stdout).not.toMatch(/UID_SIDECAR|ROUTE_SIDECAR_REFUSED/); }); + + test('a sidecar beside a readers.forbid_paths script, or beside an excluded script with a non-ASCII name, refuses too', () => { + const forbidden = founderstar611({ base: { '.gstack/ship-policy.json': JSON.stringify({ readers: { forbid_paths: ['tests'] }, min_gstack_version: READERS_KEY_VERSION }) + '\n' } }).run(); + expect([forbidden.status, lineOf(forbidden.stdout, 'ROUTE_SIDECAR_REFUSED')]).toEqual([3, expect.stringContaining(`ROUTE_SIDECAR_REFUSED: ${SIDECAR} or its sibling matches`)]); + const accented = founderstar611({ base: { '.gstack/ship-policy.json': JSON.stringify({ review_exclude_paths: ['tests/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n' }, feat: { 'tests/menü_check.gd': CHECK }, after: f => f.write({ 'tests/menü_check.gd.uid': UID }) }).run(); + expect([accented.status, lineOf(accented.stdout, 'ROUTE_SIDECAR_REFUSED')]).toEqual([3, expect.stringContaining('ROUTE_SIDECAR_REFUSED: tests/menü_check.gd.uid or its sibling matches')]); + expect(accented.stdout).not.toContain('UID_SIDECAR:'); + }); }); describe('base-declared test_code (GST-1604: GDScript tests)', () => {