Skip to content

feat(scripts): refuse an undeclared mode-160000 gitlink in the index - #18414

Open
os-try-charles wants to merge 2 commits into
mainfrom
claude/issue-17472-gitlink-declared
Open

os-try-charles wants to merge 2 commits into
mainfrom
claude/issue-17472-gitlink-declared

Conversation

@os-try-charles

@os-try-charles os-try-charles commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

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 2scripts/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

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 one mode-160000 entry 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.

`check:gitlink-declared` enumerates the index and refuses any entry at mode
160000 that no tracked `.gitmodules` row declares. A real submodule passes
untouched: `git submodule add` writes the pointer and the declaration in one
act. The declaration is read out of the index blob rather than the working
tree, because a `.gitmodules` that never gets staged does not reach the clone
that receives the pointer.

The self-test runs in both directions over throwaway repos: the card's own
measurement (a nested repo plus a literal `git add -A`) is refused and names
the path, and the same index with the declaration added is green.

Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
Co-authored-by: Claude <noreply@anthropic.com>
… reading

Three follow-ups on the gate, none of them behavioural for a clean index:

- the dispatch-gates population marker is one line, because the marker is
  matched line-by-line and a wrapped reason printed to a seat cut off
  mid-sentence;
- `findOffenders` deduplicates by path, so a conflicted index -- which carries
  the same path at stages 1, 2 and 3 -- is one finding and not three;
- the header's zero-submodule reading now names the tree it was taken against.

Claude-Session: https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk
Co-authored-by: Claude <noreply@anthropic.com>

Copy link
Copy Markdown
Collaborator Author

PM review — ACCEPT

Reviewed against GitHub and against a detached worktree at this PR's head 298e245de82dfb2059e647a6dfb37810b6db5f49, ⛔ not against the report's narrative. ⭐ The two-direction acceptance is the part of this card most easily faked, so I did not take the self-test's word for it — I built the fixtures myself, from the card's own measurement, in a throwaway repo outside every checkout.

The gate, exercised by me rather than by its own harness

nested repo at vendor/thing, then a literal `git add -A`
  ⇒ warning: adding embedded git repository: vendor/thing
  ⇒ git ls-files --stage:  160000 bb8cf93723e… 0  vendor/thing
direction what I ran exit what it said
2 · bare gitlink the index above 1 names vendor/thing -- mode 160000, commit bb8cf937, then the two remedies
1a · declared on DISK but NOT staged same index + an unstaged .gitmodules row 1 the finding stands
1b · the same row STAGED git add .gitmodules and nothing else 0 every gitlink is declared

1a → 1b is the reading that makes this gate worth having, and it is the design decision I would have got wrong: only the index changed between them. The gate reads the declaration with git config --blob :.gitmodules (:193), ⛔ not from the working tree — because a .gitmodules that is never staged never reaches the clone that receives the pointer. A gate that read the worktree copy would go green on exactly the tree that ships the dangling pointer.

⚠️ One thing I have to say about my own instrument: my first attempt at direction 2 reported EXIT=1 — from ERR_MODULE_NOT_FOUND, because I had copied the script away from its invoked-as.mjs sibling. An exit code is not a verdict about the subject until you read what produced it. I re-ran it from the script's real path before believing the 1.

Also re-derived here, independently: --self-test exit 0, 36 assertions; the live run exit 0, 8726 index entries -- 0 gitlink(s) at mode 160000. :221 confirms main(), --list and --self-test all go through the one scan(), so the self-test exercises the production path rather than a parallel one.

The two shape deviations — both accepted

The card's suggested shape was report-only-then-refusing. This ships refusing from day one, with the report half as --list. The argument, which I checked: there are zero gitlinks in the tree, so a report-only phase would never once have exercised the refusal — the day it became a refusal would be the day it was first tested. ⇒ Accepted. ⛔ The triage comment made the shape a proposal twice over, so this is inside the fence, not a breach of it.

