fix(harness): drain session-tree-mirror on HarnessAgent.close

[jialiuyang]

AgentScope-Java Version

2.0.x (main)

Description

SessionTree schedules fire-and-forget remote mirrors on a daemon session-tree-mirror executor. Example tests that close HarnessAgent via try-with-resources can still race JUnit @TempDir cleanup when that thread writes under the workspace (agents/.../sessions), causing intermittent DirectoryNotEmptyException failures in CI.

This change adds SessionTree.awaitPendingMirrors and calls it from HarnessAgent.close() so teardown waits for in-flight mirrors. SessionTreeMirrorTest now uses the same drain helper instead of a fixed sleep.

Fixes #2251

Checklist

• Code has been formatted with mvn spotless:apply
• Targeted tests passing
• Javadoc comments are complete and follow project conventions
• Related documentation has been updated — N/A
• Code is ready for review

[codecov]

Codecov Report

❌ Patch coverage is 31.25000% with 11 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...cope/harness/agent/memory/session/SessionTree.java	21.42%	11 Missing ⚠️

📢 Thoughts on this report? Let us know!

[AgentScopeJavaBot]

🤖 AI Review

This PR fixes a race condition (issue #2251) where fire-and-forget async mirror tasks in SessionTree could still be uploading session files to the remote filesystem after HarnessAgent.close() returns, racing with JUnit @TempDir cleanup or workspace teardown. The fix introduces a clever awaitPendingMirrors() drain mechanism that exploits the FIFO ordering of the single-thread MIRROR_EXECUTOR: by submitting a no-op task and waiting for it, all previously queued mirrors are guaranteed to have completed. The test helper awaitMirror() is also upgraded from a fragile Thread.sleep(200) to the new deterministic drain. The implementation is clean, the exception handling is thorough (timeout, interrupt with flag restore, execution exception), and the approach is sound.

[AgentScopeJavaBot]

[recommended] Orphaned Javadoc — placement breaks doc-to-method binding. The new awaitPendingMirrors method is inserted between the existing Javadoc for pullRemoteEntries (line 473-476) and the pullRemoteEntries method itself (line 508). In Java, only the last /** */ block immediately before a declaration is effective. The result: the old Javadoc (line 473) is now orphaned, and pullRemoteEntries (line 508) silently lost its documentation. Fix: Move the awaitPendingMirrors method above the orphaned Javadoc block, or move the orphaned Javadoc down to sit directly above pullRemoteEntries.

[AgentScopeJavaBot]

[nitpick] Return value of awaitPendingMirrors is silently discarded. The method returns false on timeout/interrupt/failure, but close() ignores it. The internal log.warn calls inside awaitPendingMirrors already provide observability, so this is functionally fine for a best-effort drain. Consider adding a brief inline comment (e.g., // best-effort; warnings logged internally) to make the intent explicit for future maintainers.

[AgentScopeJavaBot]

[nitpick] Hardcoded 30-second drain timeout. Mirrors typically complete in milliseconds, so 30 s is a generous safety net. If close() latency matters in any path, consider extracting this as a named constant (DRAIN_TIMEOUT_SECONDS) or making it configurable. Not blocking — the current value is reasonable.

[AgentScopeJavaBot]

🤖 AI Review

This PR fixes a race condition (issue #2251) where fire-and-forget async mirror tasks in SessionTree could still be uploading session files to the remote filesystem after HarnessAgent.close() returns, racing with JUnit @TempDir cleanup or workspace teardown. The fix introduces a clever awaitPendingMirrors() drain mechanism that exploits the FIFO ordering of the single-thread MIRROR_EXECUTOR: by submitting a no-op task and waiting for it, all previously queued mirrors are guaranteed to have completed. The test helper awaitMirror() is also upgraded from a fragile Thread.sleep(200) to the new deterministic drain. The implementation is clean, the exception handling is thorough (timeout, interrupt with flag restore, execution exception), and the approach is sound.

[AgentScopeJavaBot]

[recommended] Orphaned Javadoc — placement breaks doc-to-method binding. The new awaitPendingMirrors method is inserted between the existing Javadoc for pullRemoteEntries (line 473-476) and the pullRemoteEntries method itself (line 508). In Java, only the last /** */ block immediately before a declaration is effective. The result: the old Javadoc (line 473) is now orphaned, and pullRemoteEntries (line 508) silently lost its documentation. Fix: Move the awaitPendingMirrors method above the orphaned Javadoc block, or move the orphaned Javadoc down to sit directly above pullRemoteEntries.

[AgentScopeJavaBot]

[nitpick] Return value of awaitPendingMirrors is silently discarded. The method returns false on timeout/interrupt/failure, but close() ignores it. The internal log.warn calls inside awaitPendingMirrors already provide observability, so this is functionally fine for a best-effort drain. Consider adding a brief inline comment (e.g., // best-effort; warnings logged internally) to make the intent explicit for future maintainers.

[AgentScopeJavaBot]

[nitpick] Hardcoded 30-second drain timeout. Mirrors typically complete in milliseconds, so 30 s is a generous safety net. If close() latency matters in any path, consider extracting this as a named constant (DRAIN_TIMEOUT_SECONDS) or making it configurable. Not blocking — the current value is reasonable.

[oss-maintainer]

Summary

This PR contains 107 lines of changes. Review in progress.

---

Automated review by github-manager-bot