Skip to content

ci(town-crier): auto-resolve bus thread on 2-approval consensus#11

Merged
jasperboerhof merged 1 commit into
mainfrom
ci/crier-2approval-consensus
Jun 24, 2026
Merged

ci(town-crier): auto-resolve bus thread on 2-approval consensus#11
jasperboerhof merged 1 commit into
mainfrom
ci/crier-2approval-consensus

Conversation

@jasperboerhof

Copy link
Copy Markdown
Contributor

Adds the 2-approval consensus auto-resolve job to this repo's town-crier producer workflow, matching the version merged to kendo (development, 2026-06-23).

What it does: on an approved review of an Agent Review Requested PR, it tallies distinct current-head approvals; once ≥2 agree with no outstanding CHANGES_REQUESTED it POSTs /resolve to the bus to de-announce the thread (further review turns are churn once two independent approvals agree). Stale approvals cast on an earlier head are filtered out, so a regression pushed onto a reviewed head can't satisfy consensus.

No new secrets — reuses the existing TOWN_CRIER_URL variable + TOWN_CRIER_TOKEN secret; the consensus job only adds in-workflow pull-requests: read.

The announce-pr.yml is byte-identical to kendo's deployed version (sha256 eb9ccfe3…).

🤖 Generated with Claude Code

@jasperboerhof jasperboerhof added the Agent Review Requested Requesting review of specialized AI review agents. label Jun 24, 2026
@jasperboerhof

Copy link
Copy Markdown
Contributor Author

Town Crier Review · 8/10 · PASS · 🔎 Independent — 🟡 1

kendo-error-tracker #11 · AC anchor: none (PR description / config-only) · head 6e3b1a6bc9 · via the town-crier bus (request #156)

Tip

Config-only PR adding a consensus job to the town-crier producer workflow that auto-resolves a PR's bus thread once 2 distinct current-head approvals land with no outstanding change request; I ran the jq/bash tally DEEP against runtime fixtures (independent-approval dedup, DISMISSED, stale-CR, empty/gh-failure). Counting is sound and every failure path fails closed to no-resolve; the only blemish is that the changes side isn't head-filtered like the approved side, so a stale change-request blocks consensus longer than the PR's own "current-head" framing implies — but it errs toward leaving the thread open, so it's a MINOR churn/consistency note, not a correctness break.

1 finding(s) posted inline:

  • 🟡 MINOR · .github/workflows/announce-pr.yml:138 — changes-requested tally isn't head-filtered like approvals — a stale CR blocks consensus indefinitely

Comment thread .github/workflows/announce-pr.yml

@Goosterhof Goosterhof left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Approve-worthy

0 blockers · 0 concerns · 0 nits · 1 praise · 0 inline
Round 1 — first war-room review. Jasper reviewed independently before me (PASS 8/10, 1 MINOR).
CI green (check (8.4)/check (8.5)/announce pass; the skipping rows are conditional event/path skips, not failures).
Ally engagement: 1 reply in-thread to @jasperboerhof.

Config-only PR adding a consensus job to this repo's town-crier producer workflow: on an approved review of a labelled PR it tallies distinct current-head approvals and POSTs /resolve once ≥2 agree with no outstanding change request. I verified the consensus job is byte-identical to kendo's deployed development version (the PR body's parity claim holds), so this is a faithful fleet-mirror of already-reviewed work, not new logic. The tally is sound on every path I walked: group_by(.user.login) | map(max_by(.submitted_at)) correctly takes each reviewer's latest opinion, head-gates approvals via commit_id == $head, lets DISMISSED override a stale opinion, and fails closed to no-resolve on an unreadable review list.

Praise: the head-gated approval count (.commit_id == $head) is the load-bearing correctness decision — it makes consensus robust regardless of the repo's dismiss_stale_reviews setting, so a regression pushed onto a reviewed head can't inherit a prior approval. That's the non-obvious part and it's done right.

Cross-file findings

I confirm Jasper's MINOR (inline at .github/workflows/announce-pr.yml:138): the changes count selects .state == "CHANGES_REQUESTED" with no commit_id == $head filter, asymmetric to the head-gated approved count, so a stale CR blocks consensus indefinitely. It errs safe (thread stays open, never wrongly resolved) — MINOR, not blocking. Extended in-thread with a second liveness note: a later dismissal of that stale CR doesn't re-fire the consensus job (the trigger is review.state == 'approved' only), so the thread waits for the next approval/synchronize to self-heal. Same safe direction; documenting the freshness asymmetry, as Jasper proposes, covers both.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

@jasperboerhof jasperboerhof merged commit ae5e671 into main Jun 24, 2026
14 checks passed
@jasperboerhof jasperboerhof deleted the ci/crier-2approval-consensus branch June 24, 2026 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent Review Requested Requesting review of specialized AI review agents.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants