fix(pre-commit): render one config, so created is followed by unchanged (#985) #1191

Merged

coilyco-ops merged 1 commit from aos/985-created-then-unchanged into main

2026-08-22 22:50:20 +00:00

coilyco-ops commented

2026-08-22 22:43:20 +00:00

Member

Closes #985.

The create path wrote repos:\n<block> and the refresh path rebuilt it as before.rstrip() + "\n\n" + block. One blank line apart, so a config that had just been created always reported updated on its next run and only settled on the third.

Confirmed before changing anything:

tpl : 'repos:\n  # BEGIN managed by ...'
new : 'repos:\n\n  # BEGIN managed by ...'
identical: False

One renderer

Both paths now call render_config. The blank line is not cosmetic everywhere - it divides a repo's own hooks from the managed block - so the rule is stated once: separate when there is something to separate, and the bare document opener is the only before that has nothing. A fresh config therefore keeps the tighter shape it already had rather than growing a blank line to match the other path.

before: ['created', 'updated', 'unchanged']
after:  ['created', 'unchanged', 'unchanged']

Coverage

test_a_created_config_is_unchanged_on_its_next_refresh - the three-run sequence the issue asked for.
test_a_hand_written_preamble_keeps_its_blank_line - the control, so the fix cannot become "drop the separator everywhere".

One-time normalization

Consumer repos whose config was last written by the old refresh path carry that blank line today, so they will report updated once as it normalizes and stay unchanged after. That is the convergence this fixes, not a new instance of it.

625 tests pass, pre-commit run --all-files passes.

Closes #985. The create path wrote `repos:\n<block>` and the refresh path rebuilt it as `before.rstrip() + "\n\n" + block`. One blank line apart, so a config that had just been created always reported `updated` on its next run and only settled on the third. Confirmed before changing anything: ``` tpl : 'repos:\n # BEGIN managed by ...' new : 'repos:\n\n # BEGIN managed by ...' identical: False ``` ## One renderer Both paths now call `render_config`. The blank line is not cosmetic everywhere - it divides a repo's own hooks from the managed block - so the rule is stated once: separate when there is something to separate, and the bare document opener is the only `before` that has nothing. A fresh config therefore keeps the tighter shape it already had rather than growing a blank line to match the other path. ``` before: ['created', 'updated', 'unchanged'] after: ['created', 'unchanged', 'unchanged'] ``` ## Coverage * `test_a_created_config_is_unchanged_on_its_next_refresh` - the three-run sequence the issue asked for. * `test_a_hand_written_preamble_keeps_its_blank_line` - the control, so the fix cannot become "drop the separator everywhere". ## One-time normalization Consumer repos whose config was last written by the old refresh path carry that blank line today, so they will report `updated` once as it normalizes and stay unchanged after. That is the convergence this fixes, not a new instance of it. 625 tests pass, `pre-commit run --all-files` passes.

coilyco-ops added 1 commit

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

fix(pre-commit): render one config, so created is followed by unchanged (#985 )

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

Details

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

Details

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

Details

ci / gate (pull_request) Successful in 57s

Details

b4e64e3fd9

The create path wrote `repos:\n<block>` and the refresh path rebuilt it as
`before.rstrip() + "\n\n" + block`. One blank line apart, so a config that
had just been created always reported `updated` on its next run and only
settled on the third.

Both paths now call one `render_config`. The blank line separates
hand-written content from the managed block, and there is nothing to
separate when `before` is only the document opener, so a fresh config keeps
the tighter shape it already had rather than growing a blank line to match
the other path.

Consumer repos whose config was last written by the old refresh path carry
that blank line today, so they report `updated` once as it normalizes, then
stay unchanged. That is the convergence this fixes rather than a new
instance of it.

Closes #985

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 commented

2026-08-22 22:47:04 +00:00

Author

Member

Verified by exercising render_config rather than reading it. No blocking findings.

opener only            -> 'repos:\nBLOCK'
opener + own hook      -> 'repos:\n  - repo: local\n\nBLOCK'
preamble               -> '# note\nrepos:\n\nBLOCK'

The separator rule does what the docstring says: tight only when before is the bare opener, blank line whenever there is something to separate. test_a_hand_written_preamble_keeps_its_blank_line as the control is the right pairing, since the tempting wrong fix here is dropping the separator everywhere. pytest tests/test_apply_agentic_os_hooks.py is 20 passed, 1 skipped on your branch.

Calling the create path's tighter shape the canonical one rather than growing it a blank line is the right direction too. It keeps the one-time normalization to consumer repos rather than also rewriting every freshly created config.

One edge, pre-existing and not a regression

EMPTY before  ->  '\nBLOCK'

A config with no repos: key at all renders a leading blank line and still no repos: key, which is not a valid pre-commit config. The old code did the same ('' + '\n\n' + block), so this PR changes nothing about it and I would not hold the PR for it.

Worth a thought only because you have just made this the one renderer, which is the natural place for that to stop being silent. A before that is neither empty-with-opener nor contains repos: is a caller mistake, and this lane's phase 3 is about refusing rather than proceeding quietly. Your call whether that is this PR, a follow-up, or nothing.

#985's own words were "cosmetic and self-correcting", and this is the right size of change for that.

**Verified by exercising `render_config` rather than reading it. No blocking findings.** ``` opener only -> 'repos:\nBLOCK' opener + own hook -> 'repos:\n - repo: local\n\nBLOCK' preamble -> '# note\nrepos:\n\nBLOCK' ``` The separator rule does what the docstring says: tight only when `before` is the bare opener, blank line whenever there is something to separate. `test_a_hand_written_preamble_keeps_its_blank_line` as the control is the right pairing, since the tempting wrong fix here is dropping the separator everywhere. `pytest tests/test_apply_agentic_os_hooks.py` is 20 passed, 1 skipped on your branch. Calling the create path's tighter shape the canonical one rather than growing it a blank line is the right direction too. It keeps the one-time normalization to consumer repos rather than also rewriting every freshly created config. ## One edge, pre-existing and not a regression ``` EMPTY before -> '\nBLOCK' ``` A config with no `repos:` key at all renders a leading blank line and still no `repos:` key, which is not a valid pre-commit config. The old code did the same (`'' + '\n\n' + block`), so this PR changes nothing about it and I would not hold the PR for it. Worth a thought only because you have just made this the **one** renderer, which is the natural place for that to stop being silent. A `before` that is neither empty-with-opener nor contains `repos:` is a caller mistake, and this lane's phase 3 is about refusing rather than proceeding quietly. Your call whether that is this PR, a follow-up, or nothing. #985's own words were "cosmetic and self-correcting", and this is the right size of change for that.

coilyco-ops merged commit 8fd50156b5 into main

2026-08-22 22:50:20 +00:00

coilyco-ops deleted branch aos/985-created-then-unchanged

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

coilyco-ops referenced this pull request from a commit

2026-08-22 22:50:23 +00:00

fix(pre-commit): render one config, so created is followed by unchanged (#985) (#1191)