mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
train forget: restore the local notes ref on a rejected lease and refuse every in-flight state
forgetNotes removed the note locally before the leased push, so a rejected lease left a local notes ref without the note that the next plain persist pushed without a lease. It now snapshots the ref and restores it on rejection, reporting failed removals separately. forget refused only merging/gate-pending/free-pending; built, pushed, free-green, dispatch-unknown and dispatch-absent are in flight too and are refused the same way. CHANGELOG and ship-policy say so. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
db7c8c8940
commit
3909acadd7
5 files changed
+46
-19
No files matched your search
+1
-1
@@ -9,7 +9,7 @@
|
||||
- **`QUEUE_SCREEN_RED`**: a RED screen refuses `ready` under `--screen` and `--no-screen` alike, is never re-dispatched at the same merge SHA, and `train build` refuses to seat the pin. **`QUEUE_SCREEN_NOT_RUN`**: a screen run that concluded success with no slice (its route job found the head superseded by a push, or the plan selected nothing) is `NOT-RUN`: `verify --screen` refuses, the receipt records `NOT_RUN`, `ready --screen` dispatches afresh instead of following it, and `train build` seats the pin unscreened with a `SCREEN: … counted as not-run` line.
|
||||
- **Scratch-merge screens (#3465)**: when GitHub's `refs/pull/<n>/merge` lags the PR head, is missing or cannot be fetched, `ready --screen` plans the screen on a scratch merge of the head onto the base, pushed as `screen/<pr>-<head12>` (`SCREEN_MERGE:` names it); only a real conflict with the base is `QUEUE_SCREEN_STALE_REF`, whose text now says so, and an unreachable remote is `POLICY_SOURCE_UNAVAILABLE`.
|
||||
- **`train watch --train <nonce> [--timeout <s>]`** (`lib/ship-queue-train-watch.ts`): the one resumable follow command. It reads the record, runs the step its state asks for (`train build --train` to push, run the free tests and dispatch the gate; `train status` to settle an unconfirmed dispatch), waits for the pending run by polling gh, reads the verdict and prints the last step's `NEXT:`. No process state: rerun it after an interruption and it continues from the record; it never dispatches anything the record would not dispatch on its own.
|
||||
- **`train prune --stale`**: a `merging` or `green` train whose every PR is MERGED or CLOSED on GitHub is marked `stale` (a new terminal state), its branch and the PRs' `screen/<pr>-*` branches deleted, each PR's state logged on a `STALE:` line; a train with an OPEN PR is kept and says why. **`train forget --train <nonce> | --receipt <pr>@<sha> [--dry-run]`** (`lib/ship-queue-train-forget.ts`) removes one settled train (attempts included) or one READY receipt from local state and from the notes ref with an exact `--force-with-lease` on `refs/notes/gstack-trains`; a merging, gate-pending or free-pending train is refused.
|
||||
- **`train prune --stale`**: a `merging` or `green` train whose every PR is MERGED or CLOSED on GitHub is marked `stale` (a new terminal state), its branch and the PRs' `screen/<pr>-*` branches deleted, each PR's state logged on a `STALE:` line; a train with an OPEN PR is kept and says why. **`train forget --train <nonce> | --receipt <pr>@<sha> [--dry-run]`** (`lib/ship-queue-train-forget.ts`) removes one settled train (attempts included) or one READY receipt from local state and from the notes ref with an exact `--force-with-lease` on `refs/notes/gstack-trains`; a train still building, running or merging is refused, and a rejected lease removes nothing.
|
||||
- **`verify --post-merge <sha> [--timeout <s>]`** (`lib/ship-queue-post-merge.ts`): after a merge, waits for every check run attached to the merged commit to settle, prints one `CHECK: <name> <conclusion>` per check and `POST_MERGE: <sha12> green|red|pending|none (passed= failed= pending= skipped=)`; green needs a completed check and no failure, `neutral`/`skipped` count as skipped, a failure is **`QUEUE_POST_MERGE_RED`**, a timeout is `QUEUE_TRAIN_PENDING` with the same command as `NEXT:`.
|
||||
- **`ready … --dispatch-ci <wf.yml[,…]> [--timeout <s>]`** (`lib/ship-queue-dispatch-ci.ts`): for a PR that conflicts with the base and so has no pull_request CI, dispatches each named workflow on the branch (skipped when a run at this head already concluded), waits for the runs (`CI_DISPATCH:` lines), clears every requires-remote pregate lane whose workflow is one of them with the run URL (`PREGATE_CLEARED:`), then reads the head as usual. A red run clears no lane and refuses; a fork head is `QUEUE_TRAIN_UNTRUSTED`. This replaces the four-step hand procedure in docs/ship-policy.md.
|
||||
- **`train build --dry-run --stack-tests [--stack-tests-command "<cmd>"]`** (`lib/ship-queue-stack-tests.ts`): runs each PR's new test files on the integrated prefix that includes it (T<k>) in a scratch worktree and prints `STACK_TESTS: #<pr> T<k> <n> new test file(s) pass|FAIL (<files>)` or `no new test files`; a failure is **`QUEUE_TRAIN_STACK_RED`** with nothing written, pushed or dispatched.
|
||||
|
||||
+4
-1
@@ -575,7 +575,10 @@ PRs landed through another train or by hand): it logs each PR's state on a
|
||||
<pr>@<sha> [--dry-run]` removes one record (a settled train with its attempts,
|
||||
or one READY receipt) from local state and from `refs/notes/gstack-trains`
|
||||
with an exact lease on the notes ref, printing `FORGET:` then `FORGOTTEN:`; a
|
||||
merging, gate-pending or free-pending train is refused.
|
||||
train that is still building, running or merging (`built`, `pushed`,
|
||||
`free-pending`, `free-green`, `dispatch-unknown`, `dispatch-absent`,
|
||||
`gate-pending`, `merging`) is refused; a rejected lease removes nothing and
|
||||
restores the local notes ref.
|
||||
|
||||
**Following a train without a daemon.** `gstack-ship-queue train watch --train
|
||||
<nonce> [--timeout <s>]` is the one resumable follow command: it reads the
|
||||
|
||||
@@ -4,15 +4,19 @@
|
||||
* refs/notes/gstack-trains under an exact lease, then delete the local files
|
||||
* (until now a stale record needed a hand-run
|
||||
* `git notes remove` plus a file hunt on every machine that ran the train).
|
||||
* A rejected lease removes nothing. Other machines that still hold the local
|
||||
* files can push the record back with their next write; the output says so.
|
||||
* A rejected lease removes nothing (the local notes ref is restored too, so a
|
||||
* later write cannot carry the removal without a lease). Other machines that
|
||||
* still hold the local files can push the record back with their next write;
|
||||
* the output says so.
|
||||
*/
|
||||
import * as fs from 'node:fs';
|
||||
import type { QueueContext } from './ship-queue';
|
||||
import { outOk, outRefused, trainEnv, type TrainOut } from './ship-queue-train';
|
||||
import { attemptsOf, forgetNotes, loadBisect, loadReady, loadTrain, noteName, readyPath, rebuildFromNotes, trainDir, type AnyRecord } from './ship-queue-train-record';
|
||||
import { attemptsOf, forgetNotes, loadBisect, loadReady, loadTrain, noteName, readyPath, rebuildFromNotes, trainDir, type AnyRecord, type TrainState } from './ship-queue-train-record';
|
||||
|
||||
const Q = 'gstack-ship-queue';
|
||||
/** States with work still in flight (a branch being built or pushed, free tests or a gate running, a merge under way): forget never settles them. */
|
||||
const LIVE_STATES: readonly TrainState[] = ['built', 'pushed', 'free-pending', 'free-green', 'dispatch-unknown', 'dispatch-absent', 'gate-pending', 'merging'];
|
||||
|
||||
export function trainForget(ctx: QueueContext, o: { train?: string; receipt?: string; dryRun: boolean }): TrainOut {
|
||||
const env = trainEnv(ctx);
|
||||
@@ -24,8 +28,8 @@ export function trainForget(ctx: QueueContext, o: { train?: string; receipt?: st
|
||||
if (o.train) {
|
||||
const t = loadTrain(o.train);
|
||||
if (!t) return outRefused({ code: 'QUEUE_PROOF_MISSING', message: `no train record ${o.train} here or in refs/notes/gstack-trains` }, [], `${Q} train status`, 'none');
|
||||
if (t.state === 'merging' || t.state === 'gate-pending' || t.state === 'free-pending') {
|
||||
return outRefused({ code: 'QUEUE_PROOF_MISSING', message: `train ${t.nonce} is ${t.state}; forget settles nothing that is still running or merging (train prune --stale for a train whose PRs merged elsewhere)` }, [], `${Q} train status --train ${t.nonce}`, trainDir(t.nonce));
|
||||
if (LIVE_STATES.includes(t.state)) {
|
||||
return outRefused({ code: 'QUEUE_PROOF_MISSING', message: `train ${t.nonce} is ${t.state}; forget settles nothing that is still building, running or merging (train prune --stale for a train whose PRs merged elsewhere)` }, [], `${Q} train status --train ${t.nonce}`, trainDir(t.nonce));
|
||||
}
|
||||
const bisect = loadBisect(t.nonce);
|
||||
records.push(t, ...attemptsOf(t), ...(bisect ? [bisect] : []));
|
||||
|
||||
@@ -241,21 +241,26 @@ function parseNote(text: string): AnyRecord | null {
|
||||
* (a rejected lease returns `pushed: false` and nothing is deleted locally).
|
||||
* Names are note names (noteName); a name with no note is reported missing.
|
||||
*/
|
||||
export function forgetNotes(repoRoot: string, names: string[], remote = 'origin'): { removed: string[]; missing: string[]; pushed: boolean; lease: string | null; detail: string } {
|
||||
export function forgetNotes(repoRoot: string, names: string[], remote = 'origin'): { removed: string[]; missing: string[]; failed: string[]; pushed: boolean; lease: string | null; detail: string } {
|
||||
const f = fetchNotes(repoRoot, remote);
|
||||
if (!f.ok) return { removed: [], missing: [], pushed: false, lease: null, detail: f.detail };
|
||||
if (!f.ok) return { removed: [], missing: [], failed: [], pushed: false, lease: null, detail: f.detail };
|
||||
const lease = git(repoRoot, ['rev-parse', '--verify', '--quiet', NOTES_REMOTE_REF]).out || null;
|
||||
const before = git(repoRoot, ['rev-parse', '--verify', '--quiet', NOTES_REF]).out || null;
|
||||
const listed = git(repoRoot, ['notes', `--ref=${NOTES_REF}`, 'list']).out.split('\n').map(l => l.split(' ')[1]).filter(Boolean);
|
||||
const removed: string[] = [];
|
||||
const missing: string[] = [];
|
||||
const failed: string[] = [];
|
||||
const restore = () => { if (before) git(repoRoot, ['update-ref', NOTES_REF, before]); };
|
||||
for (const name of names) {
|
||||
const blob = git(repoRoot, ['hash-object', '--stdin'], undefined, name).out;
|
||||
if (!listed.includes(blob)) { missing.push(name); continue; }
|
||||
if (git(repoRoot, ['notes', `--ref=${NOTES_REF}`, 'remove', blob], notesEnv()).ok) removed.push(name); else missing.push(name);
|
||||
if (git(repoRoot, ['notes', `--ref=${NOTES_REF}`, 'remove', blob], notesEnv()).ok) removed.push(name); else failed.push(name);
|
||||
}
|
||||
if (!removed.length) return { removed, missing, pushed: false, lease, detail: 'no note to remove' };
|
||||
if (failed.length) { restore(); return { removed: [], missing, failed, pushed: false, lease, detail: `git notes remove failed for ${failed.join(', ')}; the local notes ref was restored` }; }
|
||||
if (!removed.length) return { removed, missing, failed, pushed: false, lease, detail: 'no note to remove' };
|
||||
const p = git(repoRoot, ['push', '-q', lease ? `--force-with-lease=${NOTES_REF}:${lease}` : `--force-with-lease=${NOTES_REF}`, remote, `${NOTES_REF}:${NOTES_REF}`]);
|
||||
return { removed, missing, pushed: p.ok, lease, detail: p.ok ? `notes pushed to ${remote} ${NOTES_REF} (lease ${lease?.slice(0, 12) ?? 'new ref'})` : `git push ${NOTES_REF} rejected: ${p.err.replace(/\s+/g, ' ').trim().slice(0, 160)}` };
|
||||
if (!p.ok) restore();
|
||||
return { removed, missing, failed, pushed: p.ok, lease, detail: p.ok ? `notes pushed to ${remote} ${NOTES_REF} (lease ${lease?.slice(0, 12) ?? 'new ref'})` : `git push ${NOTES_REF} rejected: ${p.err.replace(/\s+/g, ' ').trim().slice(0, 160)}; the local notes ref was restored` };
|
||||
}
|
||||
|
||||
/** Rebuild local records from the notes ref: a note whose record is missing locally, or newer than the local copy, is written. */
|
||||
|
||||
@@ -223,7 +223,7 @@ describe('train prune --stale and train watch', () => {
|
||||
|
||||
// Value: protects=a stale train record or READY receipt leaves refs/notes/gstack-trains through one command under an exact lease, with the local files, instead of a hand-run git notes remove on every machine;
|
||||
// fails_when=forget removes the wrong notes, deletes locally when the push failed, or forgets a merging train; why_new=no CLI removed notes (merge-train skill, known gaps); seam=none; no_claim=a concurrent writer's race (the lease is exercised only as a fast-forward)
|
||||
test('train forget removes a settled train (attempts included) or a READY receipt from the notes ref and the state root, leaves other records alone, and refuses a merging train', () => {
|
||||
test('train forget removes a settled train (attempts included) or a READY receipt from the notes ref and the state root, leaves other records alone, keeps everything (the local notes ref too) when the lease is rejected, and refuses a train still building, running or merging', () => {
|
||||
const fx = make();
|
||||
const { nonce, h1 } = greenTrain(fx);
|
||||
expect(queue(fx, ['train', 'verdict', '--train', nonce]).status).toBe(EXIT.ok);
|
||||
@@ -266,12 +266,27 @@ describe('train prune --stale and train watch', () => {
|
||||
expect(queue(fx, ['train', 'forget']).status).toBe(EXIT.usage);
|
||||
const { nonce: second } = (() => { readyReceipt(fx, '3', h3); readyReceipt(fx, '4', h4); const r = queue(fx, ['train', 'build', '--prs', `3@${h3},4@${h4}`, '--free-timeout', '1']); return { nonce: nonceOf(r.out) }; })();
|
||||
expect(queue(fx, ['train', 'verdict', '--train', second]).status).toBe(EXIT.ok);
|
||||
const t = JSON.parse(fs.readFileSync(path.join(fx.state, 'ship-queue', 'trains', second, 'train.json'), 'utf8'));
|
||||
t.state = 'merging';
|
||||
fs.writeFileSync(path.join(fx.state, 'ship-queue', 'trains', second, 'train.json'), JSON.stringify(t));
|
||||
const merging = queue(fx, ['train', 'forget', '--train', second]);
|
||||
expect(merging.status).toBe(EXIT.refused);
|
||||
expect(merging.err).toMatch(/is merging; forget settles nothing that is still running or merging .*\(QUEUE_PROOF_MISSING\)/);
|
||||
const hook = path.join(fx.repo.origin, 'hooks', 'pre-receive');
|
||||
fs.mkdirSync(path.dirname(hook), { recursive: true });
|
||||
fs.writeFileSync(hook, '#!/bin/sh\nwhile read o n r; do case "$r" in refs/notes/*) echo "notes frozen" >&2; exit 1;; esac; done\nexit 0\n', { mode: 0o755 });
|
||||
const localNotes = fx.repo.git('rev-parse', 'refs/notes/gstack-trains');
|
||||
const rejected = queue(fx, ['train', 'forget', '--train', second]);
|
||||
expect(rejected.status).toBe(EXIT.refused);
|
||||
expect(rejected.err).toMatch(/the notes ref was not updated \(git push refs\/notes\/gstack-trains rejected: .*notes frozen.*; the local notes ref was restored\); nothing deleted locally.*\(QUEUE_PROOF_MISSING\)/);
|
||||
expect(fs.existsSync(path.join(fx.state, 'ship-queue', 'trains', second, 'train.json'))).toBe(true);
|
||||
expect(fx.repo.git('rev-parse', 'refs/notes/gstack-trains')).toBe(localNotes);
|
||||
expect(fx.repo.git('notes', '--ref=refs/notes/gstack-trains', 'list').split('\n').length).toBeGreaterThan(0);
|
||||
fs.rmSync(hook);
|
||||
const trainFile = path.join(fx.state, 'ship-queue', 'trains', second, 'train.json');
|
||||
for (const state of ['dispatch-unknown', 'merging']) {
|
||||
const t = JSON.parse(fs.readFileSync(trainFile, 'utf8'));
|
||||
t.state = state;
|
||||
fs.writeFileSync(trainFile, JSON.stringify(t));
|
||||
const live = queue(fx, ['train', 'forget', '--train', second]);
|
||||
expect(live.status).toBe(EXIT.refused);
|
||||
expect(live.err).toMatch(new RegExp(`is ${state}; forget settles nothing that is still building, running or merging .*\\(QUEUE_PROOF_MISSING\\)`));
|
||||
expect(fs.existsSync(trainFile)).toBe(true);
|
||||
}
|
||||
}, 120_000);
|
||||
|
||||
// Value: protects=a restarted controller re-arms with one command: watch continues from the record (free tests, gate dispatch, the pending run, the verdict) and prints NEXT;
|
||||
|
||||
Reference in new issue
Block a user