Skip to content

Cap the SDK's Kotlin at the version embedded in Gradle - #50

Merged
alexander-yevsyukov merged 24 commits into
masterfrom
kotlin-version-cap
Oct 21, 2026
Merged

alexander-yevsyukov merged 24 commits into
masterfrom
kotlin-version-cap

Conversation

@alexander-yevsyukov

@alexander-yevsyukov alexander-yevsyukov commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Why

From base-libraries onward, Kotlin code serves both the client and server libraries and our tool projects (the Gradle plugins and the Spine Compiler). Tool code runs on the Kotlin embedded in Gradle, not on the Kotlin it was compiled with:

  • Gradle pins its embedded Kotlin libraries with strictly constraints on every .gradle.kts buildscript classpath, so a tool artifact whose metadata asks for a newer Kotlin stops resolving in consumers.
  • Forcing Kotlin in the consumer only gets past resolution: scripts still load Gradle's own stdlib, so newer bytecode risks NoSuchMethodError.

This bit us when config moved to Kotlin 2.4.21 under the Gradle 9.8.1 wrapper, which embeds 2.4.10. Compiler .072+ (built with 2.4.21) failed to resolve from plugins {} blocks in consumers, and three CoreJvmPluginIgTest tests in core-jvm-compiler failed. Compiler .069/.071 (built with 2.4.10) kept working.

What

  • New skill, kotlin-version-cap. The cap is the Kotlin embedded in the Gradle version of config's wrapper. The skill states the rule, the declarations it covers, and how the cap moves. Its script, scripts/kotlin_cap.py, uses only the Python standard library:
    • cap reads the wrapper version: from the working tree in config itself, and from config's master in a consumer, whose config submodule can lag well behind. It falls back to the submodule copy when offline. It then looks up the embedded Kotlin in its table, or in the unpacked distribution for a Gradle newer than the table. It exits 3 with a hint when the cap is unknown.
    • requirement [--cap X] g:a:v… reports each module's Kotlin requirement and verdict.
      • It reads both the POM and the .module file, through the Gradle cache, the Spine Artifact Registry, Maven Central, and the Gradle Plugin Portal.
      • It resolves ${…} properties through parent POMs.
      • Only a 404 from every repository counts as absent; any other failure is unverifiable.
  • dependency-update:
    • holds the Kotlin family at the cap;
    • takes the newest release of any other artifact, local/ included, whose Kotlin requirement fits, listing older versions from maven-metadata.xml;
    • reports values already above the cap, in every scope, instead of downgrading them;
    • freezes everything that depends on Kotlin when the cap is unknown;
    • gains new report sections and six new scenarios.
  • dependency-audit: new Check F flags Kotlin values and dependency versions above the cap, including every local/ artifact. A violation the diff introduces is Must fix; one present before the diff is Should fix.
  • bump-gradle: records the cap and the embedded Kotlin before and after the bump. In config, the wrapper lands before any Kotlin raise, and the bump stops only when it puts a Kotlin declaration above the cap itself.
  • pre-pr: dispatches dependency-audit when buildSrc/build.gradle.kts changes, since that file declares kotlinEmbeddedVersion.
  • Claude commands: dependency-update and bump-gradle may run the read-only kotlin_cap.py without a permission prompt.

Testing

  • scripts/tests/test_kotlin_cap.py: 21 tests against a local HTTP server, so they run offline. They cover the cap sources and the table edges, POM-vs-module requirements, property and parent resolution, 404 vs failed fetches, redirects, and the Gradle cache. The full python3 -m unittest discover -s scripts/tests suite passes (94 tests).
  • Live run: in config and core-jvm-compiler the cap is 2.4.10; Compiler .079 is above the cap, .071 within it; Java libraries and portal-only plugins show no requirement.
  • The updated skills have not been exercised end to end, and the author-skill eval loop was not run.

Follow-ups (not in this PR)

  • Roll config's Kotlin back to the cap (2.4.10 for Gradle 9.8.x), then rebuild the Spine repositories in dependency order (base-libraries → … → compiler → core-jvm-compiler).
  • The audit agent runs the script through its unrestricted Bash tool. In consumers, its fetches go through the prompts set in config/.claude/settings*.json.

🤖 Generated with Claude Code

From `base-libraries` onward, Kotlin code serves both the client and server
libraries and our tool projects — the Gradle plugins and the Spine Compiler —
and tool code runs on the Kotlin embedded in Gradle, not on the one it was
compiled with. Gradle's Kotlin DSL pins the embedded `kotlin-stdlib`,
`kotlin-reflect`, and `annotations` with `strictly` constraints on every
buildscript classpath, so a tool artifact whose metadata asks for a newer Kotlin
stops resolving. That happened when `config` moved to Kotlin `2.4.21` under the
Gradle `9.8.1` wrapper, which embeds `2.4.10`: Compiler builds made with
`2.4.21` failed in consumers, while those made with `2.4.10` kept working.
Forcing Kotlin in a consumer only satisfies resolution — scripts still load
Gradle's own stdlib, so newer bytecode risks `NoSuchMethodError`.

The rule now lives in `guidelines/kotlin-version-cap.md`: the cap is the Kotlin
embedded in the Gradle version of `config`'s wrapper, read from `config`'s
`master` in a consumer (the submodule may lag) and from the working tree in
`config` itself. The guideline snapshots Gradle's embedded-Kotlin table, names
the declarations it covers, and says how to read a library's Kotlin requirement
from its POM.

- `dependency-update` holds the Kotlin family at the cap, takes the newest
  release of any other artifact whose requirement fits, and reports values
  already above the cap instead of downgrading them.
- `dependency-audit` flags Kotlin values and dependency versions above the cap,
  including every `local/` artifact; violations a diff introduces are Must fix,
  pre-existing ones Should fix.
- `bump-gradle` records the embedded Kotlin before and after the bump, and in
  `config` lands the wrapper ahead of any Kotlin raise.
- `pre-pr` dispatches the audit when `buildSrc/build.gradle.kts` changes, since
  it declares `kotlinEmbeddedVersion`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 18:24
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-20T18:31:53.935617Z 72d3fc9 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Requirement checks can miss incompatible dependencies, and the wrapper guard omits a capped Kotlin declaration.

4 open issues
What changed in this PR

Adds a shared Kotlin version cap tied to config’s Gradle wrapper, extending dependency and wrapper workflows across the Spine SDK.

Changes:

  • Defines cap calculation, covered dependencies, and rollback guidance.
  • Adds cap checks, reporting, and dependency-update scenarios.
  • Updates agent wrappers and pre-PR audit dispatch.
File Description
skills/​pre-pr/​SKILL.md Audits build-script dependency changes.
skills/​dependency-update/​SKILL.md Applies the cap and reports blocked updates.
skills/​dependency-update/​scenarios.md Adds four cap scenarios.
skills/​dependency-update/​references/​buildsrc-build-script.md Extends cap guidance to build-script versions.
skills/​dependency-update/​agents/​openai.yaml Adds cap instructions.
skills/​dependency-audit/​SKILL.md Adds Kotlin-cap audit checks.
skills/​dependency-audit/​agents/​openai.yaml Includes cap auditing.
skills/​bump-gradle/​SKILL.md Reports embedded Kotlin and guards wrapper changes.
skills/​bump-gradle/​agents/​openai.yaml Requests cap-impact reporting.
guidelines/​kotlin-version-cap.md Defines the shared cap policy.
guidelines/​_TOC.md Indexes the new guideline.
claude/​commands/​pre-pr.md Expands dependency-audit dispatch.
claude/​commands/​dependency-update.md Adds cap enforcement instructions.
claude/​commands/​bump-gradle.md Separates wrapper and Kotlin raises.
claude/​agents/​dependency-audit.md Adds cap checks and metadata-fetch permissions.

Review effort: Balanced

Comment thread skills/bump-gradle/SKILL.md Resolved
Comment thread skills/dependency-audit/SKILL.md Resolved
Comment thread skills/dependency-audit/SKILL.md Resolved
Comment thread skills/dependency-audit/SKILL.md Resolved

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 72d3fc99fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/dependency-audit/SKILL.md Resolved
Comment thread skills/dependency-update/SKILL.md Resolved
Comment thread skills/dependency-update/SKILL.md Resolved
Comment thread skills/bump-gradle/SKILL.md Resolved
Comment thread skills/dependency-update/SKILL.md Resolved
Comment thread skills/bump-gradle/SKILL.md Resolved
alexander-yevsyukov and others added 2 commits October 20, 2026 19:37
Review of #50 found gaps that let an above-cap version through.

- A Kotlin requirement is now the highest across the POM and the `.module`
  file — Gradle reads the latter, and the two can differ — and across every
  module of a dependency object, since a Java-only first module says nothing
  about its siblings. A `local/` object, built as one unit, may stop at the
  first module that declares one. A `${…}` or parent-managed version that
  cannot be resolved is unverifiable, never "no requirement".
- The lookup loop moves into the guideline as "Reading a requirement", so the
  audit and `dependency-update` share one tested copy. It reads both files,
  prints one line per declaration, and marks a missing POM.
- `dependency-update` lists older versions from `maven-metadata.xml` for the
  walk-down (a GitHub `/releases/latest` page names only the newest), checks
  current versions in every scope rather than only Kotlin and `local/`, and
  freezes everything with a Kotlin requirement when the cap is unknown. The
  current Kotlin can itself be above the cap, so it is no stand-in ceiling.
- `bump-gradle` compares the cap with every Kotlin-family declaration,
  `Kotlin.embeddedVersion` included. It stops only when the bump itself puts
  one above the cap, so a pre-existing violation no longer blocks `config`'s
  next wrapper bump. Outside `config`, it takes the cap from `config`'s wrapper
  instead of the local one.

Also fixes a comma splice and a missing article, and notes that `BomsPlugin`
leaves Dokka and Detekt configurations alone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The review's four should-fix items already landed in the previous commit;
these are its remaining wording nits.

- Reflow paragraphs that ended on a one- or two-word line, and the ragged tail
  of the audit's `description`.
- Keep the **Capped** report columns from splitting a code span across lines.
- Say which values the third cap bullet covers, and that the cap applies to
  pinned, synced, and independent build-script versions alike.
- Explain the `^offline` alternative in the audit's wrapper `grep`.
- Split "never raise Kotlin, or adopt a release that requires a newer Kotlin,
  above …" into two plain clauses in the command wrapper and `openai.yaml`.
- Restore the dropped "changed" in `pre-pr`'s **deps** trigger.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@alexander-yevsyukov alexander-yevsyukov changed the title Cap the SDK's Kotlin at the Kotlin embedded in Gradle Cap the SDK's Kotlin at the version embedded in Gradle Oct 20, 2026
Fixes from the pre-PR re-run:

- `bump-gradle` suggested a new table row whenever the new Gradle had none
  of its own, but rows exist only where the embedded Kotlin changes; a patch
  release such as `9.8.1` is answered by its minor's row. It now suggests a
  row only when the table cannot answer, as the guideline's step 3 defines.
- The `dependency-update` walk-down names the `maven-metadata.xml` URL in full
  instead of an undefined `<repo>`, and `version-discovery.md` says that the
  search API finds the newest version while the walk-down needs the full list.
- The guideline's loop says to replace its sample entry with the modules to
  check, and its output notes say that no lines mean no Kotlin requirement.
- Reflow a one-word closing line in the **Capped** report entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread guidelines/kotlin-version-cap.md Resolved
Comment thread guidelines/kotlin-version-cap.md Resolved
alexander-yevsyukov and others added 3 commits October 20, 2026 20:09
Keep the two reasons — Gradle's `strictly` pins make a tool that asks for a
newer Kotlin unresolvable, and forcing past them leaves it on Gradle's own
stdlib — and the dated incident. Drop the quoted resolution errors and the POM
and module-metadata mechanics, which are diagnostics rather than reasons, and
the `org.jetbrains:annotations` `13.0` pin, which holds only until Gradle embeds
a newer Kotlin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #50 found two ways the loop could misjudge a version.

