fix(pr-wait): /ship review pass 1 — every check gates green, --timeout, bot identity, signal disposition

Red Team and specialist findings on #3495, all fixed in the diff:

- green gates on every check on the head, as gstack-ci-gate does, so
  land-and-deploy Step 2 and Step 3 can no longer disagree while a
  non-required check pends; required checks still gate before they appear
- --timeout MINUTES (default 60, 0 = none) ends a watch that met nothing as
  PR_WAIT_BLOCKED naming the still-pending checks; restores Step 3's cap
- a bot owns logins that start with its name (greptile-apps[bot]), not any
  login containing it; a failed gating-bot check is a failed check, never a
  completed review
- ready: green alone when no gating bot is configured (pr_wait_gating_bots
  none no longer polls forever); held while merge state is UNKNOWN
- BEHIND and DIRTY announced once per head, with the short head in the line
- the bin installs no signal handlers: the synchronous loop never ran them
  and they made the process ignore SIGTERM/SIGINT; default disposition kills
  it at once and the next watcher takes over the dead pid's lock
- a gh spawn timeout (ETIMEDOUT) is retried like any transient failure
- a local fault (unwritable state root) is PR_WAIT_REFUSED exit 2, so exit 1
  always carries a PR_WAIT_BLOCKED line
- control characters in GitHub text never start a second event line

Tests: 21 new cases including a real SIGTERM against the bin, gstack-config
set-time validation and default parity, and the 14 state-matrix cells the
state-machines specialist listed (gstack-state-matrix check: ok, 33/33).

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-26 22:07:42 +00:00
1 parent 7da2d98d9d
commit 13c5784767
8 files changed
+396 -105

No files matched your search

+3 -3
View File
@@ -3,15 +3,15 @@
## [Unreleased]
### Added
- **`bin/gstack-pr-wait`, `lib/pr-wait.ts`, `lib/pr-wait-github.ts`**: an event-driven pull-request watcher (design ported from tstack's pr-wait, MIT, Theo Browne; NOTICE.md, `licenses/MIT-tstack.txt`). `gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS]` prints one stdout line per real event, prefixed `#<pr>`, and progress on stderr: a required check failed (name and link), all checks finished on the head, a comment, review, thread or reply from someone other than the PR author or the gh user, a review bot finished on the head or a gating bot's summary changed, the base branch moved and its new commits touch this PR's files, merge state BEHIND or DIRTY, merged or closed. Required checks are read from the base branch's rulesets and classic branch protection, so a required check that has not started is pending rather than green by omission; two identical polls confirm the check set before `green`. Exit 0 when the condition is met or the PR merged, 1 when it needs action (`PR_WAIT_BLOCKED`), 2 on bad input, an unreadable PR or a second watcher for the same PR (`PR_WAIT_REFUSED`; a lock whose pid is dead is taken over). State lives under `<state root>/pr-wait/` (lib/state-root.ts chain), and a baseline older than 6 hours is discarded. Transient gh failures (5xx, 429, network) retry with backoff; ten failed polls in a row end the watch as blocked.
- **`bin/gstack-pr-wait`, `lib/pr-wait.ts`, `lib/pr-wait-github.ts`**: an event-driven pull-request watcher (design ported from tstack's pr-wait, MIT, Theo Browne; NOTICE.md, `licenses/MIT-tstack.txt`). `gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES]` prints one stdout line per real event, prefixed `#<pr>`, and progress on stderr: a check failed (name and link), all checks finished on the head, a comment, review, thread or reply from someone other than the PR author or the gh user, a review bot finished on the head or a gating bot's summary changed, the base branch moved and its new commits touch this PR's files, merge state BEHIND or DIRTY, merged or closed. `green` gates on every check on the head, as `gstack-ci-gate` does, so Step 2 and Step 3 of `/land-and-deploy` never disagree; a check the base branch requires (rulesets and classic branch protection) is pending until it appears, never green by omission, advisory bots' checks never gate, and two identical polls confirm the check set. `ready` is green plus a gating bot's review of the head (green alone when no gating bot is configured), no unresolved thread, and a known merge state (GitHub's `UNKNOWN` right after a push holds it); a gating bot's failed check is a failed check, never a review. A bot owns the logins that start with its name (`greptile-apps[bot]`, not `notgreptile`) and the checks that mention it. `--timeout` (default 60 minutes, 0 for none) ends a watch that met nothing as blocked, naming the still-pending checks. Exit 0 when the condition is met or the PR merged, 1 when it needs action (`PR_WAIT_BLOCKED`), 2 on bad input, an unreadable PR, a second watcher for the same PR or a local error (`PR_WAIT_REFUSED`; a lock whose pid is dead is taken over), so exit 1 always carries a `PR_WAIT_BLOCKED` line. The loop is synchronous and installs no signal handlers, so Ctrl-C and `kill` stop it at once. State lives under `<state root>/pr-wait/` (lib/state-root.ts chain), and a baseline older than 6 hours is discarded. Transient gh failures (5xx, 429, network, gh's own 60 s spawn timeout) retry with backoff; ten failed polls in a row end the watch as blocked. BEHIND and DIRTY are announced once per head, with the short head in the line.
- **`gstack-config` keys `pr_wait_gating_bots` (default `greptile,macroscope`) and `pr_wait_advisory_bots` (default `coderabbit`)**: which review bots `--until ready` waits for and which it only reports; comma-separated names matched against bot logins and check names, `none` empties a list, anything else is rejected with the value kept.
- **Result codes `PR_WAIT_BLOCKED` and `PR_WAIT_REFUSED`** (`lib/result-codes-release.ts`) with their [troubleshooting anchors](docs/troubleshooting.md#gstack-pr-wait); a `gstack-pr-wait` row in docs/standalone-binaries.md.
### Changed
- **`/land-and-deploy` Step 3**: the 4-minute `gstack-ci-gate --wait 240` polling rounds with a 15-minute cap and a wait-more/exclude/stop question are replaced by one `gstack-pr-wait "$PR_NUMBER" --repo "$REPO" --until green` call; exit 0 reruns Step 2's gate on the pinned head, exit 1 stops with the watcher's lines (a red `required=n` check stays the one case Step 3.5 may approve), exit 2 shows the refusal. The Important Rules line about polling intervals names the watcher. The generated skill is the same size on every pinned host, so no pin moved.
- **`/land-and-deploy` Step 3**: the 4-minute `gstack-ci-gate --wait 240` polling rounds with a 15-minute cap and a wait-more/exclude/stop question are replaced by one `gstack-pr-wait "$PR_NUMBER" --repo "$REPO" --until green` call; exit 0 reruns Step 2's gate on the pinned head, exit 1 stops with the watcher's lines (a red `required=n` check stays the one case Step 3.5 may approve), exit 2 shows the refusal. The Important Rules line about polling intervals names the watcher. The old 15-minute cap lives on as the watcher's `--timeout` default of 60 minutes. The generated skill is the same size on every pinned host, so no pin moved.
### Tests
- `test/pr-wait.test.ts` (new): the fake-gh scripts for the baseline poll, the six-hour staleness and the restart carry, author/gh-user/bot/edit filtering, merged and closed, a required check pending by omission, a failed required check on the baseline poll, an unknown bucket as failed, two identical polls before green, ready with a gating bot and an advisory bot, unresolved threads and BEHIND blocking ready, the base-overlap event and its silent negative, BEHIND once per head and DIRTY, the configured bot lists and `none`, the per-PR lock (live holder refused, dead holder taken over, released on exit), argument parsing, transient retry and the 403 on classic protection, and the bin's shebang. `test/land-and-deploy-ci-gate.test.ts` asserts Step 3 is one watcher call with the three exit outcomes and no polling prose.
- `test/pr-wait.test.ts` (new): the fake-gh scripts for the baseline poll, the six-hour staleness and the restart carry, author/gh-user/bot/edit filtering, merged and closed, a required check pending by omission, a failed required check on the baseline poll, an unknown bucket as failed, two identical polls before green, ready with a gating bot and an advisory bot, unresolved threads and BEHIND blocking ready, the base-overlap event and its silent negative, BEHIND once per head and DIRTY, the configured bot lists and `none`, the per-PR lock (live holder refused, dead holder taken over, released on exit), argument parsing, transient retry and the 403 on classic protection, and the bin's shebang; from the /ship review pass: every check gating green (a pending or red non-required check), a failed gating-bot check, prefix-matched bot logins, `ready` with no gating bot and while the merge state is UNKNOWN, `--timeout`, the ten-failure and non-transient `run()` endings, a gh spawn timeout retried, a local fault as exit 2, control characters in check names, `gstack-config` rejecting malformed bot lists and its defaults matching the lib constants, a real SIGTERM against the bin, and the fourteen state-matrix cells the state-machines specialist listed (ready × fail, any-change × pass/pending/fail, BEHIND and DIRTY × any-change, DIRTY × ready, UNKNOWN × green/ready/any-change, the free and dead-pid unlocks, a restarted watcher on a failed required check and on merged). `test/land-and-deploy-ci-gate.test.ts` asserts Step 3 is one watcher call with the three exit outcomes and no polling prose.
### Gate integrity
- No threshold, timeout, trial count or case kind changed. `test/fixtures/module-size-ratchet.json` registers `lib/pr-wait.ts` and `lib/pr-wait-github.ts` under the existing 800-line cap. `test/fixtures/skill-md-size-allowlist.json` is unchanged: the land-and-deploy render is byte-for-byte the pinned size on every host.
+3 -2
View File
@@ -271,8 +271,9 @@ CONFIG_HEADER='# gstack configuration — edit freely, changes take effect on ne
# # asks once). false keeps the old order.
# # Invalid values are REJECTED (value kept).
# pr_wait_gating_bots: greptile,macroscope # review bots gstack-pr-wait
# # --until ready waits for (comma-separated
# # substrings of a bot login or check name).
# # --until ready waits for (comma-separated; a
# # bot owns logins starting with its name and
# # checks mentioning it).
# pr_wait_advisory_bots: coderabbit # review bots gstack-pr-wait reports but
# # never waits for. Both lists: lowercase names,
# # letters, digits, . _ - only; none empties a list.
+5 -8
View File
@@ -2,11 +2,13 @@
/**
* gstack-pr-wait — watch one pull request and print one line per real event.
*
* gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS]
* gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES]
*
* Exit 0 when the --until condition is met or the PR merged, 1 when it needs
* action (PR_WAIT_BLOCKED), 2 on bad input or a duplicate watcher
* (PR_WAIT_REFUSED). Review-bot lists: `gstack-config set pr_wait_gating_bots`
* action (PR_WAIT_BLOCKED), 2 on bad input, a duplicate watcher or a local
* error (PR_WAIT_REFUSED). The loop is synchronous (spawnSync, Bun.sleepSync),
* so signals keep their default disposition: Ctrl-C and kill end the process
* at once and the next watcher takes over the dead pid's lock. Review-bot lists: `gstack-config set pr_wait_gating_bots`
* and `pr_wait_advisory_bots`. Logic: lib/pr-wait.ts and lib/pr-wait-github.ts.
*
* Design ported from tstack's pr-wait (t3dotgg/tstack, MIT, Copyright (c) 2026
@@ -26,10 +28,5 @@ const code = prWaitMain(process.argv.slice(2), {
pidAlive: pid => {
try { process.kill(pid, 0); return true; } catch (error) { return (error as NodeJS.ErrnoException).code === 'EPERM'; }
},
}, release => {
process.on('exit', release);
for (const signal of ['SIGINT', 'SIGTERM', 'SIGHUP'] as const) {
process.on(signal, () => { release(); process.exit(signal === 'SIGINT' ? 130 : 143); });
}
});
process.exit(code);
+1 -1
View File
@@ -16,7 +16,7 @@ Beyond the slash-command skills, gstack ships standalone CLIs for workflows that
| `gstack-review-log` | **Review-pass receipts** — `--start <skill>` captures the working-tree fingerprint before a diff review; `'<JSON>' --finish <token>` consumes that single-use, repository/branch/skill-scoped receipt. Binding requires matching start/end content and reviewer-reported `completed:true` and `converged:true`; it is not independent proof that a model read the source. `--check-shared-libs <token>` reads a current finding as JSON on stdin and checks prior Skip decisions against the actual branch, capture and eligible raw source blobs. It returns `reusable:false` when proof is missing or unsafe, including older records without logger-versioned coverage. Final review logging computes shared-code fingerprints and coverage rather than trusting supplied proof. |
| `gstack-review-read` | **Review freshness** — emits review records with computed `review_freshness.status` and `reason`: CURRENT, STALE, or UNVERIFIED for diff reviews. `/ship` and `/land-and-deploy` use the same grade; a matching commit alone never certifies a diff review. [Dashboard rules](skills.md#review-readiness-dashboard). |
| `gstack-evidence` | **Verification-evidence ledger** — `run --label <lane> -- <cmd>` transparently wraps any test command (the child's exit code always passes through) and records what ran against which working-tree fingerprint; `check` grades each label FRESH/STALE/MISSING with `--expect-cmd`, `--max-age`, and `--allow-paths` binding. /ship and /land-and-deploy cite fresh evidence instead of re-running suites. Per-run logs are 0600, capped at 2MB, pruned after 30 days; the ledger and logs stay machine-local by design. |
| `gstack-pr-wait` | **PR event watcher** — `gstack-pr-wait <pr> [--repo owner/name] [--until green\|ready\|any-change] [--interval 20]` polls one pull request through `gh` and prints one stdout line per real event (a required check failed with its link, all checks finished, a comment or review from someone other than the author, a review bot finished on the head, the base moved and its new commits touch this PR's files, BEHIND or DIRTY, merged or closed), with progress on stderr, so a monitor wakes the agent once per event instead of on a timer. Required checks come from rulesets and classic branch protection, so an unstarted required check is pending rather than green by omission; two identical polls confirm a check set is complete. Exit 0 when the condition is met, 1 when it needs action (`PR_WAIT_BLOCKED`), 2 on bad input or a second watcher for the same PR (`PR_WAIT_REFUSED`). State under `<state root>/pr-wait/`, baseline discarded after 6 hours. Review bots are configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope`, `pr_wait_advisory_bots coderabbit` (advisory bots are reported, never waited on). `/land-and-deploy` Step 3 is one call of it. Design ported from tstack's pr-wait (MIT, Theo Browne; NOTICE.md). [Exit codes](troubleshooting.md#gstack-pr-wait). |
| `gstack-pr-wait` | **PR event watcher** — `gstack-pr-wait <pr> [--repo owner/name] [--until green\|ready\|any-change] [--interval 20] [--timeout 60]` polls one pull request through `gh` and prints one stdout line per real event (a check failed with its link, all checks finished, a comment or review from someone other than the author, a review bot finished on the head, the base moved and its new commits touch this PR's files, BEHIND or DIRTY, merged or closed), with progress on stderr, so a monitor wakes the agent once per event instead of on a timer. `green` gates on every check on the head, as `gstack-ci-gate` does; a check the base branch requires (rulesets and classic branch protection) is pending until it appears, never green by omission; two identical polls confirm a check set is complete. Exit 0 when the condition is met, 1 when it needs action or `--timeout` passes (`PR_WAIT_BLOCKED`), 2 on bad input, a second watcher for the same PR or a local error (`PR_WAIT_REFUSED`). State under `<state root>/pr-wait/`, baseline discarded after 6 hours. Review bots are configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope`, `pr_wait_advisory_bots coderabbit` (advisory bots are reported, never waited on). `/land-and-deploy` Step 3 is one call of it. Design ported from tstack's pr-wait (MIT, Theo Browne; NOTICE.md). [Exit codes](troubleshooting.md#gstack-pr-wait). |
| `gstack-issue-guard` | **Tracker-text trust envelope** — fetches GitHub issue/PR text (`issue <n>`, `pr-body`, `pr-comments`, or `--stdin`) and wraps it in a labeled envelope so agents treat it as data: injection-shaped lines get labeled even through fullwidth and invisible-character evasion, and forged envelope banners are defused. Every tracker-text ingress in gstack routes through it, enforced by a CI scanner. |
| `gstack-ios-qa-daemon` | **iOS QA daemon** — Mac-side broker between an agent and a connected iPhone or iPad over USB CoreDevice. Loopback by default; `--tailnet` opens a Tailscale-facing listener with identity-gated capability tiers. Single-instance via flock on `~/.gstack/ios-qa-daemon.pid`. See [docs/howto-ios-testing-with-gstack.md](howto-ios-testing-with-gstack.md). |
| `gstack-ios-qa-mint` | **iOS allowlist manager** — owner-grant CLI for the tailnet allowlist. `grant`/`revoke`/`list` against `~/.gstack/ios-qa-allowlist.json` (mode 0600). Remote agents never auto-allowlist; this is the explicit-intent path. |
+41 -29
View File
@@ -4363,48 +4363,60 @@ the range-diff evidence ([unattended.md](unattended.md#the-terminal-line)).
## gstack-pr-wait
`gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change]
[--interval SECONDS]` watches one pull request and prints one stdout line per
real event, prefixed `#<pr>`; progress and recoverable errors go to stderr.
`/land-and-deploy` Step 3 runs it with `--until green`. Exit 0 means the
`--until` condition is met or the PR merged; exit 1 and 2 end with one of the
two codes below. Required checks come from the base branch's rulesets and
classic branch protection; a 403 on the classic protection read (a token
without admin scope) is logged and skipped, and the rulesets read still names
the required checks. The review-bot lists are configuration:
`gstack-config set pr_wait_gating_bots greptile,macroscope` and
`pr_wait_advisory_bots coderabbit` (`none` empties a list).
[--interval SECONDS] [--timeout MINUTES]` watches one pull request and prints
one stdout line per real event, prefixed `#<pr>`; progress and recoverable
errors go to stderr. `/land-and-deploy` Step 3 runs it with `--until green`.
Exit 0 means the `--until` condition is met or the PR merged; exit 1 and 2
end with one of the two codes below. `green` gates on every check on the
head, as `gstack-ci-gate` does, so the two never disagree; a check the base
branch requires (rulesets and classic branch protection) counts as pending
until it appears, and advisory bots' checks never gate. A 403 on the classic
protection read (a token without admin scope) is logged and skipped, and the
rulesets read still names the required checks. The review-bot lists are
configuration: `gstack-config set pr_wait_gating_bots greptile,macroscope`
and `pr_wait_advisory_bots coderabbit` (`none` empties a list); a bot owns
the logins that start with its name and the checks that mention it. The loop
is synchronous, so Ctrl-C and `kill` stop it at once; the lock it leaves is
taken over by the next watcher for that PR once the pid is gone.
<a id="pr-wait-blocked"></a>
### `#<pr> stopped: action needed (PR_WAIT_BLOCKED)`
**Meaning.** Exit 1. The condition cannot be met without a change to the PR:
the lines above it say which. A required check failed (name and link; with no
required checks configured, every non-bot check gates); the merge state is
DIRTY (the branch conflicts with its base); the PR was closed without merging;
`--until ready` is blocked by unresolved review threads or a BEHIND branch; or
GitHub stayed unreachable for ten polls, or answered with a non-transient
error (`gstack-pr-wait stopped: <gh message>`).
the lines above it say which. A check failed (name and link; `required check
failed` when the base branch requires it); the merge state is DIRTY (the branch
conflicts with its base); the PR was closed without merging; `--until ready` is
blocked by unresolved review threads or a BEHIND branch; GitHub stayed
unreachable for ten polls, or answered with a non-transient error
(`gstack-pr-wait stopped: <gh message>`); or `--timeout` passed (default 60
minutes) with the condition unmet (`gstack-pr-wait stopped: no green after 60
minutes; still pending: <checks>`).
**Fix.** Act on the named line: fix the failing check and push, rebase or
update the branch, resolve the threads, or read the gh message. Then start
`gstack-pr-wait` again; its state file remembers what it already reported, so
only new events print. A watcher that stopped on a gh error restarts cleanly
once `gh auth status` passes.
update the branch, resolve the threads, or read the gh message. After a
timeout, decide whether the pending checks are worth waiting for and start
again, with `--timeout 0` to wait without limit. Then start `gstack-pr-wait`
again; its state file remembers what it already reported, so only new events
print. A watcher that stopped on a gh error restarts cleanly once `gh auth
status` passes.
<a id="pr-wait-refused"></a>
### `gstack-pr-wait: the watcher did not start — ... (PR_WAIT_REFUSED)`
**Meaning.** Exit 2, before any poll. The target is not a PR number or URL,
`--repo` is not `owner/name`, no repository could be read from the checkout,
the PR cannot be read with the current `gh` login, an option is unknown, or
another `gstack-pr-wait` process (the line names its pid) already watches
this PR from the same state root, so a second one would print duplicate
events.
**Meaning.** Exit 2, never a verdict on the PR. The target is not a PR number
or URL, `--repo` is not `owner/name`, no repository could be read from the
checkout, the PR cannot be read with the current `gh` login, an option is
unknown, another `gstack-pr-wait` process (the line names its pid) already
watches this PR from the same state root, so a second one would print
duplicate events, or a local error (`a local error stopped the watch before a
verdict: <message>`, an unwritable state root for example) ended the process.
Exit 1 is reserved for `PR_WAIT_BLOCKED`, so a local fault never reads as a
PR that needs action.
**Fix.** Pass the number or URL and `--repo owner/name`, check `gh auth
status`, or let the running watcher finish. If no events reach you from the
named pid, `kill <pid>` and start again; a lock whose pid is no longer alive
is taken over without that step.
status`, fix the named local error, or let the running watcher finish. If no
events reach you from the named pid, `kill <pid>` and start again; a lock
whose pid is no longer alive is taken over without that step.
## gstack-error-capture
+4 -4
View File
@@ -90,12 +90,12 @@ export function runGh(env: GhEnv, args: string[], allow: number[] = [0]): GhResu
let delay = 2_000;
for (let attempt = 1; ; attempt++) {
const result = env.gh(args);
if (result.error) throw new GhError(result.error, false);
// A null status is gh killed by a signal (the watcher itself being stopped): retry, never a verdict.
// gh missing is fatal; any other spawn failure (its 60 s timeout, a signal) is a slow API, so retry.
if (result.error && /not installed/.test(result.error)) throw new GhError(result.error, false);
const status = result.status ?? -1;
const transient = status !== 0 && (result.status === null || TRANSIENT.test(result.stderr));
const transient = status !== 0 && (result.error !== undefined || result.status === null || TRANSIENT.test(result.stderr));
if (!transient && allow.includes(status)) return result;
const message = lastLine(result.stderr) || `gh exited ${status}`;
const message = result.error ?? (lastLine(result.stderr) || `gh exited ${status}`);
if (!transient || attempt === RETRIES) throw new GhError(message, transient);
env.log(`gh ${args.slice(0, 2).join(' ')}: ${message}; retry in ${delay / 1000}s`);
env.sleep(delay);
+82 -47
View File
@@ -32,6 +32,8 @@ const STATUS_BOTS = ['github-actions', 'vercel', 'netlify', 'railway', 'cloudfla
export const STALE_STATE_MS = 6 * 3600 * 1000;
const MAX_FAILED_POLLS = 10;
const MIN_INTERVAL_SECONDS = 5;
/** Minutes before a watch that met nothing ends as PR_WAIT_BLOCKED, naming what is still pending; 0 waits without limit. */
export const DEFAULT_TIMEOUT_MINUTES = 60;
export interface PrWaitEnv {
gh: GhRunner;
@@ -44,12 +46,12 @@ export interface PrWaitEnv {
pidAlive(pid: number): boolean;
}
export interface PrWaitOptions { target?: string; repo?: string; until: Until; interval: number }
export interface PrWaitOptions { target?: string; repo?: string; until: Until; interval: number; timeout: number }
export interface BotLists { gating: string[]; advisory: string[] }
interface State { reported: string[]; summaries: Record<string, string>; base_oid: string | null; merge_state: string | null }
export const PR_WAIT_USAGE = `gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS]
export const PR_WAIT_USAGE = `gstack-pr-wait <pr> [--repo owner/name] [--until green|ready|any-change] [--interval SECONDS] [--timeout MINUTES]
Watches one pull request and prints one line per real event on stdout (prefixed
#<pr>), so a monitor wakes the agent once per event. Progress and recoverable
@@ -57,13 +59,18 @@ errors go to stderr. <pr> is a PR number or URL; without --repo the repository
comes from the URL or the current checkout.
--until
green required checks passed on the current head (every CI check when
the base branch requires none), confirmed by two identical polls
ready green, a gating review bot reviewed the current head, and no
review thread is unresolved (default)
green every CI check on the current head passed, confirmed by two
identical polls; a check the base branch requires counts as
pending until it appears, and advisory bots' checks never gate
ready green, a gating review bot reviewed the current head (none
configured: green alone), no review thread is unresolved and the
merge state is known (default)
any-change stop after the first poll that prints an event
Events: a required check failed (name and link); all checks finished on the
--timeout minutes before a watch that met nothing ends as PR_WAIT_BLOCKED,
naming what is still pending (default 60; 0 waits without limit)
Events: a check failed (name and link); all checks finished on the
head (one summary line); a comment, review, thread or thread reply from someone
other than the PR author or the gh user; a review bot finished on the head, or
a gating bot's summary changed afterwards; the base branch moved and the new
@@ -73,23 +80,28 @@ CI and deploy bots are skipped because checks carry that news.
Review bots: gstack-config set pr_wait_gating_bots greptile,macroscope
gstack-config set pr_wait_advisory_bots coderabbit
Advisory bots are reported but never waited on; the value none empties a list.
A bot owns the logins that start with its name (greptile-apps[bot]) and the
checks that mention it. Advisory bots are reported but never waited on; the
value none empties a list.
Exit codes
0 the --until condition is met, or the PR merged
1 the condition needs action (PR_WAIT_BLOCKED): a required check failed,
the branch conflicts with its base, the PR closed, ready is blocked by
unresolved threads or a BEHIND branch, or GitHub stayed unreachable
2 bad arguments, an unknown PR, or another watcher holds this PR
(PR_WAIT_REFUSED)
1 the condition needs action (PR_WAIT_BLOCKED): a check failed, the
branch conflicts with its base, the PR closed, ready is blocked by
unresolved threads or a BEHIND branch, GitHub stayed unreachable, or
--timeout passed
2 bad arguments, an unknown PR, another watcher holds this PR, or a local
error stopped the watch before a verdict (PR_WAIT_REFUSED)
Required checks come from rulesets and classic branch protection, so a
required check that has not started yet is pending, never green by omission.
State: <state root>/pr-wait/<owner>__<name>__<pr>.json and .lock; a baseline
older than 6 hours is discarded. Docs: docs/troubleshooting.md#gstack-pr-wait`;
older than 6 hours is discarded. Stopping the watcher (Ctrl-C, kill) leaves its
lock, which the next watcher for the PR takes over once the pid is gone.
Docs: docs/troubleshooting.md#gstack-pr-wait`;
export function parsePrWaitArgs(argv: string[]): PrWaitOptions | { help: true } | { error: string } {
const opts: PrWaitOptions = { until: 'ready', interval: 20 };
const opts: PrWaitOptions = { until: 'ready', interval: 20, timeout: DEFAULT_TIMEOUT_MINUTES };
for (let i = 0; i < argv.length; i++) {
const flag = argv[i];
if (flag === '--help' || flag === '-h') return { help: true };
@@ -106,6 +118,12 @@ export function parsePrWaitArgs(argv: string[]): PrWaitOptions | { help: true }
opts.interval = Math.max(Number(value), MIN_INTERVAL_SECONDS);
continue;
}
if (flag === '--timeout') {
const value = argv[++i] ?? '';
if (!/^[0-9]+$/.test(value)) return { error: `--timeout needs a whole number of minutes, not '${value}'` };
opts.timeout = Number(value);
continue;
}
if (flag.startsWith('-') && !/^-?\d+$/.test(flag)) return { error: `unknown argument ${flag}` };
if (opts.target !== undefined) return { error: `unexpected argument ${flag}` };
opts.target = flag;
@@ -169,9 +187,10 @@ export class Watcher {
private required: Set<string> | null = null;
private prevSnapshot: string | null = null;
private prFiles = new Map<string, Set<string>>();
private pending: string[] = [];
private readonly gh: GhEnv;
constructor(readonly env: PrWaitEnv, readonly repo: string, readonly pr: number, readonly until: Until, readonly interval: number, readonly bots: BotLists) {
constructor(readonly env: PrWaitEnv, readonly repo: string, readonly pr: number, readonly until: Until, readonly interval: number, readonly bots: BotLists, readonly timeoutMinutes = DEFAULT_TIMEOUT_MINUTES) {
const stem = path.join(stateDir(env.vars), `${repo.replace('/', '__')}__${pr}`);
this.stateFile = `${stem}.json`;
this.lockFile = `${stem}.lock`;
@@ -224,10 +243,11 @@ export class Watcher {
} catch { /* already gone */ }
}
/** One event is one line: control characters from GitHub text can never start a second one. */
private say(line: string): void {
if (this.printed.has(line)) return;
this.printed.add(line);
this.env.out(`#${this.pr} ${line}\n`);
this.env.out(`#${this.pr} ${line.replace(/[\x00-\x1f\x7f]+/g, ' ')}\n`);
}
/** Print an event once per PR, across restarts. */
@@ -246,9 +266,10 @@ export class Watcher {
return code;
}
private botOf(text: string): string | null {
/** The configured bot a check name mentions, or (for a login) that the login starts with: greptile-apps[bot] is greptile, notgreptile is nobody. */
private botOf(text: string, login = false): string | null {
const lowered = text.toLowerCase();
return [...this.bots.gating, ...this.bots.advisory].find(bot => lowered.includes(bot)) ?? null;
return [...this.bots.gating, ...this.bots.advisory].find(bot => login ? lowered.startsWith(bot) : lowered.includes(bot)) ?? null;
}
private isAdvisory(name: string): boolean {
@@ -275,32 +296,33 @@ export class Watcher {
private evaluateChecks(checks: Map<string, Check>, head: string) {
const required = this.required ?? new Set<string>();
const gate = required.size > 0 ? required : new Set([...checks.keys()].filter(name => !this.botOf(name)));
const label = required.size > 0 ? 'required check' : 'check';
// Every check on the head gates, as gstack-ci-gate does, so the two never disagree; a required
// check gates even before it appears, and advisory bots' checks never do.
const watched = new Map([...checks].filter(([name]) => !this.isAdvisory(name)));
const gate = new Set([...required, ...watched.keys()]);
const bucketOf = (name: string) => checks.get(name)?.bucket ?? 'pending';
const failLines: string[] = [];
for (const name of [...gate].sort()) {
if (bucketClass(bucketOf(name)) !== 'fail') continue;
const line = `${label} failed: ${name} ${checks.get(name)?.link ?? ''}`.trimEnd();
const line = `${required.has(name) ? 'required check' : 'check'} failed: ${name} ${checks.get(name)?.link ?? ''}`.trimEnd();
failLines.push(line);
this.emit(`fail:${head}:${name}:${checks.get(name)?.link ?? ''}`, line);
}
const watched = new Map([...checks].filter(([name]) => !this.isAdvisory(name)));
const gateDone = gate.size > 0 && [...gate].every(name => bucketClass(bucketOf(name)) !== 'pending');
const allDone = gateDone && [...watched.values()].every(check => bucketClass(check.bucket) !== 'pending');
this.pending = [...gate].filter(name => bucketClass(bucketOf(name)) === 'pending').sort();
const allDone = gate.size > 0 && this.pending.length === 0;
// Checks appear a few seconds after a push: two identical polls in a row
// guard against calling a half-registered set of checks finished.
const snapshot = JSON.stringify([...watched].map(([name, check]) => [name, check.bucket]).sort());
const stable = snapshot === this.prevSnapshot;
this.prevSnapshot = snapshot;
return { watched, failLines, allDone, stable, green: gateDone && stable && failLines.length === 0, requiredGreen: [...required].filter(name => bucketClass(bucketOf(name)) === 'pass').length };
return { watched, failLines, allDone, stable, green: allDone && stable && failLines.length === 0, requiredGreen: [...required].filter(name => bucketClass(bucketOf(name)) === 'pass').length };
}
/** People talking. Edits are never events. Returns the unresolved threads' first-comment URLs. */
private emitConversation(pr: PullRequest, mine: Set<string>): string[] {
for (const c of pr.comments) {
const { login, bot } = c.author;
if (mine.has(login) || this.botOf(login) || (bot && STATUS_BOTS.some(b => login.includes(b)))) continue;
if (mine.has(login) || this.botOf(login, true) || (bot && STATUS_BOTS.some(b => login.includes(b)))) continue;
this.emit(`c:${c.id}`, `comment from ${login}: ${snippet(c.body)} ${c.url}`);
}
for (const r of pr.reviews) {
@@ -332,10 +354,13 @@ export class Watcher {
for (const bot of [...this.bots.gating, ...this.bots.advisory]) {
const advisory = this.bots.advisory.includes(bot);
const ownChecks = [...checks.values()].filter(c => this.botOf(c.name) === bot);
const summaries = pr.comments.filter(c => this.botOf(c.author.login) === bot);
const ownReviews = pr.reviews.filter(r => this.botOf(r.author.login) === bot);
const summaries = pr.comments.filter(c => this.botOf(c.author.login, true) === bot);
const ownReviews = pr.reviews.filter(r => this.botOf(r.author.login, true) === bot);
const onHead = ownReviews.some(r => r.commitOid === head);
const done = ownChecks.length > 0 ? ownChecks.every(c => bucketClass(c.bucket) !== 'pending') : onHead || summaries.some(s => s.body.includes(short));
// A bot that reports through checks is done when they pass; a failed gating check is a failed check, never a review.
const done = ownChecks.length > 0
? ownChecks.every(c => bucketClass(c.bucket) === 'pass' || (advisory && bucketClass(c.bucket) !== 'pending'))
: onHead || summaries.some(s => s.body.includes(short));
const seen = ownChecks.length > 0 || summaries.length > 0 || ownReviews.length > 0;
if (advisory && ownChecks.length > 0) status.push(`${bot} ${ownChecks[0].description || ownChecks[0].bucket} (advisory)`);
else if (seen) status.push(`${bot} ${done ? 'done' : 'pending'}${advisory ? ' (advisory)' : ''}`);
@@ -372,14 +397,15 @@ export class Watcher {
if (pr.baseOid) this.state.base_oid = pr.baseOid;
const mergeState = pr.mergeStateStatus;
if (mergeState === 'UNKNOWN') return;
if ((mergeState === 'BEHIND' || mergeState === 'DIRTY') && this.state.merge_state !== mergeState) {
this.emit(`merge:${mergeState}:${head}:${pr.baseOid ?? ''}`, mergeState === 'DIRTY' ? this.dirtyLine(base) : this.behindLine(base));
// The key holds the head and base, so each state is announced once per head, not once per transition.
if (mergeState === 'BEHIND' || mergeState === 'DIRTY') {
this.emit(`merge:${mergeState}:${head}:${pr.baseOid ?? ''}`, mergeState === 'DIRTY' ? this.dirtyLine(base, head) : this.behindLine(base, head));
}
this.state.merge_state = mergeState;
}
private dirtyLine(base: string): string { return `merge state is DIRTY: the branch conflicts with ${base}, rebase it`; }
private behindLine(base: string): string { return `merge state is BEHIND: update the branch from ${base} before merging`; }
private dirtyLine(base: string, head: string): string { return `merge state is DIRTY on ${head.slice(0, 7)}: the branch conflicts with ${base}, rebase it`; }
private behindLine(base: string, head: string): string { return `merge state is BEHIND on ${head.slice(0, 7)}: update the branch from ${base} before merging`; }
/** One poll. Returns an exit code when the watch ends, null to keep polling. Throws GhError. */
poll(): number | null {
@@ -389,8 +415,8 @@ export class Watcher {
const base = pr.baseRefName;
if (this.required === null) {
this.required = requiredChecks(this.gh, this.repo, base);
const gate = [...this.required].sort().join(', ') || 'none set, so every CI check gates';
this.log(`watching ${this.repo}#${this.pr} until ${this.until}; required checks: ${gate}`);
const gate = [...this.required].sort().join(', ') || 'none set';
this.log(`watching ${this.repo}#${this.pr} until ${this.until}; required checks: ${gate}; every check on the head gates`);
}
const checks = fetchChecks(this.gh, this.repo, this.pr);
const head = pr.headRefOid;
@@ -417,16 +443,18 @@ export class Watcher {
// A green or ready line carries the same summary as the checks line, so it marks that event as reported.
if (this.until !== 'any-change') {
if (mergeState === 'DIRTY') return this.finish(PR_WAIT_EXIT.blocked, [this.dirtyLine(base)]);
if (mergeState === 'DIRTY') return this.finish(PR_WAIT_EXIT.blocked, [this.dirtyLine(base, head)]);
if (ci.failLines.length > 0) return this.finish(PR_WAIT_EXIT.blocked, ci.failLines);
if (ci.green && this.until === 'green') {
this.reported.add(checksKey);
return this.finish(PR_WAIT_EXIT.met, [`green on ${short}: ${summary}${this.behindSuffix(base, pr.baseOid, head)}`]);
}
// ready waits for a gating bot, never for an advisory one.
if (ci.green && bots.reviewed.length > 0) {
// ready waits for a gating bot, never for an advisory one; with no gating bot configured, green is the review.
if (ci.green && (bots.reviewed.length > 0 || this.bots.gating.length === 0)) {
if (unresolved.length > 0) return this.finish(PR_WAIT_EXIT.blocked, [`blocked by ${unresolved.length} unresolved threads on ${short}: ${unresolved.join(' ')}`]);
if (mergeState === 'BEHIND') return this.finish(PR_WAIT_EXIT.blocked, [this.behindLine(base)]);
if (mergeState === 'BEHIND') return this.finish(PR_WAIT_EXIT.blocked, [this.behindLine(base, head)]);
// GitHub reports UNKNOWN while it recomputes mergeability after a push: not ready until it knows.
if (mergeState === 'UNKNOWN') return null;
this.reported.add(checksKey);
return this.finish(PR_WAIT_EXIT.met, [`ready on ${short}: ${summary}${this.behindSuffix(base, pr.baseOid, head)}`]);
}
@@ -439,6 +467,7 @@ export class Watcher {
/** Poll until the watch ends; returns the exit code. */
run(): number {
let failures = 0;
const deadline = this.timeoutMinutes > 0 ? this.env.now() + this.timeoutMinutes * 60_000 : Infinity;
for (;;) {
try {
const code = this.poll();
@@ -447,6 +476,10 @@ export class Watcher {
this.save();
if (this.baseline) this.log(`baseline recorded for ${this.repo}#${this.pr}; reporting only new events`);
this.baseline = false;
if (this.env.now() >= deadline) {
const still = this.pending.length > 0 ? `still pending: ${this.pending.join(', ')}` : 'nothing pending, the condition was not met';
return this.finish(PR_WAIT_EXIT.blocked, [`gstack-pr-wait stopped: no ${this.until} after ${this.timeoutMinutes} minutes; ${still}`]);
}
} catch (error) {
if (!(error instanceof GhError)) throw error;
if (!error.transient) return this.finish(PR_WAIT_EXIT.blocked, [`gstack-pr-wait stopped: ${error.message}`]);
@@ -484,7 +517,7 @@ export function resolveTarget(env: PrWaitEnv, opts: PrWaitOptions): { repo: stri
return { repo, pr: target.pr };
}
export function prWaitMain(argv: string[], env: PrWaitEnv, onLocked?: (release: () => void) => void): number {
export function prWaitMain(argv: string[], env: PrWaitEnv): number {
const parsed = parsePrWaitArgs(argv);
if ('help' in parsed) {
env.out(`${PR_WAIT_USAGE}\n`);
@@ -493,14 +526,16 @@ export function prWaitMain(argv: string[], env: PrWaitEnv, onLocked?: (release:
if ('error' in parsed) return refuse(env, parsed.error);
const target = resolveTarget(env, parsed);
if ('error' in target) return refuse(env, target.error);
const watcher = new Watcher(env, target.repo, target.pr, parsed.until, parsed.interval, botLists(env.vars));
const lock = watcher.lock();
if (lock) {
return refuse(env, `pid ${lock.held} already watches ${target.repo}#${target.pr}, so this watcher exits to avoid duplicate events. If no events reach you from it, run \`kill ${lock.held}\` and start again`);
}
onLocked?.(() => watcher.unlock());
const watcher = new Watcher(env, target.repo, target.pr, parsed.until, parsed.interval, botLists(env.vars), parsed.timeout);
// Exit 1 always carries a PR_WAIT_BLOCKED line, so a local fault (an unwritable state root, a full disk) is a refusal, never a verdict.
try {
const lock = watcher.lock();
if (lock) {
return refuse(env, `pid ${lock.held} already watches ${target.repo}#${target.pr}, so this watcher exits to avoid duplicate events. If no events reach you from it, run \`kill ${lock.held}\` and start again`);
}
return watcher.run();
} catch (error) {
return refuse(env, `a local error stopped the watch before a verdict: ${error instanceof Error ? error.message : String(error)}`);
} finally {
watcher.unlock();
}
+257 -11
View File
@@ -10,9 +10,10 @@ import { afterEach, beforeEach, describe, expect, test } from 'bun:test';
import { mkdtempSync, readFileSync, rmSync, utimesSync, writeFileSync, existsSync, mkdirSync } from 'node:fs';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { spawnSync } from 'node:child_process';
import { spawn, spawnSync } from 'node:child_process';
import type { GhResult } from '../lib/ci-gate';
import { PR_WAIT_EXIT, Watcher, botLists, parsePrTarget, parsePrWaitArgs, prWaitMain, stateDir, type PrWaitEnv, type PrWaitOptions } from '../lib/pr-wait';
import { DEFAULT_ADVISORY_BOTS, DEFAULT_GATING_BOTS, PR_WAIT_EXIT, Watcher, botLists, parsePrTarget, parsePrWaitArgs, prWaitMain, stateDir, type PrWaitEnv, type PrWaitOptions } from '../lib/pr-wait';
import { GhError, runGh } from '../lib/pr-wait-github';
import { RESULT_CODES } from '../lib/result-codes';
const ROOT = join(import.meta.dir, '..');
@@ -178,7 +179,7 @@ describe('events: one stdout line each, progress on stderr', () => {
'#42 review from reviewer (changes requested): nope https://github.com/owner/project/pull/42#pullrequestreview-r1',
'#42 new thread from reviewer on lib/x.ts:12: rename this https://github.com/owner/project/pull/42#issuecomment-t1',
]);
expect(stderr.join('')).toContain('watching owner/project#42 until any-change; required checks: none set, so every CI check gates');
expect(stderr.join('')).toContain('watching owner/project#42 until any-change; required checks: none set; every check on the head gates');
expect(stderr.join('')).toContain('baseline recorded for owner/project#42');
});
@@ -220,7 +221,7 @@ describe('required checks and the green condition', () => {
expect(lines()).toEqual(['#42 green on aaaaaaa: 3 pass; required 2/2 green; no greptile or macroscope yet; 0 unresolved threads; merge state CLEAN; 2 commits behind main']);
});
test('with no required checks every CI check gates except the review bots\' own, and a gating bot\'s review on the head makes it ready', () => {
test('every check on the head gates except advisory bots\' own, and a gating bot\'s review on the head makes it ready', () => {
const pr = prJson({ reviews: [review('g1', { login: 'greptile-apps', bot: true }, 'COMMENTED', 'Confidence score: 4/5')] });
const w = watcher({ pr, checks: ok(checksJson(['build', 'pass'], ['greptile-review', 'pass'], ['coderabbit', 'pending', 'Review in progress'])) });
expect(pollUntilDone(w)).toBe(PR_WAIT_EXIT.met);
@@ -234,7 +235,7 @@ describe('required checks and the green condition', () => {
expect(lines()[0]).toBe('#42 blocked by 1 unresolved threads on aaaaaaa: https://github.com/owner/project/pull/42#issuecomment-t1');
stdout = [];
expect(pollUntilDone(watcher({ ...base, pr: prJson({ reviews: [reviewed], mergeState: 'BEHIND' }) }))).toBe(1);
expect(lines()).toContain('#42 merge state is BEHIND: update the branch from main before merging');
expect(lines()).toContain('#42 merge state is BEHIND on aaaaaaa: update the branch from main before merging');
});
});
@@ -272,14 +273,18 @@ describe('base branch and merge state', () => {
expect(lines()).toEqual(['#42 main moved to ccccccc and touches files in this PR: x']);
});
test('BEHIND is an event once per head; DIRTY ends the watch as blocked', () => {
const behind = watcher({ pr: [prJson({ mergeState: 'CLEAN' }), prJson({ mergeState: 'BEHIND' })], checks: ok(checksJson(['build', 'pending'])) }, 'green');
expect(pollUntilDone(behind, 4)).toBeNull();
expect(lines()).toEqual(['#42 merge state is BEHIND: update the branch from main before merging']);
test('BEHIND is an event once per head, again after a push that stays BEHIND; DIRTY ends the watch as blocked', () => {
const D = 'd'.repeat(40);
const behind = watcher({ pr: [prJson({ mergeState: 'CLEAN' }), prJson({ mergeState: 'BEHIND' }), prJson({ mergeState: 'BEHIND' }), prJson({ mergeState: 'UNKNOWN', head: D }), prJson({ mergeState: 'BEHIND', head: D })], checks: ok(checksJson(['build', 'pending'])) }, 'green');
expect(pollUntilDone(behind, 5)).toBeNull();
expect(lines()).toEqual([
'#42 merge state is BEHIND on aaaaaaa: update the branch from main before merging',
'#42 merge state is BEHIND on ddddddd: update the branch from main before merging',
]);
stdout = [];
const dirty = watcher({ pr: prJson({ mergeState: 'DIRTY' }) }, 'green');
expect(dirty.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()[0]).toBe('#42 merge state is DIRTY: the branch conflicts with main, rebase it');
expect(lines()[0]).toBe('#42 merge state is DIRTY on aaaaaaa: the branch conflicts with main, rebase it');
});
});
@@ -352,7 +357,9 @@ describe('arguments and the gh layer', () => {
expect(parsePrTarget('abc')).toMatchObject({ error: "'abc' is not a PR number or PR URL" });
expect(parsePrTarget('0')).toMatchObject({ error: "'0' is not a valid PR number" });
expect(parsePrTarget('42', 'nope')).toMatchObject({ error: "'nope' is not owner/name" });
expect(parsePrWaitArgs(['42', '--until', 'green', '--interval', '2'])).toEqual({ target: '42', until: 'green', interval: 5 });
expect(parsePrWaitArgs(['42', '--until', 'green', '--interval', '2'])).toEqual({ target: '42', until: 'green', interval: 5, timeout: 60 });
expect(parsePrWaitArgs(['42', '--timeout', '0'])).toMatchObject({ timeout: 0 });
expect(parsePrWaitArgs(['42', '--timeout', 'soon'])).toMatchObject({ error: expect.stringContaining('--timeout needs') });
expect(parsePrWaitArgs(['--until', 'soon'])).toMatchObject({ error: expect.stringContaining('--until needs') });
expect(parsePrWaitArgs(['--bogus'])).toMatchObject({ error: 'unknown argument --bogus' });
expect(parsePrWaitArgs(['--help'])).toEqual({ help: true });
@@ -394,3 +401,242 @@ describe('arguments and the gh layer', () => {
expect(readFileSync(join(ROOT, 'bin', 'gstack-pr-wait'), 'utf8').startsWith('#!/usr/bin/env -S bun --no-env-file --config=/dev/null\n')).toBe(true);
});
});
describe('pass-1 review fixes: gates agree, bounded waits, bot identity, local faults', () => {
function withConfig(yaml: string) {
mkdirSync(join(dir, 'state'), { recursive: true });
writeFileSync(join(dir, 'state', 'config.yaml'), yaml);
}
const GREPTILE = review('g1', { login: 'greptile-apps', bot: true }, 'COMMENTED', '');
test('green gates on every check on the head, as gstack-ci-gate does: a pending non-required check holds it and a red one blocks it', () => {
const pending = watcher({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'pass'], ['windows-free-tests', 'pass'], ['quality-gate', 'pending'])) }, 'green');
expect(pollUntilDone(pending, 4)).toBeNull();
expect(lines()).toEqual([]);
const red = watcher({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'pass'], ['windows-free-tests', 'pass'], ['quality-gate', 'fail'])) }, 'green');
expect(red.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()).toEqual(['#42 check failed: quality-gate https://ci.example/quality-gate', expect.stringContaining('(PR_WAIT_BLOCKED;')]);
});
test('a failed gating-bot check is a failed check, never a completed review; a failed advisory check is reported and not waited on', () => {
const gating = watcher({ pr: prJson({ reviews: [] }), checks: ok(checksJson(['build', 'pass'], ['greptile-review', 'fail'])) });
expect(pollUntilDone(gating)).toBe(PR_WAIT_EXIT.blocked);
expect(lines()[0]).toBe('#42 check failed: greptile-review https://ci.example/greptile-review');
stdout = [];
const advisory = watcher({ pr: prJson({ reviews: [GREPTILE] }), checks: ok(checksJson(['build', 'pass'], ['coderabbit', 'fail'])) });
expect(pollUntilDone(advisory)).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual([
'#42 greptile finished on aaaaaaa',
'#42 coderabbit finished on aaaaaaa: failed: coderabbit (advisory, not waited on)',
expect.stringContaining('ready on aaaaaaa: 1 pass; greptile done, coderabbit fail (advisory)'),
]);
});
test('a bot owns logins that start with its name, so a person called notgreptile is a reviewer, not the gating bot', () => {
const human = review('h1', { login: 'notgreptile' }, 'COMMENTED', 'looks fine');
const w = watcher({ pr: [prJson({}), prJson({ reviews: [human] })], checks: ok(checksJson(['build', 'pass'])) });
expect(pollUntilDone(w, 4)).toBeNull();
expect(lines()).toEqual(['#42 review from notgreptile (commented): looks fine https://github.com/owner/project/pull/42#pullrequestreview-h1']);
});
test('ready with no gating bot configured is green plus the thread and merge-state checks', () => {
withConfig('pr_wait_gating_bots: none\n');
const w = watcher({ checks: ok(checksJson(['build', 'pass'])) });
expect(w.bots.gating).toEqual([]);
expect(pollUntilDone(w, 4)).toBe(PR_WAIT_EXIT.met);
expect(lines()[0]).toContain('ready on aaaaaaa: 1 pass; no review bot yet');
});
test('ready waits while the merge state is UNKNOWN and is met once GitHub knows it', () => {
const w = watcher({ pr: [prJson({ mergeState: 'UNKNOWN', reviews: [GREPTILE] }), prJson({ mergeState: 'UNKNOWN', reviews: [GREPTILE] }), prJson({ mergeState: 'UNKNOWN', reviews: [GREPTILE] }), prJson({ mergeState: 'CLEAN', reviews: [GREPTILE] })], checks: ok(checksJson(['build', 'pass'])) });
expect(w.poll()).toBeNull();
w.baseline = false;
expect(w.poll()).toBeNull();
expect(w.poll()).toBeNull();
expect(lines()).toEqual([]);
expect(w.poll()).toBe(PR_WAIT_EXIT.met);
expect(lines()[0]).toContain('merge state CLEAN');
});
test('--timeout ends a watch that met nothing as PR_WAIT_BLOCKED, naming what is still pending; 0 waits without limit', () => {
function timed(timeout: number) {
const e = env(fakeGh({ checks: ok(checksJson(['build', 'pending'], ['docs', 'pass'])) }));
e.sleep = ms => { clock += ms; };
return new Watcher(e, REPO, 42, 'green', 20, botLists(e.vars), timeout);
}
expect(timed(1).run()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()).toEqual(['#42 gstack-pr-wait stopped: no green after 1 minutes; still pending: build', expect.stringContaining('(PR_WAIT_BLOCKED;')]);
stdout = [];
const forever = timed(0);
let polls = 0;
const poll = forever.poll.bind(forever);
forever.poll = () => { polls += 1; return polls < 50 ? poll() : PR_WAIT_EXIT.met; };
expect(forever.run()).toBe(PR_WAIT_EXIT.met);
expect(polls).toBe(50);
});
test('run() ends as blocked after ten transient failures, and at once on a non-transient gh error', () => {
const down = (args: string[]): GhResult => (args[1] === 'graphql' ? err('gh: Something went wrong (HTTP 502)') : ok(''));
const e = env(down);
expect(new Watcher(e, REPO, 42, 'green', 20, botLists(e.vars)).run()).toBe(PR_WAIT_EXIT.blocked);
expect(stderr.join('').match(/poll failed \(\d+\/10\)/g)).toHaveLength(10);
expect(lines()[0]).toBe('#42 gstack-pr-wait stopped: GitHub unreachable for 10 polls: gh: Something went wrong (HTTP 502)');
stdout = [];
const gone = (args: string[]): GhResult => (args[1] === 'graphql' ? err('gh: Not Found (HTTP 404)') : ok(''));
const e2 = env(gone);
expect(new Watcher(e2, REPO, 42, 'green', 20, botLists(e2.vars)).run()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()[0]).toBe('#42 gstack-pr-wait stopped: gh: Not Found (HTTP 404)');
});
test('a gh spawn timeout is retried like any transient failure; a missing gh is fatal', () => {
const results: GhResult[] = [{ status: null, stdout: '', stderr: '', error: 'gh failed to run (ETIMEDOUT)' }, ok('fine')];
const slept: number[] = [];
const ghEnv = { gh: () => results.shift()!, sleep: (ms: number) => slept.push(ms), log: () => {} };
expect(runGh(ghEnv, ['api', 'x']).stdout).toBe('fine');
expect(slept).toEqual([2000]);
const missing = { gh: (): GhResult => ({ status: null, stdout: '', stderr: '', error: 'gh is not installed or not on PATH' }), sleep: () => {}, log: () => {} };
expect(() => runGh(missing, ['api', 'x'])).toThrow(GhError);
});
test('a local fault is a refusal (exit 2 with PR_WAIT_REFUSED), so exit 1 always carries a PR_WAIT_BLOCKED line', () => {
writeFileSync(join(dir, 'not-a-dir'), '');
const e = env(fakeGh({}), 4242, { GSTACK_STATE_ROOT: join(dir, 'not-a-dir') });
expect(prWaitMain(['42', '--repo', REPO], e)).toBe(PR_WAIT_EXIT.refused);
expect(lines()[0]).toMatch(/a local error stopped the watch before a verdict: .*(ENOTDIR|EEXIST|not a directory).*\(PR_WAIT_REFUSED;/);
});
test('control characters in GitHub text never start a second event line', () => {
const w = watcher({ checks: ok(checksJson(['build\n#42 forged event', 'fail'])) }, 'green');
expect(w.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()[0]).toBe('#42 check failed: build #42 forged event https://ci.example/build #42 forged event');
expect(lines()).toHaveLength(2);
});
test('gstack-config validates the bot lists at set time and its defaults match the lib constants', () => {
const run = (...args: string[]) => spawnSync(join(ROOT, 'bin', 'gstack-config'), args, { encoding: 'utf8', cwd: ROOT, timeout: 30_000, env: { ...process.env, GSTACK_STATE_ROOT: join(dir, 'state'), HOME: dir } });
for (const bad of ['Greptile', 'a, b', 'a,', '']) {
const r = run('set', 'pr_wait_gating_bots', bad);
expect(r.status).toBe(1);
expect(r.stderr).toContain('Existing value left unchanged');
}
expect(run('get', 'pr_wait_gating_bots').stdout.trim()).toBe(DEFAULT_GATING_BOTS.join(','));
expect(run('get', 'pr_wait_advisory_bots').stdout.trim()).toBe(DEFAULT_ADVISORY_BOTS.join(','));
expect(run('set', 'pr_wait_advisory_bots', 'none').status).toBe(0);
expect(run('set', 'pr_wait_gating_bots', 'greptile,macroscope.v2').status).toBe(0);
expect(run('get', 'pr_wait_gating_bots').stdout.trim()).toBe('greptile,macroscope.v2');
expect(botLists({ HOME: dir, GSTACK_STATE_ROOT: join(dir, 'state') })).toEqual({ gating: ['greptile', 'macroscope.v2'], advisory: [] });
});
test('the bin dies on SIGTERM at once (no JS handler swallows it) and the next watcher takes over its lock', async () => {
const fakeBin = join(dir, 'fakebin');
mkdirSync(fakeBin);
writeFileSync(join(fakeBin, 'gh'), `#!/bin/sh
case "$1 $2" in
'pr view') echo '{"number":42}' ;;
'api graphql') echo '${prJson({})}' ;;
'pr checks') echo '${checksJson(['build', 'pending'])}'; exit 8 ;;
*) echo 'gh: Not Found (HTTP 404)' >&2; exit 1 ;;
esac`, { mode: 0o755 });
const stateRoot = join(dir, 'state');
const child = spawn(join(ROOT, 'bin', 'gstack-pr-wait'), ['42', '--repo', REPO, '--interval', '5'], { cwd: ROOT, env: { ...process.env, PATH: `${fakeBin}:${process.env.PATH}`, GSTACK_STATE_ROOT: stateRoot, HOME: dir } });
const lockFile = join(stateRoot, 'pr-wait', 'owner__project__42.lock');
const started = Date.now();
while (!existsSync(lockFile) && Date.now() - started < 15_000) await Bun.sleep(100);
expect(existsSync(lockFile)).toBe(true);
child.kill('SIGTERM');
const ended = await Promise.race([
new Promise<string>(resolve => child.on('exit', (code, signal) => resolve(`exit ${code} ${signal}`))),
Bun.sleep(5_000).then(() => 'alive'),
]);
expect(ended).toBe('exit null SIGTERM');
expect(readFileSync(lockFile, 'utf8')).toBe(String(child.pid));
const next = watcher({});
expect(next.lock()).toBeNull();
expect(readFileSync(lockFile, 'utf8')).toBe('4242');
}, 30_000);
});
describe('state matrix cells (gstack-state-matrix: --until × bucket, merge state × --until, lock, baseline × event)', () => {
const GREPTILE = review('g1', { login: 'greptile-apps', bot: true }, 'COMMENTED', '');
test('ready × fail: exit 1 with the failed-check line and PR_WAIT_BLOCKED', () => {
const w = watcher({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'fail'], ['windows-free-tests', 'pass'])) });
expect(w.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()).toEqual(['#42 required check failed: free-tests https://ci.example/free-tests', expect.stringContaining('(PR_WAIT_BLOCKED;')]);
});
test('any-change × pass: silent while nothing is new; one "checks finished" line once the set turns stable, then exit 0', () => {
const w = watcher({ checks: [ok(checksJson(['build', 'pending'])), ok(checksJson(['build', 'pass']))] }, 'any-change');
expect(w.poll()).toBeNull();
w.baseline = false;
expect(w.poll()).toBeNull();
expect(lines()).toEqual([]);
expect(w.poll()).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual([expect.stringMatching(/^#42 checks finished on aaaaaaa: 1 pass; no greptile or macroscope yet; 0 unresolved threads; merge state CLEAN$/)]);
});
test('any-change × pending: silent, keeps polling', () => {
expect(pollUntilDone(watcher({ checks: ok(checksJson(['build', 'pending'])) }, 'any-change'), 4)).toBeNull();
expect(lines()).toEqual([]);
});
test('any-change × fail: the failed check is an event and the watch ends with exit 0, not blocked', () => {
const w = watcher({ checks: [ok(checksJson(['build', 'pending'])), ok(checksJson(['build', 'fail']))] }, 'any-change');
expect(pollUntilDone(w)).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual(['#42 check failed: build https://ci.example/build']);
});
test('BEHIND × any-change and DIRTY × any-change: one merge-state line each, exit 0, no PR_WAIT_BLOCKED', () => {
const behind = watcher({ pr: [prJson({ mergeState: 'CLEAN' }), prJson({ mergeState: 'BEHIND' })], checks: ok(checksJson(['build', 'pending'])) }, 'any-change');
expect(pollUntilDone(behind)).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual(['#42 merge state is BEHIND on aaaaaaa: update the branch from main before merging']);
stdout = [];
const dirty = watcher({ pr: [prJson({ mergeState: 'CLEAN' }), prJson({ mergeState: 'DIRTY' })], checks: ok(checksJson(['build', 'pending'])) }, 'any-change');
expect(pollUntilDone(dirty)).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual(['#42 merge state is DIRTY on aaaaaaa: the branch conflicts with main, rebase it']);
});
test('DIRTY × ready: exit 1 with the DIRTY line and PR_WAIT_BLOCKED', () => {
const w = watcher({ pr: prJson({ mergeState: 'DIRTY', reviews: [GREPTILE] }), checks: ok(checksJson(['build', 'pass'])) });
expect(w.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()).toEqual(['#42 merge state is DIRTY on aaaaaaa: the branch conflicts with main, rebase it', expect.stringContaining('(PR_WAIT_BLOCKED;')]);
});
test('UNKNOWN × green, UNKNOWN × any-change: no merge-state event; green is decided by the checks alone', () => {
const green = watcher({ pr: prJson({ mergeState: 'UNKNOWN' }), checks: ok(checksJson(['build', 'pass'])) }, 'green');
expect(pollUntilDone(green)).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual([expect.stringMatching(/^#42 green on aaaaaaa: 1 pass; no greptile or macroscope yet; 0 unresolved threads; merge state UNKNOWN; 0 commits behind main$/)]);
expect(JSON.parse(readFileSync(green.stateFile, 'utf8')).merge_state).toBeNull();
stdout = [];
expect(pollUntilDone(watcher({ pr: prJson({ mergeState: 'UNKNOWN' }), checks: ok(checksJson(['build', 'pending'])) }, 'any-change'), 4)).toBeNull();
expect(lines()).toEqual([]);
});
test('lock: free × unlock is a no-op; dead-pid × unlock leaves the stale file for the next lock() to take over', () => {
const w = watcher({});
expect(() => w.unlock()).not.toThrow();
expect(existsSync(w.lockFile)).toBe(false);
mkdirSync(join(w.lockFile, '..'), { recursive: true });
writeFileSync(w.lockFile, '999999');
w.unlock();
expect(readFileSync(w.lockFile, 'utf8')).toBe('999999');
expect(w.lock()).toBeNull();
expect(readFileSync(w.lockFile, 'utf8')).toBe('4242');
});
test('restarted × required-check-failed and restarted × merged: the verdict line is printed again by the new process', () => {
const first = watcher({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'fail'], ['windows-free-tests', 'pass'])) }, 'green');
expect(first.poll()).toBe(PR_WAIT_EXIT.blocked);
stdout = [];
const again = watcher({ rulesets: RULESETS, checks: ok(checksJson(['free-tests', 'fail'], ['windows-free-tests', 'pass'])) }, 'green');
expect(again.baseline).toBe(false);
expect(again.poll()).toBe(PR_WAIT_EXIT.blocked);
expect(lines()[0]).toBe('#42 required check failed: free-tests https://ci.example/free-tests');
stdout = [];
const merged = watcher({ pr: prJson({ state: 'MERGED' }) }, 'green');
expect(merged.baseline).toBe(false);
expect(merged.poll()).toBe(PR_WAIT_EXIT.met);
expect(lines()).toEqual(['#42 merged: https://github.com/owner/project/pull/42']);
});
});