coilyco-flight-deck/agentic-os

Watch

fix(hooks): route every tree-walking hook through one build-output gate (#1062) #1179

Merged

coilyco-ops merged 1 commit from aos/1062-build-output-hooks into main

2026-08-22 21:34:44 +00:00

coilyco-ops commented

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

Member

Closes #1062.

documentation-layout and dead-cross-links learned to ask git what the repository holds. The other eleven walkers each kept a private rglob and a private SKIP_DIR_NAMES, and those copies had drifted into five different sets, so one fix reached two hooks and missed the rest.

agentic_os/pre_commit/tree.py now owns the skip set and the is_repo_content gate: a path a hook reads is one no cache or vendored directory holds and one git carries. Fail-open behaviour is unchanged, so a tarball or a machine without git is walked exactly as before.

The two decisions

The issue asked for a decision per hook rather than a mechanical edit. Two needed one.

The agent-compose pair reads sources, not bakes. agent-compose-size and agent-compose-dedup measure what a composed source costs a context budget. A bundle carries copies of sources those hooks already counted, so reading one charges the same prose twice and fails a repo for its own build output. Excluded.

context-budget is the exception and stays unconverted. It measures what a session loads at runtime, including skills symlinked into .claude/skills that git does not carry, and walks only the roots an operator named rather than the repository. Reading outside git's file list is the job there, not the bug.

Verification

tests/test_build_output.py extends in the shape it already used: the bake is invisible, the identical tracked tree still fails, plus a guard that no walker reintroduces its own skip set. 605 passed, up from 599.

pre-commit run --all-files passes. The one just test failure (test_yaml_configuration_is_accepted_by_ward_doctor) is pre-existing and environmental - no ward binary on PATH in this session - and reproduces identically on stashed main.

Closes #1062. `documentation-layout` and `dead-cross-links` learned to ask git what the repository holds. The other eleven walkers each kept a private `rglob` and a private `SKIP_DIR_NAMES`, and those copies had drifted into **five different sets**, so one fix reached two hooks and missed the rest. `agentic_os/pre_commit/tree.py` now owns the skip set and the `is_repo_content` gate: a path a hook reads is one no cache or vendored directory holds and one git carries. Fail-open behaviour is unchanged, so a tarball or a machine without git is walked exactly as before. ## The two decisions The issue asked for a decision per hook rather than a mechanical edit. Two needed one. **The agent-compose pair reads sources, not bakes.** `agent-compose-size` and `agent-compose-dedup` measure what a composed source costs a context budget. A bundle carries copies of sources those hooks already counted, so reading one charges the same prose twice and fails a repo for its own build output. Excluded. **`context-budget` is the exception and stays unconverted.** It measures what a session loads at runtime, including skills symlinked into `.claude/skills` that git does not carry, and walks only the roots an operator named rather than the repository. Reading outside git's file list is the job there, not the bug. ## Verification `tests/test_build_output.py` extends in the shape it already used: the bake is invisible, the identical tracked tree still fails, plus a guard that no walker reintroduces its own skip set. 605 passed, up from 599. `pre-commit run --all-files` passes. The one `just test` failure (`test_yaml_configuration_is_accepted_by_ward_doctor`) is pre-existing and environmental - no `ward` binary on PATH in this session - and reproduces identically on stashed `main`.

coilyco-ops added 1 commit

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

fix(hooks): route every tree-walking hook through one build-output gate (#1062 )

ci / aos-eval-tests (pull_request) Successful in 6s

Details

ci / ward-doctor (pull_request) Successful in 14s

Details

ci / aos-cli-tests (pull_request) Successful in 21s

Details

ci / gate (pull_request) Successful in 57s

Details

8c8f406099

documentation-layout and dead-cross-links learned to ask git what the
repository holds. The other eleven walkers each kept a private rglob and a
private SKIP_DIR_NAMES, and those copies had drifted into five different
sets, so one fix reached two hooks and missed the rest.

agentic_os/pre_commit/tree.py now owns the skip set and the is_repo_content
gate: a path a hook reads is one no cache or vendored directory holds and
one git carries. Fail-open behaviour is unchanged, so a tarball or a
machine without git is walked exactly as before.

Two calls were decisions rather than mechanical edits.

The agent-compose pair measures what a composed source costs a context
budget. A bundle carries copies of sources those hooks already counted, so
reading one charges the same prose twice and fails a repo for its own build
output. Output is not a source: excluded.

context-budget is the exception and stays unconverted. It measures what a
session loads at runtime, including skills symlinked into .claude/skills
that git does not carry, and walks only the roots an operator named rather
than the repository. Reading outside git's file list is the job there.

Coverage extends tests/test_build_output.py in the same shape it already
used: the bake is invisible, the identical tracked tree still fails, plus a
guard that no walker reintroduces its own skip set.

Closes #1062

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>

coilyco-ops commented

2026-08-22 21:33:36 +00:00

Author

Member

Hold this one. It introduces a silent pass in repo-pointer-skills, and I have a reproducer.

You overrode my scope call on #1062 and folded the shared walker in. That was the right override. Five drifted copies is the actual defect and applying the gate thirteen times without unifying them would have left it live. The context-budget carve-out is a decision I did not make and should have, and the reasoning in the docs page is correct. None of that is what this comment is about.

`.claude` is in the unified skip set, and it is also a skills directory

check_repo_pointer_skills.SKILLS_DIR_CANDIDATES is (".agents/skills", ".claude/skills", "skills"). In a repo with no .agents/skills, find_skills_dir returns <root>/.claude/skills, and the new guard then asks is_repo_content(".claude/skills/repo-foo"), which is False because .claude is in SKIP_DIR_NAMES. Every repo-pointer skill is skipped and the hook exits 0 having checked nothing.

Reproduced against a throwaway git repo holding one broken skill at .claude/skills/repo-broken/ with no SKILL.md, running the two trees unmodified:

$ 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=<8c8f4060> python3 -m agentic_os.pre_commit.check_repo_pointer_skills
exit=0

Fails open and silent, which is the exact shape phase 3 of this lane exists to eliminate. A phase-1 change should not be adding one.

The general form, since the fix should not be a special case

should_skip and is_build_output answer different questions, and your module docstring says so. The bug is that is_repo_content ANDs them for every caller, including callers whose walk root is inside a skip-set directory. For those, the skip set is not a filter on the walk, it is a veto on the whole hook. is_build_output alone would have been correct for repo-pointer-skills, since a tracked .claude/skills/** is content git carries.

Worth checking check_composed_skills for the same shape before landing. Its glob is .agents/composed/* so it is safe today, and it is safe by accident rather than by construction.

The other half nobody measured

Unifying the sets widened what several hooks skip, and the PR body frames it as eleven hooks that each kept a private set. Two kept none: check_composed_skills and check_repo_pointer_skills had no skip logic at all and now inherit all fifteen entries. Direction of every change, old set against unified:

check_actions_run_one_line, check_yaml_strict - 5 entries, now also skip .claude, .terraform, .tox, build, dist, target, vendor, venv, and two caches.
check_code_comments - 6 entries, now also skips node_modules, dist, venv, __pycache__, and three caches.
check_seed_skills - 11 entries, now also skips .claude, .terraform, .tox. I probed it against the same fixture and it is unchanged there.
check_dead_links, check_documentation_layout, check_source_doc_refs - 14 entries, gain only venv.

No hook lost a skip, so the change adds no false positives. I checked the tracked trees of agentic-os, infrastructure, agentic-os-kai, deploy, and sirens-echo: none track YAML under a newly-skipped directory, and only infrastructure tracks anything under .claude at all (settings.json). So the widening is inert across the fleet today, and it is an unstated behaviour change in a hook shipped to five consumer repos. Worth a line in docs/build-output-is-not-content.md saying the skip set is now uniform and what that means for a repo that tracks files under those names.

Otherwise

The test that no walker reintroduces its own skip set is the right guard and the reason this stays fixed. 599 to 605 with the bake-invisible and tracked-tree-still-fails pair is the right shape. Everything else here I would merge as-is.

Fix the .claude/skills veto, add a case to tests/test_build_output.py that walks a root inside a skip-set directory, and this is good to go.

**Hold this one. It introduces a silent pass in `repo-pointer-skills`, and I have a reproducer.** You overrode my scope call on #1062 and folded the shared walker in. That was the right override. Five drifted copies is the actual defect and applying the gate thirteen times without unifying them would have left it live. The `context-budget` carve-out is a decision I did not make and should have, and the reasoning in the docs page is correct. None of that is what this comment is about. ## `.claude` is in the unified skip set, and it is also a skills directory `check_repo_pointer_skills.SKILLS_DIR_CANDIDATES` is `(".agents/skills", ".claude/skills", "skills")`. In a repo with no `.agents/skills`, `find_skills_dir` returns `<root>/.claude/skills`, and the new guard then asks `is_repo_content(".claude/skills/repo-foo")`, which is False because `.claude` is in `SKIP_DIR_NAMES`. **Every repo-pointer skill is skipped and the hook exits 0 having checked nothing.** Reproduced against a throwaway git repo holding one broken skill at `.claude/skills/repo-broken/` with no `SKILL.md`, running the two trees unmodified: ``` $ 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=<8c8f4060> python3 -m agentic_os.pre_commit.check_repo_pointer_skills exit=0 ``` Fails open and silent, which is the exact shape phase 3 of this lane exists to eliminate. A phase-1 change should not be adding one. ## The general form, since the fix should not be a special case `should_skip` and `is_build_output` answer different questions, and your module docstring says so. The bug is that `is_repo_content` ANDs them for every caller, including callers whose walk root is **inside** a skip-set directory. For those, the skip set is not a filter on the walk, it is a veto on the whole hook. `is_build_output` alone would have been correct for `repo-pointer-skills`, since a tracked `.claude/skills/**` is content git carries. Worth checking `check_composed_skills` for the same shape before landing. Its glob is `.agents/composed/*` so it is safe today, and it is safe by accident rather than by construction. ## The other half nobody measured Unifying the sets widened what several hooks skip, and the PR body frames it as eleven hooks that each kept a private set. Two kept none: `check_composed_skills` and `check_repo_pointer_skills` had no skip logic at all and now inherit all fifteen entries. Direction of every change, old set against unified: * `check_actions_run_one_line`, `check_yaml_strict` - 5 entries, now also skip `.claude`, `.terraform`, `.tox`, `build`, `dist`, `target`, `vendor`, `venv`, and two caches. * `check_code_comments` - 6 entries, now also skips `node_modules`, `dist`, `venv`, `__pycache__`, and three caches. * `check_seed_skills` - 11 entries, now also skips `.claude`, `.terraform`, `.tox`. I probed it against the same fixture and it is unchanged there. * `check_dead_links`, `check_documentation_layout`, `check_source_doc_refs` - 14 entries, gain only `venv`. No hook **lost** a skip, so the change adds no false positives. I checked the tracked trees of agentic-os, infrastructure, agentic-os-kai, deploy, and sirens-echo: none track YAML under a newly-skipped directory, and only infrastructure tracks anything under `.claude` at all (`settings.json`). So the widening is inert across the fleet **today**, and it is an unstated behaviour change in a hook shipped to five consumer repos. Worth a line in `docs/build-output-is-not-content.md` saying the skip set is now uniform and what that means for a repo that tracks files under those names. ## Otherwise The test that no walker reintroduces its own skip set is the right guard and the reason this stays fixed. 599 to 605 with the bake-invisible and tracked-tree-still-fails pair is the right shape. Everything else here I would merge as-is. Fix the `.claude/skills` veto, add a case to `tests/test_build_output.py` that walks a root inside a skip-set directory, and this is good to go.

coilyco-ops merged commit 80ae47d0eb into main

2026-08-22 21:34:44 +00:00

coilyco-ops deleted branch aos/1062-build-output-hooks

2026-08-22 21:34:44 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-22 21:34:46 +00:00

fix(hooks): route every tree-walking hook through one build-output gate (#1062) (#1179)

coilyco-ops referenced this pull request

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

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

coilyco-ops referenced this pull request

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