- It never asked the Gradle Plugin Portal, which `config`'s `buildSrc` resolves
  from, so an uncached portal-only plugin read as `no-pom` and was held back
  whatever its Kotlin requirement. The portal joins the repository list, with
  `curl -L` because it answers through redirects, and the walk-down in
  `dependency-update` lists its `maven-metadata.xml` too.
- `curl -f` turned a 404 and a network, authentication, or server failure into
  the same `no-module`, which the guideline called normal — so a `.module`
  that failed to download could hide a requirement above the cap. The loop now
  judges each repository's final HTTP status: only a 404 from all of them is
  `no-pom` or `no-module`, and any other failure with no success elsewhere is
  `unreadable-pom` or `unreadable-module`, which counts as unverifiable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Half of the guideline was two procedures — computing the cap and reading a
module's Kotlin requirement — and three review rounds kept patching the bash
loop behind the second. They now live in one tested script, and the rest of the
guideline becomes the skill that says how to use it.

- `skills/kotlin-version-cap/scripts/kotlin_cap.py` (standard library only):
  `cap` reads `config`'s wrapper version — from the working tree in `config`,
  from `config`'s `master` in a consumer, the submodule copy when offline —
  and looks up the embedded Kotlin in its table, or in the unpacked
  distribution past the table; it exits 3 with a hint when the cap is unknown.
  `requirement [--cap X] g:a:v…` reads the POM and the `.module` file through
  the Gradle cache, the Spine Artifact Registry, Maven Central, and the Gradle
  Plugin Portal, resolves `${…}` properties through parent POMs, and reports
  each module's requirement and verdict. Only a 404 everywhere counts as
  absent; any other failure is unverifiable.
- `scripts/tests/test_kotlin_cap.py` covers both against a local HTTP server,
  so it runs offline.
- `dependency-update`, `dependency-audit`, and `bump-gradle` follow the skill
  and run its script; the audit's Check F shrinks to three steps. Their Claude
  commands may run that one read-only script without a prompt.
- The guideline is gone; `_TOC.md` lists the skill instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Verified metadata-handling defects can accept incompatible artifacts or reject compatible releases.

4 open issues
2 resolved since last review

Review effort: Balanced

Review of #50 found four ways `kotlin_cap.py requirement` could accept an
incompatible artifact or reject a compatible one.

- Test, provided, and system scopes, and optional dependencies, no longer count:
  consumers never receive them, so a Java library tested with a newer Kotlin
  read as above the cap.
- A parent chain longer than the script follows is now `parent-chain-too-deep`,
  not a silent partial scan that could report no requirement.
- `available-at` redirects in Gradle Module Metadata are followed for JVM
  variants, with a cycle guard. A Kotlin Multiplatform root now takes its JVM
  artifact's requirement, and an unreadable target makes it unverifiable.
- Version ranges and dynamic versions in either file are unverifiable instead
  of being compared as plain versions, so `[2.4.21,)` can no longer pass a
  `2.4.10` cap or hide behind a lower literal.

Seven tests cover these; the skill's step 3 describes the new reasons.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Metadata parsing can accept unresolved Kotlin requirements and falsely reject compatible Maven child overrides.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Maven child dependency overrides are ignored

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:333

Concatenating parent and child dependencies ignores Maven's child overrides. A parent declaring kotlin-stdlib:2.4.21 and a child overriding it to 2.4.0 leaves both versions in the result, so a 2.4.10 cap incorrectly rejects the compatible child. Merge inherited dependencies by Maven dependency identity with child precedence before filtering scopes/optional dependencies and collecting requirements. Add regression cases for child version and scope/optional overrides; deduplicating the already-filtered tuples would still miss those overrides.

Medium severity Versionless Kotlin dependencies bypass platform constraint validation

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:375

