From 48ab55ae3f805194c216aafd02186cb3a11661f6 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 22:27:08 +0000 Subject: [PATCH] fix: let git place co-author trailers; name the docs key release gstack-coauthor-trailers --message now runs git interpret-trailers --in-place, so the appended lines join the existing trailer block whatever its shape (CRLF, comment lines, folded continuations) and the existing co-author keeps attribution; the interpret-trailers check in the test carries a timeout for the spawnSync tripwire. DOCS_CAPS_KEY_VERSION names the release after the newest CHANGELOG heading on main (1.91.258.0), and a test ties every unreleased policy key to that next release so a stamp elsewhere fails loudly. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-coauthor-trailers | 18 +++++++++--------- lib/ship-policy.ts | 6 ++++-- test/gstack-coauthor-trailers.test.ts | 9 ++++++++- test/gstack-ship-policy.test.ts | 20 +++++++++++++++++++- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/bin/gstack-coauthor-trailers b/bin/gstack-coauthor-trailers index cd61abfcd..74beb65ab 100755 --- a/bin/gstack-coauthor-trailers +++ b/bin/gstack-coauthor-trailers @@ -9,9 +9,10 @@ * `..HEAD`, base `origin/HEAD`'s branch) other than this checkout's own * committer identity and any --exclude, and prints one * `Co-authored-by: Name ` line per author in first-appearance order. - * With --message , the trailers the file lacks are appended after a - * blank line (idempotent; existing trailers are matched by email, case - * folded), so a port's own fix commits keep the original author's credit. + * With --message , the trailers the file lacks join its trailer block + * (after a blank line when it has none; idempotent; existing trailers are + * matched by email, case folded), so a port's own fix commits keep the + * original author's credit. * Exit: 0 printed · 1 the range does not resolve · 2 usage. */ import * as fs from 'node:fs'; @@ -77,12 +78,11 @@ function main(argv: string[]): number { const present = new Set([...text.matchAll(/^co-authored-by:.*<([^>]+)>\s*$/gim)].map(m => m[1]!.toLowerCase())); appended = [...authors.entries()].filter(([key]) => !present.has(key)).map(([, a]) => `Co-authored-by: ${a.name} <${a.email}>`); if (appended.length) { - // Trailers are one paragraph: a blank line before the new ones would make git (and GitHub's - // attribution) read only the last paragraph as the trailer block and drop the existing co-authors. - const body = text.replace(/\s+$/, ''); - const cut = body.lastIndexOf('\n\n'); - const endsInTrailer = cut >= 0 && body.slice(cut + 2).split('\n').every(l => /^[A-Za-z][\w-]*: \S/.test(l)); - fs.writeFileSync(message, (body ? body + (endsInTrailer ? '\n' : '\n\n') : '') + appended.join('\n') + '\n'); + // git places the lines in the message's trailer block (joining an existing block, above comment + // lines, CRLF kept), so an existing co-author keeps GitHub's attribution; a separate paragraph would not. + if (!fs.existsSync(message)) fs.writeFileSync(message, ''); + const placed = git(['interpret-trailers', '--in-place', ...appended.flatMap(t => ['--trailer', t]), message]); + if (!placed.ok) { process.stderr.write(`gstack-coauthor-trailers: cannot append to ${message}: ${placed.err.split('\n')[0] ?? ''}\n`); return EXIT.fail; } } } if (argv.includes('--json')) process.stdout.write(JSON.stringify({ range, authors: [...authors.values()], trailers, appended }) + '\n'); diff --git a/lib/ship-policy.ts b/lib/ship-policy.ts index 5336a797a..a5cf1a0d8 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -161,9 +161,11 @@ export const SHIP_QUEUE_KEYS_VERSION = '1.91.108.0'; export const TRAIN_KEYS_VERSION = '1.91.146.0'; // Each constant is the release that first reads its key: a policy declaring the key must name at least // that version as min_gstack_version, so an older gstack refuses the policy by name instead of failing on -// an unknown key. review_scope shipped in 1.91.256.0 (#3481); docs.caps ships in the release after it. +// an unknown key. review_scope shipped in 1.91.256.0 (#3481). docs.caps is unreleased: its constant names the +// next release after the newest CHANGELOG heading, and test/gstack-ship-policy.test.ts fails when a release that +// does not read the key is stamped under that number (the stamper then moves the constant with the heading). export const REVIEW_SCOPE_KEY_VERSION = '1.91.256.0'; -export const DOCS_CAPS_KEY_VERSION = '1.91.257.0'; +export const DOCS_CAPS_KEY_VERSION = '1.91.258.0'; export const POLICY_KEY_INTRODUCED: Record = { bump: '1.91.73.0', queue_mode: '1.91.73.0', release_tool: '1.91.73.0', release_outputs: '1.91.73.0', stamp_paths: '1.91.73.0', changelog: '1.91.73.0', mirror: '1.91.73.0', history: '1.91.73.0', preregistration_shas: '1.91.73.0', stamp_sensitive_gates: '1.91.73.0', diff --git a/test/gstack-coauthor-trailers.test.ts b/test/gstack-coauthor-trailers.test.ts index 2c4bf1664..eb2720a8b 100644 --- a/test/gstack-coauthor-trailers.test.ts +++ b/test/gstack-coauthor-trailers.test.ts @@ -74,11 +74,18 @@ test('--message appends only the missing trailers and is idempotent; a bad range expect(run(dir, ['--message', file]).status).toBe(EXIT.ok); const expected = 'port: contributor fix\n\nCo-authored-by: ada contributor \nCo-authored-by: Bo Helper \n'; expect(fs.readFileSync(file, 'utf8')).toBe(expected); - expect(spawnSync('git', ['interpret-trailers', '--parse'], { cwd: dir, input: expected, encoding: 'utf8' }).stdout.trim().split('\n')).toHaveLength(2); + expect(spawnSync('git', ['interpret-trailers', '--parse'], { cwd: dir, input: expected, encoding: 'utf8', timeout: 30_000 }).stdout.trim().split('\n')).toHaveLength(2); const plain = path.join(dir, 'PLAIN'); fs.writeFileSync(plain, 'port: contributor fix\n'); expect(run(dir, ['--message', plain]).status).toBe(EXIT.ok); expect(fs.readFileSync(plain, 'utf8')).toBe('port: contributor fix\n\nCo-authored-by: Ada Contributor \nCo-authored-by: Bo Helper \n'); + const crlf = path.join(dir, 'CRLF'); + fs.writeFileSync(crlf, 'port: contributor fix\r\n\r\nCo-authored-by: ada contributor \r\n# comment\r\n'); + expect(run(dir, ['--message', crlf]).status).toBe(EXIT.ok); + const parsed = spawnSync('git', ['interpret-trailers', '--parse', crlf], { cwd: dir, encoding: 'utf8', timeout: 30_000 }).stdout; + expect(parsed).toContain('ada contributor'); + expect(parsed).toContain('Bo Helper'); + expect(fs.readFileSync(crlf, 'utf8')).toMatch(/Co-authored-by: Bo Helper \r?\n# comment/); const again = run(dir, ['--message', file, '--json']); expect(JSON.parse(again.stdout).appended).toEqual([]); expect(fs.readFileSync(file, 'utf8')).toBe(expected); diff --git a/test/gstack-ship-policy.test.ts b/test/gstack-ship-policy.test.ts index 5d88f2b97..7251ea64e 100644 --- a/test/gstack-ship-policy.test.ts +++ b/test/gstack-ship-policy.test.ts @@ -6,8 +6,9 @@ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; import { EXIT } from '../lib/headless-artifacts'; -import { DEFAULT_POLICY, POLICY_KEYS, REVIEW_SCOPE_KEY_VERSION, globToRegExp, matchesAny, validatePolicy } from '../lib/ship-policy'; +import { DEFAULT_POLICY, POLICY_KEYS, POLICY_KEY_INTRODUCED, REVIEW_SCOPE_KEY_VERSION, globToRegExp, matchesAny, validatePolicy } from '../lib/ship-policy'; import { fillDue } from '../lib/ship-queue-train'; import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; @@ -73,6 +74,23 @@ describe('validatePolicy (schema + containment, pure)', () => { }); }); +describe('policy key versions against the CHANGELOG', () => { + test('an unreleased key names the release right after the newest heading; a released key has a heading', () => { + // The base branch's CHANGELOG counts too: a stamp-at-merge branch carries main's headings only after the slot merge. + const root = path.join(import.meta.dir, '..'); + const base = spawnSync('git', ['show', 'origin/main:CHANGELOG.md'], { cwd: root, encoding: 'utf8', timeout: 30_000 }); + const text = fs.readFileSync(path.join(root, 'CHANGELOG.md'), 'utf8') + (base.stdout ?? ''); + const headings = [...text.matchAll(/^## \[(\d+\.\d+\.\d+\.\d+)\]/gm)].map(m => m[1]!.split('.').map(Number)); + const newest = headings.sort((a, b) => a[0]! - b[0]! || a[1]! - b[1]! || a[2]! - b[2]! || a[3]! - b[3]!).at(-1)!; + const next = `${newest[0]}.${newest[1]}.${newest[2]! + 1}.0`; + const released = new Set(headings.map(h => h.join('.'))); + for (const [key, version] of Object.entries(POLICY_KEY_INTRODUCED)) { + if (released.has(version)) continue; + expect(version, `${key} is introduced in ${version}, which has no CHANGELOG heading; an unreleased key names the next release ${next} (move it when the stamp lands elsewhere)`).toBe(next); + } + }); +}); + describe("gstack's own .gstack/ship-policy.json", () => { test('validates and declares the merge-train block against workflows that exist', () => { const root = path.join(import.meta.dir, '..');