fix(harness): auto-inject ALLOW rules for plan mode tools#1960
Open
Buktal wants to merge 6 commits into
Open
Conversation
When HarnessAgent.Builder.enablePlanMode(true) is called, plan_enter, plan_write, and todo_write now get unconditional ALLOW rules injected into PermissionContextState so PermissionEngine does not prompt ASK under DEFAULT mode. plan_exit deliberately excluded to preserve HITL confirmation. Changes: - Add permissionContext mirror field in HarnessAgent.Builder - Modify permissionContext() setter to store locally instead of forwarding to inner immediately - In build(): merge user context + plan mode allow rules, then set inner.permissionContext() once - Add 6 tests verifying auto-injection behavior Fixes agentscope-ai#1910
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
jujn
suggested changes
Jul 4, 2026
When a persisted session is loaded from AgentStateStore, the PermissionEngine is rebuilt from the stored permissionContext which may lack builder-time allow rules (e.g. plan-control tools injected by HarnessAgent). This causes plan_enter/plan_write/todo_write to fall back to ASK in DEFAULT mode on resumed sessions. Fix: inject initialPermissionContext allow rules into the PermissionEngine after construction, with dedup to avoid doubling rules already present in persisted state.
AgentScopeJavaBot
added
bug
AgentScopeJavaBot
left a comment
AgentScopeJavaBot
left a comment
Collaborator
There was a problem hiding this comment.
🤖 AI Review
Auto-injects ALLOW rules for plan mode tools. Two concerns: (1) ensureInitialAllowRules only handles ALLOW, missing DENY/ASK; (2) only 3 of 9 ALWAYS_ALLOWED tools get PermissionEngine rules.
| if (permCtx != null) { | ||
| asb.permissionContext(permCtx); | ||
| } | ||
| return asb.build(); |
Collaborator
There was a problem hiding this comment.
[minor] ensureInitialAllowRules only handles ALLOW rules. DENY/ASK rules from initial context are also lost on session restore. Consider extending or documenting scope.
| PermissionContextState.Builder pb = | ||
| PermissionContextState.builder().mode(base.getMode()); | ||
| base.getWorkingDirectories().forEach(pb::addWorkingDirectory); | ||
| base.getAllowRules() |
Collaborator
There was a problem hiding this comment.
[minor] Only 3 of 9 PlanModeMiddleware.ALWAYS_ALLOWED tools get PermissionEngine ALLOW rules. Keep in sync or share constant.
…n-mode allow list - Move rule re-insertion into PermissionEngine.ensureRulesFrom, covering allow/deny/ask symmetrically so DENY/ASK rules from the builder-time context survive session restore (previous version only handled ALLOW). - Extract the plan-mode auto-allow subset as PLAN_CONTROL_AUTO_ALLOW_TOOLS in HarnessAgent with Javadoc documenting the deliberate divergence from PlanModeMiddleware.ALWAYS_ALLOWED: plan_exit stays HITL, and the subagent/task tools remain ASK-gated in DEFAULT mode. - Tests: PermissionEngineTest.EnsureRulesFrom (allow/deny/ask, idempotence, null-safety) and HarnessAgentPlanModePermissionTest additions covering user deny/ask preservation and the end-to-end PermissionEngine decision for plan_write (ALLOW) vs plan_exit (ASK).
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
3 participants
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.
AgentScope-Java Version
2.0.0-SNAPSHOT
Description
Problem:
Issue #1910 reported that
PlanModeMiddlewarewhitelist andPermissionEnginewere not coordinated. WhenenablePlanMode(true)is called,plan_enter,plan_write, andtodo_writeare inPlanModeMiddleware.ALWAYS_ALLOWED(execution-layer whitelist), butPermissionEnginestill prompts ASK in DEFAULT mode because it has no awareness of plan mode context.Root Cause:
PlanModeMiddlewarecontrolsonActingphase (execution gate)PermissionEnginecontrolsonReasoningphase (permission prompt)Solution:
When
HarnessAgent.Builder.enablePlanMode(true)is called, automatically inject unconditional ALLOW rules forplan_enter,plan_write, andtodo_writeintoPermissionContextState.plan_exitis deliberately excluded to preserve HITL (Human-in-the-Loop) confirmation semantics.Changes:
permissionContextmirror field inHarnessAgent.Builder(default: empty context)permissionContext()setter to store locally instead of forwarding toinnerimmediatelybuild(): whenplanModeEnabled = true, merge user context + plan mode allow rules, then callinner.permissionContext()onceplanModeEnabled = false, directly forward user context toinner(behavior unchanged)Testing:
HarnessAgentPlanModePermissionTest:plan_enter/plan_write/todo_writeget ALLOW rules ✅plan_exithas no ALLOW rule (preserves ASK) ✅Backward Compatibility:
planModeEnabled = false(default): behavior unchangedFixes #1910
Checklist
mvn spotless:apply