A versionless Kotlin dependency in .module metadata contributes neither a requirement nor a problem. With a POM declaring kotlin-stdlib:2.4.0 and a JVM variant obtaining its stdlib version from a platform, this reports within-cap against 2.4.10 without resolving the platform's version. With an empty POM, it even reports requirement=none. This can let the updater accept an unchecked release. Resolve the applicable constraints/platform, or mark the dependency unverifiable when its version remains unknown. Add coverage for both resolved and unresolved versionless dependencies.

4 resolved since last review

Review effort: Balanced

Review of #50 flagged two misses in `kotlin_cap.py requirement`.

- Parent and child dependencies were concatenated, so a child that overrides
  an inherited `kotlin-stdlib` — to a lower version, to `test` scope, or to
  optional — was still judged by the parent's entry. Dependency entries now
  merge down the parent chain as Maven merges them, keyed by group, artifact,
  type, and classifier, with the child's fields winning and the rest
  inherited, and the scope and `optional` filter runs after the merge.
- A versionless Kotlin dependency in Gradle Module Metadata counted for
  nothing, so its unread platform version could pass as within the cap or as
  no requirement. It now counts only when a constraint or a Kotlin BOM in the
  same variant settles it; otherwise it is `unresolved-…`, which makes the
  result unverifiable.

Five tests cover the overrides and both versionless cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of #50: when the same BOM is both imported in the POM and declared as a
platform in module metadata, the new import filter clipped both, so an
explicit `kotlin-stdlib:2.4.0` hid the platform's `2.5.0` constraint. The
filter now applies only to a BOM the POM alone imports; a platform the
metadata declares passes all its constraints on, as Gradle applies them. One
test covers both formats together and fails on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The metadata resolver has an unresolved import-precedence issue, and organisation-wide workflows still need end-to-end human validation.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

Medium severity Preserve POM BOM import order to avoid false version-cap violations

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:659

Preserve declaration order when merging POM-imported BOMs. A POM-only library importing BOM A (managing kotlin-stdlib:2.4.0) before BOM B (managing 2.5.0), with a versionless stdlib dependency, reports requirement=2.5.0 verdict=above-cap against 2.4.10. The first imported management entry should win, so this rejects a compatible update and creates a false audit violation. Resolve POM import precedence per module before taking the highest requirement; keep .module platform conflict handling separate. Add regression cases with both import orders.

1 resolved since last review

Review effort: Balanced

Review of #50: with two imported BOMs managing `kotlin-stdlib` at `2.4.0` and
then `2.5.0`, a library's versionless stdlib read as `2.5.0`, the higher of
the two. Maven resolves imports in declaration order, so the first one that
manages a module wins. A POM-only import's contribution is now computed after
every lookup, in import order, for both library and platform reads; a
platform declared in module metadata keeps its own handling.

Order also has to follow Maven's effective model, where the POM's own
dependency-management entries come before the ones it inherits, so a child's
import beats its parent's. The parent-chain merge now lists the child first,
still letting the child's fields win and an active profile override its own
POM's entries.

Two tests cover both import orders and an inherited import; they fail on the
previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Verified BOM import handling can both accept incompatible dependencies and reject compatible ones.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Preserve imported BOM scope before dependency filtering

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:467

Imported BOM management supplies versions but loses scope before dependencies are filtered. A versionless stdlib dependency whose imported BOM manages kotlin-stdlib:2.5.0 with scope=test reports above-cap against 2.4.10; identical inline management correctly reports none. This blocks a compatible release even though consumers receive no stdlib dependency. Preserve imported management fields and apply their defaults before filtering consumer dependencies. Add a regression comparing imported and inline test-scoped management.

Review effort: Balanced

Review of #50 found two more import-handling misses for a library's
versionless Kotlin dependency.

- A `kotlin-bom` import settled it unconditionally, outside the import order,
  so a BOM imported earlier that managed `kotlin-stdlib:2.5.0` lost to a later
  `kotlin-bom:2.4.0`. A Kotlin BOM import now takes its place in declaration
  order, managing every Kotlin module at its own version.
- An imported BOM's managed scope was dropped: a stdlib it managed as `test`
  still counted, though the same inline management drops the dependency.

Both now go through `managed_entry()`, which finds the entry the first
managing import gives the module — the BOM's own, else its first import's,
recursing with a cycle guard — with its scope and `optional` flag, as Maven
resolves it. A test-scoped, provided, or optional entry means the dependency
never reaches a consumer. Fetches are cached, since BOMs are re-read.

Two tests cover both import orders and inline, imported, and nested scopes;
they fail on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Imported BOM scope handling can incorrectly accept a dependency whose Kotlin requirement exceeds the cap.

2 open issues
1 resolved since last review

Review effort: Balanced

Review of #50: the previous commit applied an imported BOM's managed scope to
a versionless Kotlin dependency even when the dependency declared its own, so
`kotlin-stdlib` declared with `scope=compile` read as test-scoped and dropped
out, letting a `2.5.0` requirement pass a `2.4.10` cap. `PomReading` now keeps
each Kotlin dependency's own scope and `optional` flag, and the imported entry
only fills in what those leave out. The scope test gains the explicit-compile
case, which fails on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Two verified BOM-handling errors can falsely reject compatible dependencies.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Kotlin BOM import incorrectly blocks libraries without Kotlin dependencies

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:503

This counts a POM's Kotlin BOM import even when an ordinary library has no consumer-visible Kotlin dependencies. Importing kotlin-bom:2.5.0 solely to version test dependencies reports above-cap against 2.4.10, incorrectly blocking a compatible Java library. Count the import's managed versions only in platform mode or when they supply a consumer-visible dependency's version; keep unresolved imports unverifiable.

Medium severity Imported defaults ignored for explicitly versioned dependencies

skills/​kotlin-version-cap/​scripts/​kotlin_cap.py:750

Imported scope/optional defaults are applied only to versionless dependencies in pom.pending. A dependency explicitly declaring kotlin-stdlib:2.5.0 without a scope therefore ignores an imported BOM's scope=test and reports above-cap against 2.4.10; equivalent inline management reports requirement=none. This falsely blocks compatible updates. Apply imported management defaults before filtering all consumer-visible dependencies, preserving explicit versions, scopes, and optional flags. Add a regression for an explicitly versioned dependency with an imported test-scope default.

1 resolved since last review

Review effort: Balanced

…reach consumers

Review of #50 found two more false rejections in a library's imports.

- A library's own `kotlin-bom` import counted as a requirement even when no
  dependency consumers receive used it, so a Java library importing
  `kotlin-bom:2.5.0` only for its tests read as above a `2.4.10` cap. Read as a
  library, a Kotlin BOM import now counts only through the dependencies it
  versions; read as a platform, or left unresolved, it still counts, and only
  a platform passes its alignment on.
- An imported BOM's scope and `optional` defaults reached only versionless
  dependencies, so a `kotlin-stdlib:2.5.0` declaring no scope ignored an
  imported test scope. Such a versioned dependency is now provisional until
  the BOMs are read, then kept or dropped by the first managing import's
  entry; the dependency's own fields and the POM's own management still win.

The scope rule lives in one `transitive()` helper. Two tests cover both
cases; they fail on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of #50 found two ways a `kotlin-bom` that is not imported still
counted as aligning every Kotlin module, so a consumer's versionless stdlib
read as settled:

- in a POM read as a platform, a managed `kotlin-bom` entry without
  `scope=import`, which only constrains the BOM's own coordinate;
- in module metadata, a `kotlin-bom` dependency of the library category,
  which does not pass its constraints on.

Alignment now comes only from an actual Kotlin BOM import — `scope=import`
in a POM, a `platform` or `enforced-platform` dependency in metadata. Two
tests cover the formats; they fail on the previous script.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

@alexander-yevsyukov
alexander-yevsyukov merged commit 9cb20bd into master Oct 21, 2026
1 check failed
@alexander-yevsyukov
alexander-yevsyukov deleted the kotlin-version-cap branch October 21, 2026 12:58
@github-project-automation github-project-automation Bot moved this from 🏗 In progress to ✅ Done in v2.0 Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants