From 184bfc75be0166eec41df889614b9509345959b2 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 03:14:12 -0400 Subject: [PATCH 1/8] docs(ci): document CI performance design --- .../specs/2026-07-12-ci-performance-design.md | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-12-ci-performance-design.md diff --git a/docs/superpowers/specs/2026-07-12-ci-performance-design.md b/docs/superpowers/specs/2026-07-12-ci-performance-design.md new file mode 100644 index 00000000..3bc813d2 --- /dev/null +++ b/docs/superpowers/specs/2026-07-12-ci-performance-design.md @@ -0,0 +1,164 @@ +# CI Performance Design + +## Summary + +PV's pull-request CI will keep one macOS job and the full application test suite while removing redundant work and fixing test-fixture defects that make the suite wait unnecessarily. The change will: + +- remove blanket serialization of daemon tests, +- fix Redis and RustFS fixture shutdown behavior, +- make the immediate-exit Mailpit fixture track the process that actually exits, +- add the repository's existing pinned Rust cache action, +- remove the internal Rust documentation gate, and +- move artifact-recipe CLI smoke coverage into the existing integration test and remove its standalone workflow invocation. + +Formatting, Clippy, unused-dependency checks, recipe shellcheck, and all non-ignored workspace tests remain required. Production shutdown behavior and its ten-second grace period remain unchanged. + +## Evidence And Root Causes + +The cited [GitHub Actions job](https://github.com/prvious/pv/actions/runs/29180358045/job/86616706422) ran for pull-request commit `d3ebe255f2de5bbeeb3aa8362e1f15ef417b1aa5` on `macos-14` and took 7 minutes 4 seconds. Its material steps were: + +| Step | Duration | +| --- | ---: | +| Run tests | 4m 32s | +| Run Clippy | 54s | +| Build docs | 50s | +| Validate artifact recipe metadata and fixtures | 20s | + +The test step spent 2 minutes 8 seconds compiling and 141.910 seconds executing 1,003 tests. + +### Blanket daemon-test serialization + +`.config/nextest.toml` puts every test in the `daemon` package, plus tests whose names match `daemon_`, into a group with one worker. That filter matched 254 tests in the cited run. Those tests accounted for approximately 136.465 of the suite's 141.910 aggregate execution seconds, so the configuration serialized nearly all meaningful test work. + +The group was introduced after a daemon fixture race. A subsequent fix made accepted diagnostic streams blocking, addressing the underlying fixture behavior. A resource audit found isolated temporary homes, databases, sockets, and ports in current daemon tests rather than a shared resource that requires package-wide serialization. + +Three complete local runs with the group disabled passed all 1,003 tests. Their nextest execution summaries were 37.885, 38.694, and 39.370 seconds. They used an empty temporary config with `cargo nextest run --config-file /tmp/pv-nextest-empty.toml --workspace --all-features --locked` after the initial compile on macOS. + +### Fixture shutdown deadlocks + +The Redis and RustFS fake servers call Python's `BaseServer.shutdown()` from signal handlers on the same thread that is running `serve_forever()`. Python requires `shutdown()` to be called from a different thread. The fixtures therefore wait until PV's intentional ten-second graceful-shutdown deadline and are force-killed. + +Seven Redis and RustFS tests consumed approximately 89 seconds in the cited serialized run. The tests cover supported Managed Resource behavior and are valuable; the fake servers are the defect. + +### Immediate-exit process race + +The fast-exit Mailpit fixture launches Python from a shell script. The Python child exits immediately after serving readiness, but PV tracks the shell PID. Depending on how quickly the shell reaps its child, the runtime can appear alive during PV's post-readiness check. This makes `demanded_resource_cleans_runtime_files_when_process_exits_after_readiness` timing-dependent even though the behavior it protects is meaningful. + +### Redundant CI gates + +The Rust documentation command validates rustdoc-specific warnings, including broken intra-doc links, for internal crate documentation. PV is an unpublished application and its workspace crates are internal. The command does not build user-facing Markdown documentation or run doctests, and neither `DESIGN.md`, `CONTEXT.md`, nor an ADR establishes internal rustdoc as a product or release artifact. Removing it deliberately stops treating internal rustdoc correctness as a pull-request requirement; Clippy and nextest are not equivalent replacements for that narrow coverage. + +The standalone artifact-recipe step invokes `pv-release` twice with committed recipe paths. `crates/pv-release/tests/recipe_fixtures.rs` covers the same inputs and generation behavior, while parser tests cover the command arguments, but those tests currently bypass the binary's dispatch arms. Before removing the workflow step, the existing recipe integration test will invoke the compiled `pv-release` executable for both commands and validate the resulting archives, records, and manifest as it does today. This preserves automatic CLI parsing, dispatch, path wiring, and output coverage without a separate Cargo invocation. The manual artifact workflow continues running the commands against real builds, and recipe scripts still require shellcheck in pull-request CI. + +### Missing compilation cache + +The CI job has no Cargo cache even though `.github/workflows/real-artifact-e2e.yml` already uses `Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32`. The cited run downloaded hundreds of crates and compiled test artifacts from scratch. + +## Goals + +- Reduce pull-request CI wall time materially from the cited seven-minute run. +- Keep the complete non-ignored workspace application test suite. +- Preserve checks that directly protect source quality or supported product behavior. +- Fix slow and flaky fixtures at their cause instead of lowering production timeouts. +- Keep CI simple enough to understand from one workflow file. +- Leave release-only and privileged validation in their existing dedicated workflows. + +## Non-Goals + +- Do not change PV's production resource-stop grace period. +- Do not remove individual application tests. +- Do not weaken Clippy, formatting, dependency, or shell-script validation. +- Do not add Linux CI; PV v1 remains macOS-only. +- Do not split the workflow into multiple jobs that repeat setup and compilation. +- Do not alter real-artifact, privileged release-candidate, or artifact-publication workflows. +- Do not change workflow triggers, the Rust toolchain policy, or platform matrices. + +## CI Job Design + +The workflow remains a single `macos-14` job so every Rust command reuses the same target directory and cache. Its steps will be: + +1. Check out the repository. +2. Install Rust. +3. Restore/save Cargo artifacts with `Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32`, the exact revision already vetted in `real-artifact-e2e.yml`. +4. Install cargo-nextest, cargo-shear, and shellcheck. +5. Check formatting. +6. Run workspace Clippy for all targets and features with the lockfile enforced and warnings denied. +7. Check unused dependencies. +8. Shellcheck artifact recipe scripts. +9. Run the complete non-ignored workspace suite with nextest. + +The current `Build docs` step will be removed. `Validate artifact recipe metadata and fixtures` will be removed only after its binary-level coverage has been folded into the retained integration test. + +The cache is an acceleration only. A cache miss must still produce a correct run, and no generated output becomes a source of truth. + +## Test Scheduling Design + +Delete `.config/nextest.toml` so nextest uses its normal scheduler for the workspace. No replacement group will be added speculatively. + +If a future test demonstrates a concrete shared-resource collision, the narrowest exact test set may receive an isolated group together with a comment documenting the resource. Package-wide or name-prefix serialization must not be reintroduced without evidence that the entire selected set conflicts. + +## Fixture Design + +### Redis + +The fake Redis server will bind and run `serve_forever()` on the main thread exactly as it does today. Its `SIGTERM` and `SIGINT` handler will start a short-lived helper thread that calls `shutdown()`, then return to the server loop. The helper requests shutdown from a different thread, satisfying Python's threading contract; `serve_forever()` then returns normally on the main thread. + +Keeping the server loop on the main thread means bind failures and unexpected loop termination continue to terminate the fixture rather than leaving a main thread waiting indefinitely. The fixture's protocol behavior, arguments, data-directory behavior, and port allocation remain unchanged. + +### RustFS + +The fake RustFS API server will remain in `serve_forever()` on the main thread, and the console server will remain on its existing worker thread. The signal handler will start a helper thread that requests API shutdown and then return to the API loop. After the API loop returns, the main thread will shut down the console server from outside its worker thread and exit. This avoids both same-thread deadlock and a new failure mode where all server loops could stop while the main thread waits indefinitely. + +The S3 behavior, credential recording, API and console addresses, object persistence, and rejection fixture remain unchanged. + +### Immediate-exit Mailpit + +The fast-exit fixture will be a directly executable Python script with a `#!/usr/bin/env python3` shebang instead of a shell script that launches Python through standard input. It will preserve the fake adapter's `[smtp_port, dashboard_port]` argument contract, read the dashboard port from `sys.argv[2]`, serve one successful readiness response, flush it, and exit immediately. + +PV will therefore track the PID of the process that exits. The executable's script path remains present in the command line so the supervisor's ownership checks continue to recognize it. Blindly changing the shell to `exec python3 -` is not acceptable because the standard-input marker would replace the script path in the relevant command-line positions. + +## Coverage Decisions + +No application test will be removed. The Redis, RustFS, and immediate-exit tests exercise real Managed Resource lifecycle, allocation, credentials, reconciliation, and cleanup behavior described by PV's design. + +The standalone recipe command step will be consolidated rather than simply discarded: + +- the existing `pv-release` recipe integration test will use the repository's established `env!("CARGO_BIN_EXE_pv-release")` and `std::process::Command` pattern to run `generate-recipe-fixtures` and `generate-manifest`, +- the test will pass the exact committed recipe paths and require both commands to exit successfully, +- its existing archive, record, manifest, and snapshot assertions will validate the generated output, +- recipe scripts remain covered by shellcheck, and +- actual recipe commands remain a gate in the manual artifact build workflow. + +Removing rustdoc intentionally permits internal documentation-only warnings, including broken internal links, to stop blocking pull requests. Internal rustdoc should return as a gate if PV introduces a publishable Rust API, doctests that form part of application verification, or an explicit documentation requirement. None exists today. + +## Error Handling And Safety + +- Fixture changes must not catch or suppress bind, startup, or protocol errors. +- Signal handlers must delegate `shutdown()` to a different thread, return to the main server loop, and allow the process to exit normally. +- Unexpected termination of a main server loop must terminate the fixture rather than leave a waiting process behind. +- Test cleanup must continue removing runtime PID, metadata, and configuration files. +- Production process supervision, shutdown escalation, and timeout values remain untouched. +- Cache misses and invalid cache entries must fall back to normal compilation through the cache action's standard behavior. + +## Verification + +Implementation is complete only after all of the following pass: + +1. Run the affected Redis, RustFS, and immediate-exit lifecycle tests individually. +2. Repeat the affected fixture tests to exercise shutdown and process-exit timing. +3. Add and run a focused macOS supervisor test that starts a directly executable Python shebang script, confirms `verify_ownership()` recognizes the live process, and stops it cleanly. +4. Update and run the `pv-release` recipe-fixtures integration test, confirming that it invokes the compiled binary for both removed workflow commands and retains its output assertions. +5. Run the complete workspace nextest suite without serialization at least three times; every run must pass all non-ignored tests. +6. Run `cargo fmt --all --check`. +7. Run `cargo clippy --workspace --all-targets --all-features --locked -- -D warnings`. +8. Run `cargo shear`. +9. Run the artifact recipe shellcheck command retained by CI. +10. Run `git diff --check` and inspect the final workflow ordering and action revision. + +The existing lifecycle tests remain the regression checks for fixture behavior and must keep their current assertions. Two narrow coverage changes are required: the recipe integration test will absorb the removed binary smoke commands, and the ownership test will protect the interpreter-script command-line shape introduced by the fast-exit fixture. + +## Expected Outcome + +Removing the blanket group should reduce nextest execution from approximately 142 seconds to roughly 40 seconds on comparable hardware before considering cache improvements. Correct fixture shutdown removes repeated ten-second waits and prevents orphan fixture processes. Removing rustdoc and the standalone recipe commands eliminates about 70 seconds from the cited cold run while the recipe commands continue running inside the already-built test suite. + +The expected GitHub Actions wall time is approximately two to three minutes on cache hits and nearer four minutes after a cold cache miss. These are operational targets rather than hard test thresholds because runner and network performance vary. The correctness acceptance criterion is unchanged: every retained gate and all non-ignored workspace tests must pass. From c3fb01958815d600335826df98c91a932a275dda Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 16:23:38 -0400 Subject: [PATCH 2/8] docs(ci): add CI performance implementation plan --- .../plans/2026-07-12-ci-performance.md | 643 ++++++++++++++++++ 1 file changed, 643 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-12-ci-performance.md diff --git a/docs/superpowers/plans/2026-07-12-ci-performance.md b/docs/superpowers/plans/2026-07-12-ci-performance.md new file mode 100644 index 00000000..cea1291f --- /dev/null +++ b/docs/superpowers/plans/2026-07-12-ci-performance.md @@ -0,0 +1,643 @@ +# CI Performance 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:** Reduce PV pull-request CI from the cited seven-minute run while retaining all meaningful application coverage and making the affected daemon fixtures deterministic. + +**Architecture:** Keep one `macos-14` job and its shared Cargo target directory. Repair test-only process fixtures at their source, consolidate recipe CLI smoke coverage into the existing integration test, restore normal nextest scheduling, add the repository's vetted Rust cache, and remove only the two approved redundant workflow gates. + +**Tech Stack:** Rust 2024, Tokio, cargo-nextest, GitHub Actions, Python 3 fixture servers, shellcheck, cargo-shear, `insta`. + +## Global Constraints + +- Execute in `/Users/clovismuneza/Apps/pv/.worktrees/ci-performance` on branch `perf/ci-performance`, created from the approved plan commit. +- Read `CONTRIBUTING.md`, `DESIGN.md`, and `docs/superpowers/specs/2026-07-12-ci-performance-design.md` before changing implementation. +- Keep PV v1 CI on `macos-14`; do not add Linux jobs or split the workflow. +- Do not change production process supervision, the ten-second Managed Resource stop grace period, workflow triggers, Rust toolchain policy, or release workflows. +- Do not remove individual application tests or weaken formatting, Clippy, cargo-shear, recipe shellcheck, or the non-ignored workspace nextest suite. +- Preserve recipe CLI parsing and dispatch coverage before deleting its standalone workflow step. +- Use exactly `Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32`. +- Do not add dependencies or modify `Cargo.lock`. +- Prefer integration tests and existing helpers; avoid `panic!`, `unreachable!`, `.unwrap()`, unsafe code, and broad clippy ignores. +- Do not add a timing assertion to the committed suite. Measure fixture and suite time operationally so runner variance cannot create a new flaky test. +- Use Conventional Commit messages exactly as listed by each task. + +--- + +## File Structure + +- Modify `crates/daemon/tests/supervisor_foundation.rs` to cover ownership verification for a directly executable Python shebang script through the public supervisor API. +- Modify `crates/daemon/src/managed_resources/tests.rs` to make the fast-exit Mailpit fixture track Python directly and to correct Redis/RustFS signal shutdown. +- Modify `crates/pv-release/tests/recipe_fixtures.rs` so the committed-recipe integration test invokes both `pv-release` commands through the compiled binary. +- Modify `.github/workflows/ci.yml` to add the pinned Rust cache and remove the approved standalone recipe and rustdoc steps. +- Delete `.config/nextest.toml` to restore nextest's normal scheduler. +- Do not modify production Rust modules, snapshots, release workflows, or dependency manifests. + +--- + +### Task 1: Deterministic Fast-Exit Fixture And Script Ownership Coverage + +**Files:** +- Modify: `crates/daemon/tests/supervisor_foundation.rs` +- Modify: `crates/daemon/src/managed_resources/tests.rs` + +**Interfaces:** +- Consumes: `ProcessSupervisor::start(ProcessSpec)`, `ProcessSupervisor::verify_ownership(&ProcessSpec)`, `ManagedProcess::stop(Duration)`, `FakeMailpitRuntimeAdapter::exits_after_readiness()`, and the fake adapter argument order `[smtp_port, dashboard_port]`. +- Produces: integration test `supervisor_verifies_owned_python_shebang_script` and a `fast_exit_fake_mailpit_script()` whose tracked process is the Python process that calls `os._exit(0)`. + +- [ ] **Step 1: Record the existing timing-dependent failure evidence** + +Run the existing regression repeatedly without editing it: + +```shell +for run in {1..20}; do + cargo nextest run -p daemon --lib --locked \ + -E 'test(demanded_resource_cleans_runtime_files_when_process_exits_after_readiness)' || break +done +``` + +Expected on current code: the test may fail with `expected fast-exit runtime failure, got Ok(...)`; it may also pass because the shell/Python PID race is timing-dependent. GitHub Actions run `29180535363` is the retained red evidence. Do not introduce sleeps or a timing assertion just to force a local failure. + +- [ ] **Step 2: Add an ownership characterization integration test** + +Add this test immediately after `supervisor_verifies_and_adopts_owned_runtime_metadata` in `crates/daemon/tests/supervisor_foundation.rs`. Existing imports already provide `Duration`, `Result`, `anyhow`, `tempdir`, `PvPaths`, `sleep`, and `timeout`; existing helpers provide `process_spec` and `set_executable`. + +```rust +#[cfg(target_os = "macos")] +#[tokio::test] +async fn supervisor_verifies_owned_python_shebang_script() -> Result<()> { + let tempdir = tempdir()?; + let paths = PvPaths::for_home(tempdir.path().join("home")); + state::fs::ensure_layout(&paths)?; + let runtime = paths.run().join("owned-python-runtime"); + state::fs::write_sensitive_file( + &runtime, + r#"#!/usr/bin/env python3 +import signal +import sys + + +def stop(_signum, _frame): + sys.exit(0) + + +if sys.argv[1:] != ["1025", "8025"]: + sys.exit(2) + +signal.signal(signal.SIGTERM, stop) +signal.pause() +"#, + )?; + set_executable(&runtime)?; + + let supervisor = ProcessSupervisor::new(paths.clone()); + let spec = process_spec( + &paths, + "owned-python-runtime", + runtime, + vec!["1025".to_string(), "8025".to_string()], + ); + let process = supervisor.start(spec.clone()).await?; + let pid = process.pid(); + let ownership = timeout(Duration::from_secs(1), async { + loop { + if let Some(owned) = supervisor.verify_ownership(&spec)? { + return Ok::<_, daemon::DaemonError>(owned); + } + + sleep(Duration::from_millis(10)).await; + } + }) + .await; + + process.stop(Duration::from_secs(1)).await?; + let owned = ownership??; + + assert_eq!(owned.pid(), pid); + + Ok(()) +} +``` + +The ownership result is retained until after `process.stop(...)`, so an assertion or ownership error cannot leak the child process. + +- [ ] **Step 3: Run the ownership characterization test** + +Run: + +```shell +cargo nextest run -p daemon --test supervisor_foundation --locked \ + -E 'test(supervisor_verifies_owned_python_shebang_script)' +``` + +Expected: PASS. This is characterization coverage for the supervisor's existing interpreter-script command-line support, not a test that should fail before the fixture refactor. + +- [ ] **Step 4: Replace the shell/Python fast-exit fixture with direct Python** + +Replace `fast_exit_fake_mailpit_script()` in `crates/daemon/src/managed_resources/tests.rs` with: + +```rust +fn fast_exit_fake_mailpit_script() -> &'static str { + r#"#!/usr/bin/env python3 +import http.server +import os +import sys + + +class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + self.send_response(200) + self.end_headers() + self.wfile.write(b"ready") + self.wfile.flush() + os._exit(0) + + def log_message(self, _format, *_args): + pass + + +server = http.server.ThreadingHTTPServer(("127.0.0.1", int(sys.argv[2])), Handler) +server.serve_forever() +"# +} +``` + +`sys.argv[1]` remains the SMTP port and `sys.argv[2]` remains the dashboard port. Do not use `exec python3 -`; the expected fixture path must remain in the live command line for ownership verification. + +- [ ] **Step 5: Run the focused regressions repeatedly** + +Run: + +```shell +cargo nextest run -p daemon --test supervisor_foundation --locked \ + -E 'test(supervisor_verifies_owned_python_shebang_script)' + +for run in {1..20}; do + cargo nextest run -p daemon --lib --locked \ + -E 'test(demanded_resource_cleans_runtime_files_when_process_exits_after_readiness)' +done +``` + +Expected: the ownership test passes and all 20 fast-exit runs report the expected runtime failure/cleanup snapshot without returning `Ok(...)`. + +- [ ] **Step 6: Run task hygiene checks** + +Run: + +```shell +cargo fmt --all --check +git diff --check +``` + +Expected: both commands exit zero. + +- [ ] **Step 7: Commit Task 1** + +```shell +git add crates/daemon/tests/supervisor_foundation.rs crates/daemon/src/managed_resources/tests.rs +git commit -m "test(daemon): stabilize fast-exit runtime fixture" +``` + +--- + +### Task 2: Graceful Redis And RustFS Fixture Shutdown + +**Files:** +- Modify: `crates/daemon/src/managed_resources/tests.rs` + +**Interfaces:** +- Consumes: the current fake Redis and RustFS protocols, ports, credentials, data directories, and Python `BaseServer.serve_forever()`/`shutdown()` contract. +- Produces: signal handlers that request shutdown from helper threads while Redis and the RustFS API continue running `serve_forever()` on the main Python thread. + +- [ ] **Step 1: Capture the focused pre-change duration** + +Run: + +```shell +/usr/bin/time -p cargo nextest run -p daemon --lib --locked \ + -E 'test(redis_) | test(rustfs_)' +``` + +Expected on current code: all selected behavior tests pass, but Redis/RustFS lifecycle cases include repeated waits near the production ten-second grace period. Record the nextest summary and `real` duration for comparison; do not add a committed timing assertion. + +- [ ] **Step 2: Make Redis request shutdown from another thread** + +Within the embedded Python returned by `redis_server_script()`: + +1. Add `import threading` with the other top-level Python imports. +2. Replace the current `stop` function with: + +```python +shutdown_requested = threading.Event() + + +def stop(_signum, _frame): + if shutdown_requested.is_set(): + return + shutdown_requested.set() + threading.Thread(target=server.shutdown, daemon=True).start() +``` + +Keep these lines unchanged after server construction: + +```python +server = RedisServer(("127.0.0.1", port), RedisPingHandler) +signal.signal(signal.SIGTERM, stop) +signal.signal(signal.SIGINT, stop) +server.serve_forever() +``` + +Python signal handlers run serially on the main thread, so the event prevents redundant helper threads after the first shutdown request. The helper is a daemon thread so a repeated late signal cannot keep the fixture alive after the main server loop exits. Do not move `serve_forever()` off the main thread and do not change protocol behavior. + +- [ ] **Step 3: Make RustFS unwind through the main API loop** + +Within the embedded Python returned by `rustfs_script_source()`, replace the current `stop` function and the final server-loop block with: + +```python +shutdown_requested = threading.Event() + + +def stop(_signum, _frame): + if shutdown_requested.is_set(): + return + shutdown_requested.set() + threading.Thread(target=api.shutdown, daemon=True).start() + + +signal.signal(signal.SIGTERM, stop) +signal.signal(signal.SIGINT, stop) + +threading.Thread(target=console.serve_forever, daemon=True).start() +api.serve_forever() +console.shutdown() +``` + +Do not call `api.shutdown()` or `console.shutdown()` directly from the signal handler. The API loop remains on the main thread so unexpected loop errors terminate the fixture; after it returns, the main thread safely shuts down the console loop running on its existing worker thread. + +- [ ] **Step 4: Run and repeat the focused fixture suite** + +Run three times: + +```shell +for run in {1..3}; do + /usr/bin/time -p cargo nextest run -p daemon --lib --locked \ + -E 'test(redis_) | test(rustfs_)' +done +``` + +Expected: every selected test passes on every run, individual fixture tests no longer spend approximately ten seconds in forced shutdown, and the focused summary is materially lower than Step 1. Treat duration as operational evidence, not a pass/fail threshold. + +- [ ] **Step 5: Confirm fixture scope and formatting** + +Run: + +```shell +cargo fmt --all --check +git diff --check +git diff -- crates/daemon/src/managed_resources/tests.rs +``` + +Expected: only the embedded Redis/RustFS Python shutdown code changed in this task; Rust assertions, snapshots, production Rust modules, and timeout constants are unchanged. + +- [ ] **Step 6: Commit Task 2** + +```shell +git add crates/daemon/src/managed_resources/tests.rs +git commit -m "fix(ci): avoid resource fixture shutdown deadlocks" +``` + +--- + +### Task 3: Consolidate Recipe CLI Coverage Into The Integration Test + +**Files:** +- Modify: `crates/pv-release/tests/recipe_fixtures.rs` + +**Interfaces:** +- Consumes: binary `env!("CARGO_BIN_EXE_pv-release")`, commands `generate-recipe-fixtures` and `generate-manifest`, the committed recipe/revocation/default files, and existing archive/manifest assertions. +- Produces: automatic parser + dispatch + committed-path + generated-output coverage inside `recipe_fixture_generation_validates_archives_records_and_manifest`. + +- [ ] **Step 1: Add binary command imports and helpers** + +At the top of `crates/pv-release/tests/recipe_fixtures.rs`, add: + +```rust +use std::process::Output; + +use anyhow::{Context, Result, bail}; +``` + +Replace the existing `anyhow::{Result, bail}` import rather than duplicating it. Retain the direct `generate_recipe_fixtures_with_backing` and `generate_manifest_file_with_defaults` imports because the two custom-recipe tests still call the library APIs. + +Add this alias after imports: + +```rust +#[expect( + clippy::disallowed_types, + reason = "release tooling CLI tests execute the pv-release binary" +)] +type StdCommand = std::process::Command; +``` + +Add these helpers immediately before the existing filesystem helpers: + +```rust +fn run_pv_release(args: &[&str]) -> Result { + StdCommand::new(env!("CARGO_BIN_EXE_pv-release")) + .args(args) + .output() + .context("failed to execute pv-release") +} + +fn assert_command_success(output: &Output, label: &str) -> Result<()> { + if output.status.success() { + return Ok(()); + } + + bail!( + "{label} failed with status {:?}\nstdout:\n{}\nstderr:\n{}", + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} +``` + +- [ ] **Step 2: Invoke `generate-recipe-fixtures` through the binary** + +In `recipe_fixture_generation_validates_archives_records_and_manifest`, change the revocation path to the committed directory: + +```rust +let revocations = workspace_root.join("release/artifacts/revocations"); +``` + +Remove `create_dir_all(&revocations)?;` from this test only. Replace its direct `generate_recipe_fixtures_with_backing(...)` call with: + +```rust +let fixture_output = run_pv_release(&[ + "generate-recipe-fixtures", + "--php", + php.as_str(), + "--composer", + composer.as_str(), + "--redis", + redis.as_str(), + "--mysql", + mysql.as_str(), + "--postgres", + postgres.as_str(), + "--mailpit", + mailpit.as_str(), + "--rustfs", + rustfs.as_str(), + "--archives", + archives.as_str(), + "--records", + records.as_str(), + "--pv-commit", + "0123456789abcdef0123456789abcdef01234567", + "--build-run-id", + "local-test", +])?; +assert_command_success(&fixture_output, "generate-recipe-fixtures")?; +``` + +Keep the complete existing `generated_archive_roots` assertion unchanged. + +- [ ] **Step 3: Invoke `generate-manifest` through the binary** + +In the same test, replace its direct `generate_manifest_file_with_defaults(...)` call with: + +```rust +let manifest_output = run_pv_release(&[ + "generate-manifest", + "--records", + records.as_str(), + "--revocations", + revocations.as_str(), + "--defaults", + defaults.as_str(), + "--output", + manifest.as_str(), + "--base-url", + "https://artifacts.example.test", +])?; +assert_command_success(&manifest_output, "generate-manifest")?; +``` + +Keep the existing manifest parse and snapshot unchanged: + +```rust +let manifest_json = read_to_string(&manifest)?; +ArtifactManifest::parse(&manifest_json)?; +assert_snapshot!(manifest_json); +``` + +- [ ] **Step 4: Run the recipe integration test** + +Run: + +```shell +cargo nextest run -p pv-release --test recipe_fixtures --locked +``` + +Expected: all recipe fixture tests pass; the committed-recipe test invokes both compiled binary commands and its archive roots and manifest snapshot remain unchanged. No new snapshot is expected because the committed revocation directory currently contains only `.gitkeep`. + +- [ ] **Step 5: Run task quality checks** + +Run: + +```shell +cargo fmt --all --check +cargo clippy -p pv-release --all-targets --locked -- -D warnings +git diff --check +``` + +Expected: all commands exit zero. + +- [ ] **Step 6: Commit Task 3** + +```shell +git add crates/pv-release/tests/recipe_fixtures.rs +git commit -m "test(release): cover recipe commands through binary" +``` + +--- + +### Task 4: Simplify And Cache Pull-Request CI + +**Files:** +- Modify: `.github/workflows/ci.yml` +- Delete: `.config/nextest.toml` + +**Interfaces:** +- Consumes: the recipe binary coverage committed in Task 3 and the vetted cache action revision from `.github/workflows/real-artifact-e2e.yml`. +- Produces: one cached `macos-14` job with formatting, Clippy, cargo-shear, recipe shellcheck, and the complete normally scheduled workspace test suite. + +- [ ] **Step 1: Replace `ci.yml` with the approved gate sequence** + +The complete `.github/workflows/ci.yml` after editing must be: + +```yaml +name: CI + +on: + push: + branches: ["main"] + pull_request: + +jobs: + rust: + name: Rust + runs-on: macos-14 + + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + + - name: Cache Rust build + uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 + + - name: Install cargo-nextest + uses: taiki-e/install-action@v2 + with: + tool: cargo-nextest + + - name: Install cargo-shear + uses: taiki-e/install-action@v2 + with: + tool: cargo-shear + + - name: Install shellcheck + run: brew install shellcheck + + - name: Check formatting + run: cargo fmt --all --check + + - name: Run Clippy + run: cargo clippy --workspace --all-targets --all-features --locked -- -D warnings + + - name: Check unused dependencies + run: cargo shear + + - name: Check artifact recipe scripts + run: shellcheck release/artifacts/recipes/common.sh release/artifacts/recipes/php/*.sh release/artifacts/recipes/composer/*.sh release/artifacts/recipes/redis/*.sh release/artifacts/recipes/mysql/*.sh release/artifacts/recipes/postgres/*.sh release/artifacts/recipes/mailpit/*.sh release/artifacts/recipes/rustfs/*.sh + + - name: Run tests + run: cargo nextest run --workspace --all-features --locked +``` + +The `Build docs` and standalone `Validate artifact recipe metadata and fixtures` steps must be absent. Do not modify any other workflow. + +- [ ] **Step 2: Delete blanket nextest serialization** + +Delete `.config/nextest.toml` entirely. Do not add another test group or override. + +- [ ] **Step 3: Verify the configuration-only diff** + +Run: + +```shell +test ! -e .config/nextest.toml +git diff --check +git diff -- .github/workflows/ci.yml .config/nextest.toml +``` + +Expected: the nextest config is absent; the workflow diff adds only the exact pinned cache action and removes only the two approved steps. No dedicated test is added for these configuration-only edits; Task 5 validates their retained commands and normal scheduling. + +- [ ] **Step 4: Run one complete suite with normal scheduling** + +Run: + +```shell +/usr/bin/time -p cargo nextest run --workspace --all-features --locked +``` + +Expected: all non-ignored tests pass (at least 1,004 after Task 1), no `daemon-lifecycle` group appears, and nextest execution is materially below the cited 141.910 seconds. + +- [ ] **Step 5: Commit Task 4** + +```shell +git add .github/workflows/ci.yml +git add -u .config/nextest.toml +git commit -m "ci: remove redundant gates and test serialization" +``` + +--- + +### Task 5: Full Verification And Performance Evidence + +**Files:** +- Verify only; modify files only if a check exposes a defect within the approved scope. + +**Interfaces:** +- Consumes: Tasks 1–4. +- Produces: final correctness evidence, three clean normally scheduled suite runs, quality-gate results, and before/after timing suitable for the handoff. + +- [ ] **Step 1: Re-run all focused regression coverage** + +Run: + +```shell +cargo nextest run -p daemon --test supervisor_foundation --locked \ + -E 'test(supervisor_verifies_owned_python_shebang_script)' +cargo nextest run -p daemon --lib --locked \ + -E 'test(demanded_resource_cleans_runtime_files_when_process_exits_after_readiness)' +cargo nextest run -p daemon --lib --locked \ + -E 'test(redis_) | test(rustfs_)' +cargo nextest run -p pv-release --test recipe_fixtures --locked +``` + +Expected: every focused test passes. + +- [ ] **Step 2: Run every retained non-test CI gate** + +Run: + +```shell +cargo fmt --all --check +cargo clippy --workspace --all-targets --all-features --locked -- -D warnings +cargo shear +shellcheck release/artifacts/recipes/common.sh release/artifacts/recipes/php/*.sh release/artifacts/recipes/composer/*.sh release/artifacts/recipes/redis/*.sh release/artifacts/recipes/mysql/*.sh release/artifacts/recipes/postgres/*.sh release/artifacts/recipes/mailpit/*.sh release/artifacts/recipes/rustfs/*.sh +``` + +Expected: every command exits zero. Do not run `cargo doc`; removing internal rustdoc from the PR contract is an approved design decision. + +- [ ] **Step 3: Run the complete suite three times and retain timings** + +Run: + +```shell +for run in {1..3}; do + /usr/bin/time -p cargo nextest run --workspace --all-features --locked +done +``` + +Expected: all non-ignored tests pass in all three runs. Record each nextest execution summary and wall time. The operational target is approximately 40 seconds of nextest execution on comparable warm local hardware; timing is not a correctness threshold. + +- [ ] **Step 4: Inspect the final branch** + +Run: + +```shell +git diff --check HEAD~4..HEAD +git status --short --branch +git log --oneline --decorate -5 +``` + +Expected: no uncommitted changes, four implementation commits after the plan commit, no lockfile or snapshot churn, and only the files declared by this plan changed. + +- [ ] **Step 5: Request final review** + +Provide reviewers with: + +- the cited CI baseline: 7m04s job, 4m32s test step, 141.910s nextest execution, +- the three post-change nextest execution summaries and wall times, +- confirmation that all focused tests, all three full suites, Clippy, formatting, cargo-shear, and shellcheck passed, +- the intentional coverage decision for internal rustdoc, and +- confirmation that recipe binary dispatch remains automatically covered. + +Do not claim a GitHub Actions wall time until the branch has an actual remote run; report the local evidence and the expected cache-hit range separately. From 38ad99aa5074768af525594a62dd2b92177fd985 Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 16:39:14 -0400 Subject: [PATCH 3/8] test(daemon): stabilize fast-exit runtime fixture --- crates/daemon/src/managed_resources/tests.rs | 12 ++--- crates/daemon/tests/supervisor_foundation.rs | 55 ++++++++++++++++++++ 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/crates/daemon/src/managed_resources/tests.rs b/crates/daemon/src/managed_resources/tests.rs index b4aa4737..86632155 100644 --- a/crates/daemon/src/managed_resources/tests.rs +++ b/crates/daemon/src/managed_resources/tests.rs @@ -4585,16 +4585,12 @@ done } fn fast_exit_fake_mailpit_script() -> &'static str { - r#"#!/bin/sh -set -eu - -dashboard_port="$2" - -python3 - "$dashboard_port" <<'PY' + r#"#!/usr/bin/env python3 import http.server import os import sys + class Handler(http.server.BaseHTTPRequestHandler): def do_GET(self): self.send_response(200) @@ -4606,9 +4602,9 @@ class Handler(http.server.BaseHTTPRequestHandler): def log_message(self, _format, *_args): pass -server = http.server.ThreadingHTTPServer(("127.0.0.1", int(sys.argv[1])), Handler) + +server = http.server.ThreadingHTTPServer(("127.0.0.1", int(sys.argv[2])), Handler) server.serve_forever() -PY "# } diff --git a/crates/daemon/tests/supervisor_foundation.rs b/crates/daemon/tests/supervisor_foundation.rs index cd81d6ab..ad602d01 100644 --- a/crates/daemon/tests/supervisor_foundation.rs +++ b/crates/daemon/tests/supervisor_foundation.rs @@ -470,6 +470,61 @@ async fn supervisor_verifies_and_adopts_owned_runtime_metadata() -> Result<()> { Ok(()) } +#[cfg(target_os = "macos")] +#[tokio::test] +async fn supervisor_verifies_owned_python_shebang_script() -> Result<()> { + let tempdir = tempdir()?; + let paths = PvPaths::for_home(tempdir.path().join("home")); + state::fs::ensure_layout(&paths)?; + let runtime = paths.run().join("owned-python-runtime"); + state::fs::write_sensitive_file( + &runtime, + r#"#!/usr/bin/env python3 +import signal +import sys + + +def stop(_signum, _frame): + sys.exit(0) + + +if sys.argv[1:] != ["1025", "8025"]: + sys.exit(2) + +signal.signal(signal.SIGTERM, stop) +signal.pause() +"#, + )?; + set_executable(&runtime)?; + + let supervisor = ProcessSupervisor::new(paths.clone()); + let spec = process_spec( + &paths, + "owned-python-runtime", + runtime, + vec!["1025".to_string(), "8025".to_string()], + ); + let process = supervisor.start(spec.clone()).await?; + let pid = process.pid(); + let ownership = timeout(Duration::from_secs(1), async { + loop { + if let Some(owned) = supervisor.verify_ownership(&spec)? { + return Ok::<_, daemon::DaemonError>(owned); + } + + sleep(Duration::from_millis(10)).await; + } + }) + .await; + + process.stop(Duration::from_secs(1)).await?; + let owned = ownership??; + + assert_eq!(owned.pid(), pid); + + Ok(()) +} + #[tokio::test] async fn supervisor_rejects_owned_runtime_when_private_environment_changes() -> Result<()> { let tempdir = tempdir()?; From 687730d6f3f815b7ab4e08a73e167f6ffd3d337d Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 16:50:16 -0400 Subject: [PATCH 4/8] fix(ci): avoid resource fixture shutdown deadlocks --- crates/daemon/src/managed_resources/tests.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/daemon/src/managed_resources/tests.rs b/crates/daemon/src/managed_resources/tests.rs index 86632155..f58c3d28 100644 --- a/crates/daemon/src/managed_resources/tests.rs +++ b/crates/daemon/src/managed_resources/tests.rs @@ -4870,6 +4870,7 @@ import signal import shlex import socketserver import sys +import threading def redis_config(argv): port = None @@ -4912,8 +4913,14 @@ class RedisPingHandler(socketserver.BaseRequestHandler): class RedisServer(socketserver.ThreadingMixIn, socketserver.TCPServer): allow_reuse_address = True +shutdown_requested = threading.Event() + + def stop(_signum, _frame): - server.shutdown() + if shutdown_requested.is_set(): + return + shutdown_requested.set() + threading.Thread(target=server.shutdown, daemon=True).start() port, data_dir = redis_config(sys.argv[1:]) if data_dir: @@ -5079,16 +5086,21 @@ class Server(http.server.ThreadingHTTPServer): api = Server(split_address(api_address), RustfsHandler) console = Server(split_address(console_address), ConsoleHandler) +shutdown_requested = threading.Event() + + def stop(_signum, _frame): - api.shutdown() - console.shutdown() - sys.exit(0) + if shutdown_requested.is_set(): + return + shutdown_requested.set() + threading.Thread(target=api.shutdown, daemon=True).start() signal.signal(signal.SIGTERM, stop) signal.signal(signal.SIGINT, stop) threading.Thread(target=console.serve_forever, daemon=True).start() api.serve_forever() +console.shutdown() PY "# .replace("__PV_REJECT_S3__", reject_s3) From da12d1abc0b7aaa31f8aae945f9bad06e473974d Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 17:02:43 -0400 Subject: [PATCH 5/8] test(release): cover recipe commands through binary --- crates/pv-release/tests/recipe_fixtures.rs | 89 ++++++++++++++++------ 1 file changed, 67 insertions(+), 22 deletions(-) diff --git a/crates/pv-release/tests/recipe_fixtures.rs b/crates/pv-release/tests/recipe_fixtures.rs index 39fd1b50..783a7a25 100644 --- a/crates/pv-release/tests/recipe_fixtures.rs +++ b/crates/pv-release/tests/recipe_fixtures.rs @@ -1,4 +1,6 @@ -use anyhow::{Result, bail}; +use std::process::Output; + +use anyhow::{Context, Result, bail}; use camino::Utf8Path; use camino_tempfile::tempdir; use flate2::read::GzDecoder; @@ -11,6 +13,12 @@ use pv_release::record::{ReleaseRecord, load_release_records}; use resources::ArtifactManifest; use tar::Archive; +#[expect( + clippy::disallowed_types, + reason = "release tooling CLI tests execute the pv-release binary" +)] +type StdCommand = std::process::Command; + #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] struct ArchiveRoot { resource: String, @@ -39,7 +47,7 @@ fn recipe_fixture_generation_validates_archives_records_and_manifest() -> Result let tempdir = tempdir()?; let archives = tempdir.path().join("archives"); let records = tempdir.path().join("records"); - let revocations = tempdir.path().join("revocations"); + let revocations = workspace_root.join("release/artifacts/revocations"); let manifest = tempdir.path().join("manifest.json"); let php = workspace_root.join("release/artifacts/recipes/php/tracks.toml"); let composer = workspace_root.join("release/artifacts/recipes/composer/composer.toml"); @@ -50,27 +58,37 @@ fn recipe_fixture_generation_validates_archives_records_and_manifest() -> Result let rustfs = workspace_root.join("release/artifacts/recipes/rustfs/recipe.toml"); let defaults = workspace_root.join("release/artifacts/default-tracks.toml"); - create_dir_all(&revocations)?; let redis_recipe = BackingRecipe::load(&redis, BackingRecipeKind::Redis)?; let Some(redis_track) = redis_recipe.tracks().first() else { bail!("committed Redis recipe should define a track"); }; let redis_upstream_version = redis_track.upstream_version().to_string(); - generate_recipe_fixtures_with_backing( - &php, - &composer, - &[ - (BackingRecipeKind::Redis, redis.clone()), - (BackingRecipeKind::Mysql, mysql), - (BackingRecipeKind::Postgres, postgres), - (BackingRecipeKind::Mailpit, mailpit), - (BackingRecipeKind::Rustfs, rustfs), - ], - &archives, - &records, + let fixture_output = run_pv_release(&[ + "generate-recipe-fixtures", + "--php", + php.as_str(), + "--composer", + composer.as_str(), + "--redis", + redis.as_str(), + "--mysql", + mysql.as_str(), + "--postgres", + postgres.as_str(), + "--mailpit", + mailpit.as_str(), + "--rustfs", + rustfs.as_str(), + "--archives", + archives.as_str(), + "--records", + records.as_str(), + "--pv-commit", "0123456789abcdef0123456789abcdef01234567", + "--build-run-id", "local-test", - )?; + ])?; + assert_command_success(&fixture_output, "generate-recipe-fixtures")?; let archive_roots = generated_archive_roots(&archives, &records)?; assert_eq!( archive_roots, @@ -216,13 +234,20 @@ fn recipe_fixture_generation_validates_archives_records_and_manifest() -> Result ), ], ); - generate_manifest_file_with_defaults( - &records, - &revocations, - Some(&defaults), - &manifest, + let manifest_output = run_pv_release(&[ + "generate-manifest", + "--records", + records.as_str(), + "--revocations", + revocations.as_str(), + "--defaults", + defaults.as_str(), + "--output", + manifest.as_str(), + "--base-url", "https://artifacts.example.test", - )?; + ])?; + assert_command_success(&manifest_output, "generate-manifest")?; let manifest_json = read_to_string(&manifest)?; ArtifactManifest::parse(&manifest_json)?; @@ -428,6 +453,26 @@ fn archive_entries(path: &Utf8Path) -> Result> { Ok(entries) } +fn run_pv_release(args: &[&str]) -> Result { + StdCommand::new(env!("CARGO_BIN_EXE_pv-release")) + .args(args) + .output() + .context("failed to execute pv-release") +} + +fn assert_command_success(output: &Output, label: &str) -> Result<()> { + if output.status.success() { + return Ok(()); + } + + bail!( + "{label} failed with status {:?}\nstdout:\n{}\nstderr:\n{}", + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + #[expect( clippy::disallowed_methods, reason = "release tooling tests create local revocation fixture directories" From 7a7f219200b8d851ea44eb6059eb6f2159cd4c2f Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 17:16:42 -0400 Subject: [PATCH 6/8] ci: remove redundant gates and test serialization --- .config/nextest.toml | 6 ------ .github/workflows/ci.yml | 30 +++--------------------------- 2 files changed, 3 insertions(+), 33 deletions(-) delete mode 100644 .config/nextest.toml diff --git a/.config/nextest.toml b/.config/nextest.toml deleted file mode 100644 index 2fda0548..00000000 --- a/.config/nextest.toml +++ /dev/null @@ -1,6 +0,0 @@ -[test-groups] -daemon-lifecycle = { max-threads = 1 } - -[[profile.default.overrides]] -filter = 'package(daemon) | test(daemon_)' -test-group = 'daemon-lifecycle' diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0172f086..f6332171 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,9 @@ jobs: - name: Install Rust uses: dtolnay/rust-toolchain@stable + - name: Cache Rust build + uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 + - name: Install cargo-nextest uses: taiki-e/install-action@v2 with: @@ -42,32 +45,5 @@ jobs: - name: Check artifact recipe scripts run: shellcheck release/artifacts/recipes/common.sh release/artifacts/recipes/php/*.sh release/artifacts/recipes/composer/*.sh release/artifacts/recipes/redis/*.sh release/artifacts/recipes/mysql/*.sh release/artifacts/recipes/postgres/*.sh release/artifacts/recipes/mailpit/*.sh release/artifacts/recipes/rustfs/*.sh - - name: Validate artifact recipe metadata and fixtures - run: | - rm -rf /tmp/pv-recipe-fixtures - cargo run -p pv-release -- generate-recipe-fixtures \ - --php release/artifacts/recipes/php/tracks.toml \ - --composer release/artifacts/recipes/composer/composer.toml \ - --redis release/artifacts/recipes/redis/recipe.toml \ - --mysql release/artifacts/recipes/mysql/recipe.toml \ - --postgres release/artifacts/recipes/postgres/recipe.toml \ - --mailpit release/artifacts/recipes/mailpit/recipe.toml \ - --rustfs release/artifacts/recipes/rustfs/recipe.toml \ - --archives /tmp/pv-recipe-fixtures/archives \ - --records /tmp/pv-recipe-fixtures/records \ - --pv-commit "$(git rev-parse HEAD)" \ - --build-run-id ci - cargo run -p pv-release -- generate-manifest \ - --records /tmp/pv-recipe-fixtures/records \ - --revocations release/artifacts/revocations \ - --defaults release/artifacts/default-tracks.toml \ - --output /tmp/pv-recipe-fixtures/manifest.json \ - --base-url https://artifacts.example.test - - name: Run tests run: cargo nextest run --workspace --all-features --locked - - - name: Build docs - run: cargo doc --workspace --all-features --no-deps --locked - env: - RUSTDOCFLAGS: -D warnings From 3188967dadaf895153a5babbc94732f04581f86c Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 19:05:51 -0400 Subject: [PATCH 7/8] fix(ci): daemonize Redis fixture handlers --- crates/daemon/src/managed_resources/tests.rs | 86 +++++++++++++++++++- 1 file changed, 85 insertions(+), 1 deletion(-) diff --git a/crates/daemon/src/managed_resources/tests.rs b/crates/daemon/src/managed_resources/tests.rs index f58c3d28..031d51de 100644 --- a/crates/daemon/src/managed_resources/tests.rs +++ b/crates/daemon/src/managed_resources/tests.rs @@ -4,7 +4,7 @@ use std::os::unix::fs::PermissionsExt; use std::sync::{Arc, Mutex}; use std::time::Duration; -use anyhow::{Result, bail}; +use anyhow::{Error, Result, bail}; use camino::Utf8Path; use camino_tempfile::tempdir; use insta::{Settings, assert_debug_snapshot}; @@ -18,11 +18,14 @@ use state::{ ResourceAllocationRecord, ResourceAllocationStatus, RuntimeObservedStatus, RuntimeSubject, StateError, }; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpStream; use crate::{ DaemonError, ProcessSpec, ProcessSupervisor, ReadinessCheck, managed_resources::{ManagedResourceRuntimeAdapter, ManagedResourceRuntimeContext}, reconciliation::{ReconciliationQueue, ReconciliationScope}, + wait_for_readiness, }; const FAKE_MAILPIT_TRACK: &str = "1.0"; @@ -2553,6 +2556,86 @@ fn redis_process_arguments_disable_snapshots_without_empty_argv() -> Result<()> Ok(()) } +#[tokio::test] +async fn redis_fixture_stops_gracefully_with_active_client_connection() -> Result<()> { + let tempdir = tempdir()?; + let paths = PvPaths::for_home(tempdir.path().join("home")); + seed_redis_fixture_artifact(&paths, REDIS_TRACK)?; + let redis_listener = TcpListener::bind(("127.0.0.1", 0))?; + let redis_port = redis_listener.local_addr()?.port(); + drop(redis_listener); + + let adapter = super::redis::RedisRuntimeAdapter::new(); + let context = ManagedResourceRuntimeContext { + resource_name: "redis".to_string(), + track: REDIS_TRACK.to_string(), + artifact_path: paths + .resources() + .join("redis") + .join(REDIS_TRACK) + .join(format!("releases/{REDIS_ARTIFACT_VERSION}")), + data_dir: paths.resource_data_dir("redis", REDIS_TRACK), + ports: BTreeMap::from([("redis".to_string(), redis_port)]), + env: BTreeMap::new(), + }; + let spec = adapter.build_process_spec(&paths, &context)?; + let process = ProcessSupervisor::new(paths.clone()).start(spec).await?; + + let redis_client_result = async { + wait_for_readiness( + ReadinessCheck::RedisPing { + host: "127.0.0.1".to_string(), + port: redis_port, + }, + Duration::from_secs(3), + ) + .await?; + + match tokio::time::timeout(Duration::from_secs(3), async { + let mut redis_client = TcpStream::connect(("127.0.0.1", redis_port)).await?; + redis_client.write_all(b"*1\r\n$4\r\nPING\r\n").await?; + let mut response = [0_u8; 7]; + redis_client.read_exact(&mut response).await?; + if &response != b"+PONG\r\n" { + bail!("Redis fixture returned unexpected PING response: {response:?}"); + } + + Ok::<_, Error>(redis_client) + }) + .await + { + Ok(result) => result, + Err(error) => Err(Error::msg(format!( + "Redis fixture PING handshake timed out after 3 seconds: {error}" + ))), + } + } + .await; + let redis_client = match redis_client_result { + Ok(redis_client) => redis_client, + Err(error) => { + return match process.stop(Duration::ZERO).await { + Ok(()) => Err(error), + Err(cleanup_error) => { + bail!("{error}; Redis fixture cleanup also failed: {cleanup_error}"); + } + }; + } + }; + + let stop_started_at = tokio::time::Instant::now(); + let stop_result = process.stop(Duration::from_secs(10)).await; + let stop_elapsed = stop_started_at.elapsed(); + drop(redis_client); + stop_result?; + + if stop_elapsed >= Duration::from_secs(3) { + bail!("Redis fixture took {stop_elapsed:?} to stop gracefully"); + } + + Ok(()) +} + #[tokio::test] async fn redis_project_demand_installs_missing_fixture_track_before_start() -> Result<()> { let tempdir = tempdir()?; @@ -4912,6 +4995,7 @@ class RedisPingHandler(socketserver.BaseRequestHandler): class RedisServer(socketserver.ThreadingMixIn, socketserver.TCPServer): allow_reuse_address = True + daemon_threads = True shutdown_requested = threading.Event() From 26eb2ec390c4ee8ef02621aec1a4b8cff0af0aed Mon Sep 17 00:00:00 2001 From: Clovis Muneza Date: Sun, 12 Jul 2026 22:52:12 -0400 Subject: [PATCH 8/8] test(daemon): remove fixture shutdown regression --- crates/daemon/src/managed_resources/tests.rs | 96 ++------------------ 1 file changed, 6 insertions(+), 90 deletions(-) diff --git a/crates/daemon/src/managed_resources/tests.rs b/crates/daemon/src/managed_resources/tests.rs index 031d51de..07f1ca80 100644 --- a/crates/daemon/src/managed_resources/tests.rs +++ b/crates/daemon/src/managed_resources/tests.rs @@ -4,7 +4,12 @@ use std::os::unix::fs::PermissionsExt; use std::sync::{Arc, Mutex}; use std::time::Duration; -use anyhow::{Error, Result, bail}; +use crate::{ + DaemonError, ProcessSpec, ProcessSupervisor, ReadinessCheck, + managed_resources::{ManagedResourceRuntimeAdapter, ManagedResourceRuntimeContext}, + reconciliation::{ReconciliationQueue, ReconciliationScope}, +}; +use anyhow::{Result, bail}; use camino::Utf8Path; use camino_tempfile::tempdir; use insta::{Settings, assert_debug_snapshot}; @@ -18,15 +23,6 @@ use state::{ ResourceAllocationRecord, ResourceAllocationStatus, RuntimeObservedStatus, RuntimeSubject, StateError, }; -use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use tokio::net::TcpStream; - -use crate::{ - DaemonError, ProcessSpec, ProcessSupervisor, ReadinessCheck, - managed_resources::{ManagedResourceRuntimeAdapter, ManagedResourceRuntimeContext}, - reconciliation::{ReconciliationQueue, ReconciliationScope}, - wait_for_readiness, -}; const FAKE_MAILPIT_TRACK: &str = "1.0"; const FAKE_MAILPIT_NEXT_TRACK: &str = "1.1"; @@ -2556,86 +2552,6 @@ fn redis_process_arguments_disable_snapshots_without_empty_argv() -> Result<()> Ok(()) } -#[tokio::test] -async fn redis_fixture_stops_gracefully_with_active_client_connection() -> Result<()> { - let tempdir = tempdir()?; - let paths = PvPaths::for_home(tempdir.path().join("home")); - seed_redis_fixture_artifact(&paths, REDIS_TRACK)?; - let redis_listener = TcpListener::bind(("127.0.0.1", 0))?; - let redis_port = redis_listener.local_addr()?.port(); - drop(redis_listener); - - let adapter = super::redis::RedisRuntimeAdapter::new(); - let context = ManagedResourceRuntimeContext { - resource_name: "redis".to_string(), - track: REDIS_TRACK.to_string(), - artifact_path: paths - .resources() - .join("redis") - .join(REDIS_TRACK) - .join(format!("releases/{REDIS_ARTIFACT_VERSION}")), - data_dir: paths.resource_data_dir("redis", REDIS_TRACK), - ports: BTreeMap::from([("redis".to_string(), redis_port)]), - env: BTreeMap::new(), - }; - let spec = adapter.build_process_spec(&paths, &context)?; - let process = ProcessSupervisor::new(paths.clone()).start(spec).await?; - - let redis_client_result = async { - wait_for_readiness( - ReadinessCheck::RedisPing { - host: "127.0.0.1".to_string(), - port: redis_port, - }, - Duration::from_secs(3), - ) - .await?; - - match tokio::time::timeout(Duration::from_secs(3), async { - let mut redis_client = TcpStream::connect(("127.0.0.1", redis_port)).await?; - redis_client.write_all(b"*1\r\n$4\r\nPING\r\n").await?; - let mut response = [0_u8; 7]; - redis_client.read_exact(&mut response).await?; - if &response != b"+PONG\r\n" { - bail!("Redis fixture returned unexpected PING response: {response:?}"); - } - - Ok::<_, Error>(redis_client) - }) - .await - { - Ok(result) => result, - Err(error) => Err(Error::msg(format!( - "Redis fixture PING handshake timed out after 3 seconds: {error}" - ))), - } - } - .await; - let redis_client = match redis_client_result { - Ok(redis_client) => redis_client, - Err(error) => { - return match process.stop(Duration::ZERO).await { - Ok(()) => Err(error), - Err(cleanup_error) => { - bail!("{error}; Redis fixture cleanup also failed: {cleanup_error}"); - } - }; - } - }; - - let stop_started_at = tokio::time::Instant::now(); - let stop_result = process.stop(Duration::from_secs(10)).await; - let stop_elapsed = stop_started_at.elapsed(); - drop(redis_client); - stop_result?; - - if stop_elapsed >= Duration::from_secs(3) { - bail!("Redis fixture took {stop_elapsed:?} to stop gracefully"); - } - - Ok(()) -} - #[tokio::test] async fn redis_project_demand_installs_missing_fixture_track_before_start() -> Result<()> { let tempdir = tempdir()?;