From fa68b598268229f7cd83f60d4a9079d4a81d9f65 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Mon, 9 Nov 2026 15:26:27 +0200 Subject: [PATCH 1/2] feat(forge): retire a migrated repository's pull request numbers sc-forge admin repo set-pull-floor sets a per-repository floor (schema migration 6); both pull-number allocation paths take the larger of the floor and the highest existing number, plus one. A repository migrated from Gitea then never gives out an old number again, so a stored .../pulls/N answers 404 instead of naming an unrelated pull request (Codento Engine ADR-0080, COD2-7135). Also gives each pulls unit test its own temp directory (two tests in one second shared one). --- crates/forge/src/admin.rs | 17 ++++++++++++++ crates/forge/src/cli.rs | 31 ++++++++++++++++++++++++++ crates/forge/src/db.rs | 3 +++ crates/forge/src/pulls.rs | 39 +++++++++++++++++++++++---------- crates/forge/tests/e2e_pulls.rs | 15 +++++++++++++ docs/FORGE-OPERATIONS.md | 9 ++++++++ 6 files changed, 103 insertions(+), 11 deletions(-) diff --git a/crates/forge/src/admin.rs b/crates/forge/src/admin.rs index d61b1cd9..9336e03e 100644 --- a/crates/forge/src/admin.rs +++ b/crates/forge/src/admin.rs @@ -85,6 +85,23 @@ pub fn generate_access_token( } } +/// `admin repo set-pull-floor`: new pull requests of `owner/name` are numbered above `floor`, so +/// the numbers a repository had on the forge it was migrated from are never given out again. +pub fn repo_set_pull_floor(data_dir: &Path, owner: &str, name: &str, floor: i64) -> Result<()> { + if floor < 0 { + bail!("pull floor must not be negative"); + } + let conn = db::open_for_admin(data_dir)?; + let repo = crate::repos::find_row(&conn, owner, name)? + .filter(|r| r.state == "live") + .ok_or_else(|| anyhow!("repository does not exist [{owner}/{name}]"))?; + conn.execute( + "UPDATE repos SET pull_floor = ?2 WHERE id = ?1", + rusqlite::params![repo.id, floor], + )?; + Ok(()) +} + /// `admin org create`. pub fn org_create(data_dir: &Path, name: &str, owner: &str) -> Result<()> { if names::check(name).is_err() { diff --git a/crates/forge/src/cli.rs b/crates/forge/src/cli.rs index 85c2f018..46a8b524 100644 --- a/crates/forge/src/cli.rs +++ b/crates/forge/src/cli.rs @@ -122,6 +122,25 @@ enum AdminCmd { #[command(subcommand)] cmd: OrgCmd, }, + Repo { + #[command(subcommand)] + cmd: RepoCmd, + }, +} + +#[derive(Subcommand)] +enum RepoCmd { + /// Number new pull requests above FLOOR (a migrated repository's highest old number). + SetPullFloor { + #[arg(long)] + owner: String, + #[arg(long)] + name: String, + #[arg(long)] + floor: i64, + #[arg(long)] + data_dir: PathBuf, + }, } #[derive(Subcommand)] @@ -259,6 +278,18 @@ fn run_admin(cmd: AdminCmd) -> Result<()> { admin::org_create(&data_dir, &name, &owner)?; println!("Organization '{name}' has been successfully created!"); } + AdminCmd::Repo { + cmd: + RepoCmd::SetPullFloor { + owner, + name, + floor, + data_dir, + }, + } => { + admin::repo_set_pull_floor(&data_dir, &owner, &name, floor)?; + println!("Pull requests of {owner}/{name} are numbered above {floor}"); + } } Ok(()) } diff --git a/crates/forge/src/db.rs b/crates/forge/src/db.rs index f7e5438a..f64bafe4 100644 --- a/crates/forge/src/db.rs +++ b/crates/forge/src/db.rs @@ -190,6 +190,9 @@ const MIGRATIONS: &[&str] = &[ created_unix INTEGER NOT NULL ); CREATE UNIQUE INDEX merge_intents_one_pending ON merge_intents (repo_id, pull_number) WHERE state = 'pending';", + // 6: a repository migrated from another forge keeps its old pull request numbers retired: new + // ones start above `pull_floor` (set by `sc-forge admin repo set-pull-floor`). + "ALTER TABLE repos ADD COLUMN pull_floor INTEGER NOT NULL DEFAULT 0;", ]; /// Every table the current schema creates; `/healthz` probes each, so a missing or unreadable one diff --git a/crates/forge/src/pulls.rs b/crates/forge/src/pulls.rs index 3fd3394e..0f6def21 100644 --- a/crates/forge/src/pulls.rs +++ b/crates/forge/src/pulls.rs @@ -175,13 +175,14 @@ pub fn list( Ok((rows, total)) } +/// One above both the repository's highest PR number and its pull-number floor. +const NEXT_NUMBER: &str = + "SELECT MAX(COALESCE((SELECT MAX(number) FROM pulls WHERE repo_id = ?1), 0), \ + COALESCE((SELECT pull_floor FROM repos WHERE id = ?1), 0)) + 1"; + /// The number the next PR of the repository gets. pub fn next_number(conn: &Connection, repo_id: i64) -> rusqlite::Result { - conn.query_row( - "SELECT COALESCE(MAX(number), 0) + 1 FROM pulls WHERE repo_id = ?1", - [repo_id], - |r| r.get(0), - ) + conn.query_row(NEXT_NUMBER, [repo_id], |r| r.get(0)) } /// Insert a PR with the next number of the repository (PRs and, one day, issues share it). @@ -198,11 +199,7 @@ pub fn insert( now: i64, ) -> rusqlite::Result { let tx = Transaction::new_unchecked(conn, TransactionBehavior::Immediate)?; - let number: i64 = tx.query_row( - "SELECT COALESCE(MAX(number), 0) + 1 FROM pulls WHERE repo_id = ?1", - [repo_id], - |r| r.get(0), - )?; + let number: i64 = tx.query_row(NEXT_NUMBER, [repo_id], |r| r.get(0))?; tx.execute( "INSERT INTO pulls (repo_id, number, head_ref, base_ref, base_sha, head_sha, title, body, \ state, creator_id, created_unix, updated_unix) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, 'open', ?9, ?10, ?10)", @@ -757,10 +754,11 @@ mod tests { use super::*; fn conn() -> (Connection, std::path::PathBuf, i64, i64) { + static N: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); let d = std::env::temp_dir().join(format!( "scl-forge-pulls-{}-{}", std::process::id(), - crate::timefmt::now() + N.fetch_add(1, std::sync::atomic::Ordering::SeqCst) )); let _ = std::fs::remove_dir_all(&d); let conn = crate::db::open_for_admin(&d).unwrap(); @@ -778,6 +776,25 @@ mod tests { (conn, d, repo.id, owner.id) } + #[test] + fn a_pull_floor_retires_every_number_up_to_it() { + let (c, d, repo, user) = conn(); + c.execute("UPDATE repos SET pull_floor = 41 WHERE id = ?1", [repo]) + .unwrap(); + assert_eq!(next_number(&c, repo).unwrap(), 42); + let a = insert(&c, repo, "f", "main", "b1", "h1", "t", "", user, 5).unwrap(); + assert_eq!(a.number, 42); + c.execute("UPDATE repos SET pull_floor = 10 WHERE id = ?1", [repo]) + .unwrap(); + let b = insert(&c, repo, "g", "main", "b1", "h2", "t", "", user, 6).unwrap(); + assert_eq!( + b.number, 43, + "a floor below the highest number changes nothing" + ); + assert_eq!(next_number(&c, repo).unwrap(), 44); + let _ = std::fs::remove_dir_all(&d); + } + #[test] fn numbers_are_sequential_and_one_open_pull_per_target_pair() { let (c, d, repo, user) = conn(); diff --git a/crates/forge/tests/e2e_pulls.rs b/crates/forge/tests/e2e_pulls.rs index 69f57c16..1f270233 100644 --- a/crates/forge/tests/e2e_pulls.rs +++ b/crates/forge/tests/e2e_pulls.rs @@ -158,6 +158,21 @@ fn create_validates_and_the_object_has_the_recorded_shape() { assert_eq!(r.json()["number"], 2); } +#[test] +fn a_pull_floor_set_on_a_running_server_retires_the_old_numbers() { + let w = world(); + w.commit("f1", "f1.txt", Some("main")); + scl_forge::admin::repo_set_pull_floor(&w.s.dir, "siteadmin", "w", 41).unwrap(); + assert_eq!(w.open("f1")["number"], 42); + assert_eq!( + w.api("GET", "/pulls/1", None).status, + 404, + "an old number names nothing" + ); + assert!(scl_forge::admin::repo_set_pull_floor(&w.s.dir, "siteadmin", "nope", 1).is_err()); + assert!(scl_forge::admin::repo_set_pull_floor(&w.s.dir, "siteadmin", "w", -1).is_err()); +} + #[test] fn the_head_follows_the_branch_and_lists_are_ordered_and_paged() { let w = world(); diff --git a/docs/FORGE-OPERATIONS.md b/docs/FORGE-OPERATIONS.md index 961b97cf..5874d12e 100644 --- a/docs/FORGE-OPERATIONS.md +++ b/docs/FORGE-OPERATIONS.md @@ -22,6 +22,15 @@ sc-forge admin user create --data-dir /data --username --password - sc-forge admin user generate-access-token --data-dir /data --username --token-name bootstrap --scopes all --raw ``` +Migrating a repository from another forge: after pushing its branches and tags, retire its old pull request numbers so +references to them answer 404 instead of naming a new pull request (works while the server runs): + +```sh +sc-forge admin repo set-pull-floor --data-dir /data --owner --name --floor +``` + +This adds schema version 6 (the `pull_floor` column); an older binary refuses a data directory this build has opened. + ## Serving ```sh From 739ba62c9e96845e79f1a965e3543b99b65c09b1 Mon Sep 17 00:00:00 2001 From: Toni Bergholm Date: Mon, 9 Nov 2026 15:29:58 +0200 Subject: [PATCH 2/2] fix(forge): a pull floor only rises --- crates/forge/src/admin.rs | 5 +++-- crates/forge/tests/e2e_pulls.rs | 7 ++++++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/forge/src/admin.rs b/crates/forge/src/admin.rs index 9336e03e..2db53387 100644 --- a/crates/forge/src/admin.rs +++ b/crates/forge/src/admin.rs @@ -86,7 +86,8 @@ pub fn generate_access_token( } /// `admin repo set-pull-floor`: new pull requests of `owner/name` are numbered above `floor`, so -/// the numbers a repository had on the forge it was migrated from are never given out again. +/// the numbers a repository had on the forge it was migrated from are never given out again. The +/// floor only rises: a lower value leaves it as it is. pub fn repo_set_pull_floor(data_dir: &Path, owner: &str, name: &str, floor: i64) -> Result<()> { if floor < 0 { bail!("pull floor must not be negative"); @@ -96,7 +97,7 @@ pub fn repo_set_pull_floor(data_dir: &Path, owner: &str, name: &str, floor: i64) .filter(|r| r.state == "live") .ok_or_else(|| anyhow!("repository does not exist [{owner}/{name}]"))?; conn.execute( - "UPDATE repos SET pull_floor = ?2 WHERE id = ?1", + "UPDATE repos SET pull_floor = MAX(pull_floor, ?2) WHERE id = ?1", rusqlite::params![repo.id, floor], )?; Ok(()) diff --git a/crates/forge/tests/e2e_pulls.rs b/crates/forge/tests/e2e_pulls.rs index 1f270233..dc1f09a4 100644 --- a/crates/forge/tests/e2e_pulls.rs +++ b/crates/forge/tests/e2e_pulls.rs @@ -163,7 +163,12 @@ fn a_pull_floor_set_on_a_running_server_retires_the_old_numbers() { let w = world(); w.commit("f1", "f1.txt", Some("main")); scl_forge::admin::repo_set_pull_floor(&w.s.dir, "siteadmin", "w", 41).unwrap(); - assert_eq!(w.open("f1")["number"], 42); + scl_forge::admin::repo_set_pull_floor(&w.s.dir, "siteadmin", "w", 10).unwrap(); + assert_eq!( + w.open("f1")["number"], + 42, + "a lower floor does not reopen retired numbers" + ); assert_eq!( w.api("GET", "/pulls/1", None).status, 404,