Skip to content

fix(core): custom HITL deny reason and tool result events on ASK deny - #2513

Open
arimu1 wants to merge 4 commits into
agentscope-ai:mainfrom
arimu1:fix/2492-hitl-deny-events
Open

fix(core): custom HITL deny reason and tool result events on ASK deny#2513
arimu1 wants to merge 4 commits into
agentscope-ai:mainfrom
arimu1:fix/2492-hitl-deny-events

Conversation

@arimu1

@arimu1 arimu1 commented Jul 31, 2026

Copy link
Copy Markdown

AgentScope-Java Version

2.0.1-SNAPSHOT (main)

Description

Fixes #2492.

When permission HITL is in ASK and the user rejects a tool call:

  1. Custom deny reasonConfirmResult now has an optional message field. If set (non-blank), that text is written into the denied ToolResultBlock instead of the fixed "Permission denied by user". Existing callers keep the old default.
  2. Tool result events — manual deny now emits ToolResultStartEventToolResultTextDeltaEventToolResultEndEvent (state DENIED), matching the auto-deny path in runToolBatch. Previously only context was updated, so stream consumers never saw tool_result events for user denials.

Usage

// Prefer factory (no ambiguous 3-arg overload with List rules):
ConfirmResult.withMessage(false, toolCall, "User rejected: do not delete production data");
// or full form:
new ConfirmResult(false, toolCall, /* rules */ null, "custom reason");
// blank/null message → "Permission denied by user"

How tested

mvn -pl agentscope-core -am test -Dtest=ReActAgentHitlTest
# Tests run: 9, Failures: 0, Errors: 0

New coverage in ReActAgentHitlTest:

  • custom deny message appears in the denied tool result
  • blank/whitespace deny message falls back to "Permission denied by user"
  • resume-with-deny stream emits Start / TextDelta / End with DENIED state

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 testReActAgentHitlTest)
  • Javadoc comments are complete and follow project conventions
  • Related documentation has been updated (e.g. links, examples, etc.)
  • Code is ready for review

When the user denies an ASK confirmation, allow ConfirmResult.message to
override the fixed "Permission denied by user" tool result, and emit the
same ToolResultStart/TextDelta/End sequence that auto-deny already uses.

Fixes agentscope-ai#2492
@gemini-code-assist

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@CLAassistant

CLAassistant commented Jul 31, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.12195% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...n/java/io/agentscope/core/event/ConfirmResult.java 87.50% 1 Missing ⚠️
.../agentscope/harness/agent/tool/AgentSpawnTool.java 92.30% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

arimu1 added 3 commits July 31, 2026 23:34
Drop the 3-arg (confirmed, toolCall, message) constructor that
collided with (confirmed, toolCall, rules) when null was passed.
Use ConfirmResult.withMessage(...) or the 4-arg constructor instead.

Add HITL test that blank/whitespace deny message falls back to
"Permission denied by user", and note the API in permission docs.
HarnessAgentSubagentStreamEventsTest.streamEvents_childAgentStartAndEndEmittedWithSource
failed once on CI (no child AGENT_END with source) after an unrelated ConfirmResult API
commit. Full harness suite (699 tests) is green locally; prior tip fe03dad was green on CI.
execWithTimeoutPromotion bridges execLocalSync via CompletableFuture
and whenComplete, which can race doFinally: the parent streamEvents sink
may complete before the child AGENT_END is emitted, so CI flaked on
HarnessAgentSubagentStreamEventsTest (empty child AGENT_END with source).

Emit the bookend in flatMap (before the value reaches the bridge) and
keep doFinally only for CANCEL, with AtomicBoolean dedupe.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants