refactor: decouple ARGI runtime from model SDKs

[yuluo-yx]

upgrade.

[Sean-Walker0]

Review: decouple ARGI runtime from model SDKs

Thanks for this PR — the direction is right and the engineering quality is high. I reviewed the neutral API, all four adapters, the agent-framework runtime rework, serialization, and the build/CI changes against e10f9ab (base). Overall this is a well-executed architectural refactor with unusually good documentation and test investment, but I found one production-data compatibility gap that should be resolved (or explicitly documented) before merge, plus several items that deserve a decision. Verdict: Request Changes — mostly for the data-compat issue and CI workflow changes, not for the architecture.

What's done well

• The neutral API (argi-api-base) is compact and disciplined: immutable records with constructor-time validation (ChatRequest enforces unique tool names and REQUIRED/NAMED consistency; ContentPart defensively copies byte[] and implements value equality). ~1k lines, no over-engineering, JDK-only as claimed.
• Tool-execution boundary is clean. ReturnToolCallsAdvisor reserves Spring AI's single ToolAdvisor slot, disables auto-registration, and throws from ToolCallback.call() — three layers of defense ensuring only the ARGI runtime executes tools (backed by tests).
• check-sdk-boundaries.py is a real gate: it checks sources, bytecode constant pools (both a.b.C and a/b/C forms), POM declarations (including the local parent chain, with cycle detection), and the resolved runtime dependency tree. I verified argi-agent-framework, argi-graph-core, and argi-studio production sources contain zero provider-SDK references, with Spring AI deps all test-scoped.
• Multi-starter selection fails fast rather than guessing by configuration order (SpringAiChatClientAutoConfiguration).
• The ADK streaming state machine correctly handles both ADK event shapes (Gemini aggregate vs OpenAI-driver split commit), usage-only tail events, and partial tool args, and matches the documented capability matrix.
• The docs are refreshingly honest about limitations (Claude profile, fail-closed snapshots, removals without aliases).
• Real-model integration tests are env-gated (@EnabledForRealModel) with no credentials in source.

Major issues

1. Existing checkpoints are silently corrupted after upgrade — no migration path, no warning, no documented statement

Base's SpringAIJacksonStateSerializer used Id.CLASS + @class, so existing checkpoints embed @class: "org.springframework.ai.chat.messages.UserMessage" (and friends) in message history. This PR deletes that serializer and all std/*MessageSerializers (−2,231 lines) and replaces them with ArgiChatRecordSupport, which only understands the new ARGI_* record types. What actually happens when an old checkpoint is read (JacksonDeserializer.valueFromNode, the @class branch):

• Without Spring AI on the classpath: Class.forName fails, the exception is swallowed ("business payloads may use @Class as an ordinary key"), and the message degrades to a LinkedHashMap.
• With Spring AI on the classpath (via the adapter): the class resolves, but the old per-class handlers are gone, so deserialization also falls through to DEGRADE_TO_MAP.

The degraded maps then flow into Messageutils.convertMapToMessage, which has no role field to read and defaults everything to "user": old assistant/system messages silently become user messages, and old ToolResponseMessages (which have a responses structure, not text) come back null and are dropped entirely — with no exception and no log. For a HITL approval interrupted across an upgrade, the resumed conversation history is quietly rewritten: wrong roles, missing tool results. Two further hard-failure paths: ObjectStreamStateSerializer checkpoints fail with ClassNotFoundException/stream corruption (the old Class-object + custom payload layout has no reader), and old StreamingOutput entries fail StreamingOutputDeserializer's new requirement that messages exist and all be ChatMessage. Rollback is also broken: the new @type: ARGI_* markers are equally unreadable by the old version.

There is no legacy→neutral converter anywhere, and no test reads a legacy payload (SerializationUtilsChatRecordTest only covers the new format). Since JDBC/Redis/MongoDB persistence lives in the Extensions repo, every production persistence user hits this on upgrade. The 2.1.0-RC1 changelog provided migrateLegacyThread for a much smaller Studio change, so data migration is clearly within this project's standards. Suggested minimum: (a) recognize legacy @class: org.springframework.ai.chat.messages.* / old @type markers in valueFromNode and map them onto neutral ChatMessage (the old field layouts are fully defined in this PR's deleted handlers and can be ported cheaply), (b) at minimum WARN when degrading a Spring AI class name, (c) an explicit "checkpoint data is not upgrade-compatible" statement in the migration notes + an RC2 changelog (none exists).

Related: ArgiToolBindingStore.attach() persists the full tool schema (name/description/schema) into the metadata of every assistant message that has tool calls — this repeats the entire toolset in each turn's checkpoint entry and in Studio streaming. Consider storing tool names only, with a deduplicated schema table.

