fix(git): stop prefixing rtk where it rewrites the output skills parse
The #113 sweep rested on CLAUDE.md's premise that rtk either filters or passes through unchanged, so prefixing is always safe. Measured against rtk 0.42.4, that premise is false for several of the commands the sweep prefixed, and two skills were left giving wrong answers silently. Why: - `rtk git worktree list --porcelain -z` discards both flags and renders its own format. The `locked`/`lock_reason` fields git-worktrees Step 2 must emit are absent entirely, and paths under $HOME are abbreviated to `~/`. - `rtk git branch --list <name>` prints a phantom `* ` line even when nothing matches, so git-branches' stated ambiguity test — "output from both means the name is ambiguous" — reported every name as ambiguous. `tag --list` is a clean passthrough, so only one half broke. - `rtk git diff --name-only`/`--name-status` append a `Changes:` trailer to output documented as "one per line"; `--word-diff` emits none of the `[-removed-] {+added+}` markers its table describes; `rtk git log -L` truncates each line at ~72 chars, on the one command whose purpose is showing line content. - `rtk git stash pop` prints only `FAILED: git stash pop`, swallowing the conflict diagnostic and retained-entry message the surrounding prose tells the agent to rely on. Implementation notes: - Eleven sites reverted to bare `git`, each carrying its reason inline so the next sweep does not undo it. `mergetool` and `rebase -i` are reverted on clause 3's interactive limb only: the TTY defect does not reproduce — rtk filters exactly twelve subcommands and execs the rest — and ADR-0023 records that measurement rather than a convenient one. - ADR-0023 states the rule repo-wide with a third clause: a command whose output the skill parses, or which is interactive, stays bare. `plugins/git/README.md` is reduced to a pointer; its claim that gitea skills "contain no git/rtk mentions at all" was false, and its citation of `hard-rules.md` pointed at a file containing no occurrence of "rtk". - Eight gitea sites swept, all verified byte-identical passthroughs first. - `scripts/check-rtk-prefix.sh` gates clause 1. Run against main's pre-sweep corpus it reports 99 findings including every gitea site, so it would have caught the drift #113 was filed about. Impact: the gate covers clause 1 only, in shell-tagged fences and the opening span of Run cells. Clause 2 is not gateable — "Run `git switch`" and "`git switch` refuses" are the same tokens — and prose bullets are invisible to it. Both limits are recorded in gates.md rather than left implied. Refs: #113 ADR: 0023 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EeH8SCbcrCAQrtymkNuhKP
This commit is contained in:
@@ -561,6 +561,91 @@ pre-commit run --all-files # size AND Vale
|
||||
|
||||
Scoping a retrofit off `skill-size-check` output alone leaves you blocked at the second gate.
|
||||
|
||||
## The `rtk` prefix gate (ADR-0023)
|
||||
|
||||
`check-rtk-prefix` is a `repo: local` pre-commit hook running `scripts/check-rtk-prefix.sh` over
|
||||
`^plugins/[^/]+/\.apm/(skills/.*\.md|agents/.*\.agent\.md)$`, with `README.md` excluded. It enforces
|
||||
**ADR-0023 clause 1 and nothing else**: an executable, instructed local git command in plugin skill
|
||||
or agent content is written `rtk git`.
|
||||
|
||||
It is wider in file scope than the ADR-0020 hooks — every markdown file under a plugin's
|
||||
`.apm/skills/` and `.apm/agents/`, not `SKILL.md` alone — because the rule it enforces is about
|
||||
commands an agent runs, and most of those live in `references/`, which the ADR-0020 gates do not
|
||||
reach ([the `references/` blind spot](#the-blind-spot-references-is-unlinted-for-two-independent-reasons)).
|
||||
|
||||
### What it can decide, and what it declines to
|
||||
|
||||
ADR-0023 has three clauses and only the first is a pattern:
|
||||
|
||||
| Clause | Rule | Gated |
|
||||
|---|---|---|
|
||||
| 1 | executable + instructed → `rtk git` | yes |
|
||||
| 2 | illustrative / referential → bare `git` | no — undecidable |
|
||||
| 3 | machine-parsed or interactive → bare `git` | no — opt-out marker |
|
||||
|
||||
Clause 2 is a judgement about what a sentence is *doing*. "Run `git switch <branch>`" and "`git
|
||||
switch` refuses rather than clobbering local edits" are the same token sequence. A gate that guessed
|
||||
would fire on correct prose, and **a gate that fires on correct content gets added to `SKIP`** —
|
||||
which disarms clause 1 along with it. So the hook looks only at the two contexts where a `git`
|
||||
mention is unambiguously an instruction to execute:
|
||||
|
||||
- a line inside a fenced code block whose info string names a shell — `bash`, `sh`, `shell`, `zsh`,
|
||||
`console`, `shell-session`. Fences tagged `text`, `yaml`, `json`, or tagged with nothing, are **not**
|
||||
checked;
|
||||
- the **opening** backticked span of a "Run" column cell in a markdown dispatch table, and only the
|
||||
opening span.
|
||||
|
||||
That last narrowing is not fussiness. A Run cell routinely carries a command followed by prose about
|
||||
it, and the prose is clause 2. `git-worktrees/SKILL.md` has both shapes on adjacent rows — one cell
|
||||
reading `` `rtk git worktree add --track …` `` — always correct. `` `git worktree add <path>
|
||||
<branch>` `` expands to exactly this (instruction, then reference), and a `**Never** …` row whose Run
|
||||
cell is entirely explanation containing a bare `git push`. Checking every backticked span flags both;
|
||||
checking only a leading span flags neither, and still catches the ordinary
|
||||
`` | List | `git worktree list -v` | `` case the gate exists for.
|
||||
|
||||
### The clause-3 opt-out
|
||||
|
||||
A command that is deliberately bare — because rtk rewrites the output the skill parses, or because
|
||||
the command is interactive — is exempted by putting the literal string `ADR-0023` **on the same
|
||||
line**: in a shell comment for a code line, in the cell text for a table row.
|
||||
|
||||
Per line, never per block. A fenced procedure routinely mixes `rtk git` steps with one deliberately
|
||||
bare command (`git-remotes/references/push.md` does exactly that), and a block-level marker would
|
||||
silently disarm every checked line around the marked one. The cost is a repeated `# bare per
|
||||
ADR-0023` in the three blocks of `git-log-format.md` where every line is deliberately bare; that
|
||||
repetition is the price of the marked line being the only line the marker speaks for.
|
||||
|
||||
The marker is a plain substring match, so a line that mentions `ADR-0023` for an unrelated reason is
|
||||
also exempt. Accepted deliberately: the marker records an author's opt-out, it is not a security
|
||||
boundary, and a stricter form would only move the same trust to a different string.
|
||||
|
||||
### What it deliberately does not cover
|
||||
|
||||
- **Clause 2.** Nothing checks that an illustrative mention stayed bare. A sweep that re-prefixes a
|
||||
referential `git` passes this gate. The inline reasons ADR-0023 requires on clause-3 sites are the
|
||||
only defence, and they are prose.
|
||||
- **Prose bullets.** Most of `branch-operations.md`, `merging.md` and `rewrite-history.md` instruct
|
||||
in list items, not fences. Those are clause-1 sites the gate cannot see, because it cannot
|
||||
distinguish them from clause-2 mentions in the same list.
|
||||
- **`README.md`, excluded by pattern.** A skill-directory README is consumer-facing prose no agent
|
||||
loads, and the `git clone https://github.com/bats-core/…` lines in the seven `tests/README.md`
|
||||
files are setup instructions for a third party who has no `rtk`. Prefixing those would be actively
|
||||
wrong, not merely noisy — see ADR-0023's consumer section.
|
||||
- **Quoting.** The line splitter breaks on `;`, `|`, `&&`, `||`, `$(` and backticks without tracking
|
||||
quotes, so a git command inside a quoted argument is decided by accident.
|
||||
`rtk git submodule foreach 'git pull origin main || :'` passes because the segment holding the
|
||||
inner command begins with `rtk` — the right answer for the wrong reason. Write
|
||||
`foreach 'git a; git b'` and the second inner command is a false positive needing the marker.
|
||||
ADR-0023 records this shape as one the rule itself does not decide.
|
||||
- **Non-git commands.** Only `git` is checked. `rtk` fronts `gh`, `docker`, `kubectl` and others; no
|
||||
gate covers those, and the corpus does not currently instruct them.
|
||||
|
||||
`tests/test-check-rtk-prefix.sh` pins all of it, including the false-positive cases. Its first case
|
||||
reconstructs the plugin corpus as it stood on `main` before the #113 sweep and asserts the gate
|
||||
fails there with at least 20 findings, one of them the `gitea-*` `git remote get-url origin` drift
|
||||
the sweep missed — a gate that only passes on the already-fixed tree proves nothing about the drift
|
||||
it was written for.
|
||||
|
||||
## Vale
|
||||
|
||||
Install the `vale` binary — `brew install vale` (macOS), `snap install vale` (Linux),
|
||||
|
||||
Reference in New Issue
Block a user