Skip to content

feat(kanban): review_coverage records carry the reviewed PR head (head_sha) (t_7fee0f83) - #1129

Merged
ang-fleet-lander[bot] merged 2 commits into
mainfrom
feat/review-coverage-head-sha-t7fee0f83
Sep 26, 2026
Merged

ang-fleet-lander[bot] merged 2 commits into
mainfrom
feat/review-coverage-head-sha-t7fee0f83

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

feat(kanban): review_coverage records carry the reviewed PR head (head_sha)

Card-sourced land requests (hermes-home scripts/land_request.py) take the
card's LATEST review_coverage record as the review of record and refuse
with NO_REVIEW_OF_RECORD unless it is an APPROVE carrying head_sha. No
kanban path wrote either (t_7fee0f83, Phase 2 step 4 blocker).

  • kanban_review_schema: head_sha is a required coverage field
    (HEAD_SHA_PATTERN 7-40 hex); prompt/tool text picks it up from
    coverage_fields_text.
  • request_changes gate: head_sha must be 7-40 hex or 'n/a: '
    (card with no PR; same applicability rule as lenses).
  • approve path: complete_task on a run claimed from review with
    metadata.head_sha writes review_coverage: {"verdict":"approve", "head_sha":...} on the reviewer's run, in the completion txn. Malformed
    head_sha raises before any mutation. Implementer runs never write one
    (metadata.head_sha cannot forge an approval). Approval without head_sha
    is unchanged (land request then fails closed at NO_REVIEW_OF_RECORD).
  • sdlc-review SKILL.md example + approve instruction.

Verified: test_kanban_review_head_sha.py 14 cases, 10 RED on fork/main,
all GREEN; with coverage_gate + sdlc skill parity 89 passed. Mutant
(approve write + gate check disabled): 9 failed. test_kanban_review_surfaces
has 5 failures identical on unmodified fork/main (pre-existing, not this diff).


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@Kyzcreig Kyzcreig closed this Sep 25, 2026
@Kyzcreig Kyzcreig reopened this Sep 25, 2026
@ang-fleet-lander

Copy link
Copy Markdown

🤖 merged-by: apollo · lane: review · gate: BYPASS: FleetReview paused since 2026-09-22 (Ace); CI green; Apollo reviewed the diff · why: t_7fee0f83 review r1 APPROVE (Apollo): review_coverage carries head_sha; reviewer approve writes the APPROVE record land_request.py reads · red-ci allowed [check suite 98027770519 (github-actions) startup_failure with 0 jobs - re-run the WHOLE workflow]: the only red is check suite 98027770519: a GitHub 'BuildFailed' startup_failure with path=BuildFailed and 0 jobs (not retryable, not tied to any workflow file); the latest ci.yaml run on this head (36198114914) is success and 41 check-runs are green

@ang-fleet-lander
ang-fleet-lander Bot added this pull request to the merge queue Sep 25, 2026
@ang-prism

ang-prism Bot commented Sep 26, 2026

Copy link
Copy Markdown

FleetReview

FleetReview's daily member-call budget is spent (600/600 for 2026-09-26 UTC); review skipped.


FleetReview · reviewKind: skipped-budget

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Sep 26, 2026
@Kyzcreig
Kyzcreig force-pushed the feat/review-coverage-head-sha-t7fee0f83 branch from 193087a to 3ef373e Compare September 26, 2026 05:13
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Rebased onto main at 5b8e56a (t_7fee0f83). The one conflict was in complete_task, against #1094 (open-PR completion routes to review).

How I resolved it: kept both blocks. head_sha validation runs first, so a malformed head_sha still raises before any mutation. Open-PR routing is skipped when a claimed reviewer run approves with head_sha.

Why: a claimed reviewer run has status running, not review, so #1094's candidate.status != 'review' exemption did not cover it. Every approval with head_sha names a PR that is OPEN by definition. Without the skip, the approval was re-routed to review and the APPROVE record that land_request.py needs was never written. I confirmed this against the real path in a scratch board: events ended review_requested, completion_routed_to_review, status review. #1094's stated intent was already "a reviewer/human approval is left alone". Implementer completions are unchanged: they never set approve_head_sha.

New test: test_reviewer_approval_of_open_pr_is_not_rerouted_to_review (oracle stubbed OPEN). It fails on the naive resolution and passes with the skip. Local run of test_kanban_review_head_sha + test_kanban_open_pr_route + test_kanban_review_coverage_gate: 93 passed.

@Kyzcreig
Kyzcreig force-pushed the feat/review-coverage-head-sha-t7fee0f83 branch 2 times, most recently from 14f8c9e to b189a7b Compare September 26, 2026 06:08
@blacksmith-sh

blacksmith-sh Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Found 1 test failure on Blacksmith runners:

Failure

Test View Logs
TestPinTransition/test_cache_busting_signature_reflects_pin_peer_name View Logs

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@ang-fleet-lander

Copy link
Copy Markdown

