fix(topic-docs): the self-ignore guard does not run where no checkout is detected - #2756
Conversation
…t is detected The convention already routes non-interactive runs to a destination outside every checkout by default (the no-project-root fallback, and non-interactive is the normal condition for forked subagents, dispatched workers, and headless runs), while the self-ignore-guard bullet still spoke unconditionally about that same destination. The document contradicted itself on its own default path, so this closes a gap rather than carving an exception. Two outcomes bind the guard: a memory-tier write is never picked up by a checkout governing the destination, and no plugin ever modifies content tracked in any checkout. The guard is the means to the first wherever a governing checkout is found; where none is detected it buys nothing toward it and its create-when-absent rule can violate the second. The rule is blanket by derivation rather than by generalizing from the measured case. A `.gitignore` absent from disk is either untracked in some undetected checkout, where creating it is harmless and even mitigating, or tracked there, where creating it overwrites committed content and cannot hide the change since a tracked file is exempt from its own pattern. Separating those requires querying a checkout, and the branch is defined by having found none — the index check that would decide it is the one check that cannot run. Guessing untracked and being wrong modifies content committed in a repository the producer cannot see; guessing tracked and being wrong forgoes a mitigation for a harm that is reachable rather than automatic. Undecidable test, asymmetric costs, so: do not write. "Not detected" is stated as a detection claim and never as a claim that none exists — the branch is entered precisely where detection can be wrong. Two dependents. detector-findings' consequence sentence was false in this case and stays a consequence, corrected rather than softened: where detection missed a checkout the findings file lands there untracked, reachable by a later `git add -A`, not committed by this producer's action. And mutation-testing's spoke drops its local derivation for a pointer, now that the rule has an owner. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
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. |
|
Claude finished @kyle-sexton's task in 36s —— View job Security review — no issues found
Skip gate applied: all four changed files are documentation prose only — No security issues found. |
|
Last security-reviewed head: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c2091869d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
The parity gate reads any change under a plugin's directory as requiring a version bump and a matching entry, and this change set edits the audit skill's spoke. 0.2.0 is published, so editing under it in place reuses a released number. Patch rather than minor: no behavior changes. The permissive-branch guard rule moved to the topic-docs convention that owns the guard, and the spoke now cites it instead of deriving it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 39s —— View job Security review — no issues found
Skip gate applied (condition 2 — no security-relevant surface): this run covers the push since the last reviewed head ( Reviewing the PR as a whole reaches the same conclusion: all six changed files ( No security issues found. |
|
Claude finished @kyle-sexton's task in 5m 48s —— View job CI code review (
|
…-no-checkout #2737 landed the rule-id crosswalk and took mutation-testing 0.2.0 -> 0.3.0, colliding with this branch's 0.2.1. Both conflicts were version races, not content: 0.2.1 becomes 0.3.1 above their 0.3.0, and both changelog entries are kept in order. The feared collision did not occur. #2737 edited detector-findings/README.md elsewhere in the file, so the self-ignore-guard bullet this branch rewrites auto-merged intact, and the spoke's pointer survived unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 19s —— View job Security review — no issues found
Scope for this run: the trigger is a
Skip gate applied (condition 2 — no security-relevant surface): every one of these is documentation/config prose (a convention README, two CHANGELOGs, a plugin manifest version bump, and a skill context doc) describing/pointing to when a "self-ignore guard" runs. No executable code, no GitHub Actions config, no permissions, credential handling, or injection surface changed. The On the Codex review comment ("Refuse artifact writes when checkout detection fails"): this raises a documentation-soundness question about whether No security issues found. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…conditional #2737 added an owner table naming this binding as owning the self-ignore guard, so a reader now arrives here first. The bullet restated create-when-absent and ended "per the contract" but named none of the roots at which the contract says the guard does not run — so through that doorway it reads as unconditional. The omission predates this branch: root-equivalence has been in the convention for a long time and this bullet never carried it. The clause adds a second omission to an already-incomplete restatement rather than creating the defect. Deference, not a copy. The bullet now states that invalid roots exist and points at the convention's Runtime guards for them, enumerating none. Copying the list would put one rule in two places — the shape that left the aridity bar stated six ways — and would still omit root-equivalence, making the binding a differently incomplete copy. A pointer cannot drift. Not touched: fanout/SKILL.md and fix-pass-mode.md INVOKE the guard rather than defining it, so an invocation resolves to the repaired definition. Editing them would be a third and fourth copy of a rule that should exist once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 43s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed
Skip gate applied (condition 2 — no security-relevant surface): all three are documentation prose and a version/changelog bump. The Reviewing the PR as a whole reaches the same conclusion: all nine changed files ( No security issues found. |
…-no-checkout #2753 published mutation-testing 0.3.1 for the Stryker4s --since correction and the write-regime column, taking the number this branch had claimed. Renumbered to 0.3.2 above it; both entries kept, theirs under the released 0.3.1 heading and mine under a new one, so no published heading is reused or dropped. No content collision. #2753 edited principles/reference/tooling.md, which this branch does not touch, and its write-regime column complements Phase 0's three-regime resolution in SKILL.md rather than restating it — the table gives the per-tool key to read, the skill gives what to do with it. The guard clause, the deference clause, and the spoke pointer are untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 3m 35s —— View job CI code review (
|
|
Claude finished @kyle-sexton's task in 1m 27s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed I confirmed this directly: Skip gate applied: condition 3 (this head already has a successful security review that still applies) — the diff content is unchanged from the prior review; condition 2 also holds independently, since every changed file remains documentation/config prose (convention docs, CHANGELOGs, plugin manifest version bumps, a skill context doc) describing when the "self-ignore guard" runs. No security issues found. |
|
Claude finished @kyle-sexton's task in 3m 27s —— View job CI code review (
|
… too Second plugin binding restating the self-ignore guard unconditionally, same shape as review's before its deference clause. This plugin resolves through the same convention and so reaches the same no-project-root default, where its readers met text saying the guard always creates. Same form as review's: state that invalid roots exist, point at the convention's Runtime guards, enumerate none. Copying the list would put one rule in a third place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-no-checkout Fourth collision, and the first that was not a version race: #2760 published topic-docs 2.4.3 correcting the prune-recovery pointer for squash-merge, in the same CHANGELOG this branch adds 2.5.0 to. Both entries kept, 2.5.0 above 2.4.3. The README auto-merged, which is the case worth checking rather than trusting: #2760 edited the contract-slice lifecycle, this branch edits Runtime guards, and a diff of the merged file against origin/main shows the guard clause as the ONLY delta — so their correction is intact and nothing of theirs was reverted by an older copy winning silently. Same check applied to the other two shared files: every line this branch removes against main is its own superseded text, none of main's. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 49s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed
Skip gate applied (condition 2 — no security-relevant surface): all three are documentation prose and a version/changelog bump. No executable code, no GitHub Actions config, no permissions, credential handling, or injection surface changed. Reviewing the PR as a whole ( Note: the code-review lane's finding on No security issues found. |
|
Claude finished @kyle-sexton's task in 2m 17s —— View job CI code review (
|
Resolve topic-docs CHANGELOG with both 2.5.0 (this PR) and 2.4.4 (main). Bump detector-findings to 2.0.1 for the rewritten self-ignore bullet. Point Runtime guards citations at the blob URL with fragment. Bump mutation-testing to 0.3.3 so it stacks after the arid-bar 0.3.2 lane. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
|
Addressed review threads on this head:
|
… to govern A P2 on #2756 is right, and measurement confirms it: the undecidability that stops the self-ignore guard binds the findings file too. Reproduced — commit a findings-file path, delete it from disk, write it again: ` M <path>`, a modified tracked file, not the untracked one the text claimed. So on the branch where neither signal finds a governing checkout, this producer now writes NOTHING and reports the resolved destination instead. "Read-only with respect to tracked source" admits no write that may land on a tracked deletion. The convention states the general form: the same undecidability binds every write on that branch, and a surface whose artifact must not modify tracked content refuses the artifact write too. A surface that can tolerate an untracked path being staged later may proceed — the two outcomes differ in what they protect, and only the first is absolute. detector-findings gains its missing version bump (2.0.0 -> 2.0.1, docs-only per its own Versioning section) and its consequence sentence drops the "untracked" claim the same measurement falsified. Both binding pointers move from raw.githubusercontent.com to the rendered blob URL with a #runtime-guards fragment: raw serves plain text with no anchor ids, so link text promising a section could not reach it. That is the repo's established cross-reference form. Swept for text the refusal invalidates: the "Where the file goes" obligation now names the second invalid case two sections before the rule, the at-most-two-writes preamble and step 1 say the empty-signals branch writes nothing, the bare-repo case is relabelled a conservative refusal it cannot distinguish from core.worktree, and SKILL.md, the plugin CHANGELOG and eval 13 follow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ions, not a blanket refuse
A concurrent push answered the same P2 differently, and its counterargument is
decisive: a blanket refusal on the no-checkout branch would also refuse the
${CLAUDE_PLUGIN_DATA} fallback, which the convention routes rootless
non-interactive runs to and which sits outside every checkout BY CONSTRUCTION.
That destination cannot be a tracked deletion, so refusing it protects nothing
and strands the one path such a run is meant to use.
Neither side was wholly right. Their text kept "it lands untracked", which
measurement falsifies — commit a findings path, delete it, write it again, and
git reports a modified tracked file. My text refused every write, which
over-corrects into the fallback.
Resolved to the distinction both halves imply: refuse where a RESOLVED ROOT has
no governing checkout, write where the destination is the plugin-data surface.
Stated once in the convention, read by detector-findings, and applied by the
spoke; SKILL.md, the plugin CHANGELOG and eval 13 follow.
Also restores the "Where the file goes" carve-out pointer lost in an earlier
revert, so the obligation a reader meets first names the case that qualifies it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keep mutation-testing 0.3.3 (pointer) stacked on 0.3.2 (arid bar). Bump detector-findings docs to 2.0.2 for the self-ignore bullet after 2.0.1's aridity-row patch from #2759. Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…crimination #2759 merged, bringing the guard-conditioning restorations, the aridity-bar work and eval fixes onto main. Five conflicts, all resolved toward whichever side was later or measured rather than by side: - SKILL.md contract item 1: main's wording, which already conditions the guard on a governing checkout. - SKILL.md Phase 6: both, composed — main's guard condition plus this branch's findings-file withholding and the plugin-data exception. - evals.json: main's file taken WHOLE, then eval 13's clause re-applied. My version would have dropped four of main's eval corrections; a --ours resolve here was wrong and is why the file was rebuilt from main instead. - Versions: main published mutation-testing 0.3.2 and detector-findings 2.0.1, so this branch moves to 0.3.3 and 2.0.2 above them. No published heading reused; --check-preserved compares 80 across 5 changelogs. Post-merge check run on every shared file per the lesson from the seven-site revert: every line this branch removes against main is its own superseded text. The `raw.githubusercontent.com` pointers it drops are replaced by blob URLs with a #runtime-guards fragment; the guard-obligation and Phase 6 sentences it drops are replaced by conditioned forms. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
…n-sync paths Both this branch and a concurrent push synced the same origin/main, producing a criss-cross whose changelog conflicts nested to three levels. Resolved by keeping every entry once, in version order: detector-findings 2.0.2 above the 2.0.1 main published, and mutation-testing 0.3.3 above main's 0.3.2. No entry from either side is dropped; --check-preserved compares 82 headings across 5 changelogs against the merged main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…t' into fix/self-ignore-guard-no-checkout
|
Claude finished @kyle-sexton's task in 58s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed
Reviewing the full PR diff against
Skip gate applied (condition 2 — no security-relevant surface): every file is documentation/config prose or JSON eval-fixture text:
No security issues found. |
…rvives the refusal
The "at most two writes" framing and the carve-out pointer were restored earlier
in this branch, but the argument under them was not: that the write taken on the
no-checkout branch is PROVEN rather than merely permitted.
It needed restating rather than restoring, because the rule changed under it. At
a resolved root the branch now refuses both writes, so the surviving write is the
${CLAUDE_PLUGIN_DATA} fallback — and what proves that one safe is step 1 itself.
Two independent signals agreeing that no checkout governs the path IS the proof
that none can track it: there is no ignore rule to satisfy and nothing for
check-ignore to answer. A proof, not an exemption, which is why step 1 needs both
signals and why a single-signal version would be fail-open rather than weaker.
Also corrects the carve-out pointer to say "a resolved root" — "writes nothing at
all" was inexact once the plugin-data fallback became an explicit exception.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 5m 25s —— View job CI code review (
|
|
Claude finished @kyle-sexton's task in 47s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed
The edit rewords the "Prove the destination is outside tracked space before writing to it" section: it clarifies that step 1's "no write happens" applies at a resolved root, and adds a paragraph explaining why the one write that does survive that branch (the Skip gate applied (condition 2 — no security-relevant surface): the change is prose in a markdown skill-context doc — no executable code, no shell/script logic, no GitHub Actions config, no permissions, credential handling, or injection surface. Reviewing the PR as a whole ( No security issues found. |
…ssed for the new rule The P2 on #2756 is right about case 9. The pushed commit e6bcd8f states the self-ignore guard unconditionally, losing both qualifications #2759 published in 0.3.2: "where a checkout governs that root" and "does not run it at all where no checkout governs the destination". Verified by count against the commit, not the working tree: 1 on origin/main, 0 at e6bcd8f. That is a genuine revert, not supersession — the destination discrimination makes the unqualified form MORE wrong, not less, since the run now refuses every write at an ungoverned resolved root. A graded case on this branch would have certified behavior this PR forbids. Restored from origin/main and then re-expressed, because main's text alone is still wrong under the new rule: it went on to assert "Writes exactly one file" unconditionally. Case 9 now carries the refusal and the ${CLAUDE_PLUGIN_DATA} exception. Case 13 is NOT a revert and is left as-is: main's "SHAPED around it / skipping the guard makes that topology harmless" grades the OLD rule the P2 falsified. Swept all 14 cases against origin/main programmatically rather than by inspection: 14/14 present, only 9 and 13 differ, both deliberate. No other case of main's work was dropped by the earlier rebuild. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 1m 1s —— View job Security review — no issues found
Scope for this run: trigger is the push that landed
This is a follow-up to the eval-content regression flagged in an earlier review round (a merge had silently reverted guard-conditioning text in eval cases 3, 9, 11, and 13 back to pre-fix wording). Diffing the change confirms all four cases are now restored to reflect this PR's own rule:
Skip gate applied (condition 2 — no security-relevant surface): the sole changed file is JSON eval fixture text — Reviewing the PR as a whole reaches the same conclusion as prior rounds: all 15 changed files ( No security issues found. |
2 participants
Follow-up to #2715, which merged while this clause was still under review. Split onto its own branch off
main.No linked issue — this closes no GitHub issue. It repairs a self-contradiction in a convention doc that #2715's merge left live on
main.The defect
docs/conventions/topic-docs/README.mdcontradicts itself on its own default path.Its no-project-root fallback routes non-interactive runs to
${CLAUDE_PLUGIN_DATA}/topic-docs/<slug>/— a destination outside every checkout — and non-interactive is the normal condition for forked subagents, dispatched workers, and headless runs. Its self-ignore-guard bullet nonetheless spoke unconditionally about that same destination, so every consumer reaching that surface ran create-when-absent against a root no checkout governs.This closes a gap rather than carving an exception, and it binds every topic-docs consumer —
review:fanoutincluded, not only detector producers.The rule
Two outcomes bind the guard:
The guard is the means to A wherever a governing checkout is found. Where none is detected it buys nothing toward A, and its create-when-absent rule can violate B. So it does not run there.
"Not detected" stays a detection claim and never shades into "none exists" — the branch is entered precisely where detection can be wrong, which is why the rule is do not write rather than nothing is at risk.
Why blanket, by derivation rather than generalization
A
.gitignoreabsent from disk is either:Separating those requires querying a checkout, and this branch is defined by having found none — the index check that would decide it is the one check that cannot run. The costs are unequal: guessing "untracked" and being wrong modifies content committed in a repository the producer cannot see; guessing "tracked" and being wrong forgoes a mitigation for a harm that is reachable rather than automatic. Undecidable test with asymmetric outcomes yields do not write.
The tracked-file case was measured on the
core.worktreetopology, where a repository governs a tree with no.gitin the destination's path and nothing in the environment to find: the guard replaced committed content, andcheck-ignorereturned exit 1 on its own file. That measurement is one route into the state, not its definition —${CLAUDE_PLUGIN_DATA}is another, and is reached by default.Two dependents
detector-findings/README.md— its consequence sentence was false in this case, and is corrected rather than softened, because a consequence sentence is exactly what belongs there. Untracked is not committed: where detection missed a checkout, the findings file lands there untracked, reachable by a latergit add -A, not committed by this producer's action.mutation-testing's spoke — drops its local derivation for a pointer, now that the rule has an owner. Behavior unchanged;mutation-testing0.2.0 → 0.2.1 (patch), since the parity gate reads any edit under a plugin directory as requiring a bump and 0.2.0 is published.Gates
markdownlint-cli2typosoverdocs/conventions+ the plugincheck-contract-clause-coverage.pycheck-contract-slice-prune.sh --check-diff origin/maincheck-shell-portability.sh --pathscheck-changed-skills.sh origin/maincheck-changelog-parity.sh--check/--check-bump/--check-preservedConvention version: topic-docs 2.4.2 → 2.5.0 (additive rule).
Related
d6440a56; this clause was held pending a location ruling and did not make that merge, leaving the contradiction live onmain. Not closed by this PR.docs/conventions/detector-findings/README.md. This PR touches exactly one hunk there: the self-ignore-guard bullet under "What the binding leaves to a producer". No overlap elsewhere in that file.🤖 Generated with Claude Code