aws-samples / aws-samples/sample-autonomous-cloud-coding-agents
bug(agent): test_server.py leaks live pipeline threads — reset fixture clears _active_threads without joining, causing cross-test mock contamination
- Lingua principale
- TypeScript
- Stelle
- 143
- Fork
- 46
- Merge medio
- 3g 10h
- PR unite (30g)
- 24
Descrizione
## Problem
`agent/tests/test_server.py` intermittently fails with a mock being called by a test that never made the call. Observed twice on PR #680 (a **CLI-only** PR that cannot touch `agent/`), each time on a *different* test:
| Run | Test |
| --- | --- |
| [33446493920](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/actions/runs/33446493920) | `TestMicrovmRunHookPlatformConfig::test_no_platform_config_and_no_baked_env_is_refused_not_run_unscoped` |
| earlier run | `TestMicrovmRunHookPreInstallAwsSilence::test_no_cloudwatch_or_credential_seam_is_touched_before_the_install[no-config]` |
```
FAILED tests/test_server.py::TestMicrovmRunHookPlatformConfig::test_no_platform_config_and_no_baked_env_is_refused_not_run_unscoped
AssertionError: Expected 'mock' to not have been called. Called 1 times.
Calls: [call(repo_url='org/repo', task_description='do it', github_token='ghp_x',
anthropic_model='', aws_region='', task_id='t-pc', max_turns=100, ...)]
```
`main` is green (verified by manual `workflow_dispatch` [33510225606](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/actions/runs/33510225606): agent 1755 passed, cdk 4322 passed, cli 768 passed), so this is a **race**, not a regression — a single green run does not clear it.
## Root cause
Three facts combine:
1. **`run_task` is resolved at call time, in a background thread.** `server.py:35` does `from pipeline import run_task`; `_run_task_background` calls it bare at `server.py:478`. A bare global resolves *when executed*, not when the thread starts. That function is the `threading.Thread` target in `_spawn_background` (`server.py:736`, `target=` at `:748`).
2. **The reset fixture drops thread references without joining them.** `test_server.py:21-29`:
```python
@pytest.fixture(autouse=True)
def reset_server_state():
...
with server._threads_lock:
server._active_threads.clear() # <-- drops the handle; thread keeps running
```
`_active_threads` (`server.py:255`) is only a tracking list. Clearing it does not stop anything.
3. **Every test in `TestMicrovmRunHookPlatformConfig` shares `task_id: "t-pc"`** (the `_payload()` default at `test_server.py:2364`), and six of them return `200`, each spawning a real `pipeline-t-pc` thread while `monkeypatch.setattr(server, "run_task", MagicMock())` is in effect.
**The sequence:** an accepting test spawns `pipeline-t-pc` → the fixture clears the list and pytest advances → the still-live thread reaches `server.py:478`, looks up `server.run_task`, and receives **the next test's mock** → that test's `assert_not_called()` fails.
The captured call args confirm it rather than merely fitting it: `task_id='t-pc'` is the class-wide default that the refusal test would never start, and `aws_region=''` / `anthropic_model=''` are env-derived values already stripped — a sibling test's environment, not this one's. Note also that the refusal test's own asserts (`400`, `MICROVM_RUN_PLATFORM_CONFIG_INCOMPLETE`, `missing_env`) all **passed**; only `assert_not_called()` failed. The request path is correct. The contamination is external.
## Why it surfaced now
Not new code — new *volume*. MicroVM P1 (#645 / `f3cfb4e3b`, Aug 6) and P2 (#733 / `4d53a73b0`, Aug 28) added several accepting `/run` hook tests that all share one `task_id`, widening the window. `build (agentcore)` has no `push:` trigger (`pull_request`, `merge_group`, `workflow_dispatch` only) and last ran on `main` on 2026-06-04, so nothing re-verifies `main` between merges either — filing separately.
Same family as #615 (scoped-session bypassing a `boto3.client` mock): a per-test mock defeated by state the fixture does not actually reset. #615 fixed the *env* half by adding `AGENT_SESSION_ROLE_ARN` to `_AGENT_ENV_VARS` (`conftest.py:162`); the *thread* half is still open.
## Fix
The file already contains the correct pattern — `test_background_thread_failure_503_and_backup_terminal_write` (`test_server.py:60-76`) polls until no thread `is_alive()`, with a comment explaining that joining "eliminates the race." Promote that into the autouse fixture.
- [ ] `reset_server_state` joins (with timeout) every live thread in `_active_threads` before clearing, on both setup and teardown
- [ ] Fail loudly if a thread outlives its timeout, rather than clearing and continuing — a silent leak is what produced this
- [ ] Give each accepting test in `TestMicrovmRunHookPlatformConfig` a distinct `task_id` so a leak is attributable to its origin instead of anonymous
- [ ] Regression test: spawn a deliberately slow `run_task`, let the fixture tear down, assert the next test's mock is untouched (should fail before the fix)
- [ ] Re-run #680's build once merged and confirm it goes green with no code change to that PR
## Not in scope
`_spawn_background`'s production behaviour is fine — daemon-free named threads tracked in a list is reasonable for the runtime. This is purely test lifecycle.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Inizia in agent/tests/test_server.py, in reset_server_state e nel pattern esistente test_background_thread_failure_503_and_backup_terminal_write; esamina server.py:_active_threads e _spawn_background per il ciclo di vita sottoposto a test. Esegui i test pertinenti di test_server.py, quindi aggiungi la copertura per un’attività in background lenta e verifica che il setup e il teardown del fixture attendano i thread attivi e falliscano in caso di timeout senza contaminare il test successivo.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- python
- Ambito
- testing-qa
- Tipo di issue
- Bug
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Stato di attività
- Attiva
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 72/100