Skip to content

fix(approve): sign off docs that link to each other in one call - #18

Merged
pasrom merged 1 commit into
mainfrom
fix/approve-linked-docs
Oct 13, 2026
Merged

pasrom merged 1 commit into
mainfrom
fix/approve-linked-docs

Conversation

@pasrom

@pasrom pasrom commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Problem

km approve gated each doc right after editing it. With the gate's bergab check on, a served doc may not link to a doc still in review. Two docs that link to each other could therefore never be signed off: each one was refused because the other was still in review, also when both were named in the same call. The only way out was to set status and the approval fields by hand, which the skill tells users not to do.

Change

  • All docs of one call are written first and gated afterwards. A doc that fails is restored byte for byte, and the remaining docs are gated again, since one of them may link to the restored doc. This repeats until no doc fails.
  • Writing first widens the window in which docs are accepted but not yet gated. Every doc that is written and not confirmed is therefore restored when the run stops early, through an exception or an interrupt.
  • A doc named twice, under any spelling of its path, is handled once (accepted once, or refused once).
  • prepare returns a Candidate (NamedTuple) instead of a positional tuple.
  • SKILL.md, "Approval": docs that link to each other are signed off in one call.

Tests

New cases in tests/gate_smoke.sh, each checked against a mutant:

Case Fails when
alone, a doc linking to a review doc is refused (pins the old behaviour for a single doc)
docs linking to each other are signed off together the old per-doc gate is restored
a doc that fails takes back the one linking to it the re-gate loop runs only once (if instead of while)
a doc named twice is handled once the dedup continue is removed
an interrupted approve leaves every doc unchanged the restore on abort is removed

All smoke tests pass: gate 85, team 96, index 30, doctor 29, encoding 4.

approve gated each doc right after editing it. With the bergab check on,
a doc linking to another doc still in review fails, so two docs that link
to each other could never be signed off: each was refused while the other
was still in review, also when both were named in the same call.

Now all docs of one call are written first and then gated. A doc that
fails is restored, and the remaining ones are gated again, since one of
them may link to the restored doc. Writing first widens the window in
which docs are accepted but not yet gated, so every doc that is written
and not confirmed is restored when the run stops early (an exception or
an interrupt). A doc named twice, under any spelling of its path, is
handled once.
@pasrom

pasrom commented Oct 13, 2026

Copy link
Copy Markdown
Owner Author

Code review (claude-opus-5-5, effort high) and how it was handled

The review ran on the first version of the commit. Eight findings, unverified by the reviewer:

# Finding Handled
1 All docs are written to accepted before any gate runs; an exception or Ctrl-C later leaves them accepted and ungated. Fixed. Every doc written and not confirmed is restored in a finally. Test: an interrupted approve leaves every doc unchanged.
2 If restoring one failed doc raises, the other failed and pending docs stay as written. Fixed by the same finally: a doc leaves the pending set only after its restore succeeded; a restore that fails again is reported on stderr.
3 The test "a doc that fails takes back the one linking to it" passes on the old code. Kept as is. It is a guard for the new re-gate loop: with the loop running only once (if instead of while) it fails, checked. The reviewer's claim that a single gate pass would also pass it does not hold.
4 "A doc named twice is signed off once" has no test. Fixed. Test added, fails without the dedup.
5 Dedup happens only on success, so a refused doc named twice is reported twice. Fixed. Dedup runs on the resolved path before any check.
6 One subprocess per pending doc per round. Not changed. Calls are small; per-doc gating keeps the refusal message per doc.
7 Writing first widens the window in which other processes see ungated accepted docs. Accepted as inherent to gating linked docs together; #1 keeps it from leaving anything behind.
8 prepare returns a positional tuple. Fixed. Candidate NamedTuple.

@pasrom
pasrom merged commit b5a3cb4 into main Oct 13, 2026
4 checks passed
@pasrom
pasrom deleted the fix/approve-linked-docs branch October 13, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant