refactor(assetregistry): isolate skill artifact lifecycle - #282
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe PR reorganizes Skill package lifecycle ownership around parent capabilities for uploads, release delivery, cleanup, and storage. It adds public contracts, package validation, authorized delivery, cleanup summaries, module-boundary tests, and updated storage integration. ChangesSkill lifecycle ownership
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SkillRegistryService
participant SkillPackageAssetService
participant SkillPackageStoragePort
participant AssetRegistryService
participant SkillReleaseDeliveryService
SkillRegistryService->>SkillPackageAssetService: import or replace package
SkillPackageAssetService->>SkillPackageStoragePort: store and validate content
SkillPackageAssetService->>AssetRegistryService: persist asset and reference
SkillReleaseDeliveryService->>SkillPackageStoragePort: open authorized release content
SkillReleaseDeliveryService-->>SkillRegistryService: return release descriptor and content
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
TegamiThis repository uses Tegami to manage releases. When your changes affect published packages, add a changelog file under Create a changelog → · Changelog format Release preview
This PR does not add changelog files. Pending changelogs from other branches are included in the preview above. Run Managed by Tegami. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java (1)
82-103: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCatch Jackson 3 write failures in the staging handlers.
upload()callsjson.writeValueAsString(spec)with Jackson 3 (tools.jackson.databind). Serialization failures are unchecked (JacksonException/RuntimeException), so thecatch (IOException failure)blocks inimportPackageandreplacePackagedo not wrap the failure asBusinessUnavailableException("skill.package-staging-unavailable", ...).Catch
tools.jackson.core.exception.JacksonExceptionhere, or match the broader handling pattern used by nearby profile code. Apply the same change to bothimportPackageandreplacePackage.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java` around lines 82 - 103, Update the exception handling in both importPackage and replacePackage to catch Jackson 3 serialization failures from upload(), including JacksonException (or the broader established profile-handling type), and wrap them as BusinessUnavailableException with the existing "skill.package-staging-unavailable" code and context. Preserve the existing IOException handling and staging behavior.core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java (1)
63-81: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the remaining failure branches introduced by this split.
The current tests do not cover:
SkillDistributionService.open()'s own catch-and-close block, which runs whenmanifest(content.descriptor())fails (for example, aspec.artifact()vsdescriptor.artifact()mismatch inverifyReference, orspecs.read()throwing) afterdeliveries.open()already returned content. In the fixture,spec().artifact().sha256()andreference.getDigest()always agree, soverifyReferencenever fails in any test.SkillReleaseDeliveryService's!reference.isBlobReference()branch and its "reference not found" branch.Add tests for these branches to confirm the content stream is closed and the correct exception type is thrown in each case.
Also applies to: 171-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java` around lines 63 - 81, Extend the tests around SkillDistributionService.open() to force manifest(content.descriptor()) to fail after delivery succeeds, covering both an artifact-versus-descriptor mismatch in verifyReference and a specs.read() exception; assert the stream closes and the expected exception is thrown. Add SkillReleaseDeliveryService tests for a non-blob reference and a missing reference, verifying each branch’s expected exception type and stream-closure behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageArtifact.java`:
- Line 18: Update the mediaType validation in SkillPackageArtifact so it accepts
only ZIP_MEDIA_TYPE (application/zip), rejecting text/plain and all other values
while preserving the existing required-text and length validation.
In
`@core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageAssetService.java`:
- Around line 57-90: In importPackage and replacePackage, validate the
payload/artifact pairing with payloadPolicy.validate using upload.payload() and
upload.artifact() before calling store(actor, upload). Remove the later
duplicate validation after artifact retrieval, while preserving the existing
stored-artifact consistency checks and cleanup behavior for subsequent failures.
In
`@core/src/main/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryService.java`:
- Around line 139-149: Apply the same normalizeCoordinate validation used by
resolution to the write path in SkillPackageAssetService.importPackage before
registering the asset, covering both namespace and upload.slug(). Ensure
SkillPackageAssetService or the registration flow rejects null, blank, overlong,
or invalid-character coordinates before persistence, while preserving the
normalized values for registration.
---
Outside diff comments:
In
`@core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java`:
- Around line 82-103: Update the exception handling in both importPackage and
replacePackage to catch Jackson 3 serialization failures from upload(),
including JacksonException (or the broader established profile-handling type),
and wrap them as BusinessUnavailableException with the existing
"skill.package-staging-unavailable" code and context. Preserve the existing
IOException handling and staging behavior.
In
`@core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java`:
- Around line 63-81: Extend the tests around SkillDistributionService.open() to
force manifest(content.descriptor()) to fail after delivery succeeds, covering
both an artifact-versus-descriptor mismatch in verifyReference and a
specs.read() exception; assert the stream closes and the expected exception is
thrown. Add SkillReleaseDeliveryService tests for a non-blob reference and a
missing reference, verifying each branch’s expected exception type and
stream-closure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8fef9ec0-c1c7-402c-acbd-046c41954e30
⛔ Files ignored due to path filters (6)
docs/increments/active/2026-07-31-spring-modulith-package-refactor/assetregistry-skill-challenge-brief.mdis excluded by!docs/**docs/increments/active/2026-07-31-spring-modulith-package-refactor/assetregistry-skill-challenge-verdict.mdis excluded by!docs/**docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.mdis excluded by!docs/**docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.mdis excluded by!docs/**docs/specs/domains/asset-registry.mdis excluded by!docs/**docs/tests/domains/asset-registry.mdis excluded by!docs/**
📒 Files selected for processing (37)
ARCHITECTURE.mdapps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.javaapps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javaapps/worker/src/main/java/com/orgmemory/worker/assetregistry/SkillPackageSupersessionCleanupScheduler.javaapps/worker/src/test/java/com/orgmemory/worker/SkillCapabilityBoundaryTests.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetPayloadReference.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageAssetService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageCleanupOutcome.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryService.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupOperations.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupSummary.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseContent.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDeliveryQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDescriptor.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageArtifact.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageAssetCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageUpload.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/SkillPackageStoragePort.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/package-info.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinatorTests.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioObjectStorageAutoConfiguration.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapter.javaintegrations/object-storage-minio/src/test/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapterTests.java
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend · Java 25
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Treat the repository and runtime evidence as the engineering system of record; do not treat chat or Northstar as authoritative.
Before changing a domain, read its specification, test-coverage document, and applicable decision filenames.
Material decisions about domain boundaries, authorization, persistence, publication, concurrency, cache isolation, parity scope, or deployment require an independent architecture challenge and documented alternatives before implementation.
Do not use completed increment documents as the source for current behavior; use them only for history or archaeology.
Before using unfamiliar Spring Boot, Spring Modulith, Spring AI, Gradle, React, Vite, Tailwind, TypeScript, Next.js, or Fumadocs APIs, consult current official documentation, Context7, and the relevant verification skill.
Readdocs/guidelines/agent-safety.mdbefore retrieval, AI, MCP, permission, upload, graph, or export work; never commit secrets or customer data.
Keepddl-auto=validateand pair every persisted-model change with a Flyway migration.
Use the testing harness; a terminating clean test is the JVM context gate, andbootRunis not verification.
Files:
core/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/package-info.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapter.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/package-info.javaapps/worker/src/test/java/com/orgmemory/worker/SkillCapabilityBoundaryTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.javaapps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupSummary.javaapps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDescriptor.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDeliveryQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageAssetCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageArtifact.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageCleanupOutcome.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupOperations.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioObjectStorageAutoConfiguration.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageUpload.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinatorTests.javaintegrations/object-storage-minio/src/test/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapterTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javaapps/worker/src/main/java/com/orgmemory/worker/assetregistry/SkillPackageSupersessionCleanupScheduler.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseContent.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetPayloadReference.javaARCHITECTURE.mdcore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageAssetService.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryService.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/SkillPackageStoragePort.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Apply IDE inspection only to edited backend Java files.
Files:
core/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/package-info.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapter.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/package-info.javaapps/worker/src/test/java/com/orgmemory/worker/SkillCapabilityBoundaryTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.javaapps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupSummary.javaapps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDescriptor.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDeliveryQuery.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageAssetCommand.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageArtifact.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageCleanupOutcome.javacore/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupOperations.javaintegrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioObjectStorageAutoConfiguration.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinator.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageUpload.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinatorTests.javaintegrations/object-storage-minio/src/test/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapterTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skillpackage/package-info.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.javaapps/worker/src/main/java/com/orgmemory/worker/assetregistry/SkillPackageSupersessionCleanupScheduler.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseContent.javacore/src/main/java/com/orgmemory/core/assetregistry/AssetPayloadReference.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.javacore/src/test/java/com/orgmemory/core/ModulithVerificationTests.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageAssetService.javacore/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryService.javacore/src/main/java/com/orgmemory/core/assetregistry/skillstorage/SkillPackageStoragePort.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.javacore/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java
🪛 ast-grep (0.45.0)
core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java
[warning] 1290-1290: Avoid user-generated class names for reflection
Context: Class.forName(name)
Note: [CWE-470] Use of Externally-Controlled Input to Select Classes or Code ('Unsafe Reflection').
(unsafe-reflection-java)
🔇 Additional comments (41)
core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupService.java (1)
3-4: LGTM!Also applies to: 13-14, 26-31, 40-48
core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageCleanupOutcome.java (1)
3-3: LGTM!apps/worker/src/main/java/com/orgmemory/worker/assetregistry/SkillPackageSupersessionCleanupScheduler.java (1)
3-4: LGTM!Also applies to: 16-29
core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinatorTests.java (1)
4-58: LGTM!apps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.java (1)
1-33: LGTM!apps/worker/src/test/java/com/orgmemory/worker/SkillCapabilityBoundaryTests.java (1)
1-31: LGTM!apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java (1)
43-43: LGTM!integrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioObjectStorageAutoConfiguration.java (1)
3-3: LGTM!integrations/object-storage-minio/src/main/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapter.java (1)
3-3: LGTM!integrations/object-storage-minio/src/test/java/com/orgmemory/integrations/storage/minio/MinioSkillPackageStorageAdapterTests.java (1)
3-11: LGTM!Also applies to: 23-29
ARCHITECTURE.md (1)
207-216: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageArtifact.java (1)
5-17: LGTM!Also applies to: 21-38
core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageAssetCommand.java (1)
7-25: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.java (1)
3-6: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackageUpload.java (1)
8-31: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/package-info.java (1)
1-2: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDeliveryQuery.java (1)
6-19: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseDescriptor.java (1)
7-14: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/SkillReleaseContent.java (1)
7-19: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skilldelivery/package-info.java (1)
1-2: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupOperations.java (1)
3-5: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/SkillPackageCleanupSummary.java (1)
3-24: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillcleanup/package-info.java (1)
1-2: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/skillstorage/SkillPackageStoragePort.java (1)
1-4: LGTM!Also applies to: 28-29, 45-46, 72-83
core/src/main/java/com/orgmemory/core/assetregistry/skillstorage/package-info.java (1)
1-2: LGTM!core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java (1)
1193-1308: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageAssetService.java (1)
1-56: LGTM!Also applies to: 125-185
core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.java (1)
3-16: LGTM!Also applies to: 37-48
core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java (1)
1-81: LGTM!Also applies to: 105-113, 136-154
core/src/main/java/com/orgmemory/core/assetregistry/AssetPayloadReference.java (1)
3-3: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java (1)
3-3: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryService.java (1)
3-3: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSupersessionCleanupCoordinator.java (1)
3-3: LGTM!core/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.java (1)
11-20: LGTM!Also applies to: 45-236
core/src/main/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryService.java (4)
21-77: LGTM!
79-102: 🔒 Security & PrivacyVerify that storage exception details do not surface to API clients.
open()wraps the raw storageRuntimeExceptionas the cause ofAssetUnavailableExceptionat Line 87-88. If the API layer serializes exception causes (directly or via a generic error handler that includesgetCause()details) rather than only the top-level message, a MinIO/storage-layer message containing the object key could leak to the client. This would undercut the stated goal of preventing storage locator exposure.Confirm that the global exception handler for
AssetUnavailableExceptiononly returns the top-level message to clients, and that causes are used for server-side logging only.
104-137: LGTM!
151-155: LGTM!core/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.java (1)
3-139: LGTM!core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java (2)
19-19: LGTM!
83-144: LGTM!
|
CodeRabbit follow-up for the outside-diff findings:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Verification
clean test --max-workers=1: 108 tasks in 7m15sskip-release: intermediate modular refactor; release follows completion of the full refactor goal
Summary by CodeRabbit
New Features
Bug Fixes
Tests