test(harness): drain fire-and-forget memory flush before @TempDir teardown to stop flaky temp-dir deletion - #2935
Conversation
…rdown to stop flaky temp-dir deletion
There was a problem hiding this comment.
🟡 Changes recommended
The new quiescence extension currently ignores timeout/interrupt failures from the await methods, which can allow the original teardown flake to persist without a deterministic, actionable failure.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses intermittent JUnit @TempDir teardown failures in harness tests by ensuring fire-and-forget background writes (session/transcript mirrors and memory background tasks) have quiesced before temporary directories are deleted.
Changes:
- Added a JUnit Jupiter
AfterEachCallbackextension to await harness background task quiescence after each test method. - Added
@HarnessQuiescenceas a composed annotation to apply the extension with a one-liner. - Applied
@HarnessQuiescenceto multiple@TempDir-using harness integration tests that build/transiently runHarnessAgentinstances.
File summaries
| File | Description |
|---|---|
| agentscope-harness/src/test/java/io/agentscope/harness/agent/tools/HarnessAgentToolsConfigTest.java | Applies @HarnessQuiescence to prevent teardown races in this test. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/testing/HarnessQuiescence.java | Introduces composed annotation to register the quiescence extension. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/testing/HarnessBackgroundTaskQuiescenceExtension.java | Adds AfterEachCallback extension that drains mirror + memory background tasks. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/subagent/SubagentIsolationIntegrationTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/PlanModeSubagentPropagationTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/JsonSessionDefaultLocationTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentSubagentStreamTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentSubagentStreamEventsTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentModelStringTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentIntegrationExampleTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentDynamicHookBuilderTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/HarnessAgentDistributedSandboxTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/gateway/SubagentRegistryRecoveryIntegrationTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/example/SandboxFilesystemIsolationScopeExampleTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/example/RemoteFilesystemIsolationScopeExampleTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/example/LocalFilesystemUserIsolationExampleTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
| agentscope-harness/src/test/java/io/agentscope/harness/agent/example/LocalFilesystemPersonalAssistantExampleTest.java | Applies @HarnessQuiescence to avoid @TempDir deletion races. |
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Problem
Two CI runs fail intermittently with JUnit errors that are not test-logic failures but teardown failures:
Failed to delete temp directory(Windows, PR fix(sandbox): docker sandbox native file transfer + reject truncated downloads (#2618) #2923)Failed to close extension context/Failed to delete temp directory(Ubuntu, PR feat(middleware): add final answer filter for ReAct streams #2926)Both surface only when a test builds a transient
HarnessAgent, drives it to completion via.block()/.stream()...block(), and uses@TempDirfor its workspace/state home.Root cause
This is a long-standing race, not a regression of any single commit. The harness memory flush has always been asynchronous: when an agent stream completes,
MemoryFlushMiddleware#onAgentdispatches the flush onSchedulers.boundedElastic()viasubscribe(...)— i.e. fire-and-forget. The calling test's.block()only waits for the business stream, not for that background flush.So the timeline is:
.block()and returns.@TempDirteardown and deletes the temp directory.boundedElasticis still writing session/transcript mirror files into that same@TempDir(or still holds open file handles).The result: directory/file deletion fails with
IOException→ wrapped asJUnitException. It is timing-dependent (depends on IO speed, scheduler, and how many files are written), which is why it flakes rather than failing deterministically, and why Windows (stricter file locking) fails more readily than Linux.The normal production path avoids this because
HarnessAgent#close()callsSessionTree.awaitMirrorQuiescence(...)+MemoryBackgroundTasks.awaitQuiescence(...). The flaky tests never callclose()on their transient agent.Fix
Add test-side quiescence that mirrors what
HarnessAgent#close()already does, run after each test method but before theTempDirextension deletes the directory:HarnessBackgroundTaskQuiescenceExtension— anAfterEachCallbackthat callsSessionTree.awaitMirrorQuiescence(5s)+MemoryBackgroundTasks.awaitQuiescence(5s). When nothing is in flight both calls return immediately, making it a no-op for tests that never trigger a flush.@HarnessQuiescence— a composed meta-annotation (@ExtendWith(HarnessBackgroundTaskQuiescenceExtension.class)) so at-risk tests only need a one-line annotation.@HarnessQuiescenceto 16 harness test classes that match the at-risk pattern (HarnessAgent.builder()+.call()/.stream()+@TempDir), including the three classes that flaked in CI:JsonSessionDefaultLocationTest,HarnessAgentIntegrationExampleTest,HarnessAgentDynamicHookBuilderTest.Production code is unchanged — the flush remains fire-and-forget so conversation completion is never blocked.