Skip to content

fix(agents): harden update_agent null-like args#2945

Closed
Eilen6316 wants to merge 1 commit into
bytedance:mainfrom
Eilen6316:fix-issue-2906-update-agent-null-args
Closed

fix(agents): harden update_agent null-like args#2945
Eilen6316 wants to merge 1 commit into
bytedance:mainfrom
Eilen6316:fix-issue-2906-update-agent-null-args

Conversation

@Eilen6316
Copy link
Copy Markdown
Contributor

Summary

  • normalize literal null-like strings for optional update_agent fields so they behave like omitted fields
  • keep skills/tool_groups schema strict as arrays while accepting null-like placeholders as omitted
  • mark update_agent validation errors as error ToolMessages and clarify the self-update prompt

Tests

  • uv run pytest tests/test_update_agent_tool.py tests/test_lead_agent_prompt.py::test_build_self_update_section_present_for_custom_agent tests/test_tool_args_schema_no_pydantic_warning.py

Closes #2906

Copy link
Copy Markdown
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

This PR hardens the custom agent self-update flow so null-like string placeholders are treated as omitted optional fields and tool errors are surfaced more clearly.

Changes:

  • Adds explicit UpdateAgentArgs schema with null-like normalization.
  • Marks update_agent error responses as error ToolMessages.
  • Updates the self-update prompt and tests for null-like placeholder handling.

Reviewed changes

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

File Description
backend/packages/harness/deerflow/tools/builtins/update_agent_tool.py Adds argument schema, null-like normalization, and error ToolMessage status.
backend/packages/harness/deerflow/agents/lead_agent/prompt.py Clarifies that agents should omit unchanged fields instead of passing null-like strings.
backend/tests/test_update_agent_tool.py Adds regression coverage for null-like field handling and error status.
backend/tests/test_lead_agent_prompt.py Verifies the self-update prompt includes null-like string guidance.

@WillemJiang
Copy link
Copy Markdown
Collaborator

@Eilen6316, thanks for your contribution. Please take a look at the lint and unit test error.

@Eilen6316 Eilen6316 force-pushed the fix-issue-2906-update-agent-null-args branch from 3bb4b23 to 4567e1f Compare May 15, 2026 02:36
@Eilen6316
Copy link
Copy Markdown
Contributor Author

@Eilen6316, thanks for your contribution. Please take a look at the lint and unit test error.

Thanks for the review. I fixed both issues: the e2e failure was caused by the custom args_schema breaking ToolRuntime injection, so I switched to parameter-level BeforeValidator instead.

@WillemJiang
Copy link
Copy Markdown
Collaborator

WillemJiang commented May 16, 2026

@Eilen6316, thanks for your contribution. Here are some suggestions for code

  1. Redundant normalization for text fields. The BeforeValidator(_normalize_nullish_string) on OptionalText converts null-like strings to None at validation time. Then _normalize_optional_text() is called again inside the function body — on values that are already None. This is harmless but redundant for the production path (.invoke()). Please document why both layers exist, or keep only the one that matters for the primary call path.
  2. Inconsistent test strategies. test_update_agent_treats_nullish_optional_text_as_omitted uses .func() (bypasses Pydantic, tests body-level normalization), while test_update_agent_treats_nullish_string_list_fields_as_omitted uses .invoke() (tests BeforeValidator). It would be clearer to test both field types through the same call path, preferably .invoke() since that's the production path.
  3. _normalize_optional_text is effectively dead code in production. Since .invoke() triggers BeforeValidator, the explicit calls at lines ~129-131 only matter for .func() direct calls. If the goal is defense-in-depth, a short comment would help future readers understand why both layers exist.

@Eilen6316 Eilen6316 force-pushed the fix-issue-2906-update-agent-null-args branch from 4567e1f to 7eb9b0e Compare May 26, 2026 07:29
@Eilen6316
Copy link
Copy Markdown
Contributor Author

Eilen6316 commented May 26, 2026

Thanks for the review. I updated the PR accordingly.

  • Removed the body-level _normalize_optional_text() path so production behavior relies on the Pydantic BeforeValidator only.
  • Updated the optional text null-like test to use .invoke(), matching the list-field test and the production call path.
  • Rebased the branch onto the latest origin/main.

@Eilen6316
Copy link
Copy Markdown
Contributor Author

Update: this PR has been rebased onto the latest main and the review feedback has been addressed. Current base is f68bcb77, current head is 7eb9b0ef.

@Eilen6316
Copy link
Copy Markdown
Contributor Author

Closing this older PR because I reopened the same fix on a fresh PR: #3237.

#3237 is based on the latest main, includes the review follow-up from this PR, and keeps the same issue linkage for #2906.

@Eilen6316 Eilen6316 closed this May 26, 2026
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]Agent 更新失败 Repeated tool calls exceeded the safety limit

3 participants