From 4822a86e2f938c922e3385c84dab4290a3a53000 Mon Sep 17 00:00:00 2001
From: webmin <birdie_7761@msn.com>
Date: Thu, 14 May 2026 21:33:31 +0800
Subject: [PATCH] fix(plan): ensure changeHook receives finished plan instead
 of null in finishPlan()

finishPlan() previously set currentPlan=null before triggering
changeHooks, causing hooks to receive a null plan argument. This
prevented downstream event emission (e.g. TASK_EXECUTION_SUCCEEDED)
and made getCurrentPlan() return null during hook execution.

Reorder: trigger hooks first, then null currentPlan.
---
 .../io/agentscope/core/plan/PlanNotebook.java | 17 ++----
 .../core/plan/PlanNotebookToolTest.java       | 57 +++++++++++++++++++
 2 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/agentscope-core/src/main/java/io/agentscope/core/plan/PlanNotebook.java b/agentscope-core/src/main/java/io/agentscope/core/plan/PlanNotebook.java
index 473992c66..50fd74a18 100644
--- a/agentscope-core/src/main/java/io/agentscope/core/plan/PlanNotebook.java
+++ b/agentscope-core/src/main/java/io/agentscope/core/plan/PlanNotebook.java
@@ -853,18 +853,13 @@ public Mono<String> finishPlan(
 
         currentPlan.finish(state, outcome);
 
+        String message =
+                String.format("The current plan is finished successfully as '%s'.", stateStr);
+
         return storage.addPlan(currentPlan)
-                .then(
-                        Mono.defer(
-                                () -> {
-                                    currentPlan = null;
-                                    return triggerPlanChangeHooks()
-                                            .thenReturn(
-                                                    String.format(
-                                                            "The current plan is finished"
-                                                                    + " successfully as '%s'.",
-                                                            stateStr));
-                                }));
+                .then(triggerPlanChangeHooks())
+                .then(Mono.fromRunnable(() -> currentPlan = null))
+                .thenReturn(message);
     }
 
     /** View the historical plans. */
diff --git a/agentscope-core/src/test/java/io/agentscope/core/plan/PlanNotebookToolTest.java b/agentscope-core/src/test/java/io/agentscope/core/plan/PlanNotebookToolTest.java
index 559519969..e8cc36d7a 100644
--- a/agentscope-core/src/test/java/io/agentscope/core/plan/PlanNotebookToolTest.java
+++ b/agentscope-core/src/test/java/io/agentscope/core/plan/PlanNotebookToolTest.java
@@ -806,4 +806,61 @@ void testMultipleHooksAndIdOverwrite() {
         assertEquals(1, count2[0], "hook2 should be called");
         assertEquals(1, count3[0], "New hook1 should be called");
     }
+
+    @Test
+    void testChangeHookReceivesNonNullPlanOnFinishPlan() {
+        Plan[] capturedPlan = {null};
+        notebook.addChangeHook("finishHook", (nb, plan) -> capturedPlan[0] = plan);
+
+        List<SubTask> subtasks = List.of(new SubTask("Task1", "Desc1", "Outcome1"));
+        notebook.createPlanWithSubTasks("Plan", "Desc", "Outcome", subtasks).block();
+
+        notebook.finishPlan("done", "All done").block();
+
+        assertNotNull(capturedPlan[0], "Hook should receive the finished plan, not null");
+        assertEquals("Plan", capturedPlan[0].getName());
+        assertEquals(PlanState.DONE, capturedPlan[0].getState());
+    }
+
+    @Test
+    void testChangeHookReceivesNonNullPlanOnFinishPlanAbandoned() {
+        Plan[] capturedPlan = {null};
+        notebook.addChangeHook("abandonHook", (nb, plan) -> capturedPlan[0] = plan);
+
+        List<SubTask> subtasks = List.of(new SubTask("Task1", "Desc1", "Outcome1"));
+        notebook.createPlanWithSubTasks("Plan", "Desc", "Outcome", subtasks).block();
+
+        notebook.finishPlan("abandoned", "Not needed").block();
+
+        assertNotNull(capturedPlan[0], "Hook should receive the abandoned plan, not null");
+        assertEquals(PlanState.ABANDONED, capturedPlan[0].getState());
+    }
+
+    @Test
+    void testGetCurrentPlanVisibleDuringFinishPlanHook() {
+        Plan[] capturedViaGetter = {null};
+        notebook.addChangeHook(
+                "getterHook", (nb, plan) -> capturedViaGetter[0] = nb.getCurrentPlan());
+
+        List<SubTask> subtasks = List.of(new SubTask("Task1", "Desc1", "Outcome1"));
+        notebook.createPlanWithSubTasks("Plan", "Desc", "Outcome", subtasks).block();
+
+        notebook.finishPlan("done", "Done").block();
+
+        assertNotNull(
+                capturedViaGetter[0],
+                "getCurrentPlan() should return the plan during hook execution");
+        assertEquals("Plan", capturedViaGetter[0].getName());
+    }
+
+    @Test
+    void testCurrentPlanIsNullAfterFinishPlanCompletes() {
+        List<SubTask> subtasks = List.of(new SubTask("Task1", "Desc1", "Outcome1"));
+        notebook.createPlanWithSubTasks("Plan", "Desc", "Outcome", subtasks).block();
+
+        notebook.finishPlan("done", "Done").block();
+
+        assertNull(
+                notebook.getCurrentPlan(), "currentPlan should be null after finishPlan completes");
+    }
 }