Allow extendWith for the new MoreKnownTypes package - #959
Merged
Conversation
`MoreKnownTypes` moves from `io.spine.tools.type` to `io.spine.tools.proto.type` in `tool-base` as part of splitting the monolithic `tool-base` module. `KnownTypes.Holder.extendWith` names its sole permitted caller as a string checked against the call stack, so the old value rejected the very class the guard exists to admit. The check has no compile-time footprint: everything compiles, and the `SecurityException` appears only when `protobuf-setup-plugins` calls `extendWith` during a consumer's Gradle build. To keep that failure in this repository, `MoreKnownTypes` test double bears the permitted name and exercises the guard from `KnownTypesSpec`, which so far covered only the rejection of client code. The double extends the known types with an empty `TypeSet`, so the singleton keeps its contents and the remaining specs are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The message interpolated the `Class` object and then appended a literal `.name`, so a rejected call reported the class as `class io.spine.tools.proto.type.MoreKnownTypes.name`. `nonAllowedCaller` now returns the exception it builds instead of throwing it, matching its return type; the callers already throw. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The reports were stale after the `2.0.0-SNAPSHOT.441` bump and the `config` update earlier on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Document the invariant that `extendWithNothing` must call the guarded method directly: the guard resolves the immediate caller frame, so an extra frame would break the match quietly rather than loudly. Also align the task plan on "test double" and wrap an over-long line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates KnownTypes.Holder.extendWith to permit the renamed MoreKnownTypes caller (io.spine.tools.proto.type.MoreKnownTypes) and adds a regression test to pin the allowed FQN, preventing runtime build failures in downstream consumers. The PR also includes build tooling updates (POM/dependency report generation) and routine version/report regenerations.
Changes:
- Update
KnownTypesinvocation guard and its@Internalnote to allowio.spine.tools.proto.type.MoreKnownTypes. - Fix
InvocationGuard’sSecurityExceptionmessage formatting and alignnonAllowedCallerbehavior with its return type. - Add tests (incl. a
MoreKnownTypestest double) to verify the guard admits the intended caller and regenerate dependency reports / bump versions.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
version.gradle.kts |
Bumps versionToPublish to 2.0.0-SNAPSHOT.441. |
docs/dependencies/pom.xml |
Updates generated POM version to .441. |
docs/dependencies/dependencies.md |
Regenerates dependency/license report for .441. |
buildSrc/src/test/kotlin/io/spine/gradle/report/pom/PomGeneratorIgTest.kt |
Adds Gradle TestKit integration test coverage for generatePom. |
buildSrc/src/test/kotlin/io/spine/gradle/report/pom/DependencyWriterSpec.kt |
Adjusts dependency writer tests to use resolved-version inputs and adds a failure-to-resolve case. |
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/ResolvedVersions.kt |
Introduces per-project resolved-version collection and serialization utilities. |
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/PomXmlWriter.kt |
Threads resolved-version lookup into POM XML generation. |
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/PomGenerator.kt |
Registers per-project collectors and makes generatePom depend on them. |
buildSrc/src/main/kotlin/io/spine/gradle/report/pom/DependencyWriter.kt |
Refactors dependency collection to consume resolved versions from a provider function. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Validation.kt |
Bumps local Validation dependency version. |
buildSrc/src/main/kotlin/io/spine/dependency/local/ToolBase.kt |
Bumps local ToolBase dependency + dogfooding version. |
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvmCompiler.kt |
Bumps Core JVM Compiler versions. |
buildSrc/src/main/kotlin/io/spine/dependency/local/CoreJvm.kt |
Bumps Core JVM dependency version. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Compiler.kt |
Bumps Compiler fallback versions. |
buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt |
Bumps local Base versions referenced from buildSrc. |
buildSrc/src/main/kotlin/io/spine/dependency/lib/JacksonV2.kt |
Improves Jackson 2.x docs/comments with link to JSTEP-1. |
buildSrc/src/main/kotlin/io/spine/dependency/lib/Jackson.kt |
Improves docs/comments and refactors Moneta artifact coordinate usage. |
buildSrc/build.gradle.kts |
Adds Gradle TestKit and passes buildSrc.classpath into tests via JVM args. |
base/src/test/kotlin/io/spine/type/KnownTypesSpec.kt |
Adds a positive test asserting extendWith is allowed via MoreKnownTypes. |
base/src/test/kotlin/io/spine/tools/proto/type/MoreKnownTypes.kt |
Adds test double with the exact permitted FQN to call extendWith directly. |
base/src/main/kotlin/io/spine/security/InvocationGuard.kt |
Fixes exception message formatting and returns exception instead of throwing inside helper. |
base/src/main/java/io/spine/type/KnownTypes.java |
Updates allowed caller FQN and @Internal comment to the new package. |
.agents/tasks/958-more-known-types-guard.md |
Adds internal task tracking note for issue #958. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #959 +/- ##
==========================================
+ Coverage 94.01% 94.30% +0.28%
==========================================
Files 194 194
Lines 4177 4177
Branches 345 345
==========================================
+ Hits 3927 3939 +12
+ Misses 149 137 -12
Partials 101 101 🚀 New features to boost your workflow:
|
armiol
approved these changes
Aug 19, 2026
This was referenced Aug 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #958.
What changed
KnownTypes.Holder.extendWithnames its sole permitted caller as a stringmatched against the call stack by
InvocationGuard. Intool-base,MoreKnownTypesmoves fromio.spine.tools.typetoio.spine.tools.proto.type— the package the newproto-codemodule ownsafter the split in SpineEventEngine/tool-base#189 — so the old value rejected
the very class the guard exists to admit.
@Internalcomment above it now readio.spine.tools.proto.type.MoreKnownTypes.MoreKnownTypestest double underbase/src/test/kotlin/io/spine/tools/proto/type/bears that name, andKnownTypesSpecgained a positive case calling through it.SecurityExceptionmessage interpolated theClassobject and thenappended a literal
.name, reporting the caller asclass io.spine.tools.proto.type.MoreKnownTypes.name— visible in thestack trace quoted in Update the
extendWithinvocation guard for the newMoreKnownTypespackage #958. Fixed, along withnonAllowedCallernowreturning the exception it builds rather than throwing it, matching its
declared return type. Both call sites already wrote
throw nonAllowedCaller(...), so behaviour is unchanged apart from the message.Why the test matters
The guard is checked at runtime, not compile time. Everything compiles
regardless, and the failure surfaces only when
extendWithis actuallycalled — in
tool-basethat happens throughprotobuf-setup-plugins, so aname mismatch breaks a consumer's Gradle build rather than a test suite
here.
KnownTypesSpecpreviously covered only the negative case (client codeis rejected); nothing pinned the allowed value. The new double closes that
gap: reverting the guard to the old name fails
allow calling 'extendWith' from 'MoreKnownTypes'with exactly theSecurityExceptionreported in the issue.Two invariants make the double work, both documented in its KDoc:
object, whosemethods compile to instance methods on the class named by the declaration,
rather than a top-level function landing in
MoreKnownTypesKt.extendWithdirectly.CallerProvider.previousCallerClassskips exactly three
StackWalkerframes, so an extra frame from adelegating helper would break the match quietly, leaving the test passing
for the wrong reason.
The double extends the known types with an empty
TypeSet;TypeSet.unionreturns
thisfor an empty argument, so the singleton is swapped for a newwrapper over the identical set. Content is unchanged, and
KnownTypesSpecre-reads
KnownTypes.instance()per test while the serialization speccompares
toString(), so no ordering dependency is introduced.For the reviewer
Version carrying the fix:
2.0.0-SNAPSHOT.441.tool-basecannotpublish the renamed package until this ships.
Single name vs. a migration window. The issue notes that one updated
name suffices given the release ordering —
base-librariesfirst, thentool-base— and that is what this PR does. Worth knowing: the mismatchalso runs the other way, so a new
basewith an oldertool-basefailstoo. Contrary to the issue's assumption,
InvocationGuard.allowOnlyalready has a vararg overload, so keeping both names during the migration
is a one-line change if you would rather have the window:
The
docs/dependencies/commit is regeneration only; it was stale afterthe
.441bump and theconfigupdate already on this branch. The addedkotlin-stdlib-jdk7/8:1.8.20andkotlinx-coroutines:1.7.3entries comefrom that
configupdate, not from this change.Also on this branch: the
configupdateTwo commits predate the #958 work and are unrelated to the invocation guard.
They are listed here for review scope; they are not part of the fix.
7d717f41ef— version bump.440->.441.78553fe153—configsubmodule update.configdistributesbuildSrc, so advancing the submodule pointer brings its content with it:the new
ResolvedVersionstype, thePomGeneratorIgTestTestKitintegration test, the
DependencyWriter/PomGenerator/PomXmlWriterrework behind them, and the local dependency bumps below.
Per
AGENTS.md§ Code review, config-distributed paths belong to a review ofthe
configrepo rather than this one, so the pre-PR reviewers were scoped tothe
base/sources andversion.gradle.kts. The dependency changes werechecked for rollbacks; all five move forward:
Base(incl.versionForBuildScript).426.440Compiler.522.523CoreJvm.080.082CoreJvmCompiler.404.410ToolBase/Validation.460.462Splitting these into a separate PR would require rewriting already-pushed
history, so they stay here; the guard fix is confined to
874ea52cdc,3cf35e9f6d, and54a465d43f.Verification
./gradlew build dokkaGenerategreen on JDK 17. Pre-PR reviewers(
spine-code-review,kotlin-engineer,review-docs) all approved; theirfindings are folded into the final commit.