fix(harness): allow SkillLoadTool in Plan Mode by promoting isReadOnly to AgentTool interface#2067
Merged
Conversation
…y to AgentTool interface SkillLoadTool implements AgentTool directly (not ToolBase), so PlanModeMiddleware's readOnlyResolver — which checked `instanceof ToolBase` — never recognized it as read-only, blocking `load_skill_through_path` calls in Plan Mode. - Add `default boolean isReadOnly()` to AgentTool interface (returns false) - Override isReadOnly() in SkillLoadTool to return true - Update HarnessAgent readOnlyResolver to use AgentTool.isReadOnly() instead of downcasting to ToolBase - Add @OverRide to ToolBase.isReadOnly() Closes #2056
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes Plan Mode’s “read-only tool” detection so tools that implement AgentTool directly (not ToolBase)—specifically SkillLoadTool / load_skill_through_path—can be permitted during Plan Mode when they declare themselves read-only.
Changes:
- Add
default boolean isReadOnly()toAgentTool(defaultfalse) and alignToolBase.isReadOnly()as an override. - Mark
SkillLoadToolas read-only by overridingisReadOnly()to returntrue. - Update
HarnessAgentPlan Mode resolver to useAgentTool.isReadOnly()and add a regression test covering toolkit-based resolution.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| agentscope-harness/src/test/java/io/agentscope/harness/agent/middleware/PlanModeMiddlewareTest.java | Adds an end-to-end regression test ensuring SkillLoadTool is treated as read-only and permitted in Plan Mode while a mutating tool is denied. |
| agentscope-harness/src/main/java/io/agentscope/harness/agent/skill/runtime/SkillLoadTool.java | Declares the skill-loading tool read-only via AgentTool.isReadOnly(). |
| agentscope-harness/src/main/java/io/agentscope/harness/agent/HarnessAgent.java | Updates Plan Mode’s read-only resolver to call AgentTool.isReadOnly() instead of downcasting to ToolBase. |
| agentscope-core/src/main/java/io/agentscope/core/tool/ToolBase.java | Marks existing isReadOnly() as an @Override of the new interface method. |
| agentscope-core/src/main/java/io/agentscope/core/tool/AgentTool.java | Introduces isReadOnly() on the interface with Javadoc and a default false implementation. |
Comment on lines
+106
to
+107
| * <p>Read-only tools are automatically permitted in restricted execution modes such as | ||
| * Plan Mode, where write operations require explicit approval. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SkillLoadToolimplementsAgentTooldirectly (notToolBase), soPlanModeMiddleware'sreadOnlyResolver— which checkedinstanceof ToolBase— never recognized it as read-only, blockingload_skill_through_pathcalls in Plan Mode.default boolean isReadOnly()to theAgentToolinterface (returnsfalseby default), overrides it inSkillLoadToolto returntrue, and updatesHarnessAgent's resolver to use the interface method instead of downcasting toToolBase.Changes
AgentTool.java: adddefault boolean isReadOnly()returningfalseToolBase.java: add@Overrideto existingisReadOnly()SkillLoadTool.java: overrideisReadOnly()to returntrueHarnessAgent.java: change resolver fromt instanceof ToolBase tb && tb.isReadOnly()tot != null && t.isReadOnly(); remove unusedToolBaseimportPlanModeMiddlewareTest.java: add test verifyingSkillLoadToolis permitted in Plan Mode via the toolkit-based resolverTest plan
PlanModeMiddlewareTesttests pass (7/7)SkillLoadToolFrontmatterViewTestpassesskillLoadToolIsReadOnlyAndPermittedInPlanModetest validates end-to-end:SkillLoadTool.isReadOnly() == true, toolkit resolver permits it,write_fileis still deniedmvn spotless:checkcleanCloses #2056