fix: pre-landing review fixes (Step 11: unique UID proof, protected paths never declared test code)

Review-Finding: v1/outside-voice#1 reader=outside-voice
Review-Finding: v1/outside-voice#2 reader=outside-voice
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-06 11:30:18 +00:00
1 parent b0d38491b4
commit c25c5de08b
5 files changed
+25 -5

No files matched your search

+4 -1
View File
@@ -70,7 +70,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
| `release_gate` | `{ cadence_hours, gate_workflow, branch, stable_ref, check_name, on_red, bisect_spend_cap_usd }` or `null` | `null` | One full paid gate on main every `cadence_hours` (8 to 168) instead of the per-push main backstop, feeding the stable release; see [release_gate](#release-gate). Every field is required: `on_red` is `hold-release`, `bisect_spend_cap_usd` 0 to 500 (0 skips the bisect). `null` keeps the per-push backstop. Needs `min_gstack_version`. |
| `bookkeeping` | glob list | `[]` | <a id="bookkeeping"></a>Root-anchored globs of bookkeeping tables a verify or ratchet step rewrites (size baselines, serial lists, limits), each ending in `.tsv`, `.csv` or `.txt` and never under `.gstack/` or `.github/` (`["scripts/*-baseline.tsv"]`). A matching file changed at most 20 lines after review binds, first touched or not, takes finish's [one scoped reader](review-policy.md#bounded-post-review-edits) instead of Steps 5–11.5; the reader still reads every changed row. Read from the base revision only. Needs `min_gstack_version`. |
| `mechanical_retake` | `[{ path, template, fields, regen }]` | `[]` | A version or rules constant a branch may have to retake after a base merge, next to `bookkeeping`'s bounded kind: `path` is one literal file, `template` the whole line with one `{n}` (`const RULE_NUMBERS := {n}`), `fields` the `{ glob, field }` fixture fields that carry the same value, and `regen` the [`.gstack/generated.json`](pregate.md#generated) entry ids whose command regenerates its goldens. A post-review edit finish proves to be only that retake routes as `mechanical=`, with no reader and no review round; see [mechanical retakes](#mechanical-retake). At most 8 constants and 16 fields or ids each; no glob, `..`, `.gstack/` or `.github/` path. Read from the base revision only. Needs `min_gstack_version`. |
| `test_code` | glob list | `[]` | Root-anchored globs of test code the built-in test names miss, never under `.gstack/` or `.github/` (`["tests/**/*.gd"]` for GDScript checks). A matching file changed after review routes as test code, one scoped reader of its test hunks, instead of behavior, beside the built-in globs; shared `helpers/` and `fixtures/` files stay behavior. See [test code and Godot UID sidecars](#test-code). Read from the base revision only. Needs `min_gstack_version`. |
| `test_code` | glob list | `[]` | Root-anchored globs of test code the built-in test names miss, never under `.gstack/` or `.github/`, and never matching a file there (`["tests/**/*.gd"]` for GDScript checks). A matching file changed after review routes as test code, one scoped reader of its test hunks, instead of behavior, beside the built-in globs; shared `helpers/` and `fixtures/` files stay behavior. See [test code and Godot UID sidecars](#test-code). Read from the base revision only. Needs `min_gstack_version`. |
| `base_red` | `{ setup, select, timeout_sec }` or `null` | `null` | How `/ship` Step 5 proves a failure is the base branch's own before pausing on it; see [base_red](#base-red). Read from the base revision only. Needs `min_gstack_version`. |
| `changelog_fragments` | `{ dir, required_checks? }` or `null` | `null` | PRs carry unversioned [CHANGELOG fragments](#changelog-fragments) in `dir` (one flat, repo-relative directory, no `..`) and the merge step ([`land`](#land)) assigns the version (under `merge: direct`, the [release PR](#release-stamp) does). `required_checks` (up to 50 distinct names) are the checks [`train land`](#stamped-trains-under-changelog_fragments-train-land) requires present and green on a stamp commit. `null` keeps `[Unreleased]` sections and per-PR stamps. Needs `min_gstack_version`. |
@@ -1857,6 +1857,9 @@ reader, when all of these hold:
whole content is `uid://<id>` with an optional trailing `\n`: the id
canonical (no leading `a` digit unless it is `a`), at most 13 digits and at
most 2^63−1 (`d4n4ub6itg400`);
- no other file in the current tree names the same `uid://<id>`: a copied
sidecar would give two resources one UID, and a scene that already refers
to the id leaves the `.uid` to a reader;
- neither it nor its sibling matches the base policy's
[`review_exclude_paths`](#review-exclude-paths) or `readers.forbid_paths`,
since no reader saw an excluded script.
+2 -1
View File
@@ -3208,7 +3208,8 @@ compared with the reviewed range`, `has no sibling <path> that the reviewed
range added or modified or the review-loop exception accepted`, `has no
sibling <path> at the current tree`, `existed at the reviewed tree or the
current base`, `is not a regular non-executable file (mode <mode>)`, `is not
exactly one Godot uid://<id> line`, or `or its sibling matches the base
exactly one Godot uid://<id> line`, `shares its UID with <paths>`, `could not
be searched for its UID in the current tree`, or `or its sibling matches the base
policy's review_exclude_paths or readers.forbid_paths`.
**Fix.** Commit the sidecar exactly as Godot wrote it, beside a script this PR
+3 -2
View File
@@ -223,11 +223,12 @@ export function packetBrief(): string {
* A test code file outside shared helpers and fixtures: `*.test.*`, `*.spec.*` and `*_test.go` anywhere, and the
* name-only conventions (`test_*.py`, `*Test.java`, `*Tests.swift`, …) only under a test directory, since product
* files can carry those names (`src/main/java/ABTest.java`). A path the base policy's `test_code` globs declare counts too
* (GDScript's `tests/*_check.gd`, GST-1604). A change to one gets the scoped review, not the full loop.
* (GDScript's `tests/*_check.gd`, GST-1604), never one under `.gstack/` or `.github/`. A change to one gets the scoped review,
* not the full loop.
*/
export function isTestCode(rel: string, declared: readonly string[] = []): boolean {
if (/(^|\/)(helpers|fixtures|__fixtures__|__snapshots__)\//.test(rel)) return false;
if (matchesAny(declared, rel)) return true;
if (!/^\.(gstack|github)\//.test(rel) && matchesAny(declared, rel)) return true;
if (!matchesAny(CARRY_TEST_CODE_GLOBS, rel)) return false;
return /\.(test|spec)\.[^/]+$|_test\.go$/.test(rel) || /(^|\/)(test|tests|spec|__tests__|src\/test|[^/]+Tests)\//.test(rel);
}
+12 -1
View File
@@ -9,7 +9,8 @@
* the script for every uid:// reference to it);
* - 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;
* - it is a regular, non-executable file whose whole content is exactly what Godot writes for one id, and no other file
* in the current tree names that uid:// (a copied sidecar gives two resources one UID);
* - 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.
@@ -48,6 +49,13 @@ function git(cwd: string, args: string[]): string | null {
return r.status === 0 ? r.stdout : null;
}
/** The other files in `tree` that name `uid` as a whole word; null when git cannot search it. */
function holders(cwd: string, tree: string, uid: string, self: string): string[] | null {
const r = spawnSync('git', ['-c', 'core.quotepath=off', 'grep', '-z', '-l', '-w', '-F', '-e', uid, tree], { cwd, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 });
if (r.status !== 0 && r.status !== 1) return null;
return r.stdout.split('\0').filter(Boolean).map(l => l.slice(tree.length + 1)).filter(p => p !== self);
}
/** The mode and blob of `rel` in `tree`; null when absent or unreadable. */
function entry(cwd: string, tree: string, rel: string): { mode: string; blob: string } | null {
const m = /^(\d{6}) \w+ ([0-9a-f]{40,64})\t/.exec(git(cwd, ['--literal-pathspecs', 'ls-tree', '-z', tree, '--', rel]) ?? '');
@@ -79,6 +87,9 @@ export function uidSidecars(cwd: string, i: SidecarInput): { proven: Record<stri
if (e?.mode !== '100644') { refuse(`is not a regular non-executable file (mode ${e?.mode ?? 'unreadable'})`); continue; }
const text = git(cwd, ['cat-file', 'blob', e.blob]);
if (text === null || !isGodotUidText(text)) { refuse('is not exactly one Godot uid://<id> line'); continue; }
const shared = holders(cwd, s.current, text.trim(), p);
if (shared === null) { refuse('could not be searched for its UID in the current tree'); continue; }
if (shared.length) { refuse(`shares its UID with ${shared.sort().join(', ')}`); 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;
}
+4
View File
@@ -105,6 +105,7 @@ describe('a Godot UID sidecar added after the fix-check (GST-1604: founderstar#6
[SIDECAR, 'is not a regular non-executable file (mode 120000)', f => fs.symlinkSync('cutscene_check.gd', path.join(f.work, SIDECAR))],
[SIDECAR, 'is not a regular non-executable file (mode 100755)', f => { f.write({ [SIDECAR]: UID }); fs.chmodSync(path.join(f.work, SIDECAR), 0o755); }],
['scripts/ui/cutscene.gd.uid', 'existed at the reviewed tree or the current base', f => f.write({ 'scripts/ui/cutscene.gd.uid': 'uid://b4w2kq0ly8s5n\n' }), { 'scripts/ui/cutscene.gd.uid': UID }],
[SIDECAR, 'shares its UID with scripts/old.gd.uid', f => f.write({ [SIDECAR]: UID }), { 'scripts/old.gd': 'extends Node\n', 'scripts/old.gd.uid': UID }],
];
for (const [p, why, after, base, feat] of cases) {
const r = founderstar611({ after, base, feat }).run();
@@ -141,6 +142,9 @@ describe('base-declared test_code (GST-1604: GDScript tests)', () => {
expect(isTestCode('tests/helpers/fake_input.gd', ['tests/**/*.gd'])).toBe(false);
expect(isTestCode('scripts/ui/cutscene.gd', ['tests/**/*.gd'])).toBe(false);
expect(isTestCode('src/a.test.ts', ['tests/**/*.gd'])).toBe(true);
expect(isTestCode('.github/workflows/release.yml', ['**/*.yml'])).toBe(false);
expect(isTestCode('.gstack/ship-policy.yml', ['**/*.yml'])).toBe(false);
expect(isTestCode('tests/fixtures.yml', ['**/*.yml'])).toBe(true);
});
test('the key validates as a glob list behind min_gstack_version, never under .gstack/ or .github/', () => {