From 5e63f0f8ea3137a88c9ee0526ef6a31b1bbf1000 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 28 Oct 2026 19:40:07 +0000 Subject: [PATCH] fix: pre-landing review fixes Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- docs/pregate.md | 4 ++-- lib/sealed-guard.ts | 8 +++----- test/pregate-guards.test.ts | 2 +- test/sealed-guard.test.ts | 10 ++++------ 5 files changed, 12 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f63370d85..a1615bc83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,12 @@ **A sealed policy can now refuse `sealed//tasks.json` while allowing the `MANIFEST.json` beside it, and can catch `HELDOUT/` with a `heldout` glob.** The sealed guard only had deny globs and one-commit waivers, so the gstack-evals layout, which keeps a public manifest of hashes and counts next to its sealed tasks, needed a fresh `.gstack/sealed-allow.tsv` line for every manifest commit (gstack-evals wiki/board/2026-10-28-gsta-9-sealed-manifest-glob-gap.md). Their first draft also wrote `sealed/**`, which never matched their nested `tasks//sealed/` directories, and garrytan/gstack-evals#3 (dc54796) hand-listed `Sealed/`, `HELDOUT/` and other case variants. ### Added -- **`sealed.allow_paths`** (`lib/sealed-guard.ts`): globs that exempt publishable files from `sealed.path` and `sealed.transcript`, applied after the deny globs and before sha waivers. They are read from the destination base branch only, so a push cannot widen its own exceptions; a pushed copy is still validated and refuses when malformed. Text rules still read an exempt file, so a manifest that quotes a sealed answer still refuses. An entry whose last segment is only wildcards (`**`, `sealed/*`) is refused. Each exempt path prints `exempt commit (sealed.allow_paths )` (at most 20 lines, then a count). +- **`sealed.allow_paths`** (`lib/sealed-guard.ts`): globs that exempt publishable files from `sealed.path` and `sealed.transcript`, applied after the deny globs and before sha waivers. They are read from the destination base branch only, so a push cannot widen its own exceptions; a pushed copy is still validated and refuses when malformed. Text rules still read an exempt file, so a manifest that quotes a sealed answer still refuses. An entry with a wildcard file name (`**`, `sealed/*`, `**/*.json`) is refused. Each exempt path prints `exempt commit (sealed.allow_paths )` on every push. - **`sealed.case_insensitive: true`**: `paths` and `transcripts` match regardless of case, so `**/heldout/**` also refuses `HELDOUT/` and `HeldOut/`. The value is the base branch's OR the pushed one, because it only tightens. `allow_paths` never fold case. Separator and spelling variants such as `hold-out/` still need their own globs. ### Changed - `gstack-pregate sealed` prints `exempt=` after `waived=` on its `SEALED:` line (`policy=` stays last), and `--json` carries an `exempt` array. Both bins build their error report through one `sealedErrorReport`. -- An unknown `sealed` key now says it may come from a newer gstack. A gstack older than this release refuses `allow_paths` and `case_insensitive` this way on every push and `gstack-pregate run`, so a repository raises `min_gstack_version` before landing either key. +- An unknown `sealed` key now says to upgrade gstack if the key comes from a newer one. A gstack older than this release refuses `allow_paths` and `case_insensitive` this way on every push and `gstack-pregate run`, so a repository raises `min_gstack_version` before landing either key. - `docs/pregate.md` ยง Sealed output explains glob anchoring: patterns match from the repository root, so `sealed/**` misses `tasks/x/sealed/a.json` and the docs say to write `**/sealed/**`. It also documents case handling, exceptions and the version skew. ### Gate integrity diff --git a/docs/pregate.md b/docs/pregate.md index 79c671854..337cbe623 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -805,8 +805,8 @@ gstack-pregate sealed [--range ] [--remote origin] [--base ] [--js `sealed.transcript`, applied after the deny globs and before waivers. They are read from the destination base branch only: an exception a push adds takes effect once it lands on the base branch, so a branch can never widen - its own. They never fold case, and an entry whose last segment is only - wildcards (`**`, `sealed/*`) is refused. An exempt file is checked by the + its own. They never fold case, and an entry with a wildcard file name + (`**`, `sealed/*`, `**/*.json`, `sealed/**.json`) is refused: name the file. An exempt file is checked by the text rules only, so configure `answer_markers` when you exempt files next to sealed data. Each exempt path is listed on every push (`exempt commit (sealed.allow_paths )`) and counted as diff --git a/lib/sealed-guard.ts b/lib/sealed-guard.ts index f7bdbe526..6dd1572fa 100644 --- a/lib/sealed-guard.ts +++ b/lib/sealed-guard.ts @@ -50,14 +50,13 @@ const MAX_REGEX = 256; const LOCAL_PATH_RE = /(?:^|[^A-Za-z0-9_.~-])(?:\/home\/[^/\s'"`]+\/|\/Users\/[^/\s'"`]+\/|[A-Za-z]:\\+Users\\+[^\\\s'"`]+\\)/; const PUBLIC_TTL_MS = 24 * 3600_000; const PRIVATE_TTL_MS = 3600_000; -const MAX_EXEMPT_LINES = 20; /** `: ` errors for a `sealed` value; empty when valid. Shared with lib/pregate/config.ts. */ export function sealedErrors(raw: unknown): Array<{ path: string; message: string }> { const errors: Array<{ path: string; message: string }> = []; if (!raw || typeof raw !== 'object' || Array.isArray(raw)) return [{ path: 'sealed', message: `must be an object { ${KEYS.join(', ')} }` }]; const o = raw as Record; - for (const k of Object.keys(o)) if (!KEYS.includes(k)) errors.push({ path: `sealed.${k}`, message: `unknown key; valid keys: ${KEYS.join(', ')} (a key from a newer gstack: upgrade gstack on this machine)` }); + for (const k of Object.keys(o)) if (!KEYS.includes(k)) errors.push({ path: `sealed.${k}`, message: `unknown key; valid keys: ${KEYS.join(', ')} (if the key comes from a newer gstack, upgrade gstack on this machine)` }); const list = (key: string, check: (s: string) => string | null) => { if (!(key in o)) return; const v = o[key]; @@ -67,7 +66,7 @@ export function sealedErrors(raw: unknown): Array<{ path: string; message: strin }; const glob = (g: string) => { const why = containedRelative(g); if (why) return why; try { globToRegExp(g); return null; } catch (e: any) { return e.message; } }; const regex = (r: string) => { if (!r || r.length > MAX_REGEX) return `must be 1-${MAX_REGEX} characters`; try { new RegExp(r); return null; } catch (e: any) { return `is not a valid regex (${e.message})`; } }; - const exception = (g: string) => glob(g) ?? (/^[*?]+$/.test(g.split('/').pop()!) ? 'ends in a wildcard-only segment, which would exempt a whole directory; name the file (e.g. **/sealed/*/MANIFEST.json)' : null); + const exception = (g: string) => { const name = g.split('/').pop()!; return glob(g) ?? (name.includes('**') || /^[*?]+(\.|$)/.test(name) ? 'has a wildcard file name, which would exempt whole directories or file types; name the file (e.g. **/sealed/*/MANIFEST.json)' : null); }; list('paths', glob); list('transcripts', glob); list('answer_markers', regex); list('seed_patterns', regex); list('allow_paths', exception); for (const k of ['local_paths', 'case_insensitive']) if (k in o && typeof o[k] !== 'boolean') errors.push({ path: `sealed.${k}`, message: 'must be true or false' }); if ('visibility' in o && o.visibility !== 'public' && o.visibility !== 'private') errors.push({ path: 'sealed.visibility', message: 'must be "public" or "private"' }); @@ -286,8 +285,7 @@ const where = (f: SealedFinding) => `${f.file || '(unknown file)'}${f.line ? `:$ /** stderr text for a report: findings one per line (` commit `), never the matched text. */ export function renderSealed(r: SealedReport, context: string): string { const lines: string[] = []; - for (const e of r.exempt.slice(0, MAX_EXEMPT_LINES)) lines.push(`${context}: exempt ${e.rule} ${where(e)} commit ${e.commit.slice(0, 12)} (sealed.allow_paths ${e.glob})`); - if (r.exempt.length > MAX_EXEMPT_LINES) lines.push(`${context}: and ${r.exempt.length - MAX_EXEMPT_LINES} more exempt path(s) (sealed.allow_paths)`); + for (const e of r.exempt) lines.push(`${context}: exempt ${e.rule} ${where(e)} commit ${e.commit.slice(0, 12)} (sealed.allow_paths ${e.glob})`); for (const w of r.waived) lines.push(`${context}: waived ${w.rule} ${where(w)} commit ${w.commit.slice(0, 12)} (${SEALED_ALLOW_PATH}: ${w.reason})`); const vis = `visibility: ${r.visibility}${r.visibility === 'unknown' ? ', counted public' : ''}; ${r.visibilitySource}`; const body = r.findings.map(f => ` ${f.rule} ${where(f)} commit ${f.commit.slice(0, 12)}`); diff --git a/test/pregate-guards.test.ts b/test/pregate-guards.test.ts index 33f1a6fb8..bf0e65bf4 100644 --- a/test/pregate-guards.test.ts +++ b/test/pregate-guards.test.ts @@ -197,7 +197,7 @@ describe('pregate config: sealed (CD4)', () => { expect(codes({ sealed: { allow_paths: '**/sealed/*/MANIFEST.json' } })).toEqual(['PREGATE_CONFIG_INVALID:sealed.allow_paths']); expect(codes({ sealed: { allow_paths: ['../x/MANIFEST.json'] } })).toEqual(['PREGATE_CONFIG_INVALID:sealed.allow_paths[0]']); expect(codes({ sealed: { allow_paths: Array.from({ length: 65 }, (_, i) => `s${i}/MANIFEST.json`) } })).toEqual(['PREGATE_CONFIG_INVALID:sealed.allow_paths']); - expect(codes({ sealed: { allow_paths: ['**', 'sealed/**', 'sealed/*', '**/sealed/*/MANIFEST.json'] } })).toEqual(['PREGATE_CONFIG_INVALID:sealed.allow_paths[0]', 'PREGATE_CONFIG_INVALID:sealed.allow_paths[1]', 'PREGATE_CONFIG_INVALID:sealed.allow_paths[2]']); + expect(codes({ sealed: { allow_paths: ['**', 'sealed/**', 'sealed/*', '**/*.json', 'sealed/*.*', '**/sealed/**.json', '**/sealed/*/MANIFEST.json', '**/sealed/*/transcript-*.jsonl', '.hidden'] } })).toEqual([0, 1, 2, 3, 4, 5].map(i => `PREGATE_CONFIG_INVALID:sealed.allow_paths[${i}]`)); expect(codes({ sealed: { case_insensitive: 'yes' } })).toEqual(['PREGATE_CONFIG_INVALID:sealed.case_insensitive']); }); }); diff --git a/test/sealed-guard.test.ts b/test/sealed-guard.test.ts index 982c63b31..a9f044f6d 100644 --- a/test/sealed-guard.test.ts +++ b/test/sealed-guard.test.ts @@ -17,7 +17,6 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { resolveVisibility } from '../lib/sealed-guard'; -import { matchesAny } from '../lib/ship-policy'; const ROOT = path.resolve(import.meta.dir, '..'); const PREPUSH = path.join(ROOT, 'bin', 'gstack-redact-prepush'); @@ -322,15 +321,14 @@ describe('root-anchoring (the docs/pregate.md example)', () => { const docs = fs.readFileSync(path.join(ROOT, 'docs', 'pregate.md'), 'utf8'); expect(docs).toContain('`sealed/**` misses `tasks/x/sealed/a.json`'); expect(docs).toContain('write `**/sealed/**`'); - expect(matchesAny(['sealed/**'], 'tasks/x/sealed/a.json')).toBe(false); - expect(matchesAny(['**/sealed/**'], 'tasks/x/sealed/a.json')).toBe(true); - expect(matchesAny(['**/sealed/**'], 'sealed/a.json')).toBe(true); const anchored = scratch({ '.gstack/pregate.json': policy({ paths: ['sealed/**'] }) }); commit(anchored, { 'tasks/x/sealed/a.json': '{}\n' }); expect(hook(anchored).code).toBe(0); const any = scratch({ '.gstack/pregate.json': policy({ paths: ['**/sealed/**'] }) }); - commit(any, { 'tasks/x/sealed/a.json': '{}\n' }); - expect(hook(any).err).toContain('sealed.path tasks/x/sealed/a.json'); + commit(any, { 'tasks/x/sealed/a.json': '{}\n', 'sealed/b.json': '{}\n' }); + const a = hook(any); + expect(a.err).toContain('sealed.path tasks/x/sealed/a.json'); + expect(a.err).toContain('sealed.path sealed/b.json'); }); });