From 4b01513702dc9955efa9621c66f9229f47f6f75c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 19:27:44 -0700 Subject: [PATCH 1/6] fix(ship): restore always-loaded PR-title-version invariant to skeleton The v1.54.0.0 carve moved the 'PR title MUST start with v$NEW_VERSION' rule out of the always-loaded ship skeleton and entirely into the lazily-loaded pr-body.md section. The agent only set the version prefix if it happened to read that section before creating the PR, so PRs landed with bare titles. Restore a one-line invariant (+ helper reference) to ship/SKILL.md.tmpl right before the {{SECTION:pr-body}} pointer, mirroring the AUQ always-loaded precedent. Full procedure stays sectioned. Regenerated all hosts. Co-Authored-By: Claude Opus 4.8 (1M context) --- ship/SKILL.md | 2 ++ ship/SKILL.md.tmpl | 2 ++ 2 files changed, 4 insertions(+) diff --git a/ship/SKILL.md b/ship/SKILL.md index 755f5ab98a..71c4264366 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -1202,6 +1202,8 @@ git push -u origin --- +**PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `~/.claude/skills/gstack/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. + > **STOP.** Before syncing docs and creating or updating the PR/MR (Steps 18-19), Read `~/.claude/skills/gstack/ship/sections/pr-body.md` and execute it > in full. Do not work from memory — that section is the source of truth for this step. diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index cd6875d94f..d461d6b84f 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -395,6 +395,8 @@ git push -u origin --- +**PR/MR title invariant (always applies — do not skip even if you don't open the section below):** Any PR or MR you create OR update in the next step MUST have a title that starts with `v$NEW_VERSION` (the version bumped in Step 12), in the format `v : `. Never create or edit a PR/MR title without this prefix. Compute the correct title with the single source of truth helper: `~/.claude/skills/gstack/bin/gstack-pr-title-rewrite.sh "$NEW_VERSION" ""`. The full create/update procedure (idempotency, redaction scan, self-check) is in the section below. + {{SECTION:pr-body}} ## Step 20: Persist ship metrics From 86bb93758ae2e352419e11d712e0007a944e1384 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 19:27:50 -0700 Subject: [PATCH 2/6] test(ship): guard PR-title-version rule + pull_request_target safety Two free gate tests so a future carve or workflow refactor can't silently regress: - ship-pr-title-version-always-loaded: asserts the invariant lives in the always-loaded ship/SKILL.md skeleton (not only sections/), and that the skeleton+sections union keeps BOTH the create and the existing-PR update title paths. Modeled on test/auq-format-always-loaded.test.ts. - pr-title-sync-workflow-safety: static tripwire that fails CI if pr-title-sync.yml checks out PR-head code or inlines an attacker-controlled ${{ github.event.pull_request.* }} field inside a run: block (the two pull_request_target footguns actionlint cannot catch). Co-Authored-By: Claude Opus 4.8 (1M context) --- test/pr-title-sync-workflow-safety.test.ts | 135 ++++++++++++++++++ ...hip-pr-title-version-always-loaded.test.ts | 92 ++++++++++++ 2 files changed, 227 insertions(+) create mode 100644 test/pr-title-sync-workflow-safety.test.ts create mode 100644 test/ship-pr-title-version-always-loaded.test.ts diff --git a/test/pr-title-sync-workflow-safety.test.ts b/test/pr-title-sync-workflow-safety.test.ts new file mode 100644 index 0000000000..dfbbea4e4f --- /dev/null +++ b/test/pr-title-sync-workflow-safety.test.ts @@ -0,0 +1,135 @@ +/** + * pr-title-sync.yml is a `pull_request_target` workflow — static injection + * tripwire (gate, free). + * + * The anxiety this kills: `pull_request_target` runs with a WRITE token in the + * base-repo context, even for fork PRs. That is what lets this workflow rewrite + * fork-PR titles (the backstop). It is also the single most dangerous workflow + * trigger in GitHub Actions. Two classic footguns turn it into remote code + * execution / token theft, and `actionlint` catches NEITHER: + * + * 1. Checking out the PR head (`actions/checkout` with a `ref:` pointing at + * `pull_request.head` / `head_ref`) and then running anything from it — + * that executes attacker-controlled fork code with the write token. + * 2. Interpolating an attacker-controlled `${{ github.event.pull_request.* }}` + * field directly INSIDE a `run:` block — the title/body are attacker- + * controlled and the `${{ }}` is expanded into the shell before execution, + * so a crafted title runs as code. Those fields MUST arrive via `env:` and + * be referenced as `"$VAR"` (shell-quoted), never inlined. + * + * This tripwire reads the workflow file directly and fails CI if either pattern + * reappears. Mirrors the static-grep invariant tests in browse/test + * (terminal-agent-pid-identity, server-sanitize-surrogates). + * + * Note: `gh api ... -q '.head.sha'` inside a run block is SAFE (reading PR + * metadata as data via a jq filter string, not `${{ }}` interpolation), so we + * ban the interpolation form specifically, not the literal substring `head.sha`. + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const WORKFLOW = path.resolve(__dirname, '..', '.github', 'workflows', 'pr-title-sync.yml'); + +/** Indentation width (count of leading spaces) of a line. */ +function indent(line: string): number { + const m = line.match(/^( *)/); + return m ? m[1].length : 0; +} + +/** + * Return the lines that live inside a `run:` block, each tagged with its 1-based + * line number. Handles both `run: |` (multiline) and `run: `. + */ +function runBlockLines(content: string): Array<{ n: number; text: string }> { + const lines = content.split('\n'); + const out: Array<{ n: number; text: string }> = []; + let inRun = false; + let runIndent = -1; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const n = i + 1; + const inlineRun = line.match(/^(\s*)run:\s*(\S.*)$/); // `run: echo foo` + const blockRun = /^(\s*)run:\s*(\|>?[+-]?)?\s*$/.test(line); // `run: |` + if (inlineRun && !/^\|/.test(inlineRun[2])) { + out.push({ n, text: inlineRun[2] }); + inRun = false; + continue; + } + if (blockRun) { + inRun = true; + runIndent = indent(line); + continue; + } + if (inRun) { + if (line.trim() === '') { + out.push({ n, text: line }); + continue; + } + // Block ends when a non-empty line is indented at or below the `run:` key. + if (indent(line) <= runIndent) { + inRun = false; + } else { + out.push({ n, text: line }); + } + } + } + return out; +} + +describe('pr-title-sync.yml pull_request_target safety', () => { + const content = fs.readFileSync(WORKFLOW, 'utf-8'); + + test('workflow file exists', () => { + expect(fs.existsSync(WORKFLOW)).toBe(true); + }); + + test('does NOT check out the PR head ref (no fork-code execution)', () => { + const offenders: string[] = []; + content.split('\n').forEach((line, i) => { + // A checkout `ref:` (or any `ref:`) pointing at the PR head is the footgun. + if (/ref:\s*\$\{\{[^}]*(pull_request\.head|head_ref)/.test(line)) { + offenders.push(` L${i + 1}: ${line.trim()}`); + } + }); + if (offenders.length > 0) { + throw new Error( + `pr-title-sync.yml checks out the PR head under pull_request_target — that ` + + `runs attacker-controlled fork code with a write token. Check out the base ` + + `repo (no ref:) and read PR-head data via the API instead.\n` + + offenders.join('\n'), + ); + } + }); + + test('does NOT interpolate ${{ github.event.pull_request.* }} inside a run: block', () => { + const offenders: string[] = []; + for (const { n, text } of runBlockLines(content)) { + if (/\$\{\{\s*github\.event\.pull_request/.test(text)) { + offenders.push(` L${n}: ${text.trim()}`); + } + } + if (offenders.length > 0) { + throw new Error( + `pr-title-sync.yml inlines an attacker-controlled PR field into a run: block ` + + `— a crafted PR title/body executes as shell. Pass it via env: and ` + + `reference "$VAR" (shell-quoted) instead.\n` + + offenders.join('\n'), + ); + } + }); + + test('uses pull_request_target (the hardening is actually present)', () => { + // Positive assertion: if someone reverts to plain pull_request, the fork + // backstop silently stops working (read-only token). Keep it intentional. + expect(/^on:\s*$/m.test(content) || /\bpull_request_target\b/.test(content)).toBe(true); + expect(content).toMatch(/\bpull_request_target\b/); + }); + + test('passes the PR title through env:, not raw interpolation', () => { + // The safe pattern: OLD_TITLE: ${{ github.event.pull_request.title }} in an + // env: mapping, consumed as "$OLD_TITLE" in script. + expect(content).toMatch(/env:/); + expect(content).toMatch(/github\.event\.pull_request\.title/); + }); +}); diff --git a/test/ship-pr-title-version-always-loaded.test.ts b/test/ship-pr-title-version-always-loaded.test.ts new file mode 100644 index 0000000000..fe5bcea5e6 --- /dev/null +++ b/test/ship-pr-title-version-always-loaded.test.ts @@ -0,0 +1,92 @@ +/** + * /ship PR-title-version rule is ALWAYS-LOADED — token-reduction safety net (gate, free). + * + * The anxiety this kills: the v1.54.0.0 carve ("carve /ship into skeleton + + * on-demand sections, -59% always-loaded") moved the rule "PR title MUST start + * with v$NEW_VERSION" out of the always-loaded monolith and entirely into the + * lazily-loaded `ship/sections/pr-body.md`. The agent then sets the version + * prefix only if it happens to read that section before creating the PR; when it + * doesn't, PRs land with bare titles. This is the exact regression that shipped + * — every recent open PR lacked a `v...` prefix until this guard + the skeleton + * invariant restored it. + * + * This is the title-rule analogue of `test/auq-format-always-loaded.test.ts`, + * which guards the AskUserQuestion format the same way. A carve that re-buries + * the title rule fails here in milliseconds instead of surfacing weeks later as + * a wave of version-less PR titles. + * + * The guarantee, made mechanical and per-PR: + * 1. SKELETON PRESENCE — `ship/SKILL.md` (the always-loaded skeleton) carries + * the invariant: the `v$NEW_VERSION` token, the single-source-of-truth + * helper name, and a directive near a "PR title" mention. Present the + * instant /ship reaches the push/PR steps, no section read required. + * 2. UNION SURVIVAL (both paths) — the FULL procedure still exists somewhere + * in skeleton+sections for BOTH the create path (`gh pr create --title + * "v$NEW_VERSION ...`) AND the existing-PR update path (the `gh pr edit + * --title` rewrite rule). A drop of either is the failure. + */ +import { describe, test, expect } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const ROOT = path.resolve(__dirname, '..'); +const SHIP_SKILL = path.join(ROOT, 'ship', 'SKILL.md'); +const SHIP_SECTIONS = path.join(ROOT, 'ship', 'sections'); + +function readUnion(): string { + let union = fs.readFileSync(SHIP_SKILL, 'utf-8'); + for (const f of fs.readdirSync(SHIP_SECTIONS)) { + if (f.endsWith('.md') && !f.endsWith('.md.tmpl')) { + union += '\n' + fs.readFileSync(path.join(SHIP_SECTIONS, f), 'utf-8'); + } + } + return union; +} + +describe('/ship PR-title-version rule is always-loaded (token-reduction safety net)', () => { + test('sanity: ship is a carved skill (has sections/*.md)', () => { + // Guards against a path regression that would make the union/skeleton checks + // vacuously pass against a non-carved skill. + expect(fs.existsSync(SHIP_SECTIONS)).toBe(true); + expect(fs.readdirSync(SHIP_SECTIONS).some(f => f.endsWith('.md'))).toBe(true); + }); + + test('skeleton (ship/SKILL.md) carries the PR-title-version invariant', () => { + const skeleton = fs.readFileSync(SHIP_SKILL, 'utf-8'); + // Robust independent markers, NOT one brittle full-sentence regex (so + // rewording the prose doesn't false-fail). All three must be present in the + // always-loaded skeleton. + const markers: Array<{ name: string; re: RegExp }> = [ + { name: 'v$NEW_VERSION token', re: /v\$NEW_VERSION/ }, + { name: 'gstack-pr-title-rewrite helper reference', re: /gstack-pr-title-rewrite/ }, + { name: 'a directive (MUST/always/never) near a PR-title mention', re: /(MUST|always|never)[^\n]{0,80}\btitle\b|\btitle\b[^\n]{0,80}(MUST|always|never)/i }, + ]; + const missing = markers.filter(m => !m.re.test(skeleton)); + if (missing.length > 0) { + throw new Error( + `ship/SKILL.md (the always-loaded skeleton) is missing the PR-title-version ` + + `invariant — a carve likely re-buried it in sections/. Missing:\n` + + missing.map(m => ` - ${m.name} (${m.re.source})`).join('\n'), + ); + } + }); + + test('union (skeleton+sections) keeps BOTH the create and the update title rules', () => { + const union = readUnion(); + const paths: Array<{ name: string; re: RegExp }> = [ + // create path: `gh pr create --title "v$NEW_VERSION ...` + { name: 'PR create path prefixes the version', re: /pr create[^\n]*--title[^\n]*v\$NEW_VERSION/i }, + // update/idempotent path: the existing-PR `gh pr edit --title` rewrite rule + { name: 'PR update path rewrites the title', re: /pr edit[^\n]*--title/i }, + ]; + const missing = paths.filter(p => !p.re.test(union)); + if (missing.length > 0) { + throw new Error( + `ship skeleton+sections dropped a PR-title-version code path. The update ` + + `path is the more important idempotent /ship path — a create-only guard ` + + `would miss its rot. Missing:\n` + + missing.map(p => ` - ${p.name} (${p.re.source})`).join('\n'), + ); + } + }); +}); From d53ee9f9e432cd833b6422c93af6c19ce32ac497 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 19:27:50 -0700 Subject: [PATCH 3/6] fix(ci): pr-title-sync covers fork PRs via hardened pull_request_target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under plain pull_request the GITHUB_TOKEN is read-only on fork PRs, so the title-sync backstop could never edit a fork/agent PR title. Switch to pull_request_target (write token in base context) and make it safe: - Check out the base repo only (no ref:) — execute trusted infra, never fork-head code. - All attacker-controlled PR fields (title, head repo, head sha) pass via env: and are referenced as shell-quoted "$VAR", never inlined into run:. - Read the PR-head VERSION as data (raw media type) from the head repo at the head sha; guard the assignment under set -e. - Same-repo read failure fails loudly; fork miss warns and skips (the backstop stays green without going silently optional). - Never echo the raw fork title (Actions parses ::workflow-command:: from stdout). Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/pr-title-sync.yml | 69 ++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pr-title-sync.yml b/.github/workflows/pr-title-sync.yml index 6f5b3d3e58..4f94d4db94 100644 --- a/.github/workflows/pr-title-sync.yml +++ b/.github/workflows/pr-title-sync.yml @@ -1,7 +1,25 @@ name: PR Title Sync +# WHY pull_request_target (not pull_request): the default GITHUB_TOKEN is +# READ-ONLY on fork PRs under `pull_request`, so the title-sync backstop could +# never `gh pr edit` a fork/agent PR. `pull_request_target` runs in the base-repo +# context with a write token, which fixes fork coverage. +# +# WHY this is SAFE (pull_request_target is the most dangerous trigger): +# - We check out the BASE repo (no `ref:`), so the only code we execute is +# trusted base-repo infra (bin/gstack-pr-title-rewrite.sh). We NEVER check +# out or run PR-head/fork code. +# - Every attacker-controlled PR field (title, head repo, head sha) arrives via +# `env:` and is referenced as a shell-quoted "$VAR". We NEVER inline a +# `${{ github.event.pull_request.* }}` expression inside the run: script +# (that would execute a crafted title as shell). +# - The PR-head VERSION is read as DATA via the API (raw media type), from the +# head repo at the head sha — never by checking out the head. +# test/pr-title-sync-workflow-safety.test.ts is the static tripwire for all of +# the above and fails CI if any of it regresses. + on: - pull_request: + pull_request_target: types: [opened, synchronize, edited] paths: - 'VERSION' @@ -19,25 +37,62 @@ jobs: pull-requests: write if: github.actor != 'github-actions[bot]' steps: - - name: Checkout PR head + # Base repo only — trusted infra (the rewrite helper). No PR-head checkout. + - name: Checkout base repo (trusted) uses: actions/checkout@v4 with: fetch-depth: 1 - ref: ${{ github.event.pull_request.head.sha }} - name: Rewrite PR title to match VERSION env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} PR_NUM: ${{ github.event.pull_request.number }} + # Attacker-controlled on fork PRs — env-only, never inlined into run:. OLD_TITLE: ${{ github.event.pull_request.title }} + BASE_REPO: ${{ github.repository }} + HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} run: | set -euo pipefail chmod +x ./bin/gstack-pr-title-rewrite.sh - VERSION=$(cat VERSION | tr -d '[:space:]') - NEW_TITLE=$(./bin/gstack-pr-title-rewrite.sh "$VERSION" "$OLD_TITLE") + + if [ "$HEAD_REPO" = "$BASE_REPO" ]; then IS_FORK=0; else IS_FORK=1; fi + + # Read the PR-head VERSION as data (raw bytes), from the head repo at + # the head sha. Guard the assignment itself: under `set -e` a bare + # `VERSION=$(...)` would abort the step before any later [ -z ] check. + if ! VERSION=$(gh api -H "Accept: application/vnd.github.raw" \ + "repos/$HEAD_REPO/contents/VERSION?ref=$HEAD_SHA" 2>/dev/null | tr -d '[:space:]'); then + VERSION="" + fi + + if [ -z "$VERSION" ]; then + # Same-repo read failure should never happen — fail loudly so we + # notice. A fork miss (public-contents quirk, private fork) is a + # convenience gap, not a gate — warn and skip so the check stays green. + if [ "$IS_FORK" = "0" ]; then + echo "::error::Could not read VERSION from same-repo PR head ($HEAD_SHA)." + exit 1 + fi + echo "::warning::Could not read VERSION from fork $HEAD_REPO ($HEAD_SHA); skipping title sync." + exit 0 + fi + + # The helper rejects a malformed VERSION (exit 2). Same policy: loud for + # same-repo, soft for forks. Never echo the raw (attacker-controlled) + # title — Actions still parses ::workflow-command:: from stdout. + if ! NEW_TITLE=$(./bin/gstack-pr-title-rewrite.sh "$VERSION" "$OLD_TITLE"); then + if [ "$IS_FORK" = "0" ]; then + echo "::error::Could not compute title for VERSION '$VERSION' on PR #$PR_NUM." + exit 1 + fi + echo "::warning::Could not compute title for fork PR #$PR_NUM; skipping." + exit 0 + fi + if [ "$NEW_TITLE" = "$OLD_TITLE" ]; then - echo "Title already correct; no change." + echo "PR #$PR_NUM title already correct; no change." exit 0 fi - echo "Rewriting: $OLD_TITLE -> $NEW_TITLE" gh pr edit "$PR_NUM" --title "$NEW_TITLE" + echo "PR #$PR_NUM title synced to VERSION." From 672a0dd7eb6bd748a8142ded07f778cf3f377736 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 19:28:22 -0700 Subject: [PATCH 4/6] fix(ship): expand binDir path in pr-body Linked Spec block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ship/sections/pr-body.md.tmpl:98-99 used ${ctx.paths.binDir}, but the gen-skill-docs generator only resolves {{TOKEN}} syntax in .tmpl files — the ${...} JS-template-literal form is substituted only inside .ts resolver files. So the token passed through literally into the generated pr-body.md, leaving the agent with an unexpandable ${ctx.paths.binDir}/gstack-paths command in the Linked Spec auto-detect block. Use the hardcoded helper path, consistent with every other path reference in this section. Co-Authored-By: Claude Opus 4.8 (1M context) --- ship/sections/pr-body.md | 4 ++-- ship/sections/pr-body.md.tmpl | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 32aa9dece4..90888b7a90 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -97,8 +97,8 @@ you missed it.> ## Linked Spec ## Linked Spec Date: Sun, 7 Jun 2026 19:37:35 -0700 Subject: [PATCH 5/6] refactor(test): fold ship PR-title skeleton guard into carve-guard registry main shipped a generalized carve-guard system (PR #1907) that is now the single source of truth for carved-skill skeleton invariants. Register the PR-title rule there instead of a standalone test: ship's mustStayInSkeleton asserts v$NEW_VERSION + the rewrite helper stay always-loaded, and mustMoveToSection asserts both the create and update PR paths stay carved into pr-body.md (present in the union, out of the skeleton). Delete the standalone ship-pr-title-version-always-loaded test it replaces. The CI-workflow safety tripwire stays standalone (not a carve concern). Co-Authored-By: Claude Opus 4.8 (1M context) --- test/helpers/carve-guards.ts | 10 +- ...hip-pr-title-version-always-loaded.test.ts | 92 ------------------- 2 files changed, 8 insertions(+), 94 deletions(-) delete mode 100644 test/ship-pr-title-version-always-loaded.test.ts diff --git a/test/helpers/carve-guards.ts b/test/helpers/carve-guards.ts index a56ef9a078..6919f284f1 100644 --- a/test/helpers/carve-guards.ts +++ b/test/helpers/carve-guards.ts @@ -106,8 +106,14 @@ export const CARVE_GUARDS: Record = { scenario: 'This is a FRESH version-changing ship: the branch has a real code change, VERSION still equals the base version (needs a bump), and CHANGELOG.md needs a new entry. Follow the skill flow for a version-changing ship: run the pre-landing review and prepare the CHANGELOG entry. Produce the ship plan / review report. Do NOT actually commit, push, or open a PR.', staticInvariants: { - mustStayInSkeleton: [], - mustMoveToSection: [], + // The PR-title-version invariant MUST stay always-loaded: the v1.54.0.0 + // carve stranded it in pr-body.md and PRs started landing with bare titles + // (CI backstop: test/pr-title-sync-workflow-safety.test.ts). + mustStayInSkeleton: ['v$NEW_VERSION', 'gstack-pr-title-rewrite'], + // ...while the full create/update procedure stays carved into pr-body.md + // (out of the skeleton, present in the union). Asserts BOTH PR paths + // survive: the create path and the idempotent update path. + mustMoveToSection: ['gh pr create --base', 'gh pr edit --title'], // ship is operational (multi-STOP, not a plan review); no single post-STOP gate. gateAfterStop: undefined, }, diff --git a/test/ship-pr-title-version-always-loaded.test.ts b/test/ship-pr-title-version-always-loaded.test.ts deleted file mode 100644 index fe5bcea5e6..0000000000 --- a/test/ship-pr-title-version-always-loaded.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -/** - * /ship PR-title-version rule is ALWAYS-LOADED — token-reduction safety net (gate, free). - * - * The anxiety this kills: the v1.54.0.0 carve ("carve /ship into skeleton + - * on-demand sections, -59% always-loaded") moved the rule "PR title MUST start - * with v$NEW_VERSION" out of the always-loaded monolith and entirely into the - * lazily-loaded `ship/sections/pr-body.md`. The agent then sets the version - * prefix only if it happens to read that section before creating the PR; when it - * doesn't, PRs land with bare titles. This is the exact regression that shipped - * — every recent open PR lacked a `v...` prefix until this guard + the skeleton - * invariant restored it. - * - * This is the title-rule analogue of `test/auq-format-always-loaded.test.ts`, - * which guards the AskUserQuestion format the same way. A carve that re-buries - * the title rule fails here in milliseconds instead of surfacing weeks later as - * a wave of version-less PR titles. - * - * The guarantee, made mechanical and per-PR: - * 1. SKELETON PRESENCE — `ship/SKILL.md` (the always-loaded skeleton) carries - * the invariant: the `v$NEW_VERSION` token, the single-source-of-truth - * helper name, and a directive near a "PR title" mention. Present the - * instant /ship reaches the push/PR steps, no section read required. - * 2. UNION SURVIVAL (both paths) — the FULL procedure still exists somewhere - * in skeleton+sections for BOTH the create path (`gh pr create --title - * "v$NEW_VERSION ...`) AND the existing-PR update path (the `gh pr edit - * --title` rewrite rule). A drop of either is the failure. - */ -import { describe, test, expect } from 'bun:test'; -import * as fs from 'node:fs'; -import * as path from 'node:path'; - -const ROOT = path.resolve(__dirname, '..'); -const SHIP_SKILL = path.join(ROOT, 'ship', 'SKILL.md'); -const SHIP_SECTIONS = path.join(ROOT, 'ship', 'sections'); - -function readUnion(): string { - let union = fs.readFileSync(SHIP_SKILL, 'utf-8'); - for (const f of fs.readdirSync(SHIP_SECTIONS)) { - if (f.endsWith('.md') && !f.endsWith('.md.tmpl')) { - union += '\n' + fs.readFileSync(path.join(SHIP_SECTIONS, f), 'utf-8'); - } - } - return union; -} - -describe('/ship PR-title-version rule is always-loaded (token-reduction safety net)', () => { - test('sanity: ship is a carved skill (has sections/*.md)', () => { - // Guards against a path regression that would make the union/skeleton checks - // vacuously pass against a non-carved skill. - expect(fs.existsSync(SHIP_SECTIONS)).toBe(true); - expect(fs.readdirSync(SHIP_SECTIONS).some(f => f.endsWith('.md'))).toBe(true); - }); - - test('skeleton (ship/SKILL.md) carries the PR-title-version invariant', () => { - const skeleton = fs.readFileSync(SHIP_SKILL, 'utf-8'); - // Robust independent markers, NOT one brittle full-sentence regex (so - // rewording the prose doesn't false-fail). All three must be present in the - // always-loaded skeleton. - const markers: Array<{ name: string; re: RegExp }> = [ - { name: 'v$NEW_VERSION token', re: /v\$NEW_VERSION/ }, - { name: 'gstack-pr-title-rewrite helper reference', re: /gstack-pr-title-rewrite/ }, - { name: 'a directive (MUST/always/never) near a PR-title mention', re: /(MUST|always|never)[^\n]{0,80}\btitle\b|\btitle\b[^\n]{0,80}(MUST|always|never)/i }, - ]; - const missing = markers.filter(m => !m.re.test(skeleton)); - if (missing.length > 0) { - throw new Error( - `ship/SKILL.md (the always-loaded skeleton) is missing the PR-title-version ` + - `invariant — a carve likely re-buried it in sections/. Missing:\n` + - missing.map(m => ` - ${m.name} (${m.re.source})`).join('\n'), - ); - } - }); - - test('union (skeleton+sections) keeps BOTH the create and the update title rules', () => { - const union = readUnion(); - const paths: Array<{ name: string; re: RegExp }> = [ - // create path: `gh pr create --title "v$NEW_VERSION ...` - { name: 'PR create path prefixes the version', re: /pr create[^\n]*--title[^\n]*v\$NEW_VERSION/i }, - // update/idempotent path: the existing-PR `gh pr edit --title` rewrite rule - { name: 'PR update path rewrites the title', re: /pr edit[^\n]*--title/i }, - ]; - const missing = paths.filter(p => !p.re.test(union)); - if (missing.length > 0) { - throw new Error( - `ship skeleton+sections dropped a PR-title-version code path. The update ` + - `path is the more important idempotent /ship path — a create-only guard ` + - `would miss its rot. Missing:\n` + - missing.map(p => ` - ${p.name} (${p.re.source})`).join('\n'), - ); - } - }); -}); From 4a3609e376d3f3471b4e3fb258cd21e574026793 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 7 Jun 2026 19:37:35 -0700 Subject: [PATCH 6/6] chore: bump version and changelog (v1.57.3.0) Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++ VERSION | 2 +- package.json | 2 +- 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf89641a2..290e794dac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,63 @@ # Changelog +## [1.57.3.0] - 2026-06-07 + +## **Every PR `/ship` opens gets the version stamped into its title, fork and agent PRs included.** +## **The rule rides in the always-loaded part of the skill now, and a guard keeps it there.** + +`/ship` stamps `vX.Y.Z.W` onto the title of every PR or MR it creates or updates, so +the version is the first thing you read in the PR list. That rule now lives in the +always-loaded core of the ship skill instead of an on-demand section, so the agent +applies it whether or not it opened the section that spells out the full procedure. +A CI workflow backs this up: it rewrites a title to match VERSION on every PR that +bumps the version, and it now reaches fork and agent PRs too, which a read-only token +could never touch before. Two free tests lock the behavior in so it cannot drift on +the next refactor. + +### The numbers that matter + +Reproduce with `bun test test/carve-section-ordering.test.ts test/pr-title-sync-workflow-safety.test.ts` +and `bun run eval:select`. + +| Property | Before | After | +|---|---|---| +| Where the title rule loads | on-demand section only (since v1.54.0.0) | always-loaded skeleton + on-demand detail | +| Fork / agent PR title sync | none (read-only token under `pull_request`) | covered via hardened `pull_request_target` | +| Test proving the rule stays put | none | carve-guard registry asserts it on every PR | +| CI injection guard for the title workflow | none | static tripwire fails CI on unsafe patterns | + +The title workflow now runs with a write token in the base-repo context but never +checks out or executes PR-head code, and every attacker-controlled field reaches the +script through `env:`, never inlined. A static test fails CI if either rule regresses. + +### What this means for you + +Ship a branch and the PR shows up titled `v1.57.3.0 fix: ...` without you touching it, +even when the PR came from a fork. The agent no longer needs to read the right section +at the right moment for the version to land in the title, and the next person who slims +the ship skill cannot quietly strand the rule again, because a free test on every PR +checks that it is still there. + +### Itemized changes + +#### Added +- Carve-guard coverage for the ship PR-title invariant: the registry now asserts the + `v$NEW_VERSION` rule and the title helper stay in the always-loaded skeleton, while + the full create and update procedure stays in the on-demand section. +- Static CI-safety test for the title-sync workflow that fails the build if it checks + out PR-head code or inlines an attacker-controlled PR field into a shell step. + +#### Changed +- The PR/MR title-version rule is always-loaded in `/ship` again, so the version + prefix lands on every PR the workflow creates or updates. +- The PR title-sync CI workflow now covers fork and agent PRs through a hardened + `pull_request_target` trigger (base-repo checkout only, PR fields passed via `env:`, + VERSION read as data from the PR head). + +#### Fixed +- A path token in the ship PR-body section that rendered literally instead of resolving + now uses the correct helper path, so the Linked Spec auto-detect step runs as written. + ## [1.57.0.0] - 2026-06-07 ## **Three more heavyweight skills load lighter, and every carved skill finally has a test that proves it loads.** diff --git a/VERSION b/VERSION index a17d4bbc0b..e97e1faf0f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.57.0.0 +1.57.3.0 diff --git a/package.json b/package.json index a9440ba156..7e483ae648 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.57.0.0", + "version": "1.57.3.0", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module",