fix(fork): make CLAUDE.md resolve, and make its guard able to fail

[NoahHendrickson]

CLAUDE.md is a symlink whose committed blob is ten bytes — AGENTS.md\n. A symlink's target is its blob verbatim, so it pointed at a filename containing a newline and resolved to nothing:

$ git cat-file -p HEAD:CLAUDE.md | xxd
00000000: 4147 454e 5453 2e6d 640a                 AGENTS.md.

$ cat CLAUDE.md
cat: CLAUDE.md: No such file or directory

Any agent whose harness reads CLAUDE.md opened this repo with no project instructions — including the session that found this, whose context contained no AGENTS.md at all.

That matters more here than it would in most repos. CLAUDE.md → AGENTS.md → .fork/AGENTS.md is how an agent learns the branch rules and placement ladder before touching anything, and fork-workflow-docs states in its intent that the symlink delivers exactly that. It didn't.

The fix

ln -s AGENTS.md CLAUDE.md — nine bytes. The resulting blob is 47dc3e3d8, byte-identical to the one upstream's own fix produced:

Commit	Blob	State
5e13f5357 fix: remove trailing newline from CLAUDE.md symlink (pingdotgg#2052), Apr 16	47dc3e3d8	correct
6891c77d3 Build for Windows ARM (pingdotgg#2080)	c31706425	broken again

An unrelated Windows ARM PR reintroduced it twenty-eight commits later — almost certainly a tool normalizing a trailing newline onto a file nobody meant to edit. Upstream has shipped this newline twice and fixed it once, so it's recurring, not settled.

The guard is the more important half

It asserted:

expect(NodeFS.readlinkSync(...).trim()).toBe("AGENTS.md");

.trim() strips precisely the byte that is the bug. So the assertion held identically while the link was broken, while upstream fixed it, and while upstream re-broke it. It never changed value and could not have — a guard that cannot fail.

That's the same vacuity FORK-DATA-ISOLATION-HANDOFF.md diagnoses for forkAppIdentity: "it tests that a rename happened, not that the rename is sufficient." This one tested that an alias was spelled correctly, not that it worked.

Dropped the trim, and added an assertion for the property actually promised — that reading through CLAUDE.md produces AGENTS.md's contents, fenced fork block included.

Proof the new guard fails

Not asserted — run. Reintroduced the broken symlink with ln -s $'AGENTS.md\n':

 ✓ keeps the fenced Fork Workflow section in AGENTS.md
 × keeps CLAUDE.md aliased to AGENTS.md so Claude agents get the same rules
 × resolves CLAUDE.md to the rules rather than merely pointing at them
   Tests  2 failed | 1 passed (3)

Both new assertions fail against the broken link and pass against the fix. The old assertion passed against both — which is the whole point.

Verification

Check	Result
cat CLAUDE.md	resolves; byte-identical to AGENTS.md
staged blob	9 bytes, mode 120000
apps/web tests	188 files, 1647 tests, pass (was 1646 — one new test; the earlier note said "assertion", which was the wrong unit in a PR about counting them)
lint	exit 0 — 0 errors, 12 warnings (unchanged)
fmt --check	clean

Note on scope

This edits a file upstream owns, which normally I'd avoid. The justification is that upstream has already demonstrated it can't hold this fixed, and the fork has a customization specifically claiming it works. When a sync reintroduces the newline the guard now fails loudly instead of sleeping through it — which is what the fork layer is for.

---

Review follow-ups (second commit)

§	Change
1	- CLAUDE.md added under watch:. Verified it now fires: a changed-file list of CLAUDE.md reports fork-workflow-docs; before, detect-drift.mjs printed nothing.
2	intent: records that the fork owns this blob, that upstream's carries the newline, that a sync conflict resolves to the fork's side — and states the fence exemption rather than leaving it unexplained.
3	expect(NodeFS.existsSync(claudePath)).toBe(true) ahead of the read. A broken link now fails as an assertion, not an uncaught ENOENT.
4	Taken. The alias test asserts the committed object — git ls-tree for mode 120000, git cat-file for the exact target. Shelling out matches customizationsManifest.test.ts, which already runs git ls-files; that also settles #17 §8's premise, corrected there. Read-through stays, since HEAD-level checks cannot see a conflicted or dirty tree, with the non-symlink checkout handled explicitly instead of red.
5	Both duplicate toContain lines dropped. Test-count wording fixed above.
6	Done in #17 — §9 now points here, §8's "none spawns a subprocess" corrected, and a new §10 records the vacuous-guard class and mutation as the check that finds it.

Mutation-checked, three states

State	Result
broken link in the working tree, HEAD good	read-through fails on the existsSync precondition; committed-blob test correctly still passes
broken link committed	both fail — blob test reads expected 'AGENTS.md\n' to be 'AGENTS.md'
core.symlinks=false (regular file holding the target)	green — no false red against a correct commit

vp fmt --check clean, vp lint 0 errors / 12 warnings (unchanged), apps/web 188 files 1647 tests pass, web typecheck clean.

---

🤖 Generated with Claude Code

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[cursor]

Thermo-nuclear code quality review: no major structural issues found.

Small, correctly scoped fix — the broken symlink blob and vacuous .trim() guard were the defects; this PR deletes that vacuity and asserts the real invariant (read-through resolution). No spaghetti growth, file-size concern, or missed simplification. Approval bar met.

  

_{Sent by Cursor Automation: Thermo-nuclear PR review}

[NoahHendrickson]

Review

I reproduced the central claim before reviewing anything else, because the whole PR rests on it. It holds:

broken  isSymlink=true  trim()==="AGENTS.md" → TRUE   exact==="AGENTS.md" → FALSE   read-through → ENOENT
fixed   isSymlink=true  trim()==="AGENTS.md" → TRUE   exact==="AGENTS.md" → TRUE    read-through → matches AGENTS.md

The old assertion returns true in both columns. It was not a weak guard, it was a constant, and it sat in the file whose manifest entry claims agents learn the fork rules before touching anything. Also confirmed independently: committed blob is 9 bytes, mode 120000, hash 47dc3e3d8 — byte-identical to upstream's own fix — and the guard genuinely gates CI (vp run test → apps/web test → --project unit, on ubuntu-latest, where symlinks materialize).

So the diagnosis and the fix are right. My problems are with what the PR left unwired, and they're not cosmetic.

---

1. The manifest change that makes this durable is missing — and it's the one the fork rules mandate

.fork/AGENTS.md:29-31, option 4, inline edit of an upstream file:

keep the hunk small, fence it … and add the file under watch: in the manifest.

CLAUDE.md is now a fork-owned inline edit of an upstream-owned root file. The fork-workflow-docs entry watches only AGENTS.md:

watch:
  - AGENTS.md

That matters because of .fork/detect-drift.mjs:65:

const hits = [...entry.shadows, ...entry.watch].filter((path) => changed.has(path));

Drift detection fires on shadows: + watch: and nothing else. An upstream sync that touches CLAUDE.md will not flag fork-workflow-docs for review — which is precisely the recurrence channel this PR was written to defend, and the one the PR body argues is live ("upstream has shipped this newline twice and fixed it once, so it's recurring, not settled"). The guard test is the backstop; drift detection is the thing that tells a human to look before the merge. Right now only the backstop got built.

Adding - CLAUDE.md under watch: is a one-line change and belongs in this PR, not a follow-up. The fence half of rule 4 is genuinely inapplicable — you cannot put a comment marker inside a symlink blob — but that's an argument for stating the exemption in the manifest, not for skipping the watch: half that works fine.

2. Nothing records that the fork now owns this blob

The intent still reads as though upstream supplies the alias:

AGENTS.md (and its CLAUDE.md symlink) carries a fenced Fork Workflow section

and files: lists only .fork/AGENTS.md. After this PR, the fork carries its own CLAUDE.md blob that deliberately diverges from what upstream ships today. The next person resolving a CLAUDE.md conflict during a sync will find no record of why the fork's side is the correct one, and the cheapest resolution — take upstream — silently restores the bug. The PR body's "Note on scope" is the right reasoning; it just needs to live in the manifest where a syncer will actually encounter it, rather than in a PR description nobody reads at merge time.

3. The read-through test throws where it claims to diff

The body says both new assertions "fail with a legible diff." For the content-divergence case, true. For the broken-symlink case — the actual regression being guarded — readFileSync throws ENOENT at line 43 before any expect runs. Reproduced above. The test does go red, so this isn't a correctness bug, but a PR whose thesis is guards must fail informatively should not describe an uncaught throw as a diff. An expect(NodeFS.existsSync(claudePath)).toBe(true) ahead of the read, or the change in §4, gives you the failure you're advertising.

4. The stronger version of this guard is also the portable one

Both new assertions inspect the working tree. The threat model in the PR body is about the committed blob — what an agent gets when it clones. Those diverge, and in the direction that hurts: on a checkout without symlink support, git materializes CLAUDE.md as a regular file containing AGENTS.md, and all three assertions go red against a repo whose commit is perfectly correct. That's a false red, which is the same class of defect as the false green you just fixed. Worth noting that the upstream commit which reintroduced the newline was Build for Windows ARM — this file's history is entangled with exactly the platform where the working-tree check stops being meaningful.

The git-level assertions are strictly stronger and core.symlinks-independent. Both verified in this repo:

$ git ls-tree HEAD CLAUDE.md
120000 blob 47dc3e3d863cfb5727b87d785d09abf9743c0a72	CLAUDE.md

$ git cat-file -p HEAD:CLAUDE.md | od -c
0000000   A   G   E   N   T   S   .   m   d          # 9 bytes, no trailing \n

That asserts mode and target on the object that actually propagates, and it cannot be defeated by a checkout quirk. It does mean shelling out to git — which is the open question in #17 §8. I'd take the shell-out here: this specific guard's subject is a git object, so reading the working tree is measuring a proxy.

Not blocking, but I think it's the assertion you actually wanted.

5. Minor

• Lines 46-47: given expect(claude).toBe(agents) on line 45 and the identical toContain pair in the first test, these two cannot fail independently of assertions that already exist. Duplicate signal, not additional coverage. Drop them and let toBe carry it.
• Body says "1647 tests, pass (was 1646 — one new assertion)". That's one new test; you added three assertions. Trivial, except in a PR about assertion counting.

6. #17 is still open and contradicts this

.fork/notes/FORK-LINT-GUARD-HANDOFF.md §9, shipping in #17:

Same blob in origin/main, so it is upstream's. … nothing is being filed upstream — recorded here only so the finding survives.

Whichever of these lands second leaves custom carrying a committed note that documents a fixed bug as live and explicitly declines to fix it. Since #18 is the smaller change, easiest resolution is a one-line edit to §9 in #17 pointing at this PR. Worth coordinating rather than discovering later.

---

The larger point

This PR is a good catch and the reasoning in it is better than the change. But it fixes one instance of a class, and the class is the more expensive problem: a guard that cannot fail is worse than a missing guard, because it consumes the attention budget a missing guard would have left free. .trim() here, and per FORK-DATA-ISOLATION-HANDOFF.md, forkAppIdentity "tests that a rename happened, not that the rename is sufficient." Two known instances across 17 guard files, both found by a human reading closely — which is not a detection strategy that scales.

I checked whether the pattern greps out mechanically: trim()/replace() elsewhere in __fork_guards__ is all legitimate parsing (cssRules.ts, phosphorIcons.test.ts), so there's no cheap textual sweep here. The real answer is mutation: for each guard, break the property it claims to protect and assert the guard goes red. You did that manually for this one, and that manual run is the most valuable artifact in the PR — it's also the part that isn't captured anywhere and won't survive.

That's a genuine companion to #17's proposal rather than a competitor. #17 makes lint warnings in fork-owned code fail; this would make vacuous guards in fork-owned code fail. Same failure mode — something that silently never goes red — in the layer that's supposed to catch everything else. Out of scope for this PR; worth a line in the handoff so it doesn't get rediscovered a third time.

Recommendation: the symlink fix and the de-trimmed assertion should land. §1 blocks — it's a one-line manifest change that the fork's own placement rules require, and without it the sync path this PR exists to protect stays unwatched. §2 is a few lines in the same file. §3-§5 are cheap. §4 and the mutation idea are the ambitious version and can follow.

---

Generated by Claude Code

[NoahHendrickson]

Addressed. ee63647 here, plus 045cb35 on #17 for §6.

§1 — blocking, fixed. - CLAUDE.md under watch:. You were right that only the backstop got built; the fix is verified by running the detector rather than by reading it:

$ printf 'CLAUDE.md\n' > /tmp/changed.txt && node .fork/detect-drift.mjs /tmp/changed.txt
- **fork-workflow-docs** — upstream touched: `CLAUDE.md`

Empty output before this commit.

§2 — fixed. The intent now says the fork owns the blob, that upstream's differs and why, and that a conflict resolves to the fork's side because taking upstream restores the bug. It also states the fence exemption rather than leaving a reader to infer it — a symlink's blob is its target verbatim, so a comment marker inside it breaks the link it describes. The watch entry is the whole fence.

One thing worth recording: the first draft of that prose contained the literal string fork:begin fence in a sentence explaining why there is no fence, and customizationsManifest.test.ts failed with fence references unknown customization "fence". Vacuous is not the only failure mode a guard has — that one is over-eager on .yaml, and it caught prose. Not worth changing; noting it because the next person to write about fences in the manifest will hit it.

§3 — fixed. existsSync precondition ahead of the read. The broken-link case now reports expected false to be true at the assertion instead of throwing ENOENT out of readFileSync.

§4 — taken. The alias test asserts the committed object now: git ls-tree for mode 120000, git cat-file for the exact target. Two things pushed me past "not blocking":

• It subsumes §3 in the case that matters. Against a committed broken symlink the failure is expected 'AGENTS.md\n' to be 'AGENTS.md' — the diff the PR claimed and did not have.
• The shell-out question you flagged as open in docs(fork): hand off the fork-owned lint guard #17 §8 is already answered by the tree. customizationsManifest.test.ts:77 runs git ls-files. The premise "none spawns a subprocess" was wrong when it was written, so this isn't establishing a precedent, it's following one. Corrected in docs(fork): hand off the fork-owned lint guard #17 §8 so the open question is framed as cost — a vp lint subprocess is far more expensive than a git ls-tree — rather than as a first violation.

Read-through stays rather than being replaced. HEAD-level assertions cannot see a conflicted or dirty working tree, which is exactly the state a sync produces, so the two cover different windows. The core.symlinks=false case returns early and asserts the fallback shape — a regular file holding AGENTS.md — instead of reddening against a correct commit.

§5 — both fixed. The toContain pair is gone. The body now says one new test, and flags that "assertion" was the wrong unit in a PR about counting them.

§6 — done in #17. §9 now points here and says why the finding still matters post-fix: upstream fixed this itself in 5e13f5357 and re-shipped it in 6891c77d3, so the note's value is now "expect the guard to fire after a sync," not "this is broken."

---

On the larger point. I put it in #17 as a new §10 rather than leaving it in a merged PR description, since that's where the companion proposal lives and your read is that they're the same failure mode one level apart. It records the two known instances, that trim()/replace() elsewhere in __fork_guards__ is legitimate parsing so there's no textual sweep, and mutation as the check that actually finds them. Framed as a decision for whoever implements #17 §5, not a plan.

The manual mutation run is captured too, and it grew a third case while addressing §4:

State	Result
broken link in working tree, HEAD good	read-through fails on the precondition; blob test correctly still passes
broken link committed	both fail — blob test with the byte-level diff
core.symlinks=false checkout	green, no false red

That third row only exists because you raised §4. The two-row version would have passed on a Windows checkout of a broken commit and failed on a Windows checkout of a good one, which is the false green and the false red in the same guard.

vp fmt --check clean, vp lint 0 errors / 12 warnings (unchanged), apps/web 188 files / 1647 tests pass, web typecheck clean.