Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -46,15 +46,15 @@ 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,
AssetRegistryService assets,
PromptExecutionService prompts,
WorkInstructionService instructions,
CapabilityPackService packs,
SkillDistributionService skills) {
SkillDistributionOperations skills) {
this.actors = actors;
this.assets = assets;
this.prompts = prompts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
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.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;
Expand All @@ -24,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;
Expand All @@ -43,6 +48,9 @@
@RequestMapping("/api/assets")
class AssetRegistryController {

private static final Logger LOG =
LoggerFactory.getLogger(AssetRegistryController.class);

enum GenericAssetType {
PROMPT_TEMPLATE,
WORK_INSTRUCTION,
Expand All @@ -54,14 +62,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;
Expand Down Expand Up @@ -124,8 +132,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);
}
}
Expand All @@ -137,6 +145,21 @@ record GitHubSkillImportRequest(
KnowledgeClassification classification) {
}

record ImportResult(
String repository,
String revision,
SkillGitHubSourcePort.Visibility visibility,
List<ImportItem> skills) {
}

record ImportItem(
String path,
boolean imported,
AssetView asset,
String errorCode,
String errorMessage) {
}

record AssetAvailabilityRequest(String reason) {
}

Expand Down Expand Up @@ -178,13 +201,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);
Expand Down Expand Up @@ -213,7 +238,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(
Expand All @@ -224,7 +249,7 @@ SkillGitHubImportService.Preview previewGitHubSkills(
@Operation(
operationId = "listGitHubSkillConnections",
summary = "List approved GitHub connections available for private Skill import")
List<com.orgmemory.core.assetregistry.SkillGitHubSourcePort.ConnectionOption>
List<SkillGitHubOperations.ConnectionOption>
listGitHubSkillConnections(
@RequestParam UUID knowledgeSpaceId,
Authentication authentication) {
Expand All @@ -236,18 +261,63 @@ 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 -> importItem(actor, item))
.toList());
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

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(
Expand All @@ -262,12 +332,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand All @@ -88,7 +88,7 @@ void bearerTokenCannotBypassDeliveryScopeThroughTheBrowserEndpoint() {

private static AssetConsumptionController controller(
CurrentActorProvider actors,
SkillDistributionService skills) {
SkillDistributionOperations skills) {
return new AssetConsumptionController(
actors,
mock(AssetRegistryService.class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -93,8 +93,8 @@ PromptExecutionService prompts() {
}

@Bean
SkillDistributionService skills() {
return mock(SkillDistributionService.class);
SkillDistributionOperations skills() {
return mock(SkillDistributionOperations.class);
}

@Bean
Expand All @@ -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);
Expand Down
Loading