All checks were successful
deploy / deploy (push) Successful in 6m1s
Closes #3 and #4. Both specs had gaps that only appeared on contact, which is worth recording because it is evidence for the plan-quality question in #10: - `enqueue(&[DiscoveredIssue])` cannot know a JobKind. It is derivable from labels, so the store now carries the LabelProtocol and core gained `job_kind_for`. The precedence when an operator applies several mode labels had to be decided: plan beats implement, because planning produces the implementation children and so loses nothing, while the reverse silently discards the decomposition that was also asked for. - `claim_next(worker, allowed_lanes)` had no lane to filter on. Migration 0002 adds one, recorded at enqueue by `routing::lane_for` — the same function `route` now delegates to, so the two cannot drift. Deriving it at claim time instead would mean a forge request per claim, since labels are not stored. - `list_opted_in_issues` returned a bare Vec with nowhere to put the ETag that #4's own step 3 requires, so a caller could not make the next poll conditional. It returns an `IssuePage` now, which also carries `not_modified` — a 304 is not an empty repo, and a poller that conflated them would treat every quiet poll as every issue having disappeared. The claim is one statement: a CTE takes the row lock with SKIP LOCKED and the update writes the claim, so select and update share a transaction without managing one by hand. Returning a job to pending clears the claim, because `pending_holds_no_claim` refuses the half-done version — the database is what makes lease expiry safe rather than the code remembering to. Enum values round-trip through serde rather than a hand-written match, so the schema's check constraints and the Rust types are provably one vocabulary. Also replaced a test from #2 that asserted exactly one migration exists. It failed on the first legitimate migration, which teaches people to edit the assertion rather than think. It now asserts what it was reaching for: versions unique and ascending. Verified against Postgres 18 — 15 database tests covering enqueue idempotency across repeated polls, lane filtering including the agent:oc override, claim metadata, lease expiry and reclaim, terminal jobs refusing transition, renewal requiring you still hold the claim, and re-routing a queued job when its labels change. Plus 12 mock-forge tests: If-None-Match sent and 304 distinguished from empty, ETag surfaced, 429 retried but bounded, 503 retried then succeeding, 404 not retried, pull requests filtered out, and every unimplemented write failing without touching the forge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013TxK1CWPkFXqdcXMJ4hVe6
533 lines
17 KiB
Rust
533 lines
17 KiB
Rust
//! Migration tests against a real Postgres.
|
|
//!
|
|
//! Skipped unless `TIRELESS_TEST_DATABASE_URL` is set, because CI has no
|
|
//! database and the house pattern is not to give it one (see `store.rs` on why
|
|
//! queries are checked at runtime). Run them by hand against a throwaway server:
|
|
//!
|
|
//! ```sh
|
|
//! podman run -d --name pg -e POSTGRES_PASSWORD=test -e POSTGRES_DB=tireless_test \
|
|
//! -p 55432:5432 docker.io/library/postgres:18-alpine
|
|
//! export TIRELESS_TEST_DATABASE_URL=postgres://postgres:test@127.0.0.1:55432/tireless_test
|
|
//! cargo test -p tireless-data -- --ignored
|
|
//! ```
|
|
//!
|
|
//! They are `#[ignore]`d rather than silently passing on a missing variable, so
|
|
//! a green `cargo test` never implies the schema was exercised.
|
|
|
|
use sqlx::{Connection, Executor, PgConnection, Row};
|
|
use tireless_data::store::MIGRATOR;
|
|
|
|
fn url() -> String {
|
|
std::env::var("TIRELESS_TEST_DATABASE_URL")
|
|
.expect("set TIRELESS_TEST_DATABASE_URL to run the ignored migration tests")
|
|
}
|
|
|
|
/// A connection scoped to its own empty schema.
|
|
///
|
|
/// Per test, not per run: `cargo test` runs these concurrently against one
|
|
/// server, so a shared `public` schema means each test's reset truncates
|
|
/// whatever its neighbours are midway through. The failure looks like a schema
|
|
/// bug — a unique violation on a row the test never inserted — which is an
|
|
/// expensive thing to misread.
|
|
async fn fresh_db(schema: &str) -> PgConnection {
|
|
let mut conn = PgConnection::connect(&url()).await.expect("connect");
|
|
conn.execute(
|
|
format!(
|
|
"drop schema if exists {schema} cascade; \
|
|
create schema {schema}; \
|
|
set search_path to {schema};"
|
|
)
|
|
.as_str(),
|
|
)
|
|
.await
|
|
.expect("private schema");
|
|
conn
|
|
}
|
|
|
|
/// A second connection into an existing test schema, for concurrency tests.
|
|
async fn join_db(schema: &str) -> PgConnection {
|
|
let mut conn = PgConnection::connect(&url()).await.expect("connect");
|
|
conn.execute(format!("set search_path to {schema};").as_str())
|
|
.await
|
|
.expect("search_path");
|
|
conn
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn migrations_apply_to_an_empty_database() {
|
|
const SCHEMA: &str = "t_apply";
|
|
let mut conn = fresh_db(SCHEMA).await;
|
|
MIGRATOR.run(&mut conn).await.expect("first run");
|
|
|
|
let tables: Vec<String> = sqlx::query(
|
|
format!("select tablename from pg_tables where schemaname='{SCHEMA}'").as_str(),
|
|
)
|
|
.fetch_all(&mut conn)
|
|
.await
|
|
.expect("list tables")
|
|
.iter()
|
|
.map(|r| r.get::<String, _>("tablename"))
|
|
.collect();
|
|
|
|
for expected in ["tracked_repo", "job", "agent_run"] {
|
|
assert!(
|
|
tables.contains(&expected.to_string()),
|
|
"missing table {expected}"
|
|
);
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn applying_twice_is_a_no_op() {
|
|
const SCHEMA: &str = "t_twice";
|
|
// The runner, poller and api all migrate on start, and systemd may restart
|
|
// any of them at any time — including all three at once after a deploy. A
|
|
// second run must be silent, not an error and not a duplicate.
|
|
let mut conn = fresh_db(SCHEMA).await;
|
|
MIGRATOR.run(&mut conn).await.expect("first run");
|
|
MIGRATOR
|
|
.run(&mut conn)
|
|
.await
|
|
.expect("second run must be a no-op");
|
|
|
|
let applied: i64 = sqlx::query("select count(*) as n from _sqlx_migrations")
|
|
.fetch_one(&mut conn)
|
|
.await
|
|
.expect("count migrations")
|
|
.get("n");
|
|
assert_eq!(applied, MIGRATOR.iter().count() as i64);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn one_live_job_per_issue_but_any_number_of_historical_ones() {
|
|
const SCHEMA: &str = "t_live";
|
|
// The property the partial unique index exists for. Getting this wrong in
|
|
// either direction is silent: too strict and discovery never recurs, too
|
|
// loose and one issue is worked twice at once.
|
|
let mut conn = fresh_db(SCHEMA).await;
|
|
MIGRATOR.run(&mut conn).await.expect("migrate");
|
|
|
|
let insert = |state: &str| {
|
|
format!(
|
|
"insert into job (forge,owner,repo,number,kind,state) \
|
|
values ('gitea','lair','tireless',1,'discover','{state}')"
|
|
)
|
|
};
|
|
|
|
conn.execute(insert("pending").as_str())
|
|
.await
|
|
.expect("first live job");
|
|
let second = conn.execute(insert("pending").as_str()).await;
|
|
assert!(
|
|
second.is_err(),
|
|
"a second live job for one issue must be rejected"
|
|
);
|
|
|
|
conn.execute("update job set state='delivered'")
|
|
.await
|
|
.expect("finish it");
|
|
conn.execute(insert("pending").as_str())
|
|
.await
|
|
.expect("a new job after a terminal one must be allowed");
|
|
|
|
let n: i64 = sqlx::query("select count(*) as n from job")
|
|
.fetch_one(&mut conn)
|
|
.await
|
|
.expect("count")
|
|
.get("n");
|
|
assert_eq!(n, 2, "history must be retained, not overwritten");
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn a_pending_job_cannot_hold_a_claim() {
|
|
const SCHEMA: &str = "t_claim";
|
|
// Lease expiry returns a job to the pool, and must clear the claim as it
|
|
// does. The database refuses the half-done version so that a buggy release
|
|
// path cannot strand an issue.
|
|
let mut conn = fresh_db(SCHEMA).await;
|
|
MIGRATOR.run(&mut conn).await.expect("migrate");
|
|
|
|
let bad = conn
|
|
.execute(
|
|
"insert into job (forge,owner,repo,number,kind,state,claimed_by,claim_expires_at) \
|
|
values ('gitea','lair','tireless',1,'plan','pending','w1',now())",
|
|
)
|
|
.await;
|
|
assert!(
|
|
bad.is_err(),
|
|
"a pending job holding a claim must be rejected"
|
|
);
|
|
|
|
conn.execute(
|
|
"insert into job (forge,owner,repo,number,kind,state,claimed_by,claim_expires_at) \
|
|
values ('gitea','lair','tireless',1,'plan','claimed','w1',now() + interval '5 min')",
|
|
)
|
|
.await
|
|
.expect("a properly claimed job is fine");
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn concurrent_claims_never_hand_out_the_same_job() {
|
|
const SCHEMA: &str = "t_concurrent";
|
|
// The property the whole design rests on (design.md §4.2). Two claimers,
|
|
// one pending job: exactly one winner, and the loser gets nothing rather
|
|
// than blocking.
|
|
let mut setup = fresh_db(SCHEMA).await;
|
|
MIGRATOR.run(&mut setup).await.expect("migrate");
|
|
setup
|
|
.execute(
|
|
"insert into job (forge,owner,repo,number,kind,state) \
|
|
values ('gitea','lair','tireless',1,'plan','pending')",
|
|
)
|
|
.await
|
|
.expect("seed one job");
|
|
|
|
const CLAIM: &str = "select id from job where state='pending' \
|
|
order by created_at limit 1 for update skip locked";
|
|
|
|
let mut a = join_db(SCHEMA).await;
|
|
let mut b = join_db(SCHEMA).await;
|
|
|
|
let mut ta = a.begin().await.expect("tx a");
|
|
let got_a = sqlx::query(CLAIM)
|
|
.fetch_all(&mut *ta)
|
|
.await
|
|
.expect("claim a");
|
|
|
|
// b runs while a still holds the row lock.
|
|
let mut tb = b.begin().await.expect("tx b");
|
|
let got_b = sqlx::query(CLAIM)
|
|
.fetch_all(&mut *tb)
|
|
.await
|
|
.expect("claim b");
|
|
|
|
assert_eq!(got_a.len(), 1, "the first claimer takes the job");
|
|
assert_eq!(
|
|
got_b.len(),
|
|
0,
|
|
"the second claimer must skip the locked row, not block and not duplicate"
|
|
);
|
|
|
|
ta.commit().await.expect("commit a");
|
|
tb.commit().await.expect("commit b");
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// JobStore behaviour.
|
|
//
|
|
// The claim protocol is the part of the design most likely to have subtle bugs
|
|
// and the last place they are free (design.md §7, stage 2). These exercise it
|
|
// against a real server rather than a mock, because every property that matters
|
|
// here — atomicity, lease expiry, the partial index — is the database's.
|
|
// ---------------------------------------------------------------------------
|
|
|
|
use sqlx::postgres::PgPoolOptions;
|
|
use tireless_core::port::{DiscoveredIssue, JobStore};
|
|
use tireless_data::PgStore;
|
|
use tireless_entities::{AgentKind, Forge, IssueRef, JobState, LabelProtocol};
|
|
|
|
/// A store bound to its own schema, so these run concurrently with each other.
|
|
async fn store(schema: &str) -> PgStore {
|
|
let mut conn = fresh_db(schema).await;
|
|
MIGRATOR.run(&mut conn).await.expect("migrate");
|
|
let pool = PgPoolOptions::new()
|
|
.max_connections(4)
|
|
.after_connect({
|
|
let schema = schema.to_string();
|
|
move |c, _| {
|
|
let schema = schema.clone();
|
|
Box::pin(async move {
|
|
sqlx::query(&format!("set search_path to {schema}"))
|
|
.execute(&mut *c)
|
|
.await?;
|
|
Ok(())
|
|
})
|
|
}
|
|
})
|
|
.connect(&url())
|
|
.await
|
|
.expect("pool");
|
|
PgStore::new(pool, LabelProtocol::default())
|
|
}
|
|
|
|
fn discovered(number: i64, labels: &[&str]) -> DiscoveredIssue {
|
|
DiscoveredIssue {
|
|
issue: IssueRef {
|
|
forge: Forge::Gitea,
|
|
owner: "lair".into(),
|
|
repo: "tireless".into(),
|
|
number,
|
|
},
|
|
title: format!("issue {number}"),
|
|
body: String::new(),
|
|
labels: labels.iter().map(|s| s.to_string()).collect(),
|
|
is_open: true,
|
|
}
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn enqueue_ignores_issues_that_express_no_work() {
|
|
let s = store("t_enqueue").await;
|
|
let p = LabelProtocol::default();
|
|
|
|
let n = s
|
|
.enqueue(&[
|
|
// opted in, no mode label — nothing asked for
|
|
discovered(1, &[&p.opt_in]),
|
|
// mode label, not opted in — a discovery proposal awaiting a human
|
|
discovered(2, &[&p.mode_implement]),
|
|
// neither
|
|
discovered(3, &[]),
|
|
// the real thing
|
|
discovered(4, &[&p.opt_in, &p.mode_plan]),
|
|
])
|
|
.await
|
|
.expect("enqueue");
|
|
|
|
assert_eq!(n, 1, "only the opted-in issue with a mode label enqueues");
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn enqueue_is_idempotent_while_a_job_is_live() {
|
|
let s = store("t_idem").await;
|
|
let p = LabelProtocol::default();
|
|
let issue = discovered(1, &[&p.opt_in, &p.mode_plan]);
|
|
|
|
assert_eq!(
|
|
s.enqueue(std::slice::from_ref(&issue))
|
|
.await
|
|
.expect("first"),
|
|
1
|
|
);
|
|
// Every poll re-reports the same open issue. Enqueue has to be a no-op, or
|
|
// a repo polled every five minutes accumulates a job every five minutes.
|
|
assert_eq!(
|
|
s.enqueue(std::slice::from_ref(&issue))
|
|
.await
|
|
.expect("second"),
|
|
0
|
|
);
|
|
assert_eq!(
|
|
s.enqueue(std::slice::from_ref(&issue))
|
|
.await
|
|
.expect("third"),
|
|
0
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn a_closed_issue_is_never_enqueued() {
|
|
let s = store("t_closed").await;
|
|
let p = LabelProtocol::default();
|
|
let mut issue = discovered(1, &[&p.opt_in, &p.mode_implement]);
|
|
issue.is_open = false;
|
|
assert_eq!(s.enqueue(&[issue]).await.expect("enqueue"), 0);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn claiming_respects_the_lane_the_caller_allows() {
|
|
let s = store("t_lane").await;
|
|
let p = LabelProtocol::default();
|
|
|
|
// Plan always routes to Claude Code; the oc override forces the other lane.
|
|
s.enqueue(&[
|
|
discovered(1, &[&p.opt_in, &p.mode_plan]),
|
|
discovered(2, &[&p.opt_in, &p.mode_plan, &p.force_oc]),
|
|
])
|
|
.await
|
|
.expect("enqueue");
|
|
|
|
let oc = s
|
|
.claim_next("w1", &[AgentKind::Opencode])
|
|
.await
|
|
.expect("claim oc")
|
|
.expect("an opencode job is available");
|
|
assert_eq!(oc.issue.number, 2, "the forced-oc job is the opencode one");
|
|
|
|
let cc = s
|
|
.claim_next("w2", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("claim cc")
|
|
.expect("a claude code job is available");
|
|
assert_eq!(cc.issue.number, 1);
|
|
|
|
// Both lanes are now empty.
|
|
assert!(
|
|
s.claim_next("w3", &[AgentKind::ClaudeCode, AgentKind::Opencode])
|
|
.await
|
|
.expect("claim")
|
|
.is_none()
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn claiming_with_no_allowed_lanes_returns_nothing() {
|
|
// Every lane held by the governor. This must not hand out work.
|
|
let s = store("t_nolane").await;
|
|
let p = LabelProtocol::default();
|
|
s.enqueue(&[discovered(1, &[&p.opt_in, &p.mode_plan])])
|
|
.await
|
|
.expect("enqueue");
|
|
assert!(s.claim_next("w1", &[]).await.expect("claim").is_none());
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn a_claim_sets_the_worker_and_a_lease_and_counts_the_attempt() {
|
|
let s = store("t_claimmeta").await;
|
|
let p = LabelProtocol::default();
|
|
s.enqueue(&[discovered(1, &[&p.opt_in, &p.mode_plan])])
|
|
.await
|
|
.expect("enqueue");
|
|
|
|
let job = s
|
|
.claim_next("worker-a", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("claim")
|
|
.expect("job");
|
|
|
|
assert_eq!(job.state, JobState::Claimed);
|
|
assert_eq!(job.claimed_by.as_deref(), Some("worker-a"));
|
|
assert!(
|
|
job.claim_expires_at.is_some(),
|
|
"a claim without a lease can never expire"
|
|
);
|
|
assert_eq!(
|
|
job.attempts, 1,
|
|
"the attempt is counted at claim, not at completion"
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn an_expired_lease_returns_the_job_and_clears_the_claim() {
|
|
let s = store("t_expiry").await;
|
|
let p = LabelProtocol::default();
|
|
s.enqueue(&[discovered(1, &[&p.opt_in, &p.mode_plan])])
|
|
.await
|
|
.expect("enqueue");
|
|
let job = s
|
|
.claim_next("dead-worker", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("claim")
|
|
.expect("job");
|
|
|
|
// A live lease is left alone.
|
|
assert_eq!(s.expire_stale_claims().await.expect("sweep"), 0);
|
|
|
|
sqlx::query("update job set claim_expires_at = now() - interval '1 minute' where id = $1")
|
|
.bind(job.id)
|
|
.execute(s.pool())
|
|
.await
|
|
.expect("age the lease");
|
|
|
|
assert_eq!(s.expire_stale_claims().await.expect("sweep"), 1);
|
|
|
|
// And it is claimable again — which only works if the claim was cleared,
|
|
// since `pending_holds_no_claim` would otherwise have rejected the update.
|
|
let again = s
|
|
.claim_next("live-worker", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("reclaim")
|
|
.expect("the job returned to the pool");
|
|
assert_eq!(again.claimed_by.as_deref(), Some("live-worker"));
|
|
assert_eq!(
|
|
again.attempts, 2,
|
|
"the retry is visible in the attempt count"
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn a_terminal_job_cannot_be_transitioned() {
|
|
let s = store("t_terminal").await;
|
|
let p = LabelProtocol::default();
|
|
s.enqueue(&[discovered(1, &[&p.opt_in, &p.mode_plan])])
|
|
.await
|
|
.expect("enqueue");
|
|
let job = s
|
|
.claim_next("w1", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("claim")
|
|
.expect("job");
|
|
|
|
s.transition(job.id, JobState::Delivered)
|
|
.await
|
|
.expect("deliver");
|
|
|
|
// Re-running a delivered job creates a new one; it never reopens the old,
|
|
// so history stays true (design.md §4.3).
|
|
let err = s.transition(job.id, JobState::Pending).await;
|
|
assert!(err.is_err(), "a delivered job must not be reopened");
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn renewing_a_claim_requires_still_holding_it() {
|
|
let s = store("t_renew").await;
|
|
let p = LabelProtocol::default();
|
|
s.enqueue(&[discovered(1, &[&p.opt_in, &p.mode_plan])])
|
|
.await
|
|
.expect("enqueue");
|
|
let job = s
|
|
.claim_next("w1", &[AgentKind::ClaudeCode])
|
|
.await
|
|
.expect("claim")
|
|
.expect("job");
|
|
|
|
assert!(
|
|
s.renew_claim(job.id, "w1").await.expect("renew"),
|
|
"the holder may renew"
|
|
);
|
|
assert!(
|
|
!s.renew_claim(job.id, "w2").await.expect("renew"),
|
|
"a worker that does not hold the claim must not be able to extend it"
|
|
);
|
|
}
|
|
|
|
#[tokio::test]
|
|
#[ignore = "needs TIRELESS_TEST_DATABASE_URL"]
|
|
async fn a_label_change_reroutes_a_queued_job() {
|
|
// An operator adding tireless/agent:oc to something already queued has to
|
|
// reach it, or the override only works before the poller sees the issue.
|
|
let s = store("t_reroute").await;
|
|
let p = LabelProtocol::default();
|
|
let issue = discovered(1, &[&p.opt_in, &p.mode_plan]);
|
|
s.enqueue(std::slice::from_ref(&issue))
|
|
.await
|
|
.expect("enqueue");
|
|
|
|
assert!(
|
|
s.claim_next("w", &[AgentKind::Opencode])
|
|
.await
|
|
.expect("claim")
|
|
.is_none(),
|
|
"it starts on the claude code lane"
|
|
);
|
|
|
|
let relabelled = vec![p.opt_in.clone(), p.mode_plan.clone(), p.force_oc.clone()];
|
|
assert_eq!(
|
|
s.refresh_lane(&issue.issue, &relabelled, &p)
|
|
.await
|
|
.expect("refresh"),
|
|
1
|
|
);
|
|
assert!(
|
|
s.claim_next("w", &[AgentKind::Opencode])
|
|
.await
|
|
.expect("claim")
|
|
.is_some(),
|
|
"after the relabel it belongs to opencode"
|
|
);
|
|
}
|