Isolate Codex logs per top-level test (generalize #274)#275
Merged
Conversation
Generalize #274's per-run_graph Codex log isolation to each top-level Lua test. This closes the producer-side ambient FKST_RUNTIME_LOG_DIR leak that let observation tests pollute later run_graph consumers in the same harness process. Reuse the existing Department env guard for error-safe restoration, and cover cross-test status isolation plus error and panic cleanup. ⟦AI:FKST⟧
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Generalizes #274 ("Isolate run-graph Codex logs"). #274 gave each run_graph invocation a tempdir-scoped
FKST_RUNTIME_LOG_DIR(consumer side). This gives each top-level test function its own fresh tempdir-scopedFKST_RUNTIME_LOG_DIR, so no top-level test can see another's Codex status records.Why
A downstream packages test that drives a real (mocked) Codex-spawning department leaks a Codex status record into the ambient
FKST_RUNTIME_LOG_DIR. #274 stopped run-graph consumers from seeing such records, but the producer leak remained: any top-level test's leaked status record was still visible to sibling top-level tests in the same process viafkst.codex_runs(running/recent). That is the class this closes — a real incident had an observation test leak arole=fixstatus record that suppressed a later run-graph test's mocked spawn on the pre-#274 engine.Change
with_top_level_runtime_log_dircreates a fresh tempdir/logs per top-level test, appliesFKST_RUNTIME_LOG_DIRviaDeptRunEnvGuard, and restores/removes the prior env value error-safely on success, Lua error, and panic unwind (test_runner.rs:111,157-174). Reuses #274's mechanism; the inner per-run_graph-invocation isolation is preserved. Scope is minimal — only the Codex log dir, notRUNTIME_ROOT/DURABLE_ROOT.Tests
cargo build -p fkst-framework= 0;cargo test -p fkst-framework --test test_runner_cli= 38 passed/0;--test run_graph_cli= 0;--test sdk_codex= 0.tests/test_runner_cli.rs:2258: top-level test A records a mocked Codex status, top-level test B assertsfkst.codex_runsshows no cross-test record — exits 101 before this change and 0 after, proving isolation.Reviewed by 3 independent perspectives (error-safe restore / minimal scope / regression proves isolation) — all approve.
⟦AI:FKST⟧