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

Offen
#841 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
agent-runtime bug
Vorherrschende Sprache
TypeScript
Sterne
143
Forks
46
Ø Merge
3 T. 10 Std.
Gemergte PRs (30 T.)
24

Beschreibung

## 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.

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Beginne in agent/tests/test_server.py bei reset_server_state und dem bestehenden Muster test_background_thread_failure_503_and_backup_terminal_write; untersuche server.py:_active_threads und _spawn_background hinsichtlich des getesteten Lebenszyklus. Führe die relevanten test_server.py-Tests aus und füge anschließend Abdeckung für eine langsame Hintergrundaufgabe hinzu. Überprüfe dabei, dass Fixture-Setup und -Teardown auf aktive Threads warten und bei einem Timeout fehlschlagen, ohne den nächsten Test zu beeinträchtigen.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
python
Bereich
testing-qa
Issue-Typ
Bug
Schwierigkeit
3/5
Geschätzter Aufwand
1-2 Tage
Aktivitätsstatus
Aktiv
Klarheit
Klar beschrieben
Anfängerfreundlichkeit
72/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.