test: strip sibling env overrides from isolated generation-lifecycle children (#55)
## Summary Fixes the intermittent `integration_b` failure observed under parallel load (once on 2026-08-23 local validation; the same class was last seen in CI on 2026-08-18 and partially addressed by #46). **Root cause.** `generation_lifecycle` tests run in child processes spawned by `run_in_isolated_process`, which strips a fixed key list from the child environment. A sibling `app_server_transport` test (`turn_event_stdout_eof_fails_immediately`) sets `APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS=5000` process-wide in the shared parent via `EnvGuard`. When a lifecycle spawn happened inside that window, the child inherited the 5-second override, the fake app-server turn hit `turn did not complete within 5000 ms`, the edit job landed in `recoverable_error`/`Failed`, and the test then waited 180 s for a `generation_failed` terminal message that never came — surfacing as an unrelated timeout panic far from the real trigger. ## Changes (`crates/petcore/tests/integration_b/generation_lifecycle.rs`, +93 lines, no production code) - Extend `ISOLATED_ENV_KEYS` from 5 to 17 keys: every process-wide override any sibling integration_b test can hold (`APC_TEST_*` timing/count/path hooks and the skill-source strictness gates). - Take `test_support::lock_process_state()` around the isolated-child spawn so no sibling holds an override while the child environment is captured. - Add `isolated_child_environment_strips_every_sibling_process_wide_override`: parses the sibling suite source and fails if any `EnvGuard::set` key is missing from the strip list (drift guard in both directions). - Add `isolated_child_spawn_clears_leaked_studio_turn_timeout_override`: reproduces the exact leak (parent holds the 5 s override, spawns child) and asserts the child does not observe it. - Changelog fragment under `changes/unreleased/`. ## Validation - New tests: both pass; parity test verified to fail against the pre-fix list. - Leak-window stress (EOF test holding the override, concurrent with all lifecycle isolated spawns, 8 threads × 8 rounds): 8/8 pass. - Full `integration_b` × 3: 3/3 pass (42 tests each). `integration_a` 96, `integration_c` 66, `integration_d` 164, petcore lib 301, CLI + types: all pass. - `cargo fmt --check` clean, `clippy --all-targets` zero warnings, `script/validate_pre_push.sh --base origin/main` passed. - No production code changed; runtime behavior is untouched (test-only env gating already existed). ## Development claim / 开发声明 - Domain: `release-control-plane` - Claim: `engineering-speedup` - Base commit: `ee53d683413b2c4ba6a308dc5e7d76293ca0cf6f` - Release preparation: `no` - Focused validation: - `python3 script/tests/test_validation_tooling.py` - `./script/validate_build_scripts_safety.sh --static-only` <!-- apc-engineering-coordination-v1 {"claim_overlap_rejections":1,"merge_tree_conflicts":0,"schema_version":"apc.engineering-coordination.v1","task_created_at":"2026-08-24T01:52:17Z","train_conflict_resolutions":0} --> --------- Co-authored-by: lilei <lilei@xmfunny.com>
This commit is contained in:
1 parent
b51d2ce1f3
commit
2844e08037
3 files changed
+104
No files matched your search
@@ -0,0 +1,7 @@
|
||||
{
|
||||
"schema_version": "apc.changelog-fragment.v1",
|
||||
"kind": "Fixed",
|
||||
"scope": "testing",
|
||||
"summary_en": "A concurrency flake in the PetCore integration suite is fixed: a transport test's process-wide Studio turn-timeout override could leak into an isolated generation-lifecycle child process and shrink its real 25-minute turn timeout to seconds, surfacing later as an unrelated terminal-message timeout. Isolated children now strip every sibling test's process-wide override and are spawned while holding the process-state lock; a parity test keeps the strip list in sync with the sibling suite.",
|
||||
"summary_zh": "修复 PetCore 集成测试的一个并发 flake:传输层测试进程级设置的 Studio turn 超时覆盖变量可能泄漏进隔离子进程,把真实 25 分钟 turn 超时压成数秒,随后以无关的终态消息超时形式暴露。隔离子进程现在会剥离兄弟测试的全部进程级覆盖变量,并在派生时持有进程状态锁;另加奇偶校验测试保证剥离清单与兄弟套件保持同步。"
|
||||
}
|
||||
@@ -15,17 +15,39 @@ use std::process::Command;
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
const ISOLATED_ENV_TEST: &str = "APC_GENERATION_LIFECYCLE_ISOLATED_TEST";
|
||||
/// Keys stripped from every isolated child. Besides the fake-server controls
|
||||
/// this file owns, every process-wide override that a sibling integration_b
|
||||
/// test can hold must be listed: the child inherits the shared parent
|
||||
/// environment at spawn time, and a leaked override (for example
|
||||
/// `APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS=5000` from the transport suite)
|
||||
/// silently turns a real 25-minute Studio turn timeout into seconds.
|
||||
const ISOLATED_ENV_KEYS: &[&str] = &[
|
||||
"CODEX_APP_SERVER_CMD",
|
||||
"APC_FAKE_APP_SERVER_WAIT_FILE",
|
||||
"APC_FAKE_APP_SERVER_ARCHIVE_FILE",
|
||||
"APC_FAKE_APP_SERVER_ARCHIVE_FAIL_ONCE_FILE",
|
||||
"APC_FAKE_APP_SERVER_JOB_CWD",
|
||||
"APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS",
|
||||
"APC_TEST_EXTERNAL_CHECKPOINT_ELAPSED_MS",
|
||||
"APC_TEST_TURN_COUNT_FILE",
|
||||
"APC_TEST_INTERRUPT_COUNT_FILE",
|
||||
"APC_TEST_JOB_DIR",
|
||||
"APC_TEST_JOB_CWD",
|
||||
"APC_TEST_ACTIVITY_PHASE_FILE",
|
||||
"APC_TEST_HELPER_PID_FILE",
|
||||
"APC_TEST_HELPER_EXIT_FILE",
|
||||
"APC_TEST_INTERRUPT_FILE",
|
||||
"APC_REQUIRE_EXTERNAL_SKILL_SOURCE",
|
||||
"APC_REQUIRE_SKILL_FULL_SOURCE",
|
||||
];
|
||||
|
||||
/// Runs environment-mutating scenarios in separate processes. Rust's test
|
||||
/// harness can execute the parents concurrently, while every child receives
|
||||
/// its own process environment and exact single-test filter.
|
||||
///
|
||||
/// The spawn takes the process-state lock: a sibling integration_b test can
|
||||
/// hold an `APC_TEST_*` override in the shared parent environment while this
|
||||
/// test spawns, and the child would otherwise inherit that value mid-flight.
|
||||
fn run_in_isolated_process(test_name: &str) -> bool {
|
||||
let exact_test_name = format!("generation_lifecycle::{test_name}");
|
||||
if let Some(active_test) = std::env::var_os(ISOLATED_ENV_TEST) {
|
||||
@@ -37,6 +59,7 @@ fn run_in_isolated_process(test_name: &str) -> bool {
|
||||
return false;
|
||||
}
|
||||
|
||||
let _lock = crate::test_support::lock_process_state();
|
||||
let mut command = Command::new(std::env::current_exe().expect("current test executable"));
|
||||
command
|
||||
.arg("--exact")
|
||||
@@ -1357,3 +1380,73 @@ fn generation_lifecycle_cancel_is_noop_for_failed_terminal_job() {
|
||||
0
|
||||
);
|
||||
}
|
||||
|
||||
/// A sibling integration_b test that sets a process-wide environment override
|
||||
/// while this file's isolated children spawn can leak that value into the
|
||||
/// child unless the key is stripped here. Parse the sibling source instead of
|
||||
/// duplicating the key list by hand, so adding a new override without
|
||||
/// registering it in `ISOLATED_ENV_KEYS` fails this test instead of becoming
|
||||
/// the next timing flake.
|
||||
#[test]
|
||||
fn isolated_child_environment_strips_every_sibling_process_wide_override() {
|
||||
let sibling = include_str!("app_server_transport.rs");
|
||||
let mut stripped: Vec<&str> = ISOLATED_ENV_KEYS.to_vec();
|
||||
stripped.push(ISOLATED_ENV_TEST);
|
||||
let mut unregistered: Vec<String> = Vec::new();
|
||||
for (index, line) in sibling.lines().enumerate() {
|
||||
let Some((_, rest)) = line.split_once("EnvGuard::set(") else {
|
||||
continue;
|
||||
};
|
||||
let Some(rest) = rest.trim().strip_prefix('"') else {
|
||||
continue;
|
||||
};
|
||||
let Some(key) = rest.split('"').next() else {
|
||||
continue;
|
||||
};
|
||||
if !stripped.contains(&key) {
|
||||
unregistered.push(format!("line {}: {}", index + 1, key));
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
unregistered.is_empty(),
|
||||
"sibling process-wide env overrides missing from ISOLATED_ENV_KEYS: {unregistered:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Reproduces the original flake trigger: a sibling test holds the Studio
|
||||
/// turn-timeout override in the shared parent environment while an isolated
|
||||
/// child is spawned. The child must never observe it, or a real 25-minute
|
||||
/// turn timeout silently shrinks to seconds and surfaces as an unrelated
|
||||
/// `wait_for_terminal_message` timeout hundreds of lines away.
|
||||
#[test]
|
||||
fn isolated_child_spawn_clears_leaked_studio_turn_timeout_override() {
|
||||
const TEST_NAME: &str =
|
||||
"generation_lifecycle::isolated_child_spawn_clears_leaked_studio_turn_timeout_override";
|
||||
if std::env::var_os(ISOLATED_ENV_TEST).is_some() {
|
||||
assert!(
|
||||
std::env::var_os("APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS").is_none(),
|
||||
"isolated child inherited APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS"
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
let _leaked = EnvVarGuard::set("APC_TEST_PET_STUDIO_TURN_TIMEOUT_MS", "5000");
|
||||
let _lock = crate::test_support::lock_process_state();
|
||||
let mut command = Command::new(std::env::current_exe().expect("current test executable"));
|
||||
command
|
||||
.arg("--exact")
|
||||
.arg(TEST_NAME)
|
||||
.arg("--nocapture")
|
||||
.arg("--test-threads=1")
|
||||
.env(ISOLATED_ENV_TEST, TEST_NAME);
|
||||
for key in ISOLATED_ENV_KEYS {
|
||||
command.env_remove(key);
|
||||
}
|
||||
let output = command.output().expect("launch isolated env probe child");
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"isolated child saw a leaked env override\nstdout:\n{}\nstderr:\n{}",
|
||||
String::from_utf8_lossy(&output.stdout),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
}
|
||||
@@ -440,6 +440,10 @@
|
||||
"path": "crates/petcore/src/rpc/**",
|
||||
"risk": "amber"
|
||||
},
|
||||
{
|
||||
"path": "crates/petcore/tests/integration_b/**",
|
||||
"risk": "amber"
|
||||
},
|
||||
{
|
||||
"path": "crates/petcore-types/src/**",
|
||||
"risk": "amber"
|
||||
|
||||
Reference in new issue
Block a user