From 958e907caa08057160c29939f9c6500138266834 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:01:20 +0200 Subject: [PATCH 01/25] docs: design terminal journal diagnostics --- ...-23-journal-terminal-diagnostics-design.md | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-23-journal-terminal-diagnostics-design.md diff --git a/docs/superpowers/specs/2026-08-23-journal-terminal-diagnostics-design.md b/docs/superpowers/specs/2026-08-23-journal-terminal-diagnostics-design.md new file mode 100644 index 0000000..b182c00 --- /dev/null +++ b/docs/superpowers/specs/2026-08-23-journal-terminal-diagnostics-design.md @@ -0,0 +1,92 @@ +# Journal Terminal Diagnostics Design + +## Purpose + +Issue #63 identified a terminal CCP run whose durable journal contained only +`failure_kind: "unknown"`. The journal did not retain a safe diagnostic and no +cache artifact existed, so the event cannot be investigated or safely retried +from the retained evidence. + +This change makes future unmatched top-level failures actionable without +weakening the fail-closed recovery model or persisting sensitive execution +data. It does not reinterpret the historical run and it does not authorize a +new R5 run. + +## Scope and non-goals + +In scope: + +- Persist a bounded, enum-only diagnostic for a terminal `unknown` failure. +- Surface the diagnostic through the read-only recovery status JSON. +- Preserve strict parsing and terminal non-actionability. +- Prove the behavior with a deterministic synthetic failure after `executing`. + +Out of scope: + +- Capturing `Display` or debug error text, paths, environment variables, + commands, check output, or secrets. +- Retrying, recovering, running Docker, publishing a receipt, or qualifying + R5. +- Changing existing v1 journal entries, ownership markers, or source-binding + token derivation. + +## Chosen design + +Keep the existing `RunJournalEntryV1` wire contract unchanged. Instead, add a +separate owned `terminal-diagnostic-v1.json` artifact inside the exact run +directory. Its strict, bounded payload contains: + +- the existing `schema_version` and exact `run_id`; +- the terminal `failure_kind` (which must be `unknown`); and +- `diagnostic_code`, a closed enum initially containing + `internal_command_failure` and `unclassified_top_level`. + +The artifact is written before the terminal journal transition. Thus, if a +`failed` entry is durable, the new writer attempted to preserve its diagnostic; +if the later transition fails, recovery still sees the last non-terminal state +and remains fail-closed. Historic v1 journals legitimately have no diagnostic +artifact and remain readable. + +`RunJournalStore` validates the artifact as an owned regular file with the +same small-size limit as markers. A present-but-invalid artifact makes the run +operator-required rather than silently accepted. The recovery status model +gains an optional bounded `failure_diagnostic` field only when a valid artifact +exists. `recover apply` continues to reject all terminal failed runs. + +## Failure classification and data flow + +`cli_failure_kind` retains its current coarse result. A companion classifier +returns a diagnostic only when that result is `unknown`: + +1. `CliError::Internal(_)` maps to `internal_command_failure`. +2. Any other currently-unmapped top-level `CliError` maps to + `unclassified_top_level`. +3. Mapped error domains retain their present `failure_kind` and no new + diagnostic artifact. + +`JournalLifecycleObserver::fail` accepts the coarse kind and optional +diagnostic. It delegates to the store so artifact persistence and terminal +transition remain the only write path. No raw error value is serialized. + +## Compatibility and safety + +An additive field on `RunJournalEntryV1` would make old strict readers reject +new entries because they use `deny_unknown_fields`. A standalone owned v1 +artifact avoids changing that public entry schema and avoids changing the +global marker/version contract. Old journals without the artifact are valid; +unknown future artifact fields or versions fail closed. + +The artifact is not an authorization signal. Recovery classification continues +to derive from the terminal lifecycle state. A diagnostic never changes a +failed run into a restartable or recoverable run. + +## Verification + +TDD starts with a unit-level synthetic `CliError::Internal` after the journal +has reached `executing`. The red test asserts a terminal `unknown` failure, +the safe code, absence of the synthetic error text, and no receipt behavior. +Additional tests prove strict artifact parsing, legacy no-artifact readability, +read-only recovery status, terminal `recover apply` rejection, and generated +schema/serialization redaction. Focused Rust tests precede the appropriate +full test suite. No CCP `run`, Docker, network, or R5 activity is part of this +verification. From 73cb7c9eb7fb8b8aeb7175d5fefb4e32f3a72921 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:06:43 +0200 Subject: [PATCH 02/25] docs: plan terminal journal diagnostics --- ...2026-08-23-journal-terminal-diagnostics.md | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-23-journal-terminal-diagnostics.md diff --git a/docs/superpowers/plans/2026-08-23-journal-terminal-diagnostics.md b/docs/superpowers/plans/2026-08-23-journal-terminal-diagnostics.md new file mode 100644 index 0000000..cf02bdb --- /dev/null +++ b/docs/superpowers/plans/2026-08-23-journal-terminal-diagnostics.md @@ -0,0 +1,154 @@ +# Journal Terminal Diagnostics Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Preserve an actionable, redacted diagnostic for future top-level CCP failures that otherwise end in a terminal `unknown` journal record. + +**Architecture:** Keep `RunJournalEntryV1` and all v1 markers unchanged. A strict owned `terminal-diagnostic-v1.json` sidecar is written before a failed transition only for unknown failures, then exposed as an optional safe value in read-only recovery status. The top-level classifier supplies only closed enum codes, never a raw error value. + +**Tech Stack:** Rust, serde, schemars, existing durable filesystem and run-journal tests, Cargo integration tests. + +**Spec:** `docs/superpowers/specs/2026-08-23-journal-terminal-diagnostics-design.md` + +## Global Constraints + +- Do not alter `RunJournalEntryV1`, `RUN_JOURNAL_SCHEMA_VERSION`, ownership-marker semantics, or source-binding token derivation. +- Persist only bounded enums: no error `Display`/`Debug`, path, environment, command, check output, or secret. +- Historic journals without a diagnostic sidecar remain readable and terminal failures remain non-actionable. +- A malformed or unknown sidecar is fail-closed and must surface as operator-required / a recovery error, not as a trusted diagnosis. +- Tests must be synthetic; do not invoke CCP `run`, Docker, network, R5, receipt publication, or GitHub mutation. + +--- + +### Task 1: Strict terminal-diagnostic sidecar and recovery projection + +**Files:** +- Modify: `src/run_journal.rs:22-147, 267-285, 342-494, 671-685, 780-1055` +- Test: `src/run_journal.rs:780-1055` +- Test: `tests/recover_cli.rs:104-169` + +**Interfaces:** +- Consumes: existing `RunJournalStore`, `RunFailureKindV1`, `RunJournalStateV1`, `RecoveryRunStatusV1`. +- Produces: `RunFailureDiagnosticCodeV1`, `RunFailureDiagnosticV1`, `RunJournalStore::fail`, and optional `RecoveryRunStatusV1::failure_diagnostic` for Task 2. + +- [ ] **Step 1: Write the failing unit tests for a safe terminal diagnostic** + +Add tests that construct a created journal, reach `executing`, and expect an absent `RunJournalStore::fail` API to write a `failed/unknown` record carrying `internal_command_failure` in `status()`. Add a legacy journal test that still writes `transition(... Failed, Some(Unknown))` with no sidecar and remains terminal. Add a tampered sidecar test that expects `status()` to classify the run as `operator_required`. + +```rust +store.transition(RUN_ID, RunJournalStateV1::Executing, AT, None)?; +store.fail( + RUN_ID, + AT, + RunFailureKindV1::Unknown, + Some(RunFailureDiagnosticCodeV1::InternalCommandFailure), +)?; +let status = store.status()?; +assert_eq!(status.runs[0].failure_diagnostic.as_ref().unwrap().diagnostic_code, + RunFailureDiagnosticCodeV1::InternalCommandFailure); +``` + +- [ ] **Step 2: Run the focused test target and observe RED** + +Run: `cargo test run_journal::tests::terminal_unknown_diagnostic_is_projected --lib` + +Expected: compilation failure because `RunJournalStore::fail`, `RunFailureDiagnosticCodeV1`, and `failure_diagnostic` do not exist yet. + +- [ ] **Step 3: Implement the smallest strict sidecar contract** + +In `src/run_journal.rs`, add the private filename constant `terminal-diagnostic-v1.json`; public serde/schemars `RunFailureDiagnosticCodeV1` and `RunFailureDiagnosticV1`; and optional `failure_diagnostic` on `RecoveryRunStatusV1`. Implement `RunJournalStore::fail(run_id, at_utc, failure_kind, diagnostic_code)` so a code is allowed only with `Unknown`, serializes a strict sidecar before delegating to `transition(... Failed, ...)`, and refuses duplicate or invalid writes. Filter only this exact filename from journal entries. + +Implement a strict sidecar reader that checks regular-file type, small size, exact schema version, run ID, `failure_kind == Unknown`, and the closed enum. `status()` and terminal `apply()` must read it when it is present; malformed data must not be trusted. Old failed entries with no file remain valid. + +- [ ] **Step 4: Run focused unit tests and observe GREEN** + +Run: `cargo test run_journal::tests --lib` + +Expected: journal transition, legacy compatibility, sidecar projection, redaction, and tamper tests pass. + +- [ ] **Step 5: Add recovery CLI coverage and verify it** + +In `tests/recover_cli.rs`, create a terminal unknown failure through `store.fail` and assert `recover status --json` exposes `failure_diagnostic.diagnostic_code`; retain the existing assertion that `recover apply` exits `5` for a terminal run. Run: `cargo test --test recover_cli`. + +- [ ] **Step 6: Commit the independently testable store change** + +```bash +git add src/run_journal.rs tests/recover_cli.rs +git commit -m "feat: persist bounded terminal diagnostics" +``` + +### Task 2: Top-level classifier and synthetic `executing` regression + +**Files:** +- Modify: `src/main.rs:1392-1454, 1995-2707` +- Test: `src/main.rs` unit-test module + +**Interfaces:** +- Consumes: `RunFailureDiagnosticCodeV1` and `RunJournalStore::fail` from Task 1. +- Produces: `cli_failure_diagnostic` and a `JournalLifecycleObserver::fail` path that stores a safe diagnostic before the terminal transition. + +- [ ] **Step 1: Write the failing synthetic top-level failure test** + +In the `src/main.rs` test module, create an isolated journal, move it through `created → admitted → prepared → executing`, construct `CliError::Internal(std::io::Error::other("synthetic-top-level-secret"))`, and call the new lifecycle/classifier path. Assert the final entry has `Unknown`, status exposes `internal_command_failure`, its serialized JSON excludes `synthetic-top-level-secret`, and no receipt is created by the unit test. + +```rust +let error = CliError::internal(std::io::Error::other("synthetic-top-level-secret")); +lifecycle.fail(cli_failure_kind(&error), cli_failure_diagnostic(&error))?; +assert_eq!(diagnostic.diagnostic_code, + RunFailureDiagnosticCodeV1::InternalCommandFailure); +assert!(!serialized.contains("synthetic-top-level-secret")); +``` + +- [ ] **Step 2: Run the single test and observe RED** + +Run: `cargo test synthetic_internal_failure_after_executing_is_redacted --bin commit-ci-preflight` + +Expected: compilation failure because the lifecycle `fail` signature and `cli_failure_diagnostic` do not exist. + +- [ ] **Step 3: Implement the minimal classifier wiring** + +Extend `JournalLifecycleObserver::fail` to accept an optional diagnostic code and call `RunJournalStore::fail`. Add `cli_failure_diagnostic(&CliError) -> Option`: map `CliError::Internal(_)` to `InternalCommandFailure`; map every remaining unmatched branch that has coarse `Unknown` to `UnclassifiedTopLevel`; return `None` for known coarse kinds. Update only existing `lifecycle.fail(cli_failure_kind(&error))` call sites to pass the companion result. + +- [ ] **Step 4: Run focused binary tests and observe GREEN** + +Run: `cargo test --bin commit-ci-preflight` + +Expected: the new synthetic regression and existing main unit tests pass without invoking a real target run. + +- [ ] **Step 5: Commit the classifier integration** + +```bash +git add src/main.rs +git commit -m "fix: classify unknown terminal failures" +``` + +### Task 3: Full regression and contract review + +**Files:** +- Modify only if verification identifies a concrete defect: `src/run_journal.rs`, `src/main.rs`, `tests/recover_cli.rs` + +**Interfaces:** +- Consumes: completed Task 1 and Task 2. +- Produces: verified implementation evidence; no receipt, plan, or external state. + +- [ ] **Step 1: Run formatting and targeted complete regression** + +Run: `cargo fmt --check && cargo test --lib && cargo test --test recover_cli && cargo test --bin commit-ci-preflight` + +Expected: all selected tests pass and formatting reports no diff. + +- [ ] **Step 2: Run the full repository suite** + +Run: `cargo test` + +Expected: exit code `0`; record the exact test count/output and any environment-bound skips. + +- [ ] **Step 3: Inspect the final diff for safety boundaries** + +Run: `git diff origin/main...HEAD -- src/run_journal.rs src/main.rs tests/recover_cli.rs docs/superpowers` + +Verify: no raw error serialization; only exact sidecar is exempted from journal-entry discovery; historic no-sidecar records remain readable; failed runs remain terminal; no command starts a target run. + +- [ ] **Step 4: Commit only an evidence-driven correction, if one was necessary** + +If and only if a verification defect required a source correction, re-run the exact relevant RED/GREEN test and commit that correction separately. Otherwise make no empty commit. From ee4bc3a17af8f735d768998ba85ccab4a5b0d3a6 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:15:06 +0200 Subject: [PATCH 03/25] feat: persist bounded terminal diagnostics --- src/run_journal.rs | 234 ++++++++++++++++++++++++++++++++++++++++++- tests/recover_cli.rs | 27 ++++- 2 files changed, 255 insertions(+), 6 deletions(-) diff --git a/src/run_journal.rs b/src/run_journal.rs index 50768d3..9b9a5c9 100644 --- a/src/run_journal.rs +++ b/src/run_journal.rs @@ -26,6 +26,7 @@ const ROOT_MARKER: &str = ".ccp-run-journal-root-v1.json"; const RUN_MARKER: &str = ".ccp-run-owner-v1.json"; const RESOURCES_DIR: &str = "resources"; const SOURCE_BINDING: &str = "source-snapshot-v1.json"; +const TERMINAL_DIAGNOSTIC: &str = "terminal-diagnostic-v1.json"; const QUARANTINE_PREFIX: &str = "quarantined-"; static TOKEN_SEQUENCE: AtomicU64 = AtomicU64::new(0); @@ -97,6 +98,22 @@ pub enum RunFailureKindV1 { Unknown, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum RunFailureDiagnosticCodeV1 { + InternalCommandFailure, + UnclassifiedTopLevel, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct RunFailureDiagnosticV1 { + pub schema_version: String, + pub run_id: String, + pub failure_kind: RunFailureKindV1, + pub diagnostic_code: RunFailureDiagnosticCodeV1, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] pub struct RunJournalEntryV1 { @@ -124,6 +141,8 @@ pub struct RecoveryRunStatusV1 { pub run_id: String, pub state: Option, pub classification: RecoveryClassificationV1, + #[serde(skip_serializing_if = "Option::is_none")] + pub failure_diagnostic: Option, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema)] @@ -284,6 +303,27 @@ impl RunJournalStore { self.append_entry(run_id, previous.seq + 1, state, at_utc, failure_kind) } + pub fn fail( + &self, + run_id: &str, + at_utc: &str, + failure_kind: RunFailureKindV1, + diagnostic_code: Option, + ) -> Result { + if failure_kind != RunFailureKindV1::Unknown && diagnostic_code.is_some() { + return Err(RunJournalError::InvalidTransition); + } + if let Some(diagnostic_code) = diagnostic_code { + self.write_terminal_diagnostic(run_id, failure_kind, diagnostic_code)?; + } + self.transition( + run_id, + RunJournalStateV1::Failed, + at_utc, + Some(failure_kind), + ) + } + /// Reserve one exact CCP-owned location for ephemeral run resources. /// The directory remains inside the owned run tree so `recover apply` /// quarantines it together with an interrupted journal. @@ -365,6 +405,7 @@ impl RunJournalStore { run_id: bounded_run_id(run_id), state: None, classification, + failure_diagnostic: None, }); continue; } @@ -375,11 +416,20 @@ impl RunJournalStore { match self.read_entries(&name) { Ok(journal) if !journal.is_empty() => { let last = journal.last().expect("non-empty journal"); - runs.push(RecoveryRunStatusV1 { - run_id: name, - state: Some(last.state), - classification: classify(last.state), - }); + let diagnostic = if last.state == RunJournalStateV1::Failed { + self.read_terminal_diagnostic(&name) + } else { + Ok(None) + }; + match diagnostic { + Ok(failure_diagnostic) => runs.push(RecoveryRunStatusV1 { + run_id: name, + state: Some(last.state), + classification: classify(last.state), + failure_diagnostic, + }), + Err(_) => runs.push(operator_required(&name)), + } } Ok(_) | Err(_) => runs.push(operator_required(&name)), } @@ -406,6 +456,9 @@ impl RunJournalStore { } let entries = self.read_entries(run_id)?; let last = entries.last().ok_or(RunJournalError::Corrupt)?; + if last.state == RunJournalStateV1::Failed { + self.read_terminal_diagnostic(run_id)?; + } if classify(last.state) == RecoveryClassificationV1::Terminal { return Err(RunJournalError::NonActionable); } @@ -439,6 +492,59 @@ impl RunJournalStore { Ok(entry) } + fn write_terminal_diagnostic( + &self, + run_id: &str, + failure_kind: RunFailureKindV1, + diagnostic_code: RunFailureDiagnosticCodeV1, + ) -> Result<(), RunJournalError> { + validate_run_id(run_id)?; + if failure_kind != RunFailureKindV1::Unknown { + return Err(RunJournalError::InvalidTransition); + } + let run = self.run_path(run_id); + self.validate_run_marker(&run, run_id)?; + let diagnostic = RunFailureDiagnosticV1 { + schema_version: RUN_JOURNAL_SCHEMA_VERSION.to_owned(), + run_id: run_id.to_owned(), + failure_kind, + diagnostic_code, + }; + let mut bytes = serde_json::to_vec(&diagnostic).map_err(|_| RunJournalError::Corrupt)?; + bytes.push(b'\n'); + self.durable + .create_new(&run.join(TERMINAL_DIAGNOSTIC), &bytes)?; + Ok(()) + } + + fn read_terminal_diagnostic( + &self, + run_id: &str, + ) -> Result, RunJournalError> { + let path = self.run_path(run_id).join(TERMINAL_DIAGNOSTIC); + match fs::symlink_metadata(&path) { + Err(error) if error.kind() == io::ErrorKind::NotFound => return Ok(None), + Err(error) => return Err(RunJournalError::Io(error)), + Ok(metadata) + if metadata.file_type().is_symlink() + || !metadata.is_file() + || metadata.len() > 4096 => + { + return Err(RunJournalError::Corrupt); + } + Ok(_) => {} + } + let diagnostic: RunFailureDiagnosticV1 = + serde_json::from_slice(&fs::read(path)?).map_err(|_| RunJournalError::Corrupt)?; + if diagnostic.schema_version != RUN_JOURNAL_SCHEMA_VERSION + || diagnostic.run_id != run_id + || diagnostic.failure_kind != RunFailureKindV1::Unknown + { + return Err(RunJournalError::Corrupt); + } + Ok(Some(diagnostic)) + } + fn read_entries(&self, run_id: &str) -> Result, RunJournalError> { let run = self.run_path(run_id); validate_existing_plain_directory(&run)?; @@ -450,6 +556,7 @@ impl RunJournalStore { entry.file_name() != RUN_MARKER && entry.file_name() != RESOURCES_DIR && entry.file_name() != SOURCE_BINDING + && entry.file_name() != TERMINAL_DIAGNOSTIC }) .map(|entry| entry.path()) .collect(); @@ -785,6 +892,7 @@ fn operator_required(run_id: &str) -> RecoveryRunStatusV1 { run_id: bounded_run_id(run_id), state: None, classification: RecoveryClassificationV1::OperatorRequired, + failure_diagnostic: None, } } @@ -956,6 +1064,122 @@ mod tests { fs::remove_dir_all(root).expect("cleanup"); } + #[test] + fn terminal_unknown_diagnostic_is_projected() { + let root = cache_root("terminal-diagnostic"); + let store = RunJournalStore::initialize(&root).expect("store"); + store.create_run(RUN_ID, AT).expect("created"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); + + store + .fail( + RUN_ID, + AT, + RunFailureKindV1::Unknown, + Some(RunFailureDiagnosticCodeV1::InternalCommandFailure), + ) + .expect("terminal diagnostic"); + + let status = store.status().expect("status"); + assert_eq!( + status.runs[0].classification, + RecoveryClassificationV1::Terminal + ); + assert_eq!( + status.runs[0] + .failure_diagnostic + .as_ref() + .expect("diagnostic") + .diagnostic_code, + RunFailureDiagnosticCodeV1::InternalCommandFailure + ); + assert!(matches!( + store.apply(RUN_ID), + Err(RunJournalError::NonActionable) + )); + + fs::remove_dir_all(root).expect("cleanup"); + } + + #[test] + fn legacy_unknown_failure_without_diagnostic_remains_terminal() { + let root = cache_root("legacy-terminal-diagnostic"); + let store = RunJournalStore::initialize(&root).expect("store"); + store.create_run(RUN_ID, AT).expect("created"); + store + .transition( + RUN_ID, + RunJournalStateV1::Failed, + AT, + Some(RunFailureKindV1::Unknown), + ) + .expect("legacy terminal"); + + let status = store.status().expect("status"); + assert_eq!( + status.runs[0].classification, + RecoveryClassificationV1::Terminal + ); + assert_eq!(status.runs[0].failure_diagnostic, None); + assert!(matches!( + store.apply(RUN_ID), + Err(RunJournalError::NonActionable) + )); + + fs::remove_dir_all(root).expect("cleanup"); + } + + #[test] + fn malformed_terminal_diagnostic_requires_operator() { + let root = cache_root("malformed-terminal-diagnostic"); + let store = RunJournalStore::initialize(&root).expect("store"); + store.create_run(RUN_ID, AT).expect("created"); + store + .transition( + RUN_ID, + RunJournalStateV1::Failed, + AT, + Some(RunFailureKindV1::Unknown), + ) + .expect("terminal"); + fs::write( + store.run_path(RUN_ID).join(TERMINAL_DIAGNOSTIC), + br#"{\"schema_version\":\"1.0\",\"unexpected\":true}"#, + ) + .expect("tamper diagnostic"); + + let status = store.status().expect("status"); + assert_eq!( + status.runs[0].classification, + RecoveryClassificationV1::OperatorRequired + ); + assert!(matches!(store.apply(RUN_ID), Err(RunJournalError::Corrupt))); + + fs::remove_dir_all(root).expect("cleanup"); + } + + #[test] + fn terminal_diagnostic_payload_is_path_and_secret_free() { + let diagnostic = RunFailureDiagnosticV1 { + schema_version: RUN_JOURNAL_SCHEMA_VERSION.to_owned(), + run_id: RUN_ID.to_owned(), + failure_kind: RunFailureKindV1::Unknown, + diagnostic_code: RunFailureDiagnosticCodeV1::InternalCommandFailure, + }; + let json = serde_json::to_string(&diagnostic).expect("json"); + for forbidden in ["/Users/", "C:\\\\\\", "..", "HOME", "TOKEN", "SECRET"] { + assert!(!json.contains(forbidden)); + } + } + #[test] fn initialization_never_adopts_a_foreign_nonempty_root() { let root = cache_root("foreign-root"); diff --git a/tests/recover_cli.rs b/tests/recover_cli.rs index 086165d..14cfeee 100644 --- a/tests/recover_cli.rs +++ b/tests/recover_cli.rs @@ -15,7 +15,8 @@ use commit_ci_preflight::cache::{ CacheRootOptions, ManagedCache, PlatformFamily, ResolvedCacheRoot, }; use commit_ci_preflight::run_journal::{ - RecoveryClassificationV1, RunFailureKindV1, RunJournalStateV1, RunJournalStore, + RecoveryClassificationV1, RunFailureDiagnosticCodeV1, RunFailureKindV1, RunJournalStateV1, + RunJournalStore, }; use serde_json::Value; @@ -168,6 +169,30 @@ fn malformed_and_terminal_run_ids_fail_closed_with_stable_codes() { assert_eq!(terminal.output().expect("terminal").status.code(), Some(5)); } +#[test] +fn status_projects_bounded_terminal_diagnostic() { + let fixture = Fixture::new("terminal-diagnostic"); + let store = RunJournalStore::initialize(&fixture.cache).expect("journal"); + store.create_run(RUN_ID, AT).expect("run"); + store + .fail( + RUN_ID, + AT, + RunFailureKindV1::Unknown, + Some(RunFailureDiagnosticCodeV1::InternalCommandFailure), + ) + .expect("terminal diagnostic"); + + let output = fixture.command().arg("--json").output().expect("status"); + + assert!(output.status.success()); + let value: Value = serde_json::from_slice(&output.stdout).expect("JSON"); + assert_eq!( + value["runs"][0]["failure_diagnostic"]["diagnostic_code"], + "internal_command_failure" + ); +} + fn tree_fingerprint(root: &std::path::Path) -> Vec<(PathBuf, u64)> { fn walk(root: &std::path::Path, path: &std::path::Path, out: &mut Vec<(PathBuf, u64)>) { let mut entries: Vec<_> = fs::read_dir(path) From e101bc31aa9ef5847edbffc02187a69719608bf4 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:21:44 +0200 Subject: [PATCH 04/25] fix: reject diagnostics on nonterminal journals --- src/run_journal.rs | 64 +++++++++++++++++++++++++++++++++++++++----- tests/recover_cli.rs | 18 +++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) diff --git a/src/run_journal.rs b/src/run_journal.rs index 9b9a5c9..0d170f4 100644 --- a/src/run_journal.rs +++ b/src/run_journal.rs @@ -416,11 +416,13 @@ impl RunJournalStore { match self.read_entries(&name) { Ok(journal) if !journal.is_empty() => { let last = journal.last().expect("non-empty journal"); - let diagnostic = if last.state == RunJournalStateV1::Failed { - self.read_terminal_diagnostic(&name) - } else { - Ok(None) - }; + let diagnostic = self.read_terminal_diagnostic(&name).and_then(|diagnostic| { + if diagnostic.is_some() && last.state != RunJournalStateV1::Failed { + Err(RunJournalError::Corrupt) + } else { + Ok(diagnostic) + } + }); match diagnostic { Ok(failure_diagnostic) => runs.push(RecoveryRunStatusV1 { run_id: name, @@ -456,8 +458,10 @@ impl RunJournalStore { } let entries = self.read_entries(run_id)?; let last = entries.last().ok_or(RunJournalError::Corrupt)?; - if last.state == RunJournalStateV1::Failed { - self.read_terminal_diagnostic(run_id)?; + if self.read_terminal_diagnostic(run_id)?.is_some() + && last.state != RunJournalStateV1::Failed + { + return Err(RunJournalError::Corrupt); } if classify(last.state) == RecoveryClassificationV1::Terminal { return Err(RunJournalError::NonActionable); @@ -1109,11 +1113,48 @@ mod tests { fs::remove_dir_all(root).expect("cleanup"); } + #[test] + fn nonterminal_malformed_diagnostic_requires_operator_and_apply_is_corrupt() { + let root = cache_root("nonterminal-malformed-diagnostic"); + let store = RunJournalStore::initialize(&root).expect("store"); + store.create_run(RUN_ID, AT).expect("created"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); + fs::write( + store.run_path(RUN_ID).join(TERMINAL_DIAGNOSTIC), + br#"{"unexpected":true}"#, + ) + .expect("tamper diagnostic"); + + assert_eq!( + store.status().expect("status").runs[0].classification, + RecoveryClassificationV1::OperatorRequired + ); + assert!(matches!(store.apply(RUN_ID), Err(RunJournalError::Corrupt))); + fs::remove_dir_all(root).expect("cleanup"); + } + #[test] fn legacy_unknown_failure_without_diagnostic_remains_terminal() { let root = cache_root("legacy-terminal-diagnostic"); let store = RunJournalStore::initialize(&root).expect("store"); store.create_run(RUN_ID, AT).expect("created"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); store .transition( RUN_ID, @@ -1142,6 +1183,15 @@ mod tests { let root = cache_root("malformed-terminal-diagnostic"); let store = RunJournalStore::initialize(&root).expect("store"); store.create_run(RUN_ID, AT).expect("created"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); store .transition( RUN_ID, diff --git a/tests/recover_cli.rs b/tests/recover_cli.rs index 14cfeee..e539f5f 100644 --- a/tests/recover_cli.rs +++ b/tests/recover_cli.rs @@ -146,6 +146,15 @@ fn malformed_and_terminal_run_ids_fail_closed_with_stable_codes() { let fixture = Fixture::new("codes"); let store = RunJournalStore::initialize(&fixture.cache).expect("journal"); store.create_run(RUN_ID, AT).expect("run"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); store .transition( RUN_ID, @@ -174,6 +183,15 @@ fn status_projects_bounded_terminal_diagnostic() { let fixture = Fixture::new("terminal-diagnostic"); let store = RunJournalStore::initialize(&fixture.cache).expect("journal"); store.create_run(RUN_ID, AT).expect("run"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); store .fail( RUN_ID, From 1c075822e954d32fd0ee6b84e71e1b4547e3a961 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:27:11 +0200 Subject: [PATCH 05/25] fix: classify unknown terminal failures --- src/main.rs | 108 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 91 insertions(+), 17 deletions(-) diff --git a/src/main.rs b/src/main.rs index 3fae962..3dac0dc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -58,8 +58,8 @@ use commit_ci_preflight::run::{ SystemClock, execute_local_run_with_barrier_and_lifecycle_and_runtime_preflight, }; use commit_ci_preflight::run_journal::{ - RUN_JOURNAL_SCHEMA_VERSION, RecoveryStatusV1, RunFailureKindV1, RunJournalError, - RunJournalStateV1, RunJournalStore, + RUN_JOURNAL_SCHEMA_VERSION, RecoveryStatusV1, RunFailureDiagnosticCodeV1, RunFailureKindV1, + RunJournalError, RunJournalStateV1, RunJournalStore, }; use commit_ci_preflight::runtime::{ DockerRuntimeCapabilityProbe, DryRunPlan, RuntimeError, RuntimeProbe, doctor_guard, @@ -908,7 +908,7 @@ fn print_run( ) { Ok(commit) => commit, Err(error) => { - lifecycle.fail(RunFailureKindV1::PreparationFailed)?; + lifecycle.fail(RunFailureKindV1::PreparationFailed, None)?; return Err(CliError::Run(RunError::SourceSnapshot(error))); } }; @@ -922,7 +922,7 @@ fn print_run( let source_resource = match journal.reserve_resource(&journal_id, "source-snapshot-v1") { Ok(path) => path, Err(error) => { - lifecycle.fail(RunFailureKindV1::PreparationFailed)?; + lifecycle.fail(RunFailureKindV1::PreparationFailed, None)?; return Err(CliError::RunJournal(error)); } }; @@ -937,12 +937,12 @@ fn print_run( ) { Ok(snapshot) => snapshot, Err(error) => { - lifecycle.fail(RunFailureKindV1::PreparationFailed)?; + lifecycle.fail(RunFailureKindV1::PreparationFailed, None)?; return Err(CliError::Run(RunError::SourceSnapshot(error))); } }; if let Err(error) = source_snapshot.prepare_mount_overlay(&envelope) { - lifecycle.fail(RunFailureKindV1::PreparationFailed)?; + lifecycle.fail(RunFailureKindV1::PreparationFailed, None)?; return Err(CliError::Run(RunError::SourceSnapshot(error))); } if let Err(error) = journal.bind_source( @@ -951,7 +951,7 @@ fn print_run( &source_snapshot.evidence().manifest_digest, source_snapshot.evidence().entry_count, ) { - lifecycle.fail(RunFailureKindV1::PreparationFailed)?; + lifecycle.fail(RunFailureKindV1::PreparationFailed, None)?; return Err(CliError::RunJournal(error)); } let admission = @@ -962,7 +962,7 @@ fn print_run( ) { Ok(guard) => guard, Err(error) => { - lifecycle.fail(RunFailureKindV1::AdmissionRejected)?; + lifecycle.fail(RunFailureKindV1::AdmissionRejected, None)?; return Err(CliError::Admission(error)); } }; @@ -970,7 +970,7 @@ fn print_run( if let Err(error) = resource_pre_start(supervisor.clone(), &cancellation) { return match guard.release() { Ok(()) => { - lifecycle.fail(cli_failure_kind(&error))?; + lifecycle.fail(cli_failure_kind(&error), cli_failure_diagnostic(&error))?; Err(error) } Err(release_error) => { @@ -1024,7 +1024,7 @@ fn print_run( Ok(()) => match outcome { Ok(outcome) => outcome, Err(error) => { - lifecycle.fail(cli_failure_kind(&error))?; + lifecycle.fail(cli_failure_kind(&error), cli_failure_diagnostic(&error))?; return Err(error); } }, @@ -1113,7 +1113,7 @@ fn print_matrix_run( ) { Ok(guard) => guard, Err(error) => { - lifecycle.fail(RunFailureKindV1::AdmissionRejected)?; + lifecycle.fail(RunFailureKindV1::AdmissionRejected, None)?; return Err(CliError::Admission(error)); } }; @@ -1121,7 +1121,7 @@ fn print_matrix_run( if let Err(error) = resource_pre_start(supervisor.clone(), &cancellation) { return match guard.release() { Ok(()) => { - lifecycle.fail(cli_failure_kind(&error))?; + lifecycle.fail(cli_failure_kind(&error), cli_failure_diagnostic(&error))?; Err(error) } Err(release_error) => { @@ -1173,7 +1173,7 @@ fn print_matrix_run( Ok(()) => match result { Ok(outcome) => outcome, Err(error) => { - lifecycle.fail(cli_failure_kind(&error))?; + lifecycle.fail(cli_failure_kind(&error), cli_failure_diagnostic(&error))?; return Err(error); } }, @@ -1263,8 +1263,16 @@ impl JournalLifecycleObserver<'_> { Ok(()) } - fn fail(&mut self, kind: RunFailureKindV1) -> Result<(), CliError> { - self.transition_state(RunJournalStateV1::Failed, Some(kind)) + fn fail( + &mut self, + kind: RunFailureKindV1, + diagnostic_code: Option, + ) -> Result<(), CliError> { + let at_utc = self.clock.now_utc().map_err(CliError::Run)?; + self.store + .fail(self.run_id, &at_utc, kind, diagnostic_code) + .map_err(CliError::RunJournal)?; + Ok(()) } } @@ -1309,6 +1317,16 @@ fn cli_failure_kind(error: &CliError) -> RunFailureKindV1 { } } +fn cli_failure_diagnostic(error: &CliError) -> Option { + match error { + CliError::Internal(_) => Some(RunFailureDiagnosticCodeV1::InternalCommandFailure), + _ if cli_failure_kind(error) == RunFailureKindV1::Unknown => { + Some(RunFailureDiagnosticCodeV1::UnclassifiedTopLevel) + } + _ => None, + } +} + fn load_plan(path: &Path) -> Result { ConfigV1::load(path) .map_err(CliError::usage)? @@ -2299,8 +2317,9 @@ mod tests { use super::{ Cli, CliError, GuardCommand, GuardExecArgs, GuardExecError, ResourceCacheStateArg, ResourceExecutionModeArg, ResourceExecutorArg, WatchdogCompletionBarrier, - detect_resource_executor, finalize_guard_exec_result, new_journal_id, - reconcile_watchdog_outcome, resource_run_outcome, resource_terminal_detail, + cli_failure_diagnostic, cli_failure_kind, detect_resource_executor, + finalize_guard_exec_result, new_journal_id, reconcile_watchdog_outcome, + resource_run_outcome, resource_terminal_detail, }; use clap::{CommandFactory, Parser}; use commit_ci_preflight::process::CancellationToken; @@ -2311,6 +2330,10 @@ mod tests { ResourceCommand, ResourceCommandRunner, ResourceProbe, ResourceProbeError, ResourceWatchdog, }; use commit_ci_preflight::resource_history::{ResourceExecutorV2, ResourceTerminalDetailV2}; + use commit_ci_preflight::run::Clock; + use commit_ci_preflight::run_journal::{ + RunFailureDiagnosticCodeV1, RunJournalStateV1, RunJournalStore, + }; use std::ffi::OsString; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -2355,6 +2378,57 @@ mod tests { ); } + #[test] + fn synthetic_internal_failure_after_executing_is_redacted() { + let root = std::env::temp_dir().join(format!( + "ccp-task2-synthetic-{}-{}", + std::process::id(), + super::JOURNAL_SEQUENCE.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + std::fs::create_dir_all(&root).expect("root directory"); + let store = RunJournalStore::initialize(&root).expect("journal initializes"); + let run_id = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let clock = super::SystemClock; + store + .create_run(run_id, &clock.now_utc().expect("time")) + .expect("created"); + let mut lifecycle = super::JournalLifecycleObserver { + store: &store, + run_id, + clock: &clock, + }; + lifecycle + .transition_state(RunJournalStateV1::Admitted, None) + .expect("admitted"); + lifecycle + .transition_state(RunJournalStateV1::Prepared, None) + .expect("prepared"); + lifecycle + .transition_state(RunJournalStateV1::Executing, None) + .expect("executing"); + let error = CliError::internal(std::io::Error::other("synthetic-top-level-secret")); + lifecycle + .fail(cli_failure_kind(&error), cli_failure_diagnostic(&error)) + .expect("failed"); + let status = store.status().expect("status"); + let recovered = status + .runs + .iter() + .find(|run| run.run_id == run_id) + .expect("run"); + assert_eq!(recovered.state, Some(RunJournalStateV1::Failed)); + assert_eq!( + recovered + .failure_diagnostic + .as_ref() + .map(|d| d.diagnostic_code), + Some(RunFailureDiagnosticCodeV1::InternalCommandFailure) + ); + let serialized = serde_json::to_string(&status).expect("serialize"); + assert!(!serialized.contains("synthetic-top-level-secret")); + let _ = std::fs::remove_dir_all(root); + } + #[test] fn guard_exec_requires_double_dash_and_program() { let cli = Cli::try_parse_from([ From 2a9cf707c99ae244a6fb4d0b391d070f935d09ac Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 12:35:39 +0200 Subject: [PATCH 06/25] Bind terminal diagnostics to unknown failures --- src/run_journal.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/src/run_journal.rs b/src/run_journal.rs index 0d170f4..0f61b20 100644 --- a/src/run_journal.rs +++ b/src/run_journal.rs @@ -417,7 +417,10 @@ impl RunJournalStore { Ok(journal) if !journal.is_empty() => { let last = journal.last().expect("non-empty journal"); let diagnostic = self.read_terminal_diagnostic(&name).and_then(|diagnostic| { - if diagnostic.is_some() && last.state != RunJournalStateV1::Failed { + if diagnostic.is_some() + && (last.state != RunJournalStateV1::Failed + || last.failure_kind != Some(RunFailureKindV1::Unknown)) + { Err(RunJournalError::Corrupt) } else { Ok(diagnostic) @@ -459,7 +462,8 @@ impl RunJournalStore { let entries = self.read_entries(run_id)?; let last = entries.last().ok_or(RunJournalError::Corrupt)?; if self.read_terminal_diagnostic(run_id)?.is_some() - && last.state != RunJournalStateV1::Failed + && (last.state != RunJournalStateV1::Failed + || last.failure_kind != Some(RunFailureKindV1::Unknown)) { return Err(RunJournalError::Corrupt); } @@ -1216,6 +1220,48 @@ mod tests { fs::remove_dir_all(root).expect("cleanup"); } + #[test] + fn unknown_diagnostic_requires_unknown_terminal_entry_binding() { + let root = cache_root("diagnostic-entry-binding"); + let store = RunJournalStore::initialize(&root).expect("store"); + store.create_run(RUN_ID, AT).expect("created"); + store + .transition(RUN_ID, RunJournalStateV1::Admitted, AT, None) + .expect("admitted"); + store + .transition(RUN_ID, RunJournalStateV1::Prepared, AT, None) + .expect("prepared"); + store + .transition(RUN_ID, RunJournalStateV1::Executing, AT, None) + .expect("executing"); + store + .transition( + RUN_ID, + RunJournalStateV1::Failed, + AT, + Some(RunFailureKindV1::ExecutionFailed), + ) + .expect("execution failed"); + let diagnostic = RunFailureDiagnosticV1 { + schema_version: RUN_JOURNAL_SCHEMA_VERSION.to_owned(), + run_id: RUN_ID.to_owned(), + failure_kind: RunFailureKindV1::Unknown, + diagnostic_code: RunFailureDiagnosticCodeV1::InternalCommandFailure, + }; + let mut bytes = serde_json::to_vec(&diagnostic).expect("diagnostic json"); + bytes.push(b'\n'); + fs::write(store.run_path(RUN_ID).join(TERMINAL_DIAGNOSTIC), bytes) + .expect("diagnostic sidecar"); + + assert_eq!( + store.status().expect("status").runs[0].classification, + RecoveryClassificationV1::OperatorRequired + ); + assert!(matches!(store.apply(RUN_ID), Err(RunJournalError::Corrupt))); + + fs::remove_dir_all(root).expect("cleanup"); + } + #[test] fn terminal_diagnostic_payload_is_path_and_secret_free() { let diagnostic = RunFailureDiagnosticV1 { From e12b08b2ba037225e4fd93b47a45b465ca760200 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Sun, 23 Aug 2026 19:55:30 +0200 Subject: [PATCH 07/25] docs: note terminal diagnostic recovery --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 806d6bd..0105ba6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -198,6 +198,11 @@ Semantic Versioning after its first public release. ### Fixed +- Preserve a bounded, redacted terminal diagnostic for otherwise-unclassified + top-level `run` failures. Recovery status exposes only a closed diagnostic + code; malformed, misplaced, or mismatched sidecars remain fail-closed, and + historic terminal journals without the sidecar stay readable. No native run + or receipt qualification is implied. - Route recovery CLI test fixtures through the declared `CCP_TEST_ROOT` so the Linux CI contract can keep the repository mount read-only. - Normalize recovery-journal identifiers to the filesystem-safe 64-hex From 1e4bc9c3f7fbf06da82c99786e2a77f512205bf5 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 03:22:38 +0200 Subject: [PATCH 08/25] docs: design admission layout recovery --- ...-08-24-admission-layout-recovery-design.md | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md diff --git a/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md new file mode 100644 index 0000000..e36f0c2 --- /dev/null +++ b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md @@ -0,0 +1,244 @@ +# Admission Layout Recovery Design + +## Status and purpose + +Status: approved in chat on 2026-08-24; implementation not started. + +PR #64 cannot produce its exact-head receipt because the canonical CCP +candidate rejects the host-wide admission root at the historical +`agent-tickets/` child. The root has the exact CCP ownership marker and the +child is currently a plain empty directory, but current `main` does not +implement the experimental agent-ticket lifecycle that created it. + +This change adds a bounded, explicit, non-destructive recovery path for that +one known historical layout. It preserves fail-closed admission, does not +interpret agent-ticket state, and does not authorize a CCP run or any other +heavy operation. + +## Scope and non-goals + +In scope: + +- Diagnose the exact historical `agent-tickets/` layout without relying on the + normal status path that rejects unknown root children. +- Produce a versioned, hash-bound recovery plan only when the target is a + plain empty directory and the complete coordinator is provably idle. +- Apply that exact plan only after reacquiring authoritative locks and + revalidating every safety condition. +- Preserve the empty directory by atomically moving it into CCP's existing + quarantine directory. +- Document the operator boundary and prove it with deterministic temporary-root + tests. + +Out of scope: + +- Accepting or ignoring `agent-tickets/` during normal admission. +- Parsing, adopting, repairing, deleting, or quarantining any agent-ticket + record, staging file, ticket, lease, or active lock. +- Merging the experimental agent-admission lifecycle. +- Recovering arbitrary unknown root children. +- Running CCP checks, Docker, a model, or network activity during verification. +- Automatically applying recovery from `status`, `run`, `benchmark`, or + `guard exec`. + +## Rejected approaches + +### Allowlist the directory + +Adding `agent-tickets/` to `validate_layout` would let the current coordinator +ignore state it cannot understand. A non-empty directory could contain an +active record, a staging file, malformed state, or contradictory companions. +This would bypass host-wide coordination and is rejected. + +### Import the experimental lifecycle + +The historical branch changes admission, durable filesystem, CLI, execution, +tests, and documentation across thousands of lines. It is not a bounded +prerequisite for terminal diagnostics and would materially enlarge PR #64. + +### Manual deletion or quarantine + +The coordination contract prohibits operators from deleting, moving, or +reinterpreting shared admission state. Recovery must be a CCP-owned operation +whose preconditions and outcome are testable and machine-readable. + +Only the new hash-bound CCP command is supported. Its implementation does not +make an equivalent manual filesystem operation supported. + +## CLI contract + +Add a nested admission recovery command: + +```console +commit-ci-preflight admission layout-recovery status --json \ + --timeout-seconds + +commit-ci-preflight admission layout-recovery apply \ + --expected-plan --json \ + --timeout-seconds +``` + +Both operations default to five seconds and accept only integer timeouts from +1 through 60 seconds. Zero, values above 60, and values that cannot be parsed +are CLI input errors with exit code `2`, before any coordinator access. A +deadline reached while `status` is acquiring its snapshot returns +`operator_required` with the closed reason `lock_timeout`. The same deadline +during `apply` returns `not_applied`, exit code `70`, and no mutation. + +`status` is read-only. Its schema `admission-layout-recovery/1.0` returns one +of these closed classifications: + +- `not_needed`: the canonical layout is already valid and no historical + target exists; +- `recoverable_empty_historical_agent_tickets`: all recovery preconditions are + true, accompanied by `plan_sha256`; +- `operator_required`: any state is unsupported, non-empty, active, + contradictory, foreign, malformed, or uncertain. + +`apply` requires the exact 64-character digest returned by `status`. A stale, +malformed, or mismatched digest fails closed and performs no mutation. The +command is not invoked implicitly and is not evidence that a later heavy run +is authorized. + +The JSON is privacy-bounded. It contains schema, classification, target kind, +bounded reason codes, plan digest, and outcome. It omits absolute paths, +process data, commands, repositories, users, raw record contents, and +environment values. + +## Recovery plan and binding + +The plan digest is SHA-256 over canonical JSON containing only: + +- plan schema and recovery kind; +- the validated CCP root-owner tuple; +- the exact historical child name and plain-directory type; +- canonical root-child names and types; +- empty canonical ticket and lease inventories; +- proof that the slot lock was acquirable and the queue lock is exclusively + held by the recovery snapshot; and +- empty target inventory. + +The digest is an operator binding, not a substitute for locking or validation. +`apply` reconstructs the plan under lock and requires exact digest equality. +Even when the digest matches, every semantic precondition is checked again +immediately before the rename. + +The destination basename is derived from the completed plan digest, so it is +deterministic and append-once without making the digest definition circular: + +```text +agent-tickets.recovered-v1- +``` + +An existing destination is a collision and blocks recovery. The implementation +never overwrites or removes a quarantine entry. + +## Locking and state validation + +Both commands inspect the existing platform admission root. They do not create +an alternate root. + +The recovery snapshot follows the coordinator's lock order: + +1. validate that the root and existing queue-lock path are plain objects; +2. acquire `queue.lock` exclusively with the bounded timeout; +3. acquire `slot.lock` exclusively without changing either lock file; +4. validate the exact CCP owner marker; +5. validate every canonical root child and reject any unknown sibling other + than the exact `agent-tickets` target; +6. require canonical `tickets/` and `leases/` to be plain directories with + empty inventories; +7. require `agent-tickets/` to be a plain, non-symlink directory with an empty + inventory; +8. require `quarantine/` to be a plain directory and the destination to be + absent. + +Failure to open or lock either lock, missing ownership, unexpected types, +permission uncertainty, clock/timeout uncertainty, or any directory entry +returns `operator_required` from `status`. A status lock deadline uses reason +`lock_timeout`; `apply` instead returns the separately defined `not_applied` +outcome and exit code `70`. The process releases locks without changing state. + +Normal `admission status`, acquisition, and release behavior remains unchanged: +it continues to reject `agent-tickets/` until an explicitly authorized recovery +has completed. This prevents the current coordinator from ever ignoring agent +state it cannot interpret. + +## Apply transaction + +After reconstructing and matching the plan under both locks, `apply`: + +1. atomically renames the empty `agent-tickets/` directory to the planned child + of `quarantine/` on the same filesystem; +2. durably synchronizes the affected parent directories where the platform + supports directory synchronization; +3. runs the existing strict canonical layout validation; +4. releases the slot lock, then the queue lock; and +5. returns `recovered` with the bounded destination basename. + +No file is deleted. If the rename fails, the target remains in its original +location and the command reports a failure. If post-rename durable sync or +validation is uncertain, the command returns the internal/unsafe-state exit +class and does not claim successful recovery. The preserved quarantine entry +remains operator evidence. + +Re-running `status` after a successful apply returns `not_needed`. Re-running +`apply` with the old digest is non-actionable and does not change state. + +## Code boundaries + +- `src/admission.rs`: recovery report/plan types, strict snapshot validation, + lock-scoped status and apply operations, and unit tests. +- `src/main.rs`: nested CLI parsing, bounded JSON/text rendering, and existing + admission error/exit-code mapping. +- A focused integration test file for the CLI contract and no-mutation failure + cases. +- `docs/COORDINATION_RUNBOOK.md` and `docs/TROUBLESHOOTING.md`: exact operator + sequence and explicit statement that manual recovery remains unsupported. + +Receipt schemas, run journals, source-binding tokens, resource policy, and +normal admission status schema are unchanged. + +## TDD and verification + +The first RED test is black-box: it constructs an owned temporary coordinator +root containing only an empty plain `agent-tickets/` incompatibility, proves +that normal status fails, then invokes `admission layout-recovery status`. The +unmodified binary compiles but rejects that unknown subcommand, providing an +observable RED without referencing an absent Rust API. GREEN introduces only +the plan/status surface before apply. + +Required deterministic coverage: + +- recoverable owned empty historical directory; +- normal status remains fail-closed before apply and works after apply; +- exact plan digest is required and stale/malformed digests make no changes; +- timeout values `0` and `61` are rejected before coordinator access, while a + lock deadline reports the bounded fail-closed timeout outcome; +- non-empty target, including one staging entry, is operator-required; +- target symlink, regular file, permission/type uncertainty, or foreign owner + is operator-required; +- held slot, queued ticket, lease, unknown sibling, or quarantine collision is + operator-required; +- injected rename or synchronization failure never reports success; +- successful apply preserves the directory at the append-once quarantine path; +- repeated status/apply is idempotent and non-mutating; +- JSON contains only closed reason codes and no absolute path or record data; +- legacy admission, run-journal, and receipt tests remain unchanged and pass. + +Verification uses formatting, focused admission unit tests, focused CLI tests, +and the full deterministic Rust suite. It performs no global-root recovery, +CCP `run`, Docker operation, evidence publication, network action, or R5 work. + +## Operational gates after implementation + +Implementation success does not authorize mutation of the live coordinator. +Before a live apply, the exact candidate path, source commit, and binary +SHA-256 must be frozen; read-only recovery status and its exact plan digest +must be preserved; and the user must explicitly authorize that one hash-bound +apply operation. + +After apply, a fresh canonical `admission status --json` must report an idle, +readable coordinator before any separate heavy-run authorization is requested. +PR #64 receipt production and publication remain distinct later gates bound to +the resulting exact PR head. From 6686f39de44fa52f632a87648b85fd3630d8ffd1 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 03:27:19 +0200 Subject: [PATCH 09/25] docs: isolate admission recovery CLI tests --- ...-08-24-admission-layout-recovery-design.md | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md index e36f0c2..56cafad 100644 --- a/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md +++ b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md @@ -201,12 +201,20 @@ normal admission status schema are unchanged. ## TDD and verification -The first RED test is black-box: it constructs an owned temporary coordinator -root containing only an empty plain `agent-tickets/` incompatibility, proves -that normal status fails, then invokes `admission layout-recovery status`. The -unmodified binary compiles but rejects that unknown subcommand, providing an -observable RED without referencing an absent Rust API. GREEN introduces only -the plan/status surface before apply. +The first RED test exercises the CLI parser in-process: it constructs an owned +temporary coordinator root containing only an empty plain `agent-tickets/` +incompatibility, proves that normal status fails, then parses `admission +layout-recovery status`. The unmodified binary parser rejects that unknown +subcommand, providing an observable RED without referencing an absent Rust +API. GREEN adds a dispatcher seam that accepts an explicitly injected +coordinator only from `src/main.rs` unit tests; production dispatch continues +to resolve `AdmissionCoordinator::platform()` internally. + +Integration behavior is covered through core coordinator tests plus in-process +CLI parse/dispatch tests. The release binary gains no coordinator-root flag, +environment override, hidden test mode, or path supplied by a caller. This is +required because the production admission boundary intentionally rejects +temporary roots and roots inside the current repository. Required deterministic coverage: From 64fe45006c8c8827d129c149aba1050751109b56 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 03:48:35 +0200 Subject: [PATCH 10/25] docs: plan admission layout recovery --- .../2026-08-24-admission-layout-recovery.md | 1187 +++++++++++++++++ ...-08-24-admission-layout-recovery-design.md | 23 +- 2 files changed, 1203 insertions(+), 7 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-24-admission-layout-recovery.md diff --git a/docs/superpowers/plans/2026-08-24-admission-layout-recovery.md b/docs/superpowers/plans/2026-08-24-admission-layout-recovery.md new file mode 100644 index 0000000..48ca025 --- /dev/null +++ b/docs/superpowers/plans/2026-08-24-admission-layout-recovery.md @@ -0,0 +1,1187 @@ +# Admission Layout Recovery Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an explicit, hash-bound CCP command that diagnoses and preserves one provably empty historical `agent-tickets/` directory without weakening normal admission. + +**Architecture:** `AdmissionCoordinator` keeps its strict normal layout validator. A separate recovery-only validator acquires the existing queue and slot locks, constructs a privacy-bounded canonical plan for the exact empty historical layout, and applies it only when the caller supplies the matching SHA-256. Apply atomically moves the empty directory beneath the existing quarantine directory; it never parses or ignores agent state. + +**Tech Stack:** Rust 2024; existing `clap`, `fs2`, `serde`, `serde_json`, and `sha2`; deterministic unit and contract tests. No new dependency, Docker, network, model, or global coordinator access. + +**Spec:** `docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md` + +## Global Constraints + +- Normal `validate_layout`, `admission status`, `run`, `benchmark`, and `guard exec` must continue to reject `agent-tickets/` until explicit recovery succeeds. +- Recovery supports only the exact child name `agent-tickets`; every other unknown root child remains blocking. +- Recovery never parses, adopts, deletes, overwrites, or silently ignores agent-ticket records, staging files, canonical tickets, leases, or active locks. +- `status` is read-only; `apply` requires one exact lowercase 64-character plan SHA-256 and revalidates under lock. +- Lock order is queue first, slot second; release order is slot first, queue second. +- Timeouts default to 5 seconds and accept only integers from 1 through 60 seconds. +- JSON contains no absolute path, process data, repository, user, command, record content, or environment value. +- The release binary gains no admission-root flag, environment override, hidden test mode, or alternate-root behavior. +- Receipt, journal, source-binding, resource-policy, and normal admission-status schemas remain unchanged. +- Tests use owned temporary roots only; no test or implementation step accesses the live platform coordinator. +- No CCP `run`, Docker operation, evidence publication, push, or R5 action belongs to this plan. + +--- + +## File map + +| File | Responsibility | +|---|---| +| `src/admission.rs` | Recovery schemas, reason codes, plan digest, recovery-only structural validation, lock-scoped status/apply, and coordinator unit tests. | +| `src/durable_fs.rs` | One fail-closed primitive that moves a plain empty directory between owned plain-directory parents and synchronizes both parents. | +| `src/main.rs` | Nested CLI parsing, 1–60 second validation, test-only injected-coordinator dispatch seam, bounded rendering, and reported exit 70 for both non-success apply outcomes. | +| `docs/COORDINATION_RUNBOOK.md` | Normative two-step operator flow and explicit authorization boundaries. | +| `docs/TROUBLESHOOTING.md` | Safe diagnosis for the exact historical empty-directory case; manual filesystem recovery remains unsupported. | +| `tests/agent_integration_contract.rs` | Public documentation contract for hash-bound recovery, no manual cleanup, and no implicit heavy authorization. | + +### Task 1: Add the read-only recovery plan and CLI status surface + +**Files:** +- Modify: `src/admission.rs:25-110,197-420,926-990,1483-1605,1640-end` +- Modify: `src/main.rs:80-110,404-435,437-520,1534-1565,2315-end` + +**Interfaces:** +- Consumes: existing `AdmissionCoordinator`, `AdmissionDeadline`, `CancellationToken`, `QUEUE_LOCK`, `SLOT_LOCK`, `OWNER_BYTES`, `validate_regular`, `lock_exclusive_until`, and `unlock`. +- Produces: + +```rust +pub const ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION: &str = + "admission-layout-recovery/1.0"; +pub const DEFAULT_LAYOUT_RECOVERY_TIMEOUT: Duration = Duration::from_secs(5); +pub const MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS: u64 = 60; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryClassificationV1 { + NotNeeded, + RecoverableEmptyHistoricalAgentTickets, + OperatorRequired, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryReasonV1 { + CanonicalLayout, + EmptyHistoricalAgentTickets, + LockTimeout, + ForeignOwner, + UnsupportedLayout, + TargetNotEmpty, + CoordinatorNotIdle, + QuarantineCollision, + PlanMismatch, + FilesystemUncertain, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct AdmissionLayoutRecoveryStatusV1 { + pub schema_version: String, + pub classification: AdmissionLayoutRecoveryClassificationV1, + pub target_kind: Option, + pub reason: AdmissionLayoutRecoveryReasonV1, + pub plan_sha256: Option, +} + +impl AdmissionCoordinator { + pub fn layout_recovery_status_with_timeout( + &self, + timeout: Duration, + cancellation: &CancellationToken, + ) -> AdmissionLayoutRecoveryStatusV1; +} +``` + +- Produces CLI shapes: + +```rust +#[derive(Debug, Subcommand)] +enum AdmissionCommand { + Status { + #[arg(long)] + json: bool, + #[arg(long, default_value_t = DEFAULT_STATUS_TIMEOUT.as_secs())] + timeout_seconds: u64, + }, + LayoutRecovery { + #[command(subcommand)] + action: AdmissionLayoutRecoveryCommand, + }, +} + +#[derive(Debug, Subcommand)] +enum AdmissionLayoutRecoveryCommand { + Status { + #[arg(long)] + json: bool, + #[arg( + long, + default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), + value_parser = parse_layout_recovery_timeout + )] + timeout_seconds: u64, + }, +} + +fn parse_layout_recovery_timeout(value: &str) -> Result { + let seconds = value + .parse::() + .map_err(|_| "layout recovery timeout must be an integer from 1 through 60".to_owned())?; + if !(1..=MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS).contains(&seconds) { + return Err("layout recovery timeout must be an integer from 1 through 60".to_owned()); + } + Ok(seconds) +} +``` + +- [ ] **Step 1: Write the CLI RED tests** + +Add `src/main.rs` tests that parse the public command and reject out-of-range timeouts without running a dispatcher: + +```rust +#[test] +fn admission_layout_recovery_status_parses_only_bounded_timeouts() { + let parsed = Cli::try_parse_from([ + "commit-ci-preflight", + "admission", + "layout-recovery", + "status", + "--json", + "--timeout-seconds", + "5", + ]); + assert!(parsed.is_ok()); + + for invalid in ["0", "61", "not-a-number"] { + assert!(Cli::try_parse_from([ + "commit-ci-preflight", + "admission", + "layout-recovery", + "status", + "--timeout-seconds", + invalid, + ]) + .is_err()); + } +} +``` + +- [ ] **Step 2: Run the parser test and confirm RED** + +Run: `rtk cargo test --locked --bin commit-ci-preflight tests::admission_layout_recovery_status_parses_only_bounded_timeouts -- --exact` + +Expected: FAIL because `layout-recovery` is not an `AdmissionCommand` variant. + +- [ ] **Step 3: Write the coordinator RED tests** + +In `src/admission.rs`, change `AdmissionCoordinator::test_at` to `pub(crate)` under `#[cfg(test)]`, then add a fixture that initializes a canonical root before adding the incompatible child: + +```rust +fn coordinator_with_empty_historical_agent_tickets( + label: &str, +) -> AdmissionCoordinator { + let coordinator = coordinator(label); + coordinator.initialize().expect("canonical coordinator"); + fs::create_dir(coordinator.root().join("agent-tickets")) + .expect("historical empty directory"); + coordinator +} + +#[test] +fn normal_status_stays_closed_but_recovery_status_plans_empty_historical_directory() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-status"); + assert!(matches!( + coordinator.status(), + Err(AdmissionError::UnsafeLayout(_)) + )); + + let before = tree_fingerprint(coordinator.root()); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets + ); + let digest = report.plan_sha256.expect("recovery plan"); + assert_eq!(digest.len(), 64); + assert!(digest.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))); + assert_eq!(before, tree_fingerprint(coordinator.root())); +} +``` + +Add this test helper; it records every entry and excludes nothing: + +```rust +fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { + fn walk( + root: &Path, + path: &Path, + out: &mut Vec<(PathBuf, &'static str, Vec)>, + ) { + let mut entries = fs::read_dir(path) + .expect("read fingerprint directory") + .collect::, _>>() + .expect("fingerprint entries"); + entries.sort_by_key(fs::DirEntry::file_name); + for entry in entries { + let path = entry.path(); + let relative = path.strip_prefix(root).expect("relative path").to_path_buf(); + let metadata = fs::symlink_metadata(&path).expect("fingerprint metadata"); + if metadata.file_type().is_symlink() { + let target = fs::read_link(&path) + .expect("symlink target") + .to_string_lossy() + .as_bytes() + .to_vec(); + out.push((relative, "symlink", target)); + } else if metadata.is_dir() { + out.push((relative, "directory", Vec::new())); + walk(root, &path, out); + } else { + out.push((relative, "file", fs::read(&path).expect("file bytes"))); + } + } + } + let mut entries = Vec::new(); + walk(root, root, &mut entries); + entries +} +``` + +- [ ] **Step 4: Run the coordinator test and confirm RED** + +Run: `rtk cargo test --locked --lib admission::tests::normal_status_stays_closed_but_recovery_status_plans_empty_historical_directory -- --exact` + +Expected: FAIL to compile because the recovery status types and method do not exist. + +- [ ] **Step 5: Implement the minimal read-only recovery model** + +Add private strict plan types and digesting without a new dependency: + +```rust +#[derive(Serialize)] +struct RecoveryRootEntryV1 { + name: String, + kind: String, +} + +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct CoordinatorOwnerMarkerV1 { + owner: String, + purpose: String, + schema_version: String, +} + +#[derive(Serialize)] +struct AdmissionLayoutRecoveryPlanV1 { + schema_version: &'static str, + recovery_kind: &'static str, + owner: String, + purpose: String, + owner_schema_version: String, + root_entries: Vec, + queue_lock_name: &'static str, + queue_lock_kind: &'static str, + queue_lock_exclusively_held: bool, + slot_lock_name: &'static str, + slot_lock_kind: &'static str, + slot_lock_was_free: bool, + ticket_count: usize, + lease_count: usize, + target_entry_count: usize, +} + +fn layout_recovery_plan_digest(plan: &AdmissionLayoutRecoveryPlanV1) -> Result { + let bytes = serde_json::to_vec(plan).map_err(AdmissionError::Json)?; + let digest = Sha256::digest(bytes); + let mut value = String::with_capacity(64); + for byte in digest { + use std::fmt::Write as _; + write!(&mut value, "{byte:02x}").expect("writing to String cannot fail"); + } + Ok(value) +} +``` + +Implement a dedicated structural validator that recognizes only the exact +plain-directory target, validates every other canonical entry with existing +helpers, and never calls normal `validate_layout` after deciding to inspect +the historical case. Require existing plain `queue.lock` and `slot.lock` files, +then acquire queue followed by slot; do not create or truncate either. Read and +strictly deserialize the actual owner marker, require its raw bytes to equal +`OWNER_BYTES`, and put its validated tuple into the plan rather than copying +unobserved constants. Require empty `tickets`, `leases`, +and target inventories. Set `target_kind` only to the bounded literal +`historical_agent_tickets`. After hashing the plan, derive +`agent-tickets.recovered-v1-{plan_sha256}` and require that exact quarantine +destination to be absent before returning a plan. Release slot then queue on +every return. + +Map failures into closed reason codes. `AdmissionError::Timeout` while locking +maps to `OperatorRequired/LockTimeout`; unknown siblings and invalid types map +to `OperatorRequired/UnsupportedLayout`; target contents map to +`OperatorRequired/TargetNotEmpty`; any canonical ticket or lease maps to +`OperatorRequired/CoordinatorNotIdle`. + +- [ ] **Step 6: Implement parser and test-only dispatcher injection** + +Production dispatch must construct the platform coordinator internally: + +```rust +fn run_admission_command(action: AdmissionCommand) -> Result<(), CliError> { + let coordinator = AdmissionCoordinator::platform().map_err(CliError::Admission)?; + run_admission_command_with(action, coordinator) +} + +fn run_admission_command_with( + action: AdmissionCommand, + coordinator: AdmissionCoordinator, +) -> Result<(), CliError> { + match action { + AdmissionCommand::Status { json, timeout_seconds } => { + render_admission_status(&coordinator, json, timeout_seconds) + } + AdmissionCommand::LayoutRecovery { + action: AdmissionLayoutRecoveryCommand::Status { json, timeout_seconds }, + } => render_layout_recovery_status(&coordinator, json, timeout_seconds), + } +} + +fn render_admission_status( + coordinator: &AdmissionCoordinator, + json: bool, + timeout_seconds: u64, +) -> Result<(), CliError> { + let cancellation = CancellationToken::default(); + let status = coordinator + .status_with_timeout(Duration::from_secs(timeout_seconds), &cancellation) + .map_err(CliError::Admission)?; + if json { + println!("{}", serde_json::to_string(&status).map_err(CliError::internal)?); + } else { + println!("Admission schema: {ADMISSION_STATUS_SCHEMA_VERSION}"); + println!("Active: {}", status.active); + println!("Queued: {}", status.queue_count); + println!("Slot lock: {}", status.slot.state); + println!("Slot owner/run: {:?}", status.slot.owner_run_id); + println!("Slot lease: {}", status.slot.lease_state); + println!("Queue lock: {}", status.queue_lock.state); + for ticket in &status.ticket_ids { + println!(" - {ticket}"); + } + println!("Note: {}", status.process_visibility_note); + } + Ok(()) +} + +fn render_layout_recovery_status( + coordinator: &AdmissionCoordinator, + json: bool, + timeout_seconds: u64, +) -> Result<(), CliError> { + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(timeout_seconds), + &CancellationToken::default(), + ); + if json { + println!("{}", serde_json::to_string(&report).map_err(CliError::internal)?); + } else { + println!("Layout recovery schema: {}", report.schema_version); + println!("Classification: {:?}", report.classification); + println!("Reason: {:?}", report.reason); + println!("Plan SHA-256: {:?}", report.plan_sha256); + println!("Read-only: no state was changed."); + } + Ok(()) +} +``` + +The injected coordinator function stays private to `src/main.rs`; only its unit +tests call it. Do not add a process environment lookup or public root option. + +- [ ] **Step 7: Verify Task 1 GREEN** + +Run: `rtk cargo test --locked --lib layout_recovery` + +Expected: PASS with at least one selected read-only recovery status test; zero +selected tests is a failure. + +Run: `rtk cargo test --locked --bin commit-ci-preflight admission_layout_recovery` + +Expected: PASS with at least one selected parser/dispatch test; zero selected +tests is a failure. + +Run: `rtk cargo test --locked --lib status_` + +Expected: existing normal admission status tests remain PASS with a nonzero +selected-test count. + +- [ ] **Step 8: Commit Task 1** + +```bash +rtk git add src/admission.rs src/main.rs +rtk git commit -m "feat: plan empty admission layout recovery" +``` + +### Task 2: Apply one exact plan through a durable empty-directory move + +**Files:** +- Modify: `src/durable_fs.rs:35-180,260-end` +- Modify: `src/admission.rs` recovery types and methods added in Task 1 +- Modify: `src/main.rs` by extending Task 1's status-only recovery command + +**Interfaces:** +- Consumes: `AdmissionLayoutRecoveryStatusV1.plan_sha256`, Task 1's lock-scoped plan constructor, `DurableFileSystem`, and the existing `keep_first_error`/unlock pattern. +- Produces: + +```rust +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryOutcomeV1 { + Recovered, + NotApplied, + RecoveryUncertain, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct AdmissionLayoutRecoveryApplyV1 { + pub schema_version: String, + pub outcome: AdmissionLayoutRecoveryOutcomeV1, + pub reason: AdmissionLayoutRecoveryReasonV1, + pub quarantine_entry: Option, +} + +impl AdmissionCoordinator { + pub fn apply_layout_recovery_with_timeout( + &self, + expected_plan: &str, + timeout: Duration, + cancellation: &CancellationToken, + ) -> AdmissionLayoutRecoveryApplyV1; +} + +impl DurableFileSystem { + pub(crate) fn relocate_empty_directory( + &self, + source: &Path, + destination: &Path, + ) -> Result<(), DurableFsError>; +} + +#[derive(Debug, Subcommand)] +enum AdmissionLayoutRecoveryCommand { + Status { + #[arg(long)] + json: bool, + #[arg( + long, + default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), + value_parser = parse_layout_recovery_timeout + )] + timeout_seconds: u64, + }, + Apply { + #[arg(long, value_parser = parse_plan_sha256)] + expected_plan: String, + #[arg(long)] + json: bool, + #[arg( + long, + default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), + value_parser = parse_layout_recovery_timeout + )] + timeout_seconds: u64, + }, +} + +fn parse_plan_sha256(value: &str) -> Result { + if value.len() == 64 + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + { + Ok(value.to_owned()) + } else { + Err("expected plan must be exactly 64 lowercase hexadecimal characters".to_owned()) + } +} +``` + +- [ ] **Step 1: Write durable-filesystem RED tests** + +```rust +#[test] +fn relocate_empty_directory_preserves_source_as_append_only_destination() { + let root = temporary_directory("relocate-empty"); + let source = root.join("agent-tickets"); + let quarantine = root.join("quarantine"); + let destination = quarantine.join("agent-tickets.recovered-v1-plan"); + fs::create_dir(&source).expect("source"); + fs::create_dir(&quarantine).expect("quarantine"); + + DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .expect("relocate"); + + assert!(!source.exists()); + assert!(destination.is_dir()); + assert!(DurableFileSystem::default() + .relocate_empty_directory(&destination, &destination) + .is_err()); + fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn relocate_empty_directory_rejects_contents_and_existing_destination() { + let root = temporary_directory("relocate-reject"); + let source = root.join("agent-tickets"); + let quarantine = root.join("quarantine"); + let destination = quarantine.join("agent-tickets.recovered-v1-plan"); + fs::create_dir(&source).expect("source"); + fs::create_dir(&quarantine).expect("quarantine"); + fs::write(source.join("entry"), b"blocked\n").expect("source entry"); + + assert!(DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .is_err()); + assert_eq!(fs::read(source.join("entry")).expect("entry remains"), b"blocked\n"); + assert!(!destination.exists()); + + fs::remove_file(source.join("entry")).expect("remove fixture entry"); + fs::create_dir(&destination).expect("destination collision"); + assert!(DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .is_err()); + assert!(source.is_dir()); + assert!(destination.is_dir()); + fs::remove_dir_all(root).expect("cleanup"); +} +``` + +- [ ] **Step 2: Run durable tests and confirm RED** + +Run: `rtk cargo test --locked --lib durable_fs::tests::relocate_empty_directory` + +Expected: FAIL to compile because `relocate_empty_directory` does not exist. + +- [ ] **Step 3: Implement the minimal durable move** + +```rust +pub(crate) fn relocate_empty_directory( + &self, + source: &Path, + destination: &Path, +) -> Result<(), DurableFsError> { + let source_parent = checked_parent(source)?; + let destination_parent = checked_parent(destination)?; + validate_plain_directory(source_parent)?; + validate_plain_directory(destination_parent)?; + validate_plain_directory(source)?; + if fs::read_dir(source)?.next().transpose()?.is_some() { + return Err(DurableFsError::UnsafePath("source directory must be empty")); + } + if fs::symlink_metadata(destination).is_ok() { + return Err(DurableFsError::UnsafePath( + "quarantine destination already exists", + )); + } + self.checkpoint()?; + fs::rename(source, destination)?; + self.checkpoint()?; + sync_directory(destination_parent)?; + self.checkpoint()?; + sync_directory(source_parent)?; + Ok(()) +} +``` + +Handle destination metadata errors other than `NotFound` as I/O uncertainty; +do not treat them as absence. `fs::rename` is the only move operation: a +cross-device/`EXDEV` error maps to filesystem uncertainty and must never fall +back to copy, delete, or recursive movement. Keep both parent syncs even when +one parent is an ancestor of the other. + +- [ ] **Step 4: Write coordinator apply RED tests** + +```rust +#[test] +fn apply_requires_exact_plan_and_preserves_empty_directory_in_quarantine() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-apply"); + let status = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + let plan = status.plan_sha256.expect("plan"); + + let wrong_before = tree_fingerprint(coordinator.root()); + let wrong = coordinator.apply_layout_recovery_with_timeout( + &"0".repeat(64), + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(wrong.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(wrong_before, tree_fingerprint(coordinator.root())); + + let applied = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(applied.outcome, AdmissionLayoutRecoveryOutcomeV1::Recovered); + let entry = applied.quarantine_entry.expect("quarantine basename"); + assert_eq!(entry, format!("agent-tickets.recovered-v1-{plan}")); + assert!(coordinator.root().join("quarantine").join(entry).is_dir()); + assert!(!coordinator.root().join("agent-tickets").exists()); + assert!(!coordinator.status().expect("canonical status").active); + + let after = tree_fingerprint(coordinator.root()); + let status_after = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + status_after.classification, + AdmissionLayoutRecoveryClassificationV1::NotNeeded + ); + assert!(status_after.plan_sha256.is_none()); + let repeated = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(repeated.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(after, tree_fingerprint(coordinator.root())); +} +``` + +- [ ] **Step 5: Run apply test and confirm RED** + +Run: `rtk cargo test --locked --lib admission::tests::apply_requires_exact_plan_and_preserves_empty_directory_in_quarantine -- --exact` + +Expected: FAIL to compile because apply types and method do not exist. + +- [ ] **Step 6: Implement lock-scoped apply** + +Validate `expected_plan` before filesystem access: + +```rust +fn valid_plan_sha256(value: &str) -> bool { + value.len() == 64 + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) +} +``` + +Then acquire queue and slot in the Task 1 order, rebuild the plan, compare the +digest, derive `agent-tickets.recovered-v1-{digest}`, verify the destination is +absent, and call `relocate_empty_directory`. Before returning `Recovered`, run +the unchanged strict `validate_layout(true)` while both locks are still held. +Explicitly release slot then queue; if release fails, return +`RecoveryUncertain/FilesystemUncertain` because the preserved quarantine entry +may already exist. +An invalid or unequal digest returns `NotApplied/PlanMismatch`; an already +canonical layout returns `NotApplied/CanonicalLayout`. + +Every early return must leave the source tree byte-identical unless the durable +move itself completed. A post-rename synchronization or validation failure +returns `RecoveryUncertain/FilesystemUncertain` and never attempts rollback or +deletion. Any pre-rename I/O error, including `EXDEV`, returns +`NotApplied/FilesystemUncertain`. + +Classify a relocation error from observed paths while the locks are still held: + +```rust +fn outcome_after_relocation_error( + source: &Path, + destination: &Path, +) -> AdmissionLayoutRecoveryOutcomeV1 { + match (fs::symlink_metadata(source), fs::symlink_metadata(destination)) { + (Ok(source_meta), Err(destination_error)) + if source_meta.is_dir() + && !source_meta.file_type().is_symlink() + && destination_error.kind() == io::ErrorKind::NotFound => + { + AdmissionLayoutRecoveryOutcomeV1::NotApplied + } + _ => AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + } +} +``` + +This makes an `EXDEV` rename failure `NotApplied` only when the unchanged +source and absent destination are observable. Any contradictory or unreadable +post-error state is `RecoveryUncertain`. + +- [ ] **Step 7: Complete CLI apply rendering and exit behavior** + +Add `ReportedExit(i32)` to the existing `CliError` enum. It suppresses an extra +raw error line after the bounded report has been printed. Change main's +terminal error handling to: + +```rust +if let Err(error) = result { + if !matches!(error, CliError::ReportedExit(_)) { + eprintln!("error: {error}"); + } + std::process::exit(error.exit_code()); +} +``` + +`run_admission_command_with` prints the apply report. It returns `Ok(())` only +for `Recovered`; `NotApplied` and `RecoveryUncertain` return +`Err(CliError::ReportedExit(70))`. +Malformed or absent `--expected-plan` remains a Clap usage error with code `2`. + +Implement the apply renderer exactly as follows: + +```rust +fn render_layout_recovery_apply( + coordinator: &AdmissionCoordinator, + expected_plan: &str, + json: bool, + timeout_seconds: u64, +) -> Result<(), CliError> { + let report = coordinator.apply_layout_recovery_with_timeout( + expected_plan, + Duration::from_secs(timeout_seconds), + &CancellationToken::default(), + ); + if json { + println!("{}", serde_json::to_string(&report).map_err(CliError::internal)?); + } else { + println!("Layout recovery schema: {}", report.schema_version); + println!("Outcome: {:?}", report.outcome); + println!("Reason: {:?}", report.reason); + println!("Quarantine entry: {:?}", report.quarantine_entry); + } + match report.outcome { + AdmissionLayoutRecoveryOutcomeV1::Recovered => Ok(()), + AdmissionLayoutRecoveryOutcomeV1::NotApplied + | AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain => { + Err(CliError::ReportedExit(70)) + } + } +} +``` + +Add this arm to `run_admission_command_with`: + +```rust +AdmissionCommand::LayoutRecovery { + action: + AdmissionLayoutRecoveryCommand::Apply { + expected_plan, + json, + timeout_seconds, + }, +} => render_layout_recovery_apply( + &coordinator, + &expected_plan, + json, + timeout_seconds, +), +``` + +Extend `CliError::exit_code` with `Self::ReportedExit(code) => *code` and its +`Display` implementation with +`Self::ReportedExit(_) => formatter.write_str("command outcome already reported")`. + +- [ ] **Step 8: Verify Task 2 GREEN** + +Run: `rtk cargo test --locked --lib durable_fs::tests::relocate_empty_directory` + +Expected: PASS with at least two selected durable relocation tests; zero +selected tests is a failure. + +Run: `rtk cargo test --locked --lib layout_recovery` + +Expected: PASS with at least two selected coordinator recovery tests covering +plan mismatch, success, canonical post-status, and idempotence; zero selected +tests is a failure. + +Run: `rtk cargo test --locked --bin commit-ci-preflight admission_layout_recovery` + +Expected: PASS with at least two selected parser/dispatch tests for status and +apply; zero selected tests is a failure. + +- [ ] **Step 9: Commit Task 2** + +```bash +rtk git add src/durable_fs.rs src/admission.rs src/main.rs +rtk git commit -m "feat: apply hash-bound admission layout recovery" +``` + +### Task 3: Prove adversarial state remains fail-closed and non-mutating + +**Files:** +- Modify: `src/admission.rs` tests and test-only recovery seams +- Modify: `src/durable_fs.rs` fault-injection tests +- Modify: `src/main.rs` tests + +**Interfaces:** +- Consumes: Task 1 status report and Task 2 apply report/durable move. +- Produces: deterministic regression coverage for every blocked state in the specification; no new public production interface. + +- [ ] **Step 1: Add table-driven structural RED tests** + +Add fixtures that start from the owned historical layout and introduce exactly +one mutation per case: + +```rust +#[test] +fn layout_recovery_rejects_unsupported_or_nonempty_state_without_mutation() { + for case in [ + RecoveryFixtureCase::TargetEntry(".agent-ticket-staging-partial"), + RecoveryFixtureCase::TargetIsFile, + RecoveryFixtureCase::TargetIsSymlink, + RecoveryFixtureCase::ForeignOwner, + RecoveryFixtureCase::MalformedOwner, + RecoveryFixtureCase::MissingQueueLock, + RecoveryFixtureCase::QueueLockIsSymlink, + RecoveryFixtureCase::MissingSlotLock, + RecoveryFixtureCase::SlotLockIsSymlink, + RecoveryFixtureCase::CanonicalTicket, + RecoveryFixtureCase::CanonicalLease, + RecoveryFixtureCase::UnknownSibling, + ] { + let coordinator = recovery_fixture(case); + let before = tree_fingerprint(coordinator.root()); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_millis(200), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired, + "case: {case:?}" + ); + assert!(report.plan_sha256.is_none(), "case: {case:?}"); + assert_eq!(before, tree_fingerprint(coordinator.root()), "case: {case:?}"); + } +} +``` + +Do not use a mode-`000` test, whose behavior changes with platform and +privilege. Add a deterministic test-only effect at the exact target-inventory +read boundary: + +```rust +#[derive(Debug, Clone, Copy, Default)] +struct LayoutRecoveryEffects { + #[cfg(test)] + deny_target_inventory: bool, +} + +impl LayoutRecoveryEffects { + fn before_target_inventory(&self, path: &Path) -> Result<(), AdmissionError> { + #[cfg(test)] + if self.deny_target_inventory { + return Err(AdmissionError::Io { + path: path.to_path_buf(), + source: io::Error::new( + io::ErrorKind::PermissionDenied, + "injected layout recovery permission denial", + ), + }); + } + Ok(()) + } +} +``` + +Production calls the planner with `LayoutRecoveryEffects::default()`. A +`#[cfg(test)]` private status seam passes `deny_target_inventory: true`; assert +`OperatorRequired/FilesystemUncertain`, no plan, no path in JSON, and an +identical tree fingerprint. + +- [ ] **Step 2: Run structural test and confirm RED** + +Run: `rtk cargo test --locked --lib admission::tests::layout_recovery_rejects_unsupported_or_nonempty_state_without_mutation -- --exact` + +Expected: at least one case FAILS until reason mapping and inventory checks are complete. + +- [ ] **Step 3: Add lock and deadline RED tests** + +Hold the canonical slot file with `fs2::FileExt::lock_exclusive`, then request a +50 ms recovery status. Assert completion in under one second, +`OperatorRequired/LockTimeout`, no plan, and an identical tree fingerprint. +Repeat for apply and assert `NotApplied/LockTimeout`. + +Also test parser values `0` and `61`; parsing must fail before constructing or +calling a coordinator. + +- [ ] **Step 4: Run deadline tests and confirm RED** + +Run: `rtk cargo test --locked layout_recovery_lock_timeout` + +Expected: FAIL until slot locking and timeout mappings follow the specification. + +- [ ] **Step 5: Add plan-race and collision RED tests** + +After obtaining a valid plan, separately: + +- create one target entry; +- create an unknown root sibling; +- create the deterministic quarantine destination; and +- add one canonical ticket marker. + +Call apply with the previously valid plan. Each call must return `NotApplied`, +must preserve the exact pre-apply fingerprint, and must not create another +quarantine entry. + +- [ ] **Step 6: Add durable-failure RED tests** + +Use `DurableFileSystem::failing_at` through a `#[cfg(test)]` private +`apply_layout_recovery_with_filesystem` seam: + +```rust +let before = tree_fingerprint(coordinator.root()); +let before_rename = coordinator.apply_layout_recovery_with_filesystem( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + &DurableFileSystem::failing_at(1), +); +assert_eq!(before_rename.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); +assert_eq!(before, tree_fingerprint(coordinator.root())); + +let after_rename = coordinator.apply_layout_recovery_with_filesystem( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + &DurableFileSystem::failing_at(2), +); +assert_eq!( + after_rename.outcome, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain +); +assert!(!coordinator.root().join("agent-tickets").exists()); +assert!(coordinator + .root() + .join("quarantine") + .join(format!("agent-tickets.recovered-v1-{plan}")) + .is_dir()); +``` + +No test rolls the directory back or deletes the evidence before fixture cleanup. + +- [ ] **Step 7: Add privacy and schema assertions** + +Serialize every classification and apply outcome. Assert exact object key +counts and that output contains neither the fixture root nor `/`, `\\`, +`ticket-000`, `lease-`, `HOME`, `repository`, or `command`. + +Assert `ADMISSION_STATUS_SCHEMA_VERSION` remains `2.0` and existing +`AdmissionStatusV1` still serializes exactly seven top-level fields. + +- [ ] **Step 8: Implement the minimal missing validation and reason mapping** + +Complete only the branches exposed by Steps 1–7. Do not add recovery for +non-empty, foreign, malformed, active, or unknown state. Ensure plan building +sorts all bounded root-entry facts before serialization so the digest is stable +across directory enumeration order. + +- [ ] **Step 9: Verify Task 3 GREEN** + +Run: `rtk cargo test --locked --lib layout_recovery` + +Expected: all structural, deadline, race, collision, durability, idempotence, +and privacy tests PASS, with at least six selected tests. + +Run: `rtk cargo test --locked --bin commit-ci-preflight admission_layout_recovery` + +Expected: all parser/dispatch/exit tests PASS, with at least three selected +tests. + +Run: `rtk cargo test --locked --lib admission::tests` + +Expected: all legacy admission tests PASS with a nonzero selected-test count. + +- [ ] **Step 10: Commit Task 3** + +```bash +rtk git add src/admission.rs src/durable_fs.rs src/main.rs +rtk git commit -m "test: harden admission layout recovery boundaries" +``` + +### Task 4: Publish the supported operator contract + +**Files:** +- Modify: `docs/COORDINATION_RUNBOOK.md:20-40,92-112,198-212` +- Modify: `docs/TROUBLESHOOTING.md:39-80` +- Modify: `tests/agent_integration_contract.rs` + +**Interfaces:** +- Consumes: exact Task 1/2 command names, schema, digest, timeout, and outcomes. +- Produces: public instructions that permit only hash-bound CCP recovery and keep all later runs separately authorized. + +- [ ] **Step 1: Write failing documentation contract assertions** + +```rust +#[test] +fn admission_layout_recovery_guidance_is_hash_bound_and_never_manual() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + let runbook = read(root, "docs/COORDINATION_RUNBOOK.md"); + let troubleshooting = read(root, "docs/TROUBLESHOOTING.md"); + for required in [ + "admission layout-recovery status --json", + "admission layout-recovery apply", + "--expected-plan", + "recovery_uncertain", + "does not authorize", + "Manual deletion", + ] { + assert!( + runbook.contains(required) || troubleshooting.contains(required), + "missing recovery boundary: {required}" + ); + } + assert!(!runbook.contains("ignore `agent-tickets`")); +} +``` + +- [ ] **Step 2: Run the contract and confirm RED** + +Run: `rtk cargo test --locked --test agent_integration_contract admission_layout_recovery_guidance_is_hash_bound_and_never_manual -- --exact` + +Expected: FAIL because the commands and exact boundary are undocumented. + +- [ ] **Step 3: Add the minimal normative runbook sequence** + +Document: + +```console +commit-ci-preflight admission layout-recovery status --json --timeout-seconds 5 +# preserve the exact plan_sha256 and obtain one explicit apply authorization +commit-ci-preflight admission layout-recovery apply \ + --expected-plan --json --timeout-seconds 5 +commit-ci-preflight admission status --json +``` + +State that status is read-only, apply supports only the exact empty historical +directory, the directory is preserved beneath quarantine, manual filesystem +equivalents remain unsupported, and successful apply does not authorize a +heavy run, Docker, receipt publication, or R5 retry. + +Document `recovery_uncertain` as a hard stop: preserve both paths and the JSON, +do not retry apply, do not move either directory manually, and do not start a +heavy run. + +- [ ] **Step 4: Update troubleshooting without creating a cleanup recipe** + +Route only the exact `UnsafeLayout(.../agent-tickets)` case to recovery status. +Any non-empty, foreign, malformed, active, lock-timeout, plan-mismatch, or +unknown-child result remains code 70/operator-required. Do not publish absolute +example home paths or shell `mv`/`rm` commands. + +- [ ] **Step 5: Verify Task 4 GREEN** + +Run: `rtk cargo test --locked --test agent_integration_contract admission_layout_recovery_guidance_is_hash_bound_and_never_manual -- --exact` + +Expected: PASS with a nonzero selected-test count. + +Run: `rtk git diff --check` + +Expected: PASS with no whitespace errors. + +- [ ] **Step 6: Commit Task 4** + +```bash +rtk git add docs/COORDINATION_RUNBOOK.md docs/TROUBLESHOOTING.md tests/agent_integration_contract.rs +rtk git commit -m "docs: add hash-bound admission layout recovery" +``` + +### Task 5: Full deterministic verification and review gate + +**Files:** +- Review: `src/admission.rs`, `src/durable_fs.rs`, `src/main.rs` +- Review: `docs/COORDINATION_RUNBOOK.md`, `docs/TROUBLESHOOTING.md` +- Review: `tests/agent_integration_contract.rs` + +**Interfaces:** +- Consumes: Tasks 1–4 exact commits. +- Produces: a source/test review receipt only; no live coordinator recovery or PR evidence receipt. + +- [ ] **Step 1: Run formatting and diff validation** + +Run: `rtk cargo fmt --all -- --check` + +Expected: PASS. + +Run: `rtk git diff --check origin/main...HEAD` + +Expected: PASS. + +- [ ] **Step 2: Run focused deterministic suites** + +Run: `rtk cargo test --locked --lib layout_recovery` + +Expected: PASS with at least six selected tests; zero selected tests is a +verification failure. + +Run: `rtk cargo test --locked --lib durable_fs::tests` + +Expected: PASS with a nonzero selected-test count. + +Run: `rtk cargo test --locked --bin commit-ci-preflight admission_layout_recovery` + +Expected: PASS with at least three selected tests; zero selected tests is a +verification failure. + +Run: `rtk cargo test --locked --test agent_integration_contract` + +Expected: PASS with a nonzero selected-test count. + +- [ ] **Step 3: Run static compilation with warnings denied** + +Run: `rtk env RUSTFLAGS=-Dwarnings cargo check --locked --all-targets` + +Expected: PASS with no new dependency or warning. + +Run: `rtk git diff origin/main...HEAD -- Cargo.toml Cargo.lock` + +Expected: empty. + +- [ ] **Step 4: Run the full deterministic suite** + +Run: `rtk cargo test --locked` + +Expected: PASS with a nonzero selected-test count. Record ignored native/heavy +tests as `NOT_RUN`; do not execute them. + +- [ ] **Step 5: Verify privacy and scope** + +Run: `rtk rg -n '/Users/|/private/tmp|CCP_ADMISSION_TEST_ROOT|GitNexus|Serena|Bearer |ghp_|token=' src docs tests` + +Expected: no new match in the Task 1–4 diff. Existing unrelated matches must be +reported separately rather than edited. + +Run: `rtk git diff --stat 6686f39..HEAD` + +Expected: only the planned Rust, docs, and contract-test files; no receipt, +cache, target artifact, binary, or global-state record. + +- [ ] **Step 6: Request two-stage review** + +First review specification compliance against the design and this plan. Then +review code quality, lock ordering, error preservation, path privacy, and +no-mutation tests. Any Critical or Important finding starts one bounded fix +round with focused RED/GREEN evidence before re-review. + +- [ ] **Step 7: Freeze the implementation handoff** + +Record exact base, head, changed files, focused/full test counts, ignored tests, +and that no live `layout-recovery apply`, CCP `run`, Docker, push, evidence +publication, or R5 action occurred. + +Stop for a separate authorization before building a live candidate or touching +the platform admission root. A future live apply must bind exact source commit, +absolute candidate path, candidate SHA-256, recovery plan SHA-256, and one +explicit apply authorization. A later receipt run requires another exact-head +authorization. diff --git a/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md index 56cafad..94ecc43 100644 --- a/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md +++ b/docs/superpowers/specs/2026-08-24-admission-layout-recovery-design.md @@ -100,6 +100,12 @@ malformed, or mismatched digest fails closed and performs no mutation. The command is not invoked implicitly and is not evidence that a later heavy run is authorized. +Apply outcomes are closed: `recovered` means the move, synchronization, +post-validation, and explicit lock release all completed; `not_applied` means +no recovery mutation occurred; `recovery_uncertain` means the directory may +already have moved but a later synchronization, validation, or unlock step +failed. Both non-success outcomes exit `70`; only `recovered` exits `0`. + The JSON is privacy-bounded. It contains schema, classification, target kind, bounded reason codes, plan digest, and outcome. It omits absolute paths, process data, commands, repositories, users, raw record contents, and @@ -142,7 +148,8 @@ The recovery snapshot follows the coordinator's lock order: 1. validate that the root and existing queue-lock path are plain objects; 2. acquire `queue.lock` exclusively with the bounded timeout; -3. acquire `slot.lock` exclusively without changing either lock file; +3. require the existing `slot.lock` to be a plain file and acquire it + exclusively without creating or changing either lock file; 4. validate the exact CCP owner marker; 5. validate every canonical root child and reject any unknown sibling other than the exact `agent-tickets` target; @@ -178,9 +185,10 @@ After reconstructing and matching the plan under both locks, `apply`: No file is deleted. If the rename fails, the target remains in its original location and the command reports a failure. If post-rename durable sync or -validation is uncertain, the command returns the internal/unsafe-state exit -class and does not claim successful recovery. The preserved quarantine entry -remains operator evidence. +validation or explicit lock release is uncertain, the command returns +`recovery_uncertain` with the internal/unsafe-state exit class and does not +claim successful recovery. The preserved quarantine entry remains operator +evidence. Re-running `status` after a successful apply returns `not_needed`. Re-running `apply` with the old digest is non-actionable and does not change state. @@ -190,9 +198,10 @@ Re-running `status` after a successful apply returns `not_needed`. Re-running - `src/admission.rs`: recovery report/plan types, strict snapshot validation, lock-scoped status and apply operations, and unit tests. - `src/main.rs`: nested CLI parsing, bounded JSON/text rendering, and existing - admission error/exit-code mapping. -- A focused integration test file for the CLI contract and no-mutation failure - cases. + admission error/exit-code mapping; its unit tests exercise parsing and the + test-only injected-coordinator dispatch seam. +- `src/admission.rs` unit tests cover coordinator behavior and no-mutation + failure cases against owned temporary roots. - `docs/COORDINATION_RUNBOOK.md` and `docs/TROUBLESHOOTING.md`: exact operator sequence and explicit statement that manual recovery remains unsupported. From 94aa830716179b8ee7faa19cdb8ae9544ad5d097 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 04:34:43 +0200 Subject: [PATCH 11/25] feat: plan empty admission layout recovery --- src/admission.rs | 118 ++++++++++++++++++++++++++++++++++++++++++++++- src/main.rs | 47 +++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) diff --git a/src/admission.rs b/src/admission.rs index 8408e99..3b5f1b1 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -23,6 +23,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use fs2::FileExt; use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; use crate::durable_fs::{DurableFileSystem, DurableFsError}; use crate::process::CancellationToken; @@ -32,6 +33,37 @@ pub const ADMISSION_STATUS_SCHEMA_VERSION: &str = "2.0"; pub const DEFAULT_QUEUE_TIMEOUT: Duration = Duration::from_secs(15 * 60); pub const DEFAULT_STATUS_TIMEOUT: Duration = Duration::from_secs(5); pub const MAX_QUEUE_TICKETS: usize = 1024; +pub const ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION: &str = "admission-layout-recovery/1.0"; +pub const DEFAULT_LAYOUT_RECOVERY_TIMEOUT: Duration = Duration::from_secs(5); +pub const MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS: u64 = 60; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryClassificationV1 { NotNeeded, RecoverableEmptyHistoricalAgentTickets, OperatorRequired } + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryReasonV1 { CanonicalLayout, EmptyHistoricalAgentTickets, LockTimeout, ForeignOwner, UnsupportedLayout, TargetNotEmpty, CoordinatorNotIdle, QuarantineCollision, PlanMismatch, FilesystemUncertain } + +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct AdmissionLayoutRecoveryStatusV1 { + pub schema_version: String, + pub classification: AdmissionLayoutRecoveryClassificationV1, + pub target_kind: Option, + pub reason: AdmissionLayoutRecoveryReasonV1, + pub plan_sha256: Option, +} + +#[derive(Serialize)] +struct AdmissionLayoutRecoveryPlanV1 { + schema_version: &'static str, recovery_kind: &'static str, owner: String, purpose: String, + owner_schema_version: String, root_entries: Vec, + queue_lock_name: &'static str, queue_lock_kind: &'static str, queue_lock_exclusively_held: bool, + slot_lock_name: &'static str, slot_lock_kind: &'static str, slot_lock_was_free: bool, + ticket_count: usize, lease_count: usize, target_entry_count: usize, +} +#[derive(Serialize)] +struct RecoveryRootEntryV1 { name: String, kind: String } const OWNER_FILE: &str = ".ccp-admission-root-v1.json"; const PLATFORM_DIRECTORY: &str = "commit-ci-preflight-admission"; @@ -200,7 +232,7 @@ pub struct AdmissionCoordinator { impl AdmissionCoordinator { #[cfg(test)] - fn test_at(root: PathBuf) -> Self { + pub(crate) fn test_at(root: PathBuf) -> Self { Self { root } } @@ -350,6 +382,42 @@ impl AdmissionCoordinator { }) } + pub fn layout_recovery_status_with_timeout(&self, timeout: Duration, cancellation: &CancellationToken) -> AdmissionLayoutRecoveryStatusV1 { + let base = || AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), classification: AdmissionLayoutRecoveryClassificationV1::OperatorRequired, target_kind: None, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, plan_sha256: None }; + let Ok(deadline) = AdmissionDeadline::from_timeout(timeout) else { return base() }; + if !self.root_exists().unwrap_or(false) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + let Ok(entries) = fs::read_dir(&self.root) else { return base() }; + let mut root_entries = Vec::new(); + let mut target = false; + for entry in entries.flatten() { + let path = entry.path(); let name = entry.file_name().to_string_lossy().into_owned(); + if name == "agent-tickets" { target = true; continue; } + let Ok(meta) = fs::symlink_metadata(&path) else { return base() }; + if meta.file_type().is_symlink() || (!meta.is_dir() && !meta.is_file()) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + root_entries.push(RecoveryRootEntryV1 { name, kind: if meta.is_dir() { "directory" } else { "file" }.to_owned() }); + } + if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, ..base() }; } + let target_path = self.root.join("agent-tickets"); + let Ok(meta) = fs::symlink_metadata(&target_path) else { return base() }; + if meta.file_type().is_symlink() || !meta.is_dir() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + if fs::read_dir(&target_path).map(|mut d| d.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, ..base() }; } + let Ok(owner) = fs::read(self.root.join(OWNER_FILE)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; + if owner != OWNER_BYTES { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } + let Ok(mut queue) = self.open_queue(false) else { return base() }; + if lock_exclusive_until(&queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } + let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(file)) => file, Ok(None) => { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, target_kind: Some("historical_agent_tickets".into()), reason: AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, plan_sha256: Some("0".repeat(64)), ..base() }; }, Err(_) => { let _ = unlock(&mut queue); return base(); } }; + let slot_free = slot.try_lock_exclusive().is_ok(); + if !slot_free { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } + let mut slot = slot; let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + root_entries.sort_by(|a,b| a.name.cmp(&b.name)); + let plan = AdmissionLayoutRecoveryPlanV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, recovery_kind: "empty_historical_agent_tickets", owner: "commit-ci-preflight".into(), purpose: "host-admission-coordinator".into(), owner_schema_version: "1.0".into(), root_entries, queue_lock_name: QUEUE_LOCK, queue_lock_kind: "queue_lock", queue_lock_exclusively_held: true, slot_lock_name: SLOT_LOCK, slot_lock_kind: "slot_lock", slot_lock_was_free: true, ticket_count: 0, lease_count: 0, target_entry_count: 0 }; + let Ok(bytes) = serde_json::to_vec(&plan) else { return base() }; let digest = Sha256::digest(bytes); let plan_sha256 = digest.iter().map(|b| format!("{b:02x}")).collect::(); + let quarantine = self.root.join(format!("agent-tickets.recovered-v1-{plan_sha256}")); + if quarantine.exists() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::QuarantineCollision, ..base() }; } + AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), classification: AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, target_kind: Some("historical_agent_tickets".into()), reason: AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, plan_sha256: Some(plan_sha256) } + } + #[cfg(test)] fn initialize(&self) -> Result<(), AdmissionError> { let cancellation = CancellationToken::default(); @@ -1644,6 +1712,54 @@ mod tests { AdmissionCoordinator::test_at(root) } + fn coordinator_with_empty_historical_agent_tickets(label: &str) -> AdmissionCoordinator { + let coordinator = coordinator(label); + coordinator.initialize().expect("canonical coordinator"); + fs::create_dir(coordinator.root().join("agent-tickets")) + .expect("historical empty directory"); + coordinator + } + + fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { + fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { + let mut entries = fs::read_dir(path).expect("read fingerprint directory") + .collect::, _>>().expect("fingerprint entries"); + entries.sort_by_key(fs::DirEntry::file_name); + for entry in entries { + let path = entry.path(); + let relative = path.strip_prefix(root).expect("relative path").to_path_buf(); + let metadata = fs::symlink_metadata(&path).expect("fingerprint metadata"); + if metadata.file_type().is_symlink() { + out.push((relative, "symlink", fs::read_link(&path).expect("symlink target") + .to_string_lossy().as_bytes().to_vec())); + } else if metadata.is_dir() { + out.push((relative, "directory", Vec::new())); + walk(root, &path, out); + } else { + out.push((relative, "file", fs::read(&path).expect("file bytes"))); + } + } + } + let mut entries = Vec::new(); + walk(root, root, &mut entries); + entries + } + + #[test] + fn layout_recovery_normal_status_stays_closed_but_plans_empty_historical_directory() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-status"); + assert!(matches!(coordinator.status(), Err(AdmissionError::UnsafeLayout(_)))); + let before = tree_fingerprint(coordinator.root()); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, + AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets); + let digest = report.plan_sha256.expect("recovery plan"); + assert_eq!(digest.len(), 64); + assert!(digest.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))); + assert_eq!(before, tree_fingerprint(coordinator.root())); + } + struct ChildHandle { child: Child, output: BufReader, diff --git a/src/main.rs b/src/main.rs index 3dac0dc..66b14dd 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,6 +25,7 @@ use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; use commit_ci_preflight::admission::{ ADMISSION_STATUS_SCHEMA_VERSION, AdmissionCoordinator, AdmissionError, AdmissionGuard, DEFAULT_QUEUE_TIMEOUT, DEFAULT_STATUS_TIMEOUT, + DEFAULT_LAYOUT_RECOVERY_TIMEOUT, MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS, }; use commit_ci_preflight::benchmark::{ BenchmarkError, run_benchmark, verify_benchmark_document, write_new_receipt, @@ -431,6 +432,26 @@ enum AdmissionCommand { #[arg(long, default_value_t = DEFAULT_STATUS_TIMEOUT.as_secs())] timeout_seconds: u64, }, + LayoutRecovery { + #[command(subcommand)] + action: AdmissionLayoutRecoveryCommand, + }, +} + +#[derive(Debug, Subcommand)] +enum AdmissionLayoutRecoveryCommand { + Status { + #[arg(long)] + json: bool, + #[arg(long, default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), value_parser = parse_layout_recovery_timeout)] + timeout_seconds: u64, + }, +} + +fn parse_layout_recovery_timeout(value: &str) -> Result { + let seconds = value.parse::().map_err(|_| "layout recovery timeout must be an integer from 1 through 60".to_owned())?; + if !(1..=MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS).contains(&seconds) { return Err("layout recovery timeout must be an integer from 1 through 60".to_owned()); } + Ok(seconds) } fn main() { @@ -1562,6 +1583,12 @@ fn run_admission_command(action: AdmissionCommand) -> Result<(), CliError> { } Ok(()) } + AdmissionCommand::LayoutRecovery { action: AdmissionLayoutRecoveryCommand::Status { json, timeout_seconds } } => { + let report = AdmissionCoordinator::platform().map_err(CliError::Admission)?.layout_recovery_status_with_timeout(Duration::from_secs(timeout_seconds), &CancellationToken::default()); + if json { println!("{}", serde_json::to_string(&report).map_err(CliError::internal)?); } + else { println!("Layout recovery schema: {}", report.schema_version); println!("Classification: {:?}", report.classification); println!("Reason: {:?}", report.reason); println!("Plan SHA-256: {:?}", report.plan_sha256); println!("Read-only: no state was changed."); } + Ok(()) + } } } @@ -1572,6 +1599,26 @@ fn run_resource_command(action: ResourceAction) -> Result<(), CliError> { } } +#[cfg(test)] +mod task1_layout_recovery_tests { + use super::*; + + #[test] + fn admission_layout_recovery_status_parses_only_bounded_timeouts() { + let parsed = Cli::try_parse_from([ + "commit-ci-preflight", "admission", "layout-recovery", "status", "--json", + "--timeout-seconds", "5", + ]); + assert!(parsed.is_ok()); + for invalid in ["0", "61", "not-a-number"] { + assert!(Cli::try_parse_from([ + "commit-ci-preflight", "admission", "layout-recovery", "status", + "--timeout-seconds", invalid, + ]).is_err()); + } + } +} + fn print_resource_history(json: bool) -> Result<(), CliError> { let report = ResourceHistoryStore::platform() .and_then(|store| store.report_v2()) From 59b01151eb74dbf89da4a913bb8182b2e0b20b85 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 04:38:14 +0200 Subject: [PATCH 12/25] fix: require admission recovery lock evidence --- src/admission.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index 3b5f1b1..7e41014 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -64,6 +64,9 @@ struct AdmissionLayoutRecoveryPlanV1 { } #[derive(Serialize)] struct RecoveryRootEntryV1 { name: String, kind: String } +#[derive(Deserialize)] +#[serde(deny_unknown_fields)] +struct CoordinatorOwnerMarkerV1 { owner: String, purpose: String, schema_version: String } const OWNER_FILE: &str = ".ccp-admission-root-v1.json"; const PLATFORM_DIRECTORY: &str = "commit-ci-preflight-admission"; @@ -403,15 +406,16 @@ impl AdmissionCoordinator { if fs::read_dir(&target_path).map(|mut d| d.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, ..base() }; } let Ok(owner) = fs::read(self.root.join(OWNER_FILE)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; if owner != OWNER_BYTES { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } + let Ok(owner_marker) = serde_json::from_slice::(&owner) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; let Ok(mut queue) = self.open_queue(false) else { return base() }; if lock_exclusive_until(&queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } - let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(file)) => file, Ok(None) => { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, target_kind: Some("historical_agent_tickets".into()), reason: AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, plan_sha256: Some("0".repeat(64)), ..base() }; }, Err(_) => { let _ = unlock(&mut queue); return base(); } }; + let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(file)) => file, Ok(None) => { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }, Err(_) => { let _ = unlock(&mut queue); return base(); } }; let slot_free = slot.try_lock_exclusive().is_ok(); if !slot_free { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } let mut slot = slot; let _ = unlock(&mut slot); let _ = unlock(&mut queue); root_entries.sort_by(|a,b| a.name.cmp(&b.name)); - let plan = AdmissionLayoutRecoveryPlanV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, recovery_kind: "empty_historical_agent_tickets", owner: "commit-ci-preflight".into(), purpose: "host-admission-coordinator".into(), owner_schema_version: "1.0".into(), root_entries, queue_lock_name: QUEUE_LOCK, queue_lock_kind: "queue_lock", queue_lock_exclusively_held: true, slot_lock_name: SLOT_LOCK, slot_lock_kind: "slot_lock", slot_lock_was_free: true, ticket_count: 0, lease_count: 0, target_entry_count: 0 }; + let plan = AdmissionLayoutRecoveryPlanV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, recovery_kind: "empty_historical_agent_tickets", owner: owner_marker.owner, purpose: owner_marker.purpose, owner_schema_version: owner_marker.schema_version, root_entries, queue_lock_name: QUEUE_LOCK, queue_lock_kind: "queue_lock", queue_lock_exclusively_held: true, slot_lock_name: SLOT_LOCK, slot_lock_kind: "slot_lock", slot_lock_was_free: true, ticket_count: 0, lease_count: 0, target_entry_count: 0 }; let Ok(bytes) = serde_json::to_vec(&plan) else { return base() }; let digest = Sha256::digest(bytes); let plan_sha256 = digest.iter().map(|b| format!("{b:02x}")).collect::(); let quarantine = self.root.join(format!("agent-tickets.recovered-v1-{plan_sha256}")); if quarantine.exists() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::QuarantineCollision, ..base() }; } @@ -1715,11 +1719,24 @@ mod tests { fn coordinator_with_empty_historical_agent_tickets(label: &str) -> AdmissionCoordinator { let coordinator = coordinator(label); coordinator.initialize().expect("canonical coordinator"); + File::create(coordinator.root().join(SLOT_LOCK)).expect("pre-existing slot lock"); fs::create_dir(coordinator.root().join("agent-tickets")) .expect("historical empty directory"); coordinator } + #[test] + fn layout_recovery_missing_required_lock_is_operator_required_without_plan() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-missing-lock"); + fs::remove_file(coordinator.root().join(SLOT_LOCK)).expect("remove slot lock"); + let before = tree_fingerprint(coordinator.root()); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::UnsupportedLayout); + assert!(report.plan_sha256.is_none()); + assert_eq!(before, tree_fingerprint(coordinator.root())); + } + fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { let mut entries = fs::read_dir(path).expect("read fingerprint directory") From c4ba32d89e42beebad139b746720c7e70cf310c5 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 04:41:54 +0200 Subject: [PATCH 13/25] fix: validate admission recovery layout --- src/admission.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/admission.rs b/src/admission.rs index 7e41014..821513e 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -392,13 +392,27 @@ impl AdmissionCoordinator { let Ok(entries) = fs::read_dir(&self.root) else { return base() }; let mut root_entries = Vec::new(); let mut target = false; + let mut required = [false; 5]; for entry in entries.flatten() { let path = entry.path(); let name = entry.file_name().to_string_lossy().into_owned(); if name == "agent-tickets" { target = true; continue; } + let known = match name.as_str() { + OWNER_FILE => { required[0] = true; true }, + QUEUE_LOCK => { required[1] = true; true }, + SLOT_LOCK => { required[2] = true; true }, + NEXT_TICKET => { required[3] = true; true }, + TICKETS_DIR => { required[4] = true; true }, + LEASES_DIR | QUARANTINE_DIR => true, + _ => false, + }; + if !known { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } let Ok(meta) = fs::symlink_metadata(&path) else { return base() }; if meta.file_type().is_symlink() || (!meta.is_dir() && !meta.is_file()) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + if matches!(name.as_str(), TICKETS_DIR | LEASES_DIR | QUARANTINE_DIR) && !meta.is_dir() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + if matches!(name.as_str(), OWNER_FILE | QUEUE_LOCK | SLOT_LOCK | NEXT_TICKET) && !meta.is_file() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } root_entries.push(RecoveryRootEntryV1 { name, kind: if meta.is_dir() { "directory" } else { "file" }.to_owned() }); } + if !required.into_iter().all(|value| value) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, ..base() }; } let target_path = self.root.join("agent-tickets"); let Ok(meta) = fs::symlink_metadata(&target_path) else { return base() }; @@ -1720,6 +1734,7 @@ mod tests { let coordinator = coordinator(label); coordinator.initialize().expect("canonical coordinator"); File::create(coordinator.root().join(SLOT_LOCK)).expect("pre-existing slot lock"); + File::create(coordinator.root().join(NEXT_TICKET)).expect("pre-existing ticket counter"); fs::create_dir(coordinator.root().join("agent-tickets")) .expect("historical empty directory"); coordinator @@ -1737,6 +1752,28 @@ mod tests { assert_eq!(before, tree_fingerprint(coordinator.root())); } + #[test] + fn layout_recovery_unknown_sibling_is_operator_required() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-unknown"); + fs::write(coordinator.root().join("unexpected"), b"x").expect("unknown sibling"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::UnsupportedLayout); + assert!(report.plan_sha256.is_none()); + } + + #[test] + fn layout_recovery_malformed_canonical_without_target_is_not_canonical() { + let coordinator = coordinator("layout-malformed-canonical"); + fs::create_dir_all(coordinator.root()).expect("root"); + fs::write(coordinator.root().join(OWNER_FILE), OWNER_BYTES).expect("owner"); + fs::write(coordinator.root().join(QUEUE_LOCK), b"locked").expect("queue lock"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert_ne!(report.reason, AdmissionLayoutRecoveryReasonV1::CanonicalLayout); + assert!(report.plan_sha256.is_none()); + } + fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { let mut entries = fs::read_dir(path).expect("read fingerprint directory") From fc3630ffd806d90ac82ec48f4da26e6672fc4805 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 04:45:38 +0200 Subject: [PATCH 14/25] fix: validate canonical recovery contents --- src/admission.rs | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/admission.rs b/src/admission.rs index 821513e..64e78d2 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -413,6 +413,15 @@ impl AdmissionCoordinator { root_entries.push(RecoveryRootEntryV1 { name, kind: if meta.is_dir() { "directory" } else { "file" }.to_owned() }); } if !required.into_iter().all(|value| value) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + let Ok(owner_bytes) = fs::read(self.root.join(OWNER_FILE)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; + if owner_bytes != OWNER_BYTES { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } + if serde_json::from_slice::(&owner_bytes).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } + let Ok(counter) = fs::read_to_string(self.root.join(NEXT_TICKET)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; + if counter.trim().parse::().ok().filter(|value| *value > 0).is_none() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + for directory in [TICKETS_DIR, LEASES_DIR] { + let path = self.root.join(directory); + if fs::read_dir(path).map(|mut entries| entries.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } + } if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, ..base() }; } let target_path = self.root.join("agent-tickets"); let Ok(meta) = fs::symlink_metadata(&target_path) else { return base() }; @@ -1734,7 +1743,7 @@ mod tests { let coordinator = coordinator(label); coordinator.initialize().expect("canonical coordinator"); File::create(coordinator.root().join(SLOT_LOCK)).expect("pre-existing slot lock"); - File::create(coordinator.root().join(NEXT_TICKET)).expect("pre-existing ticket counter"); + fs::write(coordinator.root().join(NEXT_TICKET), b"1\n").expect("pre-existing ticket counter"); fs::create_dir(coordinator.root().join("agent-tickets")) .expect("historical empty directory"); coordinator @@ -1774,6 +1783,27 @@ mod tests { assert!(report.plan_sha256.is_none()); } + #[test] + fn layout_recovery_target_absent_wrong_owner_is_operator_required() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-owner"); + fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); + fs::write(coordinator.root().join(OWNER_FILE), b"{}\n").expect("wrong owner"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::ForeignOwner); + assert!(report.plan_sha256.is_none()); + } + + #[test] + fn layout_recovery_target_absent_malformed_counter_is_operator_required() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-counter"); + fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); + fs::write(coordinator.root().join(NEXT_TICKET), b"invalid\n").expect("bad counter"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert!(report.plan_sha256.is_none()); + } + fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { let mut entries = fs::read_dir(path).expect("read fingerprint directory") From d3ef7122df871b123a4a47585e6bc5683ba90fe0 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 04:50:40 +0200 Subject: [PATCH 15/25] fix: snapshot admission recovery locks --- src/admission.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/admission.rs b/src/admission.rs index 64e78d2..e9a12f6 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -422,6 +422,11 @@ impl AdmissionCoordinator { let path = self.root.join(directory); if fs::read_dir(path).map(|mut entries| entries.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } } + let Ok(mut snapshot_queue) = self.open_queue(false) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; + if lock_exclusive_until(&snapshot_queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } + let Ok(snapshot_slot) = open_existing_lock_file(&self.root.join(SLOT_LOCK)).and_then(|x| x.ok_or(AdmissionError::UnsafeLayout(self.root.join(SLOT_LOCK)))) else { let _ = unlock(&mut snapshot_queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; + if snapshot_slot.try_lock_exclusive().is_err() { let _ = unlock(&mut snapshot_queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } + let mut snapshot_slot = snapshot_slot; let _ = unlock(&mut snapshot_slot); let _ = unlock(&mut snapshot_queue); if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, ..base() }; } let target_path = self.root.join("agent-tickets"); let Ok(meta) = fs::symlink_metadata(&target_path) else { return base() }; @@ -1804,6 +1809,28 @@ mod tests { assert!(report.plan_sha256.is_none()); } + #[test] + fn layout_recovery_target_absent_held_queue_lock_is_operator_required() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-held-queue"); + fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); + let queue = OpenOptions::new().read(true).write(true).open(coordinator.root().join(QUEUE_LOCK)).expect("queue"); + queue.try_lock_exclusive().expect("hold queue"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_millis(30), &CancellationToken::default()); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); + assert!(report.plan_sha256.is_none()); + } + + #[test] + fn layout_recovery_target_absent_held_slot_lock_is_operator_required() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-held-slot"); + fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); + let slot = OpenOptions::new().read(true).write(true).open(coordinator.root().join(SLOT_LOCK)).expect("slot"); + slot.try_lock_exclusive().expect("hold slot"); + let report = coordinator.layout_recovery_status_with_timeout(Duration::from_millis(30), &CancellationToken::default()); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); + assert!(report.plan_sha256.is_none()); + } + fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { let mut entries = fs::read_dir(path).expect("read fingerprint directory") From 60621687793d36f0158cedd744fa48f31637e574 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:07:24 +0200 Subject: [PATCH 16/25] feat: apply hash-bound admission layout recovery --- src/admission.rs | 787 +++++++++++++++++++++++++++++++++++++++++----- src/durable_fs.rs | 82 +++++ src/main.rs | 139 +++++++- 3 files changed, 904 insertions(+), 104 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index e9a12f6..8274b84 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -39,11 +39,26 @@ pub const MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS: u64 = 60; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] -pub enum AdmissionLayoutRecoveryClassificationV1 { NotNeeded, RecoverableEmptyHistoricalAgentTickets, OperatorRequired } +pub enum AdmissionLayoutRecoveryClassificationV1 { + NotNeeded, + RecoverableEmptyHistoricalAgentTickets, + OperatorRequired, +} #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] -pub enum AdmissionLayoutRecoveryReasonV1 { CanonicalLayout, EmptyHistoricalAgentTickets, LockTimeout, ForeignOwner, UnsupportedLayout, TargetNotEmpty, CoordinatorNotIdle, QuarantineCollision, PlanMismatch, FilesystemUncertain } +pub enum AdmissionLayoutRecoveryReasonV1 { + CanonicalLayout, + EmptyHistoricalAgentTickets, + LockTimeout, + ForeignOwner, + UnsupportedLayout, + TargetNotEmpty, + CoordinatorNotIdle, + QuarantineCollision, + PlanMismatch, + FilesystemUncertain, +} #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct AdmissionLayoutRecoveryStatusV1 { @@ -54,19 +69,51 @@ pub struct AdmissionLayoutRecoveryStatusV1 { pub plan_sha256: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum AdmissionLayoutRecoveryOutcomeV1 { + Recovered, + NotApplied, + RecoveryUncertain, +} +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct AdmissionLayoutRecoveryApplyV1 { + pub schema_version: String, + pub outcome: AdmissionLayoutRecoveryOutcomeV1, + pub reason: AdmissionLayoutRecoveryReasonV1, + pub quarantine_entry: Option, +} + #[derive(Serialize)] struct AdmissionLayoutRecoveryPlanV1 { - schema_version: &'static str, recovery_kind: &'static str, owner: String, purpose: String, - owner_schema_version: String, root_entries: Vec, - queue_lock_name: &'static str, queue_lock_kind: &'static str, queue_lock_exclusively_held: bool, - slot_lock_name: &'static str, slot_lock_kind: &'static str, slot_lock_was_free: bool, - ticket_count: usize, lease_count: usize, target_entry_count: usize, + schema_version: &'static str, + recovery_kind: &'static str, + owner: String, + purpose: String, + owner_schema_version: String, + root_entries: Vec, + queue_lock_name: &'static str, + queue_lock_kind: &'static str, + queue_lock_exclusively_held: bool, + slot_lock_name: &'static str, + slot_lock_kind: &'static str, + slot_lock_was_free: bool, + ticket_count: usize, + lease_count: usize, + target_entry_count: usize, } #[derive(Serialize)] -struct RecoveryRootEntryV1 { name: String, kind: String } +struct RecoveryRootEntryV1 { + name: String, + kind: String, +} #[derive(Deserialize)] #[serde(deny_unknown_fields)] -struct CoordinatorOwnerMarkerV1 { owner: String, purpose: String, schema_version: String } +struct CoordinatorOwnerMarkerV1 { + owner: String, + purpose: String, + schema_version: String, +} const OWNER_FILE: &str = ".ccp-admission-root-v1.json"; const PLATFORM_DIRECTORY: &str = "commit-ci-preflight-admission"; @@ -385,69 +432,490 @@ impl AdmissionCoordinator { }) } - pub fn layout_recovery_status_with_timeout(&self, timeout: Duration, cancellation: &CancellationToken) -> AdmissionLayoutRecoveryStatusV1 { - let base = || AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), classification: AdmissionLayoutRecoveryClassificationV1::OperatorRequired, target_kind: None, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, plan_sha256: None }; - let Ok(deadline) = AdmissionDeadline::from_timeout(timeout) else { return base() }; - if !self.root_exists().unwrap_or(false) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - let Ok(entries) = fs::read_dir(&self.root) else { return base() }; + pub fn layout_recovery_status_with_timeout( + &self, + timeout: Duration, + cancellation: &CancellationToken, + ) -> AdmissionLayoutRecoveryStatusV1 { + let base = || AdmissionLayoutRecoveryStatusV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), + classification: AdmissionLayoutRecoveryClassificationV1::OperatorRequired, + target_kind: None, + reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + plan_sha256: None, + }; + let Ok(deadline) = AdmissionDeadline::from_timeout(timeout) else { + return base(); + }; + if !self.root_exists().unwrap_or(false) { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + let Ok(entries) = fs::read_dir(&self.root) else { + return base(); + }; let mut root_entries = Vec::new(); let mut target = false; let mut required = [false; 5]; for entry in entries.flatten() { - let path = entry.path(); let name = entry.file_name().to_string_lossy().into_owned(); - if name == "agent-tickets" { target = true; continue; } + let path = entry.path(); + let name = entry.file_name().to_string_lossy().into_owned(); + if name == "agent-tickets" { + target = true; + continue; + } let known = match name.as_str() { - OWNER_FILE => { required[0] = true; true }, - QUEUE_LOCK => { required[1] = true; true }, - SLOT_LOCK => { required[2] = true; true }, - NEXT_TICKET => { required[3] = true; true }, - TICKETS_DIR => { required[4] = true; true }, + OWNER_FILE => { + required[0] = true; + true + } + QUEUE_LOCK => { + required[1] = true; + true + } + SLOT_LOCK => { + required[2] = true; + true + } + NEXT_TICKET => { + required[3] = true; + true + } + TICKETS_DIR => { + required[4] = true; + true + } LEASES_DIR | QUARANTINE_DIR => true, _ => false, }; - if !known { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - let Ok(meta) = fs::symlink_metadata(&path) else { return base() }; - if meta.file_type().is_symlink() || (!meta.is_dir() && !meta.is_file()) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - if matches!(name.as_str(), TICKETS_DIR | LEASES_DIR | QUARANTINE_DIR) && !meta.is_dir() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - if matches!(name.as_str(), OWNER_FILE | QUEUE_LOCK | SLOT_LOCK | NEXT_TICKET) && !meta.is_file() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - root_entries.push(RecoveryRootEntryV1 { name, kind: if meta.is_dir() { "directory" } else { "file" }.to_owned() }); - } - if !required.into_iter().all(|value| value) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - let Ok(owner_bytes) = fs::read(self.root.join(OWNER_FILE)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; - if owner_bytes != OWNER_BYTES { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } - if serde_json::from_slice::(&owner_bytes).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } - let Ok(counter) = fs::read_to_string(self.root.join(NEXT_TICKET)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; - if counter.trim().parse::().ok().filter(|value| *value > 0).is_none() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } + if !known { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + let Ok(meta) = fs::symlink_metadata(&path) else { + return base(); + }; + if meta.file_type().is_symlink() || (!meta.is_dir() && !meta.is_file()) { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + if matches!(name.as_str(), TICKETS_DIR | LEASES_DIR | QUARANTINE_DIR) && !meta.is_dir() + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + if matches!( + name.as_str(), + OWNER_FILE | QUEUE_LOCK | SLOT_LOCK | NEXT_TICKET + ) && !meta.is_file() + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + root_entries.push(RecoveryRootEntryV1 { + name, + kind: if meta.is_dir() { "directory" } else { "file" }.to_owned(), + }); + } + if !required.into_iter().all(|value| value) { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + let Ok(owner_bytes) = fs::read(self.root.join(OWNER_FILE)) else { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + }; + if owner_bytes != OWNER_BYTES { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + } + if serde_json::from_slice::(&owner_bytes).is_err() { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + } + let Ok(counter) = fs::read_to_string(self.root.join(NEXT_TICKET)) else { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + }; + if counter + .trim() + .parse::() + .ok() + .filter(|value| *value > 0) + .is_none() + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } for directory in [TICKETS_DIR, LEASES_DIR] { let path = self.root.join(directory); - if fs::read_dir(path).map(|mut entries| entries.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } - } - let Ok(mut snapshot_queue) = self.open_queue(false) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; - if lock_exclusive_until(&snapshot_queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } - let Ok(snapshot_slot) = open_existing_lock_file(&self.root.join(SLOT_LOCK)).and_then(|x| x.ok_or(AdmissionError::UnsafeLayout(self.root.join(SLOT_LOCK)))) else { let _ = unlock(&mut snapshot_queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; - if snapshot_slot.try_lock_exclusive().is_err() { let _ = unlock(&mut snapshot_queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } - let mut snapshot_slot = snapshot_slot; let _ = unlock(&mut snapshot_slot); let _ = unlock(&mut snapshot_queue); - if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, ..base() }; } + if fs::read_dir(path) + .map(|mut entries| entries.next().is_some()) + .unwrap_or(true) + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, + ..base() + }; + } + } + let Ok(mut snapshot_queue) = self.open_queue(false) else { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + }; + if lock_exclusive_until( + &snapshot_queue, + &self.root.join(QUEUE_LOCK), + &deadline, + cancellation, + ) + .is_err() + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, + ..base() + }; + } + let Ok(snapshot_slot) = open_existing_lock_file(&self.root.join(SLOT_LOCK)) + .and_then(|x| x.ok_or(AdmissionError::UnsafeLayout(self.root.join(SLOT_LOCK)))) + else { + let _ = unlock(&mut snapshot_queue); + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + }; + if snapshot_slot.try_lock_exclusive().is_err() { + let _ = unlock(&mut snapshot_queue); + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, + ..base() + }; + } + let mut snapshot_slot = snapshot_slot; + let _ = unlock(&mut snapshot_slot); + let _ = unlock(&mut snapshot_queue); + if !target { + return AdmissionLayoutRecoveryStatusV1 { + classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, + reason: AdmissionLayoutRecoveryReasonV1::CanonicalLayout, + ..base() + }; + } let target_path = self.root.join("agent-tickets"); - let Ok(meta) = fs::symlink_metadata(&target_path) else { return base() }; - if meta.file_type().is_symlink() || !meta.is_dir() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } - if fs::read_dir(&target_path).map(|mut d| d.next().is_some()).unwrap_or(true) { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, ..base() }; } - let Ok(owner) = fs::read(self.root.join(OWNER_FILE)) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; - if owner != OWNER_BYTES { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; } - let Ok(owner_marker) = serde_json::from_slice::(&owner) else { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, ..base() }; }; - let Ok(mut queue) = self.open_queue(false) else { return base() }; - if lock_exclusive_until(&queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation).is_err() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } - let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(file)) => file, Ok(None) => { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }, Err(_) => { let _ = unlock(&mut queue); return base(); } }; + let Ok(meta) = fs::symlink_metadata(&target_path) else { + return base(); + }; + if meta.file_type().is_symlink() || !meta.is_dir() { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + if fs::read_dir(&target_path) + .map(|mut d| d.next().is_some()) + .unwrap_or(true) + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, + ..base() + }; + } + let Ok(owner) = fs::read(self.root.join(OWNER_FILE)) else { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + }; + if owner != OWNER_BYTES { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + } + let Ok(owner_marker) = serde_json::from_slice::(&owner) else { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::ForeignOwner, + ..base() + }; + }; + let Ok(mut queue) = self.open_queue(false) else { + return base(); + }; + if lock_exclusive_until(&queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation) + .is_err() + { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, + ..base() + }; + } + let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { + Ok(Some(file)) => file, + Ok(None) => { + let _ = unlock(&mut queue); + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + ..base() + }; + } + Err(_) => { + let _ = unlock(&mut queue); + return base(); + } + }; let slot_free = slot.try_lock_exclusive().is_ok(); - if !slot_free { let _ = unlock(&mut queue); return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } - let mut slot = slot; let _ = unlock(&mut slot); + if !slot_free { + let _ = unlock(&mut queue); + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, + ..base() + }; + } + let mut slot = slot; + let _ = unlock(&mut slot); let _ = unlock(&mut queue); - root_entries.sort_by(|a,b| a.name.cmp(&b.name)); - let plan = AdmissionLayoutRecoveryPlanV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, recovery_kind: "empty_historical_agent_tickets", owner: owner_marker.owner, purpose: owner_marker.purpose, owner_schema_version: owner_marker.schema_version, root_entries, queue_lock_name: QUEUE_LOCK, queue_lock_kind: "queue_lock", queue_lock_exclusively_held: true, slot_lock_name: SLOT_LOCK, slot_lock_kind: "slot_lock", slot_lock_was_free: true, ticket_count: 0, lease_count: 0, target_entry_count: 0 }; - let Ok(bytes) = serde_json::to_vec(&plan) else { return base() }; let digest = Sha256::digest(bytes); let plan_sha256 = digest.iter().map(|b| format!("{b:02x}")).collect::(); - let quarantine = self.root.join(format!("agent-tickets.recovered-v1-{plan_sha256}")); - if quarantine.exists() { return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::QuarantineCollision, ..base() }; } - AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), classification: AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, target_kind: Some("historical_agent_tickets".into()), reason: AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, plan_sha256: Some(plan_sha256) } + root_entries.sort_by(|a, b| a.name.cmp(&b.name)); + let plan = AdmissionLayoutRecoveryPlanV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, + recovery_kind: "empty_historical_agent_tickets", + owner: owner_marker.owner, + purpose: owner_marker.purpose, + owner_schema_version: owner_marker.schema_version, + root_entries, + queue_lock_name: QUEUE_LOCK, + queue_lock_kind: "queue_lock", + queue_lock_exclusively_held: true, + slot_lock_name: SLOT_LOCK, + slot_lock_kind: "slot_lock", + slot_lock_was_free: true, + ticket_count: 0, + lease_count: 0, + target_entry_count: 0, + }; + let Ok(bytes) = serde_json::to_vec(&plan) else { + return base(); + }; + let digest = Sha256::digest(bytes); + let plan_sha256 = digest + .iter() + .map(|b| format!("{b:02x}")) + .collect::(); + let quarantine = self + .root + .join(format!("agent-tickets.recovered-v1-{plan_sha256}")); + if quarantine.exists() { + return AdmissionLayoutRecoveryStatusV1 { + reason: AdmissionLayoutRecoveryReasonV1::QuarantineCollision, + ..base() + }; + } + AdmissionLayoutRecoveryStatusV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), + classification: + AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, + target_kind: Some("historical_agent_tickets".into()), + reason: AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, + plan_sha256: Some(plan_sha256), + } + } + + fn locked_layout_plan_sha256(&self) -> Option { + let owner = serde_json::from_slice::( + &fs::read(self.root.join(OWNER_FILE)).ok()?, + ) + .ok()?; + let mut root_entries = Vec::new(); + for entry in fs::read_dir(&self.root).ok()?.flatten() { + let path = entry.path(); + let name = entry.file_name().to_string_lossy().into_owned(); + let meta = fs::symlink_metadata(&path).ok()?; + if meta.file_type().is_symlink() { + return None; + } + if name == "agent-tickets" { + continue; + } + root_entries.push(RecoveryRootEntryV1 { + name, + kind: if meta.is_dir() { + "directory".into() + } else { + "file".into() + }, + }); + } + root_entries.sort_by(|a, b| a.name.cmp(&b.name)); + let plan = AdmissionLayoutRecoveryPlanV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, + recovery_kind: "empty_historical_agent_tickets", + owner: owner.owner, + purpose: owner.purpose, + owner_schema_version: owner.schema_version, + root_entries, + queue_lock_name: QUEUE_LOCK, + queue_lock_kind: "queue_lock", + queue_lock_exclusively_held: true, + slot_lock_name: SLOT_LOCK, + slot_lock_kind: "slot_lock", + slot_lock_was_free: true, + ticket_count: 0, + lease_count: 0, + target_entry_count: 0, + }; + let bytes = serde_json::to_vec(&plan).ok()?; + Some( + Sha256::digest(bytes) + .iter() + .map(|b| format!("{b:02x}")) + .collect(), + ) + } + + pub fn apply_layout_recovery_with_timeout( + &self, + expected_plan: &str, + timeout: Duration, + cancellation: &CancellationToken, + ) -> AdmissionLayoutRecoveryApplyV1 { + let report = |outcome, reason, entry: Option| AdmissionLayoutRecoveryApplyV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), + outcome, + reason, + quarantine_entry: entry, + }; + if expected_plan.len() != 64 + || !expected_plan + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + { + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::PlanMismatch, + None, + ); + } + let Ok(deadline) = AdmissionDeadline::from_timeout(timeout) else { + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::LockTimeout, + None, + ); + }; + let Ok(mut queue) = self.open_queue(false) else { + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + }; + if lock_exclusive_until(&queue, &self.root.join(QUEUE_LOCK), &deadline, cancellation) + .is_err() + { + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::LockTimeout, + None, + ); + } + let mut slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { + Ok(Some(f)) => f, + _ => { + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } + }; + if slot.try_lock_exclusive().is_err() { + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, + None, + ); + } + let Some(plan) = self.locked_layout_plan_sha256() else { + let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + }; + if plan != expected_plan { + let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::PlanMismatch, + None, + ); + } + let entry = format!("agent-tickets.recovered-v1-{plan}"); + let source = self.root.join("agent-tickets"); + let quarantine = self.root.join(QUARANTINE_DIR); + let destination = quarantine.join(&entry); + let result = DurableFileSystem::default().relocate_empty_directory(&source, &destination); + let outcome = match result { + Ok(()) => { + if self.validate_layout(true).is_ok() { + ( + AdmissionLayoutRecoveryOutcomeV1::Recovered, + AdmissionLayoutRecoveryReasonV1::EmptyHistoricalAgentTickets, + Some(entry), + ) + } else { + ( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + Some(entry), + ) + } + } + Err(_) => ( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ), + }; + let unlock_slot = unlock(&mut slot); + let unlock_queue = unlock(&mut queue); + if unlock_slot.is_err() || unlock_queue.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + outcome.2, + ); + } + report(outcome.0, outcome.1, outcome.2) } #[cfg(test)] @@ -1748,7 +2216,8 @@ mod tests { let coordinator = coordinator(label); coordinator.initialize().expect("canonical coordinator"); File::create(coordinator.root().join(SLOT_LOCK)).expect("pre-existing slot lock"); - fs::write(coordinator.root().join(NEXT_TICKET), b"1\n").expect("pre-existing ticket counter"); + fs::write(coordinator.root().join(NEXT_TICKET), b"1\n") + .expect("pre-existing ticket counter"); fs::create_dir(coordinator.root().join("agent-tickets")) .expect("historical empty directory"); coordinator @@ -1759,9 +2228,18 @@ mod tests { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-missing-lock"); fs::remove_file(coordinator.root().join(SLOT_LOCK)).expect("remove slot lock"); let before = tree_fingerprint(coordinator.root()); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); - assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::UnsupportedLayout); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + report.reason, + AdmissionLayoutRecoveryReasonV1::UnsupportedLayout + ); assert!(report.plan_sha256.is_none()); assert_eq!(before, tree_fingerprint(coordinator.root())); } @@ -1770,9 +2248,18 @@ mod tests { fn layout_recovery_unknown_sibling_is_operator_required() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-unknown"); fs::write(coordinator.root().join("unexpected"), b"x").expect("unknown sibling"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); - assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::UnsupportedLayout); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + report.reason, + AdmissionLayoutRecoveryReasonV1::UnsupportedLayout + ); assert!(report.plan_sha256.is_none()); } @@ -1782,9 +2269,18 @@ mod tests { fs::create_dir_all(coordinator.root()).expect("root"); fs::write(coordinator.root().join(OWNER_FILE), OWNER_BYTES).expect("owner"); fs::write(coordinator.root().join(QUEUE_LOCK), b"locked").expect("queue lock"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); - assert_ne!(report.reason, AdmissionLayoutRecoveryReasonV1::CanonicalLayout); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_ne!( + report.reason, + AdmissionLayoutRecoveryReasonV1::CanonicalLayout + ); assert!(report.plan_sha256.is_none()); } @@ -1793,8 +2289,14 @@ mod tests { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-owner"); fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); fs::write(coordinator.root().join(OWNER_FILE), b"{}\n").expect("wrong owner"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::ForeignOwner); assert!(report.plan_sha256.is_none()); } @@ -1804,8 +2306,14 @@ mod tests { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-counter"); fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); fs::write(coordinator.root().join(NEXT_TICKET), b"invalid\n").expect("bad counter"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); assert!(report.plan_sha256.is_none()); } @@ -1813,9 +2321,16 @@ mod tests { fn layout_recovery_target_absent_held_queue_lock_is_operator_required() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-held-queue"); fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); - let queue = OpenOptions::new().read(true).write(true).open(coordinator.root().join(QUEUE_LOCK)).expect("queue"); + let queue = OpenOptions::new() + .read(true) + .write(true) + .open(coordinator.root().join(QUEUE_LOCK)) + .expect("queue"); queue.try_lock_exclusive().expect("hold queue"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_millis(30), &CancellationToken::default()); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_millis(30), + &CancellationToken::default(), + ); assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); assert!(report.plan_sha256.is_none()); } @@ -1824,25 +2339,44 @@ mod tests { fn layout_recovery_target_absent_held_slot_lock_is_operator_required() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-held-slot"); fs::remove_dir(coordinator.root().join("agent-tickets")).expect("remove target"); - let slot = OpenOptions::new().read(true).write(true).open(coordinator.root().join(SLOT_LOCK)).expect("slot"); + let slot = OpenOptions::new() + .read(true) + .write(true) + .open(coordinator.root().join(SLOT_LOCK)) + .expect("slot"); slot.try_lock_exclusive().expect("hold slot"); - let report = coordinator.layout_recovery_status_with_timeout(Duration::from_millis(30), &CancellationToken::default()); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_millis(30), + &CancellationToken::default(), + ); assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); assert!(report.plan_sha256.is_none()); } fn tree_fingerprint(root: &Path) -> Vec<(PathBuf, &'static str, Vec)> { fn walk(root: &Path, path: &Path, out: &mut Vec<(PathBuf, &'static str, Vec)>) { - let mut entries = fs::read_dir(path).expect("read fingerprint directory") - .collect::, _>>().expect("fingerprint entries"); + let mut entries = fs::read_dir(path) + .expect("read fingerprint directory") + .collect::, _>>() + .expect("fingerprint entries"); entries.sort_by_key(fs::DirEntry::file_name); for entry in entries { let path = entry.path(); - let relative = path.strip_prefix(root).expect("relative path").to_path_buf(); + let relative = path + .strip_prefix(root) + .expect("relative path") + .to_path_buf(); let metadata = fs::symlink_metadata(&path).expect("fingerprint metadata"); if metadata.file_type().is_symlink() { - out.push((relative, "symlink", fs::read_link(&path).expect("symlink target") - .to_string_lossy().as_bytes().to_vec())); + out.push(( + relative, + "symlink", + fs::read_link(&path) + .expect("symlink target") + .to_string_lossy() + .as_bytes() + .to_vec(), + )); } else if metadata.is_dir() { out.push((relative, "directory", Vec::new())); walk(root, &path, out); @@ -1856,18 +2390,97 @@ mod tests { entries } + #[test] + fn apply_requires_exact_plan_and_preserves_empty_directory_in_quarantine() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-apply"); + let status = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + let plan = status.plan_sha256.expect("plan"); + let before = tree_fingerprint(coordinator.root()); + let wrong = coordinator.apply_layout_recovery_with_timeout( + &"0".repeat(64), + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(wrong.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(before, tree_fingerprint(coordinator.root())); + let applied = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(applied.outcome, AdmissionLayoutRecoveryOutcomeV1::Recovered); + let entry = applied.quarantine_entry.expect("entry"); + assert_eq!(entry, format!("agent-tickets.recovered-v1-{plan}")); + assert!(coordinator.root().join("quarantine").join(entry).is_dir()); + assert!(!coordinator.root().join("agent-tickets").exists()); + assert_eq!( + coordinator + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default() + ) + .classification, + AdmissionLayoutRecoveryClassificationV1::NotNeeded + ); + let repeated = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + repeated.outcome, + AdmissionLayoutRecoveryOutcomeV1::NotApplied + ); + } + + #[test] + fn apply_rejects_changed_plan_without_mutation() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-changed-plan"); + let plan = coordinator + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + fs::write(coordinator.root().join("unexpected-change"), b"changed\n").expect("change"); + let before = tree_fingerprint(coordinator.root()); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(result.reason, AdmissionLayoutRecoveryReasonV1::PlanMismatch); + assert_eq!(before, tree_fingerprint(coordinator.root())); + } + #[test] fn layout_recovery_normal_status_stays_closed_but_plans_empty_historical_directory() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-status"); - assert!(matches!(coordinator.status(), Err(AdmissionError::UnsafeLayout(_)))); + assert!(matches!( + coordinator.status(), + Err(AdmissionError::UnsafeLayout(_)) + )); let before = tree_fingerprint(coordinator.root()); let report = coordinator.layout_recovery_status_with_timeout( - Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(report.classification, - AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets); + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets + ); let digest = report.plan_sha256.expect("recovery plan"); assert_eq!(digest.len(), 64); - assert!(digest.bytes().all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b))); + assert!( + digest + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + ); assert_eq!(before, tree_fingerprint(coordinator.root())); } diff --git a/src/durable_fs.rs b/src/durable_fs.rs index 4017818..98e05a6 100644 --- a/src/durable_fs.rs +++ b/src/durable_fs.rs @@ -61,6 +61,36 @@ struct FaultPlan { } impl DurableFileSystem { + pub(crate) fn relocate_empty_directory( + &self, + source: &Path, + destination: &Path, + ) -> Result<(), DurableFsError> { + let source_parent = checked_parent(source)?; + let destination_parent = checked_parent(destination)?; + validate_plain_directory(source_parent)?; + validate_plain_directory(destination_parent)?; + validate_plain_directory(source)?; + if fs::read_dir(source)?.next().transpose()?.is_some() { + return Err(DurableFsError::UnsafePath("source directory must be empty")); + } + match fs::symlink_metadata(destination) { + Ok(_) => { + return Err(DurableFsError::UnsafePath( + "quarantine destination already exists", + )); + } + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(DurableFsError::Io(error)), + } + self.checkpoint()?; + fs::rename(source, destination)?; + self.checkpoint()?; + sync_directory(destination_parent)?; + self.checkpoint()?; + sync_directory(source_parent)?; + Ok(()) + } pub fn create_new_directory(&self, path: &Path) -> Result<(), DurableFsError> { let parent = checked_parent(path)?; validate_plain_directory(parent)?; @@ -326,6 +356,58 @@ mod tests { fs::remove_dir_all(root).expect("cleanup"); } + #[test] + fn relocate_empty_directory_preserves_source_as_append_only_destination() { + let root = temporary_directory("relocate-empty"); + let source = root.join("agent-tickets"); + let quarantine = root.join("quarantine"); + let destination = quarantine.join("agent-tickets.recovered-v1-plan"); + fs::create_dir(&source).expect("source"); + fs::create_dir(&quarantine).expect("quarantine"); + DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .expect("relocate"); + assert!(!source.exists()); + assert!(destination.is_dir()); + assert!( + DurableFileSystem::default() + .relocate_empty_directory(&destination, &destination) + .is_err() + ); + fs::remove_dir_all(root).expect("cleanup"); + } + + #[test] + fn relocate_empty_directory_rejects_contents_and_existing_destination() { + let root = temporary_directory("relocate-reject"); + let source = root.join("agent-tickets"); + let quarantine = root.join("quarantine"); + let destination = quarantine.join("agent-tickets.recovered-v1-plan"); + fs::create_dir(&source).expect("source"); + fs::create_dir(&quarantine).expect("quarantine"); + fs::write(source.join("entry"), b"blocked\n").expect("source entry"); + assert!( + DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .is_err() + ); + assert_eq!( + fs::read(source.join("entry")).expect("entry remains"), + b"blocked\n" + ); + assert!(!destination.exists()); + fs::remove_file(source.join("entry")).expect("remove fixture entry"); + fs::create_dir(&destination).expect("destination collision"); + assert!( + DurableFileSystem::default() + .relocate_empty_directory(&source, &destination) + .is_err() + ); + assert!(source.is_dir()); + assert!(destination.is_dir()); + fs::remove_dir_all(root).expect("cleanup"); + } + #[test] fn quarantine_requires_exact_ownership_and_same_parent() { let root = temporary_directory("quarantine"); diff --git a/src/main.rs b/src/main.rs index 66b14dd..e2589a1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,8 +24,8 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use clap::{Args, CommandFactory, Parser, Subcommand, ValueEnum}; use commit_ci_preflight::admission::{ ADMISSION_STATUS_SCHEMA_VERSION, AdmissionCoordinator, AdmissionError, AdmissionGuard, - DEFAULT_QUEUE_TIMEOUT, DEFAULT_STATUS_TIMEOUT, - DEFAULT_LAYOUT_RECOVERY_TIMEOUT, MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS, + AdmissionLayoutRecoveryOutcomeV1, DEFAULT_LAYOUT_RECOVERY_TIMEOUT, DEFAULT_QUEUE_TIMEOUT, + DEFAULT_STATUS_TIMEOUT, MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS, }; use commit_ci_preflight::benchmark::{ BenchmarkError, run_benchmark, verify_benchmark_document, write_new_receipt, @@ -446,11 +446,35 @@ enum AdmissionLayoutRecoveryCommand { #[arg(long, default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), value_parser = parse_layout_recovery_timeout)] timeout_seconds: u64, }, + Apply { + #[arg(long, value_parser = parse_plan_sha256)] + expected_plan: String, + #[arg(long)] + json: bool, + #[arg(long, default_value_t = DEFAULT_LAYOUT_RECOVERY_TIMEOUT.as_secs(), value_parser = parse_layout_recovery_timeout)] + timeout_seconds: u64, + }, +} + +fn parse_plan_sha256(value: &str) -> Result { + if value.len() == 64 + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + { + Ok(value.to_owned()) + } else { + Err("expected plan must be exactly 64 lowercase hexadecimal characters".to_owned()) + } } fn parse_layout_recovery_timeout(value: &str) -> Result { - let seconds = value.parse::().map_err(|_| "layout recovery timeout must be an integer from 1 through 60".to_owned())?; - if !(1..=MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS).contains(&seconds) { return Err("layout recovery timeout must be an integer from 1 through 60".to_owned()); } + let seconds = value + .parse::() + .map_err(|_| "layout recovery timeout must be an integer from 1 through 60".to_owned())?; + if !(1..=MAX_LAYOUT_RECOVERY_TIMEOUT_SECONDS).contains(&seconds) { + return Err("layout recovery timeout must be an integer from 1 through 60".to_owned()); + } Ok(seconds) } @@ -537,7 +561,9 @@ fn main() { } }; if let Err(error) = result { - eprintln!("error: {error}"); + if !matches!(error, CliError::ReportedExit(_)) { + eprintln!("error: {error}"); + } std::process::exit(error.exit_code()); } } @@ -1553,14 +1579,21 @@ fn run_recover_command(action: RecoverCommand) -> Result<(), CliError> { } fn run_admission_command(action: AdmissionCommand) -> Result<(), CliError> { + let coordinator = AdmissionCoordinator::platform().map_err(CliError::Admission)?; + run_admission_command_with(action, &coordinator) +} + +fn run_admission_command_with( + action: AdmissionCommand, + coordinator: &AdmissionCoordinator, +) -> Result<(), CliError> { match action { AdmissionCommand::Status { json, timeout_seconds, } => { let cancellation = CancellationToken::default(); - let status = AdmissionCoordinator::platform() - .map_err(CliError::Admission)? + let status = coordinator .status_with_timeout(Duration::from_secs(timeout_seconds), &cancellation) .map_err(CliError::Admission)?; if json { @@ -1583,12 +1616,68 @@ fn run_admission_command(action: AdmissionCommand) -> Result<(), CliError> { } Ok(()) } - AdmissionCommand::LayoutRecovery { action: AdmissionLayoutRecoveryCommand::Status { json, timeout_seconds } } => { - let report = AdmissionCoordinator::platform().map_err(CliError::Admission)?.layout_recovery_status_with_timeout(Duration::from_secs(timeout_seconds), &CancellationToken::default()); - if json { println!("{}", serde_json::to_string(&report).map_err(CliError::internal)?); } - else { println!("Layout recovery schema: {}", report.schema_version); println!("Classification: {:?}", report.classification); println!("Reason: {:?}", report.reason); println!("Plan SHA-256: {:?}", report.plan_sha256); println!("Read-only: no state was changed."); } + AdmissionCommand::LayoutRecovery { + action: + AdmissionLayoutRecoveryCommand::Status { + json, + timeout_seconds, + }, + } => { + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(timeout_seconds), + &CancellationToken::default(), + ); + if json { + println!( + "{}", + serde_json::to_string(&report).map_err(CliError::internal)? + ); + } else { + println!("Layout recovery schema: {}", report.schema_version); + println!("Classification: {:?}", report.classification); + println!("Reason: {:?}", report.reason); + println!("Plan SHA-256: {:?}", report.plan_sha256); + println!("Read-only: no state was changed."); + } Ok(()) } + AdmissionCommand::LayoutRecovery { + action: + AdmissionLayoutRecoveryCommand::Apply { + expected_plan, + json, + timeout_seconds, + }, + } => render_layout_recovery_apply(coordinator, &expected_plan, json, timeout_seconds), + } +} + +fn render_layout_recovery_apply( + coordinator: &AdmissionCoordinator, + expected_plan: &str, + json: bool, + timeout_seconds: u64, +) -> Result<(), CliError> { + let report = coordinator.apply_layout_recovery_with_timeout( + expected_plan, + Duration::from_secs(timeout_seconds), + &CancellationToken::default(), + ); + if json { + println!( + "{}", + serde_json::to_string(&report).map_err(CliError::internal)? + ); + } else { + println!("Layout recovery schema: {}", report.schema_version); + println!("Outcome: {:?}", report.outcome); + println!("Reason: {:?}", report.reason); + println!("Quarantine entry: {:?}", report.quarantine_entry); + } + match report.outcome { + AdmissionLayoutRecoveryOutcomeV1::Recovered => Ok(()), + AdmissionLayoutRecoveryOutcomeV1::NotApplied + | AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain => Err(CliError::ReportedExit(70)), } } @@ -1606,15 +1695,27 @@ mod task1_layout_recovery_tests { #[test] fn admission_layout_recovery_status_parses_only_bounded_timeouts() { let parsed = Cli::try_parse_from([ - "commit-ci-preflight", "admission", "layout-recovery", "status", "--json", - "--timeout-seconds", "5", + "commit-ci-preflight", + "admission", + "layout-recovery", + "status", + "--json", + "--timeout-seconds", + "5", ]); assert!(parsed.is_ok()); for invalid in ["0", "61", "not-a-number"] { - assert!(Cli::try_parse_from([ - "commit-ci-preflight", "admission", "layout-recovery", "status", - "--timeout-seconds", invalid, - ]).is_err()); + assert!( + Cli::try_parse_from([ + "commit-ci-preflight", + "admission", + "layout-recovery", + "status", + "--timeout-seconds", + invalid, + ]) + .is_err() + ); } } } @@ -2228,6 +2329,7 @@ impl std::error::Error for GuardExecError {} #[derive(Debug)] enum CliError { + ReportedExit(i32), Usage(Box), Cache(CacheError), Workspace(WorkspaceError), @@ -2259,6 +2361,7 @@ impl CliError { fn exit_code(&self) -> i32 { match self { + Self::ReportedExit(code) => *code, Self::Usage(_) => 2, Self::Cache(error) => error.exit_code(), Self::Workspace(_) => 2, @@ -2297,6 +2400,7 @@ impl CliError { impl fmt::Display for CliError { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Self::ReportedExit(_) => formatter.write_str("command outcome already reported"), Self::Usage(error) => write!(formatter, "{error}"), Self::Cache(error) => write!(formatter, "{error}"), Self::Workspace(error) => write!(formatter, "{error}"), @@ -2328,6 +2432,7 @@ impl fmt::Display for CliError { impl std::error::Error for CliError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { + Self::ReportedExit(_) => None, Self::Usage(error) | Self::Internal(error) => Some(error.as_ref()), Self::Cache(error) => Some(error), Self::Workspace(error) => Some(error), From 9aa52e1374506f147d07f17f1e08d4824dc6fd23 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:13:54 +0200 Subject: [PATCH 17/25] fix: classify admission recovery uncertainty --- src/admission.rs | 146 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 138 insertions(+), 8 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index 8274b84..c9df9c4 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -278,12 +278,25 @@ struct StaleTicket { #[derive(Debug, Clone)] pub struct AdmissionCoordinator { root: PathBuf, + #[cfg(test)] + durable_fault: Option, } impl AdmissionCoordinator { #[cfg(test)] pub(crate) fn test_at(root: PathBuf) -> Self { - Self { root } + Self { + root, + durable_fault: None, + } + } + + #[cfg(test)] + pub(crate) fn test_at_with_durable_fault(root: PathBuf, fail_at: usize) -> Self { + Self { + root, + durable_fault: Some(fail_at), + } } pub fn platform() -> Result { @@ -304,7 +317,11 @@ impl AdmissionCoordinator { pub fn at(root: PathBuf) -> Result { let root = validate_root_candidate(&root)?; - Ok(Self { root }) + Ok(Self { + root, + #[cfg(test)] + durable_fault: None, + }) } pub fn root(&self) -> &Path { @@ -727,6 +744,7 @@ impl AdmissionCoordinator { .collect::(); let quarantine = self .root + .join(QUARANTINE_DIR) .join(format!("agent-tickets.recovered-v1-{plan_sha256}")); if quarantine.exists() { return AdmissionLayoutRecoveryStatusV1 { @@ -861,6 +879,15 @@ impl AdmissionCoordinator { None, ); } + if !self.root.join("agent-tickets").exists() { + let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::CanonicalLayout, + None, + ); + } let Some(plan) = self.locked_layout_plan_sha256() else { let _ = unlock(&mut slot); let _ = unlock(&mut queue); @@ -883,7 +910,14 @@ impl AdmissionCoordinator { let source = self.root.join("agent-tickets"); let quarantine = self.root.join(QUARANTINE_DIR); let destination = quarantine.join(&entry); - let result = DurableFileSystem::default().relocate_empty_directory(&source, &destination); + #[cfg(test)] + let durable_fs = self + .durable_fault + .map(DurableFileSystem::failing_at) + .unwrap_or_default(); + #[cfg(not(test))] + let durable_fs = DurableFileSystem::default(); + let result = durable_fs.relocate_empty_directory(&source, &destination); let outcome = match result { Ok(()) => { if self.validate_layout(true).is_ok() { @@ -900,11 +934,22 @@ impl AdmissionCoordinator { ) } } - Err(_) => ( - AdmissionLayoutRecoveryOutcomeV1::NotApplied, - AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, - None, - ), + Err(_) => { + let outcome = outcome_after_relocation_error(&source, &destination); + let entry = + if matches!(outcome, AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain) + && destination.exists() + { + Some(entry) + } else { + None + }; + ( + outcome, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + entry, + ) + } }; let unlock_slot = unlock(&mut slot); let unlock_queue = unlock(&mut queue); @@ -1786,6 +1831,25 @@ fn unlock(file: &mut File) -> Result<(), AdmissionError> { }) } +fn outcome_after_relocation_error( + source: &Path, + destination: &Path, +) -> AdmissionLayoutRecoveryOutcomeV1 { + match ( + fs::symlink_metadata(source), + fs::symlink_metadata(destination), + ) { + (Ok(source_meta), Err(error)) + if source_meta.is_dir() + && !source_meta.file_type().is_symlink() + && error.kind() == io::ErrorKind::NotFound => + { + AdmissionLayoutRecoveryOutcomeV1::NotApplied + } + _ => AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + } +} + fn durable_error(path: PathBuf, error: DurableFsError) -> AdmissionError { match error { DurableFsError::Io(source) => AdmissionError::Io { path, source }, @@ -2458,6 +2522,72 @@ mod tests { assert_eq!(before, tree_fingerprint(coordinator.root())); } + #[test] + fn status_rejects_existing_real_quarantine_destination() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-collision"); + let plan = coordinator + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + fs::create_dir( + coordinator + .root() + .join(QUARANTINE_DIR) + .join(format!("agent-tickets.recovered-v1-{plan}")), + ) + .expect("collision"); + let status = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + status.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + status.reason, + AdmissionLayoutRecoveryReasonV1::QuarantineCollision + ); + assert!(status.plan_sha256.is_none()); + } + + #[test] + fn apply_reports_uncertain_when_durability_fails_after_rename() { + let base = coordinator_with_empty_historical_agent_tickets("layout-post-rename"); + let coordinator = + AdmissionCoordinator::test_at_with_durable_fault(base.root().to_path_buf(), 2); + let plan = coordinator + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + result.outcome, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain + ); + assert_eq!( + result.quarantine_entry, + Some(format!("agent-tickets.recovered-v1-{plan}")) + ); + assert!( + coordinator + .root() + .join(QUARANTINE_DIR) + .join(result.quarantine_entry.expect("entry")) + .is_dir() + ); + } + #[test] fn layout_recovery_normal_status_stays_closed_but_plans_empty_historical_directory() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-status"); From 531d77d2416da1cdf7d557de936a88c0c8b047a7 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:17:25 +0200 Subject: [PATCH 18/25] fix: preserve uncertain recovery destination --- src/admission.rs | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index c9df9c4..be85260 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -936,14 +936,7 @@ impl AdmissionCoordinator { } Err(_) => { let outcome = outcome_after_relocation_error(&source, &destination); - let entry = - if matches!(outcome, AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain) - && destination.exists() - { - Some(entry) - } else { - None - }; + let entry = relocation_entry_after_error(outcome, entry); ( outcome, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, @@ -1850,6 +1843,13 @@ fn outcome_after_relocation_error( } } +fn relocation_entry_after_error( + outcome: AdmissionLayoutRecoveryOutcomeV1, + entry: String, +) -> Option { + matches!(outcome, AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain).then_some(entry) +} + fn durable_error(path: PathBuf, error: DurableFsError) -> AdmissionError { match error { DurableFsError::Io(source) => AdmissionError::Io { path, source }, @@ -2588,6 +2588,16 @@ mod tests { ); } + #[test] + fn uncertain_relocation_always_reports_planned_entry() { + let entry = "agent-tickets.recovered-v1-0123456789abcdef".to_owned(); + let reported = relocation_entry_after_error( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + entry.clone(), + ); + assert_eq!(reported, Some(entry)); + } + #[test] fn layout_recovery_normal_status_stays_closed_but_plans_empty_historical_directory() { let coordinator = coordinator_with_empty_historical_agent_tickets("layout-status"); From 3eab640e5b63c39767005ec12f93c7968b3ac3da Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:22:42 +0200 Subject: [PATCH 19/25] test: harden admission layout recovery boundaries --- src/admission.rs | 93 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/src/admission.rs b/src/admission.rs index be85260..f4a01d3 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -137,6 +137,28 @@ const PROCESS_VISIBILITY_NOTE: &str = "No process visible in the local shell does not prove global inactivity."; static QUARANTINE_SEQUENCE: AtomicU64 = AtomicU64::new(0); +#[derive(Debug, Clone, Copy, Default)] +struct LayoutRecoveryEffects { + #[cfg(test)] + deny_target_inventory: bool, +} + +impl LayoutRecoveryEffects { + fn before_target_inventory(&self, path: &Path) -> Result<(), AdmissionError> { + #[cfg(test)] + if self.deny_target_inventory { + return Err(AdmissionError::Io { + path: path.to_path_buf(), + source: io::Error::new( + io::ErrorKind::PermissionDenied, + "injected layout recovery permission denial", + ), + }); + } + Ok(()) + } +} + #[derive(Debug, Clone, Copy)] struct AdmissionDeadline { at: Instant, @@ -453,6 +475,15 @@ impl AdmissionCoordinator { &self, timeout: Duration, cancellation: &CancellationToken, + ) -> AdmissionLayoutRecoveryStatusV1 { + self.layout_recovery_status_with_effects(timeout, cancellation, LayoutRecoveryEffects::default()) + } + + fn layout_recovery_status_with_effects( + &self, + timeout: Duration, + cancellation: &CancellationToken, + effects: LayoutRecoveryEffects, ) -> AdmissionLayoutRecoveryStatusV1 { let base = || AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.to_owned(), @@ -644,6 +675,9 @@ impl AdmissionCoordinator { }; } let target_path = self.root.join("agent-tickets"); + if effects.before_target_inventory(&target_path).is_err() { + return base(); + } let Ok(meta) = fs::symlink_metadata(&target_path) else { return base(); }; @@ -875,7 +909,7 @@ impl AdmissionCoordinator { let _ = unlock(&mut queue); return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, - AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, + AdmissionLayoutRecoveryReasonV1::LockTimeout, None, ); } @@ -2327,6 +2361,26 @@ mod tests { assert!(report.plan_sha256.is_none()); } + #[test] + fn layout_recovery_target_inventory_denial_is_uncertain_and_non_mutating() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-denied-inventory"); + let before = tree_fingerprint(coordinator.root()); + let report = coordinator.layout_recovery_status_with_effects( + Duration::from_millis(200), + &CancellationToken::default(), + LayoutRecoveryEffects { + #[cfg(test)] + deny_target_inventory: true, + }, + ); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); + assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain); + assert!(report.plan_sha256.is_none()); + let json = serde_json::to_string(&report).expect("serialize report"); + assert!(!json.contains(coordinator.root().to_string_lossy().as_ref())); + assert_eq!(before, tree_fingerprint(coordinator.root())); + } + #[test] fn layout_recovery_malformed_canonical_without_target_is_not_canonical() { let coordinator = coordinator("layout-malformed-canonical"); @@ -2588,6 +2642,43 @@ mod tests { ); } + #[test] + fn apply_reports_not_applied_when_durability_fails_before_rename() { + let base = coordinator_with_empty_historical_agent_tickets("layout-pre-rename"); + let coordinator = AdmissionCoordinator::test_at_with_durable_fault(base.root().to_path_buf(), 1); + let plan = coordinator + .layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()) + .plan_sha256 + .expect("plan"); + let before = tree_fingerprint(coordinator.root()); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(before, tree_fingerprint(coordinator.root())); + assert!(coordinator.root().join("agent-tickets").is_dir()); + } + + #[test] + fn apply_reports_lock_timeout_without_mutation() { + let coordinator = coordinator_with_empty_historical_agent_tickets("layout-apply-lock-timeout"); + let plan = coordinator + .layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()) + .plan_sha256 + .expect("plan"); + let slot = OpenOptions::new().read(true).write(true) + .open(coordinator.root().join(SLOT_LOCK)).expect("slot"); + slot.try_lock_exclusive().expect("hold slot"); + let before = tree_fingerprint(coordinator.root()); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, Duration::from_millis(50), &CancellationToken::default()); + assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); + assert_eq!(result.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); + assert_eq!(before, tree_fingerprint(coordinator.root())); + } + #[test] fn uncertain_relocation_always_reports_planned_entry() { let entry = "agent-tickets.recovered-v1-0123456789abcdef".to_owned(); From 94ba8d97e0ad39b844d9a264afc4a0b17376f610 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:27:40 +0200 Subject: [PATCH 20/25] test: complete admission layout recovery correction --- src/admission.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 13 ++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/admission.rs b/src/admission.rs index f4a01d3..8c998a5 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -940,10 +940,33 @@ impl AdmissionCoordinator { None, ); } + for directory in [TICKETS_DIR, LEASES_DIR] { + if fs::read_dir(self.root.join(directory)) + .map(|mut entries| entries.next().is_some()) + .unwrap_or(true) + { + let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, + None, + ); + } + } let entry = format!("agent-tickets.recovered-v1-{plan}"); let source = self.root.join("agent-tickets"); let quarantine = self.root.join(QUARANTINE_DIR); let destination = quarantine.join(&entry); + if destination.exists() { + let _ = unlock(&mut slot); + let _ = unlock(&mut queue); + return report( + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryReasonV1::QuarantineCollision, + None, + ); + } #[cfg(test)] let durable_fs = self .durable_fault @@ -2381,6 +2404,49 @@ mod tests { assert_eq!(before, tree_fingerprint(coordinator.root())); } + #[test] + fn layout_recovery_rejects_unsupported_or_nonempty_state_without_mutation() { + let cases: Vec<(&str, Box)> = vec![ + ("target staging", Box::new(|c| { fs::write(c.root().join("agent-tickets").join(".agent-ticket-staging-partial"), b"x").unwrap(); })), + ("target file", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); fs::write(c.root().join("agent-tickets"), b"x").unwrap(); })), + ("target symlink", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); std::os::unix::fs::symlink("tickets", c.root().join("agent-tickets")).unwrap(); })), + ("foreign owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"{}\n").unwrap(); })), + ("malformed owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"not-json\n").unwrap(); })), + ("missing queue lock", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); })), + ("queue symlink", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); std::os::unix::fs::symlink(SLOT_LOCK, c.root().join(QUEUE_LOCK)).unwrap(); })), + ("missing slot lock", Box::new(|c| { fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); })), + ("slot symlink", Box::new(|c| { fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); std::os::unix::fs::symlink(QUEUE_LOCK, c.root().join(SLOT_LOCK)).unwrap(); })), + ("canonical ticket", Box::new(|c| { fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); })), + ("canonical lease", Box::new(|c| { fs::write(c.root().join(LEASES_DIR).join("lease-000.json"), b"{}\n").unwrap(); })), + ("unknown sibling", Box::new(|c| { fs::write(c.root().join("unknown"), b"x").unwrap(); })), + ]; + for (name, mutate) in cases { + let c = coordinator_with_empty_historical_agent_tickets(&format!("layout-case-{name}")); + mutate(&c); + let before = tree_fingerprint(c.root()); + let report = c.layout_recovery_status_with_timeout(Duration::from_millis(200), &CancellationToken::default()); + assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired, "{name}"); + assert!(report.plan_sha256.is_none(), "{name}"); + assert_eq!(before, tree_fingerprint(c.root()), "{name}"); + } + } + + #[test] + fn layout_recovery_apply_stale_plan_matrix_is_non_mutating() { + for (name, mutate) in [ + ("target-entry", 0), ("unknown-sibling", 1), ("collision", 2), ("ticket", 3) + ] { + let c = coordinator_with_empty_historical_agent_tickets(&format!("layout-race-{name}")); + let plan = c.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()).plan_sha256.unwrap(); + let quarantine = c.root().join(QUARANTINE_DIR).join(format!("agent-tickets.recovered-v1-{plan}")); + match mutate { 0 => { fs::write(c.root().join("agent-tickets").join("late"), b"x").unwrap(); }, 1 => { fs::write(c.root().join("unknown"), b"x").unwrap(); }, 2 => { fs::create_dir(quarantine).unwrap(); }, _ => { fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); } } + let before = tree_fingerprint(c.root()); + let result = c.apply_layout_recovery_with_timeout(&plan, Duration::from_secs(1), &CancellationToken::default()); + assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied, "{name}"); + assert_eq!(before, tree_fingerprint(c.root()), "{name}"); + } + } + #[test] fn layout_recovery_malformed_canonical_without_target_is_not_canonical() { let coordinator = coordinator("layout-malformed-canonical"); diff --git a/src/main.rs b/src/main.rs index e2589a1..19b1497 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1718,6 +1718,19 @@ mod task1_layout_recovery_tests { ); } } + + #[test] + fn admission_layout_recovery_apply_parses_plan_and_timeout() { + let parsed = Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--plan-sha256", &"a".repeat(64), "--timeout-seconds", "1"]); + assert!(parsed.is_ok()); + } + + #[test] + fn admission_layout_recovery_rejects_zero_and_sixty_one_before_dispatch() { + for value in ["0", "61"] { + assert!(Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--plan-sha256", &"b".repeat(64), "--timeout-seconds", value]).is_err()); + } + } } fn print_resource_history(json: bool) -> Result<(), CliError> { From 56b5c16b8ec88a7f8a84ed383597ed87e571fa50 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:32:41 +0200 Subject: [PATCH 21/25] test: close admission recovery review gaps --- src/admission.rs | 37 ++++++++++++++++++++++++++++++++++++- src/main.rs | 4 ++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index 8c998a5..0a53ee9 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -145,6 +145,8 @@ struct LayoutRecoveryEffects { impl LayoutRecoveryEffects { fn before_target_inventory(&self, path: &Path) -> Result<(), AdmissionError> { + #[cfg(not(test))] + let _ = path; #[cfg(test)] if self.deny_target_inventory { return Err(AdmissionError::Io { @@ -2410,7 +2412,7 @@ mod tests { ("target staging", Box::new(|c| { fs::write(c.root().join("agent-tickets").join(".agent-ticket-staging-partial"), b"x").unwrap(); })), ("target file", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); fs::write(c.root().join("agent-tickets"), b"x").unwrap(); })), ("target symlink", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); std::os::unix::fs::symlink("tickets", c.root().join("agent-tickets")).unwrap(); })), - ("foreign owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"{}\n").unwrap(); })), + ("foreign owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"{\"owner\":\"foreign\",\"purpose\":\"host-admission-coordinator\",\"schema_version\":\"1.0\"}\n").unwrap(); })), ("malformed owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"not-json\n").unwrap(); })), ("missing queue lock", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); })), ("queue symlink", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); std::os::unix::fs::symlink(SLOT_LOCK, c.root().join(QUEUE_LOCK)).unwrap(); })), @@ -2425,7 +2427,9 @@ mod tests { mutate(&c); let before = tree_fingerprint(c.root()); let report = c.layout_recovery_status_with_timeout(Duration::from_millis(200), &CancellationToken::default()); + let expected = match name { "foreign owner" | "malformed owner" => AdmissionLayoutRecoveryReasonV1::ForeignOwner, "canonical ticket" | "canonical lease" => AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, "target staging" => AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, _ => AdmissionLayoutRecoveryReasonV1::UnsupportedLayout }; assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired, "{name}"); + assert_eq!(report.reason, expected, "{name}"); assert!(report.plan_sha256.is_none(), "{name}"); assert_eq!(before, tree_fingerprint(c.root()), "{name}"); } @@ -2441,10 +2445,35 @@ mod tests { let quarantine = c.root().join(QUARANTINE_DIR).join(format!("agent-tickets.recovered-v1-{plan}")); match mutate { 0 => { fs::write(c.root().join("agent-tickets").join("late"), b"x").unwrap(); }, 1 => { fs::write(c.root().join("unknown"), b"x").unwrap(); }, 2 => { fs::create_dir(quarantine).unwrap(); }, _ => { fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); } } let before = tree_fingerprint(c.root()); + let quarantine_count = fs::read_dir(c.root().join(QUARANTINE_DIR)).unwrap().count(); let result = c.apply_layout_recovery_with_timeout(&plan, Duration::from_secs(1), &CancellationToken::default()); assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied, "{name}"); assert_eq!(before, tree_fingerprint(c.root()), "{name}"); + assert_eq!(quarantine_count, fs::read_dir(c.root().join(QUARANTINE_DIR)).unwrap().count(), "{name}"); + } + } + + #[test] + fn layout_recovery_serialization_privacy_and_schema_are_bounded() { + let statuses = [ + AdmissionLayoutRecoveryClassificationV1::NotNeeded, + AdmissionLayoutRecoveryClassificationV1::RecoverableEmptyHistoricalAgentTickets, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired, + ]; + for classification in statuses { + let value = serde_json::to_value(AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), classification, target_kind: None, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, plan_sha256: None }).unwrap(); + assert_eq!(value.as_object().unwrap().len(), 5); + let text = value.to_string(); + for forbidden in ["ticket-000", "lease-", "HOME", "repository", "command"] { assert!(!text.contains(forbidden)); } + for (key, val) in value.as_object().unwrap() { if key != "schema_version" { assert!(!val.to_string().contains('/')); assert!(!val.to_string().contains('\\')); } } + } + for outcome in [AdmissionLayoutRecoveryOutcomeV1::Recovered, AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain] { + let value = serde_json::to_value(AdmissionLayoutRecoveryApplyV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), outcome, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, quarantine_entry: None }).unwrap(); + assert_eq!(value.as_object().unwrap().len(), 4); } + let status = serde_json::to_value(AdmissionStatusV1 { schema_version: ADMISSION_STATUS_SCHEMA_VERSION.into(), active: false, queue_count: 0, ticket_ids: vec![], slot: AdmissionLockStatusV1 { kind: "slot".into(), state: "free".into(), owner_run_id: None, acquired_at_unix_seconds: None, heartbeat_at_unix_seconds: None, lease_state: "none".into() }, queue_lock: AdmissionLockStatusV1 { kind: "queue".into(), state: "free".into(), owner_run_id: None, acquired_at_unix_seconds: None, heartbeat_at_unix_seconds: None, lease_state: "none".into() }, process_visibility_note: PROCESS_VISIBILITY_NOTE.into() }).unwrap(); + assert_eq!(ADMISSION_STATUS_SCHEMA_VERSION, "2.0"); + assert_eq!(status.as_object().unwrap().len(), 7); } #[test] @@ -2511,10 +2540,12 @@ mod tests { .open(coordinator.root().join(QUEUE_LOCK)) .expect("queue"); queue.try_lock_exclusive().expect("hold queue"); + let started = Instant::now(); let report = coordinator.layout_recovery_status_with_timeout( Duration::from_millis(30), &CancellationToken::default(), ); + assert!(started.elapsed() < Duration::from_secs(1)); assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); assert!(report.plan_sha256.is_none()); } @@ -2529,10 +2560,12 @@ mod tests { .open(coordinator.root().join(SLOT_LOCK)) .expect("slot"); slot.try_lock_exclusive().expect("hold slot"); + let started = Instant::now(); let report = coordinator.layout_recovery_status_with_timeout( Duration::from_millis(30), &CancellationToken::default(), ); + assert!(started.elapsed() < Duration::from_secs(1)); assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); assert!(report.plan_sha256.is_none()); } @@ -2738,8 +2771,10 @@ mod tests { .open(coordinator.root().join(SLOT_LOCK)).expect("slot"); slot.try_lock_exclusive().expect("hold slot"); let before = tree_fingerprint(coordinator.root()); + let started = Instant::now(); let result = coordinator.apply_layout_recovery_with_timeout( &plan, Duration::from_millis(50), &CancellationToken::default()); + assert!(started.elapsed() < Duration::from_secs(1)); assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); assert_eq!(result.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); assert_eq!(before, tree_fingerprint(coordinator.root())); diff --git a/src/main.rs b/src/main.rs index 19b1497..d19157d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1721,14 +1721,14 @@ mod task1_layout_recovery_tests { #[test] fn admission_layout_recovery_apply_parses_plan_and_timeout() { - let parsed = Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--plan-sha256", &"a".repeat(64), "--timeout-seconds", "1"]); + let parsed = Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--expected-plan", &"a".repeat(64), "--timeout-seconds", "1"]); assert!(parsed.is_ok()); } #[test] fn admission_layout_recovery_rejects_zero_and_sixty_one_before_dispatch() { for value in ["0", "61"] { - assert!(Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--plan-sha256", &"b".repeat(64), "--timeout-seconds", value]).is_err()); + assert!(Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--expected-plan", &"b".repeat(64), "--timeout-seconds", value]).is_err()); } } } From c00c9174049c6e1b17394234fffde0e55ac5b022 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:41:00 +0200 Subject: [PATCH 22/25] test: cover recovery apply privacy outputs --- src/admission.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/admission.rs b/src/admission.rs index 0a53ee9..719a07f 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -2476,6 +2476,36 @@ mod tests { assert_eq!(status.as_object().unwrap().len(), 7); } + #[test] + fn layout_recovery_apply_serialization_privacy_covers_all_outcomes() { + let basename = "agent-tickets.recovered-v1-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + for (outcome, entry) in [ + (AdmissionLayoutRecoveryOutcomeV1::Recovered, Some(basename.to_owned())), + (AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, Some(basename.to_owned())), + (AdmissionLayoutRecoveryOutcomeV1::NotApplied, None), + ] { + let value = serde_json::to_value(AdmissionLayoutRecoveryApplyV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), + outcome, + reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + quarantine_entry: entry, + }).unwrap(); + assert_eq!(value.as_object().unwrap().len(), 4); + assert_eq!(value["schema_version"], ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION); + let text = value.to_string(); + for forbidden in ["ticket-000", "lease-", "HOME", "repository", "command"] { + assert!(!text.contains(forbidden)); + } + for (key, val) in value.as_object().unwrap() { + if key != "schema_version" { + let rendered = val.to_string(); + assert!(!rendered.contains('/')); + assert!(!rendered.contains('\\')); + } + } + } + } + #[test] fn layout_recovery_malformed_canonical_without_target_is_not_canonical() { let coordinator = coordinator("layout-malformed-canonical"); From b4ee40f00532941c3936ee14e94f38034b41a831 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:44:40 +0200 Subject: [PATCH 23/25] docs: add hash-bound admission layout recovery --- docs/COORDINATION_RUNBOOK.md | 28 ++++++++++++++++++++++++++++ docs/TROUBLESHOOTING.md | 21 +++++++++++++++++++++ tests/agent_integration_contract.rs | 21 +++++++++++++++++++++ 3 files changed, 70 insertions(+) diff --git a/docs/COORDINATION_RUNBOOK.md b/docs/COORDINATION_RUNBOOK.md index f24eb10..f35f6c7 100644 --- a/docs/COORDINATION_RUNBOOK.md +++ b/docs/COORDINATION_RUNBOOK.md @@ -104,6 +104,34 @@ files, lease files, counters, ownership markers, or the admission root. CCP may reclaim a ticket only when its ticket OS lock is demonstrably unlocked and its valid lease is definitely expired. Manual quarantine is unsupported. +### Admission layout recovery + +When CCP reports the exact unsafe `.../agent-tickets` admission layout, use +the two-step, separately authorized recovery contract below. First obtain a +read-only status result and preserve its exact lowercase `plan_sha256`: + +```console +commit-ci-preflight admission layout-recovery status --json --timeout-seconds 5 +# preserve the exact plan_sha256 and obtain one explicit apply authorization +commit-ci-preflight admission layout-recovery apply \ + --expected-plan --json --timeout-seconds 5 +commit-ci-preflight admission status --json +``` + +Status is read-only and does not authorize apply, a heavy run, Docker, +receipt/publication, or R5. Apply authorizes only one hash-bound recovery of +the exact empty historical directory; it preserves the recovered directory +beneath deterministic quarantine and makes no rollback or deletion. A +successful apply does not authorize a later CCP run, which requires fresh +exact-head, hash-bound authorization. Manual deletion, moving, or quarantine +of filesystem entries remains unsupported. + +`recovered` is the successful outcome. `not_applied` means no recovery was +performed. `recovery_uncertain` is a hard stop with exit code `70`: preserve +both paths and the JSON, do not retry apply, move either directory manually, +or start a heavy run. Any non-empty, foreign, malformed, active, lock-timeout, +plan-mismatch, or unknown-child result remains operator-required. + ### Planned agent continuation safety boundary The owner-approved agent continuation mode is opt-in. The opt-in agent diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index e97d802..97998e6 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -76,6 +76,27 @@ supported recovery procedure. Use the [cross-activity coordination runbook](COORDINATION_RUNBOOK.md) for the owner handoff and safe-recovery matrix. +### Unsafe admission layout + +Route only the exact `UnsafeLayout(.../agent-tickets)` case to the read-only +recovery status command: + +```console +commit-ci-preflight admission layout-recovery status --json --timeout-seconds 5 +``` + +If the result is eligible, preserve its lowercase `plan_sha256` and obtain +separate authorization for one exact hash-bound apply. Apply preserves the +deterministic quarantine entry; it does not authorize a later CCP run, Docker, +receipt/publication, or R5. A later heavy run needs fresh exact-head, +hash-bound authorization. Manual deletion, moving, or quarantine is not a +supported cleanup recipe. + +`recovery_uncertain` is a hard stop and exit code `70`: preserve both paths and +the JSON, do not retry apply, and do not start a heavy run. `not_applied` means +no recovery occurred. Any non-empty, foreign, malformed, active, lock-timeout, +plan-mismatch, or unknown-child result remains code `70` and operator-required. + ## Run ended but no receipt exists A receipt is the final product of successful orchestration, not a start marker. diff --git a/tests/agent_integration_contract.rs b/tests/agent_integration_contract.rs index aada562..5005de6 100644 --- a/tests/agent_integration_contract.rs +++ b/tests/agent_integration_contract.rs @@ -124,6 +124,27 @@ fn multi_harness_reference_has_a_complete_truthful_l1_surface() { assert!(readme.contains("docs/agent-integrations/HARNESS_INTEGRATION.md")); } +#[test] +fn admission_layout_recovery_guidance_is_hash_bound_and_never_manual() { + let root = Path::new(env!("CARGO_MANIFEST_DIR")); + let runbook = read(root, "docs/COORDINATION_RUNBOOK.md"); + let troubleshooting = read(root, "docs/TROUBLESHOOTING.md"); + for required in [ + "admission layout-recovery status --json", + "admission layout-recovery apply", + "--expected-plan", + "recovery_uncertain", + "does not authorize", + "Manual deletion", + ] { + assert!( + runbook.contains(required) || troubleshooting.contains(required), + "missing recovery boundary: {required}" + ); + } + assert!(!runbook.contains("ignore `agent-tickets`")); +} + fn read(root: &Path, relative: &str) -> String { fs::read_to_string(root.join(relative)).unwrap_or_else(|error| { panic!("read {relative}: {error}"); From 89b48a5a9e135a4fb45cc326e99f5e3d41c846a6 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 05:53:55 +0200 Subject: [PATCH 24/25] style: format admission layout recovery --- src/admission.rs | 291 ++++++++++++++++++++++++++++++++++++++++------- src/main.rs | 25 +++- 2 files changed, 271 insertions(+), 45 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index 719a07f..1a0be7a 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -478,7 +478,11 @@ impl AdmissionCoordinator { timeout: Duration, cancellation: &CancellationToken, ) -> AdmissionLayoutRecoveryStatusV1 { - self.layout_recovery_status_with_effects(timeout, cancellation, LayoutRecoveryEffects::default()) + self.layout_recovery_status_with_effects( + timeout, + cancellation, + LayoutRecoveryEffects::default(), + ) } fn layout_recovery_status_with_effects( @@ -2388,7 +2392,8 @@ mod tests { #[test] fn layout_recovery_target_inventory_denial_is_uncertain_and_non_mutating() { - let coordinator = coordinator_with_empty_historical_agent_tickets("layout-denied-inventory"); + let coordinator = + coordinator_with_empty_historical_agent_tickets("layout-denied-inventory"); let before = tree_fingerprint(coordinator.root()); let report = coordinator.layout_recovery_status_with_effects( Duration::from_millis(200), @@ -2398,8 +2403,14 @@ mod tests { deny_target_inventory: true, }, ); - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired); - assert_eq!(report.reason, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + report.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); assert!(report.plan_sha256.is_none()); let json = serde_json::to_string(&report).expect("serialize report"); assert!(!json.contains(coordinator.root().to_string_lossy().as_ref())); @@ -2409,26 +2420,112 @@ mod tests { #[test] fn layout_recovery_rejects_unsupported_or_nonempty_state_without_mutation() { let cases: Vec<(&str, Box)> = vec![ - ("target staging", Box::new(|c| { fs::write(c.root().join("agent-tickets").join(".agent-ticket-staging-partial"), b"x").unwrap(); })), - ("target file", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); fs::write(c.root().join("agent-tickets"), b"x").unwrap(); })), - ("target symlink", Box::new(|c| { fs::remove_dir(c.root().join("agent-tickets")).unwrap(); std::os::unix::fs::symlink("tickets", c.root().join("agent-tickets")).unwrap(); })), - ("foreign owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"{\"owner\":\"foreign\",\"purpose\":\"host-admission-coordinator\",\"schema_version\":\"1.0\"}\n").unwrap(); })), - ("malformed owner", Box::new(|c| { fs::write(c.root().join(OWNER_FILE), b"not-json\n").unwrap(); })), - ("missing queue lock", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); })), - ("queue symlink", Box::new(|c| { fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); std::os::unix::fs::symlink(SLOT_LOCK, c.root().join(QUEUE_LOCK)).unwrap(); })), - ("missing slot lock", Box::new(|c| { fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); })), - ("slot symlink", Box::new(|c| { fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); std::os::unix::fs::symlink(QUEUE_LOCK, c.root().join(SLOT_LOCK)).unwrap(); })), - ("canonical ticket", Box::new(|c| { fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); })), - ("canonical lease", Box::new(|c| { fs::write(c.root().join(LEASES_DIR).join("lease-000.json"), b"{}\n").unwrap(); })), - ("unknown sibling", Box::new(|c| { fs::write(c.root().join("unknown"), b"x").unwrap(); })), + ( + "target staging", + Box::new(|c| { + fs::write( + c.root() + .join("agent-tickets") + .join(".agent-ticket-staging-partial"), + b"x", + ) + .unwrap(); + }), + ), + ( + "target file", + Box::new(|c| { + fs::remove_dir(c.root().join("agent-tickets")).unwrap(); + fs::write(c.root().join("agent-tickets"), b"x").unwrap(); + }), + ), + ( + "target symlink", + Box::new(|c| { + fs::remove_dir(c.root().join("agent-tickets")).unwrap(); + std::os::unix::fs::symlink("tickets", c.root().join("agent-tickets")).unwrap(); + }), + ), + ( + "foreign owner", + Box::new(|c| { + fs::write(c.root().join(OWNER_FILE), b"{\"owner\":\"foreign\",\"purpose\":\"host-admission-coordinator\",\"schema_version\":\"1.0\"}\n").unwrap(); + }), + ), + ( + "malformed owner", + Box::new(|c| { + fs::write(c.root().join(OWNER_FILE), b"not-json\n").unwrap(); + }), + ), + ( + "missing queue lock", + Box::new(|c| { + fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); + }), + ), + ( + "queue symlink", + Box::new(|c| { + fs::remove_file(c.root().join(QUEUE_LOCK)).unwrap(); + std::os::unix::fs::symlink(SLOT_LOCK, c.root().join(QUEUE_LOCK)).unwrap(); + }), + ), + ( + "missing slot lock", + Box::new(|c| { + fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); + }), + ), + ( + "slot symlink", + Box::new(|c| { + fs::remove_file(c.root().join(SLOT_LOCK)).unwrap(); + std::os::unix::fs::symlink(QUEUE_LOCK, c.root().join(SLOT_LOCK)).unwrap(); + }), + ), + ( + "canonical ticket", + Box::new(|c| { + fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); + }), + ), + ( + "canonical lease", + Box::new(|c| { + fs::write(c.root().join(LEASES_DIR).join("lease-000.json"), b"{}\n").unwrap(); + }), + ), + ( + "unknown sibling", + Box::new(|c| { + fs::write(c.root().join("unknown"), b"x").unwrap(); + }), + ), ]; for (name, mutate) in cases { let c = coordinator_with_empty_historical_agent_tickets(&format!("layout-case-{name}")); mutate(&c); let before = tree_fingerprint(c.root()); - let report = c.layout_recovery_status_with_timeout(Duration::from_millis(200), &CancellationToken::default()); - let expected = match name { "foreign owner" | "malformed owner" => AdmissionLayoutRecoveryReasonV1::ForeignOwner, "canonical ticket" | "canonical lease" => AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, "target staging" => AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, _ => AdmissionLayoutRecoveryReasonV1::UnsupportedLayout }; - assert_eq!(report.classification, AdmissionLayoutRecoveryClassificationV1::OperatorRequired, "{name}"); + let report = c.layout_recovery_status_with_timeout( + Duration::from_millis(200), + &CancellationToken::default(), + ); + let expected = match name { + "foreign owner" | "malformed owner" => { + AdmissionLayoutRecoveryReasonV1::ForeignOwner + } + "canonical ticket" | "canonical lease" => { + AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle + } + "target staging" => AdmissionLayoutRecoveryReasonV1::TargetNotEmpty, + _ => AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, + }; + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired, + "{name}" + ); assert_eq!(report.reason, expected, "{name}"); assert!(report.plan_sha256.is_none(), "{name}"); assert_eq!(before, tree_fingerprint(c.root()), "{name}"); @@ -2438,18 +2535,55 @@ mod tests { #[test] fn layout_recovery_apply_stale_plan_matrix_is_non_mutating() { for (name, mutate) in [ - ("target-entry", 0), ("unknown-sibling", 1), ("collision", 2), ("ticket", 3) + ("target-entry", 0), + ("unknown-sibling", 1), + ("collision", 2), + ("ticket", 3), ] { let c = coordinator_with_empty_historical_agent_tickets(&format!("layout-race-{name}")); - let plan = c.layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()).plan_sha256.unwrap(); - let quarantine = c.root().join(QUARANTINE_DIR).join(format!("agent-tickets.recovered-v1-{plan}")); - match mutate { 0 => { fs::write(c.root().join("agent-tickets").join("late"), b"x").unwrap(); }, 1 => { fs::write(c.root().join("unknown"), b"x").unwrap(); }, 2 => { fs::create_dir(quarantine).unwrap(); }, _ => { fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); } } + let plan = c + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .unwrap(); + let quarantine = c + .root() + .join(QUARANTINE_DIR) + .join(format!("agent-tickets.recovered-v1-{plan}")); + match mutate { + 0 => { + fs::write(c.root().join("agent-tickets").join("late"), b"x").unwrap(); + } + 1 => { + fs::write(c.root().join("unknown"), b"x").unwrap(); + } + 2 => { + fs::create_dir(quarantine).unwrap(); + } + _ => { + fs::write(c.root().join(TICKETS_DIR).join("ticket-000.json"), b"{}\n").unwrap(); + } + } let before = tree_fingerprint(c.root()); let quarantine_count = fs::read_dir(c.root().join(QUARANTINE_DIR)).unwrap().count(); - let result = c.apply_layout_recovery_with_timeout(&plan, Duration::from_secs(1), &CancellationToken::default()); - assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied, "{name}"); + let result = c.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + result.outcome, + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + "{name}" + ); assert_eq!(before, tree_fingerprint(c.root()), "{name}"); - assert_eq!(quarantine_count, fs::read_dir(c.root().join(QUARANTINE_DIR)).unwrap().count(), "{name}"); + assert_eq!( + quarantine_count, + fs::read_dir(c.root().join(QUARANTINE_DIR)).unwrap().count(), + "{name}" + ); } } @@ -2461,17 +2595,64 @@ mod tests { AdmissionLayoutRecoveryClassificationV1::OperatorRequired, ]; for classification in statuses { - let value = serde_json::to_value(AdmissionLayoutRecoveryStatusV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), classification, target_kind: None, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, plan_sha256: None }).unwrap(); + let value = serde_json::to_value(AdmissionLayoutRecoveryStatusV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), + classification, + target_kind: None, + reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + plan_sha256: None, + }) + .unwrap(); assert_eq!(value.as_object().unwrap().len(), 5); let text = value.to_string(); - for forbidden in ["ticket-000", "lease-", "HOME", "repository", "command"] { assert!(!text.contains(forbidden)); } - for (key, val) in value.as_object().unwrap() { if key != "schema_version" { assert!(!val.to_string().contains('/')); assert!(!val.to_string().contains('\\')); } } + for forbidden in ["ticket-000", "lease-", "HOME", "repository", "command"] { + assert!(!text.contains(forbidden)); + } + for (key, val) in value.as_object().unwrap() { + if key != "schema_version" { + assert!(!val.to_string().contains('/')); + assert!(!val.to_string().contains('\\')); + } + } } - for outcome in [AdmissionLayoutRecoveryOutcomeV1::Recovered, AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain] { - let value = serde_json::to_value(AdmissionLayoutRecoveryApplyV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), outcome, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, quarantine_entry: None }).unwrap(); + for outcome in [ + AdmissionLayoutRecoveryOutcomeV1::Recovered, + AdmissionLayoutRecoveryOutcomeV1::NotApplied, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + ] { + let value = serde_json::to_value(AdmissionLayoutRecoveryApplyV1 { + schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION.into(), + outcome, + reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + quarantine_entry: None, + }) + .unwrap(); assert_eq!(value.as_object().unwrap().len(), 4); } - let status = serde_json::to_value(AdmissionStatusV1 { schema_version: ADMISSION_STATUS_SCHEMA_VERSION.into(), active: false, queue_count: 0, ticket_ids: vec![], slot: AdmissionLockStatusV1 { kind: "slot".into(), state: "free".into(), owner_run_id: None, acquired_at_unix_seconds: None, heartbeat_at_unix_seconds: None, lease_state: "none".into() }, queue_lock: AdmissionLockStatusV1 { kind: "queue".into(), state: "free".into(), owner_run_id: None, acquired_at_unix_seconds: None, heartbeat_at_unix_seconds: None, lease_state: "none".into() }, process_visibility_note: PROCESS_VISIBILITY_NOTE.into() }).unwrap(); + let status = serde_json::to_value(AdmissionStatusV1 { + schema_version: ADMISSION_STATUS_SCHEMA_VERSION.into(), + active: false, + queue_count: 0, + ticket_ids: vec![], + slot: AdmissionLockStatusV1 { + kind: "slot".into(), + state: "free".into(), + owner_run_id: None, + acquired_at_unix_seconds: None, + heartbeat_at_unix_seconds: None, + lease_state: "none".into(), + }, + queue_lock: AdmissionLockStatusV1 { + kind: "queue".into(), + state: "free".into(), + owner_run_id: None, + acquired_at_unix_seconds: None, + heartbeat_at_unix_seconds: None, + lease_state: "none".into(), + }, + process_visibility_note: PROCESS_VISIBILITY_NOTE.into(), + }) + .unwrap(); assert_eq!(ADMISSION_STATUS_SCHEMA_VERSION, "2.0"); assert_eq!(status.as_object().unwrap().len(), 7); } @@ -2480,8 +2661,14 @@ mod tests { fn layout_recovery_apply_serialization_privacy_covers_all_outcomes() { let basename = "agent-tickets.recovered-v1-0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; for (outcome, entry) in [ - (AdmissionLayoutRecoveryOutcomeV1::Recovered, Some(basename.to_owned())), - (AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, Some(basename.to_owned())), + ( + AdmissionLayoutRecoveryOutcomeV1::Recovered, + Some(basename.to_owned()), + ), + ( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + Some(basename.to_owned()), + ), (AdmissionLayoutRecoveryOutcomeV1::NotApplied, None), ] { let value = serde_json::to_value(AdmissionLayoutRecoveryApplyV1 { @@ -2489,9 +2676,13 @@ mod tests { outcome, reason: AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, quarantine_entry: entry, - }).unwrap(); + }) + .unwrap(); assert_eq!(value.as_object().unwrap().len(), 4); - assert_eq!(value["schema_version"], ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION); + assert_eq!( + value["schema_version"], + ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION + ); let text = value.to_string(); for forbidden in ["ticket-000", "lease-", "HOME", "repository", "command"] { assert!(!text.contains(forbidden)); @@ -2774,9 +2965,13 @@ mod tests { #[test] fn apply_reports_not_applied_when_durability_fails_before_rename() { let base = coordinator_with_empty_historical_agent_tickets("layout-pre-rename"); - let coordinator = AdmissionCoordinator::test_at_with_durable_fault(base.root().to_path_buf(), 1); + let coordinator = + AdmissionCoordinator::test_at_with_durable_fault(base.root().to_path_buf(), 1); let plan = coordinator - .layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()) + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) .plan_sha256 .expect("plan"); let before = tree_fingerprint(coordinator.root()); @@ -2792,18 +2987,28 @@ mod tests { #[test] fn apply_reports_lock_timeout_without_mutation() { - let coordinator = coordinator_with_empty_historical_agent_tickets("layout-apply-lock-timeout"); + let coordinator = + coordinator_with_empty_historical_agent_tickets("layout-apply-lock-timeout"); let plan = coordinator - .layout_recovery_status_with_timeout(Duration::from_secs(1), &CancellationToken::default()) + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) .plan_sha256 .expect("plan"); - let slot = OpenOptions::new().read(true).write(true) - .open(coordinator.root().join(SLOT_LOCK)).expect("slot"); + let slot = OpenOptions::new() + .read(true) + .write(true) + .open(coordinator.root().join(SLOT_LOCK)) + .expect("slot"); slot.try_lock_exclusive().expect("hold slot"); let before = tree_fingerprint(coordinator.root()); let started = Instant::now(); let result = coordinator.apply_layout_recovery_with_timeout( - &plan, Duration::from_millis(50), &CancellationToken::default()); + &plan, + Duration::from_millis(50), + &CancellationToken::default(), + ); assert!(started.elapsed() < Duration::from_secs(1)); assert_eq!(result.outcome, AdmissionLayoutRecoveryOutcomeV1::NotApplied); assert_eq!(result.reason, AdmissionLayoutRecoveryReasonV1::LockTimeout); diff --git a/src/main.rs b/src/main.rs index d19157d..6806555 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1721,14 +1721,35 @@ mod task1_layout_recovery_tests { #[test] fn admission_layout_recovery_apply_parses_plan_and_timeout() { - let parsed = Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--expected-plan", &"a".repeat(64), "--timeout-seconds", "1"]); + let parsed = Cli::try_parse_from([ + "commit-ci-preflight", + "admission", + "layout-recovery", + "apply", + "--expected-plan", + &"a".repeat(64), + "--timeout-seconds", + "1", + ]); assert!(parsed.is_ok()); } #[test] fn admission_layout_recovery_rejects_zero_and_sixty_one_before_dispatch() { for value in ["0", "61"] { - assert!(Cli::try_parse_from(["commit-ci-preflight", "admission", "layout-recovery", "apply", "--expected-plan", &"b".repeat(64), "--timeout-seconds", value]).is_err()); + assert!( + Cli::try_parse_from([ + "commit-ci-preflight", + "admission", + "layout-recovery", + "apply", + "--expected-plan", + &"b".repeat(64), + "--timeout-seconds", + value + ]) + .is_err() + ); } } } From 742ac544ebaedfcc656b683424d9045da30a2b32 Mon Sep 17 00:00:00 2001 From: Marco Porcellato Date: Mon, 24 Aug 2026 06:25:27 +0200 Subject: [PATCH 25/25] fix: fail closed on recovery unlock errors --- src/admission.rs | 313 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 289 insertions(+), 24 deletions(-) diff --git a/src/admission.rs b/src/admission.rs index 1a0be7a..cd8318f 100644 --- a/src/admission.rs +++ b/src/admission.rs @@ -16,6 +16,10 @@ use std::fmt; use std::fs::{self, File, OpenOptions}; use std::io::{self, Write}; use std::path::{Component, Path, PathBuf}; +#[cfg(test)] +use std::sync::Arc; +#[cfg(test)] +use std::sync::atomic::AtomicUsize; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc::{self, RecvTimeoutError, Sender}; use std::thread; @@ -304,6 +308,8 @@ pub struct AdmissionCoordinator { root: PathBuf, #[cfg(test)] durable_fault: Option, + #[cfg(test)] + unlock_fault: Option>, } impl AdmissionCoordinator { @@ -312,6 +318,7 @@ impl AdmissionCoordinator { Self { root, durable_fault: None, + unlock_fault: None, } } @@ -320,6 +327,16 @@ impl AdmissionCoordinator { Self { root, durable_fault: Some(fail_at), + unlock_fault: None, + } + } + + #[cfg(test)] + pub(crate) fn test_at_with_unlock_fault(root: PathBuf, fail_at: usize) -> Self { + Self { + root, + durable_fault: None, + unlock_fault: Some(Arc::new(AtomicUsize::new(fail_at))), } } @@ -345,6 +362,8 @@ impl AdmissionCoordinator { root, #[cfg(test)] durable_fault: None, + #[cfg(test)] + unlock_fault: None, }) } @@ -657,22 +676,36 @@ impl AdmissionCoordinator { let Ok(snapshot_slot) = open_existing_lock_file(&self.root.join(SLOT_LOCK)) .and_then(|x| x.ok_or(AdmissionError::UnsafeLayout(self.root.join(SLOT_LOCK)))) else { - let _ = unlock(&mut snapshot_queue); + if self + .release_recovery_locks(None, &mut snapshot_queue) + .is_err() + { + return base(); + } return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; }; if snapshot_slot.try_lock_exclusive().is_err() { - let _ = unlock(&mut snapshot_queue); + if self + .release_recovery_locks(None, &mut snapshot_queue) + .is_err() + { + return base(); + } return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::LockTimeout, ..base() }; } let mut snapshot_slot = snapshot_slot; - let _ = unlock(&mut snapshot_slot); - let _ = unlock(&mut snapshot_queue); + if self + .release_recovery_locks(Some(&mut snapshot_slot), &mut snapshot_queue) + .is_err() + { + return base(); + } if !target { return AdmissionLayoutRecoveryStatusV1 { classification: AdmissionLayoutRecoveryClassificationV1::NotNeeded, @@ -734,28 +767,41 @@ impl AdmissionCoordinator { let slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(file)) => file, Ok(None) => { - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(None, &mut queue); + if release.is_err() { + return base(); + } return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::UnsupportedLayout, ..base() }; } Err(_) => { - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(None, &mut queue); + if release.is_err() { + return base(); + } return base(); } }; let slot_free = slot.try_lock_exclusive().is_ok(); if !slot_free { - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(None, &mut queue); + if release.is_err() { + return base(); + } return AdmissionLayoutRecoveryStatusV1 { reason: AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, ..base() }; } let mut slot = slot; - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + if self + .release_recovery_locks(Some(&mut slot), &mut queue) + .is_err() + { + return base(); + } root_entries.sort_by(|a, b| a.name.cmp(&b.name)); let plan = AdmissionLayoutRecoveryPlanV1 { schema_version: ADMISSION_LAYOUT_RECOVERY_SCHEMA_VERSION, @@ -802,6 +848,53 @@ impl AdmissionCoordinator { } } + #[cfg(test)] + fn recovery_unlock(&self, file: &mut File, path: &Path) -> Result<(), AdmissionError> { + let result = FileExt::unlock(file).map_err(|source| AdmissionError::Lock { + path: path.to_path_buf(), + source, + }); + if let Some(counter) = &self.unlock_fault + && counter + .fetch_update(Ordering::SeqCst, Ordering::SeqCst, |n| n.checked_sub(1)) + .is_ok() + && result.is_ok() + { + return Err(AdmissionError::Lock { + path: path.to_path_buf(), + source: io::Error::new(io::ErrorKind::Other, "injected unlock failure"), + }); + } + result + } + + fn release_recovery_locks( + &self, + slot: Option<&mut File>, + queue: &mut File, + ) -> Result<(), AdmissionError> { + let mut error = None; + if let Some(slot) = slot { + #[cfg(test)] + let result = self.recovery_unlock(slot, &self.root.join(SLOT_LOCK)); + #[cfg(not(test))] + let result = unlock(slot); + if let Err(err) = result { + error = Some(err); + } + } + #[cfg(test)] + let result = self.recovery_unlock(queue, &self.root.join(QUEUE_LOCK)); + #[cfg(not(test))] + let result = unlock(queue); + if let Err(err) = result { + if error.is_none() { + error = Some(err); + } + } + error.map_or(Ok(()), Err) + } + fn locked_layout_plan_sha256(&self) -> Option { let owner = serde_json::from_slice::( &fs::read(self.root.join(OWNER_FILE)).ok()?, @@ -903,7 +996,13 @@ impl AdmissionCoordinator { let mut slot = match open_existing_lock_file(&self.root.join(SLOT_LOCK)) { Ok(Some(f)) => f, _ => { - let _ = unlock(&mut queue); + if self.release_recovery_locks(None, &mut queue).is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, @@ -912,7 +1011,13 @@ impl AdmissionCoordinator { } }; if slot.try_lock_exclusive().is_err() { - let _ = unlock(&mut queue); + if self.release_recovery_locks(None, &mut queue).is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::LockTimeout, @@ -920,8 +1025,14 @@ impl AdmissionCoordinator { ); } if !self.root.join("agent-tickets").exists() { - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(Some(&mut slot), &mut queue); + if release.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::CanonicalLayout, @@ -929,8 +1040,14 @@ impl AdmissionCoordinator { ); } let Some(plan) = self.locked_layout_plan_sha256() else { - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(Some(&mut slot), &mut queue); + if release.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, @@ -938,8 +1055,14 @@ impl AdmissionCoordinator { ); }; if plan != expected_plan { - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(Some(&mut slot), &mut queue); + if release.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::PlanMismatch, @@ -951,8 +1074,14 @@ impl AdmissionCoordinator { .map(|mut entries| entries.next().is_some()) .unwrap_or(true) { - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(Some(&mut slot), &mut queue); + if release.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::CoordinatorNotIdle, @@ -965,8 +1094,14 @@ impl AdmissionCoordinator { let quarantine = self.root.join(QUARANTINE_DIR); let destination = quarantine.join(&entry); if destination.exists() { - let _ = unlock(&mut slot); - let _ = unlock(&mut queue); + let release = self.release_recovery_locks(Some(&mut slot), &mut queue); + if release.is_err() { + return report( + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, + None, + ); + } return report( AdmissionLayoutRecoveryOutcomeV1::NotApplied, AdmissionLayoutRecoveryReasonV1::QuarantineCollision, @@ -1007,9 +1142,10 @@ impl AdmissionCoordinator { ) } }; - let unlock_slot = unlock(&mut slot); - let unlock_queue = unlock(&mut queue); - if unlock_slot.is_err() || unlock_queue.is_err() { + if self + .release_recovery_locks(Some(&mut slot), &mut queue) + .is_err() + { return report( AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain, AdmissionLayoutRecoveryReasonV1::FilesystemUncertain, @@ -2587,6 +2723,135 @@ mod tests { } } + #[test] + fn layout_recovery_status_unlock_failure_is_filesystem_uncertain() { + let ordinary = coordinator_with_empty_historical_agent_tickets("layout-unlock-status"); + let coordinator = + AdmissionCoordinator::test_at_with_unlock_fault(ordinary.root().to_path_buf(), 1); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + report.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); + } + + #[test] + fn layout_recovery_canonical_snapshot_release_failure_is_uncertain() { + let ordinary = coordinator_with_empty_historical_agent_tickets("layout-unlock-canonical"); + fs::remove_dir(ordinary.root().join("agent-tickets")).expect("canonical target absent"); + let coordinator = + AdmissionCoordinator::test_at_with_unlock_fault(ordinary.root().to_path_buf(), 1); + let report = coordinator.layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + report.classification, + AdmissionLayoutRecoveryClassificationV1::OperatorRequired + ); + assert_eq!( + report.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); + } + + #[test] + fn layout_recovery_apply_pre_move_unlock_failure_is_uncertain_and_non_mutating() { + let ordinary = coordinator_with_empty_historical_agent_tickets("layout-unlock-premove"); + let plan = ordinary + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + fs::write(ordinary.root().join("late-unknown"), b"x").expect("race mutation"); + let before = tree_fingerprint(ordinary.root()); + let coordinator = + AdmissionCoordinator::test_at_with_unlock_fault(ordinary.root().to_path_buf(), 1); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + result.outcome, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain + ); + assert_eq!( + result.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); + assert_eq!(before, tree_fingerprint(ordinary.root())); + } + + #[test] + fn layout_recovery_apply_final_unlock_failure_preserves_moved_entry() { + let ordinary = coordinator_with_empty_historical_agent_tickets("layout-unlock-final"); + let plan = ordinary + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + let coordinator = + AdmissionCoordinator::test_at_with_unlock_fault(ordinary.root().to_path_buf(), 1); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + result.outcome, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain + ); + assert_eq!( + result.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); + let entry = result.quarantine_entry.expect("preserved entry"); + assert!(coordinator.root().join(QUARANTINE_DIR).join(entry).is_dir()); + } + + #[test] + fn layout_recovery_apply_missing_slot_release_failure_is_uncertain() { + let ordinary = + coordinator_with_empty_historical_agent_tickets("layout-unlock-missing-slot"); + let plan = ordinary + .layout_recovery_status_with_timeout( + Duration::from_secs(1), + &CancellationToken::default(), + ) + .plan_sha256 + .expect("plan"); + fs::remove_file(ordinary.root().join(SLOT_LOCK)).expect("replace slot lock"); + let before = tree_fingerprint(ordinary.root()); + let coordinator = + AdmissionCoordinator::test_at_with_unlock_fault(ordinary.root().to_path_buf(), 1); + let result = coordinator.apply_layout_recovery_with_timeout( + &plan, + Duration::from_secs(1), + &CancellationToken::default(), + ); + assert_eq!( + result.outcome, + AdmissionLayoutRecoveryOutcomeV1::RecoveryUncertain + ); + assert_eq!( + result.reason, + AdmissionLayoutRecoveryReasonV1::FilesystemUncertain + ); + assert_eq!(before, tree_fingerprint(ordinary.root())); + } + #[test] fn layout_recovery_serialization_privacy_and_schema_are_bounded() { let statuses = [