From f9884f2f2faab0f6cf8b49df4d7fb3b8b9ebf3ef Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 31 Oct 2026 04:41:47 +0000 Subject: [PATCH 1/3] docs: batched graft landing in CONTRIBUTING and AGENTS (How changes land, Finishing) Graft-Changeset: cdd191d321660a50946afc2e2caf5321d904de4f380f8245ad2fbac1291c50af7 Graft-Session: ses_01M68411YM3RKP3KWQE582GSTJ Claimed-Author: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Graft-Batch: 27 --- AGENTS.md | 17 +++++++---------- CONTRIBUTING.md | 29 ++++++++++++----------------- 2 files changed, 19 insertions(+), 27 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bf57b1b4..f6d70167 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,16 +60,13 @@ cargo nextest run -p --all-features # or: cargo test -p ## Finishing - Add a changelog fragment `changes/.md`. -- If your branch conflicts with `main`, run `git merge origin/main` and push - normally. Never rebase a pushed branch or force-push: the merge queue squashes, - so merge commits on a branch are fine. -- Batch PRs (`graft/batch-`, made by `pr-batch`) are the exception: they - bypass the merge queue, and `land-pr` rebase-merges them with `--admin` once - the required checks pass, so each land's commit survives (D28). Every other - PR squashes through the queue. -- Commit with the identity in your brief, push the branch, and **stop**. - Don't open the PR, don't poll CI. Oversight opens it, reviews it and lands - it. +- Land with `g land` on the graft line (`backlog-1`): commit, `g sync`, then + `g land`. Once landed, **stop**. Don't open a PR, don't poll CI. +- Batch PRs are the normal path. Oversight cuts one batch PR from the line + (`graft/batch-`, made by `pr-batch`) and `land-pr` rebase-merges it with + `--admin` once the required checks pass, so each land's commit survives + (D28). Hand-made PRs squash through the merge queue. +- Commit with the identity in your brief. - Leave a Vogt hand-off comment on your work item: branch, head sha, and which checks you ran. Then list **every** Deliver and Accept item from the work item, each with the file and test that meets it, or `NOT DONE: `. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4b1d15ea..1d6b3e1b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -6,24 +6,19 @@ an issue to discuss a change to the design first. ## How changes land -graft follows trunk-based development, with a merge queue. +graft lands through its own mirror, batched. Workers `g land` on graft line +`backlog-1`. Oversight cuts one batch PR from the line (`graft-mirror +pr-batch`) and merges it with `land-pr --method rebase`, so each land keeps +its own commit (D28). When a backlog of work is under way, all of it goes into +a single batch PR. Batch rules: workspace `.claude/rules/delivery-process.md` +("Landing"). -1. **Branch** off `main`: one short-lived branch per work item or change, - named `g--` for plan items. Keep PRs small, and merge them - within hours, not days. -2. **Push** the branch and open a PR against `main`. Agent workers push and - stop. Oversight opens the PR. -3. **Review.** Every PR gets a review, from a different model than the author - where an agent wrote it. Blockers go back to the author. -4. **Merge queue.** A clean review is added to the merge queue, which - **squashes**: each PR lands as one commit on `main`. The queue runs the - required checks on the latest `main`, then merges. Nobody pushes to `main` - directly. -5. **Behind or conflicting with `main`?** Run `git merge origin/main` on your - branch, resolve, and push normally. Never rebase a pushed branch or - force-push. Resolve the root `Cargo.toml` by keeping every dependency line, - sorted. Resolve `Cargo.lock` by taking `main`'s copy and running - `cargo update -w`. +- Graft line: `backlog-1`; mirror: `graft-dev-mirror-graft` +- Required checks: `fmt`, `clippy`, `test` (`.github/workflows/ci.yml`) +- Review: every batch gets a review from a different model than the author + where an agent wrote it. Blockers go back to the author. +- Every change adds a changelog fragment (see [Rules for every change](#rules-for-every-change)). +- Hand-made PRs (rare, operator-agreed) still squash through the merge queue. **Required checks** (`.github/workflows/ci.yml`, on `pull_request` and `merge_group`): From bffdea81a2219261ba2d536c6fa995d87e815f93 Mon Sep 17 00:00:00 2001 From: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Date: Sat, 31 Oct 2026 04:44:14 +0000 Subject: [PATCH 2/3] graft-mirror: pr-batch defers undersized batches (--min-lands, --min-age, [batch]) unless --force (WI-2719) Graft-Changeset: cc8a7df2d41418b173638e2f285fec1b8fbef34c9ae7cec91d370e0582f23274f Graft-Session: ses_01M683ZW2TDC3RHD1024VRYFVJ Graft-Work-Item: WI-2719 Claimed-Author: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com> Graft-Batch: 27 --- changes/WI-2719.md | 1 + crates/graft-mirror/src/batch/batcher.rs | 29 ++++++- crates/graft-mirror/src/batch/render.rs | 42 +++++++++- crates/graft-mirror/src/batch/tests.rs | 79 ++++++++++++++++++ crates/graft-mirror/src/batch/types.rs | 26 ++++++ crates/graft-mirror/src/config.rs | 91 +++++++++++++++++++- crates/graft-mirror/src/db.rs | 13 ++- crates/graft-mirror/src/entry.rs | 96 +++++++++++++++++++++- crates/graft-mirror/src/sample_mirror.toml | 13 ++- crates/graft-mirror/tests/batch.rs | 47 +++++++++++ docs/guide/git-migration.md | 22 +++++ 11 files changed, 447 insertions(+), 12 deletions(-) create mode 100644 changes/WI-2719.md diff --git a/changes/WI-2719.md b/changes/WI-2719.md new file mode 100644 index 00000000..2ce57ed5 --- /dev/null +++ b/changes/WI-2719.md @@ -0,0 +1 @@ +- `graft-mirror pr-batch` gains `--min-lands N`, `--min-age MINUTES` and `--force`, and a `[batch]` table in `mirror.toml` (`min_lands`, `min_age_minutes`, defaults 6 and 60). With thresholds set, a run cuts a batch only when enough lands wait or the previous batch settled long enough ago; otherwise it prints `batch deferred: …` and exits 0. `--force` cuts anyway and marks the PR body `forced`. Unset keeps today's behaviour. diff --git a/crates/graft-mirror/src/batch/batcher.rs b/crates/graft-mirror/src/batch/batcher.rs index dc0cf337..0da283f9 100644 --- a/crates/graft-mirror/src/batch/batcher.rs +++ b/crates/graft-mirror/src/batch/batcher.rs @@ -24,7 +24,8 @@ use crate::text::{clean_message, clean_text}; use crate::{ExportLog, MAX_RECORD_EXPORTS, MirrorError, current_ref, move_ref}; use super::render::{ - BatchHeader, keep_with_reverts, land_commit, parse_commit, render, settled_keep, + BatchHeader, FORCED_LINE, defer_cut, keep_with_reverts, land_commit, parse_commit, render, + settled_keep, }; use super::types::{ Attested, BATCH, BRANCH_PREFIX, BatchCommit, BatchConfig, BatchLine, BatchOutcome, BatchRecord, @@ -1106,6 +1107,27 @@ where .max_lands .unwrap_or(MAX_COMMITS) .min(MAX_COMMITS); + if !self.config.force { + // The thresholds count from the line's previous batch, which is + // settled here (an open one stops the run before the plan). A + // batch settled before WI-2719 counts from when it was made. + let settled = match (self.config.min_lands, self.config.min_age_ms) { + (None, None) => None, + _ => self + .db + .last_batch(&self.config.line)? + .map(|r| r.settled_ms.unwrap_or(r.created_ms)), + }; + if let Some(deferred) = defer_cut( + lands.len(), + self.config.min_lands, + self.config.min_age_ms, + settled, + (self.clock)(), + ) { + return Ok(Err(deferred)); + } + } if let Some(age) = self.config.max_age_ms { let oldest_ms = lands.iter().map(|l| l.authored_ms).min().unwrap_or(0); let full = lands.len() >= max_lands; @@ -1277,6 +1299,7 @@ where checked: self.config.checked, fallback: fallback.clone(), appended: HashSet::new(), + forced: self.config.force, }, &lands, ); @@ -1324,6 +1347,7 @@ where detail: String::new(), merge_sha: None, merged: None, + settled_ms: None, }; if !self.db.insert_batch(&rec, &commits)? { return Err(config_err(format!( @@ -1826,6 +1850,7 @@ where checked: self.config.checked, fallback: None, appended, + forced: rec.body.contains(FORCED_LINE), }, &lands, ); @@ -2027,6 +2052,7 @@ where let commits = self.map_merged(rec, merge).await?; rec.merged = Some(merged); rec.state = BatchState::Merged; + rec.settled_ms = Some((self.clock)()); rec.detail = format!("merged as {merge} (graft {merged})"); // The merge commit's snapshot is the batch tip's (plus whatever // GitHub's main gained meanwhile), so the next sync diffs from @@ -2132,6 +2158,7 @@ where "mirror batch rejected; the line is left as it is" ); rec.state = BatchState::Rejected; + rec.settled_ms = Some((self.clock)()); rec.detail = detail; self.db.put_batch(rec)?; Ok(()) diff --git a/crates/graft-mirror/src/batch/render.rs b/crates/graft-mirror/src/batch/render.rs index e23d0dd3..6e239b36 100644 --- a/crates/graft-mirror/src/batch/render.rs +++ b/crates/graft-mirror/src/batch/render.rs @@ -11,8 +11,8 @@ use graft_proto::types::{LandCheck, LandCheckState}; use crate::text::clean_text; use super::types::{ - BATCH_TRAILER, BRANCH_PREFIX, CHANGESET_TRAILER, Dropped, FIELD_CHARS, LandLine, MAX_CHECKS, - MAX_DROPS_LISTED, MAX_LANDS, MAX_PR_BODY, git_time_at, short, + BATCH_TRAILER, BRANCH_PREFIX, BatchOutcome, CHANGESET_TRAILER, Dropped, FIELD_CHARS, LandLine, + MAX_CHECKS, MAX_DROPS_LISTED, MAX_LANDS, MAX_PR_BODY, git_time_at, short, }; /// What a batch's PR body and commit message say before the land list. @@ -42,6 +42,41 @@ pub(crate) struct BatchHeader { pub(crate) fallback: Option, /// Lands added after the PR opened. pub(crate) appended: HashSet, + /// Cut with `--force`, past the batch thresholds (WI-2719). + pub(crate) forced: bool, +} + +/// The PR body line of a batch cut with `--force`. +pub(crate) const FORCED_LINE: &str = + "- Forced: cut with `--force`, ignoring the batch thresholds (forced)\n"; + +/// Whether a batch with `lands` waiting is held back by the cut thresholds +/// (WI-2719): `Some(Deferred)` unless `min_lands` lands wait, or the +/// previous batch settled (at `settled_ms`) at least `min_age_ms` before +/// `now_ms`, or there is no previous batch. Both thresholds `None` never +/// defers. +pub(crate) fn defer_cut( + lands: usize, + min_lands: Option, + min_age_ms: Option, + settled_ms: Option, + now_ms: i64, +) -> Option { + if min_lands.is_none() && min_age_ms.is_none() { + return None; + } + let settled_ms = settled_ms?; + if min_lands.is_some_and(|n| lands >= n) { + return None; + } + if min_age_ms.is_some_and(|a| now_ms.saturating_sub(settled_ms) >= a) { + return None; + } + Some(BatchOutcome::Deferred { + lands, + min_lands, + due_ms: min_age_ms.map(|a| settled_ms.saturating_add(a)), + }) } /// How many of `lands` (newest first) a batch capped at `max` takes, oldest @@ -350,6 +385,9 @@ pub(crate) fn render(h: &BatchHeader, lands: &[LandLine]) -> (String, String) { if let Some(why) = &h.uncut { head.push_str(&format!("- Not cut: {why}\n")); } + if h.forced { + head.push_str(FORCED_LINE); + } if !h.drops.is_empty() { head.push_str(&format!( "- Dropped: {} land{}, each reverted on the line (both are in this batch, so \ diff --git a/crates/graft-mirror/src/batch/tests.rs b/crates/graft-mirror/src/batch/tests.rs index 5a849ed1..d8a9c193 100644 --- a/crates/graft-mirror/src/batch/tests.rs +++ b/crates/graft-mirror/src/batch/tests.rs @@ -50,6 +50,7 @@ fn header(capped: bool) -> BatchHeader { checked: false, fallback: Some("test".to_owned()), appended: std::collections::HashSet::new(), + forced: false, } } @@ -583,3 +584,81 @@ fn crlf_and_trailing_whitespace_still_join_the_block() { ) ); } + +mod cut_thresholds { + use super::super::render::{FORCED_LINE, defer_cut, render}; + use super::super::types::BatchOutcome; + use super::{header, land}; + + const MIN: i64 = 60_000; + const SETTLED: i64 = 1_000 * MIN; + + #[test] + fn below_both_thresholds_defers() { + let out = defer_cut( + 2, + Some(6), + Some(60 * MIN), + Some(SETTLED), + SETTLED + 20 * MIN, + ); + assert_eq!( + out, + Some(BatchOutcome::Deferred { + lands: 2, + min_lands: Some(6), + due_ms: Some(SETTLED + 60 * MIN), + }) + ); + } + + #[test] + fn enough_lands_cuts() { + assert_eq!( + defer_cut(6, Some(6), Some(60 * MIN), Some(SETTLED), SETTLED + MIN), + None + ); + assert_eq!(defer_cut(9, Some(6), None, Some(SETTLED), SETTLED), None); + } + + #[test] + fn old_enough_previous_batch_cuts() { + assert_eq!( + defer_cut( + 1, + Some(6), + Some(60 * MIN), + Some(SETTLED), + SETTLED + 60 * MIN + ), + None + ); + assert_eq!( + defer_cut(1, None, Some(60 * MIN), Some(SETTLED), SETTLED + 61 * MIN), + None + ); + } + + #[test] + fn no_previous_batch_cuts() { + assert_eq!(defer_cut(1, Some(6), Some(60 * MIN), None, 0), None); + } + + #[test] + fn unset_thresholds_cut() { + assert_eq!(defer_cut(1, None, None, Some(SETTLED), SETTLED), None); + } + + #[test] + fn forced_batch_says_so_in_the_body() { + let mut h = header(false); + h.fallback = None; + let (plain, _) = render(&h, &[land(1, false)]); + assert!(!plain.contains(FORCED_LINE)); + h.forced = true; + let (forced, message) = render(&h, &[land(1, false)]); + assert!(forced.contains(FORCED_LINE)); + assert!(forced.contains("(forced)")); + assert!(message.contains("forced")); + } +} diff --git a/crates/graft-mirror/src/batch/types.rs b/crates/graft-mirror/src/batch/types.rs index c9b28e07..3aad11e8 100644 --- a/crates/graft-mirror/src/batch/types.rs +++ b/crates/graft-mirror/src/batch/types.rs @@ -172,6 +172,9 @@ pub struct BatchRecord { pub merge_sha: Option, /// The changeset follow imported that merge commit as. pub merged: Option, + /// When a run found the batch merged or closed (ms); `None` while open, + /// and for a batch settled before WI-2719. + pub settled_ms: Option, } /// One per-land commit of a batch (D26). @@ -248,6 +251,16 @@ pub struct BatchConfig { /// checked line batched with this `false` would ship lands whose checks /// have not run. An unimplemented RPC is a warning, not a refusal. pub checked: bool, + /// Cut thresholds (WI-2719): with either set, a batch is cut only when + /// at least `min_lands` lands wait, or the previous batch settled at + /// least `min_age_ms` ago, or the line has no previous batch. Both + /// `None` cuts whenever there is something to batch. + pub min_lands: Option, + /// See [`min_lands`](Self::min_lands) (ms). + pub min_age_ms: Option, + /// Cut regardless of `min_lands` and `min_age_ms` (a release blocker); + /// the PR body says so. + pub force: bool, } impl BatchConfig { @@ -267,6 +280,9 @@ impl BatchConfig { max_lands: None, max_age_ms: None, checked: false, + min_lands: None, + min_age_ms: None, + force: false, } } } @@ -299,6 +315,16 @@ pub enum BatchOutcome { /// The oldest one's author time (ms). oldest_ms: i64, }, + /// Fewer than `min_lands` lands wait and the previous batch settled + /// less than `min_age_ms` ago; no batch yet (WI-2719). + Deferred { + /// Lands waiting. + lands: usize, + /// The land count that makes a batch due, if set. + min_lands: Option, + /// When the age threshold makes a batch due (ms), if set. + due_ms: Option, + }, /// The line is checked and its oldest land's post-land checks are not /// settled (pending, or bad with no revert yet); no batch yet. Unsettled { diff --git a/crates/graft-mirror/src/config.rs b/crates/graft-mirror/src/config.rs index e6b131d4..e8e90091 100644 --- a/crates/graft-mirror/src/config.rs +++ b/crates/graft-mirror/src/config.rs @@ -64,9 +64,10 @@ pub struct FileConfig { /// Emit JSON logs. #[serde(default)] pub json_logs: bool, - /// Changesets per import batch. + /// Changesets per import batch (`batch = 256`), or the `[batch]` table + /// of `pr-batch` cut thresholds (which can also hold the import size). #[serde(default)] - pub batch: Option, + pub batch: Option, /// Commits allowed to fail the fidelity check (40-hex shas). #[serde(default)] pub allow_lossy: Vec, @@ -89,6 +90,48 @@ pub struct FileConfig { pub check_refs: Vec, } +/// The top-level `batch` key: the import size as a count (the original +/// form), or a `[batch]` table. +#[derive(Clone, Debug, Deserialize, PartialEq, Eq)] +#[serde(untagged)] +pub enum BatchKey { + /// `batch = 256`: changesets per import batch. + Size(usize), + /// `[batch]`: `pr-batch` cut thresholds. + Table(BatchFile), +} + +/// `pr-batch` cut thresholds (WI-2719). With the table present, a batch is +/// cut only when at least `min_lands` lands wait, or the previous batch +/// merged or closed at least `min_age_minutes` ago, or there is no previous +/// batch. Without it `pr-batch` cuts whenever there is something to batch. +/// +/// ```toml +/// [batch] +/// min_lands = 6 # default 6 +/// min_age_minutes = 60 # default 60 +/// import_size = 256 # changesets per import batch; replaces `batch = 256` +/// ``` +#[derive(Clone, Debug, Default, Deserialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] +pub struct BatchFile { + /// Lands that make a batch due. Default [`DEFAULT_MIN_LANDS`]. + #[serde(default)] + pub min_lands: Option, + /// Minutes after the previous batch settled that make a batch due. + /// Default [`DEFAULT_MIN_AGE_MINUTES`]. + #[serde(default)] + pub min_age_minutes: Option, + /// Changesets per import batch. + #[serde(default)] + pub import_size: Option, +} + +/// `[batch] min_lands` when the table omits it. +pub const DEFAULT_MIN_LANDS: usize = 6; +/// `[batch] min_age_minutes` when the table omits it. +pub const DEFAULT_MIN_AGE_MINUTES: u64 = 60; + /// One `[[check_refs]]` line. /// /// ```toml @@ -280,6 +323,11 @@ impl FileConfig { } seen.push(line); } + if let Some(BatchKey::Table(t)) = &config.batch + && t.min_lands == Some(0) + { + return Err(bad("[batch] min_lands must be at least 1")); + } if config.poll_interval_secs == Some(0) { return Err(bad("poll_interval_secs must be at least 1")); } @@ -350,6 +398,19 @@ impl FileConfig { .collect() } + /// The `[batch]` cut thresholds `(min_lands, min_age_minutes)`, with + /// their defaults, when the table is present; `None` keeps `pr-batch` + /// cutting whenever there is something to batch. + pub fn batch_cut(&self) -> Option<(usize, u64)> { + match &self.batch { + Some(BatchKey::Table(t)) => Some(( + t.min_lands.unwrap_or(DEFAULT_MIN_LANDS), + t.min_age_minutes.unwrap_or(DEFAULT_MIN_AGE_MINUTES), + )), + _ => None, + } + } + /// The `[export] default_author` identity, when set. pub fn default_author(&self) -> Result, MirrorError> { let Some(raw) = self @@ -437,7 +498,13 @@ impl FileConfig { repo: self.repo.clone(), lines: self.lines(), allow_lossy: parse_shas(&self.allow_lossy)?, - batch: self.batch.unwrap_or(256).max(1), + batch: match &self.batch { + Some(BatchKey::Size(n)) => Some(*n), + Some(BatchKey::Table(t)) => t.import_size, + None => None, + } + .unwrap_or(256) + .max(1), interval: self .poll_interval_secs .map_or(POLL_INTERVAL, Duration::from_secs), @@ -500,6 +567,24 @@ db = "/var/lib/m.sqlite" assert!(FileConfig::parse(&bad).unwrap().default_author().is_err()); } + #[test] + fn batch_table_sets_cut_thresholds_with_defaults() { + let lines = "[[lines]]\nbranch = \"main\"\n"; + let unset = FileConfig::parse(&format!("{BASE}batch = 64\n{lines}")).unwrap(); + assert_eq!(unset.batch_cut(), None); + assert_eq!(unset.follow().unwrap().batch, 64); + let empty = FileConfig::parse(&format!("{BASE}{lines}[batch]\n")).unwrap(); + assert_eq!(empty.batch_cut(), Some((6, 60))); + assert_eq!(empty.follow().unwrap().batch, 256); + let set = FileConfig::parse(&format!( + "{BASE}{lines}[batch]\nmin_lands = 3\nmin_age_minutes = 15\nimport_size = 32\n" + )) + .unwrap(); + assert_eq!(set.batch_cut(), Some((3, 15))); + assert_eq!(set.follow().unwrap().batch, 32); + assert!(FileConfig::parse(&format!("{BASE}{lines}[batch]\nmin_lands = 0\n")).is_err()); + } + #[test] fn defaults_follow_d9() { let c = FileConfig::parse(&format!("{BASE}[[lines]]\nbranch = \"main\"\n")).unwrap(); diff --git a/crates/graft-mirror/src/db.rs b/crates/graft-mirror/src/db.rs index 9931183d..32100555 100644 --- a/crates/graft-mirror/src/db.rs +++ b/crates/graft-mirror/src/db.rs @@ -257,6 +257,7 @@ impl MirrorDb { "initial_lands", "INTEGER NOT NULL DEFAULT 0", )?; + add_column(&conn, "batches", "settled_ms", "INTEGER")?; conn.execute( "UPDATE batches SET initial_lands = lands WHERE initial_lands = 0", [], @@ -917,7 +918,7 @@ impl MirrorDb { .prepare(&format!( "SELECT number, line, base_branch, base, base_sha, start, tip, lands, initial_lands, created_ms, branch, title, body, squash_sha, pr_number, - pr_url, state, detail, merge_sha, merged + pr_url, state, detail, merge_sha, merged, settled_ms FROM batches {filter}" )) .map_err(db_err)?; @@ -944,6 +945,7 @@ impl MirrorDb { detail: r.get(17)?, merge_sha: r.get(18)?, merged: r.get(19)?, + settled_ms: r.get(20)?, }) }) .map_err(db_err)?; @@ -1143,14 +1145,14 @@ fn write_batch(conn: &Connection, rec: &BatchRecord) -> Result<(), GitError> { conn.execute( "INSERT INTO batches (number, line, base_branch, base, base_sha, start, tip, lands, initial_lands, created_ms, branch, title, body, squash_sha, pr_number, pr_url, state, - detail, merge_sha, merged) + detail, merge_sha, merged, settled_ms) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, ?17, - ?18, ?19, ?20) + ?18, ?19, ?20, ?21) ON CONFLICT (number) DO UPDATE SET line = ?2, base_branch = ?3, base = ?4, base_sha = ?5, start = ?6, tip = ?7, lands = ?8, initial_lands = ?9, created_ms = ?10, branch = ?11, title = ?12, body = ?13, squash_sha = ?14, pr_number = ?15, pr_url = ?16, state = ?17, detail = ?18, merge_sha = ?19, - merged = ?20", + merged = ?20, settled_ms = ?21", params![ rec.number as i64, rec.line, @@ -1172,6 +1174,7 @@ fn write_batch(conn: &Connection, rec: &BatchRecord) -> Result<(), GitError> { rec.detail, rec.merge_sha.map(|s| s.0.to_vec()), rec.merged.map(|c| c.0.0.to_vec()), + rec.settled_ms, ], ) .map(drop) @@ -1200,6 +1203,7 @@ struct BatchRow { detail: String, merge_sha: Option>, merged: Option>, + settled_ms: Option, } impl BatchRow { @@ -1227,6 +1231,7 @@ impl BatchRow { detail: self.detail, merge_sha: sha_col(self.merge_sha)?, merged: self.merged.map(id).transpose()?, + settled_ms: self.settled_ms, }) } } diff --git a/crates/graft-mirror/src/entry.rs b/crates/graft-mirror/src/entry.rs index d5fac59d..debd5989 100644 --- a/crates/graft-mirror/src/entry.rs +++ b/crates/graft-mirror/src/entry.rs @@ -55,6 +55,7 @@ const USAGE: &str = "usage: graft-mirror [args] pr-batch --base --config [--track] [--exclude ]... [--reason ] [--max-lands ] [--max-age ] + [--min-lands ] [--min-age ] [--force] [--checks post-land|off] sync an integration line onto the followed base, export its net change @@ -66,7 +67,12 @@ const USAGE: &str = "usage: graft-mirror [args] would ship lands whose checks have not run. An older server that does not implement GetLineConfig warns and - cannot be checked. + cannot be checked. With --min-lands + or --min-age (or [batch] in the + config) a batch is cut only once that + many lands wait or the previous batch + settled that long ago; otherwise the + run is deferred. --force cuts anyway. import-pr /# --github-token-file [--dir ] [--message-out ] [--api ] apply an outside GitHub PR's net @@ -943,6 +949,9 @@ async fn cmd_pr_batch(args: &[String]) -> Result<(), CmdError> { let mut max_lands = None; let mut max_age_ms = None; let mut checked = false; + let mut min_lands = None; + let mut min_age_mins = None; + let mut force = false; let mut it = args.iter(); while let Some(a) = it.next() { let mut value = |flag: &str| { @@ -979,6 +988,23 @@ async fn cmd_pr_batch(args: &[String]) -> Result<(), CmdError> { .ok_or_else(|| CmdError::Usage("--max-age needs seconds".into()))?; max_age_ms = Some(secs.saturating_mul(1000)); } + "--min-lands" => { + let n: usize = value("--min-lands")? + .parse() + .ok() + .filter(|n| *n >= 1) + .ok_or_else(|| { + CmdError::Usage("--min-lands needs a count of at least 1".into()) + })?; + min_lands = Some(n); + } + "--min-age" => { + let mins: u64 = value("--min-age")? + .parse() + .map_err(|_| CmdError::Usage("--min-age needs minutes".into()))?; + min_age_mins = Some(mins); + } + "--force" => force = true, "--checks" => match value("--checks")?.as_str() { "post-land" => checked = true, "off" => checked = false, @@ -1020,6 +1046,16 @@ async fn cmd_pr_batch(args: &[String]) -> Result<(), CmdError> { batch.max_lands = max_lands; batch.max_age_ms = max_age_ms; batch.checked = checked; + // A flag overrides `[batch]`; setting either flag turns the thresholds + // on, as the table does. + let file_cut = file.batch_cut(); + if min_lands.is_some() || min_age_mins.is_some() || file_cut.is_some() { + batch.min_lands = min_lands.or(file_cut.map(|c| c.0)); + batch.min_age_ms = min_age_mins + .or(file_cut.map(|c| c.1)) + .map(|m| i64::try_from(m.saturating_mul(60_000)).unwrap_or(i64::MAX)); + } + batch.force = force; with_credentials( file.github.as_ref(), RunBatch { @@ -1110,6 +1146,11 @@ impl AsyncCredentialFn<()> for RunBatch<'_> { not due for a batch yet" ); } + BatchOutcome::Deferred { + lands, + min_lands, + due_ms, + } => println!("{}", deferred_line(lands, min_lands, due_ms)), BatchOutcome::Unsettled { lands, waiting } => { println!( "line {line}: {lands} land(s) waiting; post-land checks of {waiting} \ @@ -1121,6 +1162,40 @@ impl AsyncCredentialFn<()> for RunBatch<'_> { } } +/// `pr-batch`'s report of a batch held back by the cut thresholds. +fn deferred_line(lands: usize, min_lands: Option, due_ms: Option) -> String { + let at = due_ms.map(utc); + let next = match (at, min_lands) { + (Some(at), Some(n)) => format!("{at} or {n} lands"), + (Some(at), None) => at, + (None, Some(n)) => format!("{n} lands"), + (None, None) => "the next run".to_owned(), + }; + format!("batch deferred: {lands} land(s) waiting, next cut at {next}") +} + +/// `YYYY-MM-DDTHH:MM:SSZ` for `ms` since the epoch. +fn utc(ms: i64) -> String { + let secs = ms.div_euclid(1000); + let (days, day) = (secs.div_euclid(86_400), secs.rem_euclid(86_400)); + // Howard Hinnant's civil_from_days. + let z = days + 719_468; + let era = z.div_euclid(146_097); + let doe = z.rem_euclid(146_097); + let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let d = doy - (153 * mp + 2) / 5 + 1; + let m = if mp < 10 { mp + 3 } else { mp - 9 }; + let y = yoe + era * 400 + i64::from(m <= 2); + format!( + "{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}Z", + day / 3600, + day % 3600 / 60, + day % 60 + ) +} + fn print_batch(rec: &BatchRecord) { println!( "batch {} ({}): {} land(s), {} on {} at {}", @@ -1382,6 +1457,25 @@ async fn shutdown() { } } +#[cfg(test)] +mod deferred_tests { + use super::{deferred_line, utc}; + + #[test] + fn deferred_line_names_the_next_cut() { + assert_eq!(utc(0), "1970-01-01T00:00:00Z"); + assert_eq!(utc(1_793_448_000_000), "2026-10-31T12:00:00Z"); + assert_eq!( + deferred_line(2, Some(6), Some(1_793_448_000_000)), + "batch deferred: 2 land(s) waiting, next cut at 2026-10-31T12:00:00Z or 6 lands" + ); + assert_eq!( + deferred_line(1, Some(6), None), + "batch deferred: 1 land(s) waiting, next cut at 6 lands" + ); + } +} + #[cfg(test)] mod tests { use std::fs; diff --git a/crates/graft-mirror/src/sample_mirror.toml b/crates/graft-mirror/src/sample_mirror.toml index 5817e666..d02ba88b 100644 --- a/crates/graft-mirror/src/sample_mirror.toml +++ b/crates/graft-mirror/src/sample_mirror.toml @@ -20,7 +20,8 @@ poll_interval_secs = 15 metrics_addr = "127.0.0.1:9465" # Emit logs as JSON lines instead of text. json_logs = false -# Changesets sent in one import batch. Default 256. +# Changesets sent in one import batch. Default 256. To set the pr-batch +# thresholds below as well, drop this line and use [batch] import_size. batch = 256 # Upstream commits (40 hex characters) allowed to fail the fidelity check. allow_lossy = [] @@ -42,6 +43,16 @@ line = "main" # line = "backlog-1" # prefix = "graft/check/backlog-1/" +# pr-batch cut thresholds. With this table, pr-batch cuts a batch only when +# min_lands lands wait, or the previous batch merged or closed min_age_minutes +# ago, or there is no previous batch; otherwise it reports "batch deferred" +# and exits 0. --min-lands, --min-age and --force override it. Omit the table +# to cut whenever there is something to batch. +# [batch] +# min_lands = 6 # default 6 +# min_age_minutes = 60 # default 60 +# import_size = 256 # replaces the top-level `batch = 256` + # GitHub repository for pr-batch, pull import and the authority switch. # Omit the whole section to follow and import only. [github] diff --git a/crates/graft-mirror/tests/batch.rs b/crates/graft-mirror/tests/batch.rs index a4e57817..8f3cf3a9 100644 --- a/crates/graft-mirror/tests/batch.rs +++ b/crates/graft-mirror/tests/batch.rs @@ -3230,3 +3230,50 @@ async fn a_merged_batch_closes_the_outside_pr_it_re_landed_with_credit() { assert!(!w.github.prs()[0].open); assert!(w.github.prs()[1].open, "a graft batch PR is left open"); } + +#[tokio::test] +async fn min_lands_defers_a_small_batch_after_a_recent_one_until_forced() { + let w = World::new().await; + w.start().await; + let thresholds = |c: &mut BatchConfig| { + c.min_lands = Some(3); + c.min_age_ms = Some(i64::MAX); + }; + // No previous batch: the first is cut at once. + w.land(&[("a.txt", "a1\n")], "add a", 1).await; + let rec = opened( + w.batcher_with(&w.db, thresholds) + .run() + .await + .unwrap() + .outcome, + ); + assert_eq!(rec.lands, 1); + w.github.squash_merge(1); + // The previous batch just merged and one land waits: deferred. + w.land(&[("b.txt", "b1\n")], "add b", 2).await; + let run = w.batcher_with(&w.db, thresholds).run().await.unwrap(); + assert_eq!(run.settled.unwrap().state, BatchState::Merged); + assert!(matches!( + run.outcome, + BatchOutcome::Deferred { + lands: 1, + min_lands: Some(3), + .. + } + )); + assert_eq!(w.github.prs().len(), 1); + // --force cuts anyway and says so. + let rec = opened( + w.batcher_with(&w.db, |c| { + thresholds(c); + c.force = true; + }) + .run() + .await + .unwrap() + .outcome, + ); + assert_eq!(rec.lands, 1); + assert!(rec.body.contains("(forced)")); +} diff --git a/docs/guide/git-migration.md b/docs/guide/git-migration.md index 339c945f..3841d70c 100644 --- a/docs/guide/git-migration.md +++ b/docs/guide/git-migration.md @@ -95,6 +95,28 @@ commit in the batch. `graft-mirror` creates or updates the pull request but never merges it. Review and merge that pull request with your normal process; afterward, `follow` imports the resulting GitHub history and settles the batch. +To avoid many small batches, set cut thresholds in `mirror.toml`: + +```toml +[batch] +min_lands = 6 # default 6 +min_age_minutes = 60 # default 60 +import_size = 256 # optional; replaces a top-level `batch = 256` +``` + +With the `[batch]` table (or the `--min-lands N` / `--min-age MINUTES` +flags, which override it), `pr-batch` cuts a batch only when at least +`min_lands` lands are waiting, or the previous batch merged or was closed at +least `min_age_minutes` ago, or the line has no previous batch. Otherwise it +opens nothing, exits 0 and prints +`batch deferred: land(s) waiting, next cut at