Skip to content

Confirm pauses on GitHub and alert people when they are not - #218

Merged
tzhouam merged 1 commit into
mainfrom
kb/confirm-pauses
Sep 28, 2026
Merged

tzhouam merged 1 commit into
mainfrom
kb/confirm-pauses

Conversation

@tzhouam

@tzhouam tzhouam commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Knowledge-service redesign follow-up, from the design's procedure P: a pause counts only once it is observed on GitHub, and a pause not confirmed within 5 minutes goes to people. Before this, an ack alone marked a PR paused.

  • Confirmation:
    • An acked pause is confirmed only when GitHub shows the PR as a draft (a draft can't be in, or re-enter, the merge queue); otherwise it is re-paused.
    • A confirmed PR later marked ready again counts as unconfirmed, with a fresh alert clock.
    • A completed re-pause clears its pending item, so the next undo is re-paused at once.
  • Escalation: a pause neither acked nor confirmed 5 minutes after the request goes to people once, with the manual commands. This runs even when GitHub can't be read; such failures skip only that change set.
  • Pause tracking runs before the head checks, so a paused PR whose head moves stays watched.
  • kb status reports pause_unconfirmed per repository (a breaker or rollback isn't done until it reaches 0), and kb resume clears the bookkeeping, including never-acked requests.

Commit hook approved; full suite and SPEC freshness pass locally (rebased on #217).

🤖 Generated with Claude Code

The design's procedure P counts a pause only once it is observed: the
service must see the PR out of the queue and a draft. A pause not confirmed
within 5 minutes goes to people with the commands to do it by hand. Until
now an ack alone marked a PR paused, and a pause that never ran went
unnoticed.

- pause_open_prs records each pause request's time and reason.
- A "paused" change set (acked) is confirmed only when GitHub shows the PR as
  a draft (a draft cannot be in, or re-enter, the merge queue). Otherwise the
  pause is issued again, and a confirmed PR observed un-drafted later counts
  as unconfirmed again (with a fresh alert clock). Pause tracking runs before
  head checks, so a paused PR whose head moves stays watched. A completed
  re-pause (its ack, or a confirmation) clears the pending item, so a later
  undo is re-paused at once instead of waiting out the old item.
- A request neither acked nor confirmed 5 minutes later goes to people once,
  with "remove it from the merge queue" and `gh pr ready <n> --undo`. The
  escalation does not depend on GitHub answering: if reading a PR fails,
  only that change set is skipped, and an overdue pause still goes to people.
- kb status reports pause_unconfirmed per repository: a breaker or rollback is
  not complete while it is non-zero. kb resume clears the bookkeeping, also
  for requests that were never acked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-28T16:36:43.285832Z 7721ef5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7721ef538e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +548 to +550
requested = float(detail.get("pause_requested_at") or 0)
if not requested or detail.get("pause_alerted") or detail.get("pause_confirmed") \
or rt.clock() - requested < PAUSE_CONFIRM:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Seed an alert deadline for existing paused rows

When this version starts with a change set already in paused status from the previous release, its detail has no pause_requested_at. If GitHub does not show the PR as a draft, advance() reissues the pause, but neither that retry nor this helper initializes the timestamp; this early return therefore repeats forever. Such a PR is reported by pause_unconfirmed() but never escalated to people after five minutes, defeating the new safety behavior during an upgrade. Initialize tracking when an untracked paused row is encountered, or derive a deadline from the retry/row timestamp.

Useful? React with 👍 / 👎.

@tzhouam
tzhouam merged commit bf73f93 into main Sep 28, 2026
5 checks passed
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