Skip to content

fix(guard-commit,doctor): scope the decision-file predicate to the two real layouts; report absent enforcement gates as DRIFT - #357

Open
thrillmot wants to merge 2 commits into
devfrom
fix/gate-scope-and-doctor-absence
Open

fix(guard-commit,doctor): scope the decision-file predicate to the two real layouts; report absent enforcement gates as DRIFT#357
thrillmot wants to merge 2 commits into
devfrom
fix/gate-scope-and-doctor-absence

Conversation

@thrillmot

Copy link
Copy Markdown
Collaborator

Two independent holes in the enforcement layer. Both fixed, both mutation-tested (18 mutants, all
compiled, all killed), full suite green from an isolated worktree.

B1 — the commit gate accepted a decision file anywhere in the tree

isDecisionFile matched any path ending /decisions.md, so a well-formed entry written to
internal/x/decisions.md satisfied the gate. It is now the exact image of resolveDecisionsPath,
built from shared layout constants in internal/decisions: docs/decisions.md, or a single .md
directly under docs/decisions-branches/.

Verified independently by me, not taken from the lane's report — two binaries, one built from
origin/dev (0f78a04), one from this branch, run against freshly-scaffolded repos:

case dev binary this branch
entry in docs/decisions-branches/main.md + 302 Go lines — must pass exit 0 ✓ allowed exit 0 ✓ allowed
identical entry at internal/x/decisions.md + 302 Go lines — decoy exit 0 — bypass exit 65 — blocked

The must-pass row is the control: it stays green on both binaries, so the fix blocks the decoy without
over-blocking the legitimate layout. (Two earlier harness attempts of mine were invalid — one built
both binaries from the same tree, one omitted --msg-file so every run died on argument parsing
before reaching the gate. Recording that so the matrix is read as the third, working harness.)

check-decisions agrees on both scopes — the decoy was exit 0 in range mode pre-fix, exit 1 now, so
the §6.2 merge gate was open to the same decoy.

Explicitly NOT closed by this PR

A staged rename or copy of an existing decision file still clears the gate (#335). This predicate
cannot close it: a pathspec limits git's tree walk before rename detection, so a rename renders as
new file mode with every line added, and no path rule ever sees it. Documented in situ with both
candidate mechanisms and their costs — --name-status is cheap for renames, but
--find-copies-harder silently disables past diff.renameLimit, i.e. fails open under load, which
§3.4 forbids. Named as a design fork rather than picked.

B3 — doctor reported OK with every enforcement gate deleted

missing and markerless were excluded from DRIFT, so deleting all three enforcement surfaces
produced Stack status: OK, exit 0. SPEC §3.4: "Failing open MUST NOT be silent."

New StatusReport.GateAbsences (json:"gate_absences") is the only non-advisory list and flips
Overall. Scoped to three surfaces, reported only when the repo is initialised, is a git repo, and
git.enforce_commits is true.

before: Stack status: OK      exit 0
after:  Stack status: DRIFT   exit 1
        Enforcement gates absent (3) — SPEC §3.4: "Failing open MUST NOT be silent."
          • .claude/settings.json / .git/hooks/commit-msg / .github/workflows/check-decisions.yml

Suppressed for agents.claude:false, for no sibling workflow installed, and in linked worktrees
there .git is a file, probeHook cannot see the shared hooks dir, and this repo's own worktrees were
measured reporting a hook that is in fact installed and firing. Escalating that would flip every
worktree to exit 1 over a present gate, unfixable by --fix.

Opt-out is git.enforce_commits: false, which already means "logmind does not gate commits here" to
guard-commit, the config template and AGENTS.md. A second key would be a second owner for one fact.
Accepted cost: local-off/CI-on is not expressible.

Notes for review

Filed separately from this lane's out-of-brief findings: #353 (hooks resolve the default branch in
shell, hard-fallback main) and #270 (bare-name engine resolution).

@vercel

vercel Bot commented Aug 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
logmind-site Ready Ready Preview Aug 24, 2026 11:26pm

Request Review

@thrillmot

Copy link
Copy Markdown
Collaborator Author

Panel verdict: BLOCK from both lenses

Two independent reviewers, disjoint scopes. Both blocked. Fix lane running.


Lens B1 — the commit-gate predicate

Binaries: subject 1a26541, control 0f78a04. Both logmind 2.0.0-dev; the system 1.2.0 was never on
PATH. Baseline controls: real entry + 302 Go lines → 0/0; no decision file → 65/65.

CONFIRMED — over-blocking, product-breaking

guardcommit.go:398 tests rel == legacyDecisionPath, exact against the literal docs/decisions.md.
The writer at internal/cli/log.go:227 uses filepath.Join(cwd, "docs")cwd, not the git
root, and not case-normalized.
The old suffix arm forgave both. Two configurations where
logmind log's own output now fails its own gate:

trigger control (0f78a04) subject (1a26541)
pre-existing Docs/ dir, macOS core.ignorecase=true, detached HEAD, stock config → git stages Docs/decisions.md 0 65
logmind initialised below the git root (logmind init in pkg/api/ exits 0) → pkg/api/docs/decisions.md 0 65

