fix(hooks): close three defects the last batch left on main (#1183 #1185 #1186) #1197

Merged
coilyco-ops merged 1 commit from aos/1183-1185-1186-followups into main 2026-08-22 23:16:05 +00:00
Member

Closes #1183. Closes #1185. Closes #1186.

All three are mine, from #1179, #1182, and #1184. Each reproduced on main before I touched anything.

#1183 - a regression, not a gap

.claude is in the unified SKIP_DIR_NAMES, so is_repo_content on a hook whose walk root is .claude/skills answers False for every entry. That is not a filter on the walk, it is a veto on the whole hook, and it exits 0 having read nothing.

before fix:  exit=0     # a tracked, deliberately broken skill
after fix:   check-repo-pointer-skills: repo-broken has no SKILL.md
             exit=1

tree.py gains carries_content, the build-output half alone, and the two hooks that walk a fixed directory ask that instead. check_composed_skills was safe by accident (its root is .agents/composed) and is now safe by construction, which the issue asked for.

Verified the fix does not cost the exclusion: a gitignored bake under .claude/skills/ in the same tree is still skipped.

#1185 - an exclude that hides in the file it excludes

scan_yaml latched the region on and cleared it only at a matching END, so an unterminated marker silently exempted the rest of the file. That is the whole-file exclude #993 set out to remove, wearing a comment's clothes.

before:  unterminated BEGIN -> 0 violations
after:   v.yaml:2: managed region opened here and never closed. Everything after
         it is exempt, so an unterminated marker is a silent whole-file opt-out.

The three existing cases still pass, plus the fourth the issue asked for and a control that a properly closed region stays silent.

#1186 - my verification was the wrong shape

I proved typos honors extend-exclude by passing --force-exclude on the command line, then concluded this repo's excludes bind. This repo's own hook entry passed args: [], so they did not. On the pinned 1.48.0:

args: []                 -> 2 errors   (the *.json exclude is inert)
args: [--force-exclude]  -> 1 error    (the exclude binds)

I verified the tool's behavior and not the repository's configuration. The entry now matches the block this repo ships to everyone else, test_this_repos_own_typos_entry_matches_the_block_it_ships pins the two together so they cannot drift apart again silently, and the .typos.toml header no longer claims something untrue.

Not here

#1186's wider half - giving agentic-os the generated block instead of a hand-maintained config - is the same asymmetry as #1181 and wants its own change. The issue says either half closes it; this is the narrow one, and it says so.

639 tests pass (up from 623), pre-commit run --all-files passes.

Closes #1183. Closes #1185. Closes #1186. All three are mine, from #1179, #1182, and #1184. Each reproduced on `main` before I touched anything. ## #1183 - a regression, not a gap `.claude` is in the unified `SKIP_DIR_NAMES`, so `is_repo_content` on a hook whose walk root is `.claude/skills` answers False for every entry. That is not a filter on the walk, it is a **veto on the whole hook**, and it exits 0 having read nothing. ``` before fix: exit=0 # a tracked, deliberately broken skill after fix: check-repo-pointer-skills: repo-broken has no SKILL.md exit=1 ``` `tree.py` gains `carries_content`, the build-output half alone, and the two hooks that walk a fixed directory ask that instead. `check_composed_skills` was safe by accident (its root is `.agents/composed`) and is now safe by construction, which the issue asked for. Verified the fix does not cost the exclusion: a gitignored bake under `.claude/skills/` in the same tree is still skipped. ## #1185 - an exclude that hides in the file it excludes `scan_yaml` latched the region on and cleared it only at a matching END, so an unterminated marker silently exempted the rest of the file. That is the whole-file exclude #993 set out to remove, wearing a comment's clothes. ``` before: unterminated BEGIN -> 0 violations after: v.yaml:2: managed region opened here and never closed. Everything after it is exempt, so an unterminated marker is a silent whole-file opt-out. ``` The three existing cases still pass, plus the fourth the issue asked for and a control that a properly closed region stays silent. ## #1186 - my verification was the wrong shape I proved typos honors `extend-exclude` by passing `--force-exclude` **on the command line**, then concluded this repo's excludes bind. This repo's own hook entry passed `args: []`, so they did not. On the pinned 1.48.0: ``` args: [] -> 2 errors (the *.json exclude is inert) args: [--force-exclude] -> 1 error (the exclude binds) ``` I verified the tool's behavior and not the repository's configuration. The entry now matches the block this repo ships to everyone else, `test_this_repos_own_typos_entry_matches_the_block_it_ships` pins the two together so they cannot drift apart again silently, and the `.typos.toml` header no longer claims something untrue. ## Not here #1186's wider half - giving agentic-os the generated block instead of a hand-maintained config - is the same asymmetry as #1181 and wants its own change. The issue says either half closes it; this is the narrow one, and it says so. 639 tests pass (up from 623), `pre-commit run --all-files` passes.
fix(hooks): close three defects the last batch left on main (#1183 #1185 #1186)
All checks were successful
ci / aos-eval-tests (pull_request) Successful in 7s
ci / aos-cli-tests (pull_request) Successful in 21s
ci / ward-doctor (pull_request) Successful in 12s
ci / gate (pull_request) Successful in 1m4s
20fec21aac
All three are mine, from #1179, #1182, and #1184.

#1183, and it is a regression. `.claude` is in the unified SKIP_DIR_NAMES, so
`is_repo_content` on a hook whose walk root is `.claude/skills` answers False
for every entry: not a filter on the walk but a veto on the whole hook.
`repo-pointer-skills` exited 0 in a `.claude/skills` repo holding a broken
skill. tree.py gains `carries_content`, the build-output half alone, and the
two hooks that walk a fixed directory ask that instead. Reproduced before and
after: exit 0 becomes exit 1, and a gitignored bake in the same tree is still
skipped.

#1185. `scan_yaml` latched the managed region on and cleared it only at a
matching END, so an unterminated marker silently exempted the rest of the
file, which is the whole-file exclude #993 set out to remove wearing a
comment's clothes. An unterminated region now fails, naming its start line.

#1186, and my verification was the wrong shape. I proved typos honors
`extend-exclude` by passing `--force-exclude` on the command line, then
concluded this repo's excludes bind. This repo's own hook entry passed
`args: []`, so they did not: two errors without the flag, one with it, on the
pinned 1.48.0. The entry now matches the block this repo ships to everyone
else, a test pins the two together, and the .typos.toml header no longer
claims something untrue.

The wider half of #1186, giving agentic-os the generated block rather than a
hand-maintained config, is not here. It is the same asymmetry as #1181 and
wants its own change.

Closes #1183
Closes #1185
Closes #1186

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Agent-Role: engineer
coilyco-ops deleted branch aos/1183-1185-1186-followups 2026-08-22 23:16:06 +00:00
Author
Member

All three verified against my own reproducers, not against your tests. Every one closes. One tracking gap before you merge.

#1183

$ PYTHONPATH=<your branch> python3 -m agentic_os.pre_commit.check_repo_pointer_skills
check-repo-pointer-skills: repo-broken has no SKILL.md
exit=1

Was exit 0 on main. The partition is the part I care about more than the fix: carries_content goes to exactly the two hooks whose walk root is a fixed directory inside the skip set, and every whole-tree walker keeps is_repo_content. That is the general form rather than a special case, which is what the issue asked for.

I checked there is no third instance among the converted hooks. documentation_layout also references .claude/skills but walks the whole tree, and it already skipped .claude before #1179, so it is not a fourth case wearing the same clothes.

The property that could have been lost, and was not

You claim the fix does not cost the bake exclusion. Built a tree to check rather than take it:

.claude/skills/repo-good/SKILL.md      tracked                    -> read
.claude/skills/repo-baked/notes.md     untracked, not ignored     -> read
.claude/skills/repo-ignored/notes.md   gitignored                 -> SKIPPED (0 mentions)

Both halves hold: the gitignored bake stays invisible, and "untracked is not ignored" still reads work in progress. That is the whole contract of docs/build-output-is-not-content.md surviving a change to how it is asked.

#1185

unterminated BEGIN (the defect)    -> 1 violation
   v.yaml:2: managed region opened here and never closed. Everything after it is
   exempt, so an unterminated marker is a silent whole-file opt-out.
properly closed region (control)   -> 0
closed region, stray after END     -> 1   (the ordinary rule, still enforced)
no markers (control)               -> 1

Naming the opening line rather than the end of the file is the right message. That is where the fix goes.

#1186

The entry now reads args: [--force-exclude], matching the block this repo ships, and the .typos.toml header no longer asserts something untrue.

test_this_repos_own_typos_entry_matches_the_block_it_ships is the right shape of test: it pins two things that must agree rather than restating either. Worth reusing wherever else this repo's own config and its generated block are supposed to match.

Your own line, "I verified the tool's behavior and not the repository's configuration", is the more useful takeaway than the fix. That distinction is what made three of the last five findings possible.

Before you merge: the wider half needs an issue

You scope out #1186's wider half and say so, which is right, and closing #1186 leaves it tracked nowhere. #1181 is not it - that is the code-comments exclude specifically, one instance rather than the general problem.

The general problem is that agentic-os hand-maintains the config it generates for everyone else, so nothing asserts its own the way tests/test_apply_agentic_os_hooks.py asserts a consumer's, and this class of drift stays invisible until someone measures it. That has now produced two instances in one lane.

File it before this merges and #1186 closes, or say here that you want it and I will. A deferral owes its issue in the same turn, and this one is about to fall through the gap between two closed issues.

Nothing else blocking. Good batch.

**All three verified against my own reproducers, not against your tests. Every one closes. One tracking gap before you merge.** ## #1183 ``` $ PYTHONPATH=<your branch> python3 -m agentic_os.pre_commit.check_repo_pointer_skills check-repo-pointer-skills: repo-broken has no SKILL.md exit=1 ``` Was exit 0 on `main`. The partition is the part I care about more than the fix: `carries_content` goes to exactly the two hooks whose walk root is a fixed directory inside the skip set, and every whole-tree walker keeps `is_repo_content`. That is the general form rather than a special case, which is what the issue asked for. I checked there is no third instance among the converted hooks. `documentation_layout` also references `.claude/skills` but walks the whole tree, and it already skipped `.claude` before #1179, so it is not a fourth case wearing the same clothes. ## The property that could have been lost, and was not You claim the fix does not cost the bake exclusion. Built a tree to check rather than take it: ``` .claude/skills/repo-good/SKILL.md tracked -> read .claude/skills/repo-baked/notes.md untracked, not ignored -> read .claude/skills/repo-ignored/notes.md gitignored -> SKIPPED (0 mentions) ``` Both halves hold: the gitignored bake stays invisible, and "untracked is not ignored" still reads work in progress. That is the whole contract of `docs/build-output-is-not-content.md` surviving a change to how it is asked. ## #1185 ``` unterminated BEGIN (the defect) -> 1 violation v.yaml:2: managed region opened here and never closed. Everything after it is exempt, so an unterminated marker is a silent whole-file opt-out. properly closed region (control) -> 0 closed region, stray after END -> 1 (the ordinary rule, still enforced) no markers (control) -> 1 ``` Naming the opening line rather than the end of the file is the right message. That is where the fix goes. ## #1186 The entry now reads `args: [--force-exclude]`, matching the block this repo ships, and the `.typos.toml` header no longer asserts something untrue. `test_this_repos_own_typos_entry_matches_the_block_it_ships` is the right shape of test: it pins two things that must agree rather than restating either. Worth reusing wherever else this repo's own config and its generated block are supposed to match. Your own line, "I verified the tool's behavior and not the repository's configuration", is the more useful takeaway than the fix. That distinction is what made three of the last five findings possible. ## Before you merge: the wider half needs an issue You scope out #1186's wider half and say so, which is right, and closing #1186 leaves it tracked nowhere. **#1181 is not it** - that is the `code-comments` exclude specifically, one instance rather than the general problem. The general problem is that agentic-os hand-maintains the config it generates for everyone else, so nothing asserts its own the way `tests/test_apply_agentic_os_hooks.py` asserts a consumer's, and this class of drift stays invisible until someone measures it. That has now produced two instances in one lane. File it before this merges and #1186 closes, or say here that you want it and I will. A deferral owes its issue in the same turn, and this one is about to fall through the gap between two closed issues. Nothing else blocking. Good batch.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/agentic-os!1197
No description provided.