fix(standardize-repo): require a staged change before reporting staged state - #409
Merged
Conversation
…d state Two P2s from the cloud review of #400's final head, both confirmed against the code and both reproduced by the negative control below. They share one defect: the staged-state findings added in #400 compare the index against the RENDER and never check that the index differs from HEAD, so they report committed state as though it were staged. - A committed customization whose worktree copy is edited BACK to the template stages nothing, yet its index entry still differs from the render. The audit reported `staged content differs … the next commit carries it` and gated, turning an ordinary unstaged reconciliation into a finding — and an ordinary `git commit` writes no such entry, so the message was false as well as noisy. The same held for an unstaged `chmod` and the MODE line. - Mode and content stage INDEPENDENTLY: `git update-index --chmod` records a mode with the bytes untouched. For a co-owned file whose committed bytes already match the template, staging only a mode correction satisfied every other condition of the prose-clobber gate — index bytes equal to the render, something staged — and claimed a clobber that no commit performs. `load_staged_entries` now reads the (mode, blob) pair from HEAD and from the index through the existing fail-closed probes, and `index_diverges` keeps a divergence only on the dimension that actually changed: bytes staged for the content and structural verdicts, mode staged for the MODE verdict. The clobber gate keys on `index_bytes_staged` rather than "anything staged", which is what `has_staged_change` had meant — that helper is deleted, since the blob comparison answers its question exactly and per dimension. Verification: 7 new assertions across the three shapes (inherited content, inherited mode, mode-only staging). Negative-controlled by stashing only `diff-template.sh` — 6 of them fail against the pre-fix script, one reproducing the reported message verbatim: MODE scripts/status.sh (staged mode differs from the template though the worktree matches — the next commit carries it) for a chmod that was never staged. Refs #400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAX9K1JuEUVk6TW2KAP5ge
…t an unborn index Challenge round 1 on this branch. Both fixes are about the same blind spot in the previous commit's staged-ness filter: it asked only whether the BYTES moved. P1 (confirmed): a type change is staged in the MODE, not in the bytes. git records a regular file as 100644 and a symlink as 120000, and a file whose contents are exactly the link's target text has the SAME blob under both — so an index-only file↔symlink conversion moves the mode while the blob stands still. Clearing the structural verdict on unchanged bytes dropped it, and the exec-bit branch cannot catch it either because it exempts symlinks by design. Structure now survives whenever EITHER dimension moved; only a wholly unstaged entry is cleared, and a mode-only staging still drops an inherited byte divergence. P2 (confirmed): `load_staged_entries` ended the inspection at an unborn HEAD, making a pre-first-commit repo the one place staged divergence went unreported. There is no committed state before the first commit, so every index entry is staged by definition and the initial commit carries all of it. The HEAD fields are now simply left empty. That interacts with the clobber gate, and the interaction is the point: with an empty HEAD blob the gate would have claimed a prose clobber for every unborn repo. It now requires `staged_head_blob` — "there was a COMMITTED customization to lose" — which is the same argument declined on #400's unborn-HEAD thread, except enforced by a condition instead of by the absence of a probe. Adjudicated without code change: - intent-to-add entries, and reading the staged mode from the index rather than `test -x` — both already filed as #401, unchanged by this branch. - `scripts/test-skills.sh:1030` (`trigger_phrase_present` and ` ` entities) — OFF-DIFF. That guard is pre-existing on main and untouched here; noted in the PR body rather than fixed under an unrelated change. Verification: 4 new assertions (index-only type conversion, unborn staged divergence, and the two unborn clobber controls). Zero regressions against the main baseline. Refs #400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAX9K1JuEUVk6TW2KAP5ge
…he blob Challenge round 2, P1, confirmed — and the mirror image of round 1's P1, which is the tell that I had been patching directions instead of naming the concept. Round 1 fixed file→symlink: same blob, mode moves 100644→120000, so a bytes-only test missed it. Round 2 is symlink→file: HEAD holds a symlink, the index stages that unchanged blob as a regular file, the worktree holds the template's bytes. `index_structural` is 0 because index and render are both regular files; `index_bytes_staged` is 0 because the blob never moved — so the whole content divergence was cleared as "inherited", while committing produces a file whose CONTENT is the old link target rather than the template's. The concept both cases share: **a blob has no meaning without its mode.** The same bytes are a path string under 120000 and file content under 100644, so a staged TYPE change reinterprets an inherited blob into a genuinely new artifact and "the bytes did not move" stops being evidence of anything. An ordinary 100644→100755 chmod is emphatically not that — the bytes still mean what they meant — which is why `index_type_staged` is a separate question from `index_mode_staged`, and why the chmod case still drops an inherited byte divergence exactly as #400's review required. So the rule is now stated once, in both directions, instead of per-direction: clear the content verdict only when the bytes are inherited AND no type change reinterprets them AND the types agree with the render. Verification: a regression for each direction. The reverse case deliberately uses renovate.json rather than AGENTS.md — it is a CONTENT verdict, and a co-owned path's staged content stays exempt by the documented contract, which would have masked the mechanism under test. (That masking is what the first draft of this test hit, and it is a policy rather than a defect.) Refs #400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QAX9K1JuEUVk6TW2KAP5ge
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
evanharmon1
marked this pull request as ready for review
August 11, 2026 13:08
Owner
Author
Readiness gate — passed, promoted onceEvaluated against
Local gates: Ready for human review. Merging remains a human decision. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to #400, closing the two cloud-review findings that landed on its
final head after it was already merged, plus two more the local gates found
while fixing them. One invariant runs through all four:
400 taught
diff-template.shto inspect the index. It compared the index onlyagainst the render, never against HEAD, so committed state was reported as
staged; and it reasoned about blobs without their modes, so a staged type change
slipped through in both directions.
The four fixes
7e44403)staged content differs … the next commit carries itand gated. Same for an unstagedchmodand the MODE line.7e44403)chmodon a file whose committed bytes already matched the template claimed a prose clobber no commit performs.d44864e,4c903a3)120000and file content under100644. Both directions were missed: file→symlink (blob unchanged, mode moves — and the exec-bit branch exempts symlinks) and symlink→file (index and render are both regular files, so nothing looked structural, while committing yields a file whose content is the old link target).d44864e)load_staged_entriesstopped at the absent HEAD, making a pre-first-commit repo the one place staged divergence went unreported.has_staged_changeis deleted rather than patched: "is anything staged" was thewrong question, and the per-dimension
(mode, blob)comparison answers the rightone exactly. The clobber gate additionally requires a committed blob to lose,
which is #400's unborn-HEAD declination now enforced by a condition instead of by
the absence of a probe.
Why
Refs #400 — the two originating findings, both replied to in-thread:
Verification
diff-templatefamilies. Negativecontrol on the two originating findings: stash only
diff-template.sh,re-run — 6 fail, one reproducing the reported message verbatim
(
MODE scripts/status.sh (staged mode differs … the next commit carries it)for a chmod that was never staged).
task challenge: 3 rounds, clean at R3.task review: 1 round, clean —it ran the suite hermetically at 1068 tests, all passing, which is
independent confirmation that the local reds below are environmental.
task cireproducesmain's pre-existing local failure set exactly (53assertions, diffed both ways, byte-identical) and nothing new.
Local
test:skillsnote: those 53 macOS failures are the known #370 familyplus a guarded-update rehearsal family that also fails on unmodified
mainhere. Separately, one of them was caused by a full disk mid-session: the suite
leaks its whole temp tree per run and a fixture's
git addfailed withNo space left on device, surfacing as an ordinary red assertion. Filed as#407 with the cleanup shape.
Adjudicated without a code change
than
test -x— both already filed as standardize-repo: index-state and mode-detection follow-ups from the 400 review rounds #401 and untouched by this branch.scripts/test-skills.sh:1030(trigger_phrase_present, entities inthe trigger guard) — off-diff: that guard is pre-existing on
mainandgit diff origin/mainshows no hits in it. Declined for scope rather thanfixed under an unrelated change; recorded here so it is not lost.