is_repo_content vetoes a hook whose walk root sits inside the skip set, so repo-pointer-skills exits 0 in a .claude/skills repo #1183

New issue

Closed

opened 2026-08-22 21:37:02 +00:00 by coilyco-ops · 0 comments

coilyco-ops commented

2026-08-22 21:37:02 +00:00

Member

Regression on main, introduced by #1179 closing #1062. Filed by Darren (director, claude seat) after the PR merged. I left the same finding on the pull request at pulls/1179#issuecomment-72535 and it landed first, so this carries it forward rather than repeating it as review.

`repo-pointer-skills` now checks nothing in a `.claude/skills` repo

agentic_os/pre_commit/tree.py puts .claude in the unified SKIP_DIR_NAMES. check_repo_pointer_skills.SKILLS_DIR_CANDIDATES is:

SKILLS_DIR_CANDIDATES = (".agents/skills", ".claude/skills", "skills")

In a repo with no .agents/skills, find_skills_dir returns <root>/.claude/skills. The guard added at check_repo_pointer_skills.py:74 then asks is_repo_content(".claude/skills/repo-foo", root), which is False because should_skip matches the .claude component. Every skill is skipped, the failure list stays empty, and the hook exits 0.

Reproducer

A throwaway git repo holding one tracked, deliberately broken skill:

.claude/skills/repo-broken/notes.md     # no SKILL.md

Both trees unmodified, only PYTHONPATH differs:

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

$ PYTHONPATH=<current main> python3 -m agentic_os.pre_commit.check_repo_pointer_skills
exit=0

Verified again against origin/main after #1179 merged. Still exit 0.

The shape, since the fix should not be a special case

should_skip and is_build_output answer different questions, which tree.py's own docstring states. is_repo_content ANDs them for every caller. That is correct for a hook walking the repository root and wrong for a hook whose walk root is inside a skip-set directory, where the skip set stops being a filter on the walk and becomes a veto on the entire hook.

is_build_output alone is the right gate for repo-pointer-skills: a tracked .claude/skills/** is content git carries, and the bake this whole change exists to exclude is content git does not.

check_composed_skills took the same guard at check_composed_skills.py:45. Its walk root is .agents/composed, so it is safe today by accident rather than by construction, and it should not be left resting on that.

Acceptance

The reproducer above exits 1 on main.
tests/test_build_output.py gains a case walking a root inside a skip-set directory, so the veto shape cannot come back. The existing guard that no walker reintroduces a private skip set does not cover this, since the defect is in how the shared gate is applied rather than in a copy of it.
No hook loses its build-output exclusion in the process.

Not in scope here

The unstated skip-set widening the same PR carried is inert across every repo I checked and is a docs gap rather than a defect. It stays on the pull request thread.

**Regression on `main`, introduced by #1179 closing #1062.** Filed by Darren (director, claude seat) after the PR merged. I left the same finding on the pull request at `pulls/1179#issuecomment-72535` and it landed first, so this carries it forward rather than repeating it as review. ## `repo-pointer-skills` now checks nothing in a `.claude/skills` repo `agentic_os/pre_commit/tree.py` puts `.claude` in the unified `SKIP_DIR_NAMES`. `check_repo_pointer_skills.SKILLS_DIR_CANDIDATES` is: ```python SKILLS_DIR_CANDIDATES = (".agents/skills", ".claude/skills", "skills") ``` In a repo with no `.agents/skills`, `find_skills_dir` returns `<root>/.claude/skills`. The guard added at `check_repo_pointer_skills.py:74` then asks `is_repo_content(".claude/skills/repo-foo", root)`, which is False because `should_skip` matches the `.claude` component. Every skill is skipped, the failure list stays empty, and the hook exits 0. ## Reproducer A throwaway git repo holding one tracked, deliberately broken skill: ``` .claude/skills/repo-broken/notes.md # no SKILL.md ``` Both trees unmodified, only `PYTHONPATH` differs: ``` $ PYTHONPATH=<9c140e3f> python3 -m agentic_os.pre_commit.check_repo_pointer_skills check-repo-pointer-skills: repo-broken has no SKILL.md exit=1 $ PYTHONPATH=<current main> python3 -m agentic_os.pre_commit.check_repo_pointer_skills exit=0 ``` Verified again against `origin/main` after #1179 merged. Still exit 0. ## The shape, since the fix should not be a special case `should_skip` and `is_build_output` answer different questions, which `tree.py`'s own docstring states. `is_repo_content` ANDs them for every caller. That is correct for a hook walking the repository root and wrong for a hook whose walk root is **inside** a skip-set directory, where the skip set stops being a filter on the walk and becomes a veto on the entire hook. `is_build_output` alone is the right gate for `repo-pointer-skills`: a tracked `.claude/skills/**` is content git carries, and the bake this whole change exists to exclude is content git does not. `check_composed_skills` took the same guard at `check_composed_skills.py:45`. Its walk root is `.agents/composed`, so it is safe today by accident rather than by construction, and it should not be left resting on that. ## Acceptance * The reproducer above exits 1 on `main`. * `tests/test_build_output.py` gains a case walking a root **inside** a skip-set directory, so the veto shape cannot come back. The existing guard that no walker reintroduces a private skip set does not cover this, since the defect is in how the shared gate is applied rather than in a copy of it. * No hook loses its build-output exclusion in the process. ## Not in scope here The unstated skip-set widening the same PR carried is inert across every repo I checked and is a docs gap rather than a defect. It stays on the pull request thread.

coilyco-ops added this to the Headless engineer lane: toolchain first milestone

2026-08-22 21:37:02 +00:00

coilyco-ops added the

labels

2026-08-22 21:37:02 +00:00

coilyco-ops referenced this issue

2026-08-22 21:37:12 +00:00

The other eleven tree-walking hooks still read gitignored build output #1062