test: repair three suite failures that are red on main (#3196)

## Summary

Three tests fail on a clean `main` full-suite run. None is a product
defect — all three are tests that stopped describing reality, and they
will noise up or block the 0.36.4 release.

| Test | Why it fails | Fix |
|---|---|---|
| `test_release_workflows::test_no_native_tls_in_wheel_build_tree` |
Shells out to `cargo`; raises `FileNotFoundError` wherever the Rust
toolchain is absent | Copied the skip guards its own dual already had |
|
`test_learn/test_integration::TestCodexIntegration::test_full_pipeline`
| Asserts `"Bash" in all_tools` against **real local Codex data**; Codex
renamed its shell tool | Assert what the test is for, across Codex
versions |
|
`test_graceful_shutdown::test_run_server_installs_cancelled_error_filter`
| Counts installs on the **process-global** `uvicorn.error` logger;
order-dependent | Isolate the global state; assert the real contract |

## 1. native-tls / cargo

The `openssl-sys` gate 30 lines above is described in-code as this
test's dual. It already skips when `cargo` is missing, **and** when
cargo fails for a reason other than `"package did not match"` (the Linux
wheel target not being installed locally). The native-tls test never
copied either guard.

Not disabled: CI installs the toolchain via `dtolnay/rust-toolchain`, so
the check still executes there. The skip only applies where cargo is
genuinely absent.

## 2. Codex tool vocabulary

This test runs against whatever Codex sessions the machine actually has
(gated by `HAS_CODEX_DATA`), and asserted:

```python
# Codex has only Bash tool (shell)
assert "Bash" in all_tools
```

Codex has since renamed its shell tool (`Bash` → `shell` → `exec`), and
0.149.0 added agent tools (`spawn_agent`, `send_message`, `wait`) beside
it. The assertion pinned one release's vocabulary, so it fails on any
current install.

It now asserts what the pipeline is actually being tested for — that
tool calls were extracted, including a shell-execution tool under any of
its known names — and names the remedy in the failure message for the
next rename.

**Still discriminating** (verified, not assumed):

| Scenario | Result |
|---|---|
| pipeline parsed nothing | fails ✓ |
| tool names garbled | fails ✓ |
| agent tools only, no shell tool | fails ✓ |
| real current Codex data | passes ✓ |

## 3. Global logger state

```python
if not any(isinstance(item, _SuppressCancelledErrorFilter) for item in uvicorn_error_logger.filters):
    uvicorn_error_logger.addFilter(_SuppressCancelledErrorFilter())
```

`run_server` is deliberately idempotent and `uvicorn.error` is a
process-global logger, so any earlier test in the session that reached
`run_server` leaves the filter attached — and this test then observes
**zero** installs against its `== 1` assertion. It passes alone and
fails in a full run, which is exactly the symptom.

The test now clears and restores that global state around itself, and
additionally asserts the idempotence guard that is the real contract:
calling `run_server` twice must not stack a duplicate filter. The test
got stronger, not just quieter.

## Scope

Tests only — no product code is touched.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Tejas Chopra <tejas@Tejass-MacBook-Pro.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Tejas Chopra 2026-08-21 23:18:40 -07:00 committed by GitHub
parent 3e3c409436
commit 5d25abd356
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 74 additions and 20 deletions

View file

@ -109,9 +109,40 @@ def test_run_server_installs_cancelled_error_filter(monkeypatch: pytest.MonkeyPa
from headroom.proxy.server import run_server
run_server(ProxyConfig(), print_banner=False)
# `uvicorn.error` is a process-global logger and run_server only installs
# the filter when one is not already attached. Any earlier test in the
# session that reached run_server therefore leaves it installed, and this
# test would observe zero installs. Isolate the global state rather than
# depend on test ordering.
uvicorn_error_logger = logging.getLogger("uvicorn.error")
preexisting = [
item
for item in uvicorn_error_logger.filters
if isinstance(item, _SuppressCancelledErrorFilter)
]
for item in preexisting:
uvicorn_error_logger.removeFilter(item)
assert len(installed_filters) == 1, "Expected exactly one _SuppressCancelledErrorFilter"
try:
run_server(ProxyConfig(), print_banner=False)
assert len(installed_filters) == 1, "Expected exactly one _SuppressCancelledErrorFilter"
# The idempotence guard is the real contract: a second call must not
# stack a duplicate filter on the shared logger.
run_server(ProxyConfig(), print_banner=False)
attached = [
item
for item in uvicorn_error_logger.filters
if isinstance(item, _SuppressCancelledErrorFilter)
]
assert len(attached) == 1, f"filter stacked on repeat calls: {len(attached)}"
finally:
for item in list(uvicorn_error_logger.filters):
if isinstance(item, _SuppressCancelledErrorFilter):
uvicorn_error_logger.removeFilter(item)
for item in preexisting:
original_add_filter(uvicorn_error_logger, item)
# ---------------------------------------------------------------------------

View file

@ -294,9 +294,20 @@ class TestCodexIntegration:
assert len(sessions) > 0
# Codex has only Bash tool (shell)
# This runs against whatever Codex sessions the machine actually has,
# so it must not pin one release's tool vocabulary. Codex has renamed
# its shell tool across versions (`Bash` -> `shell` -> `exec`) and
# 0.149.0 added agent tools alongside it, which is what the pipeline
# has to keep parsing.
all_tools = {tc.name for s in sessions for tc in s.tool_calls}
assert "Bash" in all_tools
assert all_tools, "pipeline extracted no tool calls from real Codex sessions"
shell_tool_aliases = {"Bash", "shell", "exec", "local_shell", "container.exec"}
assert all_tools & shell_tool_aliases, (
"no shell-execution tool recognised in real Codex sessions; "
f"saw {sorted(all_tools)} -- if Codex renamed it again, add the "
"new name to shell_tool_aliases"
)
def test_codex_writer_targets_agents_md(self, tmp_path):
"""Codex writer should target AGENTS.md, not CLAUDE.md."""

View file

@ -339,23 +339,35 @@ def test_no_native_tls_in_wheel_build_tree() -> None:
import subprocess
for crate in ("headroom-py", "headroom-proxy", "headroom-core"):
result = subprocess.run(
[
"cargo",
"tree",
"--target",
"x86_64-unknown-linux-gnu",
"-p",
crate,
"-i",
"native-tls",
],
cwd=str(ROOT),
capture_output=True,
text=True,
check=False,
)
try:
result = subprocess.run(
[
"cargo",
"tree",
"--target",
"x86_64-unknown-linux-gnu",
"-p",
crate,
"-i",
"native-tls",
],
cwd=str(ROOT),
capture_output=True,
text=True,
check=False,
)
except FileNotFoundError:
pytest.skip("cargo is unavailable in this environment")
# Same environment gates as the openssl-sys dual above: a cargo
# failure that is not "package did not match" means the Linux wheel
# target is unavailable here, not that native-tls came back.
not_in_tree = result.returncode != 0 and "did not match any packages" in result.stderr
if result.returncode != 0 and "package ID specification `native-tls` did not match" not in (
result.stderr + result.stdout
):
pytest.skip(
"cargo dependency tree for the Linux wheel target is unavailable in this environment"
)
assert not_in_tree, (
f"native-tls is back in {crate}'s build tree — likely some "
f"crate's `default-features = true` re-enabled native-tls "