mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 14:06:50 +01:00
review fixes: a carried child entry never repeats a released parent line; min_gstack_version 1.91.340.0; required-check names as GitHub reports them
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
7c8bdf6f8a
commit
c50c93bf2f
6 files changed
+24
-15
No files matched your search
@@ -15,5 +15,5 @@
|
||||
"maintainers": ["garrytan"],
|
||||
"train": { "gate_workflow": "evals.yml", "free_workflow": "free-tests.yml", "full_gate_check": "gstack/full-gate", "record_artifact": "gstack-train-record", "max_size": 3, "fill_hours": 1, "screen_selection": "gstack-paid", "trial_reservation_usd": 1.5 },
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-free-tests", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint"] },
|
||||
"min_gstack_version": "1.91.336.0"
|
||||
"min_gstack_version": "1.91.340.0"
|
||||
}
|
||||
+3
-3
@@ -2,7 +2,7 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
**gstack now versions its own PRs at merge time: a PR adds `changelog.d/<branch-slug>.md` instead of a version, and trains merge through `train land` instead of slot calls.** `.gstack/ship-policy.json` turns `changelog_fragments` on with the checks every PR runs as `required_checks`, and `min_gstack_version` moves to 1.91.336.0, the release that introduced the key, so an older gstack refuses the file by name.
|
||||
**gstack now versions its own PRs at merge time: a PR adds `changelog.d/<branch-slug>.md` instead of a version, and trains merge through `train land` instead of slot calls.** `.gstack/ship-policy.json` turns `changelog_fragments` on with the checks every PR runs as `required_checks`, and `min_gstack_version` moves to 1.91.340.0, the first release that reads `required_checks` and counts a stamped slot as merged only when GitHub reports it MERGED, so an older gstack refuses the file by name.
|
||||
|
||||
### Changed
|
||||
- **`changelog_fragments` is on for gstack** (`.gstack/ship-policy.json`, `changelog.d/README.md`): `required_checks` lists `free-tests`, `windows-free-tests`, `Check VERSION is not stale vs queue`, `quality`, `check-freshness` and `actionlint`. Path-filtered workflows stay out of the list, because a stamp commit of a PR outside their paths never gets those checks.
|
||||
@@ -13,8 +13,8 @@
|
||||
- **A stacked PR whose only own line sits inside its released parent's list merges its CHANGELOG instead of refusing** (`lib/changelog-check.ts`): `mergeChangelogs` filed the line as an addition to the parent's released entry and returned null, so #3613's slot needed a hand resolve. When that would leave the child nothing of its own, the inserted lines are now the child's entry; on #3613's real CHANGELOGs the result equals the resolve that shipped.
|
||||
|
||||
### Tests
|
||||
- `test/changelog-merge-damage.test.ts`: a child whose only own bullet sits at the top of the released parent's `### Changed` list keeps it as its entry and passes `ownEntryAboveMain`; a child with no line of its own stays null.
|
||||
- `test/gstack-ship-policy.test.ts`: every `required_checks` name is a job of a pull_request workflow without a path filter, and `min_gstack_version` equals the key's introducing version.
|
||||
- `test/changelog-merge-damage.test.ts`: a child whose only own bullet sits at the top of the released parent's `### Changed` list keeps it as its entry and passes `ownEntryAboveMain`; a child with no line of its own, a reordered parent bullet and a duplicated one stay null.
|
||||
- `test/gstack-ship-policy.test.ts`: every `required_checks` name is a check a pull_request workflow without a path filter reports (a job's `name:`, or its id when it has none), and `min_gstack_version` is at least the key's introducing version.
|
||||
|
||||
## [1.91.340.0] - 2026-10-30
|
||||
|
||||
|
||||
+4
-3
@@ -92,7 +92,7 @@ hard-code:
|
||||
"history": "squash-ok",
|
||||
"stamp_sensitive_gates": ["tests"],
|
||||
"changelog_fragments": { "dir": "changelog.d", "required_checks": ["free-tests", "windows-free-tests", "Check VERSION is not stale vs queue", "quality", "check-freshness", "actionlint"] },
|
||||
"min_gstack_version": "1.91.336.0"
|
||||
"min_gstack_version": "1.91.340.0"
|
||||
}
|
||||
```
|
||||
|
||||
@@ -104,8 +104,9 @@ every PR (branch protection enforces only `free-tests`); a workflow with a path
|
||||
filter (Windows Setup E2E, make-pdf gate, Native ARM Setup Smoke, Dependency
|
||||
Review) is left out, because a stamp commit of a PR outside its paths never
|
||||
gets that check and `train land` would hold on it. `min_gstack_version` is
|
||||
the release that introduced the key, so an older gstack refuses the file by
|
||||
name (`POLICY_GSTACK_TOO_OLD`).
|
||||
1.91.340.0, the first release that reads `required_checks` and counts a
|
||||
stamped slot as merged only when GitHub reports it MERGED, so an older gstack
|
||||
refuses the file by name (`POLICY_GSTACK_TOO_OLD`).
|
||||
|
||||
`.gstack/ship-policy.example.json` shows every key set the way a serial-merge
|
||||
repo with a release script, docs mentions and a mirror would set them.
|
||||
|
||||
@@ -257,10 +257,12 @@ function fragments(removed: string, added: string): { old: string; new: string }
|
||||
* byte for byte main's. A line the child inserted between two of the parent's
|
||||
* lines counts as an addition to the parent's entry, unless that would leave
|
||||
* the child nothing of its own: then the inserted lines are the child's entry
|
||||
* (#3613 put its only bullet at the top of #3595's `### Changed` list). Null
|
||||
* when the shape is something else: no single `[Unreleased]` on our side, no
|
||||
* released entry of theirs the base lacks whose text our section holds, or
|
||||
* nothing of the child's own left either way.
|
||||
* (#3613 put its only bullet at the top of #3595's `### Changed` list), and a
|
||||
* line meant as an amendment to the parent lands there too. Null when the
|
||||
* shape is something else: no single `[Unreleased]` on our side, no released
|
||||
* entry of theirs the base lacks whose text our section holds, nothing of the
|
||||
* child's own left either way, or a child entry that repeats a released
|
||||
* parent line (a reordered or duplicated bullet is a hand resolve).
|
||||
*/
|
||||
export function carryReleasedParent(ours: string, theirs: string, base: string): { text: string; notes: string[] } | null {
|
||||
return carryWith(ours, theirs, base, true) ?? carryWith(ours, theirs, base, false);
|
||||
@@ -276,11 +278,13 @@ function carryWith(ours: string, theirs: string, base: string, insertsJoinParent
|
||||
const notes: string[] = [];
|
||||
const bullets: string[] = [];
|
||||
let carried = false;
|
||||
const parentLines = new Set<string>();
|
||||
for (const parent of parseChangelog(theirs).sections.filter(s => !s.unreleased && !baseKeys.has(s.version!))) {
|
||||
const pl = parent.lines.slice(1);
|
||||
const ownBlocks = entryBlocks({ ...unreleased[0]!, lines: ['', ...lines] });
|
||||
if (!entryBlocks(parent).some(b => ownBlocks.includes(b))) continue;
|
||||
carried = true;
|
||||
for (const l of pl) if (content(l)) parentLines.add(l);
|
||||
// LCS over content lines only: blanks and sub-headings recur in both entries and would anchor wrongly.
|
||||
const lcs = Array.from({ length: pl.length + 1 }, () => new Uint32Array(lines.length + 1));
|
||||
for (let i = pl.length - 1; i >= 0; i--) for (let j = lines.length - 1; j >= 0; j--) {
|
||||
@@ -328,6 +332,7 @@ function carryWith(ours: string, theirs: string, base: string, insertsJoinParent
|
||||
});
|
||||
while (kept.length && !kept.at(-1)!.trim()) kept.pop();
|
||||
if (!kept.some(content)) return null;
|
||||
if (kept.some(l => content(l) && parentLines.has(l))) return null;
|
||||
if (bullets.length) {
|
||||
const at = kept.findIndex(l => GATE_INTEGRITY_RE.test(l));
|
||||
if (at < 0) kept.push('', '### Gate integrity', ...bullets);
|
||||
|
||||
@@ -152,6 +152,9 @@ describe("a stacked child whose parent was released since the fork (GSTA-96, tra
|
||||
expect(merged).toBe(`# C\n\n## [Unreleased]\n\n### Changed\n- mine\n\n${released}\n## [1.0.0] - d\n\n- base\n`);
|
||||
expect(ownEntryAboveMain(merged, theirs, new Set(['### Changed', '- mine', '- one', '- two', '- new thing', '### Added', '**Parent paragraph.**']))).toBeNull();
|
||||
expect(carryReleasedParent('# C\n\n## [Unreleased]\n\n**Parent paragraph.**\n\n### Added\n- new thing\n\n### Changed\n- one\n- two\n\n## [1.0.0] - d\n\n- base\n', theirs, base)).toBeNull();
|
||||
// A reordered or duplicated parent bullet is not the child's entry: it stays a hand resolve.
|
||||
expect(mergeChangelogs('# C\n\n## [Unreleased]\n\n**Parent paragraph.**\n\n### Added\n- new thing\n\n### Changed\n- two\n- one\n\n## [1.0.0] - d\n\n- base\n', theirs, base)).toBeNull();
|
||||
expect(mergeChangelogs('# C\n\n## [Unreleased]\n\n**Parent paragraph.**\n\n### Added\n- new thing\n\n### Changed\n- one\n- one\n- two\n\n## [1.0.0] - d\n\n- base\n', theirs, base)).toBeNull();
|
||||
});
|
||||
|
||||
// Value: protects=carryReleasedParent rewrites only a child that still holds the released parent's entry and keeps an existing Gate integrity section's neighbours in place; fails_when=an unrelated [Unreleased] or an already-stamped head is rewritten or the correction lands outside Gate integrity; why_new=the shapes test above never has a child without the parent's entry, a head without [Unreleased], or a Gate integrity section followed by another heading; seam=none; no_claim=the wording of corrections beyond one edited line
|
||||
|
||||
@@ -10,6 +10,7 @@ import { spawnSync } from 'node:child_process';
|
||||
import { EXIT } from '../lib/headless-artifacts';
|
||||
import { CHANGELOG_FRAGMENTS_KEY_VERSION, 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 { cmpVersion, parseVersion } from '../lib/version-source';
|
||||
import { BASE_POLICY, cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture';
|
||||
|
||||
const repos: FixtureRepo[] = [];
|
||||
@@ -126,13 +127,13 @@ describe("gstack's own .gstack/ship-policy.json", () => {
|
||||
for (const wf of [raw.train.gate_workflow, raw.train.free_workflow]) expect(fs.existsSync(path.join(root, '.github/workflows', wf))).toBe(true);
|
||||
});
|
||||
|
||||
// Value: protects=train land on gstack itself: every changelog_fragments.required_checks name is a job of a pull_request workflow without a path filter, so a stamp commit always gets it; fails_when=a name is misspelled or names a path-filtered workflow's job, and train land holds every slot whose PR skips those paths; why_new=the key was off on gstack; seam=the policy file and .github/workflows; no_claim=that the checks pass
|
||||
// Value: protects=train land on gstack itself: every changelog_fragments.required_checks name is a job of a pull_request workflow without a path filter, so a stamp commit always gets it; fails_when=a name is misspelled, names a path-filtered workflow's job, or names a job id GitHub reports under its `name:` instead, and train land holds every slot whose PR skips those paths; why_new=the key was off on gstack; seam=the policy file and .github/workflows; no_claim=that the checks pass
|
||||
test('turns changelog_fragments on with required checks every PR runs', () => {
|
||||
const root = path.join(import.meta.dir, '..');
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(root, '.gstack', 'ship-policy.json'), 'utf8'));
|
||||
expect(raw.changelog_fragments.dir).toBe('changelog.d');
|
||||
expect(fs.existsSync(path.join(root, 'changelog.d', 'README.md'))).toBe(true);
|
||||
expect(raw.min_gstack_version).toBe(CHANGELOG_FRAGMENTS_KEY_VERSION);
|
||||
expect(cmpVersion(parseVersion(raw.min_gstack_version)!, parseVersion(CHANGELOG_FRAGMENTS_KEY_VERSION)!)).toBeGreaterThanOrEqual(0);
|
||||
const jobs = new Set<string>();
|
||||
for (const f of fs.readdirSync(path.join(root, '.github/workflows')).filter(f => f.endsWith('.yml'))) {
|
||||
const text = fs.readFileSync(path.join(root, '.github/workflows', f), 'utf8');
|
||||
@@ -140,9 +141,8 @@ describe("gstack's own .gstack/ship-policy.json", () => {
|
||||
if (!/^\s+pull_request:/m.test(on) || /^\s+paths(-ignore)?:/m.test(on)) continue;
|
||||
const body = text.slice(text.search(/^jobs:/m));
|
||||
for (const m of body.matchAll(/^ ([A-Za-z0-9_-]+):\n((?: .*\n|\s*\n)*)/gm)) {
|
||||
jobs.add(m[1]!);
|
||||
const name = /^ name:\s*(.+)$/m.exec(m[2]!)?.[1]?.trim();
|
||||
if (name) jobs.add(name);
|
||||
jobs.add(name ?? m[1]!);
|
||||
}
|
||||
}
|
||||
expect(raw.changelog_fragments.required_checks.filter((n: string) => !jobs.has(n))).toEqual([]);
|
||||
|
||||
Reference in new issue
Block a user