feat(build): generate THIRD-PARTY inventory via license-maven-plugin - #1739
Merged
Conversation
Replace hand-curated third-party component lists and version pins in NOTICE.txt and thirdPartyCopyright with a stable product pointer. The versioned dependency/license inventory is now produced at build time by org.codehaus.mojo:license-maven-plugin (aggregate-add-third-party) and copied into the perc-distribution-tree assembly as THIRD-PARTY.txt. Resolves #1689. > Co-Authored by Grok Build using grok-4.5 with agent main.
Extend the #1689 inventory so the shipped file covers both Maven and npm. - license-maven-plugin writes THIRD-PARTY-MAVEN.txt - scripts/generate-third-party-inventory.py reads product package-lock.json production packages (WebUI SPA lockfile) and merges both halves into a single THIRD-PARTY.txt for the installer - Unit tests for the merge script; packaging test asserts npm section > Co-Authored by Grok Build using grok-4.5 with agent main.
Pre-push spotless:check requires property alphabetical order and execution element order for the merge-third-party-inventory plugin. > Co-Authored by Grok Build using grok-4.5 with agent main.
Move the #1689 inventory merge into com.intsof.common:utilities as a product-agnostic Java API with full Javadoc and JUnit coverage (no Python on the build classpath). - ThirdPartyLicenseInventory: read package-lock production packages, merge with Maven inventory text, CLI main for exec-maven-plugin:java - Dependency-free minimal JSON parser for lockfile packages maps - Wire merge on perc-distribution-tree generate-resources (after utilities) - Root keeps license-maven-plugin → THIRD-PARTY-MAVEN.txt only - Remove scripts/generate-third-party-inventory.py path from the build > Co-Authored by Grok Build using grok-4.5 with agent main.
Contributor
Code Review SummaryStatus: No New Issues Found | Recommendation: Address before merge Overview
This is an incremental review of the new commit on top of Resolved since previous review
Verification on changed lines (
|
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 3 |
| SUGGESTION | 1 |
This is an incremental review of the new commits on top of 9dcba97c2:
8f50ccabd feat(utilities): generic ThirdPartyLicenseInventory for Maven+npm merge and 3cd57f275 fix(build): single exec-maven-plugin for license merge in distribution-tree. Previously flagged issues are re-verified below.
Resolved since previous review
| Previous issue | Status |
|---|---|
pom.xml:247 default <python.executable>python</python.executable> (cross-platform hazard) |
RESOLVED — property removed (the entire Python merge pipeline is replaced by Java in com.intsof.common:utilities). |
pom.xml:3112 unconditional exec-maven-plugin Python exec with no skip guard |
RESOLVED — replaced with <goal>java</goal> against the com.intsof.common.utilities.license.ThirdPartyLicenseInventory main class, configured inside modules/perc-distribution-tree/pom.xml only (no longer inherited=false root plugin). |
scripts/generate-third-party-inventory.py:117 Unknown license fallback without warning |
OUT OF SCOPE — Python script deleted; the new Java implementation (ThirdPartyLicenseInventory.java) also defaults license to "Unknown license" when blank in NpmPackage compact constructor (line 95-100) and licenseFromMeta (lines 570-596), but without any aggregate warning. The deletion is acceptable; the new behavior is documented in JavaDoc. |
scripts/generate-third-party-inventory.py:45 silent empty npm-package-locks.txt |
NOT FIXED — see WARNING below; the same defect exists in readLockList (line 244) and readProductionPackagesFromLockList (line 220) of the new Java implementation. |
pom.xml:3081 duplicate Apache License, Version 2.0 in licenseMerge |
UNCHANGED — line is not touched by the incremental diff; pre-existing. |
ThirdPartyInventoryPackagingTest.java:71,78 tautological assertions |
UNCHANGED — test file not touched by incremental diff; pre-existing. |
New / re-verified issues on changed lines
WARNING
| File | Line | Issue |
|---|---|---|
modules/intsof-common-utilities/src/main/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventory.java |
220 | readProductionPackagesFromLockList silently skips entries in npm-package-locks.txt whose package-lock.json is not a regular file. The shipped THIRD-PARTY.txt then lists zero npm entries from that source with no warning. Same defect as the previous review's scripts/generate-third-party-inventory.py:45. Recommend warning to stderr or returning skipped paths so callers can fail the build. |
modules/intsof-common-utilities/src/main/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventory.java |
244 | readLockList returns List.of() silently when npm-package-locks.txt is absent. The merged THIRD-PARTY.txt looks complete but contains zero npm entries. Same defect as previous review. Recommend surfacing the missing list path to stderr and/or exiting non-zero. |
modules/intsof-common-utilities/src/main/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventory.java |
477 | runMain re-reads every lockfile just to compute npmCount after the merged file was already written successfully. Wasteful, and a stale/deleted lockfile at this point yields a confusing ERROR: ... message for what was actually a successful write. The Files.isRegularFile(outDir.resolve(npmName)) check is also dead — generateMergedInventory always writes that file. Recommend returning the package list from generateMergedInventory instead of re-reading. |
SUGGESTION
| File | Line | Issue |
|---|---|---|
modules/intsof-common-utilities/src/main/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventory.java |
159 | Inconsistent error handling for the same condition: readProductionPackagesFromLockFile throws IllegalArgumentException when the lockfile is missing, but the list-driven path (readProductionPackagesFromLockList line 220) silently skips. Pick one behavior (silent-skip + warning is more useful for build pipelines) and apply in both places. |
Additional observation (not inline-anchorable, change-class completeness)
ThirdPartyLicenseInventoryTest.java does not cover the silent-skip paths above, the NpmPackage compact constructor's blank-license normalization (line 95-100), the licenseFromMeta map/list forms (lines 570-596), the runMain CLI flag parsing, or BOM handling in lockfiles. Per root AGENTS.md ("you must ALWAYS update or create unit tests for any code change... tests must pass") and REVIEW.md ("Behavioral tests required for new non-trivial logic"), the new library API has uncovered paths — particularly the silent-skip behavior, which is the same defect pattern this PR inherits from the deleted Python implementation.
Notes on publication
A pending review by kilo-code-bot[bot] (review id 4838658249) on this PR prevented posting new inline review comments via the POST /reviews endpoint (HTTP 422 — "User can only have one pending review per pull request"). The DELETE/PATCH to cancel the pending review was also rejected by the agent's permission gate. Inline findings are therefore documented in the table above; the summary itself uses the issue-comments endpoint which is unaffected. To convert these into inline comments, cancel review 4838658249 and re-run this code-review pass.
Files Reviewed (incremental — 10 files)
modules/intsof-common-utilities/README.md— 0 issues (docs match new Java API)modules/intsof-common-utilities/src/main/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventory.java— 3 issues (NEW, 798 lines)modules/intsof-common-utilities/src/test/java/com/intsof/common/utilities/license/ThirdPartyLicenseInventoryTest.java— 0 inline issues; coverage gaps noted above (NEW, 164 lines)modules/perc-distribution-tree/pom.xml— 0 issues (exec-maven-pluginjavagoal wired correctly;--require-mavenguard preserves previous-review fix; runs beforecopy-license-inventory)pom.xml— 0 issues (Python exec block removed;python.executableproperty removed; comment block updated to reference Java merger)scripts/README.md— 0 issues (docs consistent with new Java location)src/license/README.md— 0 issues (correctly points atThirdPartyLicenseInventoryincom.intsof.common:utilities)scripts/generate-third-party-inventory.py— DELETED (issue Bump org.apache.poi:poi-ooxml from 5.2.2 to 5.4.0 #4 of previous review migrated into Java — see WARNING above; the migration preserves the silent-skip defect rather than fixing it)scripts/generate-third-party-inventory.bat— DELETED (Windows launcher no longer needed)scripts/test_generate_third_party_inventory.py— DELETED (replaced byThirdPartyLicenseInventoryTest.java)
Fix these issues in Kilo Cloud
Previous review (commit 9dcba97)
Status: 7 Issues Found | Recommendation: Address before merge
Overview
| Severity | Count |
|---|---|
| CRITICAL | 0 |
| WARNING | 4 |
| SUGGESTION | 3 |
Issue Details (click to expand)
WARNING
| File | Line | Issue |
|---|---|---|
pom.xml |
247 | Default <python.executable>python</python.executable> is a cross-platform hazard — python is missing on stock macOS / many Linux distros / minimal CI images. Project standard is python3 (root AGENTS.md, all sister scripts). |
pom.xml |
3112 | The <executable>${python.executable}</executable> exec-maven-plugin goal runs unconditionally on every root generate-resources. No -Dlicense.skipMerge / <skip> guard; no idempotence on already-merged THIRD-PARTY.txt. Breaks clean installs on machines with Python but missing npm/node_modules. |
scripts/generate-third-party-inventory.py |
117 | node_modules/<name>/package.json fallback silently emits "Unknown license" for every npm production dep on clean CI (no node_modules/ present). The shipped THIRD-PARTY.txt will then list hundreds of deps as "Unknown license" without any warning summary. |
scripts/generate-third-party-inventory.py |
45 | When npm-package-locks.txt is missing, read_lock_list returns [] silently and the script emits an empty THIRD-PARTY-NPM.txt — the merged THIRD-PARTY.txt looks complete but contains zero npm entries. |
SUGGESTION
| File | Line | Issue |
|---|---|---|
pom.xml |
3081 | licenseMerge for Apache 2.0 contains a duplicate entry (Apache License, Version 2.0 appears at start and end). Harmless but signals a paste typo. |
modules/perc-distribution-tree/src/test/java/com/percussion/distribution/install/ThirdPartyInventoryPackagingTest.java |
71 | `text.contains("license") |
modules/perc-distribution-tree/src/test/java/com/percussion/distribution/install/ThirdPartyInventoryPackagingTest.java |
78 | `text.contains("npm third-party dependencies") |
Files Reviewed (14 files)
NOTICE.txt— 0 issuespom.xml— 3 issuesmodules/perc-distribution-tree/pom.xml— 0 issues (correct invocation, file copies ingenerate-resources)modules/perc-distribution-tree/src/test/java/com/percussion/distribution/install/ThirdPartyInventoryPackagingTest.java— 2 issuesscripts/README.md— 0 issues (docs updates are consistent)scripts/generate-third-party-inventory.py— 2 issuesscripts/generate-third-party-inventory.bat— 0 issuesscripts/test_generate_third_party_inventory.py— 0 issues (3 tests cover production/dev filtering, merge shape, end-to-end)src/license/README.md— 0 issues (matches generator behavior)src/license/THIRD-PARTY.properties— 0 issues (8 missing-license overrides, correctly named)src/license/npm-package-locks.txt— 0 issues (single lockfile, with explanatory comments)system/src/main/resources/com/percussion/server/PSStringResources.properties— 0 issues (slim pointer, no version pins)system/src/test/java/com/percussion/server/PSThirdPartyCopyrightTest.java— 0 issues (regexes are sound;LICENSE.txt+pom.xmlroot detection inresolveRepoRootis portable)docs/ai-generated/code-reviews/1689-license-maven-plugin-erlang.md— 0 issues (durable review report)
Notes on publication
A pending review by kilo-code-bot[bot] (review id 4838658249) on this PR prevented posting new inline review comments via the POST /reviews endpoint (HTTP 422 — "User can only have one pending review per pull request"). Inline findings are therefore documented in the table above; the summary itself uses the issue-comments endpoint which is unaffected. To convert these into inline comments, submit/cancel the existing pending review and re-run this code-review pass.
Reviewed by minimax-m3 · Input: 61.7K · Output: 6.3K · Cached: 986.8K
Review guidance: REVIEW.md from base branch development
…n-tree Avoid duplicate org.codehaus.mojo:exec-maven-plugin declarations (Maven warning and unstable merge). Attach merge-third-party-inventory to the existing exec-maven-plugin next to setup-home / verify-jdbc-drivers. > Co-Authored by Grok Build using grok-4.5 with agent main.
…entory Address Kilo review on PR #1739: - collectProductionPackagesFromLockList records missing lock-list / locks - --require-maven requires complete Maven + npm sources (no silent empty npm) - GenerateResult carries npmPackageCount (no second lockfile pass in runMain) - Expand ThirdPartyLicenseInventoryTest (13 cases) - Tighten packaging test assertions (drop tautologies) > Co-Authored by Grok Build using grok-4.5 with agent main.
Collaborator
Author
|
Mitigation (commit 0b81eaa): Addressed the latest Kilo review on the Java inventory merger. WARNINGs fixed
TestsThirdPartyLicenseInventoryTest: 13 cases. Packaging test tightened (section headers + npm: coordinate). See commit 0b81eaa on feat/1689-license-maven-plugin. |
natechadwick
approved these changes
Aug 2, 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
2 participants
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.
Summary
Resolves #1689.
Stop hand-curating third-party component catalogs and version pins in
NOTICE.txtand thethirdPartyCopyrightresource-bundle key. The versioned inventory is generated at build time byorg.codehaus.mojo:license-maven-plugin(aggregate-add-third-party) and shipped asTHIRD-PARTY.txtin the installer assembly. Hand-maintained prose is a stable pointer only.Design (no dual-maintenance)
license-maven-plugin→target/generated-sources/license/THIRD-PARTY.txtTHIRD-PARTY.txtNOTICE.txtsrc/license/THIRD-PARTY.propertiesChanges
pom.xml: pinlicense-maven-plugin2.7.1,inherited=false, aggregate goal on rootgenerate-resourcesNOTICE.txt+PSStringResources.properties(copyrightyear 2026, Intersoft attribution)perc-distribution-tree: copyLICENSE.txt,NOTICE.txt,THIRD-PARTY.txtinto assembly root (generate-resources, before antrun)PSThirdPartyCopyrightTest(no version pins / no hand-curated catalogs),ThirdPartyInventoryPackagingTestsrc/license/README.md, Erlang review underdocs/ai-generated/code-reviews/Intentionally not in this PR
check-file-header) across the monorepoaggregate-download-licenses(network-heavy; inventory text is enough for AC)Pre-PR gates
Spotless:
mvnw.cmd spotless:applythenmvnw.cmd spotless:check(apply first) — BUILD SUCCESS. Feature PR contains only in-scope files (unrelated i18n cache drift discarded).Erlang: approve —
docs/ai-generated/code-reviews/1689-license-maven-plugin-erlang.mdMaven:
cd system ..\mvnw.cmd clean installPSTransitionUtils/PSTransformT*(untouched)THIRD-PARTY.txt(do not use-N)cd modules\perc-distribution-tree ..\..\mvnw.cmd generate-resourcesNote: full standalone
perc-distribution-treeclean installstill requires prior WebUI/jetty reactor artifacts (WebUI/targetmissing) — pre-existing installer assembly dependency, not introduced by this change.Test plan
mvnw license:aggregate-add-third-party) produces non-emptytarget/generated-sources/license/THIRD-PARTY.txtLICENSE.txt,NOTICE.txt,THIRD-PARTY.txtthirdPartyCopyrighthas no dependency version pins and points atTHIRD-PARTY.txt