Skip to content

fix(aistio): rename task.submit_result tool to satisfy OpenAI function-name rules #3151

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jinison wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(aistio): rename task.submit_result tool to satisfy OpenAI function-name rules #3151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0386717
fix(aistio): rename task.submit_result tool to satisfy OpenAI functio…
Jinison Sep 15, 2026
c972c1e
test(aistio): cover the renamed task_submit_result tool name
Jinison Sep 15, 2026
fe5e796
Merge branch 'main' into fix/aistio-task-outcome-tool-name
Jinison Sep 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ns-aistio/src/main/java/io/agentscope/extensions/aistio/adapter/AgentTaskOutcomeTool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
/** Captures intent. Only the adapter is allowed to commit the physical AgentTask lifecycle. */
public final class AgentTaskOutcomeTool {
@Tool(
name = "task.submit_result",
name = "task_submit_result",

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] The registered name and the prompt/guard references are now the same string literal repeated in 5 places across two files (HarnessAgentTaskStarter role instructions, the "call task_submit_result" nudge, the retry message and the getToolNames().contains("task_submit_result") guard). Any future rename has to touch all of them, and a partial update fails silently: the registration would succeed while the guard re-registers a duplicate tool, or the model would be told to call a name that is not registered.

Could you hoist it into a constant and reference it everywhere?

public final class AgentTaskOutcomeTool {
    /** Registered tool name; must match `^[a-zA-Z0-9_-]+$` (OpenAI-compatible function-name rule). */
    public static final String TOOL_NAME = "task_submit_result";

    @Tool(name = TOOL_NAME, description = "...")

and in the adapter: if (!runtimeAgent.getToolkit().getToolNames().contains(AgentTaskOutcomeTool.TOOL_NAME)). Annotation values must be compile-time constants, which a static final String literal satisfies.

@oss-maintainer oss-maintainer Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Compatibility of the rename for already-persisted sessions. ToolUseBlock keeps the tool name inside the message payload, and JsonAgentStateStore/JsonFileAgentStateStore persist those messages, so a session created before this rename still replays history containing task.submit_result. For the same OpenAI-compatible endpoint that rejects the definition with HTTP 400, the tools[i].function.name pattern check is a request-level validation, so resuming such a session can keep failing even after this fix lands.

Could you confirm the behaviour on a resumed session with pre-rename history? If it does fail, the options are a read-side rewrite when loading legacy sessions (map the old name to the new one), or stating explicitly in the changelog that in-flight AgentTasks must be re-dispatched rather than resumed.

description =
"Submit the actual outcome of this AgentTask, then end your turn. succeeded"
+ " requires the full deliverable in result, not a plan or promise. waiting"
Expand Down
10 changes: 5 additions & 5 deletions ...aistio/src/main/java/io/agentscope/extensions/aistio/adapter/HarnessAgentTaskStarter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private void execute(AgentTaskAssignment assignment) {
+ " available CollaborationClient actions are registered as tools with"
+ " the exact names shown in availableActions. The adapter owns"
+ " task.complete and task.fail; do not call them. Before returning,"
+ " call task.submit_result with an explicit business outcome and the"
+ " call task_submit_result with an explicit business outcome and the"

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] This is exactly the line where the dotted-name problem survives the rename: task.complete, task.fail, run.node.complete, run.node.fail, run.replan, issue.child.create are all still spelled with dots here and in roleInstructions (below), and a name containing . is rejected by OpenAI-compatible /chat/completions with the same HTTP 400 you are fixing here (^[a-zA-Z0-9_-]+$) as soon as such a tool is actually registered.

Today the first two are prohibitions ("do not call them") so no request payload is generated for them, which is why the PR is enough to unblock the reported failure — but the text still teaches the model a name shape that cannot be called, and the same block says the collaboration tools are registered "with the exact names shown in availableActions", i.e. control-plane supplied. Two questions:

