fix(ci): match the version PR by base too, and keep the write monotonic

Three findings from the claude review on #133.

**The pull request was selected on its head ref alone.** GitHub allows several
open pull requests from one head branch to different bases, so
`head=<owner>:changeset-release/main` can return more than one and `.find()`
would annotate whichever the API listed first. Both refs are now matched in
the script, and the query carries `base=main` as well — the script re-checks
so that widening the query cannot silently widen what gets written.

**An older run could roll the region back.** `needs:` orders jobs inside one
run, and this workflow has no concurrency group on purpose, so two pushes in
quick succession can leave the older run's breadcrumb job writing last. The
write is now monotonic: the version already on the pull request is read back
out of the install line and compared on the 14-digit UTC stamp, which is
fixed-width precisely so a lexical compare orders builds. A concurrency group
would have been the wrong instrument — it would cancel in-progress publishes
to fix a cosmetic race.

**"At the end" was claimed unconditionally, and the stack breadcrumb can
break it.** `stack-breadcrumb.cjs`'s `canonicalizeBody` re-lays a stacked
body as breadcrumb → description → carried regions, and its `ownDescription`
strips only the regions it owns — so this one rides inside "description" and
lands above the carried blocks. Nothing here runs at that moment. The spec now
asserts placement of the write and requires the next publish to return the
region to the end, with a scenario for each; the script says where the real
fix belongs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jwc9FFroR3mTZ4hLiSkkX3

Author: theCodeDrift

.github/scripts/nightly-breadcrumb.cjs

@@ -39,6 +39,19 @@
  * on different names and neither pattern can match the other's markers; this
  * one never rewrites a region it does not own.
  *
+ * THE CONVERSE DOES NOT HOLD, AND CANNOT BE FIXED FROM HERE. If the Version
+ * Packages pull request is ever part of a stack that carries `<!-- PR:N -->`
+ * regions, stack-breadcrumb.cjs's `canonicalizeBody` re-lays the whole body as
+ * breadcrumb → description → carried regions. Its `ownDescription` strips only
+ * the regions IT owns, so this one travels inside "description" and lands
+ * ABOVE the carried blocks — no longer at the end. Nothing here runs at that
+ * moment, so "at the end" is a property of each write rather than of the body
+ * for all time (the spec says so in those words). The next publish moves the
+ * region back, which is the same self-healing the lost-update window relies on
+ * — see the workflow header. Teaching the other canonicalizer about this
+ * region would be the real fix, and it belongs in that file, on a change that
+ * can test it there.
+ *
  * There is NO GitHub I/O here. The workflow fetches the pull request list with
  * `gh api` and applies the body with `gh api -X PATCH` (never `gh pr edit` —
  * its GraphQL path is broken by GitHub's Projects-classic deprecation), so this
@@ -70,9 +83,22 @@ const NIGHTLY_PACKAGE = "@taskless/cli-nightly";
 /** The head branch changesets opens its "Version Packages" pull request from. */
 const VERSION_PR_BRANCH = "changeset-release/main";
 
+/** The base branch that pull request targets — matched, not assumed. */
+const DEFAULT_BRANCH = "main";
+
 const REGION_OPEN = "<!-- nightly -->";
 const REGION_CLOSE = "<!-- /nightly -->";
 
+/**
+ * The install line inside a region, which is where the version a previous
+ * publish wrote can be read back from. Built from `NIGHTLY_PACKAGE` rather than
+ * spelled out, so renaming the package cannot leave the reader looking for a
+ * name the renderer no longer writes.
+ */
+const INSTALL_LINE_PATTERN = new RegExp(
+  `\`npx ${NIGHTLY_PACKAGE.replaceAll(/[$()*+.?[\\\]^{|}]/g, String.raw`\$&`)}@([^\`\\s]+)\``
+);
+
 /**
  * The whole region, opening marker through closing marker. Non-greedy, so two
  * regions in a hand-mangled body are removed one at a time rather than
@@ -127,7 +153,9 @@ function parseStampedVersion(version) {
   if (Number.isNaN(parsed.getTime())) {
     throw new Error(`stamped version carries an impossible time: ${version}`);
   }
-  return { builtAt, shortSha: sha };
+  // `stamp` is the raw 14 digits: fixed-width and UTC, so a lexical compare of
+  // two of them orders the builds (see isNewerBuild).
+  return { builtAt, shortSha: sha, stamp };
 }
 
 /** Render the region for `version`, markers included, with no trailing newline. */
@@ -197,11 +225,23 @@ function upsertRegion(body, version) {
  * returned, or `undefined` when there is none.
  *
  * `undefined` is a NORMAL answer, not an error: the branch exists only while
- * changesets are pending. The head ref is still matched here rather than
- * trusted from the query string, so a widened or mistyped filter cannot lead to
- * this writing its region onto an unrelated pull request.
+ * changesets are pending.
+ *
+ * BOTH REFS ARE MATCHED HERE, not just the head, and neither is trusted from
+ * the query string. GitHub allows several open pull requests from ONE head
+ * branch to different bases, so `head=<owner>:changeset-release/main` alone can
+ * return more than one — a second PR opened from that branch for testing, say —
+ * and a `.find()` on the head ref would then write this region onto whichever
+ * the API happened to list first. The one this workflow means is the one
+ * changesets opens, which targets the default branch. The workflow's query
+ * filters on both as well; this is the check that holds if that query is ever
+ * widened or mistyped.
  */
-function selectVersionPullRequest(pulls, branch = VERSION_PR_BRANCH) {
+function selectVersionPullRequest(
+  pulls,
+  branch = VERSION_PR_BRANCH,
+  base = DEFAULT_BRANCH
+) {
   if (!Array.isArray(pulls)) {
     throw new Error(
       "the pulls response is not an array — expected the body of GET /repos/{owner}/{repo}/pulls"
@@ -212,10 +252,59 @@ function selectVersionPullRequest(pulls, branch = VERSION_PR_BRANCH) {
       pull &&
       pull.head &&
       pull.head.ref === branch &&
+      pull.base &&
+      pull.base.ref === base &&
       (pull.state === undefined || pull.state === "open")
   );
 }
 
+/**
+ * The version named by the region already on `body`, or `undefined` when there
+ * is none (or when the region has been edited past recognition).
+ */
+function readRegionVersion(body) {
+  const region = REGION_PATTERN.exec(String(body ?? ""));
+  if (!region) {
+    return undefined;
+  }
+  const match = INSTALL_LINE_PATTERN.exec(region[0]);
+  return match ? match[1] : undefined;
+}
+
+/**
+ * Is `candidate` a LATER build than `existing`? Used to keep this write
+ * monotonic.
+ *
+ * ORDERING ACROSS RUNS IS NOT GUARANTEED BY `needs:`. That only orders jobs
+ * within one run, and this workflow deliberately has no concurrency group (see
+ * the file header — gate 2 is per-SHA, so two runs cannot both publish). Two
+ * pushes to `main` in quick succession therefore start two independent runs,
+ * and nothing stops the OLDER run's breadcrumb job from reaching the PATCH
+ * after the newer one did. Without this check that older job would leave the
+ * pull request advertising a build that has already been superseded — an
+ * install line pointing at yesterday's nightly, with every run green.
+ *
+ * A concurrency group would be the wrong instrument: it would also cancel
+ * in-progress PUBLISHES, trading a cosmetic staleness for a lost package. The
+ * comparison is on the 14-digit UTC stamp, which is fixed-width, so a lexical
+ * compare orders builds chronologically for any base version (design D3 chose
+ * that layout for exactly this).
+ */
+function isNewerBuild(candidate, existing) {
+  if (existing === undefined) {
+    return true;
+  }
+  let existingStamp;
+  try {
+    existingStamp = parseStampedVersion(existing).stamp;
+  } catch {
+    // The region was edited into something unrecognizable. Treat it as absent
+    // and rewrite it: a body carrying a broken region should converge.
+    return true;
+  }
+  return parseStampedVersion(candidate).stamp > existingStamp;
+}
+
 function requireValue(argv, index, flag) {
   const value = argv[index];
   if (value === undefined || value.length === 0 || value.startsWith("--")) {
@@ -271,8 +360,22 @@ function main() {
     return;
   }
 
-  const body = upsertRegion(pull.body ?? "", options.version);
   setOutput("pull_number", pull.number);
+
+  // Monotonic: an older run that reaches this point after a newer one wrote
+  // must not roll the pull request back to the build it published. See
+  // isNewerBuild — job ordering across runs is not guaranteed and a concurrency
+  // group would cancel publishes to fix a cosmetic race.
+  const present = readRegionVersion(pull.body ?? "");
+  if (!isNewerBuild(options.version, present)) {
+    console.log(
+      `#${pull.number} already names ${present}, which is not older than ${options.version} — leaving it alone.`
+    );
+    setOutput("changed", "false");
+    return;
+  }
+
+  const body = upsertRegion(pull.body ?? "", options.version);
   if (body === (pull.body ?? "")) {
     console.log(`#${pull.number} already carries this build's region.`);
     setOutput("changed", "false");
@@ -285,12 +388,15 @@ function main() {
 }
 
 module.exports = {
+  DEFAULT_BRANCH,
   NIGHTLY_PACKAGE,
   REGION_CLOSE,
   REGION_OPEN,
   VERSION_PR_BRANCH,
   hasRegion,
+  isNewerBuild,
   parseArguments,
+  readRegionVersion,
   parseStampedVersion,
   renderRegion,
   selectVersionPullRequest,

.github/scripts/nightly-breadcrumb.test.cjs

@@ -8,7 +8,9 @@ const {
   NIGHTLY_PACKAGE,
   VERSION_PR_BRANCH,
   hasRegion,
+  isNewerBuild,
   parseArguments,
+  readRegionVersion,
   parseStampedVersion,
   renderRegion,
   selectVersionPullRequest,
@@ -36,6 +38,8 @@ test("parseStampedVersion reads the build time and sha back out of the version",
   assert.deepEqual(parseStampedVersion(VERSION), {
     builtAt: "2026-08-18 12:34:56",
     shortSha: "05b3c88",
+    // The raw stamp is kept so builds can be ordered without re-parsing.
+    stamp: "20260818123456",
   });
 });
 
@@ -188,14 +192,61 @@ test("stripRegion returns a region-free body byte-for-byte", () => {
   assert.equal(stripRegion(body), body.trimEnd());
 });
 
+/** The shape the pulls endpoint returns, trimmed to what the selector reads. */
+const pull = (number, ref, base = "main") => ({
+  number,
+  state: "open",
+  head: { ref },
+  base: { ref: base },
+  body: "",
+});
+
 test("selectVersionPullRequest finds the open Version Packages pull request", () => {
-  const pulls = [
-    { number: 12, state: "open", head: { ref: "feat/other" }, body: "" },
-    { number: 74, state: "open", head: { ref: VERSION_PR_BRANCH }, body: "hi" },
-  ];
+  const pulls = [pull(12, "feat/other"), pull(74, VERSION_PR_BRANCH)];
   assert.equal(selectVersionPullRequest(pulls).number, 74);
 });
 
+// claude[bot] review on #133: GitHub allows several open PRs from one head
+// branch to different bases, so matching the head alone can annotate the wrong
+// one — whichever the API happened to list first.
+test("a same-head pull request targeting another base is not selected", () => {
+  const decoy = pull(90, VERSION_PR_BRANCH, "some/test-base");
+  const real = pull(74, VERSION_PR_BRANCH);
+
+  // Listed FIRST, so a head-only `.find()` would return it.
+  assert.equal(selectVersionPullRequest([decoy, real]).number, 74);
+  assert.equal(selectVersionPullRequest([decoy]), undefined);
+});
+
+test("a pull request with no base is not selected", () => {
+  assert.equal(
+    selectVersionPullRequest([
+      { number: 74, state: "open", head: { ref: VERSION_PR_BRANCH } },
+    ]),
+    undefined
+  );
+});
+
+test("readRegionVersion reads back the version a publish wrote", () => {
+  assert.equal(readRegionVersion(upsertRegion(DESCRIPTION, VERSION)), VERSION);
+  assert.equal(readRegionVersion(DESCRIPTION), undefined);
+  assert.equal(readRegionVersion(""), undefined);
+});
+
+// The cross-run race: `needs:` orders jobs inside ONE run, and this workflow
+// has no concurrency group on purpose, so an older run's breadcrumb job can
+// reach the write after a newer one did.
+test("isNewerBuild keeps the write monotonic", () => {
+  assert.equal(isNewerBuild(NEXT_VERSION, VERSION), true);
+  assert.equal(isNewerBuild(VERSION, NEXT_VERSION), false);
+  assert.equal(isNewerBuild(VERSION, VERSION), false);
+  assert.equal(isNewerBuild(VERSION, undefined), true);
+  // A base-version bump does not reorder builds: the stamp decides.
+  assert.equal(isNewerBuild("0.12.0-20260817000000xaaaaaaa", VERSION), false);
+  // A region edited past recognition is treated as absent and rewritten.
+  assert.equal(isNewerBuild(VERSION, "hand-edited"), true);
+});
+
 // The whole point of the "no open pull request" branch: it is a normal state,
 // and a cosmetic breadcrumb must never fail a run that already published.
 test("no Version Packages pull request is undefined, not an error", () => {

.github/workflows/release-cli-nightly.yml

@@ -444,7 +444,14 @@ jobs:
           # A plain query, allowed to fail. No `|| echo '[]'`: a fallback here
           # would turn an API error into "no pull request found" and the run
           # would go green having silently skipped the update.
-          gh api "repos/${REPO}/pulls?state=open&head=${OWNER}:changeset-release/main" > nightly-pulls.json
+          #
+          # `base=main` is not redundant with `head=`. GitHub allows several
+          # open pull requests from ONE head branch to different bases, so the
+          # head filter alone can return more than one and the script would
+          # annotate whichever was listed first. The script re-checks both refs
+          # for the same reason, so widening this query cannot silently widen
+          # what gets written.
+          gh api "repos/${REPO}/pulls?state=open&head=${OWNER}:changeset-release/main&base=main" > nightly-pulls.json
 
           # Writes nightly-body.md only when the body actually changes, and
           # exits 0 with changed=false when there is no open Version Packages

openspec/specs/cli-nightly-builds/spec.md

@@ -163,7 +163,9 @@ When a nightly is published, the open pull request that carries the pending rele
 
 Every fact in that region SHALL be derived from the version the publish stamped, not determined independently. The version already encodes the build time and the commit, and a second determination reads a second clock.
 
-The region SHALL be placed at the end of the pull request body, SHALL replace any region a previous publish left rather than adding to it, and SHALL be restored if a human deletes it. It SHALL NOT modify any other managed region on that body.
+Each publish SHALL append the region at the end of the body it writes, SHALL replace any region a previous publish left rather than adding to it, and SHALL be restored if a human deletes it. It SHALL NOT modify any other managed region on that body.
+
+Placement is asserted of the write, not of the body for all time: other writers maintain their own regions on the same body and may re-lay it, moving this region out of last place. A publish SHALL return the region to the end rather than leave it where it was found. A publish SHALL NOT overwrite a region naming a build newer than its own.
 
 The annotation SHALL depend on the publish having succeeded, and SHALL be performed by a job that holds permission to write pull requests and holds no publishing credential — the ability to publish under the organization's scope and the ability to rewrite pull request text SHALL NOT be held by one job.
 
@@ -177,6 +179,16 @@ The annotation SHALL depend on the publish having succeeded, and SHALL be perfor
 - **WHEN** a second nightly is published while the same pull request is open
 - **THEN** the pull request SHALL carry exactly one build-info region, describing the most recent publish
 
+#### Scenario: Another writer moves the region
+
+- **WHEN** another writer re-lays the body and the region no longer sits at the end
+- **THEN** the next publish SHALL move it back to the end rather than leave it in place or write a second one
+
+#### Scenario: An older build does not overwrite a newer one
+
+- **WHEN** the region on the pull request names a build newer than the one being announced
+- **THEN** the body SHALL be left unchanged
+
 #### Scenario: No open release pull request is not a failure
 
 - **WHEN** a nightly is published and no pull request carrying pending release metadata is open