🤖 merged-by: daedalus · lane: kanban · gate: ADVISORY (FleetReview not green for b189a7b): fleetreview-advisory-20260926-off.md · why: t_7fee0f83 head_sha review records; Apollo r1 APPROVE (card comment); rebased onto main, #1094 conflict resolved + regression test, #1171 fixture fixed · red-ci allowed [All required checks pass,Python tests / Tests complete,Python tests / Run tests slice 14/16]: only red = slice 14 tests/honcho_plugin/test_pin_peer_name.py::test_cache_busting_signature_reflects_pin_peer_name (assert True != True: two honcho.json writes read back the same value, mtime-memo coarse-tick class like #1222); PR touches no honcho/gateway files; passes locally; 3754 other tests in the slice green

…d_sha)

Card-sourced land requests (hermes-home scripts/land_request.py) take the
card's LATEST review_coverage record as the review of record and refuse
with NO_REVIEW_OF_RECORD unless it is an APPROVE carrying head_sha. No
kanban path wrote either (t_7fee0f83, Phase 2 step 4 blocker).

- kanban_review_schema: head_sha is a required coverage field
  (HEAD_SHA_PATTERN 7-40 hex); prompt/tool text picks it up from
  coverage_fields_text.
- request_changes gate: head_sha must be 7-40 hex or 'n/a: <reason>'
  (card with no PR; same applicability rule as lenses).
- approve path: complete_task on a run claimed from review with
  metadata.head_sha writes `review_coverage: {"verdict":"approve",
  "head_sha":...}` on the reviewer's run, in the completion txn. Malformed
  head_sha raises before any mutation. Implementer runs never write one
  (metadata.head_sha cannot forge an approval). Approval without head_sha
  is unchanged (land request then fails closed at NO_REVIEW_OF_RECORD).
- sdlc-review SKILL.md example + approve instruction.

Verified: test_kanban_review_head_sha.py 14 cases, 10 RED on fork/main,
all GREEN; with coverage_gate + sdlc skill parity 89 passed. Mutant
(approve write + gate check disabled): 9 failed. test_kanban_review_surfaces
has 5 failures identical on unmodified fork/main (pre-existing, not this diff).
#1171 (merged after this PR opened) builds its own review_coverage
JSON without head_sha, which this PR makes required. Sweep of every
test building coverage (git grep batch_id tests/): only this file
lacked it; test_sdlc_review_skill checks a key subset and passes.

Verified: test_kanban_human_review_sendback.py + test_sdlc_review_skill.py
17 passed (CI slice 1 had 2 failures with the head_sha refusal).
@ang-fleet-interactive
ang-fleet-interactive Bot force-pushed the feat/review-coverage-head-sha-t7fee0f83 branch from b189a7b to 4989394 Compare September 26, 2026 14:53
@ang-fleet-lander

Copy link
Copy Markdown

🤖 merged-by: apollo · lane: review · gate: BYPASS: FleetReview paused since 2026-09-22 (Ace) · why: t_7fee0f83 r1 APPROVE stands; rebased head 4989394 green after re-run of honcho load-flake (test_pin_peer_name, untouched by PR, green on main)

@ang-fleet-lander
ang-fleet-lander Bot added this pull request to the merge queue Sep 26, 2026
Merged via the queue into main with commit 6c7d44d Sep 26, 2026
56 checks passed
@ang-fleet-lander
ang-fleet-lander Bot deleted the feat/review-coverage-head-sha-t7fee0f83 branch September 26, 2026 15:15
Kyzcreig pushed a commit that referenced this pull request Sep 26, 2026
…e2ff3)

#1129 made head_sha a required review_coverage field; the parked-review
send-back fixtures predated it, so 6 tests failed in the merge group.
Fixture now carries a 40-hex head_sha; new case asserts a parked send-back
without head_sha is refused and the review claim rolled back.

Verified (test-gate, venv py3.11): sendback + human_review_sendback +
review_head_sha 38 passed; coverage_gate + home_session 156 passed.
@ang-prism

ang-prism Bot commented Sep 27, 2026

Copy link
Copy Markdown

FleetReview

Review: post-merge · head 6c7d44d1d684 · duration 10m 14s
Profile: light (merit: default light: lines 202<800, files 7<1000000, hunks 11<1000000, no hot path) · policy: below-size-and-path-gates
Roster: B-state → gpt-6-sol (openai), C-assert-xhigh → claude-code-opus-5-5 (anthropic), G → grok-4.6 (xai), L6 → gpt-6-sol (openai)

Post-merge review (fleetreview:post-merge override): this reviewed the merge commit against its first parent — the bytes that already shipped. It is not a pre-merge gate pass.

profile: light (rule: default light: lines 202<800, files 7<1000000, hunks 11<1000000, no hot path) · round 0 · members: B-state, L6, C-assert-xhigh, G · families: anthropic,openai,xai

Confidence: 3/5

Findings

  • P1 hermes_cli/kanban_db.py:9406 — Premature completion · agreed: B-state,L6,C-assert-xhigh (openai, anthropic)
  • P1 hermes_cli/kanban_db.py:9550 — APPROVE review-of-record is a plain comment that any worker can forge · agreed: C-assert-xhigh (anthropic)

FleetReview provenance · models: B=gpt-6-sol, C=claude-code-opus-5-5, D=grok-4.6 · cost: $1.69 · duration: 10m 09s · rounds: 1 · files examined: 7

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