Fix flaky plugin hook env test (#20088)

The test was flaky because it was checking the right thing in a
roundabout way.

What it wanted to prove:
- plugin hooks receive the right environment variables.

What it actually did:
1. Run a plugin hook.
2. Have that hook write those env vars into a temporary `env.json` file.
3. After the hook finished, read `env.json` back from disk.

On Windows, that last file was sometimes not there when the test tried
to read it, so the test failed with `read env log: file not found`. The
hook system itself was not what the test failure was directly proving;
the test was failing on the extra filesystem side effect it introduced.

The fix is to stop using a temp file as the proof mechanism. The hook
now prints the env values in its normal structured output, and the test
asserts on the output that the hook engine already captures. So we still
verify the same behavior, but without depending on a separate file being
created and read back correctly on Windows.
This commit is contained in:
Abhinav
2026-04-28 15:45:26 -07:00
committed by GitHub
Unverified
parent 2e598df6fc
commit 3291463ff1
+17 -13
View File
@@ -19,6 +19,8 @@ use codex_config::TomlValue;
use codex_plugin::PluginHookSource;
use codex_plugin::PluginId;
use codex_protocol::ThreadId;
use codex_protocol::protocol::HookOutputEntryKind;
use codex_protocol::protocol::HookRunStatus;
use codex_protocol::protocol::HookSource;
use pretty_assertions::assert_eq;
use tempfile::tempdir;
@@ -345,22 +347,18 @@ async fn plugin_hook_sources_run_with_plugin_env_and_plugin_source() {
AbsolutePathBuf::try_from(temp.path().join("plugin-data")).expect("plugin data root");
fs::create_dir_all(plugin_root.join("hooks")).expect("create hooks dir");
let source_path = plugin_root.join("hooks/hooks.json");
let log_path = plugin_root.join("env.json");
let script_path = plugin_root.join("hooks/write_env.py");
fs::write(
script_path.as_path(),
format!(
r#"import json
r#"import json
import os
from pathlib import Path
Path(r"{log_path}").write_text(json.dumps({{
"plugin": os.environ.get("PLUGIN_ROOT"),
"claude": os.environ.get("CLAUDE_PLUGIN_ROOT"),
}}), encoding="utf-8")
print(json.dumps({
"systemMessage": json.dumps({
"plugin": os.environ.get("PLUGIN_ROOT"),
"claude": os.environ.get("CLAUDE_PLUGIN_ROOT"),
})
}))
"#,
log_path = log_path.display(),
),
)
.expect("write hook script");
let plugin_id = PluginId::parse("demo-plugin@test-marketplace").expect("plugin id");
@@ -427,9 +425,15 @@ Path(r"{log_path}").write_text(json.dumps({{
assert_eq!(outcome.hook_events.len(), 1);
assert_eq!(outcome.hook_events[0].run.source, HookSource::Plugin);
assert_eq!(outcome.hook_events[0].run.status, HookRunStatus::Completed);
assert_eq!(outcome.hook_events[0].run.entries.len(), 1);
assert_eq!(
outcome.hook_events[0].run.entries[0].kind,
HookOutputEntryKind::Warning
);
let logged: serde_json::Value =
serde_json::from_str(&fs::read_to_string(log_path.as_path()).expect("read env log"))
.expect("parse env log");
serde_json::from_str(&outcome.hook_events[0].run.entries[0].text)
.expect("parse env payload");
assert_eq!(
logged,
serde_json::json!({