config PRs: remove the hive's config-PR webhook, poll and core merge
An operator's merge on the forge deploys a config PR through
swarm-controller's DeployRequest{rev}. The hive-side path that queued a
MergeConfigPr approval and merged the PR as `core` goes:
- the `/webhook/config-pr` receiver, its HMAC secret, the WebhookRegister
boot node and the org-hook registration; the hive vhost's `/webhook/`
location
- the 5-minute config-PR poll
- ApprovalKind::MergeConfigPr, its dashboard card, and the deploy DAG it
drove (DeployWindow, MergeVerify, DeployApply, FinalizeDeploy,
DeployTail), with verify_commit, the two-phase meta deploy, rollback
refs, the PR-failure comment and forge/pr_merge.rs
- `fetched_sha`, `sha_short`/`pr_number` on approval events, and
`sha`/`tag` on HelperEvent::ApprovalResolved: only the merge path set
them
`config_repo`, `merged_pr_for_commit` and `post_pr_comment` move to
forge/pr_comment.rs for the merged-rev deploy's refusal comment.
Approvals v5 drops stored `merge_config_pr` rows; a test reopens a v4
database holding them.
Closes #4850
This commit is contained in:
parent
a5ea015bc6
commit
efbfec6d01
39 changed files with 286 additions and 3199 deletions
|
|
@ -1,7 +1,6 @@
|
|||
//! Approval queue. Requests are submitted by an agent
|
||||
//! (`RequestSchedulePrompt`) or the config-PR webhook (`MergeConfigPr`); the
|
||||
//! user approves/denies via the host admin CLI; on approval the host runs the
|
||||
//! corresponding action.
|
||||
//! (`RequestSchedulePrompt`); the user approves/denies via the host admin
|
||||
//! CLI; on approval the host runs the corresponding action.
|
||||
//!
|
||||
//! `UpdateMetaInputs` rows are legacy: the MCP tool that queued them was
|
||||
//! removed and nothing produces the kind any more. The variant and
|
||||
|
|
@ -58,6 +57,13 @@ const MIGRATIONS: &[Migration] = &[
|
|||
sql: "ALTER TABLE approvals ADD COLUMN submitter TEXT",
|
||||
adds_column: Some(("approvals", "submitter")),
|
||||
},
|
||||
// v5: drop `merge_config_pr` rows. Config PRs merge on the forge, so no
|
||||
// approval of that kind can be acted on, and `row_to_approval` rejects
|
||||
// the kind.
|
||||
Migration {
|
||||
sql: "DELETE FROM approvals WHERE kind = 'merge_config_pr'",
|
||||
adds_column: None,
|
||||
},
|
||||
];
|
||||
|
||||
pub struct Approvals {
|
||||
|
|
@ -75,10 +81,7 @@ impl Approvals {
|
|||
})
|
||||
}
|
||||
|
||||
/// Insert a new pending approval row. `fetched_sha` may be supplied
|
||||
/// when the sha is already known at submission time (e.g. `MergeConfigPr`
|
||||
/// fetches the PR head before inserting), making the insert + sha-set
|
||||
/// atomic. Pass `None` when the kind carries no sha (e.g. `SchedulePrompt`).
|
||||
/// Insert a new pending approval row.
|
||||
pub fn submit_kind(
|
||||
&self,
|
||||
agent: &str,
|
||||
|
|
@ -86,14 +89,12 @@ impl Approvals {
|
|||
commit_ref: &str,
|
||||
description: Option<&str>,
|
||||
submitter: &str,
|
||||
fetched_sha: Option<&str>,
|
||||
) -> Result<i64> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
conn.execute(
|
||||
"INSERT INTO approvals
|
||||
(agent, kind, commit_ref, requested_at, status, description, submitter,
|
||||
fetched_sha)
|
||||
VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6, ?7)",
|
||||
(agent, kind, commit_ref, requested_at, status, description, submitter)
|
||||
VALUES (?1, ?2, ?3, ?4, 'pending', ?5, ?6)",
|
||||
params![
|
||||
agent,
|
||||
<&str>::from(kind),
|
||||
|
|
@ -101,7 +102,6 @@ impl Approvals {
|
|||
Utc::now().timestamp(),
|
||||
description,
|
||||
submitter,
|
||||
fetched_sha,
|
||||
],
|
||||
)?;
|
||||
Ok(conn.last_insert_rowid())
|
||||
|
|
@ -128,37 +128,12 @@ impl Approvals {
|
|||
Ok(submitter)
|
||||
}
|
||||
|
||||
/// Return the `(id, fetched_sha)` of the pending `merge_config_pr`
|
||||
/// approval for `(agent, pr_number)`, if one exists. Drives
|
||||
/// `submit_merge_config_pr`'s idempotency + PR-drift handling: same
|
||||
/// `fetched_sha` → no new request (the webhook + poll both call submit,
|
||||
/// so re-submits of an unchanged PR must be no-ops); a drifted head →
|
||||
/// cancel this stale row and queue a fresh approval.
|
||||
pub fn pending_merge_config_pr(
|
||||
&self,
|
||||
agent: &str,
|
||||
pr_number: u64,
|
||||
) -> Result<Option<(i64, Option<String>)>> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
let row = conn
|
||||
.query_row(
|
||||
"SELECT id, fetched_sha FROM approvals \
|
||||
WHERE agent = ?1 AND kind = 'merge_config_pr' \
|
||||
AND commit_ref = ?2 AND status = 'pending' \
|
||||
ORDER BY id DESC LIMIT 1",
|
||||
params![agent, pr_number.to_string()],
|
||||
|row| Ok((row.get(0)?, row.get(1)?)),
|
||||
)
|
||||
.optional()?;
|
||||
Ok(row)
|
||||
}
|
||||
|
||||
/// Last `limit` resolved approvals (approved / denied / failed),
|
||||
/// newest-first. Drives the history tab on the dashboard.
|
||||
pub fn recent_resolved(&self, limit: u64) -> Result<Vec<Approval>> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description
|
||||
FROM approvals
|
||||
WHERE status IN ('approved', 'denied', 'failed', 'cancelled')
|
||||
ORDER BY resolved_at DESC, id DESC
|
||||
|
|
@ -171,7 +146,7 @@ impl Approvals {
|
|||
pub fn pending(&self) -> Result<Vec<Approval>> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description
|
||||
FROM approvals
|
||||
WHERE status = 'pending'
|
||||
ORDER BY id ASC",
|
||||
|
|
@ -183,7 +158,7 @@ impl Approvals {
|
|||
pub fn get(&self, id: i64) -> Result<Option<Approval>> {
|
||||
let conn = self.conn.lock().unwrap();
|
||||
conn.query_row(
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description
|
||||
"SELECT id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description
|
||||
FROM approvals WHERE id = ?1",
|
||||
params![id],
|
||||
row_to_approval,
|
||||
|
|
@ -223,7 +198,6 @@ impl Approvals {
|
|||
status: ApprovalStatus::Approved,
|
||||
resolved_at: Some(hive_sh4re::wire_time::from_secs(resolved_at)),
|
||||
note: None,
|
||||
fetched_sha: row.fetched_sha,
|
||||
description: row.description,
|
||||
})
|
||||
}
|
||||
|
|
@ -252,7 +226,7 @@ impl Approvals {
|
|||
|
||||
/// Withdraw a pending approval. Returns the now-updated
|
||||
/// row so the caller can emit `ApprovalResolved` with the right
|
||||
/// kind / agent / sha. Errors if the approval isn't pending — once
|
||||
/// kind / agent. Errors if the approval isn't pending — once
|
||||
/// it's approved/denied/failed/cancelled, the resolution is final.
|
||||
pub fn mark_cancelled(&self, id: i64, canceller: &str) -> Result<Approval> {
|
||||
let mut conn = self.conn.lock().unwrap();
|
||||
|
|
@ -286,7 +260,6 @@ impl Approvals {
|
|||
status: ApprovalStatus::Cancelled,
|
||||
resolved_at: Some(hive_sh4re::wire_time::from_secs(resolved_at)),
|
||||
note: Some(note),
|
||||
fetched_sha: row.fetched_sha,
|
||||
description: row.description,
|
||||
})
|
||||
}
|
||||
|
|
@ -315,15 +288,14 @@ struct ApprovalLookup {
|
|||
commit_ref: String,
|
||||
requested_at: i64,
|
||||
status: String,
|
||||
fetched_sha: Option<String>,
|
||||
description: Option<String>,
|
||||
}
|
||||
|
||||
impl ApprovalLookup {
|
||||
/// The single-row lookup by id (`?1`). Column order matches
|
||||
/// [`ApprovalLookup::from_row`].
|
||||
const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, fetched_sha, \
|
||||
description FROM approvals WHERE id = ?1";
|
||||
const SELECT: &str = "SELECT agent, kind, commit_ref, requested_at, status, description \
|
||||
FROM approvals WHERE id = ?1";
|
||||
|
||||
fn from_row(row: &rusqlite::Row<'_>) -> rusqlite::Result<Self> {
|
||||
let agent: String = row.get(0)?;
|
||||
|
|
@ -340,8 +312,7 @@ impl ApprovalLookup {
|
|||
commit_ref: row.get(2)?,
|
||||
requested_at: row.get(3)?,
|
||||
status: row.get(4)?,
|
||||
fetched_sha: row.get(5)?,
|
||||
description: row.get(6)?,
|
||||
description: row.get(5)?,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -379,12 +350,11 @@ fn collect_lenient(rows: impl Iterator<Item = rusqlite::Result<Approval>>) -> Ve
|
|||
}
|
||||
|
||||
fn row_to_approval(row: &rusqlite::Row<'_>) -> rusqlite::Result<Approval> {
|
||||
// Column order: id, agent, kind, commit_ref, requested_at, status, resolved_at, note, fetched_sha, description.
|
||||
// Column order: id, agent, kind, commit_ref, requested_at, status, resolved_at, note, description.
|
||||
let kind: String = row.get(2)?;
|
||||
let kind = match kind.as_str() {
|
||||
"update_meta_inputs" => ApprovalKind::UpdateMetaInputs,
|
||||
"schedule_prompt" => ApprovalKind::SchedulePrompt,
|
||||
"merge_config_pr" => ApprovalKind::MergeConfigPr,
|
||||
other => {
|
||||
return Err(rusqlite::Error::FromSqlConversionFailure(
|
||||
2,
|
||||
|
|
@ -427,8 +397,7 @@ fn row_to_approval(row: &rusqlite::Row<'_>) -> rusqlite::Result<Approval> {
|
|||
.get::<_, Option<i64>>(6)?
|
||||
.map(hive_sh4re::wire_time::from_secs),
|
||||
note: row.get(7)?,
|
||||
fetched_sha: row.get(8)?,
|
||||
description: row.get(9)?,
|
||||
description: row.get(8)?,
|
||||
})
|
||||
}
|
||||
|
||||
|
|
@ -436,7 +405,6 @@ fn kind_from_str(s: &str) -> Result<ApprovalKind> {
|
|||
Ok(match s {
|
||||
"update_meta_inputs" => ApprovalKind::UpdateMetaInputs,
|
||||
"schedule_prompt" => ApprovalKind::SchedulePrompt,
|
||||
"merge_config_pr" => ApprovalKind::MergeConfigPr,
|
||||
other => bail!("unknown approval kind '{other}'"),
|
||||
})
|
||||
}
|
||||
|
|
@ -456,21 +424,12 @@ mod tests {
|
|||
#[test]
|
||||
fn mixed_kinds_all_listed() {
|
||||
let (_dir, _path, db) = open_temp();
|
||||
db.submit_kind(
|
||||
"a",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"deadbeef",
|
||||
None,
|
||||
"a",
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
db.submit_kind("b", ApprovalKind::SchedulePrompt, "", None, "b", None)
|
||||
db.submit_kind("b", ApprovalKind::SchedulePrompt, "", None, "b")
|
||||
.unwrap();
|
||||
db.submit_kind("c", ApprovalKind::UpdateMetaInputs, "[]", None, "c", None)
|
||||
db.submit_kind("c", ApprovalKind::UpdateMetaInputs, "[]", None, "c")
|
||||
.unwrap();
|
||||
let pending = db.pending().expect("pending");
|
||||
assert_eq!(pending.len(), 3, "all three kinds must be visible");
|
||||
assert_eq!(pending.len(), 2, "both kinds must be visible");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
@ -482,11 +441,10 @@ mod tests {
|
|||
let id = db
|
||||
.submit_kind(
|
||||
"bitburner",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"cafef00d",
|
||||
ApprovalKind::SchedulePrompt,
|
||||
"{}",
|
||||
Some("test"),
|
||||
"bitburner",
|
||||
None,
|
||||
)
|
||||
.unwrap();
|
||||
let row = db.mark_cancelled(id, "manager").expect("cancel");
|
||||
|
|
@ -506,14 +464,7 @@ mod tests {
|
|||
// final — re-cancelling errors instead of silently overwriting.
|
||||
let (_dir, _path, db) = open_temp();
|
||||
let id = db
|
||||
.submit_kind(
|
||||
"a",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"deadbeef",
|
||||
None,
|
||||
"a",
|
||||
None,
|
||||
)
|
||||
.submit_kind("a", ApprovalKind::SchedulePrompt, "{}", None, "a")
|
||||
.unwrap();
|
||||
db.mark_cancelled(id, "manager").expect("first cancel");
|
||||
let err = db
|
||||
|
|
@ -528,14 +479,7 @@ mod tests {
|
|||
// whole list — collect_lenient skips it instead of failing.
|
||||
let (_dir, path, db) = open_temp();
|
||||
let good = db
|
||||
.submit_kind(
|
||||
"good",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"cafe",
|
||||
None,
|
||||
"good",
|
||||
None,
|
||||
)
|
||||
.submit_kind("good", ApprovalKind::SchedulePrompt, "{}", None, "good")
|
||||
.unwrap();
|
||||
let raw = Connection::open(&path).unwrap();
|
||||
raw.execute(
|
||||
|
|
@ -558,21 +502,14 @@ mod tests {
|
|||
// fall back to the root agent.
|
||||
let (_dir, path, db) = open_temp();
|
||||
let id = db
|
||||
.submit_kind(
|
||||
"child",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"cafe",
|
||||
None,
|
||||
"parent",
|
||||
None,
|
||||
)
|
||||
.submit_kind("child", ApprovalKind::SchedulePrompt, "{}", None, "parent")
|
||||
.unwrap();
|
||||
assert_eq!(db.submitter_of(id).unwrap().as_deref(), Some("parent"));
|
||||
|
||||
let raw = Connection::open(&path).unwrap();
|
||||
raw.execute(
|
||||
"INSERT INTO approvals (agent, kind, commit_ref, requested_at, status)
|
||||
VALUES ('old', 'merge_config_pr', '', 0, 'pending')",
|
||||
VALUES ('old', 'schedule_prompt', '', 0, 'pending')",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
|
|
@ -580,24 +517,33 @@ mod tests {
|
|||
assert_eq!(db.submitter_of(legacy_id).unwrap(), None);
|
||||
}
|
||||
|
||||
/// A database written before v5 still holds `merge_config_pr` rows,
|
||||
/// pending and resolved. Opening it drops them and keeps every other row,
|
||||
/// so the lists and `get` read cleanly.
|
||||
#[test]
|
||||
fn fetched_sha_in_insert_is_readable_via_get() {
|
||||
// `submit_kind` with `Some(sha)` must store it atomically in the
|
||||
// INSERT — the `get()` row must reflect it without a separate
|
||||
// sha-set step. This is the MergeConfigPr path.
|
||||
let (_dir, _path, db) = open_temp();
|
||||
let sha = "abc1234567890abc1234567890abc1234567890ab";
|
||||
let id = db
|
||||
.submit_kind(
|
||||
"janet",
|
||||
ApprovalKind::MergeConfigPr,
|
||||
"42",
|
||||
None,
|
||||
"ruth",
|
||||
Some(sha),
|
||||
)
|
||||
fn a_pre_v5_merge_config_pr_row_is_dropped_on_open() {
|
||||
let (_dir, path, db) = open_temp();
|
||||
let kept = db
|
||||
.submit_kind("iris", ApprovalKind::SchedulePrompt, "{}", None, "iris")
|
||||
.unwrap();
|
||||
let row = db.get(id).unwrap().expect("row must exist");
|
||||
assert_eq!(row.fetched_sha.as_deref(), Some(sha));
|
||||
drop(db);
|
||||
let raw = Connection::open(&path).unwrap();
|
||||
raw.execute_batch(
|
||||
"INSERT INTO approvals (agent, kind, commit_ref, requested_at, status, fetched_sha)
|
||||
VALUES ('iris', 'merge_config_pr', '42', 0, 'pending', 'abc123');
|
||||
INSERT INTO approvals (agent, kind, commit_ref, requested_at, status, resolved_at)
|
||||
VALUES ('iris', 'merge_config_pr', '41', 0, 'approved', 1);
|
||||
UPDATE schema_versions SET version = 4 WHERE store = 'approvals';",
|
||||
)
|
||||
.unwrap();
|
||||
let old: i64 = raw.last_insert_rowid();
|
||||
drop(raw);
|
||||
|
||||
let db = Approvals::open(&path).expect("a pre-v5 db opens");
|
||||
let pending = db.pending().expect("pending");
|
||||
assert_eq!(pending.len(), 1);
|
||||
assert_eq!(pending[0].id, kept);
|
||||
assert!(db.recent_resolved(10).unwrap().is_empty());
|
||||
assert!(db.get(old).unwrap().is_none());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue