From 4c7a8bf2cf77ac4ad93f6bbfb99a441419dd679a Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:23:22 +0700 Subject: [PATCH 1/6] refactor(assetregistry): close skill semantics module --- .../AssetConsumptionController.java | 8 +- .../AssetDeliveryController.java | 8 +- .../AssetRegistryController.java | 73 +++- .../api/SkillCapabilityBoundaryTests.java | 25 ++ .../AssetConsumptionControllerTests.java | 14 +- .../AssetDeliveryControllerSecurityTests.java | 8 +- .../AssetRegistryIntegrationTests.java | 58 +-- .../SkillDistributionControllerTests.java | 14 +- .../AssetRegistryCoordinator.java | 34 +- .../skill/SkillDistributionOperations.java | 19 + .../{ => skill}/SkillDistributionService.java | 7 +- .../{ => skill}/SkillGitHubImportService.java | 99 +---- .../skill/SkillGitHubOperations.java | 95 +++++ .../{ => skill}/SkillGitHubSourcePort.java | 9 +- .../{ => skill}/SkillInstallManifest.java | 2 +- .../{ => skill}/SkillPackageContent.java | 2 +- .../{ => skill}/SkillPackageInspection.java | 7 +- .../{ => skill}/SkillPackageInspector.java | 2 +- .../skill/SkillPackageOperations.java | 27 ++ .../{ => skill}/SkillPackageProfile.java | 11 +- .../{ => skill}/SkillPackageSpec.java | 11 +- .../SkillPackageValidationException.java | 4 +- .../{ => skill}/SkillRegistryService.java | 29 +- .../assetregistry/skill/package-info.java | 14 + .../SkillPackagePayloadPolicy.java | 2 + .../core/ModulithVerificationTests.java | 91 ++++- .../AssetProfileValidationTests.java | 41 +-- .../SkillDistributionServiceTests.java | 341 ------------------ .../SkillPackageAssetServiceTests.java | 104 +++++- .../SkillRegistryServiceTests.java | 291 --------------- .../SkillReleaseDeliveryServiceTests.java | 212 +++++++++++ .../skill/SkillDistributionServiceTests.java | 186 ++++++++++ .../SkillGitHubImportServiceTests.java | 72 ++-- .../SkillPackageInspectorTests.java | 2 +- .../skill/SkillPackageProfileTests.java | 33 ++ .../skill/SkillRegistryServiceTests.java | 170 +++++++++ .../GitHubConnectorAutoConfiguration.java | 2 +- .../github/GitHubSkillArchiveReader.java | 2 +- .../github/GitHubSkillSourceAdapter.java | 13 +- ...GitHubConnectorAutoConfigurationTests.java | 9 +- .../github/GitHubSkillArchiveReaderTests.java | 2 +- .../github/GitHubSkillSourceAdapterTests.java | 7 +- 42 files changed, 1234 insertions(+), 926 deletions(-) create mode 100644 core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionOperations.java rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillDistributionService.java (97%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillGitHubImportService.java (75%) create mode 100644 core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubOperations.java rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillGitHubSourcePort.java (94%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillInstallManifest.java (95%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageContent.java (92%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageInspection.java (76%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageInspector.java (99%) create mode 100644 core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageOperations.java rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageProfile.java (84%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageSpec.java (96%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageValidationException.java (81%) rename core/src/main/java/com/orgmemory/core/assetregistry/{ => skill}/SkillRegistryService.java (91%) create mode 100644 core/src/main/java/com/orgmemory/core/assetregistry/skill/package-info.java delete mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java delete mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.java create mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryServiceTests.java create mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillDistributionServiceTests.java rename core/src/test/java/com/orgmemory/core/assetregistry/{ => skill}/SkillGitHubImportServiceTests.java (78%) rename core/src/test/java/com/orgmemory/core/assetregistry/{ => skill}/SkillPackageInspectorTests.java (99%) create mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfileTests.java create mode 100644 core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java diff --git a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetConsumptionController.java b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetConsumptionController.java index 9d292347..0620a7dc 100644 --- a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetConsumptionController.java +++ b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetConsumptionController.java @@ -14,8 +14,8 @@ import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; import com.orgmemory.core.assetregistry.promptcontract.PromptRenderResult; import com.orgmemory.core.assetregistry.promptcontract.PromptRunResult; -import com.orgmemory.core.assetregistry.SkillDistributionService; -import com.orgmemory.core.assetregistry.SkillInstallManifest; +import com.orgmemory.core.assetregistry.skill.SkillDistributionOperations; +import com.orgmemory.core.assetregistry.skill.SkillInstallManifest; import com.orgmemory.core.assetregistry.WorkInstructionService; import com.orgmemory.core.assetregistry.WorkInstructionView; import io.swagger.v3.oas.annotations.Operation; @@ -46,7 +46,7 @@ class AssetConsumptionController { private final PromptExecutionService prompts; private final WorkInstructionService instructions; private final CapabilityPackService packs; - private final SkillDistributionService skills; + private final SkillDistributionOperations skills; AssetConsumptionController( CurrentActorProvider actors, @@ -54,7 +54,7 @@ class AssetConsumptionController { PromptExecutionService prompts, WorkInstructionService instructions, CapabilityPackService packs, - SkillDistributionService skills) { + SkillDistributionOperations skills) { this.actors = actors; this.assets = assets; this.prompts = prompts; diff --git a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetDeliveryController.java b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetDeliveryController.java index faf468d5..bc993e7b 100644 --- a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetDeliveryController.java +++ b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetDeliveryController.java @@ -9,8 +9,8 @@ import com.orgmemory.core.assetregistry.CapabilityPackDefinition; import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; import com.orgmemory.core.assetregistry.promptcontract.PromptRenderResult; -import com.orgmemory.core.assetregistry.SkillDistributionService; -import com.orgmemory.core.assetregistry.SkillInstallManifest; +import com.orgmemory.core.assetregistry.skill.SkillDistributionOperations; +import com.orgmemory.core.assetregistry.skill.SkillInstallManifest; import io.swagger.v3.oas.annotations.Operation; import java.nio.charset.StandardCharsets; import java.util.List; @@ -45,13 +45,13 @@ class AssetDeliveryController { private final AssetDeliveryService delivery; private final PromptExecutionService prompts; - private final SkillDistributionService skills; + private final SkillDistributionOperations skills; private final CurrentActorProvider actors; AssetDeliveryController( AssetDeliveryService delivery, PromptExecutionService prompts, - SkillDistributionService skills, + SkillDistributionOperations skills, CurrentActorProvider actors) { this.delivery = delivery; this.prompts = prompts; diff --git a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java index afdf7988..8c98e0fd 100644 --- a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java +++ b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java @@ -12,9 +12,10 @@ import com.orgmemory.core.assetregistry.AssetSummary; import com.orgmemory.core.assetregistry.AssetSummaryPage; import com.orgmemory.core.assetregistry.AssetView; -import com.orgmemory.core.assetregistry.SkillGitHubImportService; -import com.orgmemory.core.assetregistry.SkillPackageInspection; -import com.orgmemory.core.assetregistry.SkillRegistryService; +import com.orgmemory.core.assetregistry.skill.SkillGitHubOperations; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillPackageInspection; +import com.orgmemory.core.assetregistry.skill.SkillPackageOperations; import com.orgmemory.core.permission.KnowledgeClassification; import io.swagger.v3.oas.annotations.Operation; import jakarta.validation.constraints.NotBlank; @@ -54,14 +55,14 @@ AssetType domainType() { } private final AssetRegistryService assets; - private final SkillRegistryService skills; - private final SkillGitHubImportService skillGitHub; + private final SkillPackageOperations skills; + private final SkillGitHubOperations skillGitHub; private final CurrentActorProvider actors; AssetRegistryController( AssetRegistryService assets, - SkillRegistryService skills, - SkillGitHubImportService skillGitHub, + SkillPackageOperations skills, + SkillGitHubOperations skillGitHub, CurrentActorProvider actors) { this.assets = assets; this.skills = skills; @@ -124,8 +125,8 @@ record GitHubSkillSourceRequest( String connectionKey, @NotNull UUID knowledgeSpaceId) { - SkillGitHubImportService.SourceRequest source() { - return new SkillGitHubImportService.SourceRequest( + SkillGitHubOperations.SourceRequest source() { + return new SkillGitHubOperations.SourceRequest( repository, revision, subpath, connectionKey, knowledgeSpaceId); } } @@ -137,6 +138,21 @@ record GitHubSkillImportRequest( KnowledgeClassification classification) { } + record ImportResult( + String repository, + String revision, + SkillGitHubSourcePort.Visibility visibility, + List skills) { + } + + record ImportItem( + String path, + boolean imported, + AssetView asset, + String errorCode, + String errorMessage) { + } + record AssetAvailabilityRequest(String reason) { } @@ -178,13 +194,15 @@ AssetView importSkill( KnowledgeClassification classification, Authentication authentication) { try (var content = file.getInputStream()) { - return skills.importPackage( - actors.current(authentication), + var actor = actors.current(authentication); + UUID assetId = skills.importPackage( + actor, namespace, knowledgeSpaceId, classification, file.getSize(), content); + return assets.get(actor, assetId); } catch (IOException failure) { throw new ApiRequestException( "The Skill package could not be read", failure); @@ -213,7 +231,7 @@ SkillPackageInspection inspectSkill( @Operation( operationId = "previewGitHubSkills", summary = "Discover and validate Skills at one GitHub repository revision") - SkillGitHubImportService.Preview previewGitHubSkills( + SkillGitHubOperations.Preview previewGitHubSkills( @Valid @RequestBody GitHubSkillSourceRequest request, Authentication authentication) { return skillGitHub.preview( @@ -224,7 +242,7 @@ SkillGitHubImportService.Preview previewGitHubSkills( @Operation( operationId = "listGitHubSkillConnections", summary = "List approved GitHub connections available for private Skill import") - List + List listGitHubSkillConnections( @RequestParam UUID knowledgeSpaceId, Authentication authentication) { @@ -236,18 +254,33 @@ SkillGitHubImportService.Preview previewGitHubSkills( @Operation( operationId = "importGitHubSkills", summary = "Import selected Skills from an exact GitHub commit") - SkillGitHubImportService.ImportResult importGitHubSkills( + ImportResult importGitHubSkills( @Valid @RequestBody GitHubSkillImportRequest request, Authentication authentication) { - return skillGitHub.importSelected( - actors.current(authentication), - new SkillGitHubImportService.ImportRequest( + var actor = actors.current(authentication); + SkillGitHubOperations.ImportResult result = skillGitHub.importSelected( + actor, + new SkillGitHubOperations.ImportRequest( request.source().source(), request.paths(), request.namespace(), request.classification() == null ? KnowledgeClassification.INTERNAL : request.classification())); + return new ImportResult( + result.repository(), + result.revision(), + result.visibility(), + result.skills().stream() + .map(item -> new ImportItem( + item.path(), + item.imported(), + item.imported() + ? assets.get(actor, item.assetId()) + : null, + item.errorCode(), + item.errorMessage())) + .toList()); } @PutMapping( @@ -262,12 +295,14 @@ AssetView replaceSkillDraft( @RequestParam long expectedLockVersion, Authentication authentication) { try (var content = file.getInputStream()) { - return skills.replacePackage( - actors.current(authentication), + var actor = actors.current(authentication); + UUID replacedId = skills.replacePackage( + actor, assetId, expectedLockVersion, file.getSize(), content); + return assets.get(actor, replacedId); } catch (IOException failure) { throw new ApiRequestException( "The Skill package could not be read", failure); diff --git a/apps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.java b/apps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.java index ca7a6fb2..2a9d7f3e 100644 --- a/apps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/SkillCapabilityBoundaryTests.java @@ -30,4 +30,29 @@ void apiDoesNotImportParentSkillCapabilities() { assertEquals(Set.of(), dependencies); } + + @Test + void apiImportsOnlyTheSkillApplicationSurface() { + var dependencies = new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("com.orgmemory.api") + .stream() + .flatMap(type -> type.getDirectDependenciesFromSelf().stream()) + .map(dependency -> dependency.getTargetClass().getName()) + .filter(name -> name.startsWith( + "com.orgmemory.core.assetregistry.skill.")) + .map(name -> name.replaceFirst("\\$.*$", "")) + .collect(TreeSet::new, Set::add, Set::addAll); + + assertEquals( + Set.of( + "com.orgmemory.core.assetregistry.skill.SkillDistributionOperations", + "com.orgmemory.core.assetregistry.skill.SkillGitHubOperations", + "com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort", + "com.orgmemory.core.assetregistry.skill.SkillInstallManifest", + "com.orgmemory.core.assetregistry.skill.SkillPackageContent", + "com.orgmemory.core.assetregistry.skill.SkillPackageInspection", + "com.orgmemory.core.assetregistry.skill.SkillPackageOperations"), + dependencies); + } } diff --git a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetConsumptionControllerTests.java b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetConsumptionControllerTests.java index 3aafad23..bca806e2 100644 --- a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetConsumptionControllerTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetConsumptionControllerTests.java @@ -13,8 +13,8 @@ import com.orgmemory.core.assetregistry.AssetRegistryService; import com.orgmemory.core.assetregistry.CapabilityPackService; import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; -import com.orgmemory.core.assetregistry.SkillDistributionService; -import com.orgmemory.core.assetregistry.SkillInstallManifest; +import com.orgmemory.core.assetregistry.skill.SkillDistributionOperations; +import com.orgmemory.core.assetregistry.skill.SkillInstallManifest; import com.orgmemory.core.assetregistry.WorkInstructionService; import com.orgmemory.core.organization.CurrentActor; import java.time.Instant; @@ -46,8 +46,8 @@ class AssetConsumptionControllerTests { @Test void browserSessionReadsTheExactSkillInstallContract() { CurrentActorProvider actors = mock(CurrentActorProvider.class); - SkillDistributionService skills = - mock(SkillDistributionService.class); + SkillDistributionOperations skills = + mock(SkillDistributionOperations.class); OAuth2AuthenticationToken authentication = browserSession(); SkillInstallManifest manifest = manifest(); when(actors.current(authentication)).thenReturn(ACTOR); @@ -69,8 +69,8 @@ void browserSessionReadsTheExactSkillInstallContract() { @Test void bearerTokenCannotBypassDeliveryScopeThroughTheBrowserEndpoint() { CurrentActorProvider actors = mock(CurrentActorProvider.class); - SkillDistributionService skills = - mock(SkillDistributionService.class); + SkillDistributionOperations skills = + mock(SkillDistributionOperations.class); AssetConsumptionController controller = controller(actors, skills); var authentication = new TestingAuthenticationToken( @@ -88,7 +88,7 @@ void bearerTokenCannotBypassDeliveryScopeThroughTheBrowserEndpoint() { private static AssetConsumptionController controller( CurrentActorProvider actors, - SkillDistributionService skills) { + SkillDistributionOperations skills) { return new AssetConsumptionController( actors, mock(AssetRegistryService.class), diff --git a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetDeliveryControllerSecurityTests.java b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetDeliveryControllerSecurityTests.java index 287a87f6..b94f9c71 100644 --- a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetDeliveryControllerSecurityTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetDeliveryControllerSecurityTests.java @@ -6,7 +6,7 @@ import com.orgmemory.api.security.CurrentActorProvider; import com.orgmemory.core.assetregistry.AssetDeliveryService; import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; -import com.orgmemory.core.assetregistry.SkillDistributionService; +import com.orgmemory.core.assetregistry.skill.SkillDistributionOperations; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -93,8 +93,8 @@ PromptExecutionService prompts() { } @Bean - SkillDistributionService skills() { - return mock(SkillDistributionService.class); + SkillDistributionOperations skills() { + return mock(SkillDistributionOperations.class); } @Bean @@ -106,7 +106,7 @@ CurrentActorProvider actors() { AssetDeliveryController controller( AssetDeliveryService delivery, PromptExecutionService prompts, - SkillDistributionService skills, + SkillDistributionOperations skills, CurrentActorProvider actors) { return new AssetDeliveryController( delivery, prompts, skills, actors); diff --git a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java index ab5cd91c..d1b92560 100644 --- a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryIntegrationTests.java @@ -41,7 +41,7 @@ import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; import com.orgmemory.core.assetregistry.promptcontract.PromptRunResult; import com.orgmemory.core.assetregistry.skillstorage.SkillPackageStoragePort; -import com.orgmemory.core.assetregistry.SkillRegistryService; +import com.orgmemory.core.assetregistry.skill.SkillPackageOperations; import com.orgmemory.core.assetregistry.WorkInstructionService; import com.orgmemory.core.assetregistry.WorkInstructionView; import com.orgmemory.core.assistant.AssistantAssetToolService; @@ -149,7 +149,7 @@ class AssetRegistryIntegrationTests { AssetRegistryService assets; @Autowired - SkillRegistryService skills; + SkillPackageOperations skills; @Autowired AssetDeliveryService delivery; @@ -1016,13 +1016,7 @@ void onlyDraftPayloadReferencesMayBeDeletedWhileAllReferenceUpdatesStayRejected( byte[] archive = skillArchive( "reference-guard", "Prove mutable Draft pointers do not weaken immutable package pins."); - AssetView created = skills.importPackage( - AUTHOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - archive.length, - new ByteArrayInputStream(archive)); + AssetView created = importSkill(archive); assets.publishSkillDraft(AUTHOR, created.id(), "1.0.0"); UUID draftReferenceId = jdbc.queryForObject( @@ -1063,13 +1057,7 @@ void replacingAReleasedSkillDraftKeepsTheImmutablePackageAndClearsTheCleanupRow( byte[] original = skillArchive( "replacement-history", "The original Skill package remains pinned by its release."); - AssetView created = skills.importPackage( - AUTHOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - original.length, - new ByteArrayInputStream(original)); + AssetView created = importSkill(original); AssetView published = assets.publishSkillDraft(AUTHOR, created.id(), "1.0.0"); String originalKey = jdbc.queryForObject( "SELECT reference_value FROM asset_payload_references WHERE owner_kind = 'DRAFT'", @@ -1078,12 +1066,13 @@ void replacingAReleasedSkillDraftKeepsTheImmutablePackageAndClearsTheCleanupRow( "replacement-history", "The mutable Draft now points at a separately validated package."); - AssetView replaced = skills.replacePackage( + UUID replacedId = skills.replacePackage( AUTHOR, created.id(), published.draft().lockVersion(), replacement.length, new ByteArrayInputStream(replacement)); + AssetView replaced = assets.get(AUTHOR, replacedId); String replacementKey = jdbc.queryForObject( "SELECT reference_value FROM asset_payload_references WHERE owner_kind = 'DRAFT'", @@ -1112,13 +1101,7 @@ void replacingAnUnreleasedSkillDraftDeletesItsUnreferencedOldPackage() byte[] original = skillArchive( "replacement-cleanup", "The original package has no immutable consumers."); - AssetView created = skills.importPackage( - AUTHOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - original.length, - new ByteArrayInputStream(original)); + AssetView created = importSkill(original); String originalKey = jdbc.queryForObject( "SELECT reference_value FROM asset_payload_references WHERE owner_kind = 'DRAFT'", String.class); @@ -1146,13 +1129,7 @@ void skillImportPublishesDirectlyAndPinsTheValidatedBlob() throws Exception { byte[] archive = skillArchive( "support-triage", "Triage a support ticket using the approved company workflow."); - AssetView created = skills.importPackage( - AUTHOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - archive.length, - new ByteArrayInputStream(archive)); + AssetView created = importSkill(archive); assertEquals(AssetType.SKILL, created.type()); assertEquals("support-triage", created.slug()); assertFalse(created.draft().payload().contains("assets/skills/")); @@ -1233,13 +1210,7 @@ void directSkillPublicationDoesNotBypassAnActiveReview() throws Exception { byte[] archive = skillArchive( "reviewed-skill", "A Skill whose author explicitly selected the reviewed path."); - AssetView created = skills.importPackage( - AUTHOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - archive.length, - new ByteArrayInputStream(archive)); + AssetView created = importSkill(archive); AssetView submitted = assets.submit(AUTHOR, created.id(), "Request independent review"); @@ -1658,6 +1629,17 @@ private static AssetDraftInput input(String payload) { """.formatted(escapedPayload)); } + private AssetView importSkill(byte[] archive) { + UUID assetId = skills.importPackage( + AUTHOR, + "support", + SPACE_ID, + KnowledgeClassification.INTERNAL, + archive.length, + new ByteArrayInputStream(archive)); + return assets.get(AUTHOR, assetId); + } + private static byte[] skillArchive(String name, String description) throws IOException { ByteArrayOutputStream output = new ByteArrayOutputStream(); diff --git a/apps/api/src/test/java/com/orgmemory/api/assetregistry/SkillDistributionControllerTests.java b/apps/api/src/test/java/com/orgmemory/api/assetregistry/SkillDistributionControllerTests.java index 49fa69cb..79259953 100644 --- a/apps/api/src/test/java/com/orgmemory/api/assetregistry/SkillDistributionControllerTests.java +++ b/apps/api/src/test/java/com/orgmemory/api/assetregistry/SkillDistributionControllerTests.java @@ -12,9 +12,9 @@ import com.orgmemory.core.assetregistry.AssetDeliveryService; import com.orgmemory.core.assetregistry.consumption.AssetPublicationMode; import com.orgmemory.core.assetregistry.prompt.PromptExecutionService; -import com.orgmemory.core.assetregistry.SkillDistributionService; -import com.orgmemory.core.assetregistry.SkillInstallManifest; -import com.orgmemory.core.assetregistry.SkillPackageContent; +import com.orgmemory.core.assetregistry.skill.SkillDistributionOperations; +import com.orgmemory.core.assetregistry.skill.SkillInstallManifest; +import com.orgmemory.core.assetregistry.skill.SkillPackageContent; import com.orgmemory.core.organization.CurrentActor; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -46,8 +46,8 @@ void streamsTheVerifiedPackageWithoutExposingItsStorageReference() TrackingInputStream stream = new TrackingInputStream(bytes); SkillInstallManifest manifest = manifest(bytes.length); - SkillDistributionService skills = - mock(SkillDistributionService.class); + SkillDistributionOperations skills = + mock(SkillDistributionOperations.class); CurrentActorProvider actors = mock(CurrentActorProvider.class); Authentication authentication = mock(Authentication.class); when(actors.current(authentication)).thenReturn(ACTOR); @@ -87,8 +87,8 @@ void streamsTheVerifiedPackageWithoutExposingItsStorageReference() void closesThePackageIfResponseMetadataCannotBeBuilt() { TrackingInputStream stream = new TrackingInputStream(new byte[] {1}); - SkillDistributionService skills = - mock(SkillDistributionService.class); + SkillDistributionOperations skills = + mock(SkillDistributionOperations.class); CurrentActorProvider actors = mock(CurrentActorProvider.class); Authentication authentication = mock(Authentication.class); when(actors.current(authentication)).thenReturn(ACTOR); diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java b/core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java index 081a0405..f26075ff 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/AssetRegistryCoordinator.java @@ -1,5 +1,7 @@ package com.orgmemory.core.assetregistry; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageArtifact; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackagePayloadPolicy; import com.orgmemory.core.assetregistry.skillstorage.SkillPackageStoragePort; import com.orgmemory.core.assetregistry.consumption.AssetAvailability; import com.orgmemory.core.assetregistry.consumption.AssetConsumptionRelease; @@ -56,7 +58,7 @@ class AssetRegistryCoordinator { private final AssetPayloadReferenceRepository payloadReferences; private final SkillPackageSupersessionRepository packageSupersessions; private final AssetTypeProfileRegistry profiles; - private final SkillPackageSpecReader skillPackages; + private final SkillPackagePayloadPolicy skillPackages; private final AssetPayloadDigester digester; AssetRegistryCoordinator( @@ -77,7 +79,7 @@ class AssetRegistryCoordinator { AssetPayloadReferenceRepository payloadReferences, SkillPackageSupersessionRepository packageSupersessions, AssetTypeProfileRegistry profiles, - SkillPackageSpecReader skillPackages, + SkillPackagePayloadPolicy skillPackages, AssetPayloadDigester digester) { this.registrations = registrations; this.identities = identities; @@ -179,8 +181,8 @@ private UUID create( throw new IllegalArgumentException( "Only Skill Assets may carry a package reference"); } - SkillPackageSpec spec = skillSpec(canonical.payload()); - requireMatchingPackage(spec.artifact(), storedPackage); + SkillPackageArtifact artifact = skillArtifact(canonical.payload()); + requireMatchingPackage(artifact, storedPackage); payloadReferences.saveAndFlush( AssetPayloadReference.forDraft(draft, storedPackage)); } @@ -460,8 +462,8 @@ SkillDraftReplacement replaceSkillDraft( } AssetPayloadDigester.CanonicalAssetPayload canonical = validateDraft(AssetType.SKILL, input); - SkillPackageSpec spec = skillSpec(canonical.payload()); - requireMatchingPackage(spec.artifact(), storedPackage); + SkillPackageArtifact artifact = skillArtifact(canonical.payload()); + requireMatchingPackage(artifact, storedPackage); AssetPayloadReference previous = payloadReferences .findByDraftIdAndOrganizationId(draft.getId(), actor.organizationId()) .orElseThrow(() -> new AssetConflictException( @@ -535,13 +537,13 @@ AssetView submit(CurrentActor actor, UUID assetId, String changeNote) { AssetReviewCase review = reviews.saveAndFlush(new AssetReviewCase( revision, REVIEW_POLICY_VERSION, actor.userId())); if (asset.type() == AssetType.SKILL) { - SkillPackageSpec spec = skillSpec(draft.getPayload()); + SkillPackageArtifact artifact = skillArtifact(draft.getPayload()); AssetPayloadReference draftReference = payloadReferences .findByDraftIdAndOrganizationId( draft.getId(), actor.organizationId()) .orElseThrow(() -> new AssetConflictException( "The Skill draft is missing its package reference")); - requireMatchingPackage(spec.artifact(), draftReference); + requireMatchingPackage(artifact, draftReference); payloadReferences.saveAndFlush( AssetPayloadReference.forRevision(revision, draftReference)); } @@ -760,8 +762,8 @@ AssetView publishSkillDraft( draft.getId(), actor.organizationId()) .orElseThrow(() -> new AssetConflictException( "The Skill draft is missing its package reference")); - SkillPackageSpec spec = skillSpec(draft.getPayload()); - requireMatchingPackage(spec.artifact(), draftReference); + SkillPackageArtifact artifact = skillArtifact(draft.getPayload()); + requireMatchingPackage(artifact, draftReference); AssetRevision revision; try { @@ -842,8 +844,8 @@ private AssetRelease createRelease( revision.getId(), actor.organizationId()) .orElseThrow(() -> new AssetConflictException( "The Skill revision is missing its package reference")); - SkillPackageSpec spec = skillSpec(revision.getPayload()); - requireMatchingPackage(spec.artifact(), revisionReference); + SkillPackageArtifact artifact = skillArtifact(revision.getPayload()); + requireMatchingPackage(artifact, revisionReference); payloadReferences.saveAndFlush( AssetPayloadReference.forRelease(release, revisionReference)); } @@ -1154,12 +1156,12 @@ private static String validatedVersionLabel(String versionLabel) { } } - private SkillPackageSpec skillSpec(String payload) { - return skillPackages.read(payload); + private SkillPackageArtifact skillArtifact(String payload) { + return skillPackages.artifact(payload); } private static void requireMatchingPackage( - SkillPackageSpec.Artifact artifact, + SkillPackageArtifact artifact, SkillPackageStoragePort.StoredSkillPackage stored) { if (!stored.sha256().equals(artifact.sha256()) || stored.contentLength() != artifact.contentLength() @@ -1170,7 +1172,7 @@ private static void requireMatchingPackage( } private static void requireMatchingPackage( - SkillPackageSpec.Artifact artifact, + SkillPackageArtifact artifact, AssetPayloadReference reference) { if (!reference.isBlobReference() || reference.getDigest() == null diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionOperations.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionOperations.java new file mode 100644 index 00000000..d15ea048 --- /dev/null +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionOperations.java @@ -0,0 +1,19 @@ +package com.orgmemory.core.assetregistry.skill; + +import com.orgmemory.core.organization.CurrentActor; +import java.util.UUID; + +public interface SkillDistributionOperations { + + SkillInstallManifest manifest( + CurrentActor actor, UUID assetId, UUID releaseId); + + SkillInstallManifest manifest( + CurrentActor actor, + String namespace, + String slug, + String version); + + SkillPackageContent open( + CurrentActor actor, UUID assetId, UUID releaseId); +} diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionService.java similarity index 97% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionService.java index 5e5b2ba3..f81aab72 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillDistributionService.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillDistributionService.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import com.orgmemory.core.assetregistry.api.AssetUnavailableException; import com.orgmemory.core.assetregistry.consumption.AssetConsumptionRelease; @@ -14,7 +14,7 @@ /** Canonical authenticated Skill distribution boundary. */ @Service -public class SkillDistributionService { +class SkillDistributionService implements SkillDistributionOperations { private static final Logger log = LoggerFactory.getLogger(SkillDistributionService.class); @@ -29,6 +29,7 @@ public class SkillDistributionService { this.specs = specs; } + @Override public SkillInstallManifest manifest( CurrentActor actor, UUID assetId, UUID releaseId) { SkillReleaseDescriptor descriptor = @@ -38,6 +39,7 @@ public SkillInstallManifest manifest( return manifest; } + @Override public SkillInstallManifest manifest( CurrentActor actor, String namespace, @@ -54,6 +56,7 @@ public SkillInstallManifest manifest( return manifest; } + @Override public SkillPackageContent open( CurrentActor actor, UUID assetId, UUID releaseId) { SkillReleaseContent content = deliveries.open(actor, assetId, releaseId); diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubImportService.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java similarity index 75% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubImportService.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java index a76fa632..4cc8ba24 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubImportService.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java @@ -1,5 +1,6 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageAssetCommand; import com.orgmemory.core.organization.CurrentActor; import com.orgmemory.core.permission.KnowledgeClassification; import com.orgmemory.core.shared.error.BusinessException; @@ -19,34 +20,38 @@ /** Coordinates stateless GitHub preview and independent per-Skill imports. */ @Service -public class SkillGitHubImportService { +class SkillGitHubImportService implements SkillGitHubOperations { private static final Logger LOG = LoggerFactory.getLogger(SkillGitHubImportService.class); private final SkillGitHubSourcePort source; private final SkillRegistryService skills; - private final AssetRegistryService assets; + private final SkillPackageAssetCommand packages; SkillGitHubImportService( SkillGitHubSourcePort source, SkillRegistryService skills, - AssetRegistryService assets) { + SkillPackageAssetCommand packages) { this.source = source; this.skills = skills; - this.assets = assets; + this.packages = packages; } - public List availableConnections( + @Override + public List availableConnections( CurrentActor actor, UUID knowledgeSpaceId) { Objects.requireNonNull(actor, "actor"); - assets.requireSkillCreate(actor, knowledgeSpaceId); - return source.availableConnections(actor.organizationId()); + packages.requireCreate(actor, knowledgeSpaceId); + return source.availableConnections(actor.organizationId()).stream() + .map(option -> new ConnectionOption(option.key())) + .toList(); } + @Override public Preview preview(CurrentActor actor, SourceRequest request) { Objects.requireNonNull(actor, "actor"); Objects.requireNonNull(request, "request"); - assets.requireSkillCreate(actor, request.knowledgeSpaceId()); + packages.requireCreate(actor, request.knowledgeSpaceId()); SkillGitHubSourcePort.FetchResult fetched = source.fetch(fetchRequest(actor, request)); List items = new ArrayList<>(); for (SkillGitHubSourcePort.FetchedPackage candidate : fetched.packages()) { @@ -82,13 +87,14 @@ public Preview preview(CurrentActor actor, SourceRequest request) { fetched.repository(), fetched.revision(), fetched.visibility(), items); } + @Override public ImportResult importSelected(CurrentActor actor, ImportRequest request) { Objects.requireNonNull(actor, "actor"); Objects.requireNonNull(request, "request"); Objects.requireNonNull(request.source(), "source"); Objects.requireNonNull(request.source().knowledgeSpaceId(), "knowledgeSpaceId"); Objects.requireNonNull(request.classification(), "classification"); - assets.requireSkillCreate(actor, request.source().knowledgeSpaceId()); + packages.requireCreate(actor, request.source().knowledgeSpaceId()); Set selected = normalizedSelection(request.paths()); if (selected.isEmpty() || selected.size() > SkillGitHubSourcePort.MAX_SKILLS_PER_IMPORT) { @@ -131,7 +137,7 @@ public ImportResult importSelected(CurrentActor actor, ImportRequest request) { } byte[] archive = candidate.archive(); try { - AssetView asset = skills.importPackage( + UUID assetId = skills.importPackage( actor, request.namespace(), request.source().knowledgeSpaceId(), @@ -143,7 +149,7 @@ public ImportResult importSelected(CurrentActor actor, ImportRequest request) { fetched.revision(), candidate.path(), fetched.visibility())); - results.add(ImportItem.imported(path, asset)); + results.add(ImportItem.imported(path, assetId)); } catch (BusinessException failure) { results.add(ImportItem.failed(path, failure.code(), failure.getMessage())); } catch (RuntimeException failure) { @@ -186,73 +192,4 @@ private static Set normalizedSelection(List paths) { .collect(Collectors.toCollection(java.util.LinkedHashSet::new)); } - public record SourceRequest( - String repository, - String revision, - String subpath, - String connectionKey, - UUID knowledgeSpaceId) { - } - - public record ImportRequest( - SourceRequest source, - List paths, - String namespace, - KnowledgeClassification classification) { - } - - public record Preview( - String repository, - String revision, - SkillPackageSpec.Visibility visibility, - List skills) { - } - - public record PreviewItem( - String path, - boolean importable, - String name, - String description, - int fileCount, - String errorCode, - String errorMessage) { - - static PreviewItem importable(String path, SkillPackageInspection inspection) { - return new PreviewItem( - path, - true, - inspection.name(), - inspection.description(), - inspection.files().size(), - "", - ""); - } - - static PreviewItem invalid(String path, String code, String message) { - return new PreviewItem(path, false, "", "", 0, code, message); - } - } - - public record ImportResult( - String repository, - String revision, - SkillPackageSpec.Visibility visibility, - List skills) { - } - - public record ImportItem( - String path, - boolean imported, - AssetView asset, - String errorCode, - String errorMessage) { - - static ImportItem imported(String path, AssetView asset) { - return new ImportItem(path, true, asset, "", ""); - } - - static ImportItem failed(String path, String code, String message) { - return new ImportItem(path, false, null, code, message); - } - } } diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubOperations.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubOperations.java new file mode 100644 index 00000000..8123dbfe --- /dev/null +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubOperations.java @@ -0,0 +1,95 @@ +package com.orgmemory.core.assetregistry.skill; + +import com.orgmemory.core.organization.CurrentActor; +import com.orgmemory.core.permission.KnowledgeClassification; +import java.util.List; +import java.util.Objects; +import java.util.UUID; + +public interface SkillGitHubOperations { + + List availableConnections( + CurrentActor actor, UUID knowledgeSpaceId); + + Preview preview(CurrentActor actor, SourceRequest request); + + ImportResult importSelected(CurrentActor actor, ImportRequest request); + + record ConnectionOption(String key) { + + public ConnectionOption { + key = Objects.requireNonNull(key, "key"); + } + } + + record SourceRequest( + String repository, + String revision, + String subpath, + String connectionKey, + UUID knowledgeSpaceId) { + } + + record ImportRequest( + SourceRequest source, + List paths, + String namespace, + KnowledgeClassification classification) { + } + + record Preview( + String repository, + String revision, + SkillGitHubSourcePort.Visibility visibility, + List skills) { + } + + record PreviewItem( + String path, + boolean importable, + String name, + String description, + int fileCount, + String errorCode, + String errorMessage) { + + static PreviewItem importable( + String path, SkillPackageInspection inspection) { + return new PreviewItem( + path, + true, + inspection.name(), + inspection.description(), + inspection.files().size(), + "", + ""); + } + + static PreviewItem invalid(String path, String code, String message) { + return new PreviewItem(path, false, "", "", 0, code, message); + } + } + + record ImportResult( + String repository, + String revision, + SkillGitHubSourcePort.Visibility visibility, + List skills) { + } + + record ImportItem( + String path, + boolean imported, + UUID assetId, + String errorCode, + String errorMessage) { + + static ImportItem imported(String path, UUID assetId) { + return new ImportItem(path, true, assetId, "", ""); + } + + static ImportItem failed(String path, String code, String message) { + return new ImportItem(path, false, null, code, message); + } + } +} diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubSourcePort.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubSourcePort.java similarity index 94% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubSourcePort.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubSourcePort.java index b51b8f40..09e44c00 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillGitHubSourcePort.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubSourcePort.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import java.util.Arrays; import java.util.List; @@ -42,7 +42,7 @@ record FetchRequest( record FetchResult( String repository, String revision, - SkillPackageSpec.Visibility visibility, + Visibility visibility, List packages) { public FetchResult { @@ -53,6 +53,11 @@ record FetchResult( } } + enum Visibility { + PUBLIC, + PRIVATE + } + record FetchedPackage( String path, byte[] archive, diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillInstallManifest.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillInstallManifest.java similarity index 95% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillInstallManifest.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillInstallManifest.java index 60857865..4847cabc 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillInstallManifest.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillInstallManifest.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import com.orgmemory.core.assetregistry.consumption.AssetPublicationMode; diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageContent.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageContent.java similarity index 92% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageContent.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageContent.java index efafa8bd..707c9207 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageContent.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageContent.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import java.io.IOException; import java.io.InputStream; diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspection.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspection.java similarity index 76% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspection.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspection.java index 543ec5bd..615cfb24 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspection.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspection.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import java.util.List; import java.util.Map; @@ -13,10 +13,13 @@ public record SkillPackageInspection( String instructions, String sha256, long contentLength, - List files) { + List files) { public SkillPackageInspection { metadata = metadata == null ? Map.of() : Map.copyOf(metadata); files = files == null ? List.of() : List.copyOf(files); } + + public record FileEntry(String path, long size, String sha256) { + } } diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspector.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspector.java similarity index 99% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspector.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspector.java index 70fb2653..733f3eb8 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageInspector.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspector.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import java.io.ByteArrayOutputStream; import java.io.IOException; diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageOperations.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageOperations.java new file mode 100644 index 00000000..815b5677 --- /dev/null +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageOperations.java @@ -0,0 +1,27 @@ +package com.orgmemory.core.assetregistry.skill; + +import com.orgmemory.core.organization.CurrentActor; +import com.orgmemory.core.permission.KnowledgeClassification; +import java.io.InputStream; +import java.util.UUID; + +public interface SkillPackageOperations { + + SkillPackageInspection inspectPackage( + CurrentActor actor, long contentLength, InputStream content); + + UUID importPackage( + CurrentActor actor, + String namespace, + UUID knowledgeSpaceId, + KnowledgeClassification classification, + long contentLength, + InputStream content); + + UUID replacePackage( + CurrentActor actor, + UUID assetId, + long expectedLockVersion, + long contentLength, + InputStream content); +} diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfile.java similarity index 84% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfile.java index a71d043c..28e54a43 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageProfile.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfile.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import com.orgmemory.core.assetregistry.api.AssetType; import com.orgmemory.core.assetregistry.profile.AssetPayloadProfile; @@ -46,6 +46,15 @@ public void validate( } } + @Override + public SkillPackageArtifact artifact(String canonicalPayload) { + SkillPackageSpec.Artifact artifact = read(canonicalPayload).artifact(); + return new SkillPackageArtifact( + artifact.sha256(), + artifact.contentLength(), + artifact.mediaType()); + } + @Override public SkillPackageSpec read(String payload) { try { diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSpec.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageSpec.java similarity index 96% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSpec.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageSpec.java index 51be9ec4..b82ed623 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageSpec.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageSpec.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import java.util.List; import java.util.Map; @@ -6,7 +6,7 @@ import java.util.Set; import java.util.regex.Pattern; -public record SkillPackageSpec( +record SkillPackageSpec( String name, String description, String license, @@ -56,7 +56,7 @@ public record Origin( String repository, String revision, String path, - Visibility visibility) { + SkillGitHubSourcePort.Visibility visibility) { public Origin { repository = required(repository, "origin.repository", 256); @@ -76,11 +76,6 @@ public record Origin( } } - public enum Visibility { - PUBLIC, - PRIVATE - } - public record Artifact( String sha256, long contentLength, diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageValidationException.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageValidationException.java similarity index 81% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageValidationException.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageValidationException.java index 7812b52d..de6bbfb4 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillPackageValidationException.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillPackageValidationException.java @@ -1,9 +1,9 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import com.orgmemory.core.shared.error.BusinessErrorCategory; import com.orgmemory.core.shared.error.BusinessException; -public final class SkillPackageValidationException extends BusinessException { +final class SkillPackageValidationException extends BusinessException { SkillPackageValidationException(String message) { super( diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java similarity index 91% rename from core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java rename to core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java index fc3d3755..6e42d93e 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/SkillRegistryService.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import com.orgmemory.core.assetregistry.skillpackage.SkillPackageArtifact; import com.orgmemory.core.assetregistry.skillpackage.SkillPackageAssetCommand; @@ -17,22 +17,20 @@ import tools.jackson.databind.json.JsonMapper; @Service -public class SkillRegistryService { +class SkillRegistryService implements SkillPackageOperations { private final SkillPackageInspector inspector; private final SkillPackageAssetCommand packages; - private final AssetRegistryService assets; private final ObjectMapper json = JsonMapper.builder().build(); SkillRegistryService( SkillPackageInspector inspector, - SkillPackageAssetCommand packages, - AssetRegistryService assets) { + SkillPackageAssetCommand packages) { this.inspector = inspector; this.packages = packages; - this.assets = assets; } + @Override public SkillPackageInspection inspectPackage( CurrentActor actor, long contentLength, InputStream content) { Objects.requireNonNull(actor, "actor"); @@ -48,11 +46,15 @@ public SkillPackageInspection inspectPackage( staged.metadata().instructions(), staged.sha256(), staged.contentLength(), - staged.files()); + staged.files().stream() + .map(file -> new SkillPackageInspection.FileEntry( + file.path(), file.size(), file.sha256())) + .toList()); } } - public AssetView importPackage( + @Override + public UUID importPackage( CurrentActor actor, String namespace, UUID knowledgeSpaceId, @@ -69,7 +71,7 @@ public AssetView importPackage( null); } - AssetView importPackage( + UUID importPackage( CurrentActor actor, String namespace, UUID knowledgeSpaceId, @@ -88,13 +90,12 @@ AssetView importPackage( staged.contentLength(), SkillPackageArtifact.ZIP_MEDIA_TYPE); SkillPackageSpec spec = specification(staged, artifact, origin); - UUID assetId = packages.importPackage( + return packages.importPackage( actor, namespace, knowledgeSpaceId, classification, upload(spec, artifact, packageContent)); - return assets.get(actor, assetId); } catch (IOException failure) { throw new BusinessUnavailableException( "skill.package-staging-unavailable", @@ -103,7 +104,8 @@ AssetView importPackage( } } - public AssetView replacePackage( + @Override + public UUID replacePackage( CurrentActor actor, UUID assetId, long expectedLockVersion, @@ -120,12 +122,11 @@ public AssetView replacePackage( staged.contentLength(), SkillPackageArtifact.ZIP_MEDIA_TYPE); SkillPackageSpec spec = specification(staged, artifact, null); - UUID replacedId = packages.replacePackage( + return packages.replacePackage( actor, assetId, expectedLockVersion, upload(spec, artifact, packageContent)); - return assets.get(actor, replacedId); } catch (IOException failure) { throw new BusinessUnavailableException( "skill.package-staging-unavailable", diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/package-info.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/package-info.java new file mode 100644 index 00000000..49a5fcd2 --- /dev/null +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/package-info.java @@ -0,0 +1,14 @@ +/** Agent Skill package semantics, GitHub acquisition, and distribution contracts. */ +@org.springframework.modulith.ApplicationModule( + type = org.springframework.modulith.ApplicationModule.Type.CLOSED, + allowedDependencies = { + "assetregistry::api", + "assetregistry::consumption", + "assetregistry::profile", + "assetregistry::skill-package", + "assetregistry::skill-delivery", + "organization", + "permission", + "shared::error" + }) +package com.orgmemory.core.assetregistry.skill; diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.java b/core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.java index 3b669b31..f4e50627 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skillpackage/SkillPackagePayloadPolicy.java @@ -3,4 +3,6 @@ public interface SkillPackagePayloadPolicy { void validate(String canonicalPayload, SkillPackageArtifact artifact); + + SkillPackageArtifact artifact(String canonicalPayload); } diff --git a/core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java b/core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java index b7900bfa..e08c268b 100644 --- a/core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java +++ b/core/src/test/java/com/orgmemory/core/ModulithVerificationTests.java @@ -1245,18 +1245,20 @@ void assetRegistrySkillCapabilitiesAreExactExplicitNamedInterfaces() { void assetRegistrySkillCapabilitiesHaveExactCoreConsumers() { assertEquals( Set.of( - "com.orgmemory.core.assetregistry.SkillDistributionService", + "com.orgmemory.core.assetregistry.AssetRegistryCoordinator", "com.orgmemory.core.assetregistry.SkillPackageAssetService", - "com.orgmemory.core.assetregistry.SkillPackageProfile", - "com.orgmemory.core.assetregistry.SkillRegistryService", "com.orgmemory.core.assetregistry.SkillReleaseDeliveryService", + "com.orgmemory.core.assetregistry.skill.SkillDistributionService", + "com.orgmemory.core.assetregistry.skill.SkillGitHubImportService", + "com.orgmemory.core.assetregistry.skill.SkillPackageProfile", + "com.orgmemory.core.assetregistry.skill.SkillRegistryService", "com.orgmemory.core.assetregistry.skilldelivery.SkillReleaseDescriptor"), directConsumersOf( "com.orgmemory.core.assetregistry.skillpackage")); assertEquals( Set.of( - "com.orgmemory.core.assetregistry.SkillDistributionService", - "com.orgmemory.core.assetregistry.SkillReleaseDeliveryService"), + "com.orgmemory.core.assetregistry.SkillReleaseDeliveryService", + "com.orgmemory.core.assetregistry.skill.SkillDistributionService"), directConsumersOf( "com.orgmemory.core.assetregistry.skilldelivery")); assertEquals( @@ -1286,6 +1288,85 @@ void assetRegistrySkillCapabilityImplementationsRemainInternal() { } } + @Test + void assetRegistrySkillIsAClosedSemanticsModule() { + var skill = modules.getModuleByName("assetregistry.skill").orElseThrow(); + var allowedDependencies = skill.getAllowedDependencies(modules).stream() + .map(Object::toString) + .map(dependency -> dependency.replace(" :: ", "::")) + .collect(TreeSet::new, Set::add, Set::addAll); + + assertFalse(skill.isOpen()); + assertEquals( + Set.of( + "assetregistry::api", + "assetregistry::consumption", + "assetregistry::profile", + "assetregistry::skill-delivery", + "assetregistry::skill-package", + "organization", + "permission", + "shared::error"), + allowedDependencies); + } + + @Test + void assetRegistrySkillExposesOnlyItsSevenTopLevelContracts() { + var publicTopLevelTypes = new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("com.orgmemory.core.assetregistry.skill") + .stream() + .filter(type -> !type.getName().contains("$")) + .filter(type -> type.getModifiers().contains(JavaModifier.PUBLIC)) + .map(type -> type.getName()) + .collect(TreeSet::new, Set::add, Set::addAll); + + assertEquals( + Set.of( + "com.orgmemory.core.assetregistry.skill.SkillDistributionOperations", + "com.orgmemory.core.assetregistry.skill.SkillGitHubOperations", + "com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort", + "com.orgmemory.core.assetregistry.skill.SkillInstallManifest", + "com.orgmemory.core.assetregistry.skill.SkillPackageContent", + "com.orgmemory.core.assetregistry.skill.SkillPackageInspection", + "com.orgmemory.core.assetregistry.skill.SkillPackageOperations"), + publicTopLevelTypes); + } + + @Test + void assetRegistrySkillDoesNotDependOnParentImplementationOrStorage() { + noClasses() + .that() + .resideInAPackage("com.orgmemory.core.assetregistry.skill..") + .should() + .dependOnClassesThat() + .resideInAnyPackage( + "com.orgmemory.core.assetregistry", + "com.orgmemory.core.assetregistry.authorization..", + "com.orgmemory.core.assetregistry.kernel..", + "com.orgmemory.core.assetregistry.skillcleanup..", + "com.orgmemory.core.assetregistry.skillstorage..") + .check(new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("com.orgmemory.core.assetregistry.skill")); + } + + @Test + void assetRegistryParentDoesNotDependOnSkillSemantics() { + noClasses() + .that() + .resideInAnyPackage( + "com.orgmemory.core.assetregistry", + "com.orgmemory.core.assetregistry.authorization..", + "com.orgmemory.core.assetregistry.kernel..") + .should() + .dependOnClassesThat() + .resideInAPackage("com.orgmemory.core.assetregistry.skill..") + .check(new ClassFileImporter() + .withImportOption(ImportOption.Predefined.DO_NOT_INCLUDE_TESTS) + .importPackages("com.orgmemory.core.assetregistry")); + } + private static Class loadClass(String name) { try { return Class.forName(name); diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/AssetProfileValidationTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/AssetProfileValidationTests.java index d806da40..a8b0570d 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/AssetProfileValidationTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/AssetProfileValidationTests.java @@ -1,7 +1,6 @@ package com.orgmemory.core.assetregistry; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -29,7 +28,21 @@ public void validate(String payload) { }; private final WorkInstructionProfile instructions = new WorkInstructionProfile(); private final CapabilityPackProfile packs = new CapabilityPackProfile(); - private final SkillPackageProfile skills = new SkillPackageProfile(); + private final AssetPayloadProfile skills = new AssetPayloadProfile() { + @Override + public AssetType type() { + return AssetType.SKILL; + } + + @Override + public java.util.Set schemaVersions() { + return java.util.Set.of("1", "2"); + } + + @Override + public void validate(String payload) { + } + }; private final AssetTypeProfileRegistry registry = new AssetTypeProfileRegistry(List.of(prompts, instructions, packs, skills)); @@ -59,28 +72,6 @@ void packSchemaRequiresExactPinsAndAtLeastOneItem() { "\"items\": [{", "\"items\": [] , \"unused\": [{"))); } - @Test - void skillSchemaRejectsInvalidPackageDigests() { - assertThrows( - IllegalArgumentException.class, - () -> skills.validate( - skillPayload().replace( - "\"sha256\": \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"", - "\"sha256\": \"not-a-sha256\""))); - } - - @Test - void skillSchemaReadsLegacyAndCurrentPayloadsWithoutOrigin() { - SkillPackageSpec legacy = skills.read(skillPayload()); - SkillPackageSpec current = skills.read(skillPayload().replace( - "\"artifact\": {", "\"origin\": null, \"artifact\": {")); - - assertNull(legacy.origin()); - assertNull(current.origin()); - registry.require(AssetType.SKILL).validate("1", skillPayload()); - registry.require(AssetType.SKILL).validate("2", skillPayload()); - } - public static String promptPayload(String template) { return """ { @@ -158,7 +149,7 @@ static String packPayload() { """; } - static String skillPayload() { + public static String skillPayload() { return """ { "name": "support-triage", diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java deleted file mode 100644 index 6529d2b8..00000000 --- a/core/src/test/java/com/orgmemory/core/assetregistry/SkillDistributionServiceTests.java +++ /dev/null @@ -1,341 +0,0 @@ -package com.orgmemory.core.assetregistry; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoInteractions; -import static org.mockito.Mockito.when; - -import com.orgmemory.core.assetregistry.api.AssetIdentity; -import com.orgmemory.core.assetregistry.api.AssetIdentityQuery; -import com.orgmemory.core.assetregistry.api.AssetNotFoundException; -import com.orgmemory.core.assetregistry.api.AssetPortfolioState; -import com.orgmemory.core.assetregistry.api.AssetType; -import com.orgmemory.core.assetregistry.api.AssetUnavailableException; -import com.orgmemory.core.assetregistry.consumption.AssetAvailability; -import com.orgmemory.core.assetregistry.consumption.AssetConsumptionRelease; -import com.orgmemory.core.assetregistry.consumption.AssetPublicationMode; -import com.orgmemory.core.assetregistry.skillstorage.SkillPackageStoragePort; -import com.orgmemory.core.organization.CurrentActor; -import java.io.ByteArrayInputStream; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.UUID; -import org.junit.jupiter.api.Test; - -class SkillDistributionServiceTests { - - private static final UUID ORGANIZATION_ID = - UUID.fromString("85000000-0000-0000-0000-000000000001"); - private static final UUID USER_ID = - UUID.fromString("85000000-0000-0000-0000-000000000002"); - private static final UUID ASSET_ID = - UUID.fromString("85000000-0000-0000-0000-000000000003"); - private static final UUID RELEASE_ID = - UUID.fromString("85000000-0000-0000-0000-000000000004"); - private static final String PACKAGE_DIGEST = "a".repeat(64); - private static final CurrentActor ACTOR = new CurrentActor( - USER_ID, - ORGANIZATION_ID, - null, - "Skill user", - "skill.user@example.test"); - - @Test - void returnsAnExactManifestWithoutExposingTheStorageReference() { - Fixture fixture = fixture(); - - SkillInstallManifest manifest = - fixture.service.manifest(ACTOR, ASSET_ID, RELEASE_ID); - - assertEquals("support/triage", manifest.coordinate()); - assertEquals("1.2.0", manifest.version()); - assertEquals(AssetPublicationMode.DIRECT, manifest.publicationMode()); - assertEquals(PACKAGE_DIGEST, manifest.packageDigest()); - assertEquals("SKILL.md", manifest.files().getFirst().path()); - assertTrue(manifest.toString().indexOf("private/skill.zip") < 0); - verify(fixture.assets).releaseForUse( - ACTOR, ASSET_ID, RELEASE_ID, AssetType.SKILL); - } - - @Test - void closesAndRejectsStoredBytesWhoseMetadataNoLongerMatchesTheRelease() { - Fixture fixture = fixture(); - TrackingInputStream stream = new TrackingInputStream(); - when(fixture.storage.open("private/skill.zip")) - .thenReturn(new SkillPackageStoragePort.StoredSkillPackageContent( - stream, - new SkillPackageStoragePort.StoredSkillPackage( - "private/skill.zip", - 7, - "application/zip", - "b".repeat(64)))); - - assertThrows( - AssetUnavailableException.class, - () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); - - assertTrue(stream.closed); - } - - @Test - void closesContentWhenTheCanonicalPayloadDoesNotMatchThePinnedReference() { - Fixture fixture = fixture(); - TrackingInputStream stream = storedContent(fixture); - when(fixture.specs.read("{\"profile\":\"skill\"}")) - .thenReturn(spec("b".repeat(64))); - - assertThrows( - AssetUnavailableException.class, - () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); - - assertTrue(stream.closed); - } - - @Test - void closesContentWhenTheCanonicalPayloadCannotBeRead() { - Fixture fixture = fixture(); - TrackingInputStream stream = storedContent(fixture); - when(fixture.specs.read("{\"profile\":\"skill\"}")) - .thenThrow(new IllegalArgumentException("invalid payload")); - - assertThrows( - AssetUnavailableException.class, - () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); - - assertTrue(stream.closed); - } - - @Test - void rejectsAReleaseWhosePackageReferenceIsMissing() { - Fixture fixture = fixture(); - when(fixture.references.findByReleaseIdAndOrganizationId( - RELEASE_ID, ORGANIZATION_ID)) - .thenReturn(Optional.empty()); - - assertThrows( - AssetUnavailableException.class, - () -> fixture.service.manifest(ACTOR, ASSET_ID, RELEASE_ID)); - - verifyNoInteractions(fixture.storage); - } - - @Test - void rejectsAReleaseWhosePackageReferenceIsNotABlob() { - Fixture fixture = fixture(); - when(fixture.reference.isBlobReference()).thenReturn(false); - - assertThrows( - AssetUnavailableException.class, - () -> fixture.service.manifest(ACTOR, ASSET_ID, RELEASE_ID)); - - verifyNoInteractions(fixture.storage); - } - - @Test - void resolvesCoordinateAndVersionBeforeApplyingTheSameLiveUseCheck() { - Fixture fixture = fixture(); - AssetIdentity asset = assetIdentity(); - AssetRelease release = mock(AssetRelease.class); - when(release.getId()).thenReturn(RELEASE_ID); - when(fixture.identities - .findByCoordinate( - ORGANIZATION_ID, - "support", - "triage")) - .thenReturn(Optional.of(asset)); - when(fixture.releaseRepository - .findByAssetIdAndOrganizationIdAndVersionLabel( - ASSET_ID, - ORGANIZATION_ID, - "1.2.0")) - .thenReturn(Optional.of(release)); - - SkillInstallManifest manifest = - fixture.service.manifest( - ACTOR, "Support", "Triage", "1.2.0"); - - assertEquals(RELEASE_ID, manifest.releaseId()); - verify(fixture.assets).releaseForUse( - ACTOR, ASSET_ID, RELEASE_ID, AssetType.SKILL); - } - - @Test - void keepsTheInvalidVersionCauseBehindTheOpaqueNotFoundError() { - Fixture fixture = fixture(); - AssetIdentity asset = assetIdentity(); - when(fixture.identities - .findByCoordinate( - ORGANIZATION_ID, - "support", - "triage")) - .thenReturn(Optional.of(asset)); - - AssetNotFoundException failure = assertThrows( - AssetNotFoundException.class, - () -> fixture.service.manifest( - ACTOR, "support", "triage", "not a version")); - - assertTrue(failure.getCause() instanceof IllegalArgumentException); - } - - @Test - void rejectsACoordinateThatResolvesToANonSkillAsset() { - Fixture fixture = fixture(); - when(fixture.identities - .findByCoordinate( - ORGANIZATION_ID, - "support", - "triage")) - .thenReturn(Optional.of(assetIdentity(AssetType.PROMPT_TEMPLATE))); - - assertThrows( - AssetNotFoundException.class, - () -> fixture.service.manifest( - ACTOR, "support", "triage", "1.2.0")); - } - - private static Fixture fixture() { - AssetRegistryService assets = mock(AssetRegistryService.class); - AssetIdentityQuery identities = mock(AssetIdentityQuery.class); - AssetReleaseRepository releaseRepository = - mock(AssetReleaseRepository.class); - AssetPayloadReferenceRepository references = - mock(AssetPayloadReferenceRepository.class); - SkillPackageSpecReader specs = mock(SkillPackageSpecReader.class); - SkillPackageStoragePort storage = - mock(SkillPackageStoragePort.class); - AssetPayloadReference reference = - mock(AssetPayloadReference.class); - when(assets.releaseForUse( - ACTOR, ASSET_ID, RELEASE_ID, AssetType.SKILL)) - .thenReturn(release()); - when(specs.read("{\"profile\":\"skill\"}")) - .thenReturn(spec()); - when(references.findByReleaseIdAndOrganizationId( - RELEASE_ID, ORGANIZATION_ID)) - .thenReturn(Optional.of(reference)); - when(reference.isBlobReference()).thenReturn(true); - when(reference.getReferenceValue()).thenReturn("private/skill.zip"); - when(reference.getDigest()).thenReturn(PACKAGE_DIGEST); - when(reference.getContentLength()).thenReturn(7L); - when(reference.getMediaType()).thenReturn("application/zip"); - SkillReleaseDeliveryService deliveries = new SkillReleaseDeliveryService( - assets, - identities, - releaseRepository, - references, - storage); - return new Fixture( - new SkillDistributionService(deliveries, specs), - assets, - identities, - releaseRepository, - references, - reference, - specs, - storage); - } - - private static AssetConsumptionRelease release() { - return new AssetConsumptionRelease( - ASSET_ID, - RELEASE_ID, - UUID.randomUUID(), - AssetType.SKILL, - "support", - "triage", - "1.2.0", - AssetPublicationMode.DIRECT, - "Support triage", - "Triage customer issues", - "INTERNAL", - "1", - "{\"profile\":\"skill\"}", - "c".repeat(64), - AssetAvailability.AVAILABLE, - java.time.Instant.parse("2026-07-27T10:00:00Z")); - } - - private static AssetIdentity assetIdentity() { - return assetIdentity(AssetType.SKILL); - } - - private static AssetIdentity assetIdentity(AssetType type) { - return new AssetIdentity( - ORGANIZATION_ID, - ASSET_ID, - type, - "support", - "triage", - UUID.randomUUID(), - AssetPortfolioState.DRAFT_ONLY, - true); - } - - private static SkillPackageSpec spec() { - return spec(PACKAGE_DIGEST); - } - - private static SkillPackageSpec spec(String packageDigest) { - return new SkillPackageSpec( - "triage", - "Triage customer issues", - "MIT", - "Claude Code and Codex", - "Read", - Map.of("owner", "support"), - null, - new SkillPackageSpec.Artifact( - packageDigest, - 7, - "application/zip"), - List.of(new SkillPackageSpec.FileEntry( - "SKILL.md", - 7, - "d".repeat(64)))); - } - - private record Fixture( - SkillDistributionService service, - AssetRegistryService assets, - AssetIdentityQuery identities, - AssetReleaseRepository releaseRepository, - AssetPayloadReferenceRepository references, - AssetPayloadReference reference, - SkillPackageSpecReader specs, - SkillPackageStoragePort storage) { - } - - private static TrackingInputStream storedContent(Fixture fixture) { - TrackingInputStream stream = new TrackingInputStream(); - when(fixture.storage.open("private/skill.zip")) - .thenReturn(new SkillPackageStoragePort.StoredSkillPackageContent( - stream, - new SkillPackageStoragePort.StoredSkillPackage( - "private/skill.zip", - 7, - "application/zip", - PACKAGE_DIGEST))); - return stream; - } - - private static final class TrackingInputStream - extends ByteArrayInputStream { - - private boolean closed; - - private TrackingInputStream() { - super(new byte[7]); - } - - @Override - public void close() throws java.io.IOException { - closed = true; - super.close(); - } - } -} diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageAssetServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageAssetServiceTests.java index 8a7f3e4e..df100151 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageAssetServiceTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageAssetServiceTests.java @@ -1,13 +1,20 @@ package com.orgmemory.core.assetregistry; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; +import com.orgmemory.core.assetregistry.api.AssetConflictException; import com.orgmemory.core.assetregistry.api.AssetPortfolioState; import com.orgmemory.core.assetregistry.api.AssetType; +import com.orgmemory.core.assetregistry.api.AssetUnavailableException; import com.orgmemory.core.assetregistry.skillpackage.SkillPackageArtifact; import com.orgmemory.core.assetregistry.skillpackage.SkillPackagePayloadPolicy; import com.orgmemory.core.assetregistry.skillpackage.SkillPackageUpload; @@ -74,6 +81,89 @@ void rejectsAnInvalidReplacementPayloadBeforeWritingStorage() { verifyNoInteractions(fixture.storage); } + @Test + void deletesStoredBytesWhenAssetIdentityCreationFails() { + Fixture fixture = fixture(); + when(fixture.storage.put(any(), any())).thenReturn(stored()); + when(fixture.assets.createValidatedSkillIdentity( + eq(ACTOR), + eq("support"), + eq("support-triage"), + eq(SPACE_ID), + any(), + any())) + .thenThrow(new AssetConflictException("Duplicate")); + + assertThrows( + AssetConflictException.class, + () -> fixture.service.importPackage( + ACTOR, + "support", + SPACE_ID, + KnowledgeClassification.INTERNAL, + upload())); + + verify(fixture.storage).delete("assets/skills/package.zip"); + } + + @Test + void retainsReferencedBytesWhenAuthorizationProjectionNeedsRetry() { + Fixture fixture = fixture(); + when(fixture.storage.put(any(), any())).thenReturn(stored()); + when(fixture.assets.createValidatedSkillIdentity( + any(), any(), any(), any(), any(), any())) + .thenReturn(ASSET_ID); + when(fixture.assets.projectCreated(ACTOR, ASSET_ID)) + .thenThrow(new AssetUnavailableException("Projection pending")); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.importPackage( + ACTOR, + "support", + SPACE_ID, + KnowledgeClassification.INTERNAL, + upload())); + + verify(fixture.storage, never()).delete(any()); + } + + @Test + void replacementCleansTheDurableSupersessionAfterTheSwap() { + Fixture fixture = fixture(); + UUID supersessionId = UUID.randomUUID(); + when(fixture.assets.get(ACTOR, ASSET_ID)).thenReturn(skillView()); + when(fixture.storage.put(any(), any())).thenReturn(stored()); + when(fixture.assets.replaceValidatedSkillDraft( + eq(ACTOR), eq(ASSET_ID), eq(4L), any(), any())) + .thenReturn(new SkillDraftReplacement(skillView(), supersessionId)); + + UUID replacedId = fixture.service.replacePackage( + ACTOR, ASSET_ID, 4, upload()); + + org.junit.jupiter.api.Assertions.assertEquals(ASSET_ID, replacedId); + verify(fixture.assets, times(1)).requireSkillEdit(ACTOR, ASSET_ID); + verify(fixture.cleanup).cleanup(supersessionId); + verify(fixture.storage, never()).delete(any()); + } + + @Test + void replacementDeletesTheNewObjectWhenTheDatabaseSwapFails() { + Fixture fixture = fixture(); + when(fixture.assets.get(ACTOR, ASSET_ID)).thenReturn(skillView()); + when(fixture.storage.put(any(), any())).thenReturn(stored()); + when(fixture.assets.replaceValidatedSkillDraft( + eq(ACTOR), eq(ASSET_ID), eq(4L), any(), any())) + .thenThrow(new AssetConflictException("Changed")); + + assertThrows( + AssetConflictException.class, + () -> fixture.service.replacePackage( + ACTOR, ASSET_ID, 4, upload())); + + verify(fixture.storage).delete("assets/skills/package.zip"); + } + private static Fixture fixture() { SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); SkillPackagePayloadPolicy policy = mock(SkillPackagePayloadPolicy.class); @@ -84,7 +174,8 @@ private static Fixture fixture() { new SkillPackageAssetService(storage, policy, assets, cleanup), storage, policy, - assets); + assets, + cleanup); } private static void rejectPayload(SkillPackagePayloadPolicy policy) { @@ -132,10 +223,19 @@ private static AssetView skillView() { List.of()); } + private static SkillPackageStoragePort.StoredSkillPackage stored() { + return new SkillPackageStoragePort.StoredSkillPackage( + "assets/skills/package.zip", + ARTIFACT.contentLength(), + ARTIFACT.mediaType(), + ARTIFACT.sha256()); + } + private record Fixture( SkillPackageAssetService service, SkillPackageStoragePort storage, SkillPackagePayloadPolicy policy, - AssetRegistryService assets) { + AssetRegistryService assets, + SkillPackageSupersessionCleanupService cleanup) { } } diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.java deleted file mode 100644 index 1b2dd9df..00000000 --- a/core/src/test/java/com/orgmemory/core/assetregistry/SkillRegistryServiceTests.java +++ /dev/null @@ -1,291 +0,0 @@ -package com.orgmemory.core.assetregistry; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.orgmemory.core.assetregistry.api.AssetConflictException; -import com.orgmemory.core.assetregistry.api.AssetPortfolioState; -import com.orgmemory.core.assetregistry.api.AssetType; -import com.orgmemory.core.assetregistry.api.AssetUnavailableException; -import com.orgmemory.core.assetregistry.skillpackage.SkillPackageAssetCommand; -import com.orgmemory.core.assetregistry.skillstorage.SkillPackageStoragePort; -import com.orgmemory.core.organization.CurrentActor; -import com.orgmemory.core.organization.OrgMemoryAccessDeniedException; -import com.orgmemory.core.permission.KnowledgeClassification; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.nio.charset.StandardCharsets; -import java.time.Instant; -import java.util.List; -import java.util.UUID; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; -import org.junit.jupiter.api.Test; - -class SkillRegistryServiceTests { - - private static final CurrentActor ACTOR = new CurrentActor( - UUID.fromString("11111111-1111-4111-8111-111111111111"), - UUID.fromString("22222222-2222-4222-8222-222222222222"), - UUID.fromString("33333333-3333-4333-8333-333333333333"), - "Skill owner", - "owner@example.test"); - private static final UUID SPACE_ID = - UUID.fromString("44444444-4444-4444-8444-444444444444"); - - @Test - void refusesUnauthorizedImportsBeforeInspectingOrStoringBytes() { - SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - doThrow(new OrgMemoryAccessDeniedException("Denied")) - .when(assets) - .requireSkillCreate(ACTOR, SPACE_ID); - SkillRegistryService service = service( - storage, - assets, - mock(SkillPackageSupersessionCleanupService.class)); - - assertThrows( - OrgMemoryAccessDeniedException.class, - () -> service.importPackage( - ACTOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - 1, - new ByteArrayInputStream(new byte[] {1}))); - - verify(storage, never()).put(any(), any()); - } - - @Test - void deletesStoredBytesWhenAssetIdentityCreationFails() throws Exception { - byte[] archive = archive(); - SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - doNothing().when(assets).requireSkillCreate(ACTOR, SPACE_ID); - when(storage.put(any(), any())).thenAnswer(invocation -> { - SkillPackageStoragePort.SkillPackageWriteRequest request = - invocation.getArgument(0); - return stored(request); - }); - when(assets.createValidatedSkillIdentity( - eq(ACTOR), - eq("support"), - eq("support-triage"), - eq(SPACE_ID), - any(), - any())) - .thenThrow(new AssetConflictException("Duplicate")); - SkillRegistryService service = service( - storage, - assets, - mock(SkillPackageSupersessionCleanupService.class)); - - assertThrows( - AssetConflictException.class, - () -> importArchive(service, archive)); - - verify(storage).delete(any()); - } - - @Test - void retainsReferencedBytesWhenAuthorizationProjectionNeedsRetry() throws Exception { - byte[] archive = archive(); - SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - doNothing().when(assets).requireSkillCreate(ACTOR, SPACE_ID); - when(storage.put(any(), any())).thenAnswer(invocation -> { - SkillPackageStoragePort.SkillPackageWriteRequest request = - invocation.getArgument(0); - return stored(request); - }); - UUID assetId = UUID.randomUUID(); - when(assets.createValidatedSkillIdentity( - any(), any(), any(), any(), any(), any())) - .thenReturn(assetId); - when(assets.projectCreated(ACTOR, assetId)) - .thenThrow(new AssetUnavailableException("Projection pending")); - SkillRegistryService service = service( - storage, - assets, - mock(SkillPackageSupersessionCleanupService.class)); - - assertThrows( - AssetUnavailableException.class, - () -> importArchive(service, archive)); - - verify(storage, never()).delete(any()); - } - - @Test - void inspectionIsStatelessAndReturnsOnlyValidatedPackageFacts() throws Exception { - byte[] archive = archive(); - SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); - SkillRegistryService service = new SkillRegistryService( - new SkillPackageInspector(), - packages, - mock(AssetRegistryService.class)); - - SkillPackageInspection inspection = service.inspectPackage( - ACTOR, archive.length, new ByteArrayInputStream(archive)); - - assertEquals("support-triage", inspection.name()); - assertEquals(1, inspection.files().size()); - assertEquals("# Support triage", inspection.instructions()); - verify(packages, never()).importPackage(any(), any(), any(), any(), any()); - } - - @Test - void replacementAuthorizesBeforeStorageAndCleansTheDurableSupersession() - throws Exception { - byte[] archive = archive(); - UUID assetId = UUID.randomUUID(); - UUID supersessionId = UUID.randomUUID(); - SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - SkillPackageSupersessionCleanupService cleanup = - mock(SkillPackageSupersessionCleanupService.class); - AssetView current = skillView(assetId, 7); - doNothing().when(assets).requireSkillEdit(ACTOR, assetId); - when(assets.get(ACTOR, assetId)).thenReturn(current); - when(storage.put(any(), any())).thenAnswer(invocation -> { - SkillPackageStoragePort.SkillPackageWriteRequest request = - invocation.getArgument(0); - return stored(request); - }); - when(assets.replaceValidatedSkillDraft( - eq(ACTOR), eq(assetId), eq(7L), any(), any())) - .thenReturn(new SkillDraftReplacement(current, supersessionId)); - SkillRegistryService service = service(storage, assets, cleanup); - - AssetView replaced = service.replacePackage( - ACTOR, - assetId, - 7, - archive.length, - new ByteArrayInputStream(archive)); - - assertEquals(assetId, replaced.id()); - verify(assets, times(2)).requireSkillEdit(ACTOR, assetId); - verify(cleanup).cleanup(supersessionId); - verify(storage, never()).delete(any()); - } - - @Test - void replacementDeletesTheNewObjectWhenTheDatabaseSwapFails() throws Exception { - byte[] archive = archive(); - UUID assetId = UUID.randomUUID(); - SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - doNothing().when(assets).requireSkillEdit(ACTOR, assetId); - when(assets.get(ACTOR, assetId)).thenReturn(skillView(assetId, 2)); - when(storage.put(any(), any())).thenAnswer(invocation -> { - SkillPackageStoragePort.SkillPackageWriteRequest request = - invocation.getArgument(0); - return stored(request); - }); - when(assets.replaceValidatedSkillDraft(any(), any(), any(Long.class), any(), any())) - .thenThrow(new AssetConflictException("Changed")); - SkillRegistryService service = service( - storage, - assets, - mock(SkillPackageSupersessionCleanupService.class)); - - assertThrows( - AssetConflictException.class, - () -> service.replacePackage( - ACTOR, - assetId, - 2, - archive.length, - new ByteArrayInputStream(archive))); - - verify(storage).delete(any()); - } - - private static AssetView importArchive( - SkillRegistryService service, byte[] archive) { - return service.importPackage( - ACTOR, - "support", - SPACE_ID, - KnowledgeClassification.INTERNAL, - archive.length, - new ByteArrayInputStream(archive)); - } - - private static SkillRegistryService service( - SkillPackageStoragePort storage, - AssetRegistryService assets, - SkillPackageSupersessionCleanupService cleanup) { - SkillPackageAssetCommand command = new SkillPackageAssetService( - storage, new SkillPackageProfile(), assets, cleanup); - return new SkillRegistryService( - new SkillPackageInspector(), command, assets); - } - - private static SkillPackageStoragePort.StoredSkillPackage stored( - SkillPackageStoragePort.SkillPackageWriteRequest request) { - return new SkillPackageStoragePort.StoredSkillPackage( - "assets/skills/" - + request.organizationId() - + "/" - + request.packageId() - + ".zip", - request.contentLength(), - "application/zip", - request.expectedSha256()); - } - - private static AssetView skillView(UUID assetId, long lockVersion) { - return new AssetView( - assetId, - AssetType.SKILL, - "support", - "support-triage", - SPACE_ID, - AssetPortfolioState.DRAFT_ONLY, - true, - new AssetView.Draft( - UUID.randomUUID(), - lockVersion, - "support-triage", - "Support triage", - "INTERNAL", - "1", - "{}", - ACTOR.userId(), - Instant.now()), - List.of(), - List.of(), - List.of(), - null, - List.of()); - } - - private static byte[] archive() throws Exception { - ByteArrayOutputStream output = new ByteArrayOutputStream(); - try (ZipOutputStream zip = new ZipOutputStream(output)) { - zip.putNextEntry(new ZipEntry("support-triage/SKILL.md")); - zip.write(""" - --- - name: support-triage - description: Triage support requests using approved guidance. - --- - # Support triage - """.getBytes(StandardCharsets.UTF_8)); - zip.closeEntry(); - } - return output.toByteArray(); - } -} diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryServiceTests.java new file mode 100644 index 00000000..82ba2ea2 --- /dev/null +++ b/core/src/test/java/com/orgmemory/core/assetregistry/SkillReleaseDeliveryServiceTests.java @@ -0,0 +1,212 @@ +package com.orgmemory.core.assetregistry; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +import com.orgmemory.core.assetregistry.api.AssetIdentity; +import com.orgmemory.core.assetregistry.api.AssetIdentityQuery; +import com.orgmemory.core.assetregistry.api.AssetNotFoundException; +import com.orgmemory.core.assetregistry.api.AssetPortfolioState; +import com.orgmemory.core.assetregistry.api.AssetType; +import com.orgmemory.core.assetregistry.api.AssetUnavailableException; +import com.orgmemory.core.assetregistry.consumption.AssetAvailability; +import com.orgmemory.core.assetregistry.consumption.AssetConsumptionRelease; +import com.orgmemory.core.assetregistry.consumption.AssetPublicationMode; +import com.orgmemory.core.assetregistry.skillstorage.SkillPackageStoragePort; +import com.orgmemory.core.organization.CurrentActor; +import java.io.ByteArrayInputStream; +import java.util.Optional; +import java.util.UUID; +import org.junit.jupiter.api.Test; + +class SkillReleaseDeliveryServiceTests { + + private static final UUID ORGANIZATION_ID = + UUID.fromString("87000000-0000-0000-0000-000000000001"); + private static final UUID USER_ID = + UUID.fromString("87000000-0000-0000-0000-000000000002"); + private static final UUID ASSET_ID = + UUID.fromString("87000000-0000-0000-0000-000000000003"); + private static final UUID RELEASE_ID = + UUID.fromString("87000000-0000-0000-0000-000000000004"); + private static final String PACKAGE_DIGEST = "a".repeat(64); + private static final CurrentActor ACTOR = new CurrentActor( + USER_ID, + ORGANIZATION_ID, + null, + "Skill user", + "skill.user@example.test"); + + @Test + void rejectsAndClosesStoredBytesWhoseMetadataDoesNotMatchTheRelease() { + Fixture fixture = fixture(); + TrackingInputStream stream = new TrackingInputStream(); + when(fixture.storage.open("private/skill.zip")) + .thenReturn(new SkillPackageStoragePort.StoredSkillPackageContent( + stream, + new SkillPackageStoragePort.StoredSkillPackage( + "private/skill.zip", + 7, + "application/zip", + "b".repeat(64)))); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); + + assertTrue(stream.closed); + } + + @Test + void rejectsAReleaseWhosePackageReferenceIsMissing() { + Fixture fixture = fixture(); + when(fixture.references.findByReleaseIdAndOrganizationId( + RELEASE_ID, ORGANIZATION_ID)) + .thenReturn(Optional.empty()); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.describe(ACTOR, ASSET_ID, RELEASE_ID)); + + verifyNoInteractions(fixture.storage); + } + + @Test + void rejectsAReleaseWhosePackageReferenceIsNotABlob() { + Fixture fixture = fixture(); + when(fixture.reference.isBlobReference()).thenReturn(false); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.describe(ACTOR, ASSET_ID, RELEASE_ID)); + + verifyNoInteractions(fixture.storage); + } + + @Test + void resolvesCoordinateAndVersionBeforeApplyingTheLiveUseCheck() { + Fixture fixture = fixture(); + AssetIdentity asset = assetIdentity(); + AssetRelease release = mock(AssetRelease.class); + when(release.getId()).thenReturn(RELEASE_ID); + when(fixture.identities.findByCoordinate( + ORGANIZATION_ID, "support", "triage")) + .thenReturn(Optional.of(asset)); + when(fixture.releases.findByAssetIdAndOrganizationIdAndVersionLabel( + ASSET_ID, ORGANIZATION_ID, "1.2.0")) + .thenReturn(Optional.of(release)); + + var descriptor = fixture.service.describe( + ACTOR, "Support", "Triage", "1.2.0"); + + assertEquals(RELEASE_ID, descriptor.release().releaseId()); + verify(fixture.assets).releaseForUse( + ACTOR, ASSET_ID, RELEASE_ID, AssetType.SKILL); + } + + @Test + void keepsInvalidVersionDetailsBehindTheOpaqueNotFoundError() { + Fixture fixture = fixture(); + when(fixture.identities.findByCoordinate( + ORGANIZATION_ID, "support", "triage")) + .thenReturn(Optional.of(assetIdentity())); + + AssetNotFoundException failure = assertThrows( + AssetNotFoundException.class, + () -> fixture.service.describe( + ACTOR, "support", "triage", "not a version")); + + assertTrue(failure.getCause() instanceof IllegalArgumentException); + } + + private static Fixture fixture() { + AssetRegistryService assets = mock(AssetRegistryService.class); + AssetIdentityQuery identities = mock(AssetIdentityQuery.class); + AssetReleaseRepository releases = mock(AssetReleaseRepository.class); + AssetPayloadReferenceRepository references = + mock(AssetPayloadReferenceRepository.class); + SkillPackageStoragePort storage = mock(SkillPackageStoragePort.class); + AssetPayloadReference reference = mock(AssetPayloadReference.class); + when(assets.releaseForUse(ACTOR, ASSET_ID, RELEASE_ID, AssetType.SKILL)) + .thenReturn(release()); + when(references.findByReleaseIdAndOrganizationId( + RELEASE_ID, ORGANIZATION_ID)) + .thenReturn(Optional.of(reference)); + when(reference.isBlobReference()).thenReturn(true); + when(reference.getReferenceValue()).thenReturn("private/skill.zip"); + when(reference.getDigest()).thenReturn(PACKAGE_DIGEST); + when(reference.getContentLength()).thenReturn(7L); + when(reference.getMediaType()).thenReturn("application/zip"); + return new Fixture( + new SkillReleaseDeliveryService( + assets, identities, releases, references, storage), + assets, + identities, + releases, + references, + reference, + storage); + } + + private static AssetConsumptionRelease release() { + return new AssetConsumptionRelease( + ASSET_ID, + RELEASE_ID, + UUID.randomUUID(), + AssetType.SKILL, + "support", + "triage", + "1.2.0", + AssetPublicationMode.DIRECT, + "Support triage", + "Triage customer issues", + "INTERNAL", + "1", + "{\"profile\":\"skill\"}", + "c".repeat(64), + AssetAvailability.AVAILABLE, + java.time.Instant.parse("2026-07-27T10:00:00Z")); + } + + private static AssetIdentity assetIdentity() { + return new AssetIdentity( + ORGANIZATION_ID, + ASSET_ID, + AssetType.SKILL, + "support", + "triage", + UUID.randomUUID(), + AssetPortfolioState.DRAFT_ONLY, + true); + } + + private record Fixture( + SkillReleaseDeliveryService service, + AssetRegistryService assets, + AssetIdentityQuery identities, + AssetReleaseRepository releases, + AssetPayloadReferenceRepository references, + AssetPayloadReference reference, + SkillPackageStoragePort storage) { + } + + private static final class TrackingInputStream extends ByteArrayInputStream { + + private boolean closed; + + private TrackingInputStream() { + super(new byte[7]); + } + + @Override + public void close() throws java.io.IOException { + closed = true; + super.close(); + } + } +} diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillDistributionServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillDistributionServiceTests.java new file mode 100644 index 00000000..2550c046 --- /dev/null +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillDistributionServiceTests.java @@ -0,0 +1,186 @@ +package com.orgmemory.core.assetregistry.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.orgmemory.core.assetregistry.api.AssetType; +import com.orgmemory.core.assetregistry.api.AssetUnavailableException; +import com.orgmemory.core.assetregistry.consumption.AssetAvailability; +import com.orgmemory.core.assetregistry.consumption.AssetConsumptionRelease; +import com.orgmemory.core.assetregistry.consumption.AssetPublicationMode; +import com.orgmemory.core.assetregistry.skilldelivery.SkillReleaseContent; +import com.orgmemory.core.assetregistry.skilldelivery.SkillReleaseDeliveryQuery; +import com.orgmemory.core.assetregistry.skilldelivery.SkillReleaseDescriptor; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageArtifact; +import com.orgmemory.core.organization.CurrentActor; +import java.io.ByteArrayInputStream; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.junit.jupiter.api.Test; + +class SkillDistributionServiceTests { + + private static final UUID ORGANIZATION_ID = + UUID.fromString("85000000-0000-0000-0000-000000000001"); + private static final UUID USER_ID = + UUID.fromString("85000000-0000-0000-0000-000000000002"); + private static final UUID ASSET_ID = + UUID.fromString("85000000-0000-0000-0000-000000000003"); + private static final UUID RELEASE_ID = + UUID.fromString("85000000-0000-0000-0000-000000000004"); + private static final String PACKAGE_DIGEST = "a".repeat(64); + private static final CurrentActor ACTOR = new CurrentActor( + USER_ID, + ORGANIZATION_ID, + null, + "Skill user", + "skill.user@example.test"); + + @Test + void returnsAnExactManifestWithoutExposingAStorageReference() { + Fixture fixture = fixture(); + + SkillInstallManifest manifest = + fixture.service.manifest(ACTOR, ASSET_ID, RELEASE_ID); + + assertEquals("support/triage", manifest.coordinate()); + assertEquals("1.2.0", manifest.version()); + assertEquals(PACKAGE_DIGEST, manifest.packageDigest()); + assertEquals("SKILL.md", manifest.files().getFirst().path()); + assertTrue(manifest.toString().indexOf("private/skill.zip") < 0); + verify(fixture.deliveries).describe(ACTOR, ASSET_ID, RELEASE_ID); + } + + @Test + void resolvesTheCoordinateThroughTheParentDeliveryCapability() { + Fixture fixture = fixture(); + when(fixture.deliveries.describe(ACTOR, "support", "triage", "1.2.0")) + .thenReturn(descriptor()); + + SkillInstallManifest manifest = fixture.service.manifest( + ACTOR, "support", "triage", "1.2.0"); + + assertEquals(RELEASE_ID, manifest.releaseId()); + verify(fixture.deliveries).describe(ACTOR, "support", "triage", "1.2.0"); + } + + @Test + void closesContentWhenTheCanonicalPayloadDoesNotMatchThePinnedReference() { + Fixture fixture = fixture(); + TrackingInputStream stream = openedContent(fixture); + when(fixture.specs.read("{\"profile\":\"skill\"}")) + .thenReturn(spec("b".repeat(64))); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); + + assertTrue(stream.closed); + } + + @Test + void closesContentWhenTheCanonicalPayloadCannotBeRead() { + Fixture fixture = fixture(); + TrackingInputStream stream = openedContent(fixture); + when(fixture.specs.read("{\"profile\":\"skill\"}")) + .thenThrow(new IllegalArgumentException("invalid payload")); + + assertThrows( + AssetUnavailableException.class, + () -> fixture.service.open(ACTOR, ASSET_ID, RELEASE_ID)); + + assertTrue(stream.closed); + } + + private static Fixture fixture() { + SkillReleaseDeliveryQuery deliveries = mock(SkillReleaseDeliveryQuery.class); + SkillPackageSpecReader specs = mock(SkillPackageSpecReader.class); + when(deliveries.describe(ACTOR, ASSET_ID, RELEASE_ID)) + .thenReturn(descriptor()); + when(specs.read("{\"profile\":\"skill\"}")) + .thenReturn(spec(PACKAGE_DIGEST)); + return new Fixture( + new SkillDistributionService(deliveries, specs), + deliveries, + specs); + } + + private static TrackingInputStream openedContent(Fixture fixture) { + TrackingInputStream stream = new TrackingInputStream(); + when(fixture.deliveries.open(ACTOR, ASSET_ID, RELEASE_ID)) + .thenReturn(new SkillReleaseContent(descriptor(), stream)); + return stream; + } + + private static SkillReleaseDescriptor descriptor() { + return new SkillReleaseDescriptor( + release(), + new SkillPackageArtifact( + PACKAGE_DIGEST, 7, SkillPackageArtifact.ZIP_MEDIA_TYPE)); + } + + private static AssetConsumptionRelease release() { + return new AssetConsumptionRelease( + ASSET_ID, + RELEASE_ID, + UUID.randomUUID(), + AssetType.SKILL, + "support", + "triage", + "1.2.0", + AssetPublicationMode.DIRECT, + "Support triage", + "Triage customer issues", + "INTERNAL", + "1", + "{\"profile\":\"skill\"}", + "c".repeat(64), + AssetAvailability.AVAILABLE, + java.time.Instant.parse("2026-07-27T10:00:00Z")); + } + + private static SkillPackageSpec spec(String packageDigest) { + return new SkillPackageSpec( + "triage", + "Triage customer issues", + "MIT", + "Claude Code and Codex", + "Read", + Map.of("owner", "support"), + null, + new SkillPackageSpec.Artifact( + packageDigest, + 7, + SkillPackageArtifact.ZIP_MEDIA_TYPE), + List.of(new SkillPackageSpec.FileEntry( + "SKILL.md", + 7, + "d".repeat(64)))); + } + + private record Fixture( + SkillDistributionService service, + SkillReleaseDeliveryQuery deliveries, + SkillPackageSpecReader specs) { + } + + private static final class TrackingInputStream extends ByteArrayInputStream { + + private boolean closed; + + private TrackingInputStream() { + super(new byte[7]); + } + + @Override + public void close() throws java.io.IOException { + closed = true; + super.close(); + } + } +} diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillGitHubImportServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java similarity index 78% rename from core/src/test/java/com/orgmemory/core/assetregistry/SkillGitHubImportServiceTests.java rename to core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java index c5715f4b..5b1d3407 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/SkillGitHubImportServiceTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -12,6 +12,7 @@ import static org.mockito.Mockito.when; import com.orgmemory.core.assetregistry.api.AssetConflictException; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageAssetCommand; import com.orgmemory.core.organization.CurrentActor; import com.orgmemory.core.permission.KnowledgeClassification; import com.orgmemory.core.shared.error.BusinessValidationException; @@ -41,7 +42,7 @@ void previewKeepsValidAndInvalidCandidatesInOneResult() { when(source.fetch(any())).thenReturn(new SkillGitHubSourcePort.FetchResult( "acme/skills", SHA, - SkillPackageSpec.Visibility.PUBLIC, + SkillGitHubSourcePort.Visibility.PUBLIC, List.of( valid("skills/triage/SKILL.md", 1), new SkillGitHubSourcePort.FetchedPackage( @@ -51,40 +52,41 @@ void previewKeepsValidAndInvalidCandidatesInOneResult() { "Too large")))); when(skills.inspectPackage(eq(ACTOR), eq(1L), any(InputStream.class))) .thenReturn(inspection("triage")); - AssetRegistryService assets = mock(AssetRegistryService.class); - SkillGitHubImportService service = new SkillGitHubImportService(source, skills, assets); + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + SkillGitHubImportService service = new SkillGitHubImportService( + source, skills, packages); - SkillGitHubImportService.Preview preview = service.preview( + SkillGitHubOperations.Preview preview = service.preview( ACTOR, - new SkillGitHubImportService.SourceRequest( + new SkillGitHubOperations.SourceRequest( "acme/skills", "main", "skills", "", SPACE_ID)); assertEquals(SHA, preview.revision()); assertEquals(List.of(true, false), preview.skills().stream() - .map(SkillGitHubImportService.PreviewItem::importable) + .map(SkillGitHubOperations.PreviewItem::importable) .toList()); assertEquals("triage", preview.skills().getFirst().name()); assertEquals( "skill.github-package-too-large", preview.skills().get(1).errorCode()); - verify(assets).requireSkillCreate(ACTOR, SPACE_ID); + verify(packages).requireCreate(ACTOR, SPACE_ID); } @Test void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { SkillGitHubSourcePort source = mock(SkillGitHubSourcePort.class); SkillRegistryService skills = mock(SkillRegistryService.class); - AssetRegistryService assets = mock(AssetRegistryService.class); - doNothing().when(assets).requireSkillCreate(ACTOR, SPACE_ID); + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + doNothing().when(packages).requireCreate(ACTOR, SPACE_ID); String firstPath = "skills/triage/SKILL.md"; String secondPath = "skills/reply/SKILL.md"; when(source.fetch(any())).thenReturn(new SkillGitHubSourcePort.FetchResult( "acme/skills", SHA, - SkillPackageSpec.Visibility.PRIVATE, + SkillGitHubSourcePort.Visibility.PRIVATE, List.of(valid(firstPath, 1), valid(secondPath, 2)))); - AssetView imported = mock(AssetView.class); + UUID importedId = UUID.randomUUID(); when(skills.importPackage( eq(ACTOR), eq("support"), @@ -93,14 +95,15 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { anyLong(), any(InputStream.class), any(SkillPackageSpec.Origin.class))) - .thenReturn(imported) + .thenReturn(importedId) .thenThrow(new AssetConflictException("Duplicate")); - SkillGitHubImportService service = new SkillGitHubImportService(source, skills, assets); + SkillGitHubImportService service = new SkillGitHubImportService( + source, skills, packages); - SkillGitHubImportService.ImportResult result = service.importSelected( + SkillGitHubOperations.ImportResult result = service.importSelected( ACTOR, - new SkillGitHubImportService.ImportRequest( - new SkillGitHubImportService.SourceRequest( + new SkillGitHubOperations.ImportRequest( + new SkillGitHubOperations.SourceRequest( "acme/skills", SHA, "skills", "private-app", SPACE_ID), List.of(firstPath, secondPath), "support", @@ -108,8 +111,9 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { assertEquals(List.of(true, false), result.skills().stream() - .map(SkillGitHubImportService.ImportItem::imported) + .map(SkillGitHubOperations.ImportItem::imported) .toList()); + assertEquals(importedId, result.skills().getFirst().assetId()); assertEquals("asset.conflict", result.skills().get(1).errorCode()); ArgumentCaptor origins = ArgumentCaptor.forClass(SkillPackageSpec.Origin.class); @@ -123,21 +127,21 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { origins.capture()); assertEquals(List.of(firstPath, secondPath), origins.getAllValues().stream().map(SkillPackageSpec.Origin::path).toList()); - assertEquals(SkillPackageSpec.Visibility.PRIVATE, + assertEquals(SkillGitHubSourcePort.Visibility.PRIVATE, origins.getAllValues().getFirst().visibility()); } @Test void connectionDiscoveryRequiresSkillCreatePermissionForTheSelectedSpace() { SkillGitHubSourcePort source = mock(SkillGitHubSourcePort.class); - AssetRegistryService assets = mock(AssetRegistryService.class); + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); when(source.availableConnections(ACTOR.organizationId())).thenReturn(List.of()); SkillGitHubImportService service = new SkillGitHubImportService( - source, mock(SkillRegistryService.class), assets); + source, mock(SkillRegistryService.class), packages); service.availableConnections(ACTOR, SPACE_ID); - verify(assets).requireSkillCreate(ACTOR, SPACE_ID); + verify(packages).requireCreate(ACTOR, SPACE_ID); verify(source).availableConnections(ACTOR.organizationId()); } @@ -145,7 +149,9 @@ void connectionDiscoveryRequiresSkillCreatePermissionForTheSelectedSpace() { void importRejectsNonCommitRevisionBeforeFetchingRepository() { SkillGitHubSourcePort source = mock(SkillGitHubSourcePort.class); SkillGitHubImportService service = new SkillGitHubImportService( - source, mock(SkillRegistryService.class), mock(AssetRegistryService.class)); + source, + mock(SkillRegistryService.class), + mock(SkillPackageAssetCommand.class)); BusinessValidationException failure = assertThrows( BusinessValidationException.class, @@ -161,10 +167,12 @@ void importRejectsARevisionThatChangesAfterPreview() { when(source.fetch(any())).thenReturn(new SkillGitHubSourcePort.FetchResult( "acme/skills", "b".repeat(40), - SkillPackageSpec.Visibility.PUBLIC, + SkillGitHubSourcePort.Visibility.PUBLIC, List.of(valid("SKILL.md", 1)))); SkillGitHubImportService service = new SkillGitHubImportService( - source, mock(SkillRegistryService.class), mock(AssetRegistryService.class)); + source, + mock(SkillRegistryService.class), + mock(SkillPackageAssetCommand.class)); BusinessValidationException failure = assertThrows( BusinessValidationException.class, @@ -177,12 +185,12 @@ void importRejectsARevisionThatChangesAfterPreview() { void importReportsASelectedPathMissingAtThePinnedRevision() { SkillGitHubSourcePort source = mock(SkillGitHubSourcePort.class); when(source.fetch(any())).thenReturn(new SkillGitHubSourcePort.FetchResult( - "acme/skills", SHA, SkillPackageSpec.Visibility.PUBLIC, List.of())); + "acme/skills", SHA, SkillGitHubSourcePort.Visibility.PUBLIC, List.of())); SkillRegistryService skills = mock(SkillRegistryService.class); SkillGitHubImportService service = new SkillGitHubImportService( - source, skills, mock(AssetRegistryService.class)); + source, skills, mock(SkillPackageAssetCommand.class)); - SkillGitHubImportService.ImportResult result = + SkillGitHubOperations.ImportResult result = service.importSelected(ACTOR, request(SHA, List.of("removed/SKILL.md"))); assertEquals("skill.github-path-not-found", result.skills().getFirst().errorCode()); @@ -198,10 +206,10 @@ void fetchRequestDefaultsAnOmittedRevisionToHead() { assertEquals("HEAD", request.revision()); } - private static SkillGitHubImportService.ImportRequest request( + private static SkillGitHubOperations.ImportRequest request( String revision, List paths) { - return new SkillGitHubImportService.ImportRequest( - new SkillGitHubImportService.SourceRequest( + return new SkillGitHubOperations.ImportRequest( + new SkillGitHubOperations.SourceRequest( "acme/skills", revision, "", "", SPACE_ID), paths, "engineering", @@ -224,7 +232,7 @@ private static SkillPackageInspection inspection(String name) { "# Skill", "b".repeat(64), 1, - List.of(new SkillPackageSpec.FileEntry( + List.of(new SkillPackageInspection.FileEntry( "SKILL.md", 1, "c".repeat(64)))); } } diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageInspectorTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspectorTests.java similarity index 99% rename from core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageInspectorTests.java rename to core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspectorTests.java index ca510d29..59903b61 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/SkillPackageInspectorTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageInspectorTests.java @@ -1,4 +1,4 @@ -package com.orgmemory.core.assetregistry; +package com.orgmemory.core.assetregistry.skill; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfileTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfileTests.java new file mode 100644 index 00000000..61eb000a --- /dev/null +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillPackageProfileTests.java @@ -0,0 +1,33 @@ +package com.orgmemory.core.assetregistry.skill; + +import static com.orgmemory.core.assetregistry.AssetProfileValidationTests.skillPayload; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import org.junit.jupiter.api.Test; + +class SkillPackageProfileTests { + + private final SkillPackageProfile skills = new SkillPackageProfile(); + + @Test + void rejectsInvalidPackageDigests() { + assertThrows( + IllegalArgumentException.class, + () -> skills.validate( + skillPayload().replace( + "\"sha256\": \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\"", + "\"sha256\": \"not-a-sha256\""))); + } + + @Test + void readsLegacyAndCurrentPayloadsWithoutOrigin() { + SkillPackageSpec legacy = skills.read(skillPayload()); + SkillPackageSpec current = skills.read(skillPayload().replace( + "\"artifact\": {", "\"origin\": null, \"artifact\": {")); + + assertNull(legacy.origin()); + assertNull(current.origin()); + skills.validate(skillPayload()); + } +} diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java new file mode 100644 index 00000000..bfbfbfa9 --- /dev/null +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java @@ -0,0 +1,170 @@ +package com.orgmemory.core.assetregistry.skill; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.orgmemory.core.assetregistry.api.AssetNotFoundException; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageAssetCommand; +import com.orgmemory.core.assetregistry.skillpackage.SkillPackageUpload; +import com.orgmemory.core.organization.CurrentActor; +import com.orgmemory.core.permission.KnowledgeClassification; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.UUID; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +class SkillRegistryServiceTests { + + private static final UUID ORGANIZATION_ID = + UUID.fromString("88000000-0000-0000-0000-000000000001"); + private static final UUID USER_ID = + UUID.fromString("88000000-0000-0000-0000-000000000002"); + private static final UUID SPACE_ID = + UUID.fromString("88000000-0000-0000-0000-000000000003"); + private static final UUID ASSET_ID = + UUID.fromString("88000000-0000-0000-0000-000000000004"); + private static final CurrentActor ACTOR = new CurrentActor( + USER_ID, + ORGANIZATION_ID, + null, + "Skill editor", + "skill.editor@example.test"); + + @Test + void refusesUnauthorizedImportsBeforeReadingPackageBytes() { + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + doThrow(new AssetNotFoundException()) + .when(packages) + .requireCreate(ACTOR, SPACE_ID); + SkillRegistryService service = service(packages); + + assertThrows( + AssetNotFoundException.class, + () -> service.importPackage( + ACTOR, + "support", + SPACE_ID, + KnowledgeClassification.INTERNAL, + 1, + new UnreadableInputStream())); + } + + @Test + void importsOneCanonicalPackageThroughTheParentCapability() throws Exception { + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + when(packages.importPackage( + eq(ACTOR), + eq("support"), + eq(SPACE_ID), + eq(KnowledgeClassification.INTERNAL), + any(SkillPackageUpload.class))) + .thenReturn(ASSET_ID); + SkillRegistryService service = service(packages); + byte[] archive = archive(); + + UUID importedId = service.importPackage( + ACTOR, + "support", + SPACE_ID, + KnowledgeClassification.INTERNAL, + archive.length, + new ByteArrayInputStream(archive)); + + assertEquals(ASSET_ID, importedId); + ArgumentCaptor upload = + ArgumentCaptor.forClass(SkillPackageUpload.class); + verify(packages).importPackage( + eq(ACTOR), + eq("support"), + eq(SPACE_ID), + eq(KnowledgeClassification.INTERNAL), + upload.capture()); + assertEquals("support-triage", upload.getValue().slug()); + assertEquals("2", upload.getValue().schemaVersion()); + assertEquals("application/zip", upload.getValue().artifact().mediaType()); + assertTrue(upload.getValue().payload().contains("\"name\":\"support-triage\"")); + } + + @Test + void inspectionIsStatelessAndReturnsOnlyValidatedFacts() throws Exception { + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + SkillRegistryService service = service(packages); + byte[] archive = archive(); + + SkillPackageInspection inspection = service.inspectPackage( + ACTOR, archive.length, new ByteArrayInputStream(archive)); + + assertEquals("support-triage", inspection.name()); + assertEquals("SKILL.md", inspection.files().getFirst().path()); + assertTrue(inspection.instructions().contains("# Support triage")); + } + + @Test + void replacementAuthorizesBeforeReadingAndRoutesTheCanonicalUpload() + throws Exception { + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + when(packages.requireEdit(ACTOR, ASSET_ID)) + .thenReturn(KnowledgeClassification.INTERNAL); + when(packages.replacePackage( + eq(ACTOR), + eq(ASSET_ID), + eq(7L), + any(SkillPackageUpload.class))) + .thenReturn(ASSET_ID); + SkillRegistryService service = service(packages); + byte[] archive = archive(); + + UUID replacedId = service.replacePackage( + ACTOR, + ASSET_ID, + 7, + archive.length, + new ByteArrayInputStream(archive)); + + assertEquals(ASSET_ID, replacedId); + verify(packages).requireEdit(ACTOR, ASSET_ID); + verify(packages).replacePackage( + eq(ACTOR), eq(ASSET_ID), eq(7L), any(SkillPackageUpload.class)); + } + + private static SkillRegistryService service( + SkillPackageAssetCommand packages) { + return new SkillRegistryService(new SkillPackageInspector(), packages); + } + + private static byte[] archive() throws Exception { + ByteArrayOutputStream output = new ByteArrayOutputStream(); + try (ZipOutputStream zip = new ZipOutputStream(output)) { + zip.putNextEntry(new ZipEntry("support-triage/SKILL.md")); + zip.write(""" + --- + name: support-triage + description: Triage support requests using approved guidance. + --- + # Support triage + """.getBytes(StandardCharsets.UTF_8)); + zip.closeEntry(); + } + return output.toByteArray(); + } + + private static final class UnreadableInputStream extends InputStream { + + @Override + public int read() { + throw new AssertionError("package bytes were read before authorization"); + } + } +} diff --git a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfiguration.java b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfiguration.java index 3f58cb2f..2479ea87 100644 --- a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfiguration.java +++ b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfiguration.java @@ -1,6 +1,6 @@ package com.orgmemory.connectors.github; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.knowledge.connector.ConnectorBatchSource; import com.orgmemory.core.knowledge.connector.ConnectorConnectionDirectory; import com.orgmemory.core.knowledge.connector.ConnectorCredentialProbe; diff --git a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillArchiveReader.java b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillArchiveReader.java index 342191cc..b6973f43 100644 --- a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillArchiveReader.java +++ b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillArchiveReader.java @@ -1,6 +1,6 @@ package com.orgmemory.connectors.github; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.shared.error.BusinessValidationException; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; diff --git a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapter.java b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapter.java index b1ae17a6..7236f323 100644 --- a/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapter.java +++ b/integrations/connectors/src/main/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapter.java @@ -1,7 +1,6 @@ package com.orgmemory.connectors.github; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; -import com.orgmemory.core.assetregistry.SkillPackageSpec; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.knowledge.connector.ConnectorConnectionConfiguration; import com.orgmemory.core.knowledge.connector.ConnectorConnectionDirectory; import com.orgmemory.core.permission.PermissionAuditCommand; @@ -97,7 +96,7 @@ public FetchResult fetch(FetchRequest request) { access = new Access( anonymousClient, "", - SkillPackageSpec.Visibility.PUBLIC, + SkillGitHubSourcePort.Visibility.PUBLIC, metadata.path("id").asString("")); } else if (isRateLimited(publicRepository)) { throw new BusinessUnavailableException( @@ -122,7 +121,7 @@ public FetchResult fetch(FetchRequest request) { "skill.github-response-invalid", "GitHub returned an invalid commit revision"); } - byte[] archive = access.visibility() == SkillPackageSpec.Visibility.PUBLIC + byte[] archive = access.visibility() == SkillGitHubSourcePort.Visibility.PUBLIC ? publicArchive(repository, revision) : privateArchive(access, repository, revision); List packages = GitHubSkillArchiveReader.read(archive, subpath); @@ -198,9 +197,9 @@ private Access privateAccess(FetchRequest request, Repository repository) { "PRIVATE_REPOSITORY_ACCESS"); if (!metadata.path("private").asBoolean(false)) { return new Access( - authenticated, token, SkillPackageSpec.Visibility.PUBLIC, repositoryId); + authenticated, token, SkillGitHubSourcePort.Visibility.PUBLIC, repositoryId); } - return new Access(authenticated, token, SkillPackageSpec.Visibility.PRIVATE, repositoryId); + return new Access(authenticated, token, SkillGitHubSourcePort.Visibility.PRIVATE, repositoryId); } private byte[] publicArchive(Repository repository, String revision) { @@ -436,7 +435,7 @@ private void auditCredentialUse( private record Access( RestClient client, String token, - SkillPackageSpec.Visibility visibility, + SkillGitHubSourcePort.Visibility visibility, String repositoryId) { } diff --git a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfigurationTests.java b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfigurationTests.java index 18e5eb1b..8e6c9032 100644 --- a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfigurationTests.java +++ b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubConnectorAutoConfigurationTests.java @@ -5,7 +5,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.knowledge.connector.ConnectorBatchSource; import com.orgmemory.core.knowledge.connector.ConnectorConnectionDirectory; import com.orgmemory.core.knowledge.connector.ConnectorCredentialProbe; @@ -53,6 +53,13 @@ void contributesProfileProbeScopesAndBatchSource() { }); } + @Test + void consumesOnlyTheSkillSourcePort() { + assertEquals( + "com.orgmemory.core.assetregistry.skill", + SkillGitHubSourcePort.class.getPackageName()); + } + @Test void classpathPresenceDoesNotAuthorizeACrawl() { runner.run((AssertableApplicationContext context) -> assertTrue( diff --git a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillArchiveReaderTests.java b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillArchiveReaderTests.java index 2a9d86d3..67354dbf 100644 --- a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillArchiveReaderTests.java +++ b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillArchiveReaderTests.java @@ -3,7 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.shared.error.BusinessValidationException; import java.io.ByteArrayOutputStream; import java.nio.charset.StandardCharsets; diff --git a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapterTests.java b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapterTests.java index 2819737d..71eb159f 100644 --- a/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapterTests.java +++ b/integrations/connectors/src/test/java/com/orgmemory/connectors/github/GitHubSkillSourceAdapterTests.java @@ -13,8 +13,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreators.withStatus; import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess; -import com.orgmemory.core.assetregistry.SkillGitHubSourcePort; -import com.orgmemory.core.assetregistry.SkillPackageSpec; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.knowledge.connector.ConnectorConnectionConfiguration; import com.orgmemory.core.knowledge.connector.ConnectorConnectionDirectory; import com.orgmemory.core.permission.PermissionAuditCommand; @@ -79,7 +78,7 @@ void importsAPublicRepositoryWithoutResolvingAStoredCredential() throws Exceptio SkillGitHubSourcePort.FetchResult result = adapter.fetch(request("main", "")); - assertEquals(SkillPackageSpec.Visibility.PUBLIC, result.visibility()); + assertEquals(SkillGitHubSourcePort.Visibility.PUBLIC, result.visibility()); assertEquals(SHA, result.revision()); assertEquals("skills/triage/SKILL.md", result.packages().getFirst().path()); verify(connections, org.mockito.Mockito.never()).resolveCredential(any(), any(), any()); @@ -139,7 +138,7 @@ void auditsPrivateCredentialUseAndStripsAuthorizationFromCodeload() throws Excep SkillGitHubSourcePort.FetchResult result = adapter.fetch(request(SHA, "private-app")); - assertEquals(SkillPackageSpec.Visibility.PRIVATE, result.visibility()); + assertEquals(SkillGitHubSourcePort.Visibility.PRIVATE, result.visibility()); ArgumentCaptor command = ArgumentCaptor.forClass(PermissionAuditCommand.class); verify(audit).record(command.capture()); From 389f657ad9b5c3f189d4b0bbff120c3bf5b5aa0c Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:26:29 +0700 Subject: [PATCH 2/6] docs(assetregistry): reconcile closed skill boundary --- ARCHITECTURE.md | 10 ++++++++++ .../design.md | 4 ++-- .../plan.md | 18 ++++++++++++++++-- docs/specs/domains/asset-registry.md | 15 ++++++++++++++- docs/tests/domains/asset-registry.md | 3 ++- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b295de12..d1acd56d 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -215,6 +215,16 @@ compensation, reference persistence, supersession retry state, cleanup, and storage opening. Skill package semantics never receive or publish the stored object key. +The closed `core.assetregistry.skill` nested module owns bounded package +inspection and validation, GitHub acquisition orchestration, API-facing Skill +operations, and install-manifest construction. Its exact public top-level +surface is `SkillPackageOperations`, `SkillGitHubOperations`, +`SkillDistributionOperations`, `SkillGitHubSourcePort`, +`SkillPackageInspection`, `SkillInstallManifest`, and `SkillPackageContent`; +all implementations and package semantics remain package-private. The child +consumes the parent only through `assetregistry::skill-package` and +`assetregistry::skill-delivery`, while the parent never depends on the child. + The browser reaches that same lifecycle through Scratch authoring, bounded `SKILL.md`/ZIP/folder upload, or GitHub import; each path ends at an ordinary private Draft in the Assets Governance workspace. GitHub preview and eligible diff --git a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.md b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.md index 17b28842..0ebe3a28 100644 --- a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.md +++ b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/design.md @@ -487,8 +487,8 @@ trigger, and `skill-storage` for exact parent persistence/cleanup classes plus MinIO. `assetregistry.skill` may consume only the first two. It never receives an object key and never orchestrates supersession cleanup. -When introduced, the closed Skill module exposes exactly four operation/source -interfaces and three immutable results. Its implementations, package +The closed Skill module exposes exactly four operation/source interfaces and +three immutable results. Its implementations, package specification, inspector, profile, parser, and validation exception remain package-private. See [the Skill challenge verdict](assetregistry-skill-challenge-verdict.md) for the diff --git a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md index b173aea1..d12ee114 100644 --- a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md +++ b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md @@ -1558,8 +1558,8 @@ slice and starts with an independent boundary challenge. - [x] PR 1: pass focused Core/API/OpenAPI/Worker/connector/MinIO/integration gates, docs and release policy, static analysis fallback, and a terminating clean repository test. -- [ ] PR 1: merge through CI and CodeRabbit without releasing. -- [ ] PR 2: add failing-first closed-module, exact-public-surface, +- [x] PR 1: merge through CI and CodeRabbit without releasing. +- [x] PR 2: add failing-first closed-module, exact-public-surface, forbidden-parent-import, and external-consumer guards, then move and immediately close `assetregistry.skill` below 70 changed paths. - [ ] PR 2: pass all focused and terminating gates, merge through CI and @@ -1604,3 +1604,17 @@ Skill slug grammar than delivery resolution. API exception translation was also confirmed to serialize only the stable top-level business message, never the storage cause. The PR remains below the 100-file cap and release remains deferred. + +PR #282 merged as `d3509d6d` after all required checks and CodeRabbit threads +were resolved. PR 2 then began with the expected failing closed-module probe +and commit `4c7a8bf2` closed `assetregistry.skill` in 42 rename-aware paths. +The child now exposes exactly three operation interfaces, one GitHub source +port, and three immutable results; all implementations and semantics remain +package-private. It consumes only the parent package and delivery capabilities, +never storage, cleanup, parent implementation, or an object key, while the +parent has no dependency on the child. API response mapping preserves the +existing Asset view and GitHub import wire shapes, and the nested inspection +entry retains the existing OpenAPI schema name. Focused Core, API, connector, +OpenAPI, Asset Registry integration, and Modulith gates are green; terminating +repository verification and the PR review loop remain before merge. Release +remains deferred. diff --git a/docs/specs/domains/asset-registry.md b/docs/specs/domains/asset-registry.md index e4f81c18..18e91701 100644 --- a/docs/specs/domains/asset-registry.md +++ b/docs/specs/domains/asset-registry.md @@ -13,7 +13,7 @@ Source: `core/src/main/java/com/orgmemory/core/assetregistry`, `apps/web/src/features/assets`, and `integrations/object-storage-minio/src/main/java`. -Reconciled: `2026-08-03-spring-modulith-package-refactor (518e0277)`. +Reconciled: `2026-08-03-spring-modulith-package-refactor (4c7a8bf2)`. ## Current Behavior @@ -262,6 +262,19 @@ The parent owns the entire storage and supersession saga. Package semantics, API results, manifests, audit values, logs, and exceptions do not receive the persisted object key. +The closed `assetregistry.skill` nested module owns bounded package inspection +and validation, GitHub acquisition orchestration, API-facing Skill operations, +and install-manifest construction. Its exact public top-level surface is +`SkillPackageOperations`, `SkillGitHubOperations`, +`SkillDistributionOperations`, `SkillGitHubSourcePort`, +`SkillPackageInspection`, `SkillInstallManifest`, and `SkillPackageContent`. +Implementations, the package profile and specification, the inspector, and the +validation exception remain package-private. The child imports only the +parent's `skill-package` and `skill-delivery` capabilities; it never imports +parent implementation, storage, cleanup, or a persisted object key. The API +and GitHub connector depend only on the child's operation/source contracts, +and the parent never depends on the child. + ### Federated Knowledge Knowledge remains owned by the canonical Knowledge ledger. The read-only diff --git a/docs/tests/domains/asset-registry.md b/docs/tests/domains/asset-registry.md index 939620b1..50d65f00 100644 --- a/docs/tests/domains/asset-registry.md +++ b/docs/tests/domains/asset-registry.md @@ -13,7 +13,7 @@ Source: `core/src/test/java/com/orgmemory/core/assetregistry`, `scripts/npm-publish-workflow-policy.test.mjs`, and `apps/web/src/features/assets/**/*.test.ts`. -Reconciled: `2026-08-03-spring-modulith-package-refactor (518e0277)`. +Reconciled: `2026-08-03-spring-modulith-package-refactor (4c7a8bf2)`. | Behavior | Evidence | Status | | --- | --- | --- | @@ -31,6 +31,7 @@ Reconciled: `2026-08-03-spring-modulith-package-refactor (518e0277)`. | Database mutation guards allow only Draft-reference deletion; payload-reference update and Revision/Release deletion remain rejected | `AssetRegistryIntegrationTests#onlyDraftPayloadReferencesMayBeDeletedWhileAllReferenceUpdatesStayRejected` | covered | | Post-commit supersession cleanup deletes only an exact unreferenced object, retains immutable pins, and durably schedules bounded retries after storage failure | `SkillPackageSupersessionCleanupCoordinatorTests` | covered | | The four parent-owned Skill capabilities expose exact type sets and exact Core/API/Worker/MinIO consumer sets; storage locators do not enter API or Worker dependencies | `ModulithVerificationTests#assetRegistrySkillCapabilitiesAreExactExplicitNamedInterfaces`, `#assetRegistrySkillCapabilitiesHaveExactCoreConsumers`, `SkillCapabilityBoundaryTests`, `MinioSkillPackageStorageAdapterTests#adapterExposesOnlyTheParentStorageCapability` | covered | +| Closed Skill owns package semantics, GitHub orchestration, API-facing operations, and manifest construction with an exact seven-type public surface; it imports only parent package/delivery capabilities, never parent implementation/storage/cleanup, and the parent never imports the child | `ModulithVerificationTests#assetRegistrySkillIsAClosedSemanticsModule`, `#assetRegistrySkillExposesOnlyItsExactPublicContracts`, `#assetRegistrySkillDoesNotDependOnParentImplementationOrStorage`, `#assetRegistryParentDoesNotDependOnSkill`, `SkillCapabilityBoundaryTests`, `GitHubConnectorAutoConfigurationTests` | covered | | A projection retry retains the already-referenced Skill object rather than deleting it | `SkillRegistryServiceTests#retainsReferencedBytesWhenAuthorizationProjectionNeedsRetry` | covered | | Skill storage uses an organization-scoped object key and verifies the stored SHA-256 | `MinioSkillPackageStorageAdapterTests` | covered | | Direct Skill publication atomically creates one Revision and Release, pins the exact validated blob through Draft, Revision, and Release, records `DIRECT` provenance, and emits the dedicated audit policy | `AssetRegistryIntegrationTests#skillImportPublishesDirectlyAndPinsTheValidatedBlob` | covered | From 880d851be5dba9eabada67567a283ffbacb04b65 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:30:59 +0700 Subject: [PATCH 3/6] docs(assetregistry): record skill verification --- .../plan.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md index d12ee114..59fbd375 100644 --- a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md +++ b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md @@ -1615,6 +1615,12 @@ never storage, cleanup, parent implementation, or an object key, while the parent has no dependency on the child. API response mapping preserves the existing Asset view and GitHub import wire shapes, and the nested inspection entry retains the existing OpenAPI schema name. Focused Core, API, connector, -OpenAPI, Asset Registry integration, and Modulith gates are green; terminating -repository verification and the PR review loop remain before merge. Release -remains deferred. +OpenAPI, Asset Registry integration, and Modulith gates are green. Worker and +MinIO consumer suites also passed, and the terminating repository-wide +`clean test` completed all 99 tasks with 1,265 tests and zero failure, error, +or skip. Documentation hygiene passed for 534 Markdown files and 8 mirrored +domain pairs; release policy passed 18 Tegami/product and 23 workflow/policy +tests on exact Node 24.15.0. The mechanical fallback found 47 total changed +paths, zero missing package declarations, zero changed zero-byte files, zero +forbidden Skill imports, and a clean diff. The PR review loop remains before +merge, and release remains deferred. From 5f7faa70d5a7b0f747acc52fe90e46c921745ea4 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:52:31 +0700 Subject: [PATCH 4/6] fix(assetregistry): isolate GitHub import results --- .../AssetRegistryController.java | 53 ++++++-- .../AssetRegistryControllerTests.java | 123 ++++++++++++++++++ .../skill/SkillGitHubImportService.java | 2 +- .../skill/SkillRegistryService.java | 26 +++- .../skill/SkillGitHubImportServiceTests.java | 7 +- .../skill/SkillRegistryServiceTests.java | 18 +++ 6 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryControllerTests.java diff --git a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java index 8c98e0fd..4af702bb 100644 --- a/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java +++ b/apps/api/src/main/java/com/orgmemory/api/assetregistry/AssetRegistryController.java @@ -16,7 +16,9 @@ import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; import com.orgmemory.core.assetregistry.skill.SkillPackageInspection; import com.orgmemory.core.assetregistry.skill.SkillPackageOperations; +import com.orgmemory.core.organization.CurrentActor; import com.orgmemory.core.permission.KnowledgeClassification; +import com.orgmemory.core.shared.error.BusinessException; import io.swagger.v3.oas.annotations.Operation; import jakarta.validation.constraints.NotBlank; import jakarta.validation.constraints.NotNull; @@ -25,6 +27,8 @@ import java.io.IOException; import java.util.List; import java.util.UUID; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; import org.springframework.security.core.Authentication; @@ -44,6 +48,9 @@ @RequestMapping("/api/assets") class AssetRegistryController { + private static final Logger LOG = + LoggerFactory.getLogger(AssetRegistryController.class); + enum GenericAssetType { PROMPT_TEMPLATE, WORK_INSTRUCTION, @@ -272,17 +279,47 @@ ImportResult importGitHubSkills( result.revision(), result.visibility(), result.skills().stream() - .map(item -> new ImportItem( - item.path(), - item.imported(), - item.imported() - ? assets.get(actor, item.assetId()) - : null, - item.errorCode(), - item.errorMessage())) + .map(item -> importItem(actor, item)) .toList()); } + private ImportItem importItem( + CurrentActor actor, + SkillGitHubOperations.ImportItem item) { + if (!item.imported()) { + return new ImportItem( + item.path(), false, null, item.errorCode(), item.errorMessage()); + } + try { + return new ImportItem( + item.path(), true, assets.get(actor, item.assetId()), "", ""); + } catch (BusinessException failure) { + LOG.warn( + "Imported GitHub Skill is not readable yet path={} assetId={} code={}", + item.path(), + item.assetId(), + failure.code()); + return new ImportItem( + item.path(), + true, + null, + failure.code(), + failure.getMessage()); + } catch (RuntimeException failure) { + LOG.warn( + "Unexpected GitHub Skill result resolution failure path={} assetId={}", + item.path(), + item.assetId(), + failure); + return new ImportItem( + item.path(), + true, + null, + "skill.github-import-resolution-failed", + "The Skill was imported but could not be read yet"); + } + } + @PutMapping( path = "/{assetId}/skill-draft", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) diff --git a/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryControllerTests.java b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryControllerTests.java new file mode 100644 index 00000000..43f9d64c --- /dev/null +++ b/apps/api/src/test/java/com/orgmemory/api/assetregistry/AssetRegistryControllerTests.java @@ -0,0 +1,123 @@ +package com.orgmemory.api.assetregistry; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.orgmemory.api.security.CurrentActorProvider; +import com.orgmemory.core.assetregistry.AssetRegistryService; +import com.orgmemory.core.assetregistry.AssetView; +import com.orgmemory.core.assetregistry.api.AssetType; +import com.orgmemory.core.assetregistry.api.AssetUnavailableException; +import com.orgmemory.core.assetregistry.skill.SkillGitHubOperations; +import com.orgmemory.core.assetregistry.skill.SkillGitHubSourcePort; +import com.orgmemory.core.assetregistry.skill.SkillPackageOperations; +import com.orgmemory.core.organization.CurrentActor; +import com.orgmemory.core.permission.KnowledgeClassification; +import java.util.List; +import java.util.UUID; +import org.junit.jupiter.api.Test; +import org.springframework.security.core.Authentication; + +class AssetRegistryControllerTests { + + private static final UUID ORGANIZATION_ID = + UUID.fromString("89000000-0000-0000-0000-000000000001"); + private static final UUID USER_ID = + UUID.fromString("89000000-0000-0000-0000-000000000002"); + private static final UUID SPACE_ID = + UUID.fromString("89000000-0000-0000-0000-000000000003"); + private static final UUID AVAILABLE_ID = + UUID.fromString("89000000-0000-0000-0000-000000000004"); + private static final UUID PENDING_ID = + UUID.fromString("89000000-0000-0000-0000-000000000005"); + private static final CurrentActor ACTOR = new CurrentActor( + USER_ID, + ORGANIZATION_ID, + null, + "Skill editor", + "skill.editor@example.test"); + + @Test + void githubImportKeepsEveryItemWhenOneImportedAssetCannotBeResolved() { + AssetRegistryService assets = mock(AssetRegistryService.class); + SkillGitHubOperations github = mock(SkillGitHubOperations.class); + CurrentActorProvider actors = mock(CurrentActorProvider.class); + Authentication authentication = mock(Authentication.class); + AssetView available = new AssetView( + AVAILABLE_ID, + AssetType.SKILL, + "support", + "triage", + SPACE_ID, + null, + true, + null, + List.of(), + List.of(), + List.of(), + null, + List.of()); + when(actors.current(authentication)).thenReturn(ACTOR); + when(github.importSelected(any(), any())).thenReturn( + new SkillGitHubOperations.ImportResult( + "acme/skills", + "a".repeat(40), + SkillGitHubSourcePort.Visibility.PUBLIC, + List.of( + new SkillGitHubOperations.ImportItem( + "skills/triage/SKILL.md", + true, + AVAILABLE_ID, + "", + ""), + new SkillGitHubOperations.ImportItem( + "skills/pending/SKILL.md", + true, + PENDING_ID, + "", + ""), + new SkillGitHubOperations.ImportItem( + "skills/duplicate/SKILL.md", + false, + null, + "asset.conflict", + "An Asset already uses this coordinate")))); + when(assets.get(ACTOR, AVAILABLE_ID)).thenReturn(available); + when(assets.get(ACTOR, PENDING_ID)) + .thenThrow(new AssetUnavailableException("Projection pending")); + AssetRegistryController controller = new AssetRegistryController( + assets, + mock(SkillPackageOperations.class), + github, + actors); + + AssetRegistryController.ImportResult result = controller.importGitHubSkills( + new AssetRegistryController.GitHubSkillImportRequest( + new AssetRegistryController.GitHubSkillSourceRequest( + "acme/skills", "a".repeat(40), "skills", "", SPACE_ID), + List.of( + "skills/triage/SKILL.md", + "skills/pending/SKILL.md", + "skills/duplicate/SKILL.md"), + "support", + KnowledgeClassification.INTERNAL), + authentication); + + assertEquals(3, result.skills().size()); + assertSame(available, result.skills().getFirst().asset()); + AssetRegistryController.ImportItem pending = result.skills().get(1); + assertTrue(pending.imported()); + assertNull(pending.asset()); + assertEquals("asset.unavailable", pending.errorCode()); + assertEquals("Projection pending", pending.errorMessage()); + AssetRegistryController.ImportItem duplicate = result.skills().get(2); + assertFalse(duplicate.imported()); + assertEquals("asset.conflict", duplicate.errorCode()); + } +} diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java index 4cc8ba24..30be478b 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportService.java @@ -137,7 +137,7 @@ public ImportResult importSelected(CurrentActor actor, ImportRequest request) { } byte[] archive = candidate.archive(); try { - UUID assetId = skills.importPackage( + UUID assetId = skills.importPreauthorizedPackage( actor, request.namespace(), request.source().knowledgeSpaceId(), diff --git a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java index 6e42d93e..8382330b 100644 --- a/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java +++ b/core/src/main/java/com/orgmemory/core/assetregistry/skill/SkillRegistryService.java @@ -61,7 +61,9 @@ public UUID importPackage( KnowledgeClassification classification, long contentLength, InputStream content) { - return importPackage( + Objects.requireNonNull(actor, "actor"); + packages.requireCreate(actor, knowledgeSpaceId); + return stageAndImport( actor, namespace, knowledgeSpaceId, @@ -71,7 +73,26 @@ public UUID importPackage( null); } - UUID importPackage( + UUID importPreauthorizedPackage( + CurrentActor actor, + String namespace, + UUID knowledgeSpaceId, + KnowledgeClassification classification, + long contentLength, + InputStream content, + SkillPackageSpec.Origin origin) { + Objects.requireNonNull(origin, "origin"); + return stageAndImport( + actor, + namespace, + knowledgeSpaceId, + classification, + contentLength, + content, + origin); + } + + private UUID stageAndImport( CurrentActor actor, String namespace, UUID knowledgeSpaceId, @@ -81,7 +102,6 @@ UUID importPackage( SkillPackageSpec.Origin origin) { Objects.requireNonNull(actor, "actor"); Objects.requireNonNull(classification, "classification"); - packages.requireCreate(actor, knowledgeSpaceId); try (SkillPackageInspector.StagedSkillPackage staged = inspector.inspect(content, contentLength); InputStream packageContent = staged.open()) { diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java index 5b1d3407..19725a80 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillGitHubImportServiceTests.java @@ -87,7 +87,7 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { SkillGitHubSourcePort.Visibility.PRIVATE, List.of(valid(firstPath, 1), valid(secondPath, 2)))); UUID importedId = UUID.randomUUID(); - when(skills.importPackage( + when(skills.importPreauthorizedPackage( eq(ACTOR), eq("support"), eq(SPACE_ID), @@ -117,7 +117,7 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { assertEquals("asset.conflict", result.skills().get(1).errorCode()); ArgumentCaptor origins = ArgumentCaptor.forClass(SkillPackageSpec.Origin.class); - verify(skills, org.mockito.Mockito.times(2)).importPackage( + verify(skills, org.mockito.Mockito.times(2)).importPreauthorizedPackage( eq(ACTOR), eq("support"), eq(SPACE_ID), @@ -125,6 +125,7 @@ void importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance() { anyLong(), any(InputStream.class), origins.capture()); + verify(packages).requireCreate(ACTOR, SPACE_ID); assertEquals(List.of(firstPath, secondPath), origins.getAllValues().stream().map(SkillPackageSpec.Origin::path).toList()); assertEquals(SkillGitHubSourcePort.Visibility.PRIVATE, @@ -194,7 +195,7 @@ void importReportsASelectedPathMissingAtThePinnedRevision() { service.importSelected(ACTOR, request(SHA, List.of("removed/SKILL.md"))); assertEquals("skill.github-path-not-found", result.skills().getFirst().errorCode()); - verify(skills, never()).importPackage( + verify(skills, never()).importPreauthorizedPackage( any(), any(), any(), any(), anyLong(), any(InputStream.class), any()); } diff --git a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java index bfbfbfa9..8d77e573 100644 --- a/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java +++ b/core/src/test/java/com/orgmemory/core/assetregistry/skill/SkillRegistryServiceTests.java @@ -139,6 +139,24 @@ void replacementAuthorizesBeforeReadingAndRoutesTheCanonicalUpload() eq(ACTOR), eq(ASSET_ID), eq(7L), any(SkillPackageUpload.class)); } + @Test + void refusesUnauthorizedReplacementBeforeReadingPackageBytes() { + SkillPackageAssetCommand packages = mock(SkillPackageAssetCommand.class); + doThrow(new AssetNotFoundException()) + .when(packages) + .requireEdit(ACTOR, ASSET_ID); + SkillRegistryService service = service(packages); + + assertThrows( + AssetNotFoundException.class, + () -> service.replacePackage( + ACTOR, + ASSET_ID, + 7, + 1, + new UnreadableInputStream())); + } + private static SkillRegistryService service( SkillPackageAssetCommand packages) { return new SkillRegistryService(new SkillPackageInspector(), packages); From 53601e52b8a42f9aa564b3911d5f627365b458c8 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:53:57 +0700 Subject: [PATCH 5/6] docs(assetregistry): reconcile review fixes --- .../plan.md | 15 +++++++++++++++ docs/specs/domains/asset-registry.md | 6 +++++- docs/tests/domains/asset-registry.md | 6 +++--- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md index 59fbd375..6a7963ca 100644 --- a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md +++ b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md @@ -1624,3 +1624,18 @@ tests on exact Node 24.15.0. The mechanical fallback found 47 total changed paths, zero missing package declarations, zero changed zero-byte files, zero forbidden Skill imports, and a clean diff. The PR review loop remains before merge, and release remains deferred. + +CodeRabbit's completed PR 2 review raised two inline findings and one +outside-diff authorization suggestion. Commit `5f7faa70` fixes the valid +failure-isolation gap: API resolution of each imported Asset is now independent, +so a projection/read failure preserves every sibling result and reports a safe +per-item code without changing the wire shape. It also adds the missing +replacement pre-authorization regression and removes only the redundant middle +preflight check from the already-authorized GitHub path. Removing the direct +import or replacement pre-authorization was rejected because those checks +prevent unauthorized ZIP reads; the parent command still rechecks authority +immediately before mutation. Eliminating the bounded maximum-20 full-view reads +would require a new parent projection capability outside the challenged +boundary, so that performance redesign remains separate from this correctness +fix. Focused tests went red before the fix, then 686 full Core/API tests passed +with zero failure, error, or skip. diff --git a/docs/specs/domains/asset-registry.md b/docs/specs/domains/asset-registry.md index 18e91701..b7a406ea 100644 --- a/docs/specs/domains/asset-registry.md +++ b/docs/specs/domains/asset-registry.md @@ -13,7 +13,7 @@ Source: `core/src/main/java/com/orgmemory/core/assetregistry`, `apps/web/src/features/assets`, and `integrations/object-storage-minio/src/main/java`. -Reconciled: `2026-08-03-spring-modulith-package-refactor (4c7a8bf2)`. +Reconciled: `2026-08-03-spring-modulith-package-refactor (5f7faa70)`. ## Current Behavior @@ -238,6 +238,10 @@ back through the canonical Skill ZIP inspector. Authorization is checked before the batch. Each selected Skill then creates its own Asset in an independent `REQUIRES_NEW` transaction, so duplicate or invalid items return stable per-item failures without rolling back successful Drafts. +After import, the API resolves each successful Asset view independently. A +temporarily unavailable projection therefore leaves the persisted item marked +as imported with its path and stable read error while preserving every sibling +result, rather than failing the whole batch response. An actor with live `can_edit` may replace the package attached to a mutable Skill Draft. Core inspects and stores a fresh object before the transaction, diff --git a/docs/tests/domains/asset-registry.md b/docs/tests/domains/asset-registry.md index 50d65f00..f7881ec5 100644 --- a/docs/tests/domains/asset-registry.md +++ b/docs/tests/domains/asset-registry.md @@ -13,7 +13,7 @@ Source: `core/src/test/java/com/orgmemory/core/assetregistry`, `scripts/npm-publish-workflow-policy.test.mjs`, and `apps/web/src/features/assets/**/*.test.ts`. -Reconciled: `2026-08-03-spring-modulith-package-refactor (4c7a8bf2)`. +Reconciled: `2026-08-03-spring-modulith-package-refactor (5f7faa70)`. | Behavior | Evidence | Status | | --- | --- | --- | @@ -26,8 +26,8 @@ Reconciled: `2026-08-03-spring-modulith-package-refactor (4c7a8bf2)`. | Stateless Skill inspection returns canonical bounded metadata without storage; Scratch, raw `SKILL.md`, ZIP, and folder packaging converge on the same server validator | `SkillRegistryServiceTests#inspectionIsStatelessAndReturnsOnlyValidatedPackageFacts`, `skill-package-browser.test.ts`, `asset-registry-golden-poc.spec.ts` | covered | | GitHub Skill preview and private-connection discovery require Skill-create permission on the selected Knowledge Space, pin a full commit SHA, discover nearest bounded `SKILL.md` roots, reject unsafe/link/colliding archives, and keep invalid candidates independently visible | `GitHubSkillArchiveReaderTests`, `GitHubSkillSourceAdapterTests`, `SkillGitHubImportServiceTests`, `asset-registry-golden-poc.spec.ts#GitHub Skill import pins preview, supports private access, and reports partial results` | covered | | Private GitHub import requires an administrator opt-in and selected GitHub App repository, fails closed on missing repository identifiers, audits allow/deny credential use, distinguishes rate limits from private repositories, applies HTTP timeouts, disables generic redirects, validates the single codeload redirect, strips Authorization before archive download, and enforces archive-size bounds | `GitHubSkillSourceAdapterTests`, `connector-github.test.ts` | covered | -| Selected GitHub Skills import in independent transactions, persist server-derived repository/SHA/path provenance, and return partial success without rolling back completed Drafts | `SkillGitHubImportServiceTests#importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance`, `asset-registry-golden-poc.spec.ts#GitHub Skill import pins preview, supports private access, and reports partial results` | covered | -| Skill Draft replacement requires live edit authorization plus the expected Draft version, compensates fresh storage on transaction failure, and never mutates an immutable package reference | `SkillRegistryServiceTests`, `AssetRegistryIntegrationTests#replacingAReleasedSkillDraftKeepsTheImmutablePackageAndClearsTheCleanupRow`, `AssetRegistryIntegrationTests#replacingAnUnreleasedSkillDraftDeletesItsUnreferencedOldPackage` | covered | +| Selected GitHub Skills import in independent transactions, persist server-derived repository/SHA/path provenance, and return partial success without rolling back completed Drafts or losing sibling results when one imported Asset view is temporarily unavailable | `SkillGitHubImportServiceTests#importsEachSelectedSkillIndependentlyAndPreservesPinnedProvenance`, `AssetRegistryControllerTests#githubImportKeepsEveryItemWhenOneImportedAssetCannotBeResolved`, `asset-registry-golden-poc.spec.ts#GitHub Skill import pins preview, supports private access, and reports partial results` | covered | +| Skill Draft replacement requires live edit authorization before package bytes are read plus an authoritative recheck before mutation and the expected Draft version, compensates fresh storage on transaction failure, and never mutates an immutable package reference | `SkillRegistryServiceTests`, `AssetRegistryIntegrationTests#replacingAReleasedSkillDraftKeepsTheImmutablePackageAndClearsTheCleanupRow`, `AssetRegistryIntegrationTests#replacingAnUnreleasedSkillDraftDeletesItsUnreferencedOldPackage` | covered | | Database mutation guards allow only Draft-reference deletion; payload-reference update and Revision/Release deletion remain rejected | `AssetRegistryIntegrationTests#onlyDraftPayloadReferencesMayBeDeletedWhileAllReferenceUpdatesStayRejected` | covered | | Post-commit supersession cleanup deletes only an exact unreferenced object, retains immutable pins, and durably schedules bounded retries after storage failure | `SkillPackageSupersessionCleanupCoordinatorTests` | covered | | The four parent-owned Skill capabilities expose exact type sets and exact Core/API/Worker/MinIO consumer sets; storage locators do not enter API or Worker dependencies | `ModulithVerificationTests#assetRegistrySkillCapabilitiesAreExactExplicitNamedInterfaces`, `#assetRegistrySkillCapabilitiesHaveExactCoreConsumers`, `SkillCapabilityBoundaryTests`, `MinioSkillPackageStorageAdapterTests#adapterExposesOnlyTheParentStorageCapability` | covered | From a706ed112420c77bfd9319540294f94d4b5593d2 Mon Sep 17 00:00:00 2001 From: kl3inIT Date: Mon, 3 Aug 2026 10:58:37 +0700 Subject: [PATCH 6/6] docs(assetregistry): record review verification --- .../2026-07-31-spring-modulith-package-refactor/plan.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md index 6a7963ca..97808f2a 100644 --- a/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md +++ b/docs/increments/active/2026-07-31-spring-modulith-package-refactor/plan.md @@ -1638,4 +1638,6 @@ immediately before mutation. Eliminating the bounded maximum-20 full-view reads would require a new parent projection capability outside the challenged boundary, so that performance redesign remains separate from this correctness fix. Focused tests went red before the fix, then 686 full Core/API tests passed -with zero failure, error, or skip. +with zero failure, error, or skip. The follow-up repository-wide `clean test` +passed all 99 tasks and 1,267 tests; docs hygiene, release policy, and diff +checks also remained green. The PR now changes 48 paths, still below the cap.