From 341db4e62835bb51f9c5f743c30f976d33ded153 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 00:40:44 -0400 Subject: [PATCH 01/12] fix(process): flush the session ledger before a fork copies it The fork of a running sandbox copies session.db while the writer still holds the last accepted rows in memory, up to its 5 s disk flush, so the fork lost the source's last seconds of events. The CloneState job now runs flush_checked under the guest freeze, then clones; a failed flush fails the fork. The new test writes a row, forks without waiting, and finds it in the clone; it failed before the barrier. Fixes #243. --- CHANGELOG.md | 8 +++ .../test_channel_source_credentials.py | 54 +++++++++++++++++++ crates/capsem-process/src/vsock.rs | 2 +- .../capsem-process/src/vsock/clone_state.rs | 34 ++++++++++-- .../src/vsock/clone_state/tests.rs | 43 +++++++++++++++ 5 files changed, 135 insertions(+), 6 deletions(-) create mode 100644 build_system/tests/release/test_channel_source_credentials.py create mode 100644 crates/capsem-process/src/vsock/clone_state/tests.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index d71ae2760..239243aeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Forking a running sandbox now carries everything its session ledger + recorded up to the fork. The ledger writer keeps accepted rows in memory + until its next disk flush, up to five seconds away, and the fork copied the + file without asking it to flush, so a fork left out the source's last few + seconds of network, tool, exec and security events. The fork now flushes + the writer while the guest is frozen, and fails rather than producing an + incomplete ledger if that flush fails (google/capsem#243). + - The guest rootfs OBOM (`obom.cdx.json`) now describes the rootfs exactly and builds about ten times faster. The build unpacked the rootfs on the build host before scanning it: on macOS the case-insensitive filesystem diff --git a/build_system/tests/release/test_channel_source_credentials.py b/build_system/tests/release/test_channel_source_credentials.py new file mode 100644 index 000000000..8fe5897e5 --- /dev/null +++ b/build_system/tests/release/test_channel_source_credentials.py @@ -0,0 +1,54 @@ +"""Resolving a channel's source manifest uses the release's own GitHub login. + +`release-binaries` dispatches its hosted lane through `gh`, so the operator's +`gh` login is already a release prerequisite. The source-manifest fetch alone +also demanded `GITHUB_TOKEN` in the environment, and found out it was missing +eight minutes in, after the release's citadel, contract and build-system +suites had all passed. +""" + +from __future__ import annotations + +import subprocess + +import pytest +from capsem_builder.release.tools import fetch_channel_source_manifest as fetch + + +def test_an_exported_token_is_used_as_is(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("GITHUB_TOKEN", "exported") + monkeypatch.setattr(fetch.subprocess, "run", pytest.fail) + assert fetch.github_token() == "exported" + + +def test_without_one_the_gh_login_supplies_it(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + calls: list[list[str]] = [] + + def run(command, **kwargs): + calls.append(command) + return subprocess.CompletedProcess(command, 0, stdout="from-gh\n", stderr="") + + monkeypatch.setattr(fetch.subprocess, "run", run) + assert fetch.github_token() == "from-gh" + assert calls == [["gh", "auth", "token"]] + + +@pytest.mark.parametrize( + "outcome", + [ + subprocess.CompletedProcess(["gh"], 1, stdout="", stderr="not logged in"), + subprocess.CompletedProcess(["gh"], 0, stdout="\n", stderr=""), + FileNotFoundError("gh"), + ], +) +def test_no_token_anywhere_is_none(monkeypatch: pytest.MonkeyPatch, outcome) -> None: + monkeypatch.delenv("GITHUB_TOKEN", raising=False) + + def run(command, **kwargs): + if isinstance(outcome, BaseException): + raise outcome + return outcome + + monkeypatch.setattr(fetch.subprocess, "run", run) + assert fetch.github_token() is None diff --git a/crates/capsem-process/src/vsock.rs b/crates/capsem-process/src/vsock.rs index 87b8d69fd..82f0b9d2d 100644 --- a/crates/capsem-process/src/vsock.rs +++ b/crates/capsem-process/src/vsock.rs @@ -655,7 +655,7 @@ pub(crate) async fn setup_vsock(options: VsockOptions) -> Result<()> { } } ServiceToProcess::CloneState { id, destination } => { - clone_state::spawn(&hub_tx, &js_for_cmd, &session_dir, id, destination); + clone_state::spawn(&hub_tx, &js_for_cmd, &db_for_cmd, &session_dir, id, destination); } ServiceToProcess::Suspend { checkpoint_path } => { let full_path = session_dir.join(checkpoint_path); diff --git a/crates/capsem-process/src/vsock/clone_state.rs b/crates/capsem-process/src/vsock/clone_state.rs index ef7d101fd..137fdc646 100644 --- a/crates/capsem-process/src/vsock/clone_state.rs +++ b/crates/capsem-process/src/vsock/clone_state.rs @@ -18,21 +18,42 @@ use crate::job_store::{with_quiescence, JobResult, JobStore}; /// How long a fork waits for the guest to freeze, as suspend does. const CLONE_FREEZE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); +/// Copy the session, with every ledger row accepted so far on disk first. +/// +/// The writer holds accepted rows in memory until its next disk flush, up to +/// five seconds away, and the clone copies the file: without this barrier a +/// fork left out whatever the source recorded in its last seconds (#243). Run +/// under the guest freeze, so nothing the guest does can land between the +/// flush and the copy. A flush that fails fails the fork rather than +/// producing a quietly incomplete one. +async fn flush_then_clone(db: &capsem_logger::DbWriter, source: PathBuf, destination: PathBuf) -> anyhow::Result { + db.flush_checked() + .await + .map_err(|error| anyhow::anyhow!("flush the session ledger before cloning: {error}"))?; + tokio::task::spawn_blocking(move || capsem_core::session::clone_sandbox_state(&source, &destination)) + .await + .map_err(|error| anyhow::anyhow!("clone task failed: {error}"))? +} + /// Clone `source` into `destination` and answer job `id` with its size. pub(super) fn spawn( hub_tx: &mpsc::Sender, job_store: &Arc, + db: &Arc, source: &std::path::Path, id: u64, destination: String, ) { - let (hub_tx, job_store, source) = (hub_tx.clone(), Arc::clone(job_store), source.to_path_buf()); + let (hub_tx, job_store, db, source) = ( + hub_tx.clone(), + Arc::clone(job_store), + Arc::clone(db), + source.to_path_buf(), + ); tokio::spawn(async move { let destination = PathBuf::from(destination); - let result = with_quiescence(&hub_tx, &job_store, CLONE_FREEZE_TIMEOUT, || async { - tokio::task::spawn_blocking(move || capsem_core::session::clone_sandbox_state(&source, &destination)) - .await - .map_err(|error| anyhow::anyhow!("clone task failed: {error}"))? + let result = with_quiescence(&hub_tx, &job_store, CLONE_FREEZE_TIMEOUT, || { + flush_then_clone(&db, source, destination) }) .await .map_err(|error| format!("{error:#}")); @@ -41,3 +62,6 @@ pub(super) fn spawn( } }); } + +#[cfg(test)] +mod tests; diff --git a/crates/capsem-process/src/vsock/clone_state/tests.rs b/crates/capsem-process/src/vsock/clone_state/tests.rs new file mode 100644 index 000000000..5435f6ff4 --- /dev/null +++ b/crates/capsem-process/src/vsock/clone_state/tests.rs @@ -0,0 +1,43 @@ +use super::*; + +use capsem_logger::{DbWriter, FileAction, FileEvent, FileKind, WriteOp}; + +/// A fork of a running session carries the rows its writer accepted a moment +/// ago (#243). The writer holds accepted rows in memory until its next disk +/// flush, up to five seconds away, and the clone copies the file. +#[tokio::test] +async fn a_fork_carries_rows_the_writer_has_not_flushed_yet() { + let tmp = tempfile::tempdir().unwrap(); + let source = tmp.path().join("src"); + let destination = tmp.path().join("dst"); + std::fs::create_dir_all(source.join("system")).unwrap(); + std::fs::create_dir_all(source.join("guest/workspace")).unwrap(); + std::fs::create_dir(&destination).unwrap(); + + let db = DbWriter::open(&source.join("session.db"), 64).unwrap(); + db.write(WriteOp::FileEvent(FileEvent { + event_id: None, + timestamp: std::time::SystemTime::now(), + action: FileAction::Created, + path: "/root/written-just-before-the-fork".into(), + size: Some(7), + kind: FileKind::File, + trace_id: None, + credential_ref: None, + })) + .await; + + flush_then_clone(&db, source.clone(), destination.clone()) + .await + .unwrap(); + + let cloned = capsem_logger::DbReader::open(&destination.join("session.db")).unwrap(); + let rows = cloned.query_raw("SELECT path FROM fs_events").unwrap(); + assert_eq!( + rows, + r#"{"columns":["path"],"rows":[["/root/written-just-before-the-fork"]]}"# + ); + tokio::task::spawn_blocking(move || db.shutdown_blocking()) + .await + .unwrap(); +} From d55219794851235c554746ffa4d61362d88c1d94 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 00:40:44 -0400 Subject: [PATCH 02/12] fix(release): resolve source manifests with the operator's gh login fetch-channel-source-manifest required GITHUB_TOKEN in the environment. release-binaries already dispatches its hosted lane through gh, so the gh login is a release prerequisite anyway, and the missing export surfaced only at channel-source, after eight minutes of citadel, contract and build-system suites had passed. An exported GITHUB_TOKEN still wins; without one the operator's gh login supplies it, and only neither is an error. --- .../tools/fetch_channel_source_manifest.py | 28 +++++++++++++++++-- build_system/tests/test_test_ownership.py | 1 + 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/build_system/builder/release/tools/fetch_channel_source_manifest.py b/build_system/builder/release/tools/fetch_channel_source_manifest.py index 8f338c083..2bbd3b6a9 100644 --- a/build_system/builder/release/tools/fetch_channel_source_manifest.py +++ b/build_system/builder/release/tools/fetch_channel_source_manifest.py @@ -136,6 +136,26 @@ def resolve_source_manifest( return payload, source +def github_token() -> str | None: + """`GITHUB_TOKEN` if exported, otherwise the operator's `gh` login. + + The release dispatches its hosted lane through `gh`, so that login is + already required; demanding a second export found its absence only after + the release's local suites had spent eight minutes passing. + """ + exported = os.environ.get("GITHUB_TOKEN", "") + if exported: + return exported + try: + result = subprocess.run( + ["gh", "auth", "token"], capture_output=True, text=True, check=False, timeout=30 + ) + except (OSError, subprocess.SubprocessError): + return None + token = result.stdout.strip() if result.returncode == 0 else "" + return token or None + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("--channel", required=True) @@ -168,9 +188,13 @@ def main() -> int: fallback_url = args.fallback_url or ( f"https://release.capsem.org/assets/{args.channel}/manifest.json" ) - token = os.environ.get("GITHUB_TOKEN", "") + token = github_token() if not token: - print("GITHUB_TOKEN is required to resolve source manifests", file=sys.stderr) + print( + "resolving source manifests needs GitHub credentials: export GITHUB_TOKEN " + "or log in with `gh auth login`", + file=sys.stderr, + ) return 1 try: retired_graph: retirement.RetiredPublicGraph | None = None diff --git a/build_system/tests/test_test_ownership.py b/build_system/tests/test_test_ownership.py index 50cd5158f..509d28516 100644 --- a/build_system/tests/test_test_ownership.py +++ b/build_system/tests/test_test_ownership.py @@ -91,6 +91,7 @@ "build_system/tests/packaging/test_macos_packaging_boundary.py", "build_system/tests/packaging/test_shared_packaging_boundary.py", "build_system/tests/policy/test_policy_modules.py", + "build_system/tests/release/test_channel_source_credentials.py", "build_system/tests/release/test_deb_package_portability.py", "build_system/tests/release/test_release_module_boundary.py", "build_system/tests/release/test_release_foundation_tool_boundary.py", From 525fb3daea128b5e844fbc99af79c80cc8c89c11 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:20:57 -0400 Subject: [PATCH 03/12] test(telemetry): prove metric export end to end The OTLP/HTTP export path (provider, HTTP client, install) landed with only its pure helpers tested, which dropped capsem-telemetry from its 97% coverage floor to 63%: nothing proved a metric ever left the process. A local collector now receives a real OTLP POST to /v1/metrics carrying the service, a session attribute and the recorded metric; install is refused a second time with RecorderAlreadySet; every facade unit maps to its exact UCUM code; uncataloged names, unit-bearing histograms and gauge increments reach the exporter. export.rs: 104/185 -> 182/185 lines. --- crates/capsem-telemetry/src/export/tests.rs | 171 ++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/crates/capsem-telemetry/src/export/tests.rs b/crates/capsem-telemetry/src/export/tests.rs index 261d34bdf..538e0052f 100644 --- a/crates/capsem-telemetry/src/export/tests.rs +++ b/crates/capsem-telemetry/src/export/tests.rs @@ -120,3 +120,174 @@ fn every_catalog_unit_has_a_ucum_spelling() { } } } + +/// One HTTP request, as a collector on loopback received it. +struct Received { + request_line: String, + body: Vec, +} + +/// Accept one request on loopback, answer 200, and hand it back. +fn one_request_collector() -> (String, std::sync::mpsc::Receiver) { + use std::io::{BufRead, BufReader, Read, Write}; + + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let mut reader = BufReader::new(stream.try_clone().unwrap()); + let mut request_line = String::new(); + reader.read_line(&mut request_line).unwrap(); + let mut length = 0; + loop { + let mut header = String::new(); + reader.read_line(&mut header).unwrap(); + if header.trim().is_empty() { + break; + } + if let Some(value) = header.to_ascii_lowercase().strip_prefix("content-length:") { + length = value.trim().parse().unwrap(); + } + } + let mut body = vec![0; length]; + reader.read_exact(&mut body).unwrap(); + let mut stream = stream; + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n") + .unwrap(); + tx.send(Received { request_line, body }).unwrap(); + }); + (base, rx) +} + +fn contains(haystack: &[u8], needle: &str) -> bool { + haystack.windows(needle.len()).any(|window| window == needle.as_bytes()) +} + +/// The whole export path: the real provider and HTTP client deliver an OTLP +/// request to the corp endpoint's `/v1/metrics`, carrying the reporting +/// service and what was recorded. Only the pure helpers were tested before, +/// so nothing proved a metric ever left the process. +#[test] +fn a_corp_destination_receives_recorded_metrics_over_otlp_http() { + let (base, received) = one_request_collector(); + let provider = provider( + &Destination::Corp(base), + "capsem-export-test", + vec![KeyValue::new("capsem.session", "sess-1")], + ) + .unwrap(); + let exporter = Exporter { provider }; + let recorder = OtelRecorder::new(exporter.meter()); + metrics::with_local_recorder(&recorder, || { + metrics::counter!(crate::db::DB_WRITE_OPS_TOTAL, "insert_type" => "net_event").increment(3); + }); + exporter.flush().unwrap(); + + let request = received.recv_timeout(EXPORT_TIMEOUT).unwrap(); + assert!( + request.request_line.starts_with("POST /v1/metrics "), + "{}", + request.request_line + ); + for expected in ["capsem-export-test", "sess-1", crate::db::DB_WRITE_OPS_TOTAL] { + assert!(contains(&request.body, expected), "the OTLP body lacks {expected}"); + } +} + +#[test] +fn install_errors_say_what_failed() { + assert_eq!( + InstallError::Build("no TLS".into()).to_string(), + "build the OTLP metric exporter: no TLS" + ); + assert_eq!( + InstallError::RecorderAlreadySet.to_string(), + "a metrics recorder is already installed in this process" + ); +} + +/// `install` routes the process's metrics facade to export, once: a second +/// exporter in the same process would split its metrics between two +/// recorders, so it is refused rather than silently ignored. +#[test] +fn install_takes_the_facade_once_and_refuses_a_second_exporter() { + let (base, _received) = one_request_collector(); + let first = install(&Destination::Corp(base.clone()), "capsem-install-test", vec![]); + assert!(first.is_ok(), "{:?}", first.err()); + let second = install(&Destination::Corp(base), "capsem-install-test", vec![]); + assert!( + matches!(second, Err(InstallError::RecorderAlreadySet)), + "{:?}", + second.err() + ); + // Keep the provider alive past both calls, then let Drop shut it down. + drop(first); +} + +/// The unit a collector sees is the UCUM code, not the facade's name for it. +#[test] +fn every_facade_unit_maps_to_its_exact_ucum_code() { + let expected = [ + (Unit::Count, "{count}"), + (Unit::Percent, "%"), + (Unit::Seconds, "s"), + (Unit::Milliseconds, "ms"), + (Unit::Microseconds, "us"), + (Unit::Nanoseconds, "ns"), + (Unit::Bytes, "By"), + (Unit::Kibibytes, "KiBy"), + (Unit::Mebibytes, "MiBy"), + (Unit::Gibibytes, "GiBy"), + (Unit::Tebibytes, "TiBy"), + (Unit::BitsPerSecond, "bit/s"), + (Unit::KilobitsPerSecond, "kbit/s"), + (Unit::MegabitsPerSecond, "Mbit/s"), + (Unit::GigabitsPerSecond, "Gbit/s"), + (Unit::TerabitsPerSecond, "Tbit/s"), + (Unit::CountPerSecond, "{count}/s"), + ]; + for (unit, code) in expected { + assert_eq!(ucum(unit), code, "{unit:?}"); + } +} + +/// A name the catalog does not know still exports, undescribed; a catalog +/// histogram carries its unit; a gauge moves up as well as down. +#[test] +fn uncataloged_names_units_and_gauge_increments_reach_the_exporter() { + let exporter = InMemoryMetricExporter::default(); + let provider = SdkMeterProvider::builder() + .with_periodic_exporter(exporter.clone()) + .build(); + let recorder = OtelRecorder::new(provider.meter("capsem")); + let histogram = crate::all() + .find(|spec| spec.kind == crate::MetricKind::Histogram && spec.unit.is_some()) + .expect("the catalog has a histogram with a unit"); + metrics::with_local_recorder(&recorder, || { + metrics::counter!("capsem.test.uncataloged_total").increment(1); + metrics::gauge!(crate::db::DB_MEMORY_UNFLUSHED_OPS).increment(4.0); + metrics::histogram!(histogram.name).record(2.0); + }); + provider.force_flush().unwrap(); + + let exported = exporter.get_finished_metrics().unwrap(); + let metrics: Vec<_> = exported + .iter() + .flat_map(|resource| resource.scope_metrics()) + .flat_map(|scope| scope.metrics()) + .collect(); + let find = |name: &str| { + *metrics + .iter() + .find(|metric| metric.name() == name) + .unwrap_or_else(|| panic!("{name} was not exported")) + }; + assert_eq!(find("capsem.test.uncataloged_total").description(), ""); + assert_eq!(find(histogram.name).unit(), ucum(histogram.unit.unwrap())); + let AggregatedMetrics::F64(MetricData::Gauge(gauge)) = find(crate::db::DB_MEMORY_UNFLUSHED_OPS).data() else { + panic!("the unflushed-ops gauge exports as an f64 gauge"); + }; + assert_eq!(gauge.data_points().next().unwrap().value(), 4.0); +} From 0af262f6c34d4386a494827111f34f6f894155ef Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:20:57 -0400 Subject: [PATCH 04/12] refactor(mcp-builtin): lift startup logic out of main and test it Removing the snapshot tools in #228 took the builtin server's tested code with them and left main() -- peer lock naming, peer index parsing, active-profile loading -- as untestable process wiring, below the crate's coverage floor. Those rules are now functions with tests, and grep_http and http_headers get the refusal test fetch_http already had: a blocked URL comes back as an error result carrying its ledger record. --- crates/capsem-mcp-builtin/src/main.rs | 54 +++++++++++++++--------- crates/capsem-mcp-builtin/src/tests.rs | 58 ++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 20 deletions(-) diff --git a/crates/capsem-mcp-builtin/src/main.rs b/crates/capsem-mcp-builtin/src/main.rs index 38829ed91..27d4fd1e3 100644 --- a/crates/capsem-mcp-builtin/src/main.rs +++ b/crates/capsem-mcp-builtin/src/main.rs @@ -237,6 +237,37 @@ fn extract_text(resp: JsonRpcResponse) -> Result { } } +// -- Startup -- + +/// Which member of a pooled server this process is; peer 0 is the singleton. +fn peer_index(raw: Option) -> u32 { + raw.and_then(|value| value.parse().ok()).unwrap_or(0) +} + +/// The per-peer singleton lock, so pool members do not fight over one file. +fn lock_file_name(peer_index: u32) -> String { + if peer_index == 0 { + "mcp-builtin.lock".to_string() + } else { + format!("mcp-builtin-{peer_index}.lock") + } +} + +/// Read, parse and validate the active profile, naming the file in any error. +fn load_active_profile(path: &str) -> Result { + let text = std::fs::read_to_string(path) + .map_err(anyhow::Error::new) + .with_context(|| format!("read active profile {path}"))?; + let profile: ActiveProfileFile = toml::from_str(&text) + .map_err(anyhow::Error::new) + .with_context(|| format!("parse active profile {path}"))?; + profile + .validate() + .map_err(anyhow::Error::msg) + .with_context(|| format!("validate active profile {path}"))?; + Ok(profile) +} + // -- Main -- #[tokio::main] @@ -262,18 +293,10 @@ async fn main() -> Result<()> { // Per-peer index for pool members (set by the aggregator's // connect_stdio when spawning peer 1..N of a pooled server). Each // peer gets its own lockfile so they don't fight over the singleton. - let peer_index: u32 = std::env::var("CAPSEM_BUILTIN_PEER_INDEX") - .ok() - .and_then(|s| s.parse().ok()) - .unwrap_or(0); + let peer_index = peer_index(std::env::var("CAPSEM_BUILTIN_PEER_INDEX").ok()); if let (Some(pid), Some(dir)) = (parent_pid, session_dir) { - let lock_name = if peer_index == 0 { - "mcp-builtin.lock".to_string() - } else { - format!("mcp-builtin-{peer_index}.lock") - }; - let lock_path = std::path::PathBuf::from(dir).join(&lock_name); + let lock_path = std::path::PathBuf::from(dir).join(lock_file_name(peer_index)); match capsem_guard::install(Some(pid), &lock_path) { Ok(Some(guards)) => { // Keep the guards alive for the process's lifetime. @@ -292,16 +315,7 @@ async fn main() -> Result<()> { let active_profile_path = std::env::var("CAPSEM_ACTIVE_PROFILE").map_err(|_| anyhow::anyhow!("CAPSEM_ACTIVE_PROFILE is required"))?; - let active_profile_text = std::fs::read_to_string(&active_profile_path) - .map_err(anyhow::Error::new) - .with_context(|| format!("read active profile {active_profile_path}"))?; - let active_profile: ActiveProfileFile = toml::from_str(&active_profile_text) - .map_err(anyhow::Error::new) - .with_context(|| format!("parse active profile {active_profile_path}"))?; - active_profile - .validate() - .map_err(anyhow::Error::msg) - .with_context(|| format!("validate active profile {active_profile_path}"))?; + let active_profile = load_active_profile(&active_profile_path)?; let security_rules = Arc::new(active_profile.compile_security_rule_set().map_err(anyhow::Error::msg)?); let plugin_policy = Arc::new(active_profile.plugins.clone()); diff --git a/crates/capsem-mcp-builtin/src/tests.rs b/crates/capsem-mcp-builtin/src/tests.rs index 73ca5c794..230f01d8c 100644 --- a/crates/capsem-mcp-builtin/src/tests.rs +++ b/crates/capsem-mcp-builtin/src/tests.rs @@ -375,3 +375,61 @@ async fn builtin_http_client_does_not_follow_redirects() { "redirects must not be followed -- a 3xx to another host would bypass the domain policy check" ); } + +/// Every HTTP tool refuses a blocked URL the same way, with its ledger record +/// on the error result -- not only `fetch_http`. +#[tokio::test] +async fn grep_and_header_tools_refuse_a_blocked_url_with_a_ledger_record() { + let grep = handler() + .grep_http(Parameters(GrepHttpParams { + url: "http://127.0.0.1:1/".to_string(), + pattern: "anything".to_string(), + context_lines: None, + max_matches: None, + start_index: None, + max_length: None, + })) + .await; + let headers = handler() + .http_headers(Parameters(HttpHeadersParams { + url: "http://127.0.0.1:1/".to_string(), + method: None, + })) + .await; + for result in [grep, headers] { + assert_eq!(result.is_error, Some(true), "{result:?}"); + let records = ledger_records(&result); + let [BuiltinLedgerRecord::HttpRequest(request)] = records.as_slice() else { + panic!("one refusal, one record: {result:?}"); + }; + assert_eq!(request.decision, builtin_ledger::HttpDecision::Denied); + } +} + +#[test] +fn pooled_peers_each_get_their_own_singleton_lock() { + assert_eq!(lock_file_name(0), "mcp-builtin.lock"); + assert_eq!(lock_file_name(3), "mcp-builtin-3.lock"); + assert_eq!(peer_index(None), 0); + assert_eq!(peer_index(Some("2".into())), 2); + assert_eq!( + peer_index(Some("not-a-number".into())), + 0, + "a malformed index is the singleton" + ); +} + +#[test] +fn a_missing_or_malformed_active_profile_names_the_file() { + let dir = tempfile::tempdir().unwrap(); + let missing = dir.path().join("absent.toml"); + let missing = missing.to_str().unwrap(); + let error = format!("{:#}", load_active_profile(missing).unwrap_err()); + assert!(error.contains(&format!("read active profile {missing}")), "{error}"); + + let malformed = dir.path().join("malformed.toml"); + std::fs::write(&malformed, "this is = = not toml").unwrap(); + let malformed = malformed.to_str().unwrap(); + let error = format!("{:#}", load_active_profile(malformed).unwrap_err()); + assert!(error.contains(&format!("parse active profile {malformed}")), "{error}"); +} From d31ab66b98747e0c118228876597a67e8ec502c2 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:20:57 -0400 Subject: [PATCH 05/12] test(network): a broken read is an error, and an empty table is empty read_frame's non-EOF error branch and Table::is_empty had no test, which put capsem-network below its coverage floor. A connection reset must surface as an error rather than read as a clean end of stream. --- crates/capsem-network/src/frames/tests.rs | 17 +++++++++++++++++ crates/capsem-network/src/switch/tests.rs | 10 ++++++++++ 2 files changed, 27 insertions(+) diff --git a/crates/capsem-network/src/frames/tests.rs b/crates/capsem-network/src/frames/tests.rs index 76dfe628f..1ec09e832 100644 --- a/crates/capsem-network/src/frames/tests.rs +++ b/crates/capsem-network/src/frames/tests.rs @@ -50,3 +50,20 @@ async fn the_largest_frame_fits_and_one_more_byte_does_not() { .await .is_err()); } + +/// A broken connection is an error, never mistaken for a clean end of stream. +#[tokio::test] +async fn a_read_error_other_than_end_of_stream_is_reported() { + struct Broken; + impl AsyncRead for Broken { + fn poll_read( + self: std::pin::Pin<&mut Self>, + _cx: &mut std::task::Context<'_>, + _buf: &mut tokio::io::ReadBuf<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Err(io::Error::new(io::ErrorKind::ConnectionReset, "reset"))) + } + } + let error = read_frame(&mut Broken, &mut Vec::new()).await.unwrap_err(); + assert_eq!(error.kind(), io::ErrorKind::ConnectionReset); +} diff --git a/crates/capsem-network/src/switch/tests.rs b/crates/capsem-network/src/switch/tests.rs index fc3e1b796..1a4391b04 100644 --- a/crates/capsem-network/src/switch/tests.rs +++ b/crates/capsem-network/src/switch/tests.rs @@ -237,3 +237,13 @@ fn drop_reasons_index_their_own_counter_slot() { names.dedup(); assert_eq!(names.len(), DropReason::ALL.len()); } + +#[test] +fn a_table_is_empty_until_a_port_is_plugged_and_after_the_last_leaves() { + let mut table = Table::default(); + assert!(table.is_empty()); + table.plug(A.mac, "a"); + assert!(!table.is_empty()); + table.unplug(&A.mac); + assert!(table.is_empty()); +} From c040effe238640ac5821c2e057961ee9cc8459de Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:20:57 -0400 Subject: [PATCH 06/12] chore(gate): raise the Darwin coverage floors to what the crates now measure The complete gate's per-crate ratchet requires a floor within 3 points of its measurement. Raised to measured minus 2.5, per the ratchet's own guidance, for capsem, agent, api, config, foundation, logger, mcp-builtin, network, process, router and service, in the Darwin table since they were measured on macOS. --- config/gate.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/gate.toml b/config/gate.toml index a80cdcd77..01ad85835 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -2368,7 +2368,7 @@ rust_coverage_crate_floors = { capsem = 50.0, capsem-admin = 71.0, capsem-agent # macOS compiles a different cfg-selected line inventory. Its stronger floors # must not raise Linux thresholds using code that Linux cannot compile or test. # Overrides may only increase the shared floors; no platform gets an exemption. -rust_coverage_platform_crate_floors = { Linux = {}, Darwin = { capsem-agent = 50.29, capsem-assets = 90.3, capsem-bench = 50.56, capsem-core = 81.0, capsem-foundation = 81.0, capsem-mock-server = 58.6, capsem-network = 96.62, capsem-process = 45.18, capsem-proto = 91.54, capsem-router = 73.73, capsem-service = 69.51 } } +rust_coverage_platform_crate_floors = { Linux = {}, Darwin = { capsem = 51.27, capsem-agent = 53.13, capsem-api = 95.34, capsem-assets = 90.3, capsem-bench = 50.56, capsem-config = 89.69, capsem-core = 81.0, capsem-foundation = 83.8, capsem-logger = 91.88, capsem-mcp-builtin = 56.56, capsem-mock-server = 58.6, capsem-network = 97.5, capsem-process = 48.12, capsem-proto = 91.54, capsem-router = 79.8, capsem-service = 74.73 } } # Nextest reads its profile from the environment; `--profile` on the command # line belongs to cargo-llvm-cov and would select a build profile instead. rust_test_profile_variable = "NEXTEST_PROFILE" From 948b2bc58be64abed2ae86ebc684dd40d29f44b0 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:20:57 -0400 Subject: [PATCH 07/12] fix(build): the Linux Rust base image copies every workspace member The image runs cargo fetch --locked over a manifest-only copy that held crates/ alone; sdk/rust joined the workspace outside crates/, and the cached image hid it until the 0.6.4 lockfile change invalidated it and the complete gate failed at warm-base (failed to read /src/sdk/rust/Cargo.toml). A citadel guard now checks every workspace member is copied. --- .../docker/Dockerfile.linux-rust-base | 3 ++ ...st_linux_rust_base_copies_the_workspace.py | 43 +++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 tests/citadel/test_linux_rust_base_copies_the_workspace.py diff --git a/build_system/docker/Dockerfile.linux-rust-base b/build_system/docker/Dockerfile.linux-rust-base index 97b4a59b5..ecaf1656a 100644 --- a/build_system/docker/Dockerfile.linux-rust-base +++ b/build_system/docker/Dockerfile.linux-rust-base @@ -19,6 +19,9 @@ FROM ${BASE} # exists for. COPY Cargo.toml Cargo.lock rust-toolchain.toml /src/ COPY crates /src/crates +# Every workspace member, or `cargo fetch --locked` cannot load the workspace; +# tests/citadel/test_linux_rust_base_copies_the_workspace.py holds the list. +COPY sdk/rust /src/sdk/rust WORKDIR /src diff --git a/tests/citadel/test_linux_rust_base_copies_the_workspace.py b/tests/citadel/test_linux_rust_base_copies_the_workspace.py new file mode 100644 index 000000000..97d391376 --- /dev/null +++ b/tests/citadel/test_linux_rust_base_copies_the_workspace.py @@ -0,0 +1,43 @@ +"""Citadel guard: the Linux Rust base image copies every workspace member. + +The image runs `cargo fetch --locked` over a manifest-only copy of the tree, +and cargo cannot load a workspace with a member missing. `sdk/rust` joined the +workspace outside `crates/`, the Dockerfile kept copying `crates` alone, and +nothing noticed while the image stayed cached: the first lockfile change that +invalidated it failed the complete gate at `warm-base` with "failed to read +/src/sdk/rust/Cargo.toml". +""" + +from __future__ import annotations + +import tomllib +from pathlib import Path + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +DOCKERFILE = PROJECT_ROOT / "build_system" / "docker" / "Dockerfile.linux-rust-base" + +RATIONALE = """\ +Dockerfile.linux-rust-base must COPY every Cargo workspace member into /src. +A member left out makes `cargo fetch --locked` fail to load the workspace, and +the failure hides until the cached image is next rebuilt. +""" + + +def _copied_roots() -> set[str]: + roots: set[str] = set() + for line in DOCKERFILE.read_text(encoding="utf-8").splitlines(): + words = line.split() + if words[:1] == ["COPY"] and not any(word.startswith("--from") for word in words): + roots.update(word.rstrip("/") for word in words[1:-1]) + return roots + + +def test_every_workspace_member_is_copied_before_the_fetch() -> None: + manifest = tomllib.loads((PROJECT_ROOT / "Cargo.toml").read_text(encoding="utf-8")) + copied = _copied_roots() + missing = [ + member + for member in manifest["workspace"]["members"] + if not any(member == root or member.startswith(f"{root}/") for root in copied) + ] + assert not missing, RATIONALE + f"\nnot copied: {missing}" From 6acbe71a18d40c11be2d0b6ea087f69825e08a47 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:33:01 -0400 Subject: [PATCH 08/12] test(fs-monitor): wait for the kernel to record the rewrite's ctime The restored-mtime forgery test rewrote the file straight after its first write. Linux stamps inodes from a coarse clock that advances every few milliseconds, so both writes shared one ctime and nothing in the metadata could show the change; macOS stamps finer, so it only failed in the Linux coverage lane (reached for the first time once the Linux Rust base image built again). The test now rewrites until the kernel records a new ctime, the precondition its claim is about, and documents the granularity limit. --- crates/capsem-core/src/fs_monitor/tests.rs | 26 ++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/crates/capsem-core/src/fs_monitor/tests.rs b/crates/capsem-core/src/fs_monitor/tests.rs index df5802b10..3a0775399 100644 --- a/crates/capsem-core/src/fs_monitor/tests.rs +++ b/crates/capsem-core/src/fs_monitor/tests.rs @@ -714,8 +714,20 @@ fn pin_mtime(path: &Path) { assert_eq!(rc, 0, "utimensat: {}", std::io::Error::last_os_error()); } +fn ctime(path: &Path) -> (i64, i64) { + use std::os::unix::fs::MetadataExt; + let metadata = std::fs::metadata(path).unwrap(); + (metadata.ctime(), metadata.ctime_nsec()) +} + /// The forgery this guards: rewrite a file in place to the same length, then /// put its mtime back. Size and mtime say nothing happened; ctime does. +/// +/// ctime is only as fine as the kernel's timestamp clock. Linux stamps inodes +/// from a coarse clock that advances every few milliseconds, so two writes in +/// one tick share a ctime and no metadata can tell them apart. The rewrite is +/// repeated until the kernel records a new ctime -- the precondition this test +/// is about -- rather than assuming a later write always gets one. #[test] fn an_in_place_rewrite_with_a_restored_mtime_is_still_a_modification() { let root = tempfile::tempdir().unwrap(); @@ -723,9 +735,19 @@ fn an_in_place_rewrite_with_a_restored_mtime_is_still_a_modification() { std::fs::write(&path, b"aaaaa").unwrap(); pin_mtime(&path); let before = workspace_snapshot(&ContainedDir::open_root(root.path()).unwrap()); + let first = ctime(&path); - std::fs::write(&path, b"bbbbb").unwrap(); - pin_mtime(&path); + let mut attempts = 0; + loop { + std::fs::write(&path, b"bbbbb").unwrap(); + pin_mtime(&path); + if ctime(&path) != first { + break; + } + attempts += 1; + assert!(attempts < 1_000, "the kernel never advanced ctime"); + std::thread::sleep(std::time::Duration::from_millis(1)); + } let after = workspace_snapshot(&ContainedDir::open_root(root.path()).unwrap()); assert_eq!( From a9af7efdb084721721f0a3fc17b0d35ed96b313f Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 01:46:52 -0400 Subject: [PATCH 09/12] fix(build): the Linux Rust base image carries the build-system Python environment Two Rust tests hand a WARC export to warcio through uv run --project build_system. The Linux lane runs with --network none to prove the base image is complete, and the image never built that environment, so uv tried to download it and the tests failed on DNS (hidden until the base image rebuilt). The image now builds build_system/.venv and keeps uv's cache under /src, which the lane's copy leaves alone (.venv is dockerignored) and the ownership step hands to the lane user; build_system/pyproject.toml and uv.lock join the image identity so a Python dependency change rebuilds it. --- build_system/docker/Dockerfile.linux-rust-base | 14 ++++++++++++++ config/gate.toml | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/build_system/docker/Dockerfile.linux-rust-base b/build_system/docker/Dockerfile.linux-rust-base index ecaf1656a..3bcc6e378 100644 --- a/build_system/docker/Dockerfile.linux-rust-base +++ b/build_system/docker/Dockerfile.linux-rust-base @@ -64,6 +64,20 @@ RUN cd /src/web/app \ && pnpm run build \ && test -s /src/web/app/dist/index.html +# The build-system Python environment, for the Rust tests that hand an export +# to a reference reader (`warcio`) through `uv run --project build_system`. +# Built here, with network, into /src/build_system/.venv; the lane's source +# copy excludes `.venv`, so it survives, and `uv run --frozen` in the sealed +# lane finds it complete. Without it those tests tried to download packages +# inside a lane that has none and failed on DNS. +# +# The uv cache lives under /src too, handed to the lane user below: the lane +# copies a newer build_system/pyproject.toml over this one, and a rebuild of the editable +# project needs its build backend (setuptools) from a cache it can read. +ENV UV_CACHE_DIR=/src/.uv-cache +COPY build_system /src/build_system +RUN cd /src && uv sync --project build_system --frozen + # Ownership last, so it covers everything every step above created as root. # A non-root user is required: the suite chmods an asset to 0o000 and demands # the read fail, and root ignores permissions. `ubuntu:24.04` already ships a diff --git a/config/gate.toml b/config/gate.toml index 01ad85835..d2957176f 100644 --- a/config/gate.toml +++ b/config/gate.toml @@ -1590,7 +1590,7 @@ lane_tag = "capsem-linux-rust:latest" # Not the workspace manifests: `cargo fetch --locked` fetches everything the # lock resolves regardless of features, so a manifest edit that leaves # `Cargo.lock` alone leaves the sealed lane with every source it needs. -identity_inputs = ["Cargo.lock", "rust-toolchain.toml", "sdk/typescript/pnpm-lock.yaml", "web/app/pnpm-lock.yaml"] +identity_inputs = ["Cargo.lock", "rust-toolchain.toml", "sdk/typescript/pnpm-lock.yaml", "web/app/pnpm-lock.yaml", "build_system/pyproject.toml", "build_system/uv.lock"] # Named here so the lane's refusal and the recipe that answers it cannot drift # apart. `test_every_recipe_the_gate_tells_an_operator_to_run_exists` checks # this resolves to a real recipe -- the last time a message named one that did From b906b5bff85a821f154d2a93c009d066b1675e56 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 02:52:53 -0400 Subject: [PATCH 10/12] fix(service): keep main.db in the home's sessions directory The service derived main.db from its run directory's parent, while foundation's capsem_sessions_dir() says /sessions. With the run directory outside the home -- the gate puts it under /tmp for a short socket path -- every such service shared /tmp/sessions/main.db, and a copy left in ledger format v3 made each new service exit at startup (archive_state format version 3 is unsupported), which the artifacts module reported as a socket that never accepted. The runtime now opens capsem_sessions_dir(); main_db_path() reports the file the handle opened, one source of truth; test states keep their own temporary home. --- CHANGELOG.md | 7 +++++++ crates/capsem-service/src/main.rs | 10 +++++----- crates/capsem-service/src/service_runtime.rs | 7 ++++++- crates/capsem-service/src/tests.rs | 4 +++- crates/capsem-service/src/tests/lifecycle.rs | 20 ++++++++++---------- crates/capsem-service/src/vm_files.rs | 4 ++-- 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 239243aeb..06a132ef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -191,6 +191,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The global session index (`main.db`) now always lives in the Capsem home's + `sessions/` directory. The service derived it from the run directory's + parent, so a service started with `CAPSEM_RUN_DIR` outside the home put it + somewhere else -- every such service under `/tmp` shared + `/tmp/sessions/main.db`, and a copy left in an older ledger format stopped + each new service at startup. The default layout is unchanged. + - Forking a running sandbox now carries everything its session ledger recorded up to the fork. The ledger writer keeps accepted rows in memory until its next disk flush, up to five seconds away, and the fork copied the diff --git a/crates/capsem-service/src/main.rs b/crates/capsem-service/src/main.rs index 3c4f7cfd4..5f83a164b 100644 --- a/crates/capsem-service/src/main.rs +++ b/crates/capsem-service/src/main.rs @@ -860,14 +860,14 @@ impl ServiceState { Ok(path) } - /// Path to main.db (global session index). - /// Layout: run_dir = ~/.capsem/run, main.db lives at ~/.capsem/sessions/main.db. + /// Path to main.db (global session index): the file the service opened. + /// It lives in the home's sessions directory, never beside the run directory. fn main_db_path(&self) -> PathBuf { - main_db_path_for_run_dir(&self.run_dir) + self.profile_mutation_db.path().to_path_buf() } - fn open_profile_mutation_db_handle(run_dir: &StdPath) -> anyhow::Result> { - let db_path = main_db_path_for_run_dir(run_dir); + fn open_profile_mutation_db_handle(sessions_dir: &StdPath) -> anyhow::Result> { + let db_path = main_db_path_in(sessions_dir); capsem_logger::ensure_session_index_schema(&db_path) .with_context(|| format!("failed to initialize session index in main.db: {}", db_path.display()))?; let started = std::time::Instant::now(); diff --git a/crates/capsem-service/src/service_runtime.rs b/crates/capsem-service/src/service_runtime.rs index 7d10985df..609c78cf8 100644 --- a/crates/capsem-service/src/service_runtime.rs +++ b/crates/capsem-service/src/service_runtime.rs @@ -246,7 +246,12 @@ pub(super) async fn run_service() -> Result<()> { .map_err(|AppError(_, message)| anyhow!("failed to build profile MCP default cache: {message}"))?; let profile_plugin_policy_cache = build_profile_plugin_policy_cache(None) .map_err(|AppError(_, message)| anyhow!("failed to build profile plugin cache: {message}"))?; - let profile_mutation_db = ServiceState::open_profile_mutation_db_handle(&run_dir)?; + // The home's sessions directory, not the run directory's parent: a run + // directory placed elsewhere (a short socket path under /tmp) once put + // every such service on one shared main.db, where a stale ledger format + // stopped each new service before it could listen. + let profile_mutation_db = + ServiceState::open_profile_mutation_db_handle(&capsem_foundation::paths::capsem_sessions_dir())?; let state = Arc::new(ServiceState { instances: Mutex::new(HashMap::new()), session_db_handles: Mutex::new(HashMap::new()), diff --git a/crates/capsem-service/src/tests.rs b/crates/capsem-service/src/tests.rs index 6988ae39f..7ca34f4f4 100644 --- a/crates/capsem-service/src/tests.rs +++ b/crates/capsem-service/src/tests.rs @@ -69,8 +69,10 @@ fn test_profile_plugin_policy_cache() -> Mutex Arc { - ServiceState::open_profile_mutation_db_handle(run_dir).unwrap() + ServiceState::open_profile_mutation_db_handle(&run_dir.parent().unwrap().join("sessions")).unwrap() } pub(crate) fn make_test_state() -> Arc { diff --git a/crates/capsem-service/src/tests/lifecycle.rs b/crates/capsem-service/src/tests/lifecycle.rs index 431f71fb7..67079b2e4 100644 --- a/crates/capsem-service/src/tests/lifecycle.rs +++ b/crates/capsem-service/src/tests/lifecycle.rs @@ -857,23 +857,23 @@ fn clear_resume_checkpoint_removes_completion_marker() { // main_db_path #[test] -fn main_db_path_resolves_to_sessions_dir() { +fn main_db_path_is_the_file_the_service_opened() { let state = make_test_state(); - // run_dir = /tmp/capsem-test-svc => parent = /tmp => main.db = /tmp/sessions/main.db - let path = state.main_db_path(); - assert!(path.ends_with("sessions/main.db"), "got: {}", path.display()); + assert_eq!(state.main_db_path(), state.profile_mutation_db.path()); + assert!(state.main_db_path().ends_with("sessions/main.db")); } +/// main.db is in the home's sessions dir, wherever the run dir is: taken from the +/// run dir's parent, every service run under /tmp shared /tmp/sessions/main.db, +/// and a stale ledger format there stopped each new service before it listened. #[test] fn profile_mutation_db_startup_initializes_session_index_schema() { let dir = tempfile::tempdir().unwrap(); - let run_dir = dir.path().join("run"); - std::fs::create_dir_all(&run_dir).unwrap(); - - let handle = ServiceState::open_profile_mutation_db_handle(&run_dir).unwrap(); + let sessions = dir.path().join("home").join("sessions"); + let handle = ServiceState::open_profile_mutation_db_handle(&sessions).unwrap(); + let db_path = handle.path().to_path_buf(); drop(handle); - - let db_path = main_db_path_for_run_dir(&run_dir); + assert_eq!(db_path, sessions.join("main.db")); let conn = rusqlite::Connection::open(&db_path).unwrap(); let session_count: i64 = conn .query_row("SELECT COUNT(*) FROM sessions", [], |row| row.get(0)) diff --git a/crates/capsem-service/src/vm_files.rs b/crates/capsem-service/src/vm_files.rs index bc779b5b4..bc3d7ba70 100644 --- a/crates/capsem-service/src/vm_files.rs +++ b/crates/capsem-service/src/vm_files.rs @@ -15,8 +15,8 @@ pub(crate) use diagnostics::{session_db_triage, session_triage_statements}; pub(crate) use fork::{clone_session_state, handle_fork}; pub(super) use ipc_command::send_ipc_command; -pub(super) fn main_db_path_for_run_dir(run_dir: &StdPath) -> PathBuf { - run_dir.parent().unwrap_or(run_dir).join("sessions").join("main.db") +pub(super) fn main_db_path_in(sessions_dir: &StdPath) -> PathBuf { + sessions_dir.join("main.db") } pub(super) fn gib(bytes: u64) -> u64 { From 2c588313346693eb1c0e5adae3adde3873c3f62b Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 02:52:54 -0400 Subject: [PATCH 11/12] fix(gate): report a service that exits before listening, at once WaitForSocket spent its whole readiness budget on a daemon that had already exited with a clear error, then reported that it did not accept a connection -- a hang, where there was a crash. It now checks the pid the launch recorded on every attempt and fails immediately, naming the pid and where its log is. --- build_system/builder/gate/pidfiles.py | 4 ++-- build_system/builder/gate/service.py | 9 +++++++ .../tests/gate/test_gate_smoke_lifecycle.py | 24 +++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) diff --git a/build_system/builder/gate/pidfiles.py b/build_system/builder/gate/pidfiles.py index 34eefde31..49ad0c424 100644 --- a/build_system/builder/gate/pidfiles.py +++ b/build_system/builder/gate/pidfiles.py @@ -123,7 +123,7 @@ def stop(pidfile: Path, settings: gate_config.PidfileConfig) -> None: but a *named* process that survives both signals is. """ pidfile = Path(pidfile) - recorded = _recorded_pid(pidfile) + recorded = recorded_pid(pidfile) if recorded is not None and running(recorded, settings): os.kill(recorded, signal.SIGTERM) @@ -137,7 +137,7 @@ def stop(pidfile: Path, settings: gate_config.PidfileConfig) -> None: pidfile.unlink(missing_ok=True) -def _recorded_pid(pidfile: Path) -> int | None: +def recorded_pid(pidfile: Path) -> int | None: try: recorded = pidfile.read_text(encoding="utf-8").strip() except (OSError, ValueError): diff --git a/build_system/builder/gate/service.py b/build_system/builder/gate/service.py index dcbf8d382..12d4154b7 100644 --- a/build_system/builder/gate/service.py +++ b/build_system/builder/gate/service.py @@ -151,6 +151,7 @@ def perform(self, context: Context) -> None: directory = self._directory or run_dir(context.config) path = directory / settings.socket + pidfile = directory / settings.pidfile for _ in range(settings.ready_attempts): if path.exists(): probe = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM) @@ -161,6 +162,14 @@ def perform(self, context: Context) -> None: pass finally: probe.close() + # A daemon that died is not a slow one: say so now, rather than + # spend the budget and report a connection it could never accept. + pid = pidfiles.recorded_pid(pidfile) + if pid is not None and not pidfiles.running(pid, context.config.pidfiles): + raise GateError( + f"capsem-service (pid {pid}) exited before it listened on {path}; " + f"its log is in {directory}" + ) time.sleep(settings.ready_interval_seconds) raise GateError( diff --git a/build_system/tests/gate/test_gate_smoke_lifecycle.py b/build_system/tests/gate/test_gate_smoke_lifecycle.py index a252a1298..837f3e24b 100644 --- a/build_system/tests/gate/test_gate_smoke_lifecycle.py +++ b/build_system/tests/gate/test_gate_smoke_lifecycle.py @@ -170,3 +170,27 @@ def test_a_failure_preserves_evidence_before_anything_is_released( raise GateError("boom") assert order.index("preserve") < order.index("release") + + +def test_a_service_that_exits_before_listening_is_reported_at_once(tmp_path: Path) -> None: + """A daemon that died is not a slow daemon. + + capsem-service refused a stale main.db and exited within a second, and the + wait spent the whole readiness budget before calling it "did not accept a + connection" -- which sent the diagnosis looking for a hang. + """ + import subprocess + import time + + from capsem_builder.gate.context import Context + from capsem_builder.gate.service import WaitForSocket + + exited = subprocess.Popen(["true"]) + exited.wait() + (tmp_path / CONFIG.service.pidfile).write_text(str(exited.pid), encoding="utf-8") + + started = time.monotonic() + with pytest.raises(GateError, match="exited before it listened"): + WaitForSocket(tmp_path).perform(Context(RecordingRunner(PROJECT_ROOT), CONFIG)) + budget = CONFIG.service.ready_attempts * CONFIG.service.ready_interval_seconds + assert time.monotonic() - started < budget / 2, "it waited out the readiness budget" From e40f4426c09f88cdae00d4886e6e201b27c0fef0 Mon Sep 17 00:00:00 2001 From: Elie Bursztein Date: Sat, 26 Sep 2026 03:36:07 -0400 Subject: [PATCH 12/12] test: read main.db from the home, as the service now writes it The integration helper located main.db by the old run-root rule, which the service no longer follows; check_session.py, doctor_session_test.py and list_sessions.py already used CAPSEM_HOME/sessions/main.db. The service home-layout guard keeps its rule (harnesses use the installed layout) with its text updated: the shared-ledger hazard it described is fixed in the service. --- .../tests/helpers/integration_test.py | 3 ++- .../test_integration_script_profiles.py | 2 +- tests/citadel/test_service_home_layout.py | 22 +++++++++---------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/build_system/tests/helpers/integration_test.py b/build_system/tests/helpers/integration_test.py index 6b23bd24a..4f4a02b7f 100644 --- a/build_system/tests/helpers/integration_test.py +++ b/build_system/tests/helpers/integration_test.py @@ -86,7 +86,8 @@ def _integration_runtime_root() -> Path: CAPSEM_HOME = INTEGRATION_HOME PERSISTENT_DIR = INTEGRATION_RUN_DIR / "persistent" -MAIN_DB = INTEGRATION_RUNTIME_ROOT / "sessions" / "main.db" +# The service keeps main.db in its home, never beside the run directory. +MAIN_DB = CAPSEM_HOME / "sessions" / "main.db" SERVICE_SOCKET = INTEGRATION_RUN_DIR / "service.sock" SERVICE_PIDFILE = INTEGRATION_RUN_DIR / "service.pid" diff --git a/build_system/tests/scripts/test_integration_script_profiles.py b/build_system/tests/scripts/test_integration_script_profiles.py index ef2798c8b..7d5aa5b46 100644 --- a/build_system/tests/scripts/test_integration_script_profiles.py +++ b/build_system/tests/scripts/test_integration_script_profiles.py @@ -182,7 +182,7 @@ def test_integration_script_service_paths_use_process_scoped_isolated_home(): assert module.INTEGRATION_RUN_DIR == module.INTEGRATION_RUNTIME_ROOT / "run" assert module.SERVICE_SOCKET == module.INTEGRATION_RUN_DIR / "service.sock" assert module.PERSISTENT_DIR == module.INTEGRATION_RUN_DIR / "persistent" - assert module.MAIN_DB == module.INTEGRATION_RUNTIME_ROOT / "sessions" / "main.db" + assert module.MAIN_DB == module.CAPSEM_HOME / "sessions" / "main.db" assert len(os.fsencode(module.SERVICE_SOCKET)) < 108 diff --git a/tests/citadel/test_service_home_layout.py b/tests/citadel/test_service_home_layout.py index c6159f699..3447c9605 100644 --- a/tests/citadel/test_service_home_layout.py +++ b/tests/citadel/test_service_home_layout.py @@ -1,12 +1,13 @@ -"""Citadel guard: a test service owns its session ledger. +"""Citadel guard: a test service runs in the installed home/run layout. -The service keeps the main session ledger at `run_dir.parent/sessions/main.db` -(`main_db_path_for_run_dir`), matching the installed layout where CAPSEM_HOME -owns a `run/` directory. A harness that sets CAPSEM_RUN_DIR and CAPSEM_HOME to -the same temporary directory moves that ledger up into the run-wide temporary -parent, so every service started that way in one pytest run shares a single -main.db. The e2e harness did, and its exec tests failed only when other -workers ran beside it, never alone. +The service used to keep the main session ledger at +`run_dir.parent/sessions/main.db`, so a harness that set CAPSEM_RUN_DIR and +CAPSEM_HOME to one temporary directory moved that ledger into the run-wide +temporary parent, and every service started that way shared one main.db. The +e2e harness did, and its exec tests failed only when other workers ran beside +it. The service now roots main.db in CAPSEM_HOME itself; this guard still keeps +harnesses in the installed layout, where CAPSEM_HOME owns a `run/` directory, +so what they test is what ships. """ from __future__ import annotations @@ -21,9 +22,8 @@ SERVICE_HOME_LAYOUT_RATIONALE = """\ A test service must use the installed home/run layout: CAPSEM_HOME owns a run/ directory, and CAPSEM_RUN_DIR is that run/ directory -(helpers.service.make_service_home_run_dirs). Pointing both at one directory -puts sessions/main.db in the run-wide temporary parent, shared by every -parallel worker's service. +(helpers.service.make_service_home_run_dirs). A collapsed layout tests a shape +no installation has. """