Skip to content

ci(town-crier): re-announce on push (synchronize + head_oid)#10

Merged
jasperboerhof merged 1 commit into
mainfrom
ci/town-crier-reannounce-on-push
Jun 22, 2026
Merged

ci(town-crier): re-announce on push (synchronize + head_oid)#10
jasperboerhof merged 1 commit into
mainfrom
ci/town-crier-reannounce-on-push

Conversation

@jasperboerhof

Copy link
Copy Markdown
Contributor

Fire the town-crier announce on synchronize (each new commit on a labelled PR) and pass the head SHA so the bus can re-open the review thread when the head changes. Producer half of re-review-on-push, mirroring kendo and town-crier#20's template change. The resolve job is unchanged.

Inert until town-crier#20 is merged + deployed (the bus must understand head_oid first); a re-announce with the same head is a harmless no-op.

🤖 Generated with Claude Code

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

Copy link
Copy Markdown
Contributor Author

Town Crier Review · 9/10 · PASS · 🔎 Independent

kendo-error-tracker #10 · AC anchor: PR description (no kendo board; config task) · head 7ae017028c · via the town-crier bus (request #125)

Tip

Config-only PR adding the synchronize trigger + threading head_oid into the town-crier announce payload so a push on a labelled PR re-announces for a fresh review round; I verified the event matrix (labeled/synchronize-labelled/synchronize-unlabelled/unrelated-label) and the new wire field against the bus consumer, both of which are correct and backward-compatible. The synchronize branch correctly uses contains(labels.*.name, ...) rather than the synchronize-null github.event.label.name, the head_oid field is fully consumed by the bus (server.ts schema + db.ts same-head-no-op/changed-head-reopen/in-review-defer logic), and CI is green.

No findings — clean against the review checklist.

@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 · 0 praise · 0 inline

Confirm-the-pattern (light sweep). This PR's announce-pr.yml change is the byte-identical diff to the canonical script-development/town-crier#22, which I reviewed in full — verified here by diffing the two. The canonical verdict carries over: correct producer-side change (adds synchronize re-announce gated behind the Agent Review Requested label via contains(labels), threads head_oid into the announce payload; provisioning-fail-loud and transient-fail-non-blocking preserved). CI green.

Same caveat as #22: the "a re-announce with the same head is a no-op on the bus" comment is the right design but not true yet — the bus is head-blind (keys on pr_url), so the no-op waits on the consumer-side head_oid dedup (the deferred town-crier-relay-debounce-covered-heads). Non-blocking — head_oid is correctly sent here.

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

@jasperboerhof jasperboerhof merged commit 401b913 into main Jun 22, 2026
4 checks passed
@jasperboerhof jasperboerhof deleted the ci/town-crier-reannounce-on-push branch June 22, 2026 18:29
@jasperboerhof

Copy link
Copy Markdown
Contributor Author

Town Crier Review · 8/10 · PASS · 🤝 Confirm — 🟡 1

kendo-error-tracker #10 · AC anchor: PR description (task, no board) · head 7ae017028c · via the town-crier bus (request #125)

Tip

Config-only PR adding a synchronize re-announce trigger and threading head_oid into the town-crier announce payload. We corroborate the prior reviews: the event matrix (labeled / synchronize-labelled / synchronize-unlabelled / unrelated-label) and the new wire field both check out at head — the synchronize branch correctly gates on contains(labels.*.name, ...) rather than the synchronize-null github.event.label.name, the field is additive and backward-compatible against the bus consumer, and the fail-loud/fail-open policy is untouched; CI green. One MINOR doc-drift: the top-of-file header still reads "tell the crier once," which means the carried-over "byte-identical to canonical #​22" verdict isn't strictly precise (the canonical rewrote that header). The "same-head no-op" caveat the prior reviews call aspirational is actually already implemented in the bus consumer.

1 finding(s) — detailed below (not anchorable to diff lines).

Bus thread · 2 prior review(s):

  • jasper (independent): Independent first look (empty thread). PASS 9/10 — no findings. Config-only PR adding the synchronize trigger + thread
  • general (confirm): Confirm-the-pattern: the byte-identical diff to canonical town-crier#​22; CI green; head_oid no-op caveat noted.

Findings outside the diff (not inline-postable)

  • 🟡 MINOR — Header comment still says "tell the crier once" but this diff makes announce re-fire on every push (.github/workflows/announce-pr.yml:8)
    • Fix: Update the line-8 bullet to match canonical: note that announce now also re-announces on every new commit (synchronize), with head_oid as the change detector.

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