From b321deb4c744516fd833016faa3d47a4d2cf3d3b Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 06:52:20 +0000 Subject: [PATCH] fix(ship-queue): refuse a dash-led branch in the hint, soften the green hint, restore the CHANGELOG tests bullet Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 3 ++- docs/ship-policy.md | 2 +- lib/ship-queue.ts | 4 ++-- test/gstack-ship-queue.test.ts | 8 +++++--- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2a6d75a..23ec354e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,11 @@ ### Fixed - **`ready`'s `ci:` line** (`lib/ship-queue.ts`, new `latestRunPerName`) judges each check (one app's check name) by its latest run (newest `completed_at`, then highest id; a run still in progress is newest), so a cancelled or re-run earlier attempt no longer counts as a failure; two apps reporting the same name stay separate, a run without a name or without a readable `completed_at` counts on its own, and commit-status contexts are counted one each, as before. -- **`ready` prints a `ci-hint:` line on a conflicting PR** (`gh pr view` now also reads `mergeable` and `headRefName`): with no CI at the head it names the procedure (dispatch `free-tests.yml` and `windows-free-tests.yml` with `gh workflow run … --ref `, clear the pregate lanes with the run URLs, rerun `ready`); with green runs at the head it says the dispatched runs count; a head on a fork gets a hint that leaves the dispatch to the coordinator; a branch name outside `[\w./-]` prints as ``. Every hint ends in `(advisory)`; nothing parses it. No hint without a conflict, or while the runs are red or pending. +- **`ready` prints a `ci-hint:` line on a conflicting PR** (`gh pr view` now also reads `mergeable` and `headRefName`): with no CI at the head it names the procedure (dispatch `free-tests.yml` and `windows-free-tests.yml` with `gh workflow run … --ref `, clear the pregate lanes with the run URLs, rerun `ready`); with green runs at the head it says the dispatched runs count; a head on a fork gets a hint that leaves the dispatch to the coordinator; a branch name outside `[\w./-]`, or starting with `-`, prints as ``. Every hint ends in `(advisory)`; nothing parses it. No hint without a conflict, or while the runs are red or pending. - **Docs:** `docs/ship-policy.md` records that procedure under "Lane: report ready" (and that `--screen` is `QUEUE_SCREEN_STALE_REF` on such a PR), rewords the `stamp-at-merge` row's `next:` description to the train wording `gstack-restamp` prints since #3439 (`commit and push only the stamp diff above (FILES), with no edit of your own`), and rewords the restamp `changelog` stage so `CHANGELOG_DUPLICATE_HEADING` is described as #3439 implements it: only a duplicate the PR introduces counts, measured against the base's CHANGELOG. ### Tests +- `test/gstack-ship-queue.test.ts`: two runs of one check name where the older is cancelled read green (either order), a newer failure reads red, a newer in-progress or queued run reads pending, equal `completed_at` falls back to the id, unnamed and undated runs stay separate, two apps with one name stay separate, a null conclusion reads red, and a commit status never collapses with a check run of the same name; a conflicting PR with no CI refuses with the dispatch hint, with green dispatched runs READYs with the "counts" hint, a fork head and an odd or dash-led branch name get the safe hint, and no hint appears without a conflict or while runs are red or pending. **An unstamped PR can change a release output again (GSTA-96 on #3434).** Regenerating `agents-digest/gstack-AGENTS.md` after a generator change made the Version Gate classify the head as stamped (the digest is in `release_outputs`) and demand base + bump, while changing only `scripts/gen-agents-digest.ts` failed `test/agents-digest.test.ts`'s freshness check, so a digest rewording could not land before its slot. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 1642eafcc..87a788589 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -191,7 +191,7 @@ dispatches the free workflows on the branch (`gh workflow run free-tests.yml clears the requires-remote pregate lanes with those runs (`gstack-pregate clear --remote --run-url `) and reruns `ready`, which passes because the dispatched check runs attach to the commit; the -`ci-hint:` line then says the dispatched runs count. The `ci-hint:` line is +`ci-hint:` line then says those non-pull_request runs count. The `ci-hint:` line is advisory text for the lane; nothing parses it, and a PR whose head is on a fork gets a hint that leaves the dispatch to the coordinator. `ready --screen` on such a PR is `QUEUE_SCREEN_STALE_REF` (there is no merge ref to screen), so it READYs diff --git a/lib/ship-queue.ts b/lib/ship-queue.ts index f9e47eabe..7a61ecb4d 100644 --- a/lib/ship-queue.ts +++ b/lib/ship-queue.ts @@ -130,10 +130,10 @@ function ciAtHead(ctx: QueueContext, sha: string): { line: string; green: boolea */ function conflictHint(mergeable: unknown, ci: { green: boolean; none: boolean }, branch: string | undefined, fork: boolean, head: string): string | null { if (mergeable !== 'CONFLICTING') return null; - const ref = branch && /^[\w./-]+$/.test(branch) ? `--ref ${branch}` : '--ref '; + const ref = branch && /^[\w.][\w./-]*$/.test(branch) ? `--ref ${branch}` : '--ref '; if (ci.none && fork) return `ci-hint: PR conflicts with the base (no merge ref, so pull_request CI did not run on ${head.slice(0, 12)}) and its head is on a fork, so the free workflows cannot be dispatched here; the coordinator decides (advisory)`; if (ci.none) return `ci-hint: PR conflicts with the base (no merge ref, so pull_request CI did not run on ${head.slice(0, 12)}); dispatch the free workflows on the branch (gh workflow run free-tests.yml ${ref} && gh workflow run windows-free-tests.yml ${ref}), clear the pregate lanes with their run URLs (gstack-pregate clear --remote --run-url ), then rerun ready (advisory)`; - if (ci.green) return `ci-hint: PR conflicts with the base (no merge ref); the runs at ${head.slice(0, 12)} were dispatched on the branch, which counts (advisory)`; + if (ci.green) return `ci-hint: PR conflicts with the base (no merge ref); the runs at ${head.slice(0, 12)} were not pull_request runs (dispatched on the branch, or push-triggered), and they count (advisory)`; return null; } diff --git a/test/gstack-ship-queue.test.ts b/test/gstack-ship-queue.test.ts index eb48827d7..fc622cb11 100644 --- a/test/gstack-ship-queue.test.ts +++ b/test/gstack-ship-queue.test.ts @@ -427,8 +427,10 @@ describe('ready on a PR that conflicts with the base (CHANGELOG-only conflicts, expect(r.status).toBe(EXIT.refused); expect(line(r.out, 'ci: ')).toBe(`ci: none at ${at} (passed=0 failed=0 pending=0 skipped=0)`); expect(line(r.out, 'ci-hint: ')).toBe(`ci-hint: PR conflicts with the base (no merge ref, so pull_request CI did not run on ${at}); dispatch the free workflows on the branch (gh workflow run free-tests.yml --ref pr-a && gh workflow run windows-free-tests.yml --ref pr-a), clear the pregate lanes with their run URLs (gstack-pregate clear --remote --run-url ), then rerun ready (advisory)`); - const odd = readyWith('CONFLICTING', { check_runs: [] }, { headRefName: 'x;$(curl evil)' }); - expect(line(odd.r.out, 'ci-hint: ')).toContain('gh workflow run free-tests.yml --ref && gh workflow run windows-free-tests.yml --ref '); + for (const name of ['x;$(curl evil)', '--field']) { + const odd = readyWith('CONFLICTING', { check_runs: [] }, { headRefName: name }); + expect(line(odd.r.out, 'ci-hint: ')).toContain('gh workflow run free-tests.yml --ref && gh workflow run windows-free-tests.yml --ref '); + } const fork = readyWith('CONFLICTING', { check_runs: [] }, { isCrossRepository: true }); expect(line(fork.r.out, 'ci-hint: ')).toBe(`ci-hint: PR conflicts with the base (no merge ref, so pull_request CI did not run on ${fork.at}) and its head is on a fork, so the free workflows cannot be dispatched here; the coordinator decides (advisory)`); }, 240_000); @@ -436,7 +438,7 @@ describe('ready on a PR that conflicts with the base (CHANGELOG-only conflicts, test('dispatched runs green at the head and a conflict: READY, with the hint saying the dispatched runs count', () => { const { r, at } = readyWith('CONFLICTING', { check_runs: [pass] }); expect(r.status).toBe(EXIT.ok); - expect(line(r.out, 'ci-hint: ')).toBe(`ci-hint: PR conflicts with the base (no merge ref); the runs at ${at} were dispatched on the branch, which counts (advisory)`); + expect(line(r.out, 'ci-hint: ')).toBe(`ci-hint: PR conflicts with the base (no merge ref); the runs at ${at} were not pull_request runs (dispatched on the branch, or push-triggered), and they count (advisory)`); }, 120_000); test('no hint without a conflict, and none while the dispatched runs are red or pending', () => {