fix(code-comments): exempt generated managed-marker regions in YAML (#993)

coilyco-ops commented

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

Member

Closes #993.

scan_yaml flagged every YAML comment below the first content line, which includes the comments scripts/apply-agentic-os-hooks.py writes into a consumer's .pre-commit-config.yaml at fixed positions: the two block markers, the actionlint note, and the typos note.

None can move to a top header without breaking the delimiters the rollout parses, and an in-repo edit is overwritten on the next rollout. So five repos carried the same whole-file exclude, which also hid a genuinely misplaced hand-written comment anywhere else in the file.

Comment lines between # BEGIN managed by ... and # END managed by ... are now exempt, and enforcement resumes at the END marker - the part a whole-file exclude gave up.

Edges covered

A comment before or after the region is still checked.
The markers do not spend the top-of-file header budget, so a generated config that opens with BEGIN is fine.
A # BEGIN managed by line inside a run: | block scalar is shell, not a marker.
The control test: the same generated lines without markers still produce two violations.

What this does not close

#993 said the fix lets all five repos drop the exclude. It lets four. agentic-os authors the generator rather than being stamped by it, so its own config is hand-written and keeps three below-content notes on the keys they explain. Filed as #1181, and the exclude comment in pyproject.toml now records that reason instead of the one this fix retired.

The four consumer repos drop theirs on the next repin sweep, which is a rollout rather than this repo's change.

pre-commit run --all-files passes, 605 python tests pass. The single just test failure is the pre-existing environmental ward-binary one.

Closes #993. `scan_yaml` flagged every YAML comment below the first content line, which includes the comments `scripts/apply-agentic-os-hooks.py` writes into a consumer's `.pre-commit-config.yaml` at fixed positions: the two block markers, the actionlint note, and the typos note. None can move to a top header without breaking the delimiters the rollout parses, and an in-repo edit is overwritten on the next rollout. So five repos carried the same whole-file exclude, which also hid a genuinely misplaced hand-written comment anywhere else in the file. Comment lines between `# BEGIN managed by ...` and `# END managed by ...` are now exempt, and **enforcement resumes at the END marker** - the part a whole-file exclude gave up. ## Edges covered * A comment before or after the region is still checked. * The markers do not spend the top-of-file header budget, so a generated config that opens with BEGIN is fine. * A `# BEGIN managed by` line inside a `run: |` block scalar is shell, not a marker. * The control test: the same generated lines **without** markers still produce two violations. ## What this does not close #993 said the fix lets all five repos drop the exclude. It lets four. agentic-os authors the generator rather than being stamped by it, so its own config is hand-written and keeps three below-content notes on the keys they explain. Filed as #1181, and the exclude comment in `pyproject.toml` now records that reason instead of the one this fix retired. The four consumer repos drop theirs on the next repin sweep, which is a rollout rather than this repo's change. `pre-commit run --all-files` passes, 605 python tests pass. The single `just test` failure is the pre-existing environmental `ward`-binary one.

coilyco-ops added 1 commit

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

fix(code-comments): exempt generated managed-marker regions in YAML (#993 )

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

Details

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

Details

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

Details

ci / gate (pull_request) Successful in 1m31s

Details

2c96e9e74a

scan_yaml flagged every YAML comment below the first content line, which
includes the comments scripts/apply-agentic-os-hooks.py writes into a
consumer's .pre-commit-config.yaml at fixed positions: the two block
markers, the actionlint note, and the typos note.

None of them can move to a top header without breaking the delimiters the
rollout parses, and an in-repo edit is overwritten on the next rollout, so
five repos carried the same whole-file exclude. That exclude also hid a
genuinely misplaced hand-written comment anywhere else in the file.

Comment lines between `# BEGIN managed by ...` and `# END managed by ...`
are now exempt, and enforcement resumes at the END marker. The markers do
not spend the header budget, and one inside a `run: |` block scalar is
shell rather than a marker.

Four consumer repos can drop the exclude on the next repin. agentic-os
cannot: it authors the generator rather than being stamped by it, so its
own config is hand-written and keeps three below-content notes on the keys
they explain. That remainder is #1181, and the exclude comment now records
that reason rather than the one this fix retired.

Closes #993

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 added 1 commit

2026-08-22 21:38:00 +00:00

Merge branch 'main' into aos/993-code-comments-managed

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

Details

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

Details

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

Details

ci / gate (pull_request) Successful in 1m2s

Details

4e3b283436

coilyco-ops commented

2026-08-22 21:38:56 +00:00

Author

Member

One fail-open to close before this lands. The headline behaviour is right.

I probed scan_yaml on your branch directly rather than reading it. Your two central claims hold:

properly closed region, stray comment after END   -> 1 violation
no markers at all (control)                       -> 1 violation

Enforcement genuinely resumes at the END marker, which is the part the whole-file exclude gave up. That is the win in this change and it works.

An unmatched BEGIN exempts the rest of the file

unmatched BEGIN, stray comment after              -> 0 violations

Fixture:

key: value
# BEGIN managed by thing
  a: 1
# a hand-written stray comment
  b: 2

managed latches True at the BEGIN and nothing ever clears it, so every comment to EOF is exempt. Any line matching ^\s*#\s*BEGIN managed by does it, hand-written or not, and it is a plausible thing for someone to type in a config they are annotating.

This is the phase-3 shape (#1177) arriving inside a phase-1 change: the check reports success while having stopped checking. A whole-file exclude at least announces itself in pyproject.toml. This does not.

Suggested: after the loop, if managed is still True, emit a violation naming the unterminated region. The generator always writes both markers, so an unmatched BEGIN is either a hand-written line or a corrupted managed block, and both deserve to fail loud. That also removes any incentive to use the marker as an exclude.

Worth a fourth test beside the three you have.

Merge order, since you have branches from different bases

This one is cut from 9c140e3f and does not contain #1179, so git diff against current main makes it look like it reverts the check_code_comments conversion to the shared walker. It does not. Against its own base the branch touches only the marker logic and never should_skip or the skip set, so the three-way merge takes #1179's conversion and your addition cleanly. I checked before raising it, and it is fine.

Flagging it anyway because #1183 is now open against the same file's neighbourhood, and the next branch cut from a stale base may not be this lucky.

The rest

Scoping four repos rather than five, and filing #1181 for agentic-os's own hand-written config instead of claiming the exclude drops everywhere, is the honest version of that claim. The run: | block-scalar case being covered is the one I would have asked about.

**One fail-open to close before this lands. The headline behaviour is right.** I probed `scan_yaml` on your branch directly rather than reading it. Your two central claims hold: ``` properly closed region, stray comment after END -> 1 violation no markers at all (control) -> 1 violation ``` Enforcement genuinely resumes at the END marker, which is the part the whole-file exclude gave up. That is the win in this change and it works. ## An unmatched BEGIN exempts the rest of the file ``` unmatched BEGIN, stray comment after -> 0 violations ``` Fixture: ```yaml key: value # BEGIN managed by thing a: 1 # a hand-written stray comment b: 2 ``` `managed` latches True at the BEGIN and nothing ever clears it, so every comment to EOF is exempt. Any line matching `^\s*#\s*BEGIN managed by ` does it, hand-written or not, and it is a plausible thing for someone to type in a config they are annotating. This is the phase-3 shape (#1177) arriving inside a phase-1 change: the check reports success while having stopped checking. A whole-file exclude at least announces itself in `pyproject.toml`. This does not. **Suggested:** after the loop, if `managed` is still True, emit a violation naming the unterminated region. The generator always writes both markers, so an unmatched BEGIN is either a hand-written line or a corrupted managed block, and both deserve to fail loud. That also removes any incentive to use the marker as an exclude. Worth a fourth test beside the three you have. ## Merge order, since you have branches from different bases This one is cut from `9c140e3f` and does not contain #1179, so `git diff` against current `main` makes it look like it reverts the `check_code_comments` conversion to the shared walker. It does not. Against its own base the branch touches only the marker logic and never `should_skip` or the skip set, so the three-way merge takes #1179's conversion and your addition cleanly. I checked before raising it, and it is fine. Flagging it anyway because #1183 is now open against the same file's neighbourhood, and the next branch cut from a stale base may not be this lucky. ## The rest Scoping four repos rather than five, and filing #1181 for agentic-os's own hand-written config instead of claiming the exclude drops everywhere, is the honest version of that claim. The `run: |` block-scalar case being covered is the one I would have asked about.

coilyco-ops merged commit 2382e11261 into main

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

coilyco-ops deleted branch aos/993-code-comments-managed

2026-08-22 21:39:47 +00:00

coilyco-ops referenced this pull request from a commit

2026-08-22 21:39:48 +00:00

fix(code-comments): exempt generated managed-marker regions in YAML (#993) (#1182)

coilyco-ops referenced this pull request

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

An unterminated # BEGIN managed by region silently exempts the rest of a YAML file from code-comments #1185

coilyco-ops referenced this pull request

2026-08-22 21:41:35 +00:00