Harden the close protocol: idempotent close, Resolution.missing_parts, mechanical merge-subsumes-review gate #261

New issue

Closed

opened 2026-08-04 14:42:31 +00:00 by jared · 1 comment

jared commented

2026-08-04 14:42:31 +00:00

Owner

Context

Follow-up to #257 (close protocol), from the 2026-08-04 review. Spec: #250. Three findings in the shipped close CLI / tracker lib (plugins/os-backlog/bin/os-backlog, lib/backlog/resolution.rb, lib/backlog/issues.rb):

Idempotent close. os-backlog close comments then closes with no existing-comment check; a retry after partial failure double-comments. The lifecycle design requires every multi-step tracker mutation to be comment-first and idempotent on re-run.
Resolution.missing_parts is dead API. Nothing calls it — either wire it into close-time validation or delete it (repo precedent 8938d15 treats dead branches as defects).
Mechanical merge-subsumes-review gate. Currently prose-only in hooks/session_start.py:38-46; the design says merge-subsumes-review is recognized mechanically by a PR/branch link. Move the gate into the close CLI: a close on a ticket that was NOT ready-for-agent requires a PR/branch link in the resolution evidence — link present, close proceeds; link absent, the CLI refuses the close, applies the review label, and prints why.

Not ready-for-agent yet: label to be applied after this ticket passes the template validator once #253 lands.

## Context Follow-up to #257 (close protocol), from the 2026-08-04 review. Spec: #250. Three findings in the shipped close CLI / tracker lib (`plugins/os-backlog/bin/os-backlog`, `lib/backlog/resolution.rb`, `lib/backlog/issues.rb`): 1. **Idempotent close.** `os-backlog close` comments then closes with no existing-comment check; a retry after partial failure double-comments. The lifecycle design requires every multi-step tracker mutation to be comment-first and idempotent on re-run. 2. **`Resolution.missing_parts` is dead API.** Nothing calls it — either wire it into close-time validation or delete it (repo precedent 8938d15 treats dead branches as defects). 3. **Mechanical merge-subsumes-review gate.** Currently prose-only in `hooks/session_start.py:38-46`; the design says merge-subsumes-review is recognized mechanically by a PR/branch link. Move the gate into the close CLI: a close on a ticket that was NOT `ready-for-agent` requires a PR/branch link in the resolution evidence — link present, close proceeds; link absent, the CLI refuses the close, applies the `review` label, and prints why. Not `ready-for-agent` yet: label to be applied after this ticket passes the template validator once #253 lands.

jared referenced this issue

2026-08-07 12:45:09 +00:00

Route test-file lint offenses to the test-writer, not the lint-fixer #308

jared referenced this issue

2026-08-07 12:45:10 +00:00

shard-plan misses code files for tests that exercise a CLI as a subprocess #309

jared commented

2026-08-07 12:54:23 +00:00

Author

Owner

Resolution

Done: Hardened the close protocol per all three findings: close is now idempotent on retry (checks the last '## Resolution' comment via Resolution.last_section/missing_parts, skips re-posting when valid, appends a corrected comment when malformed); Resolution.missing_parts is wired into close-time validation instead of being dead API; the merge-subsumes-review gate is mechanical in the close CLI (non-ready-for-agent close without a PR/branch link in --evidence is refused, review label applied, reason printed).

Evidence: Merged to main in merge commit 3a2e4b0 (branch: sdlc/261, commit 5b603cb). Suite green post-merge: 142 runs, 324 assertions, 0 failures; rubocop 0 offenses. Reviewer round 2 verdict: APPROVE. New black-box CLI contract tests in plugins/os-backlog/tests/close_protocol_test.rb.

Follow-ups: #308 (P1): route test-file lint offenses to the test-writer, not the lint-fixer. #309 (P2): shard-plan misses code files for tests exercising a CLI as a subprocess.

## Resolution **Done:** Hardened the close protocol per all three findings: close is now idempotent on retry (checks the last '## Resolution' comment via Resolution.last_section/missing_parts, skips re-posting when valid, appends a corrected comment when malformed); Resolution.missing_parts is wired into close-time validation instead of being dead API; the merge-subsumes-review gate is mechanical in the close CLI (non-ready-for-agent close without a PR/branch link in --evidence is refused, review label applied, reason printed). **Evidence:** Merged to main in merge commit 3a2e4b0 (branch: sdlc/261, commit 5b603cb). Suite green post-merge: 142 runs, 324 assertions, 0 failures; rubocop 0 offenses. Reviewer round 2 verdict: APPROVE. New black-box CLI contract tests in plugins/os-backlog/tests/close_protocol_test.rb. **Follow-ups:** #308 (P1): route test-file lint offenses to the test-writer, not the lint-fixer. #309 (P2): shard-plan misses code files for tests exercising a CLI as a subprocess.

jared closed this issue

2026-08-07 12:54:23 +00:00

jared referenced this issue

2026-08-07 12:55:20 +00:00

Late-gate routing rule: test_path offenses dispatch to test-lint-fixer #313

jared referenced this issue

2026-08-07 13:00:02 +00:00

Early gate placement: where does test-lint run relative to red-assert? #311

jared referenced this issue

2026-08-07 13:08:44 +00:00

test-lint-fixer agent charter: constraints, assignment shape, and relationship to lint-fixer #312

jared referenced this issue