Repository navigation
Conversation
The base starter pulls the shaded all-in-one artifact into consumer applications. Adding an individual extension then introduces duplicate classes that can shadow the intended extension implementation. Make agentscope-core a normal transitive dependency of the base starter and remove the all-in-one dependency. Make the AG-UI starter transitively provide its own extension module so it remains usable on its own. Add downstream Spring Boot executable-JAR regression coverage for the base starter, an explicitly added AG-UI extension, and the AG-UI starter. Verify class resource uniqueness, absence of unsolicited model extensions, and automatic creation of the agent and AG-UI registry beans. Isolate build outputs for the three dependency scenarios. Include the fixture in Spotless checks and document the dependency migration in EN/ZH. Validation on upstream main 97696a3: - The original packaged base-plus-AG-UI consumer contained 109 duplicate AgentScope class names and failed the AguiAgentAdapter uniqueness check. - The fixed versions of all three packaged consumers passed; a full scan of their nested libraries found zero duplicate AgentScope class names. - 101 focused tests passed across the base, AG-UI and five model starters. - The selected 19-module Maven verify reactor and Spotless checks passed. Local tests preloaded Mockito as a JVM agent because dynamic attachment is unavailable in this environment. Javadoc generation was skipped; this validation does not claim to run the full repository test suite. Fixes agentscope-ai#3083
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
CLA Not Signed The Contributor License Agreement (CLA) check is currently pending on this PR ( @ZYZ666-RGB please sign the CLA via the CLA assistant badge in the comment above, or visit https://cla-assistant.io/agentscope-ai/agentscope-java. Once signed, the Automated check by github-manager-bot |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
fix: add starter-runtime to BOM
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
oss-maintainer
left a comment
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Dependency-scope fix for #3083: agentscope-spring-boot-starter stops pulling the shaded io.agentscope:agentscope aggregate and exposes agentscope-core as a normal transitive dependency, and agentscope-agui-spring-boot-starter now ships agentscope-extensions-agui in compile scope so the AG-UI starter actually provides the extension it configures.
The last commit (d2bf2212) resolves everything from the previous round: the io.agentscope:starter-runtime entry is out of agentscope-bom, the Maven Invoker fixture and its plugin wiring are gone, and Check Module Sync, build (ubuntu-latest), build (windows-latest) and codecov are all green on the current HEAD. I also verified there is no other dependency left on the aggregate inside agentscope-extensions/agentscope-spring-boot-starters/**, and that the base starter compiles against core alone.
No correctness problem with the change itself. Two follow-up requests before this is merge-ready from my side, plus one nit:
- [Warning] Removing the aggregate also removes the extension classes starter consumers used to receive transitively — that is a user-visible runtime break, and
docs/v1/{en,zh}/docs/quickstart/installation.mdstill recommends the all-in-one artifact with no "do not combine with individual modules / starters" caveat. The PR checklist also still claims the EN/ZH docs were updated, which the final diff does not reflect. - [Warning] Nothing in CI pins the new invariant (a starter must not depend on
io.agentscope:agentscope); a dependency-tree or enforcerbannedDependenciescheck would keep #3083 from coming back. - [Info]
spring-boot.versionis pinned to4.0.1here while the starters parent declares4.0.3.
Overall: correct, minimal and in the right place — please add the migration note and I am happy with the rest.
Automated review by github-manager-bot
|
|
||
| <dependencies> | ||
| <!-- AgentScope Core --> | ||
| <!-- Use the modular core; the all-in-one JAR also embeds optional extension classes. --> |
There was a problem hiding this comment.
[Warning] Removing the all-in-one JAR here does more than fix the duplicate-class shadowing in #3083 — it also silently removes every extension class that starter consumers used to get transitively, because agentscope-all shades io.agentscope:agentscope-core plus all io.agentscope:agentscope-extensions-* (see the maven-shade-plugin artifactSet in agentscope-distribution/agentscope-all/pom.xml). An app that declared only agentscope-spring-boot-starter and then used e.g. io.agentscope.extensions.model.dashscope.DashScopeChatModel resolved that class before this change and will now hit NoClassDefFoundError at runtime.
The compile-time side is fine — I checked AgentscopeAutoConfiguration and the properties classes: they only import io.agentscope.core.* (and @ConditionalOnClass(ReActAgent.class)), so the base starter itself needs nothing but core.
Could you make the migration user-visible instead of keeping it only in the PR description?
docs/v1/en/docs/quickstart/installation.md(and the ZH twin) still presents all-in-one as "Recommended" and never warns that it must not be combined withagentscope-spring-boot-starteror with individual modules — which is exactly the trap that produced this bug.- The PR checklist ticks "Related EN/ZH documentation updated", but the final diff contains no documentation change, so that box no longer matches the code.
A short note in the installation docs plus a release-notes entry looks sufficient. The code itself reads correct to me — with that doc gap closed this is good to go from my side.
| <!-- Use the modular core; the all-in-one JAR also embeds optional extension classes. --> | ||
| <dependency> | ||
| <groupId>io.agentscope</groupId> | ||
| <artifactId>agentscope-core</artifactId> |
There was a problem hiding this comment.
[Info] This artifact is now the transitive supplier of agentscope-core for every sibling starter: a2a, agui, chat-completions-web and nacos still declare agentscope-core as provided + optional and compile against it directly, which is fine as compile-time intent, but they now depend on this line to actually get core at runtime.
Two small clean-ups while this area is open:
- This pom pins
spring-boot.version=4.0.1(line 34) while the parentagentscope-spring-boot-starters/pom.xmldeclares4.0.3. The override is invisible today becausespring-boot-autoconfigureisoptional, but the drift makes "which Spring Boot version does the base starter actually ship against?" unanswerable — worth aligning or deleting the property. - Consider a one-line comment here stating that the base starter is the transitive source of core for the sibling starters, so nobody re-adds
provided/optionalhere later.
| @@ -46,8 +46,6 @@ | |||
| <dependency> | |||
| <groupId>io.agentscope</groupId> | |||
| <artifactId>agentscope-extensions-agui</artifactId> | |||
There was a problem hiding this comment.
[Warning] Making agentscope-extensions-agui a regular compile dependency is what makes the fix complete, and dropping src/it/** plus the io.agentscope:starter-runtime BOM entry resolves the previous blocker cleanly — Check Module Sync, both build jobs and codecov are green on d2bf2212.
What remains uncovered is the regression risk: nothing in CI now guarantees the invariant this PR establishes (no Spring Boot starter may depend on the shaded aggregate), so a later pom.xml edit can reintroduce the duplicate classes and it will only surface at runtime inside a user's fat jar. A dependency-tree assertion is enough and avoids the reactor-module route that broke the sync check last time, e.g. a step that fails when dependency:tree for the starter modules still contains io.agentscope:agentscope:jar — or an enforcer bannedDependencies rule with that coordinate as the banned target.
SummaryDelta re-review at State check on the new head: Carried over from the last round (both still unaddressed): the EN/ZH installation docs still recommend the all-in-one artifact with no caveat about combining it with the starters (see inline), and no CI check pins the "starter must not depend on Overall: still correct, minimal and in the right place. Add the migration note and this is good to go from my side — CI being green on Automated review by github-manager-bot Findings (posting as a PR comment: review submission on this head is currently blocked by a lingering pending/draft review from a previous automated pass — sorry for the degraded format)[WARNING] Re-review gate check: this thread is still open. With the aggregate gone, consumers who followed docs/v1/{en,zh}/docs/quickstart/installation.md and add both [WARNING] Second open thread from |
oss-maintainer
left a comment
oss-maintainer
left a comment
There was a problem hiding this comment.
Summary
Delta re-review at ca91478f (previous pass was d2bf2212). The only new commits are a main sync merge — both PR-owned files (agentscope-spring-boot-starter/pom.xml, agentscope-agui-spring-boot-starter/pom.xml) are byte-identical to the reviewed state, so nothing in the actual fix changed. Good news: the branch is now conflict-free with main@6c8ccc5, and Check License / Check Module Sync / build (ubuntu-latest) / build (windows-latest) / codecov are all green on the new head, with license/cla signed.
The two follow-up items from the previous round are still open, so this stays a comment rather than an approval:
Findings
- [Warning]
docs/v1/en|zh/docs/quickstart/installation.mdstill present the all-in-oneio.agentscope:agentscopeaggregate with no caveat that it must not be combined with individual extension modules or starters. Since this PR removes that transitive aggregate from the base starter, users following the doc can still hit the #3083 classpath shadowing — please add the migration note (and align the PR checklist, which currently claims the EN/ZH docs were updated). - [Warning] Nothing in CI pins the new invariant that
agentscope-spring-boot-starters/**must not depend onio.agentscope:agentscope. Amaven-enforcerbannedDependenciesrule (or the existingcheck-shade-and-bom-sync.shscript extended with this check) would keep #3083 from regressing.
Verdict
The dependency fix itself remains correct, minimal, and verified against the freshly merged main (all builds green). Address the docs migration note — the CI guard is a nice-to-have — and this is good to go from my side.
Automated review by github-manager-bot
Fixes #3083.
Problem
agentscope-spring-boot-starterdepended on the shaded aggregate jario.agentscope:agentscopeas a normal compile dependency. That jar flattensagentscope-coreand everyagentscope-extensions-*module into a single artifact with no package relocation. When a user also declares a specific extension module (e.g.agentscope-extensions-agui), the same fully-qualified classes appear twice on the classpath. In a Spring Boot fat jar,BOOT-INF/lib/loads jars alphabetically, soagentscope-*.jarwins overagentscope-extensions-agui-*.jar— silently shadowing the user-intended version.Fix
Replace the aggregate dependency with direct module dependencies:
agentscope-spring-boot-starter: remove theio.agentscope:agentscopeshaded aggregate; changeagentscope-corefromprovided/optionalto a normal compile dependency so it transits to consumers.agentscope-agui-spring-boot-starter: changeagentscope-extensions-aguifromprovided/optionalto a normal compile dependency so the AG-UI starter transitively provides the extension.The aggregate jar was the runtime provider of all classes via shading. With it removed, individual modules must be regular transitive dependencies — otherwise consumers get
NoClassDefFoundError. Unlike the shaded jar, normal Maven dependencies cannot produce class duplication: dependency mediation ensures a single copy of each GAV on the classpath.Migration
Applications that relied on extensions arriving implicitly through the base starter must now declare those modules explicitly. The all-in-one
io.agentscope:agentscopejar should not be combined with individual modules — it embeds their classes and can shadow the intended version.Checklist