Fences, checked one by one

  • File surface: 3 files, exactly the three declared — scripts/check-gitlink-declared.mjs (+680), package.json (+1), .github/workflows/lint.yml (+22). ⛔ Zero breach.
  • Manual floor NOT hit: no new runtime dependency — git itself parses .gitmodules. This was the one floor named in the dispatch.
  • House shape: root key is node scripts/check-gitlink-declared.mjs --self-test && node scripts/check-gitlink-declared.mjs, character-for-character the sibling spelling; the lint.yml step is unconditional, inside Lint & Repo Gates.
  • ⛔ The untracked half was not re-closedlint.yml:428-429 says so in its own comment, naming PR chore(repo): ignore .worktrees/ so an agent worktree inside the checkout is not untracked #17468.
  • Commit trailers, both commits: Co-authored-by: Claude <noreply@anthropic.com> + Claude-Session:, model-free, ⛔ no card trailer. ✅ No amend, no force-push.
  • check-clause2-carriers --pair 18414exit 0; Clause-②: no, both carriers agree, no widening tell.
  • skip-changeset, re-derived by me at this head and matching the report number for number: 83 tracked package.json, 70 published, 0 with no files[], 0 whose files[] names scripts at all, 0 escaping their own directory, root manifest @objectstack/spec-monorepo private: true. Positive control that the instrument reads real entries: @objectstack/spec's files[] = dist, json-schema, liveness, prompts, llms.txt, README.md, src/**/*.zod.ts, CHANGELOG.md, api-surface, spec-changes.json. ⇒ zero published bytes move.

The declared narrowing, and why it stands

5 of the 63 derived families exited 3 — PREREQUISITE NOT MET, read as NOT MEASURED rather than as a pass: check:dts-closure, check:dual-build-cjs-loads, check:lean-entry-closure, check:sourcemap-no-sources-content, check:type-check-debt. All five read built package output that this worktree has none of. ⇒ The narrowing is declared rather than smoothed over, and it holds on the same reading that carried skip-changeset: this diff adds no package source and moves no files[] content, so it cannot move those five, and CI runs them after a build. ⚠️ ⛔ I am not treating an exit 3 as green — I am treating it as not a reading, which is what it is.

⭐ And the slow gate was not allowed to become a NOT MEASURED: check:pm-dispatch-gates ran bare, once per commit (775.1 s, then 753.4 s), each time blocked on with tail --pid per the prescription at platform-readings.md:425, and passed 1730 cases. That is the failure this seat paid for on #16421.

Two things the dev handed me, and what I did with each

  1. A real finding, verified at source before I filed it. WHOLE_TREE_POPULATION_MARKER (dispatch-gates.mjs:2832-2834) captures (\S.*)$ under the m flag ⇒ the reason ends at the first newline; NO_PATH_POPULATION_MARKER (:2754-2755) is identical; and wholeTreePopulationRefusal (:3037-3066) checks only that a reason exists, that no second population marker contradicts it, and that a root walk backs it — ⛔ never that the reason is whole. A wrapped reason therefore reaches a seat as a sentence that simply stops, with nothing red. Filed as [finding] dispatch-gates' population markers capture only the FIRST line of a declaration's reason, and no refusal checks the reason is whole — a wrapped reason reaches a seat cut off mid-sentence, with nothing red #18422. ⛔ I read all three sites myself rather than transcribing the frame; this shift has already produced three dev frames that would have sent the next person to the wrong file.

  2. A correction that is mine. My dispatch template transcribed a commit-trailer literal carrying a model identifier, which this repo's pre-push gate refuses (scripts/check-commit-card-trailers.mjs:300 declares the pair; :876/:931 assert that exact string is the offending one). This dev had already resolved it the same way before its first commit and ⛔ declined the OS_ALLOW_CARD_TRAILER_PUSH=1 override — both right, and it cost this run zero cycles. ⭐ Its recommendation is better than my fix: cite the rule rather than transcribe the literal, because a literal restated in two places is exactly the drift this repo gates elsewhere. Adopted.

Landing

This PR is human-merge only and I am not arming it: the diff touches .github/workflows/**, on which this seat's auto_merge is permanently 422. That was declared in the claim before dispatch, not discovered here. Lint & Repo Gates is still in_progress on this head; when the full check set is green by name-deduped reading I will mark it ready and request review, ⛔ not arm it.

PM seat domain:devx · round 5 · reviewed head 298e245de82dfb2059e647a6dfb37810b6db5f49 · 2026-09-16T10:17Z


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Ready for human merge — ⛔ this seat cannot arm it

@hotlong — review requested on GitHub, and the request is read back from the API rather than merely stated here (requested_reviewers: ['hotlong']).

Why you and not the queue: the diff touches .github/workflows/lint.yml, and this seat's auto_merge is permanently HTTP 422 on a workflow-touching PR (the token carries no workflows scope). ⛔ That is a real GitHub-side refusal, not a session-classifier limit, and ⛔ it is not retried. It was declared in the claim on #17472 before dispatch, not discovered at review time.

State, all three read after the ready flip — marking ready re-triggers Governed Surface Queue Guard, so the green predicate was re-taken afterwards rather than reused:

checks     53 raw → 34 deduped by check name → NOT-GREEN 0
draft      false
mergeable  clean

What it is: a new check:gitlink-declared family that refuses any index entry at mode 160000 which no tracked .gitmodules row declares. 3 files, +703 −0, skip-changeset (measured: 83 tracked manifests, 70 published, 0 whose files[] names scripts at all).

The one reading worth your time before merging. The gate reads the declaration from the index blob (git config --blob :.gitmodules), ⛔ not from the working tree. I built the fixtures myself rather than trusting the self-test, and that decision is what the gate turns on:

fixture exit
nested repo + literal git add -A ⇒ a bare 160000 row 1, naming the path
the same index + a .gitmodules row on disk but unstaged 1 — the finding stands
the same row staged, and nothing else changed 0

A gate that read the worktree copy would go green on exactly the tree that ships the dangling pointer.

Full review with every re-derived number: comment 5695851514.

PM seat domain:devx · round 5 · head 298e245de82dfb2059e647a6dfb37810b6db5f49 · 2026-09-16T10:23Z


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

两条巡查行的答复,连同它们各自要求的读数

本轮 origin/main 上的半状态巡查在本 PR 上报了两行。两行都在这里就地答复,⛔ 不留给读者去猜。

H36 —— 与 PR #18503 共享 .github/workflows/lint.yml,巡查给了处方,我跑了

巡查行的原话:「Whichever lands second re-probes before (re-)arming: fetch the PR ref and run git merge-tree --write-tree against current mainthe zero-quota reading GitHub never volunteers」。

照跑,2026-09-16T17:16Z:

main      8cf527f8e0
#18414    298e245de8
#18503    016bdeaa43

git merge-tree --write-tree  main   #18414   ⇒ exit 0   tree 5667b1994f
git merge-tree --write-tree  main   #18503   ⇒ exit 0   tree 19e5fc73d9
git merge-tree --write-tree  #18414 #18503   ⇒ exit 0   tree 45a47cbc59   ← 直接对撞,也干净

三种配对全部 exit 0,无冲突。 而且两者在同一文件上的方向是错开的:

PR lint.yml 的改动
#18414(本 PR) 1 file changed, **22 insertions(+)** — 新增一个 step
#18503 1 file changed, **32 deletions(-)** — 移除若干 step

⚠️ 说明读数的限度:git merge-tree A B 是用两者的 merge base 做的真三方合并,⛔ 不是"先把 #18503 并进 main 再并本 PR"的逐步模拟。但三种配对都干净,且 merge-tree 本就是三方合并,所以「谁先落地都不会撞」这个结论在文本层面成立。⛔ 它不保证语义层面:若 #18503 移除的 step 正是本 PR 新增 step 所依赖的东西,那是语义冲突,merge-tree 看不见。⇒ 谁后落地,重跑一次门禁而不是只看这个 exit 0。

H12 —— 「orphan landing … auto-merge unarmed」:⛔ 不是孤儿,是按设计不可武装

巡查行给了三个处置(re-queue / fix / park as draft with a stated reason)。⚠️ 本 PR 是第四种,行里没有枚举到,所以在这里写清楚:

本席武装不了它,而且这不是失误。 它的 diff 触 .github/workflows/**,本席的 token 缺 workflows scope ⇒ auto_mergeHTTP 422。⛔ 这是 GitHub 侧的真 422,不是会话分类器的限制,⛔ 也不重试。这一条在卡 #17472 的认领评论里派发前就写明了,不是复核时才发现的。

⇒ 三个选项逐一不适用:

  • re-queue —— 进不去,422;
  • fix —— 没有东西要修:本 head 298e245de853 raw → 34 去重 → NOT-GREEN **0**,mergeable_state: **clean**;
  • park as draft —— 转回 draft 会把它从人工合并的视野里拿掉,降低它落地的概率,与本行的意图相反。

实际状态:绿 · clean · ready · requested_reviewers: ['hotlong'](已从 API 回读,⛔ 不是"报告里点名")· reviews: NONE(尚无人提交复审)。⇒ 只差一次人工合并

PM seat domain:devx · session session_017ef78bLdybu3AffehKkhfk · round 11 · ⛔ 未重试武装,未改状态 · 2026-09-16T17:17Z


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/cd dependencies Pull requests that update a dependency file size/l skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants