Skip to content

fix(plan): ensure changeHook receives finished plan instead of null in finishPlan()#1400

Merged
LearningGp merged 6 commits into
agentscope-ai:mainfrom
birdie7761:fix/finishplan-hook-null
May 27, 2026
Merged

fix(plan): ensure changeHook receives finished plan instead of null in finishPlan()#1400
LearningGp merged 6 commits into
agentscope-ai:mainfrom
birdie7761:fix/finishplan-hook-null

Conversation

@birdie7761

Copy link
Copy Markdown
Contributor

Description

PlanNotebook.finishPlan() sets currentPlan = null before calling triggerPlanChangeHooks(), causing all registered change hooks to receive null instead of the finished plan. This prevents downstream event emission (e.g.
TASK_EXECUTION_SUCCEEDED, TASK_EXECUTION_CANCELLED) and makes getCurrentPlan() return null during hook execution.

All other mutation methods (createPlan, updatePlanInfo, finishSubtask, etc.) trigger hooks while currentPlan still references a valid object. finishPlan is the only method that nulls before hooking.

Root causePlanNotebook.java:

// Before fix: null first, then notify
currentPlan = null;
return triggerPlanChangeHooks()

triggerPlanChangeHooks() reads this.currentPlan directly, so hooks receive null.

Fix: trigger hooks first, then null currentPlan:

return storage.addPlan(currentPlan)
        .then(triggerPlanChangeHooks())
        .then(Mono.fromRunnable(() -> currentPlan = null))
        .thenReturn(message);

Checklist

- Code has been formatted with mvn spotless:apply
- All tests are passing (mvn test)
- Javadoc comments are complete and follow project conventions
- Related documentation has been updated (e.g. links, examples, etc.)
- Code is ready for review

…n 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.
@birdie7761
birdie7761 requested a review from a team May 15, 2026 03:32
@CLAassistant

CLAassistant commented May 15, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@LearningGp LearningGp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LearningGp
LearningGp merged commit 7408c92 into agentscope-ai:main May 27, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants