Skip to content

feat(agent): add registered business outcome mapping and outcome consistency fixes (T38) - #167

Merged
yuluo-yx merged 1 commit into
agentic-ai-java:mainfrom
konlue:feat/t38-business-outcome-mapper
Oct 15, 2026
Merged

yuluo-yx merged 1 commit into
agentic-ai-java:mainfrom
konlue:feat/t38-business-outcome-mapper

Conversation

@konlue

@konlue konlue commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Describe what this PR does / why we need it

Second and closing part of T38 (#141), following the "keep it simple" agreement: a registered business-outcome mapper is now the only component allowed to reinterpret a successful raw tool result as a business failure (no global string guessing), the reserved ToolOutcome.BUSINESS_FAILED vocabulary gets its first producer, and three outcome-consistency gaps found along the way are closed.

Does this pull request fix one issue?

Fixes #141

Describe how you did it

  • BusinessOutcomeMapper (functional interface) + BusinessOutcomeMapperRegistry mirroring ToolEffectRegistry: builder().declare(toolName, mapper), fromTools(...) picking up BusinessOutcomeAware tools, merge(). The default registry is empty and results of unregistered tools are never inspected (a test feeds a {"error": true} payload to an unregistered tool and asserts it stays a plain success).
  • The registry is applied at the single point between tool execution and ToolCallResponse.from(...): when a registered mapper marks a successful raw result as error, the response is built with status ToolOutcome.BUSINESS_FAILED — the vocabulary reserved in PR feat(agent): unify tool failure status contract and remove null-status error paths (T38) #159 now has its first producer.
  • isError() recognizes BUSINESS_FAILED as a failure, so the model-visible message derived from the response carries tool.error=true — conversions no longer lose the outcome (one of the issue's acceptance criteria).
  • LargeResultEvictionInterceptor keeps the original response metadata instead of dropping it when evicting a large result.
  • ToolEmulatorInterceptor returns success(...) instead of the legacy of(...): an emulated result is a successful outcome and must not look like a failure to retry interceptors.
  • ToolRetryInterceptor's exhausted-retries exit carries forward the last failure's structured failureKind/exceptionType entries.

Describe how to verify it

  • New tests: BusinessOutcomeMapperRegistryTest (6 cases: no-guessing, declare, fromTools, merge, null-mapper rejection, argument validation), AgentToolNodeBusinessOutcomeTest (4 cases run end to end through the node, asserting BUSINESS_FAILED + metadata + model-visible tool.error), LargeResultEvictionInterceptorTest (metadata preservation); plus one new retry case and one emulator status assertion.

  • Run:

    ./mvnw -pl argi-agent-framework -am test -DskipTests=false \
      -Dtest='BusinessOutcomeMapperRegistryTest,AgentToolNodeBusinessOutcomeTest,ToolRetryInterceptorUnitTest,ToolEmulatorInterceptorTests,LargeResultEvictionInterceptorTest' \
      -Dsurefire.failIfNoSpecifiedTests=false
  • Full regression: graph-core 701 + agent-framework 1122 tests; only the known pre-existing Windows-environment failures remain (the FileSystem* symlink / virtual-root family and the skills/prompt-template cases reproduce on clean main, as documented in feat(agent): unify tool failure status contract and remove null-status error paths (T38) #159).

Special notes for reviews

  • Behavior changes: emulated results now carry SUCCESS_STATUS (previously no status, so retry interceptors retried them as failures), and isError() returns true for BUSINESS_FAILED.
  • BUSINESS_FAILED responses are not SUCCESS_STATUS, so retry interceptors retry them like other failures; if a business failure should be terminal for the retry policy, the mapper can keep returning a success result. Happy to adjust.
  • "evicted_to_filesystem" intentionally stays outside the ToolOutcome vocabulary (it is a success variant, not a failure); let me know if you would rather fold it in or map it to SUCCESS.
  • MCP adapter error mapping into this protocol is untouched (the adapter modules are new since the refactor) and can follow up separately.

@yuluo-yx
yuluo-yx merged commit dfb1e0f into agentic-ai-java:main Oct 15, 2026
7 checks passed
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.

[P1][T38] 统一 ToolOutcome 状态、业务失败与模型可见错误协议

2 participants