  1. Are any availableActions names (e.g. issue.child.create) registered as tools verbatim? If yes, the 400 will come back for leader turns that delegate.
  2. Is there a plan to align the Go control plane (you flagged agentscope-service/aistio/internal/runtimebinding/resolver.go)? A one-line follow-up issue would be helpful so the prompt strings and the registered names do not diverge per-repo.

Optional hardening that would make the class of bug impossible: validate every name at registration time in the core toolkit, e.g. throw/warn when a tool name does not match ^[a-zA-Z0-9_-]{1,64}$, instead of discovering it at the first model request.

@oss-maintainer oss-maintainer Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is the line where the dotted-name problem survives the rename: the model is still told about task.complete, task.fail, run.node.complete, run.node.fail and run.replan, none of which are registered under those names by this module. The intent here is "do not call these", which is fine in prose, but a model that tries one of them gets an unknown-tool error instead of a clean validation message.

task.submit_result is the only dotted @Tool name in the whole repository (I grepped), so this PR does fix the only real blocker. For consistency I would phrase the negative list as capabilities the adapter owns ("completing and failing the task is owned by the adapter; never attempt to call a task-completion tool yourself") rather than by name, or state the [a-zA-Z0-9_-]+ convention once so future tool authors do not reintroduce a dotted name.

+ " actual deliverable. A promise to do work later is not completion."
+ " Check the tool capabilities before delegating: spawning a subagent"
+ " does not add missing web access."
Expand Down Expand Up @@ -259,7 +259,7 @@ private AgentTaskOutcome runToOutcome(
next =
message(
"The turn ended without a business outcome. Do the remaining work,"
+ " or call task.submit_result with blocked and the concrete"
+ " or call task_submit_result with blocked and the concrete"
+ " missing capability. Do not submit a plan or waiting promise"
+ " as successful research.");
continue;
Expand Down Expand Up @@ -433,7 +433,7 @@ private void registerCollaborationTools(
Set<String> availableActions) {
Object toolkit = runtimeAgent.getToolkit();
synchronized (toolkit) {
if (!runtimeAgent.getToolkit().getToolNames().contains("task.submit_result")) {
if (!runtimeAgent.getToolkit().getToolNames().contains("task_submit_result")) {

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Warning] Compatibility check for the rename. The tool name is part of what gets persisted in agent session/memory state and replayed on recovery: a session recorded before the upgrade contains a task.submit_result tool-use block, while after the upgrade the toolkit only exposes task_submit_result. Could you confirm how state recovery / history replay handles a tool-use block whose name is no longer registered (ignored, error, or fail-fast)? If it is a hard error, in-flight AgentTasks resumed after an upgrade would break.

If a transition is needed, the usual cheap pattern is to keep a hidden alias for one release:

runtimeAgent.getToolkit().registerTool(new AgentTaskOutcomeTool());      // task_submit_result
runtimeAgent.getToolkit().registerAlias("task.submit_result", AgentTaskOutcomeTool.TOOL_NAME); // deprecated, one release

(if the toolkit has no alias support, documenting "restart in-flight AgentTasks after upgrading" in the PR description is enough — the repo has no versioned-upgrade note for extension tool names that I can find.)

@oss-maintainer oss-maintainer Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The registered name is now the same literal repeated in 6 places across two files (this guard, the @Tool(name=...), and 4 prompt strings); only two of them are pinned by string-equality assertions in AgentTaskCollaborationToolTest, so a future rename can still drift silently.

Suggested follow-up (no need to block this PR on it):

public static final String SUBMIT_RESULT_TOOL = "task_submit_result";

...and build the prompt sentences with that constant (or a text...formatted(...) call) so registration, the guard and the instructions cannot disagree.

runtimeAgent.getToolkit().registerTool(new AgentTaskOutcomeTool());
}
for (JsonNode definition :
Expand Down Expand Up @@ -466,12 +466,12 @@ static String roleInstructions(JsonNode envelope, List<String> inputIds) {
return " You are a Team worker, not its coordinator. Do not create or accept child"
+ " Issues and do not call run.node.complete, run.node.fail, or run.replan."
+ " Complete only the assigned work and submit its result using"
+ " task.submit_result; the adapter will complete this AgentTask.";
+ " task_submit_result; the adapter will complete this AgentTask.";

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Info] Nit: these two prompt rewrites now break mid-word (using + task_submit_result; and calling + task_submit_result with waiting). Spotless keeps the line split, but since the new name is longer than the old one it is worth a manual pass over the rendered string to make sure no doubled/missing space slipped in — the concatenated literal is what the model actually sees, and it is not covered by any assertion. A tiny test like assertTrue(roleInstructions(...).contains(" task_submit_result ")) (or a snapshot of the worker/leader instructions) would lock the prompt text cheaply.

}
if (inputIds.isEmpty()) {
return " You are the Team leader's initial task. If you delegate child work, return"
+ " immediately after issue.child.create succeeds by calling"
+ " task.submit_result with waiting, a reason and the returned AgentTask"
+ " task_submit_result with waiting, a reason and the returned AgentTask"
+ " IDs; do not wait through local session/task tools and do not call"
+ " run.node.complete yet. The control plane will deliver a fresh leader"
+ " follow-up when a worker result arrives. If no work is delegated, call"
Expand Down
6 changes: 6 additions & 0 deletions ...src/test/java/io/agentscope/extensions/aistio/adapter/AgentTaskCollaborationToolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,15 @@ void roleInstructionsSeparateLeaderHandoffFromWorkerExecution() throws Exception
assertTrue(
HarnessAgentTaskStarter.roleInstructions(worker, List.of())
.contains("Team worker, not its coordinator"));
assertTrue(
HarnessAgentTaskStarter.roleInstructions(worker, List.of())
.contains("task_submit_result"));
assertTrue(
HarnessAgentTaskStarter.roleInstructions(leader, List.of())
.contains("return immediately after issue.child.create succeeds"));
assertTrue(
HarnessAgentTaskStarter.roleInstructions(leader, List.of())
.contains("task_submit_result with waiting"));
String followUp = HarnessAgentTaskStarter.roleInstructions(leader, List.of("input-1"));
assertTrue(followUp.contains("leader follow-up"));
assertTrue(followUp.contains("Never send those mutations in parallel"));
Expand Down
48 changes: 48 additions & 0 deletions ...io/src/test/java/io/agentscope/extensions/aistio/adapter/HarnessAgentTaskOutcomeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,54 @@ private HarnessAgentTaskStarter starter() throws Exception {
return new HarnessAgentTaskStarter(() -> agent, client);
}

@Test

@oss-maintainer oss-maintainer Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice addition — the second dispatch does exercise the "already registered" branch of registerCollaborationTools, which the previous patch-coverage gap pointed at. Strictly speaking the assertions (times(2) on call/finish) prove "a second dispatch still works", not "registered only once".

To assert the actual invariant, something like this is cheap:

verify(agent.getToolkit(), org.mockito.Mockito.never())
      .registerTool(org.mockito.ArgumentMatchers.any(AgentTaskOutcomeTool.class));
// or: assertEquals(1, countOf(agent, AgentTaskOutcomeTool.class))

Also consider a small factory for the 12-argument AgentTaskAssignment since it is now duplicated verbatim apart from attemptId.

void sameToolkitRegistersOutcomeToolOnlyOnce() throws Exception {
var starter = starter();
when(agent.call(any(Msg.class), any(RuntimeContext.class)))
.thenAnswer(
invocation -> {
RuntimeContext ctx = invocation.getArgument(1);
ctx.get(AgentTaskOutcome.State.class)
.submit(
new AgentTaskOutcome(
"succeeded", "delivered", "", List.of()));
return Mono.just(
Msg.builder()
.role(MsgRole.ASSISTANT)
.textContent("done")
.build());
});
starter.start(assignment).block();
// A second dispatch on the same toolkit must hit the "already registered" branch of
// registerCollaborationTools and still complete normally.
AgentTaskAssignment second =

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Info] Nit: the 12-argument AgentTaskAssignment is now duplicated verbatim except for attemptId. Since assignment is already a field, something like assignment.withAttempt("attempt-2") (or a one-line local helper) keeps a future record-shape change from breaking this test in two places instead of one.

new AgentTaskAssignment(
"attempt-2",
"task",
"run",
"node",
1,
"dispatch",
"",
"secret-token",
"attempt-secret",
"assigned-session",
new byte[0],
1);
starter.start(second).block();
verify(agent, times(2)).call(any(Msg.class), any(RuntimeContext.class));

@oss-maintainer oss-maintainer Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Info] The test name says "OnlyOnce", but nothing here asserts "only once" — verify(agent, times(2)).call(...) plus verify(client, times(2)).finish(...) only prove both dispatches completed. starter() already stubs agent.getToolkit() to return one shared Toolkit instance, so the stronger assertion is cheap, e.g. wrap it in a spy and verify(spyToolkit, times(1)).registerTool(any(AgentTaskOutcomeTool.class)), or snapshot getToolNames() before/after the second start(...). As written this closes the Codecov branch-coverage gap (which is what the commit set out to do), but a future regression that re-registers the tool and silently replaces the first instance would still pass.

verify(client, times(2))
.finish(
eq("task"),
eq("secret-token"),
eq(4L),
eq("succeeded"),
eq(""),
eq("delivered"),
eq(List.of()),
eq(List.of()));
}

@Test
void plainWaitingPromiseCannotBecomeSuccessfulBusinessCompletion() throws Exception {
var starter = starter();
Expand Down
Loading