Skip to content

Commit 36b20ca

Browse files
feat(scripts): refuse an undeclared mode-160000 gitlink in the index (#18414)
Part of #17472. Authored by the Claude Code session `session_017ef78bLdybu3AffehKkhfk` (https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk). Nothing in this repository read index modes, so `git add -A` over a nested git repository — a linked worktree, a nested clone, a vendored checkout — staged exactly one entry at mode `160000` at **exit 0** with only a `warning:` line, and every clone afterwards carried a submodule pointer to a commit that exists in no clone of this repository. This adds the gate that refuses it. ⛔ This is **not** the untracked-signal half; PR #17468 owns that one and closed it by ignoring `.worktrees/`. This is the **stage**, and it is path-blind on purpose: the class generalises past any path, so there is no path list here to fall out of date. ## The shape chosen, and the ones rejected `scripts/check-gitlink-declared.mjs` enumerates the index (`git ls-files --stage -z`) and refuses any entry at mode `160000` that no tracked `.gitmodules` row declares. Root `package.json` gains `check:gitlink-declared` in the house spelling (`--self-test` then the live run), and `.github/workflows/lint.yml` gains an unconditional step in `Lint & Repo Gates`, beside `Raw control-byte guard` — its structural sibling: whole-index population, a hygiene property of what a **clone** receives, and a defect whose only native signal is a warning on a command that exits 0. Four decisions, each with the alternative it beat: 1. **"undeclared", not "no gitlinks at all".** A flat ban is the stronger rule and was rejected: it bans the legitimate case along with the accident, and the two are told apart by a fact every repository with a real submodule already writes down. `git submodule add` writes the declaration and the pointer in one act, so a real submodule passes the day it is added — no exemption, no allowlist, no flag. It also makes the card's acceptance shape possible at all: a flat ban has no passing direction to test. 2. **The declaration is read out of the INDEX**, via `git config --blob :.gitmodules`, not off the working tree. A `.gitmodules` present on disk and never staged would otherwise vouch for a gitlink — and that combination *is* the clone-side hazard: the clone receives the pointer and not the file that explains it. A declaration that does not travel with the commit declares nothing. The self-test pins this direction separately. 3. **Refusing from day one, with the report half as `--list`.** The card suggested "report-only, then refusing". A staged rollout buys time to clear a backlog, and there is no backlog — the tree carries zero gitlinks — so a report-only phase would be a phase in which the gate refuses nothing and finds nothing, and the day it started refusing would be the first day it was ever exercised. `--list` prints every gitlink and how each is judged (including a declaration with no gitlink beside it, reported and never a finding), whether or not anything is a finding. 4. **No `.githooks/pre-commit` wiring.** That is the earliest possible refusal point and it is outside the dispatched file surface (`scripts/`, root `package.json`, `.github/workflows/lint.yml`). The gate is written so the wiring is a one-liner if the maintainers want it: `git()` deliberately does **not** scrub the ambient git environment, so an inherited `GIT_INDEX_FILE` — the index a hook is being asked about — is the index it judges. No new runtime dependency: `git` itself is the `.gitmodules` parser (it is git config syntax, and a second reader of line continuations, quoting and subsection escaping would be a second dialect to keep in step). **The manual floor was not hit.** ## Self-test, both directions, as output The gate ships a two-direction self-test over throwaway repositories under a temp dir — never inside this checkout — driven through the same `scan()` the live run calls. ``` $ node scripts/check-gitlink-declared.mjs --self-test ✓ check-gitlink-declared --self-test: 36 assertions over throwaway git repos (real scan() path) exit=0 ``` Its forward fixture is the card's own measurement, reproduced with a literal `git add -A` rather than a hand-assembled index, and the fixture is asserted before the gate is (a run in which `git add -A` staged nothing would otherwise look exactly like a gate that works). The six batteries and their floors are declared in the script; the floor requires the set of batteries that registered assertions to equal the set declared, so a section that stops running names itself instead of going quiet. The same two directions on the **production** path — the script's own `main()`, its failure text and its exit code — measured in a throwaway repo outside every checkout: ``` $ git add -A # the card's own measurement warning: adding embedded git repository: vendor/thing hint: You've added another git repository inside your current repository. exit=0 $ git ls-files --stage 100644 45b983be36b73c0788dc9cbcb76cbb80fc7bb057 0 readme.md 160000 4707cf6e9b8a1d7cda7df17d731cd4b7066b300d 0 vendor/thing DIRECTION 2 -- a bare gitlink fails $ node scripts/check-gitlink-declared.mjs check-gitlink-declared: 1 index entry is a gitlink that .gitmodules does not declare • vendor/thing -- mode 160000, commit 4707cf6e A mode-160000 index entry is a SUBMODULE POINTER. Staging a nested git ... (remedy text elided here; it is in the script) exit=1 DIRECTION 1 -- the same index, plus the row `git submodule add` writes $ node scripts/check-gitlink-declared.mjs check-gitlink-declared: OK (3 index entries -- 1 gitlink(s) at mode 160000; 1 submodule path(s) declared in .gitmodules; every gitlink is declared). exit=0 ``` The nested repository sits at `vendor/thing` rather than under `.worktrees/` deliberately: the ignore PR #17468 landed covers that one path, and this gate is about the class. On this repository the live run is: ``` $ pnpm check:gitlink-declared check-gitlink-declared: OK (8726 index entries -- 0 gitlink(s) at mode 160000; no .gitmodules in the index, so nothing is declared; nothing to declare). exit=0 ``` The gitlink count is printed unconditionally, `0` included: a summary that named gitlinks only when it found some would make "there are none" and "I did not look" render identically. ## The card's citation, corrected The card body and its triage comment both state that `160000` *"appears in the tree only as two skip comments in `scripts/check-nul-bytes.mjs`"*. Re-derived at `7358c1c5b` with controls taken from the probed tree itself: | reading | value | | --- | --- | | `git grep 160000` across the tree | **0** — the literal appears nowhere | | firing control `git grep -i gitlink` | **2** — `scripts/check-nul-bytes.mjs:193`, `:521`, both prose skip comments | | firing control `git grep -ic nul scripts/check-nul-bytes.mjs` | **76** | | dark control (a nonsense token) | **0** | | `git grep -i gitmodules` | **0**, and no `.gitmodules` file exists | | `git ls-files --stage` mode histogram | `100644` × 8691, `100755` × 34 — no other mode | The substance holds and holds harder than the card claimed; the citation form does not. ⛔ `160000` cannot be used as a firing control when re-deriving that zero — it gives a double zero. ## Changeset: `skip-changeset`, measured against `files[]` The sole criterion is whether anything **published** moves, so this was measured rather than argued from the path names, over the 83 tracked manifests at `298e245de`: | reading | value | | --- | --- | | tracked `package.json` manifests | 83 | | of those, published (not `private: true`) | 70 | | published packages whose directory is the repo root | **0** | | published packages with no `files[]` (i.e. shipping their whole directory) | **0** | | published `files[]` entries escaping their own package directory (`../` or a leading slash) | **0** | | published `files[]` entries naming `scripts` at all | **0** | | positive control — `@objectstack/spec` `files[]` | `dist`, `json-schema`, `liveness`, `prompts`, `llms.txt`, `README.md`, `src/**/*.zod.ts`, `CHANGELOG.md`, `api-surface`, `spec-changes.json` | The root manifest is `@objectstack/spec-monorepo`, `private: true` — it is never published. Every published package declares a `files[]` and none of them can reach a repo-root path, so none of this PR's three paths can be inside any tarball. ⇒ `skip-changeset`, applied on the PR. ## Verification `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derives **63** families for this change set; the new gate discovers itself and is placed under the always-runs whole-tree heading with its liveness spelling vouched (`a git ls-files enumeration of the tracked corpus`). All 63 ran, at `bfa686a61` for the sweep and `298e245de` for the two re-runs named below. **58 exit 0.** The other **five exit 3 — `PREREQUISITE NOT MET`, which is NOT MEASURED and neither a pass nor a finding**: `check:dts-closure`, `check:dual-build-cjs-loads`, `check:lean-entry-closure`, `check:sourcemap-no-sources-content` and `check:type-check-debt` all read built package output, and no package was built in this worktree. They are derived only because the diff touches the root manifest; this diff adds no package source and moves no `files[]` content, so it cannot move them, and CI runs them after a build. That narrowing is declared here rather than smoothed over. Beyond the derived set: - `pnpm lint` — the **full** repo-wide run, not a narrowed one, re-run on the final commit `298e245de`: `eslint . --no-inline-config --format json` over **6790** files, **0 errors, 0 warnings**, exit 0. The file count is read off eslint's own `--format json` output and the population is eslint's own config resolution, not a guess; this repo's single `eslint.config.mjs` never enables type-aware linting (no `parserOptions.project`, no typed rules) for any file, so nothing in this diff can move the verdict on a file it does not touch. - `node scripts/check-ci-filter-parity.mjs --self-test` — exit 0, 47 assertions. - `node scripts/pr-labels.mjs --self-test` — exit 0, `VERDICT: pr-labels self-test PASSED`. - `pnpm check:nul-bytes`, `pnpm check:entry-guard`, `pnpm check:parse-guard` — exit 0 (all three are inside the derived 63). - `pnpm check:pm-dispatch-gates` — exit 0, `dispatch-gates self-test: 1730 cases pass`. Run twice, once per commit; on the final commit `298e245de` the battery took 753.4s on this box. **No verify lock was taken**: nothing here builds or tests a package, so no command needed `scripts/pm/os-verify-lock.sh`. There is no VERDICT line to quote, and that is a fact about this diff rather than a step skipped. ## Acceptance notes - **The gate caught its own author, before it was even wired up.** The first draft of this script declared its `-z` delimiter by writing the backslash-u escape for the NUL byte as a string literal. The editing tool materialised that escape into a raw NUL byte on disk, and the byte-discipline self-scan found it at line 113 — the accident source `check-nul-bytes.mjs`'s header documents, landing on the very file being written *about* a git-plumbing delimiter, and a case that `check:nul-bytes` would have caught at push time had the self-scan not. Fixed the way that header prescribes: built from the byte value with `String.fromCharCode(0)`, never written as a literal anywhere in the file. - **A `.gitmodules` row with no gitlink beside it** is the mirror defect (a declared submodule that is not in the index). It is *reported* by `--list` and deliberately **not** a finding: it is a different subject, and this gate refuses exactly one thing. Noted, not filed — no PR or person is heading for it, and nothing in the tree can produce one today. - **`.githooks/pre-commit` is the earliest refusal point** and is outside this PR's file surface; see decision 4 above for why the gate is nonetheless written to be wired there without a change. - **`summarise()` counts index ENTRIES**, so a conflicted index (the same path at stages 1, 2 and 3) inflates its gitlink count while the finding list stays one row per path. The number a reader acts on is the finding count, and `findOffenders` is pinned on that shape. --- _Generated by [Claude Code](https://claude.ai/code)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 092d460 commit 36b20ca

3 files changed

Lines changed: 703 additions & 0 deletions

File tree

‎.github/workflows/lint.yml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,28 @@ jobs:
409409
- name: Raw control-byte guard
410410
run: pnpm check:nul-bytes
411411

412+
# Gitlink guard (#17472). Refuses an index entry at mode 160000 — a
413+
# SUBMODULE POINTER — that no tracked `.gitmodules` row declares. Placed
414+
# beside the control-byte guard because it is its structural sibling: a
415+
# whole-index population, a hygiene property of what a CLONE receives, and
416+
# a defect whose only native signal is a warning on a command that exits 0.
417+
# WHY mode 160000 and why "undeclared" rather than "none at all" are stated
418+
# and argued once, in the gate script's header — `scripts/check-gitlink-
419+
# declared.mjs`. That header is authoritative and this comment cites it
420+
# rather than restating it. The one-line summary, for whoever is reading
421+
# this because the step just went red: `git add -A` over a nested git
422+
# repository (a linked worktree, a nested clone, a vendored checkout)
423+
# stages ONE 160000 entry at exit 0 with only a `warning:` line, and the
424+
# commit that follows carries a pointer to an object no clone of this
425+
# repository has. A real submodule passes untouched — `git submodule add`
426+
# writes the declaration and the pointer in one act — so a hit here is
427+
# never a submodule you added on purpose.
428+
# ⛔ Not the untracked-signal half: PR #17468 closed that one by ignoring
429+
# `.worktrees/`. This step is the STAGE, and it is path-blind on purpose.
430+
# Reads the index only, no network, well under a second.
431+
- name: Staged gitlinks are declared in .gitmodules
432+
run: pnpm check:gitlink-declared
433+
412434
# The first three of these are the shared modules the two `scripts/**`
413435
# routing gates below DELEGATE their design arguments to (#10608). Both of
414436
# those gates are

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"check:i18n-stale-fill": "node scripts/check-i18n-stale-fill.mjs --self-test && node scripts/check-i18n-stale-fill.mjs",
3838
"check:app-nav-i18n": "pnpm --filter @objectstack/cli run check:app-nav-i18n",
3939
"check:nul-bytes": "node scripts/check-nul-bytes.mjs --self-test && node scripts/check-nul-bytes.mjs",
40+
"check:gitlink-declared": "node scripts/check-gitlink-declared.mjs --self-test && node scripts/check-gitlink-declared.mjs",
4041
"check:entry-guard": "node scripts/check-entry-guard.mjs --self-test && node scripts/check-entry-guard.mjs",
4142
"check:parse-guard": "node scripts/check-parse-guard.mjs --self-test && node scripts/check-parse-guard.mjs",
4243
"check:bash32-floor": "node scripts/check-bash32-floor.mjs --self-test && node scripts/check-bash32-floor.mjs",

0 commit comments

Comments
 (0)