2. CI workflow: PR-specific trigger and removed repo guard are now shared-config liabilities

build-and-test.yml adds push: branches: ['feat/**'] and removes every if: (github.repository == 'agentic-ai-java/argi') guard:

• The feat/** trigger looks like a temporary hack to run CI on this PR's branch; after merge it means every feat/** branch push runs the full pipeline (now including pnpm build, -Prelease package, and the OSV network audit), and same-repo PRs will double-run (push + pull_request events with different concurrency groups).
• Removing the repo guard means fork PRs now execute PR-controlled code in the upstream org's runners: uses: ./tools/github-actions/setup-deps loads a composite action from the PR checkout, and make tools / make test / make api-contract-check run PR-author-controlled shell/Python. permissions: contents: read and the absence of secrets in this workflow keep the blast radius small, but it is still a supply-chain surface regression worth a deliberate decision (and CONTRIBUTING note) rather than a drive-by removal.

Suggest restoring the guard (at least for jobs that execute local actions/scripts) and dropping the feat/** trigger before merge.

3. Cross-adapter reasoning metadata is inconsistent — the neutral layer is missing the standard it exists to provide

Spring AI puts reasoning at message level under reasoningContent (SpringAiChatClientAdapter line ~97), while LangChain4j / Google ADK / AgentScope put it at response level under thinking (LangChain4jChatClientAdapter line ~253, GoogleAdkChatClientAdapter line ~291). Downstream consumers (Studio, token accounting, user code) must special-case per adapter — exactly what a neutral API should prevent. Please define a standard constant (e.g. ChatMetadataKeys.REASONING) in argi-api and normalize all adapters onto it (message level seems right, since it's per-turn content).

4. Strict "final text must prefix-match streamed deltas" validation is implemented in three layers and is fragile against provider normalization

LangChain4jChatClientAdapter (line ~130), GoogleAdkChatClientAdapter (StreamingTurn, line ~379), and AgentChatProtocol.stream (line ~74) all hard-fail when the aggregate text doesn't start with the concatenation of emitted deltas. Any provider-side normalization (trailing-whitespace trimming, whitespace collapsing — not rare behind OpenAI-compatible gateways) kills an otherwise successful stream with IllegalStateException. The invariant is worth having; consider emitting a corrective delta + WARN instead of hard failure, at least in the runtime-level check in AgentChatProtocol.

5. OSV dependency audit is fail-closed with no suppression mechanism, and UNKNOWN severity blocks

tools/scripts/audit-runtime-dependencies.py treats HIGH/CRITICAL/UNKNOWN advisories across the full transitive runtime tree (including optional drivers) as hard failures in both the PR build and the release workflow. Maven ecosystem OSV entries frequently lack database_specific.severity (→ UNKNOWN → block), and a single new advisory anywhere in the tree red-lights all PRs and releases simultaneously with no allowlist/bypass. 429s get one retry. If this is meant as a gate, please pair it with: an expiry-dated suppression list (with issue links), optional=true handling, exponential backoff, and possibly report-only on PRs / blocking only on release.

6. Dynamic tool-binding snapshots fail closed across process restarts — please document the operational consequence

AgentToolBindingStore keeps interceptor-selected (dynamic) bindings only in a process-local Caffeine cache (4096 entries). After a process restart or agent rebuild, resuming a checkpointed turn whose tools were dynamically selected throws IllegalStateException ("Tool binding snapshot is unavailable..."). Fail-closed is the right security default (never execute tools that weren't advertised), but the human-in-the-loop + rolling-restart combination is precisely this framework's flagship scenario. At minimum this deserves a prominent ops note; ideally the dynamic selection would be replayable (persist enough to reconstruct it through the interceptor chain).

7. Cross-repo compatibility gates removed; tools/compatibility/policy.yaml is now dead config

The compatibility wiring/binary/source verification scripts and their ~490 lines of tests are deleted, and policy.yaml's extensions_repository / extensions_refs fields now have zero consumers. Downstream breakage (Extensions/Examples) therefore has no automated verification at all — only the migration text in argi-api/README.md. Please either clean up the dead fields or add a lightweight downstream-compile smoke job against migration branches of Extensions/Examples, and open the tracking issues in those repos before this merges.

Minor issues

• ToolChoice.NONE semantic drift (Spring AI adapter): NONE is implemented by sending no tool schemas (requestSpec, line ~113), i.e. the model can't even see the tools — OpenAI's none still advertises them. ADK sends mode=NONE. Cross-adapter inconsistency; pick one semantic and document it.
• AgentToolNode.neutralInvocationExecutor (one pool per node, ArrayBlockingQueue(maxParallelTools)) is never shut down; daemon threads + core-thread timeout make this survivable, and queue overflow degrades to RejectedExecutionException → tool-error message rather than deadlock — but that degradation path deserves a comment or doc.
• Spring AI adapter streaming: response.messages().get(0) (line ~97) can throw IndexOutOfBoundsException if a chunk has results but a null generation output — defensive isEmpty() check suggested.
• ADK adapter toResponse throws UnsupportedOperationException on any unrecognized part (e.g. inlineData, code-execution results, line ~277). A model returning such a part fails the whole turn; consider skipping with a WARN for parts the profile doesn't claim.
• AgentScope Turn.response() computes totalTokens = input + output, ignoring the native total (wrong for providers that include cached/thought tokens in the total).
• Messageutils.convertMapToMessage doesn't recognize a "tool" role (or toolCallId/toolName) even for newly produced map-form messages — worth fixing together with the legacy-conversion work in item 1.
• Studio StreamingMessages.split copies the same tokenUsage reference into every SSE event of a multi-message turn; front-ends that accumulate will double-count. Consider attaching usage to the last event only. (Also note the SSE field rename promptTokens/completionTokens → inputTokens/outputTokens/totalTokens requires front/back-end release coordination.)
• All adapters synthesize random UUIDs for blank tool-call ids. Self-consistent within a turn, but non-deterministic across checkpoint replays and test assertions; a deterministic scheme (name+index) would be kinder.
• AgentToolNode.java:196 error message still says "ToolResponseMessage" (a Spring AI class name).
• json-schema-validator exists at two major versions (3.0.1 managed/compile in agent-framework; 1.5.7 explicitly pinned test-scope in studio). Intentional per the comment, but please record the reason in the root POM or unify.
• argi-bom now pins third-party patches (Jackson 2/3, Bouncy Castle, jsoup, Tomcat embed). BOM import order means downstream apps importing argi-bom first get these pins overriding their own Boot-managed versions — a policy worth stating loudly in the BOM description/README, and worth double-checking Tomcat 11.0.26 / Jackson 3.1.7 against what Boot 4.1.1 manages (to avoid a silent downgrade).
• make api-contract-check in the test job triggers a second full mvnw install after make test — roughly doubles CI time; consider reusing artifacts.
• Version declarations are inconsistent across modules (${project.parent.version} vs ${revision} vs managed no-version); and the adapter SDK versions (langchain4j 1.22.0 / adk 1.11.0 / agentscope 2.0.4) are not in the root dependencyManagement or the BOM, so consumers can't align them via the BOM.
• Missing changelogs/2.1.0-RC2.md for a change of this size — the removal list currently lives only in argi-api/README.md.

Compatibility surface (for the record)

I confirmed the following are removed without deprecated aliases (as documented): Builder.model(ChatModel), chatOptions, tools(ToolCallback...), methodTools, toolCallbackProviders, toolNames, resolver, toolExecutionExceptionProcessor, templateRenderer(TemplateRenderer), customObservationConvention, advisorObservationConvention, stateSerializer(SpringAIStateSerializer), wrapSyncToolsAsAsync; classes AsyncToolCallback{,Adapter}, CancellableAsyncToolCallback, StateAwareToolCallback, ToolCallbackUtils, SpringAiSkillAdvisor, SkillPromptAugmentAdvisor, AgentTool.getFunctionToolCallback; and the spring-boot-starters/ → argi-spring-boot-starter/ directory move breaks -pl path selection (coordinates unchanged). throwOnToolError now defaults to false (exceptions become tool-error messages). The README declares "direct replacement, not a source or binary compatibility promise" — that's a defensible call, but at version 2.1.0-RC2-SNAPSHOT it reads as a patch-level RC carrying a wholesale breaking replacement. Please clarify the versioning strategy (this arguably warrants 2.2.0 or 3.0.0, plus a migration guide entry in the changelog).

Not covered by this review

I read the core paths line-by-line, but only spot-checked: the ~20 individual model/tool interceptors (retry, fallback, PII, summarization, tool-emulator, …), Studio DTO/frontend changes, TokenCounter, the benchmark submodule pointer bump (content lives out-of-tree), and the A2A node.

Suggested pre-merge checklist

1. Decide + act on checkpoint data compatibility (converter or explicit docs) — blocking in my view.
2. Restore CI repo guards / remove feat/** push trigger; document the fork-execution decision.
3. Add a suppression/expiry mechanism (or report-only mode on PRs) to the OSV audit gate.
4. Unify reasoning metadata keys across adapters.
5. Relax or de-duplicate the triple prefix-match validation.
6. Clean up or re-wire tools/compatibility/policy.yaml; open Extension/Examples migration tracking issues.
7. Add an RC2 changelog and expand the PR description (a 535-file refactor deserves more than "upgrade.").

Happy to help follow up on any of these — the overall architecture is solid and the work is much appreciated.