Skip to content

fix: skillBox prompt should merge into system prompts#895

Open
fang-tech wants to merge 2 commits intoagentscope-ai:mainfrom
fang-tech:fxi/skill/hook-skill-prompt-should-merge-into-sysPrompt
Open

fix: skillBox prompt should merge into system prompts#895
fang-tech wants to merge 2 commits intoagentscope-ai:mainfrom
fang-tech:fxi/skill/hook-skill-prompt-should-merge-into-sysPrompt

Conversation

@fang-tech
Copy link
Contributor

@fang-tech fang-tech commented Mar 9, 2026

AgentScope-Java Version

[The version of AgentScope-Java you are working on, e.g. 1.0.9, check your pom.xml dependency version or run mvn dependency:tree | grep agentscope-parent:pom(only mac/linux)]

Description

fix & close #845

Checklist

Please check the following items before code is ready to be reviewed.

  • 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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes issue #845 where using skillBox() alongside sysPrompt() could produce multiple SYSTEM messages, which some model providers reject (e.g., “System message must be at the beginning.”).

Changes:

  • Update SkillHook to merge the skill prompt into an existing SYSTEM message when present, instead of always prepending a new SYSTEM message.
  • Adjust existing tests and add a new regression test for issue #845 behavior.

Reviewed changes

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

File Description
agentscope-core/src/main/java/io/agentscope/core/skill/SkillHook.java Merges skill prompt into an existing SYSTEM message (or prepends one if none exists).
agentscope-core/src/test/java/io/agentscope/core/skill/SkillHookTest.java Updates the “inject at first” test and adds a new test to validate merging behavior for #845.

@fang-tech fang-tech closed this Mar 9, 2026
@fang-tech fang-tech reopened this Mar 9, 2026
@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

[Bug]:使用skillBox调用Qwen3.5报错

2 participants