From 8e5e30eb1ba8426bb7df9bae105d544a6e685f09 Mon Sep 17 00:00:00 2001 From: Daniel Vianna <1708810+pasunboneleve@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:13:36 +1000 Subject: [PATCH] Add transactional artifact generations Context: Destructive site builds could empty a directory while a long-running server still held the old asset manifest. A status-only readiness probe then accepted a server that returned transient 404s for its assets. Decision: Add one high-level publish_artifact workflow action backed by named artifact declarations. Devloop now builds in an isolated candidate, injects absolute generation paths, switches non-autostart consumers, requires exact-generation HTTP readiness, rolls back failed or interrupted switches, ignores engine-owned output in the watcher, and retains bounded history. Embed the complete agent contract in devloop docs artifacts and the durable documentation. Alternatives considered: Separate prepare, promote, restart, and cleanup workflow actions were rejected because they expose invalid orderings and make partial publication easy to configure. A project-only shell solution was rejected because every directory-serving HTTP project needs the same lifecycle invariant. Tradeoffs: Consumers must expose a small generation endpoint and read the generated directory and generation from environment variables. Artifact names and consumer startup are deliberately constrained so filesystem containment and crash recovery remain enforceable. Architectural impact: Configuration gains artifact declarations and exact-body HTTP probes. The workflow core emits one publication effect; the process boundary owns filesystem and process switching. Session state records active and rollback generations, and runtime watch filtering excludes state and artifact storage. SemVer: MINOR. This ordinary feature commit updates [Unreleased], does not bump Cargo.toml, and creates no dated release section. Validation: cargo test cargo clippy --all-targets --all-features -- -D warnings cargo fmt --all -- --check git diff --check Roborev jobs 256 and 257 reviewed the dirty tree; all findings were resolved locally under the two-pass cap. Kata: 3tvx --- CHANGELOG.md | 10 + README.md | 6 + docs/README.md | 1 + docs/artifacts.md | 107 ++++++++ docs/behavior.md | 20 ++ docs/configuration.md | 41 +++ src/browser_reload.rs | 1 + src/config.rs | 188 +++++++++++++- src/core.rs | 42 +++ src/engine.rs | 94 ++++++- src/external_events.rs | 1 + src/main.rs | 12 + src/processes.rs | 561 ++++++++++++++++++++++++++++++++++++++++- src/state.rs | 10 + 14 files changed, 1087 insertions(+), 7 deletions(-) create mode 100644 docs/artifacts.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 043e304..4775dbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,16 @@ All notable changes to `devloop` will be recorded in this file. ## [Unreleased] +### Added + +- Added transactional artifact generations through the single + `publish_artifact` workflow action. Devloop now builds in isolated candidate + directories, switches declared consumers only after build success, requires + exact-generation HTTP readiness, rolls back failed switches, cleans crash + remnants, and bounds retained generations. +- Added `devloop docs artifacts` with an agent-oriented configuration contract + and complete lifecycle guidance. + ## [0.10.5] - 2026-09-02 ### Fixed diff --git a/README.md b/README.md index b71ffdb..db95ae9 100644 --- a/README.md +++ b/README.md @@ -104,6 +104,7 @@ Built-in reference docs are also available from the CLI: ```bash devloop docs config devloop docs behavior +devloop docs artifacts devloop docs development devloop docs security ``` @@ -197,6 +198,11 @@ For the runtime behavior reference, see For the full configuration reference, see [`docs/configuration.md`](docs/configuration.md). +For servers that consume directories replaced by a build, see +[`docs/artifacts.md`](docs/artifacts.md). It defines the agent-safe +`publish_artifact` workflow, exact-generation readiness, rollback, and bounded +retention. + For local contributor workflow details, including the opt-in watch flake smoke test, see [`docs/development.md`](docs/development.md). diff --git a/docs/README.md b/docs/README.md index cc8c733..b1e9aaf 100644 --- a/docs/README.md +++ b/docs/README.md @@ -2,6 +2,7 @@ - [Behavior Reference](behavior.md) - [Configuration Reference](configuration.md) +- [Transactional Artifact Generations](artifacts.md) - [Development Guide](development.md) - [Security Notes](security.md) diff --git a/docs/artifacts.md b/docs/artifacts.md new file mode 100644 index 0000000..ed3f490 --- /dev/null +++ b/docs/artifacts.md @@ -0,0 +1,107 @@ +# Transactional Artifact Generations + +Use artifact generations when a build command replaces a directory that a +long-running HTTP server is already serving. A status-only readiness probe can +observe the server while its asset manifest still points at deleted files. +`publish_artifact` prevents that mixed state. + +This guide is also available in the CLI: + +```bash +devloop docs artifacts +``` + +## Agent rule + +If a rebuild deletes or replaces served output, configure an artifact. Do not +teach a workflow to delete the live directory, restart the server, or poll `/` +with a status-only probe. Devloop must own the candidate directory, generation +switch, exact readiness check, rollback, and retention. + +## Complete configuration + +```toml +root = "." +startup_workflows = ["build_site"] + +[watch.site] +paths = ["src/**", "public/"] +workflow = "build_site" + +[hook.build_site] +command = ["./scripts/build-site.sh"] + +[process.site] +command = ["./scripts/serve-site.sh"] +autostart = false + +[process.site.readiness] +kind = "http" +url = "http://127.0.0.1:8787/__devloop_generation" +expect_body = "{{ artifact.site.generation }}" +interval_ms = 250 +timeout_ms = 30000 + +[artifact.site] +build_hook = "build_site" +consumers = ["site"] +retain = 2 + +[workflow.build_site] +steps = [{ action = "publish_artifact", artifact = "site" }] +triggers = ["browser_reload"] + +[workflow.browser_reload] +steps = [{ action = "notify_reload" }] +``` + +The build hook writes only to `DEVLOOP_ARTIFACT_CANDIDATE`. The consumer reads +`DEVLOOP_ARTIFACT_SITE_DIR` and `DEVLOOP_ARTIFACT_SITE_GENERATION` from its +environment, serves that directory, and returns the generation value as the +complete response body of `/__devloop_generation`. Artifact names are converted +to uppercase environment components; non-alphanumeric characters become `_`. +Artifact names must start with a lowercase letter and contain only lowercase +letters, digits, and underscores. + +Set artifact consumers to `autostart = false` and put the publication workflow +in `startup_workflows`. The publish action starts each stopped consumer after a +successful initial build. + +## Guarantees + +For each publication, devloop: + +1. removes incomplete candidate directories left by an interrupted build +2. creates a private candidate directory +3. runs the build hook with candidate and generation environment variables +4. preserves the live process and active generation if the build fails +5. makes the completed directory immutable by generation name +6. switches session state and restarts the declared consumers +7. accepts readiness only when the response body matches the expected generation +8. restores and restarts the previous generation if switching fails +9. removes old generations beyond `retain` +10. runs downstream triggers, including browser reload, only after success + +The workflow API deliberately exposes only `publish_artifact`. Partial +`prepare_artifact` or `promote_artifact` steps do not exist because their order +would be easy to misconfigure. + +Once consumers pass exact-generation readiness, the switch is committed. +Failure to remove an older retained directory is logged but cannot turn that +successful switch into a failed workflow. An interrupted, unverified switch is +marked in session state and conservatively restored on the next publication. + +## Environment + +During the build hook: + +- `DEVLOOP_ARTIFACT`: configured artifact name +- `DEVLOOP_ARTIFACT_GENERATION`: candidate generation identifier +- `DEVLOOP_ARTIFACT_CANDIDATE`: absolute candidate directory +- `DEVLOOP_ARTIFACT__GENERATION`: same candidate identifier +- `DEVLOOP_ARTIFACT__DIR`: same candidate directory + +After promotion, every hook and managed process receives the named `DIR` and +`GENERATION` variables for every active artifact. Project code remains +responsible only for writing to the supplied directory, serving the supplied +directory, and returning the supplied generation from its readiness endpoint. diff --git a/docs/behavior.md b/docs/behavior.md index 3aff514..2eb8656 100644 --- a/docs/behavior.md +++ b/docs/behavior.md @@ -100,12 +100,32 @@ Workflows run step by step, in order. emitting output. - `notify_reload` broadcasts a generic `reload` event to browser listeners connected to `devloop`'s browser reload event stream. +- `publish_artifact` is one atomic workflow effect. The engine does not expose + partial preparation or promotion steps. If any step fails, that workflow fails immediately and logs the error loudly, but `devloop` itself keeps running so later file changes or external events can retry the workflow without restarting the supervisor. +## Artifact publication + +Artifact publication isolates destructive builds from live consumers. A build +failure deletes only its private candidate. A successful build becomes a named +generation, then devloop changes the active session state and restarts the +declared consumers. HTTP readiness succeeds only when its response body equals +the active generation. A mismatch or consumer failure restores the previous +state and process generation before the workflow reports failure. + +Interrupted candidates are removed at the next publication. Successful +generations are retained newest-first according to the artifact's `retain` +limit. Cleanup failure after a ready switch is logged without failing the +already committed publication. Browser reload belongs in a triggered workflow, so clients are notified +only after exact-generation readiness succeeds. + +See [Transactional Artifact Generations](artifacts.md) for the agent-facing +configuration and environment contract. + ## Processes Managed processes are long-running child commands. diff --git a/docs/configuration.md b/docs/configuration.md index 61b314a..8ab0e21 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -223,8 +223,13 @@ kind = "http" url = "http://127.0.0.1:$CONTAINER_PORT/" interval_ms = 500 timeout_ms = 30000 +expect_body = "{{ artifact.site.generation }}" ``` +- `expect_body`: optional exact response-body match after trimming the response. + Templates read current session state. Artifact consumers must match their + `artifact..generation`; a status-only probe is rejected for them. + ### State-key probe ```toml @@ -275,6 +280,31 @@ Hooks default to dimmed inherited output because they are typically short-lived helper commands whose output is useful context but not the primary long-running log stream. +## Artifacts + +Artifacts describe build outputs that long-running processes consume. + +```toml +[artifact.site] +build_hook = "build_site" +consumers = ["site"] +retain = 2 +``` + +- `build_hook`: hook that writes to `DEVLOOP_ARTIFACT_CANDIDATE`. +- `consumers`: managed processes restarted against a completed generation. +- `retain`: total successful generation directories to keep. Default: `2`; + must be greater than zero. + +Artifact table names must start with a lowercase letter and contain only +lowercase letters, digits, and underscores. + +Each consumer must have HTTP readiness whose `expect_body` is exactly +`{{ artifact..generation }}`. Use the artifact only through +`publish_artifact`; lifecycle fragments are intentionally not configurable. +See [Transactional Artifact Generations](artifacts.md) for the complete +contract and environment variables. + ### Observed hooks ```toml @@ -422,6 +452,7 @@ would make ordering and duplication ambiguous. - `restart_process` - `wait_for_process` - `run_hook` +- `publish_artifact` - `run_workflow` - `sleep_ms` - `write_state` @@ -442,6 +473,16 @@ would make ordering and duplication ambiguous. `value` supports `{{state_key}}` interpolation from the current session state. +### `publish_artifact` + +```toml +{ action = "publish_artifact", artifact = "site" } +``` + +Builds a private candidate, switches all declared consumers, verifies that +they serve the selected generation, rolls back a failed switch, and cleans old +generations. Downstream triggers run only after publication succeeds. + ### `log` ```toml diff --git a/src/browser_reload.rs b/src/browser_reload.rs index 1aa6202..0fa78cf 100644 --- a/src/browser_reload.rs +++ b/src/browser_reload.rs @@ -141,6 +141,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: EventServerConfig::default(), browser_reload_server: BrowserReloadServerConfig::default(), event: BTreeMap::new(), diff --git a/src/config.rs b/src/config.rs index c987acd..ef06d1b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -25,6 +25,8 @@ pub struct Config { #[serde(default)] pub hook: BTreeMap, #[serde(default)] + pub artifact: BTreeMap, + #[serde(default)] pub event_server: EventServerConfig, #[serde(default)] pub browser_reload_server: BrowserReloadServerConfig, @@ -36,7 +38,14 @@ pub struct Config { impl Config { pub fn load(path: &Path) -> Result { - let raw = std::fs::read_to_string(path) + let path = if path.is_absolute() { + path.to_path_buf() + } else { + std::env::current_dir() + .context("failed to resolve current directory for config path")? + .join(path) + }; + let raw = std::fs::read_to_string(&path) .with_context(|| format!("failed to read config at {}", path.display()))?; let mut config: Config = toml::from_str(&raw) .with_context(|| format!("failed to parse config at {}", path.display()))?; @@ -92,6 +101,11 @@ impl Config { )); } } + for (name, artifact) in &self.artifact { + artifact + .validate(self, name) + .with_context(|| format!("invalid artifact '{name}'"))?; + } self.event_server.validate()?; self.browser_reload_server.validate()?; self.watcher.validate()?; @@ -379,6 +393,8 @@ impl ProcessSpec { pub enum ProbeSpec { Http { url: String, + /// Optional exact response body required for a successful probe. + expect_body: Option, #[serde(default = "default_interval_ms")] interval_ms: u64, #[serde(default = "default_timeout_ms")] @@ -508,6 +524,72 @@ pub struct HookSpec { pub observe: Option, } +/// One independently built output whose consumers must switch generations atomically. +#[derive(Debug, Clone, Deserialize)] +pub struct ArtifactSpec { + pub build_hook: String, + pub consumers: Vec, + #[serde(default = "default_artifact_retention")] + pub retain: usize, +} + +impl ArtifactSpec { + fn validate(&self, config: &Config, artifact_name: &str) -> Result<()> { + if artifact_name.is_empty() + || !artifact_name + .chars() + .enumerate() + .all(|(index, character)| match index { + 0 => character.is_ascii_lowercase(), + _ => { + character.is_ascii_lowercase() + || character.is_ascii_digit() + || character == '_' + } + }) + { + return Err(anyhow!( + "artifact name must start with a lowercase letter and contain only lowercase letters, digits, and underscores" + )); + } + if !config.hook.contains_key(&self.build_hook) { + return Err(anyhow!( + "artifact references missing build hook '{}'", + self.build_hook + )); + } + if self.consumers.is_empty() { + return Err(anyhow!("artifact must define at least one consumer")); + } + if self.retain == 0 { + return Err(anyhow!("artifact retain must be greater than zero")); + } + let expected = format!("{{{{ artifact.{artifact_name}.generation }}}}"); + for consumer in &self.consumers { + let process = config.process.get(consumer).ok_or_else(|| { + anyhow!("artifact references missing consumer process '{consumer}'") + })?; + if process.autostart { + return Err(anyhow!( + "artifact consumer process '{consumer}' must set autostart = false so interrupted switches recover before process start" + )); + } + match process.readiness.as_ref() { + Some(ProbeSpec::Http { + expect_body: Some(body), + .. + }) if body == &expected => {} + _ => { + return Err(anyhow!( + "consumer process '{consumer}' must use exact generation readiness with expect_body = '{expected}'" + )); + } + } + } + Ok(()) + } +} + #[derive(Debug, Clone, Deserialize)] pub struct EventServerConfig { #[serde(default = "default_event_server_bind")] @@ -687,6 +769,11 @@ impl WorkflowSpec { return Err(anyhow!("workflow references missing hook '{hook}'")); } } + WorkflowStep::PublishArtifact { artifact } => { + if !config.artifact.contains_key(artifact) { + return Err(anyhow!("workflow references missing artifact '{artifact}'")); + } + } WorkflowStep::RunWorkflow { workflow } => { validate_nested_workflow(config, stack, workflow)?; } @@ -804,6 +891,9 @@ pub enum WorkflowStep { RunHook { hook: String, }, + PublishArtifact { + artifact: String, + }, RunWorkflow { workflow: String, }, @@ -877,6 +967,10 @@ fn default_observe_interval_ms() -> u64 { 1_000 } +fn default_artifact_retention() -> usize { + 2 +} + fn default_event_server_bind() -> String { "127.0.0.1:0".to_string() } @@ -893,6 +987,35 @@ fn normalize_path_buf(path: PathBuf) -> PathBuf { mod tests { use super::*; + const ARTIFACT_CONFIG: &str = r#" +root = "." +startup_workflows = ["build_site"] + +[watch.site] +paths = ["src/**"] +workflow = "build_site" + +[hook.build_site] +command = ["sh", "-c", "mkdir -p \"$DEVLOOP_ARTIFACT_CANDIDATE\" && printf built > \"$DEVLOOP_ARTIFACT_CANDIDATE/index.html\""] + +[process.site] +command = ["serve-site"] +autostart = false +readiness = { kind = "http", url = "http://127.0.0.1:8787/__devloop_generation", expect_body = "{{ artifact.site.generation }}" } + +[artifact.site] +build_hook = "build_site" +consumers = ["site"] +retain = 2 + +[workflow.build_site] +steps = [{ action = "publish_artifact", artifact = "site" }] +triggers = ["browser_reload"] + +[workflow.browser_reload] +steps = [{ action = "notify_reload" }] +"#; + fn base_config() -> Config { Config { root: PathBuf::from("."), @@ -903,6 +1026,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: EventServerConfig::default(), browser_reload_server: BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -910,6 +1034,68 @@ mod tests { } } + #[test] + fn accepts_atomic_artifact_publication_contract() { + let config: Config = toml::from_str(ARTIFACT_CONFIG).expect("parse artifact config"); + + config + .validate() + .expect("complete artifact publication contract should validate"); + } + + #[test] + fn rejects_artifact_consumer_without_exact_generation_readiness() { + let raw = ARTIFACT_CONFIG.replace( + "readiness = { kind = \"http\", url = \"http://127.0.0.1:8787/__devloop_generation\", expect_body = \"{{ artifact.site.generation }}\" }", + "readiness = { kind = \"http\", url = \"http://127.0.0.1:8787/\" }", + ); + let config: Config = toml::from_str(&raw).expect("parse artifact config"); + + let error = config + .validate() + .expect_err("status-only readiness must not guard artifact promotion"); + assert!( + format!("{error:#}").contains("exact generation readiness"), + "unexpected validation error: {error:#}" + ); + } + + #[test] + fn rejects_exposed_artifact_lifecycle_steps() { + let raw = ARTIFACT_CONFIG.replace( + "{ action = \"publish_artifact\", artifact = \"site\" }", + "{ action = \"prepare_artifact\", artifact = \"site\" }, { action = \"promote_artifact\", artifact = \"site\" }", + ); + + let error = toml::from_str::(&raw) + .expect_err("partial lifecycle actions must not be part of the public API"); + assert!(error.to_string().contains("unknown variant")); + } + + #[test] + fn rejects_artifact_names_that_are_not_safe_path_components() { + let raw = ARTIFACT_CONFIG.replace("[artifact.site]", "[artifact.'..']"); + let config: Config = toml::from_str(&raw).expect("parse unsafe artifact config"); + + let error = config + .validate() + .expect_err("unsafe artifact name must fail validation"); + + assert!(format!("{error:#}").contains("artifact name must start")); + } + + #[test] + fn rejects_autostart_artifact_consumers() { + let raw = ARTIFACT_CONFIG.replace("autostart = false", "autostart = true"); + let config: Config = toml::from_str(&raw).expect("parse autostart artifact config"); + + let error = config + .validate() + .expect_err("artifact consumer must not start before recovery"); + + assert!(format!("{error:#}").contains("must set autostart = false")); + } + #[test] fn validate_rejects_recursive_workflows() { let mut config = base_config(); diff --git a/src/core.rs b/src/core.rs index b6098b7..083bb0f 100644 --- a/src/core.rs +++ b/src/core.rs @@ -40,6 +40,11 @@ pub enum WorkflowEffect { workflow_name: String, changed_files: Vec, }, + PublishArtifact { + artifact: String, + workflow_name: String, + changed_files: Vec, + }, SleepMs { duration_ms: u64, }, @@ -241,6 +246,13 @@ impl WorkflowMachine { changed_files, })); } + WorkflowStep::PublishArtifact { artifact } => { + return Ok(Some(WorkflowEffect::PublishArtifact { + artifact, + workflow_name, + changed_files, + })); + } WorkflowStep::RunWorkflow { workflow } => { if self .stack @@ -628,6 +640,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -675,6 +688,35 @@ mod tests { assert_eq!(machine.next_effect(&config).expect("effect"), None); } + #[test] + fn machine_exposes_artifact_publication_as_one_effect() { + let mut config = base_config(); + config.workflow.insert( + "site".into(), + WorkflowSpec { + steps: vec![WorkflowStep::PublishArtifact { + artifact: "site".into(), + }], + triggers: vec![], + }, + ); + let changed_files = vec!["src/page.ts".into()]; + let mut machine = WorkflowMachine::start(&config, Map::new(), "site", &changed_files) + .expect("start machine"); + let _ = machine.next_effect(&config).expect("workflow state effect"); + let _ = machine.next_effect(&config).expect("changed files effect"); + + assert_eq!( + machine.next_effect(&config).expect("publication effect"), + Some(WorkflowEffect::PublishArtifact { + artifact: "site".into(), + workflow_name: "site".into(), + changed_files, + }) + ); + assert_eq!(machine.next_effect(&config).expect("complete"), None); + } + #[test] fn machine_renders_write_state_from_session_snapshot() { let mut config = base_config(); diff --git a/src/engine.rs b/src/engine.rs index 52738e7..001906d 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -44,6 +44,12 @@ trait WorkflowEffectAdapter { changed_files: &[String], workflow_name: &str, ) -> Result<()>; + async fn publish_artifact( + &mut self, + artifact: &str, + changed_files: &[String], + workflow_name: &str, + ) -> Result<()>; async fn notify_reload(&mut self) -> Result<()>; async fn sleep_ms(&mut self, duration_ms: u64) -> Result<()>; async fn persist_state(&mut self, key: String, value: Value) -> Result<()>; @@ -113,7 +119,16 @@ impl Engine { .with_session_log(self.session_log.clone()); let watch_groups = self.config.compiled_watchers()?; let watched_targets = self.config.compiled_watch_targets(); - let ignored_watch_paths = vec![self.session_log.path().to_path_buf()]; + let state_path = state.path().to_path_buf(); + let artifact_root = state_path + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("artifacts"); + let ignored_watch_paths = vec![ + self.session_log.path().to_path_buf(), + state_path, + artifact_root, + ]; let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); let (external_event_tx, mut external_event_rx) = tokio::sync::mpsc::unbounded_channel(); let tx_watcher = tx.clone(); @@ -268,6 +283,17 @@ impl WorkflowEffectAdapter for LiveWorkflowAdapter<'_, '_> { .await } + async fn publish_artifact( + &mut self, + artifact: &str, + changed_files: &[String], + workflow_name: &str, + ) -> Result<()> { + self.processes + .publish_artifact(artifact, self.state, changed_files, workflow_name) + .await + } + async fn notify_reload(&mut self) -> Result<()> { if let Some(sender) = &self.browser_reload_sender { notify_browser_reload(sender); @@ -609,6 +635,15 @@ async fn execute_workflow_effect( .run_hook(&hook, &changed_files, &workflow_name) .await } + WorkflowEffect::PublishArtifact { + artifact, + workflow_name, + changed_files, + } => { + adapter + .publish_artifact(&artifact, &changed_files, &workflow_name) + .await + } WorkflowEffect::NotifyReload => adapter.notify_reload().await, WorkflowEffect::SleepMs { duration_ms } => adapter.sleep_ms(duration_ms).await, WorkflowEffect::PersistState { key, value } => adapter.persist_state(key, value).await, @@ -808,16 +843,16 @@ fn relativize_event_path<'a>(root: &'a Path, path: &'a Path) -> Option<&'a Path> } fn path_is_equivalent_to_ignored_path(path: &Path, ignored_path: &Path) -> bool { - if path == ignored_path { + if path == ignored_path || path.starts_with(ignored_path) { return true; } if let Some(private_path) = private_path_variant(ignored_path) - && path == private_path + && (path == private_path || path.starts_with(&private_path)) { return true; } if let Some(public_path) = public_path_variant(ignored_path) - && path == public_path + && (path == public_path || path.starts_with(&public_path)) { return true; } @@ -954,6 +989,29 @@ mod tests { assert!(grouped.is_empty()); } + #[test] + fn classify_events_ignore_engine_owned_state_and_artifact_tree() { + let root = PathBuf::from("/tmp/example"); + let groups = vec![CompiledWatchGroup::for_test(&["**/*"], "all").expect("watch group")]; + let ignored_paths = vec![ + root.join(".devloop/state.json"), + root.join(".devloop/artifacts"), + ]; + let events = vec![Event { + kind: EventKind::Modify(ModifyKind::Any), + paths: vec![ + root.join(".devloop/state.json"), + root.join(".devloop/artifacts/site/123/index.html"), + root.join("src/page.ts"), + ], + attrs: Default::default(), + }]; + + let grouped = classify_events(&root, &groups, &events, &ignored_paths); + + assert_eq!(grouped["all"], vec!["src/page.ts"]); + } + #[test] fn resolve_watch_registration_uses_parent_for_existing_poll_file() { let dir = tempdir().expect("tempdir"); @@ -1279,6 +1337,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1338,6 +1397,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1410,6 +1470,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1507,6 +1568,19 @@ mod tests { Ok(()) } + async fn publish_artifact( + &mut self, + artifact: &str, + changed_files: &[String], + workflow_name: &str, + ) -> Result<()> { + self.calls.push(format!( + "artifact:{artifact}:{workflow_name}:{}", + changed_files.join(",") + )); + Ok(()) + } + async fn notify_reload(&mut self) -> Result<()> { self.calls.push("notify_reload".into()); if self.fail_reload { @@ -1547,6 +1621,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1605,6 +1680,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1656,6 +1732,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1734,6 +1811,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1794,6 +1872,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -1945,6 +2024,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -2030,6 +2110,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -2106,6 +2187,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::from([( @@ -2172,6 +2254,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::from([( @@ -2224,6 +2307,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -2259,6 +2343,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -2295,6 +2380,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), diff --git a/src/external_events.rs b/src/external_events.rs index 5b8f54e..8cc8cf2 100644 --- a/src/external_events.rs +++ b/src/external_events.rs @@ -246,6 +246,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), diff --git a/src/main.rs b/src/main.rs index 338e86e..68b97ca 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,6 +73,7 @@ enum Command { enum DocsTopic { Config, Behavior, + Artifacts, Development, Security, } @@ -325,6 +326,7 @@ fn docs_text(topic: DocsTopic) -> &'static str { match topic { DocsTopic::Config => include_str!("../docs/configuration.md"), DocsTopic::Behavior => include_str!("../docs/behavior.md"), + DocsTopic::Artifacts => include_str!("../docs/artifacts.md"), DocsTopic::Development => include_str!("../docs/development.md"), DocsTopic::Security => include_str!("../docs/security.md"), } @@ -662,6 +664,16 @@ mod tests { assert!(rendered.contains("DEVLOOP_RUN_WATCH_FLAKE_SMOKE")); } + #[test] + fn docs_text_exposes_agent_safe_artifact_workflow() { + let rendered = docs_text(DocsTopic::Artifacts); + + assert!(rendered.starts_with("# Transactional Artifact Generations")); + assert!(rendered.contains("## Agent rule")); + assert!(rendered.contains("publish_artifact")); + assert!(rendered.contains("DEVLOOP_ARTIFACT_CANDIDATE")); + } + #[test] fn rendered_docs_drop_markdown_heading_markers() { let rendered = render_docs_text(DocsTopic::Config); diff --git a/src/processes.rs b/src/processes.rs index 67a80c2..0cffa42 100644 --- a/src/processes.rs +++ b/src/processes.rs @@ -7,11 +7,13 @@ use std::path::{Path, PathBuf}; use std::process::Stdio; use std::sync::{Arc, Mutex as StdMutex}; use std::time::{Duration, Instant}; +use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context, Result, anyhow}; use regex::Regex; use rustix::io::Errno; use rustix::process::{Pid, Signal, kill_process_group}; +use serde_json::Value; use tokio::io::{AsyncReadExt, AsyncWriteExt, Stderr, Stdout}; use tokio::process::{Child, Command}; use tokio::sync::{Mutex, mpsc}; @@ -313,17 +315,32 @@ impl<'a> ProcessManager<'a> { state: &SessionState, changed_files: &[String], workflow: &str, + ) -> Result<()> { + self.run_hook_with_env(name, state, changed_files, workflow, &BTreeMap::new()) + .await + } + + async fn run_hook_with_env( + &self, + name: &str, + state: &SessionState, + changed_files: &[String], + workflow: &str, + extra_env: &BTreeMap, ) -> Result<()> { let spec = self .config .hook .get(name) .ok_or_else(|| anyhow!("unknown hook '{name}'"))?; + let mut hook_env = spec.env.clone(); + hook_env.extend(active_artifact_env(self.config, state)?); + hook_env.extend(extra_env.clone()); let command = configure_command( &spec.command, resolve_cwd(&self.config.root, spec.cwd.as_deref()), CommandContext { - env: &spec.env, + env: &hook_env, external_event_env: self.external_event_env.as_ref(), browser_reload_env: self.browser_reload_env.as_ref(), root: &self.config.root, @@ -380,6 +397,165 @@ impl<'a> ProcessManager<'a> { apply_hook_capture(spec, stdout.trim(), state) } + /// Builds and switches one artifact generation as a single recoverable operation. + pub async fn publish_artifact( + &mut self, + name: &str, + state: &SessionState, + changed_files: &[String], + workflow: &str, + ) -> Result<()> { + let spec = self + .config + .artifact + .get(name) + .cloned() + .ok_or_else(|| anyhow!("unknown artifact '{name}'"))?; + let artifact_root = state + .path() + .parent() + .unwrap_or_else(|| Path::new(".")) + .join("artifacts") + .join(name); + std::fs::create_dir_all(&artifact_root).with_context(|| { + format!( + "failed to create artifact directory {}", + artifact_root.display() + ) + })?; + let artifact_root = std::fs::canonicalize(&artifact_root).with_context(|| { + format!( + "failed to resolve absolute artifact directory {}", + artifact_root.display() + ) + })?; + recover_interrupted_artifact(name, &artifact_root, state)?; + remove_candidate_directories(&artifact_root)?; + + let generation = new_artifact_generation()?; + let candidate = artifact_root.join(format!(".candidate-{generation}")); + let published = artifact_root.join(&generation); + std::fs::create_dir(&candidate).with_context(|| { + format!( + "failed to create artifact candidate {}", + candidate.display() + ) + })?; + + let env_name = artifact_env_name(name); + let build_env = BTreeMap::from([ + ("DEVLOOP_ARTIFACT".into(), name.to_owned()), + ("DEVLOOP_ARTIFACT_GENERATION".into(), generation.clone()), + ( + "DEVLOOP_ARTIFACT_CANDIDATE".into(), + candidate.to_string_lossy().into_owned(), + ), + ( + format!("DEVLOOP_ARTIFACT_{env_name}_GENERATION"), + generation.clone(), + ), + ( + format!("DEVLOOP_ARTIFACT_{env_name}_DIR"), + candidate.to_string_lossy().into_owned(), + ), + ]); + if let Err(error) = self + .run_hook_with_env(&spec.build_hook, state, changed_files, workflow, &build_env) + .await + { + let _ = std::fs::remove_dir_all(&candidate); + return Err(error.context(format!( + "failed to build candidate generation for artifact '{name}'" + ))); + } + std::fs::rename(&candidate, &published).with_context(|| { + format!( + "failed to publish artifact candidate {} as {}", + candidate.display(), + published.display() + ) + })?; + + let generation_key = artifact_generation_key(name); + let path_key = artifact_path_key(name); + let switching_key = artifact_switching_key(name); + let rollback_generation_key = artifact_rollback_generation_key(name); + let rollback_path_key = artifact_rollback_path_key(name); + let previous_generation = state.get_string(&generation_key)?; + let previous_path = state.get_string(&path_key)?; + state.merge_json_object(serde_json::Map::from_iter([ + (generation_key.clone(), Value::String(generation.clone())), + ( + path_key.clone(), + Value::String(published.to_string_lossy().into_owned()), + ), + (switching_key, Value::String(generation.clone())), + ( + rollback_generation_key, + Value::String(previous_generation.clone().unwrap_or_default()), + ), + ( + rollback_path_key, + Value::String(previous_path.clone().unwrap_or_default()), + ), + ]))?; + + let switch_result = async { + for consumer in &spec.consumers { + self.restart_named(consumer, state).await?; + } + for consumer in &spec.consumers { + self.wait_for_named(consumer, state, false).await?; + } + Ok::<(), anyhow::Error>(()) + } + .await; + + if let Err(error) = switch_result { + restore_state_value(state, &generation_key, previous_generation.as_deref())?; + restore_state_value(state, &path_key, previous_path.as_deref())?; + let mut rollback_failures = Vec::new(); + for consumer in &spec.consumers { + let rollback = if previous_generation.is_some() { + async { + self.restart_named(consumer, state).await?; + self.wait_for_named(consumer, state, false).await + } + .await + } else { + self.stop_named(consumer, state).await + }; + if let Err(rollback_error) = rollback { + rollback_failures.push(format!("{consumer}: {rollback_error:#}")); + } + } + if !rollback_failures.is_empty() { + return Err(error.context(format!( + "artifact '{name}' generation {generation} did not become ready; rollback failed and the rejected generation was preserved for recovery: {}", + rollback_failures.join("; ") + ))); + } + clear_artifact_switch(state, name)?; + if let Err(cleanup_error) = std::fs::remove_dir_all(&published) { + warn!( + "artifact {name} rolled back, but rejected generation cleanup failed: {cleanup_error}" + ); + } + return Err(error.context(format!( + "artifact '{name}' generation {generation} did not become ready; restored previous generation" + ))); + } + + clear_artifact_switch(state, name)?; + if let Err(error) = retain_artifact_generations(&artifact_root, &generation, spec.retain) { + warn!( + "artifact {name} generation {generation} is active, but old-generation cleanup failed: {error:#}" + ); + } + info!("published artifact {name} generation {generation}"); + Ok(()) + } + pub async fn run_observed_hook( &self, name: &str, @@ -532,11 +708,13 @@ impl<'a> ProcessManager<'a> { } let output_generation = self.next_output_state_generation(name, &spec.output.rules, state)?; + let mut process_env = spec.env.clone(); + process_env.extend(active_artifact_env(self.config, state)?); let command = configure_command( &spec.command, resolve_cwd(&self.config.root, spec.cwd.as_deref()), CommandContext { - env: &spec.env, + env: &process_env, external_event_env: self.external_event_env.as_ref(), browser_reload_env: self.browser_reload_env.as_ref(), root: &self.config.root, @@ -1657,6 +1835,164 @@ fn configure_command( Ok(cmd) } +fn artifact_generation_key(name: &str) -> String { + format!("artifact.{name}.generation") +} + +fn artifact_path_key(name: &str) -> String { + format!("artifact.{name}.path") +} + +fn artifact_switching_key(name: &str) -> String { + format!("artifact.{name}.switching_generation") +} + +fn artifact_rollback_generation_key(name: &str) -> String { + format!("artifact.{name}.rollback_generation") +} + +fn artifact_rollback_path_key(name: &str) -> String { + format!("artifact.{name}.rollback_path") +} + +fn validate_generation_component(generation: &str) -> Result<()> { + if generation.is_empty() + || !generation + .chars() + .all(|character| character.is_ascii_alphanumeric() || character == '-') + { + return Err(anyhow!("invalid persisted artifact generation identifier")); + } + Ok(()) +} + +fn artifact_env_name(name: &str) -> String { + name.chars() + .map(|character| { + if character.is_ascii_alphanumeric() { + character.to_ascii_uppercase() + } else { + '_' + } + }) + .collect() +} + +fn active_artifact_env(config: &Config, state: &SessionState) -> Result> { + let mut env = BTreeMap::new(); + for name in config.artifact.keys() { + let Some(generation) = state.get_string(&artifact_generation_key(name))? else { + continue; + }; + let Some(path) = state.get_string(&artifact_path_key(name))? else { + continue; + }; + let name = artifact_env_name(name); + env.insert(format!("DEVLOOP_ARTIFACT_{name}_GENERATION"), generation); + env.insert(format!("DEVLOOP_ARTIFACT_{name}_DIR"), path); + } + Ok(env) +} + +fn new_artifact_generation() -> Result { + let millis = SystemTime::now() + .duration_since(UNIX_EPOCH) + .context("system clock is before the Unix epoch")? + .as_millis(); + Ok(format!("{millis}-{:08x}", rand::random::())) +} + +fn restore_state_value(state: &SessionState, key: &str, value: Option<&str>) -> Result<()> { + match value { + Some(value) => state.set(key, Value::String(value.to_owned())), + None => state.remove(key), + } +} + +fn clear_artifact_switch(state: &SessionState, name: &str) -> Result<()> { + state.remove(&artifact_switching_key(name))?; + state.remove(&artifact_rollback_generation_key(name))?; + state.remove(&artifact_rollback_path_key(name)) +} + +fn recover_interrupted_artifact(name: &str, root: &Path, state: &SessionState) -> Result<()> { + let Some(interrupted) = state.get_string(&artifact_switching_key(name))? else { + return Ok(()); + }; + validate_generation_component(&interrupted)?; + let rollback_generation = state + .get_string(&artifact_rollback_generation_key(name))? + .filter(|value| !value.is_empty()); + let rollback_path = state + .get_string(&artifact_rollback_path_key(name))? + .filter(|value| !value.is_empty()); + restore_state_value( + state, + &artifact_generation_key(name), + rollback_generation.as_deref(), + )?; + restore_state_value(state, &artifact_path_key(name), rollback_path.as_deref())?; + clear_artifact_switch(state, name)?; + let interrupted_path = root.join(&interrupted); + if interrupted_path.exists() { + std::fs::remove_dir_all(&interrupted_path).with_context(|| { + format!( + "failed to remove interrupted artifact generation {}", + interrupted_path.display() + ) + })?; + } + Ok(()) +} + +fn remove_candidate_directories(root: &Path) -> Result<()> { + for entry in std::fs::read_dir(root) + .with_context(|| format!("failed to inspect artifact directory {}", root.display()))? + { + let entry = entry?; + if entry + .file_name() + .to_string_lossy() + .starts_with(".candidate-") + { + std::fs::remove_dir_all(entry.path()).with_context(|| { + format!( + "failed to remove stale candidate {}", + entry.path().display() + ) + })?; + } + } + Ok(()) +} + +fn retain_artifact_generations(root: &Path, active: &str, retain: usize) -> Result<()> { + let mut generations = std::fs::read_dir(root) + .with_context(|| format!("failed to inspect artifact directory {}", root.display()))? + .filter_map(|entry| entry.ok()) + .filter(|entry| entry.file_type().is_ok_and(|kind| kind.is_dir())) + .filter_map(|entry| { + let name = entry.file_name().to_string_lossy().into_owned(); + (!name.starts_with('.')).then_some((name, entry.path())) + }) + .collect::>(); + generations.sort_by(|left, right| right.0.cmp(&left.0)); + let mut kept_others = 0; + let other_limit = retain.saturating_sub(1); + for (generation, path) in generations { + if generation == active { + continue; + } + if kept_others < other_limit { + kept_others += 1; + continue; + } + std::fs::remove_dir_all(&path) + .with_context(|| format!("failed to remove old generation {}", path.display()))?; + } + Ok(()) +} + fn resolve_program(root: &Path, program: &str) -> PathBuf { let path = Path::new(program); if path.is_absolute() || path.components().count() == 1 { @@ -1806,10 +2142,12 @@ fn expand_probe_env(process: &str, probe: &ProbeSpec) -> Result { match probe { ProbeSpec::Http { url, + expect_body, interval_ms, timeout_ms, } => Ok(ProbeSpec::Http { url: env_expand::expand_value(url, &format!("process '{process}' http probe url"))?, + expect_body: expect_body.clone(), interval_ms: *interval_ms, timeout_ms: *timeout_ms, }), @@ -1834,6 +2172,7 @@ async fn check_probe( match probe { ProbeSpec::Http { url, + expect_body, interval_ms, timeout_ms, } => match client @@ -1843,6 +2182,21 @@ async fn check_probe( .await { Ok(response) if response.status().is_success() => { + if let Some(expected) = expect_body { + let expected = + crate::state::render_template_values(&state.snapshot()?, expected)?; + let actual = response + .text() + .await + .with_context(|| format!("failed to read probe body for '{name}'"))?; + if actual.trim() != expected { + return Err(anyhow!( + "probe for '{name}' at {url} returned generation body {:?}, expected {:?}", + actual.trim(), + expected + )); + } + } info!("process {} is healthy at {}", name, url); Ok(()) } @@ -1889,6 +2243,7 @@ mod tests { use crate::test_support::{EnvVarGuard, RustLogGuard}; use rustix::process::test_kill_process; use serde_json::Value; + use std::io::{Read, Write}; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -1907,6 +2262,24 @@ mod tests { std::env::temp_dir().join(format!("devloop-process-state-{unique}-{sequence}.json")) } + fn serve_probe_body(body: &'static str) -> String { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind probe server"); + let address = listener.local_addr().expect("probe address"); + std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept probe request"); + let mut request = [0_u8; 1024]; + let _ = stream.read(&mut request); + write!( + stream, + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), + body + ) + .expect("write probe response"); + }); + format!("http://{address}/generation") + } + fn test_config(root: &Path) -> Config { Config { root: root.to_path_buf(), @@ -1917,6 +2290,7 @@ mod tests { watch: BTreeMap::new(), process: BTreeMap::new(), hook: BTreeMap::new(), + artifact: BTreeMap::new(), event_server: crate::config::EventServerConfig::default(), browser_reload_server: crate::config::BrowserReloadServerConfig::default(), event: BTreeMap::new(), @@ -3582,6 +3956,188 @@ exec sleep 600 std::fs::remove_file(state_path).expect("cleanup state file"); } + #[tokio::test] + async fn http_probe_rejects_a_different_artifact_generation() { + let state_path = unique_state_path(); + let state = SessionState::load(state_path.clone()).expect("load state"); + state + .set("artifact.site.generation", Value::String("new".into())) + .expect("set generation"); + let probe = ProbeSpec::Http { + url: serve_probe_body("old"), + expect_body: Some("{{ artifact.site.generation }}".into()), + interval_ms: 100, + timeout_ms: 1000, + }; + + let error = check_probe(&reqwest::Client::new(), "site", &probe, &state) + .await + .expect_err("stale generation must not be ready"); + + assert!(error.to_string().contains("expected \"new\"")); + std::fs::remove_file(state_path).expect("cleanup state file"); + } + + #[tokio::test] + async fn failed_artifact_build_preserves_active_generation() { + use crate::config::{ArtifactSpec, HookOutputConfig, HookSpec}; + + let directory = tempdir().expect("artifact fixture"); + let state_path = directory.path().join("state.json"); + let state = SessionState::load(state_path).expect("load state"); + state + .merge_json_object(serde_json::Map::from_iter([ + ( + "artifact.site.generation".into(), + Value::String("old".into()), + ), + ( + "artifact.site.path".into(), + Value::String(directory.path().join("old").to_string_lossy().into_owned()), + ), + ])) + .expect("seed active generation"); + let mut config = test_config(directory.path()); + config.hook.insert( + "build".into(), + HookSpec { + command: vec!["sh".into(), "-c".into(), "exit 23".into()], + cwd: None, + env: BTreeMap::new(), + output: HookOutputConfig::default(), + capture: None, + state_key: None, + observe: None, + }, + ); + config.artifact.insert( + "site".into(), + ArtifactSpec { + build_hook: "build".into(), + consumers: vec!["unused".into()], + retain: 2, + }, + ); + let mut manager = + ProcessManager::new(&config, GuardianExecutable::open().expect("open guardian")); + + manager + .publish_artifact("site", &state, &[], "build") + .await + .expect_err("failed build must fail publication"); + + assert_eq!( + state + .get_string("artifact.site.generation") + .expect("read generation") + .as_deref(), + Some("old") + ); + let artifact_root = directory.path().join("artifacts/site"); + assert!( + std::fs::read_dir(artifact_root) + .expect("read artifact root") + .next() + .is_none(), + "failed candidate should be removed" + ); + } + + #[test] + fn cleanup_removes_stale_candidates_and_bounds_successful_generations() { + let directory = tempdir().expect("artifact fixture"); + for generation in ["100-a", "200-b", "300-c", ".candidate-crash"] { + std::fs::create_dir(directory.path().join(generation)).expect("create generation"); + } + + remove_candidate_directories(directory.path()).expect("remove stale candidate"); + retain_artifact_generations(directory.path(), "300-c", 2).expect("retain generations"); + + assert!(!directory.path().join(".candidate-crash").exists()); + assert!(!directory.path().join("100-a").exists()); + assert!(directory.path().join("200-b").exists()); + assert!(directory.path().join("300-c").exists()); + } + + #[test] + fn retention_counts_an_older_active_generation_toward_the_limit() { + let directory = tempdir().expect("artifact fixture"); + for generation in ["100-active", "200-b", "300-c"] { + std::fs::create_dir(directory.path().join(generation)).expect("create generation"); + } + + retain_artifact_generations(directory.path(), "100-active", 2).expect("retain generations"); + + assert!(directory.path().join("100-active").exists()); + assert!(!directory.path().join("200-b").exists()); + assert!(directory.path().join("300-c").exists()); + } + + #[test] + fn recovery_restores_previous_generation_without_leaving_artifact_root() { + let directory = tempdir().expect("artifact fixture"); + let artifact_root = directory.path().join("artifacts/site"); + std::fs::create_dir_all(artifact_root.join("300-c")).expect("create interrupted output"); + let state = SessionState::load(directory.path().join("state.json")).expect("load state"); + state + .merge_json_object(serde_json::Map::from_iter([ + ( + "artifact.site.generation".into(), + Value::String("300-c".into()), + ), + ( + "artifact.site.path".into(), + Value::String(artifact_root.join("300-c").to_string_lossy().into_owned()), + ), + ( + "artifact.site.switching_generation".into(), + Value::String("300-c".into()), + ), + ( + "artifact.site.rollback_generation".into(), + Value::String("200-b".into()), + ), + ( + "artifact.site.rollback_path".into(), + Value::String(artifact_root.join("200-b").to_string_lossy().into_owned()), + ), + ])) + .expect("seed interrupted switch"); + + recover_interrupted_artifact("site", &artifact_root, &state) + .expect("recover interrupted switch"); + + assert_eq!( + state + .get_string("artifact.site.generation") + .expect("read generation") + .as_deref(), + Some("200-b") + ); + assert!(!artifact_root.join("300-c").exists()); + assert_eq!( + state + .get_string("artifact.site.switching_generation") + .expect("read switch marker"), + None + ); + + let outside = directory.path().join("outside"); + std::fs::create_dir(&outside).expect("create outside directory"); + state + .set( + "artifact.site.switching_generation", + Value::String("../outside".into()), + ) + .expect("seed malicious marker"); + recover_interrupted_artifact("site", &artifact_root, &state) + .expect_err("path-like generation must be rejected"); + assert!( + outside.exists(), + "recovery must remain inside artifact root" + ); + } + #[test] fn expands_http_probe_urls_from_parent_env() { let _guard = EnvVarGuard::set("CONTAINER_PORT", Some("18080")); @@ -3590,6 +4146,7 @@ exec sleep 600 "server", &ProbeSpec::Http { url: "http://127.0.0.1:$CONTAINER_PORT/".into(), + expect_body: None, interval_ms: 100, timeout_ms: 1000, }, diff --git a/src/state.rs b/src/state.rs index a9fdc29..717d3e8 100644 --- a/src/state.rs +++ b/src/state.rs @@ -66,6 +66,16 @@ impl SessionState { self.save_snapshot(&snapshot) } + pub fn remove(&self, key: &str) -> Result<()> { + let mut values = self.lock_values()?; + if values.remove(key).is_none() { + return Ok(()); + } + let snapshot = values.clone(); + drop(values); + self.save_snapshot(&snapshot) + } + pub fn get_string(&self, key: &str) -> Result> { let values = self.lock_values()?; Ok(values