End-to-end on the first, with the hook installed: control rc=0, ✓ Committed changes; subject
rc=1, Error: the decision was written to docs/decisions.md, but the commit was REFUSED. HEAD stayed
at the seed — the decision is written to disk and is uncommittable.

The reviewer's own caveat, kept: with branch_aware:true on a named branch both sets already blocked
pre-PR (65/65), so this narrows an already half-broken surface. It still converts two working
configurations into hard failures.

CONFIRMED — the "exact image" claim is false where it matters

I wrote that the predicate is the exact image of resolveDecisionsPath, built from shared constants.
decisions.DocsDirName has exactly one consumerguardcommit.go:355-356. The writer never reads
it. Mutating it to "documentation" compiles, the writer still emits
docs/decisions-branches/main.md, and the gate exits 65 on it. LegacyFileName co-moves correctly.

So 2 of 3 constants are genuinely shared, and the docs-dir half — precisely the half failing above
is not. That is this repo's house defect in my own PR text: a true statement about a narrow thing
presented as a conclusion about a wider one. Ruled: route writer and gate through one primitive, so
the bad state is unrepresentable. A constant only one side reads is not a shared owner.

Refuted

Nested docs/decisions-branches/feat/x.md (0/65) is correctListBranchFiles skips IsDir(), so
no read path ever sees it. Slash-branch feat__x.md, docs/decisions.md, bare .md: 0/0 both.
docs/decisions-archive.md: 65/65 both. Under-blocking hunted hard and found nothing new: symlink into
the branch dir, mode-only --chmod=+x, git add -N, gitignored + add -f, submodule, entry split
across two -U0 hunks, entry halves in two branch files — all 65/65, except gitignored + add -f
(0/0, correct, the file is committed). Per-hunk judgement holds. check-decisions agrees with the
index gate on all six path shapes; non-zero control: a decision in an earlier branch commit gives
INDEX=1 RANGE=0, so the detector fires. All five B1 mutants compiled, go vet clean, all FAILed.

Non-ASCII branch (feature/café) is 65/65 on both — pre-existing, not a regression — but it makes
internal/gitcli/gitcli.go:240-243 stale: core.quotepath emits "docs/...caf\303\251.md", and the
comment claiming the encoding round-trip is invisible "because we only do prefix/suffix comparison" no
longer holds now that the comparison is exact. That file belongs to another lane; noted, not touched.


Lens B3 — doctor gate-absence reporting

Three severe false OK paths. Each was driven with a real 30-line raw commit, and in each the commit
landed with no block.

  1. core.hooksPath. probeHook/hooks.Install* hardcode .git/hooks/<name> and never call
    git rev-parse --git-path hooks — which doctor.go:411's own comment cites as "the way git does
    it". With core.hooksPath .githooks and a working relocated hook, doctor reports false DRIFT;
    the tool's own suggested doctor --fix then writes a new hook to .git/hooks/commit-msg, a path
    git never reads, after which doctor reports OK, gate_absences: null, and the raw commit
    succeeds. A --fix that manufactures a false OK is the worst outcome available in this file.
  2. Present but inert. Empty (: >) or chmod -x hook → overall=OK, gate_absences: null; drift
    lands on markerless/current, neither counted by collectGateAbsences's if wf.Drift != "missing"
    filter. The feature checks existence, not functionThe harness layer's commit gate still cannot report its own absence — §3.4's fail-open-but-loud rule is half-implemented #298's class exactly.
  3. Gutted workflow, marker intact. probeWorkflow has no content-diff fast-path, only
    ExtractTemplateMarker. Gut check-decisions.yml's jobs: and keep the first line: drift: current,
    gate_absences: []. The §6.2 merge gate is silently defanged.

Also: gate_absences serializes as null, not [], on a healthy repo — pre-existing pattern across
every StatusReport list, but it contradicts this PR's own "present-and-empty" framing.

Refuted, with controls: linked worktree, submodule, bare repo, shallow clone, fresh pre-init clone,
--github-actions=false, agents.claude:false, git.enforce_commits:false all report correctly; each
of the three surfaces independently flips DRIFT; --fix round-trips, is idempotent (byte-identical on
rerun) and correctly leaves markerless/foreign hooks alone per SPEC §5.2.

doctor writing everything including --json to stderr is out of scope here — wider than this PR,
filed as #366.


Required to clear

Rebase onto d2b28fe (keeping both GateAbsences and #346's GateAdvisories), the one-primitive fix
for B1, and content-or-function checks for the three false-OK paths. The fix re-enters the panel.

Disclosure: the B3 reviewer's cd failed silently mid-run and a doctor --fix executed against the
maintainer's real working tree, modifying .claude/settings.json and nine docs/decisions-branches/*.md
files. It reverted them and disclosed unprompted. I verified independently afterwards: git status
clean, tree identical to origin/fix/plan-md-stale-gate-claims at 9db8273, no untracked residue,
hooks present and executable with core.hooksPath unset. No damage survived.

…eal layouts; doctor: report absent enforcement gates as DRIFT
…gate route through; ask git where hooks live; treat inert gates as absent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant