fix(harness): subagent event stream never closes when the parent is cancelled

[gxgeek-n]

Fixes #2480.

Problem

AgentSpawnTool emits AgentStartEvent unconditionally before subscribing to a sync-spawned child, then emits the matching AgentEndEvent from doOnTerminate. doOnTerminate fires on onComplete / onError but not on cancel, so a parent cancel (user "Stop", outer timeout(), Reactor dispose()) leaves the child's event stream open — consumers see a start with no end and keep rendering the subagent as running.

Verified Reactor semantics before touching anything (reactor-core 3.7.6):

terminal signal	doOnTerminate	doFinally
complete	1	1
error	1	1
cancel	0	1

Change

One call swapped, doOnTerminate → doFinally, so all three terminal signals close the stream.

Two things suggest this was an oversight rather than intent: doOnTerminate appears exactly once in the file, and the timeout-promotion path a few lines below already uses doFinally with a comment explicitly reasoning about doFinally(CANCEL).

Path 2 (deprecated SubagentEventBus) pairs events via the child's own isLast() and Path 3 is non-streaming, so neither needed changing.

Verification

AgentSpawnToolCancelEndEventTest:

case	without fix	with fix
parent cancel emits AgentEndEvent	FAIL — expected 1, got 0	PASS
normal completion emits exactly one start + one end	PASS	PASS

The second case passing both ways is the point: it pins that the fix does not start emitting duplicate end events on the happy path.

Full agentscope-harness suite: 693 tests, 0 failures, 3 skipped. Worth running given the change makes the cancel path emit an event it previously did not.

Relationship to #2408 / #2412

Complementary, not overlapping. #2412 fixes whether the child execution actually stops (interruptAgent being a no-op for HarnessAgent, plus the child RuntimeContext mismatch); this fixes whether the emitted event stream closes. I checked #2412's diff — it does not reference doOnTerminate, AgentStartEvent or AgentEndEvent. Both touch AgentSpawnTool.java but different lines, so if #2412 lands first I am happy to rebase.

spotless:apply has been run.

[gemini-code-assist]

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

[Copilot]

Pull request overview

This PR fixes a harness event-stream pairing bug where sync-spawned subagents could emit AgentStartEvent without a matching AgentEndEvent when the parent subscription is cancelled (because Reactor doOnTerminate does not run on cancel). The change ensures downstream consumers always see a closing end event and don’t keep rendering subagents as “running” after a parent cancel.

Changes:

• Replace doOnTerminate with doFinally in AgentSpawnTool so AgentEndEvent is emitted on complete/error/cancel.
• Add a regression test covering the cancel path (and a “happy path” assertion to prevent duplicate end events).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
agentscope-harness/src/main/java/io/agentscope/harness/agent/tool/AgentSpawnTool.java	Ensures AgentEndEvent is emitted even when the parent cancels, by switching to doFinally.
agentscope-harness/src/test/java/io/agentscope/harness/agent/tool/AgentSpawnToolCancelEndEventTest.java	Adds coverage for parent-cancel event pairing and confirms normal completion emits exactly one start/end pair.

[oss-maintainer]

Review — Approved ✅

Summary

One-line fix: doOnTerminate → doFinally in AgentSpawnTool#execLocalSync() Path 1, ensuring AgentEndEvent is emitted on all three terminal signals (complete, error, cancel).

Analysis

Correctness: The Reactor semantics are accurately described. doOnTerminate skips cancel; doFinally handles all three. This is the standard pattern for cleanup that must run regardless of termination reason.

Scope: Minimal and precise — only the affected call site is changed. Path 2 (deprecated) and Path 3 (non-streaming) are correctly left untouched.

Comment quality: The inline comment clearly explains why doFinally is used instead of doOnTerminate, preventing future regressions. Good practice.

Test coverage: AgentSpawnToolCancelEndEventTest (187 lines) covers:

• Cancel path emits AgentEndEvent (the bug fix)
• Happy path still emits exactly one start + one end (no duplicates)
• Uses Mono.never() + dispose() for deterministic cancel simulation

CI: License ✅, Module Sync ✅, builds pending (just triggered). Previous run on the same commit passed all checks.

Verdict

Clean, well-tested fix for a real event-stream leak. LGTM.

[gxgeek-n]

Thanks @copilot — good point, fixed.

Thread.sleep(300) replaced with a bounded wait loop that returns as soon as the AgentEndEvent is observed (5 s ceiling, 20 ms interval):

long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5);
while (emitter.count(AgentEndEvent.class) == 0 && System.nanoTime() < deadline) {
    Thread.sleep(20);
}

Both cases still pass, and the cancel case no longer spends a fixed 300 ms waiting. It also still fails without the doFinally change in AgentSpawnTool (expected 1, got 0), so the bounded wait did not weaken the assertion into a no-op.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!