diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/authoring/SkillAuthoringController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/authoring/SkillAuthoringController.java index 936de7b9c..504ac3b5e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/authoring/SkillAuthoringController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/authoring/SkillAuthoringController.java @@ -2,9 +2,13 @@ import com.iflytek.skillhub.controller.BaseApiController; import com.iflytek.skillhub.domain.authoring.SkillDraft; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.authoring.service.DraftSubmitService; +import com.iflytek.skillhub.domain.authoring.service.FindingFixService; import com.iflytek.skillhub.domain.authoring.service.RuntimeBindingService; import com.iflytek.skillhub.domain.authoring.service.SkillDraftService; import com.iflytek.skillhub.domain.authoring.service.ValidationRunService; +import com.iflytek.skillhub.domain.authoring.validation.ValidationFinding; import com.iflytek.skillhub.domain.authoring.validation.ValidationRun; import com.iflytek.skillhub.dto.ApiResponse; import com.iflytek.skillhub.dto.CreateDraftRequest; @@ -13,6 +17,8 @@ import com.iflytek.skillhub.dto.DraftResponse; import com.iflytek.skillhub.dto.RuntimeBindingRequest; import com.iflytek.skillhub.dto.RuntimeBindingResponse; +import com.iflytek.skillhub.dto.SubmitDraftRequest; +import com.iflytek.skillhub.dto.SubmitDraftResponse; import com.iflytek.skillhub.dto.SaveDraftFileRequest; import com.iflytek.skillhub.dto.SaveDraftFileResponse; import com.iflytek.skillhub.dto.ValidationEventResponse; @@ -42,8 +48,8 @@ * SKILL.md scaffold and arbitrary resource files; runtime bindings configure * the agent, tool allowlist and MCP servers; validation runs execute the * structure, config and behavior layers and report findings with fix - * suggestions. The fix-apply loop and publish integration arrive with the - * next layer. + * suggestions that can be applied in one click; validated drafts submit into + * the existing SkillHub publish pipeline. */ @RestController @RequestMapping({"/api/v1/authoring", "/api/web/authoring"}) @@ -52,17 +58,23 @@ public class SkillAuthoringController extends BaseApiController { private final SkillDraftService draftService; private final RuntimeBindingService bindingService; private final ValidationRunService runService; + private final DraftSubmitService submitService; + private final FindingFixService fixService; private final ValidationRunOrchestrator orchestrator; public SkillAuthoringController(com.iflytek.skillhub.dto.ApiResponseFactory responseFactory, SkillDraftService draftService, RuntimeBindingService bindingService, ValidationRunService runService, + DraftSubmitService submitService, + FindingFixService fixService, ValidationRunOrchestrator orchestrator) { super(responseFactory); this.draftService = draftService; this.bindingService = bindingService; this.runService = runService; + this.submitService = submitService; + this.fixService = fixService; this.orchestrator = orchestrator; } @@ -272,4 +284,49 @@ public ApiResponse> listFindings( .map(ValidationFindingResponse::from) .toList()); } + + // ---------------------------------------------------------------- fix loop + + @Operation(operationId = "applyFindingFix", summary = "Apply a finding's fix suggestion to the draft") + @PostMapping("/runs/{runId}/findings/{findingId}/apply") + public ApiResponse applyFindingFix( + @PathVariable Long runId, + @PathVariable Long findingId, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "platformRoles", required = false) + Set platformRoles) { + FindingFixService.FixOutcome outcome = fixService.applyFix(findingId, userId, platformRoles); + return ok("response.success.updated", ValidationFindingResponse.from(outcome.finding())); + } + + @Operation(operationId = "dismissFindingFix", summary = "Dismiss a finding as not applicable") + @PostMapping("/runs/{runId}/findings/{findingId}/dismiss") + public ApiResponse dismissFinding( + @PathVariable Long runId, + @PathVariable Long findingId, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "platformRoles", required = false) + Set platformRoles) { + ValidationFinding finding = fixService.dismissFinding(findingId, userId, platformRoles); + return ok("response.success.updated", ValidationFindingResponse.from(finding)); + } + + // ---------------------------------------------------------------- submission + + @Operation(operationId = "submitValidatedDraft", summary = "Submit a validated draft into the publish pipeline") + @PostMapping("/drafts/{draftId}/submit") + public ApiResponse submitDraft(@PathVariable Long draftId, + @RequestBody(required = false) SubmitDraftRequest request, + @RequestAttribute("userId") String userId, + @RequestAttribute(value = "platformRoles", required = false) + Set platformRoles) { + SkillVisibility visibility = request == null || request.visibility() == null + ? SkillVisibility.PRIVATE + : SkillVisibility.valueOf(request.visibility()); + Set roles = request == null || request.platformRoles() == null + ? platformRoles + : request.platformRoles(); + return ok("response.success.created", + SubmitDraftResponse.from(submitService.submit(draftId, userId, visibility, roles))); + } } diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftRequest.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftRequest.java new file mode 100644 index 000000000..b6fe7eaff --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftRequest.java @@ -0,0 +1,10 @@ +package com.iflytek.skillhub.dto; + +import java.util.Set; + +/** Request body for submitting a validated draft into the publish pipeline. */ +public record SubmitDraftRequest( + String visibility, + Set platformRoles +) { +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftResponse.java new file mode 100644 index 000000000..ebaaf9823 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SubmitDraftResponse.java @@ -0,0 +1,17 @@ +package com.iflytek.skillhub.dto; + +import com.iflytek.skillhub.domain.authoring.service.DraftSubmitService; + +/** Outcome of submitting a validated draft: the published skill version coordinates. */ +public record SubmitDraftResponse( + Long skillId, + Long versionId, + String slug, + String version +) { + + public static SubmitDraftResponse from(DraftSubmitService.SubmitOutcome outcome) { + return new SubmitDraftResponse( + outcome.skillId(), outcome.versionId(), outcome.slug(), outcome.version()); + } +} diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index b49e891ff..387e579ae 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -354,3 +354,6 @@ error.authoring.binding.notFound=No runtime binding configured for draft {0} error.authoring.binding.agentType.unknown=Unknown agent runtime type: {0} error.authoring.binding.invalid=Invalid runtime binding: {0} error.authoring.binding.unserializable=Runtime binding could not be serialized +# authoring fix loop and submission +error.authoring.submit.notValidated=Revision {0} has no passing validation run; run validation first +error.authoring.submit.runActive=Draft {0} has an active validation run diff --git a/server/skillhub-app/src/main/resources/messages_ru.properties b/server/skillhub-app/src/main/resources/messages_ru.properties index 72c1168b2..af3e0c0da 100644 --- a/server/skillhub-app/src/main/resources/messages_ru.properties +++ b/server/skillhub-app/src/main/resources/messages_ru.properties @@ -297,3 +297,6 @@ error.authoring.binding.notFound=Для черновика {0} не настро error.authoring.binding.agentType.unknown=Неизвестный тип среды выполнения агента: {0} error.authoring.binding.invalid=Недопустимая привязка среды выполнения: {0} error.authoring.binding.unserializable=Привязка среды выполнения не может быть сериализована +# authoring fix loop and submission +error.authoring.submit.notValidated=У ревизии {0} нет пройденной задачи проверки; сначала выполните проверку +error.authoring.submit.runActive=У черновика {0} есть активная задача проверки diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index 657302f94..7f5da4b5e 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -356,3 +356,6 @@ error.authoring.binding.notFound=草稿 {0} 尚未配置运行时绑定 error.authoring.binding.agentType.unknown=未知的 Agent 运行时类型：{0} error.authoring.binding.invalid=运行时绑定不合法：{0} error.authoring.binding.unserializable=运行时绑定无法序列化 +# authoring fix loop and submission +error.authoring.submit.notValidated=版本 {0} 没有通过的验证任务，请先执行验证 +error.authoring.submit.runActive=草稿 {0} 有正在进行的验证任务 diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/authoring/AuthoringFlowIntegrationTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/authoring/AuthoringFlowIntegrationTest.java new file mode 100644 index 000000000..6464abb16 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/authoring/AuthoringFlowIntegrationTest.java @@ -0,0 +1,397 @@ +package com.iflytek.skillhub.authoring; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.domain.authoring.SkillDraft; +import com.iflytek.skillhub.domain.authoring.service.DraftSubmitService; +import com.iflytek.skillhub.domain.authoring.service.FindingFixService; +import com.iflytek.skillhub.domain.authoring.service.RuntimeBindingService; +import com.iflytek.skillhub.domain.authoring.service.SkillDraftService; +import com.iflytek.skillhub.domain.authoring.service.ValidationRunService; +import com.iflytek.skillhub.domain.authoring.validation.FindingSeverity; +import com.iflytek.skillhub.domain.authoring.validation.ValidationFinding; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRun; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRunStatus; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.service.authoring.ValidationRunOrchestrator; +import com.iflytek.skillhub.storage.ObjectMetadata; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.time.Duration; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.Primary; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.containers.PostgreSQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +/** + * End-to-end authoring flow against the real service stack (Testcontainers + * PostgreSQL, in-memory object storage, mocked publish pipeline): create draft → + * edit files → bind runtime → validate (structure + config + behavior) → fix + * findings → re-validate → submit. Runs on real PostgreSQL because the JSONB + * columns must round-trip exactly as they do in production. + */ +@SpringBootTest +@ActiveProfiles("test") +@Import({TestRedisConfig.class, AuthoringFlowIntegrationTest.InMemoryStorageConfig.class}) +@Testcontainers +class AuthoringFlowIntegrationTest { + + @Container + private static final PostgreSQLContainer POSTGRES = + new PostgreSQLContainer<>("postgres:16-alpine"); + + @DynamicPropertySource + static void configurePostgres(DynamicPropertyRegistry registry) { + registry.add("spring.datasource.url", POSTGRES::getJdbcUrl); + registry.add("spring.datasource.username", POSTGRES::getUsername); + registry.add("spring.datasource.password", POSTGRES::getPassword); + registry.add("spring.datasource.driver-class-name", () -> "org.postgresql.Driver"); + registry.add("spring.jpa.database-platform", () -> "org.hibernate.dialect.PostgreSQLDialect"); + registry.add("spring.flyway.enabled", () -> true); + registry.add("spring.jpa.hibernate.ddl-auto", () -> "validate"); + } + + private static final String USER = "author-user-1"; + private static final String NS_SLUG = "authoring-test-ns"; + + @Autowired private SkillDraftService draftService; + @Autowired private RuntimeBindingService bindingService; + @Autowired private ValidationRunService runService; + @Autowired private DraftSubmitService submitService; + @Autowired private FindingFixService fixService; + @Autowired private ValidationRunOrchestrator orchestrator; + @Autowired private NamespaceRepository namespaceRepository; + @Autowired private NamespaceMemberRepository namespaceMemberRepository; + @Autowired private UserAccountRepository userAccountRepository; + + @MockBean private SkillPublishService publishService; + + @TestConfiguration + static class InMemoryStorageConfig { + + @Bean + @Primary + ObjectStorageService inMemoryObjectStorage() { + return new InMemoryObjectStorage(); + } + } + + static final class InMemoryObjectStorage implements ObjectStorageService { + private final Map objects = new ConcurrentHashMap<>(); + + @Override + public void putObject(String key, InputStream data, long size, String contentType) { + try { + objects.put(key, data.readAllBytes()); + } catch (Exception exception) { + throw new IllegalStateException(exception); + } + } + + @Override + public InputStream getObject(String key) { + byte[] content = objects.get(key); + if (content == null) { + throw new IllegalArgumentException("missing object: " + key); + } + return new java.io.ByteArrayInputStream(content); + } + + @Override + public void deleteObject(String key) { + objects.remove(key); + } + + @Override + public void deleteObjects(List keys) { + keys.forEach(objects::remove); + } + + @Override + public boolean exists(String key) { + return objects.containsKey(key); + } + + @Override + public ObjectMetadata getMetadata(String key) { + byte[] content = objects.get(key); + if (content == null) { + throw new IllegalArgumentException("missing object: " + key); + } + return new ObjectMetadata(content.length, "application/octet-stream", java.time.Instant.now()); + } + + @Override + public String generatePresignedUrl(String key, Duration expiry, String downloadFilename) { + return "http://localhost/" + key; + } + } + + @BeforeEach + void setUpNamespace() { + if (userAccountRepository.findById(USER).isEmpty()) { + userAccountRepository.save(new UserAccount(USER, "Author User", "author@example.com", null)); + } + if (namespaceRepository.findBySlug(NS_SLUG).isEmpty()) { + Namespace namespace = new Namespace(NS_SLUG, "Authoring Test", USER); + namespace.setStatus(com.iflytek.skillhub.domain.namespace.NamespaceStatus.ACTIVE); + Namespace saved = namespaceRepository.save(namespace); + if (namespaceMemberRepository.findByNamespaceIdAndUserId(saved.getId(), USER).isEmpty()) { + namespaceMemberRepository.save(new NamespaceMember(saved.getId(), USER, NamespaceRole.MEMBER)); + } + } + } + + @Test + void validatedDraftRunsAllLayersAndSubmits() { + // ---- create with scaffold + SkillDraft draft = draftService.createDraft(NS_SLUG, USER, "demo-flow", "summarizes documents", null); + assertThat(draft.getRevision()).isEqualTo(1); + assertThat(draftService.listFiles(draft.getId())) + .extracting(file -> file.getFilePath()) + .containsExactly("SKILL.md"); + // the scaffold bytes must be readable straight away, before any manual + // save — creation persists content, not just the metadata row + assertThat(draftService.readFile(draft.getId(), USER, "SKILL.md", null).asText()) + .contains("name: demo-flow") + .contains("description: summarizes documents"); + + // ---- edit files: overwrite SKILL.md, add script + validation.yaml + draftService.saveFile(draft.getId(), USER, "SKILL.md", validSkillMd(), "text/markdown", null, null); + draftService.saveFile(draft.getId(), USER, "scripts/check.sh", + "echo 'smoke OK'\n".getBytes(StandardCharsets.UTF_8), null, null, null); + draftService.saveFile(draft.getId(), USER, "validation.yaml", + validationYaml().getBytes(StandardCharsets.UTF_8), null, null, null); + assertThat(draftService.getDraft(draft.getId()).getRevision()).isEqualTo(4); + + // ---- bind runtime: local script execution + bindingService.saveBinding(draft.getId(), USER, "local-script", + Map.of("interpreter", "sh"), null, null, null); + + // ---- validate: all three layers + ValidationRun run = runService.startRun(draft.getId(), USER, null); + orchestrator.submit(run.getId()); + ValidationRun finished = awaitTerminal(run.getId()); + + assertThat(finished.getStatus()).isEqualTo(ValidationRunStatus.SUCCEEDED); + assertThat(finished.getErrorCount()).isZero(); + assertThat(runService.listFindings(run.getId())).isEmpty(); + assertThat(runService.listEvents(run.getId(), null)) + .extracting(event -> event.getEventType().name()) + .contains("RUN_STARTED", "PHASE_STARTED", "TOOL_CALL", "LOG", "RUN_FINISHED"); + + // ---- draft is validated for this revision + SkillDraft validated = draftService.getDraft(draft.getId()); + assertThat(validated.isCurrentRevisionValidated()).isTrue(); + assertThat(validated.getValidatedRevision()).isEqualTo(validated.getRevision()); + + // ---- submit into the (mocked) publish pipeline + SkillVersion publishedVersion = new SkillVersion(99L, "1.0.0", USER); + when(publishService.publishFromEntries(eq(NS_SLUG), any(), eq(USER), + eq(SkillVisibility.PRIVATE), any(), anyBoolean())) + .thenReturn(new SkillPublishService.PublishResult(99L, "demo-flow", publishedVersion)); + + DraftSubmitService.SubmitOutcome outcome = submitService.submit( + draft.getId(), USER, SkillVisibility.PRIVATE, null); + + assertThat(outcome.skillId()).isEqualTo(99L); + assertThat(outcome.version()).isEqualTo("1.0.0"); + assertThat(draftService.getDraft(draft.getId()).getSubmittedSkillId()).isEqualTo(99L); + } + + @Test + void brokenFrontmatterProducesFixableFindingAndRevalidationLoop() { + SkillDraft draft = draftService.createDraft(NS_SLUG, USER, "broken-flow", null, null); + // SKILL.md without the required description field + draftService.saveFile(draft.getId(), USER, "SKILL.md", + "---\nname: broken-flow\n---\n\n# Broken\n".getBytes(StandardCharsets.UTF_8), + "text/markdown", null, null); + + ValidationRun run = runService.startRun(draft.getId(), USER, null); + orchestrator.submit(run.getId()); + awaitTerminal(run.getId()); + + List findings = runService.listFindings(run.getId()); + assertThat(findings).isNotEmpty(); + Optional missingField = findings.stream() + .filter(finding -> "FRONTMATTER_FIELD_MISSING".equals(finding.getRuleCode())) + .findFirst(); + assertThat(missingField).isPresent(); + assertThat(missingField.get().getSeverity()).isEqualTo(FindingSeverity.ERROR); + assertThat(missingField.get().hasSuggestion()).isTrue(); + assertThat(runService.getRun(run.getId()).getStatus()).isEqualTo(ValidationRunStatus.FAILED); + + // ---- apply the suggested fix + int revisionBefore = draftService.getDraft(draft.getId()).getRevision(); + FindingFixService.FixOutcome outcome = fixService.applyFix( + missingField.get().getId(), USER, null); + assertThat(outcome.finding().getStatus().name()).isEqualTo("APPLIED"); + assertThat(outcome.draft().getRevision()).isGreaterThan(revisionBefore); + + // ---- validation verdict is invalidated by the new revision + SkillDraft fixed = draftService.getDraft(draft.getId()); + assertThat(fixed.isCurrentRevisionValidated()).isFalse(); + + // ---- re-validate: now the description field exists and the run passes + ValidationRun second = runService.startRun(draft.getId(), USER, null); + orchestrator.submit(second.getId()); + ValidationRun finishedSecond = awaitTerminal(second.getId()); + + assertThat(finishedSecond.getStatus()).isEqualTo(ValidationRunStatus.SUCCEEDED); + assertThat(draftService.getDraft(draft.getId()).isCurrentRevisionValidated()).isTrue(); + } + + @Test + void deadMcpServerFailsTheRunWithoutValidationYaml() { + // no validation.yaml on purpose: the config layer must still check the + // runtime binding — a declared MCP server that cannot be reached is a + // config error even when the behavior layer has no tasks to run + SkillDraft draft = draftService.createDraft(NS_SLUG, USER, "mcp-probe-flow", + "declares a dead MCP server", null); + draftService.saveFile(draft.getId(), USER, "SKILL.md", validSkillMd(), "text/markdown", null, null); + + bindingService.saveBinding(draft.getId(), USER, "local-script", + Map.of("interpreter", "sh"), null, + List.of(Map.of("name", "dead-server", "transport", "http", + "endpoint", "http://127.0.0.1:9/mcp")), null); + + ValidationRun run = runService.startRun(draft.getId(), USER, null); + orchestrator.submit(run.getId()); + ValidationRun finished = awaitTerminal(run.getId()); + + assertThat(finished.getStatus()).isEqualTo(ValidationRunStatus.FAILED); + assertThat(runService.listFindings(run.getId())) + .extracting(ValidationFinding::getRuleCode) + .contains("SPEC_MISSING", "MCP_CONNECT_FAILED"); + } + + @Test + void securityPolicyRejectsUnsafeBindingsAtSaveTime() { + SkillDraft draft = draftService.createDraft(NS_SLUG, USER, "security-flow", + "exercises the binding security policy", null); + draftService.saveFile(draft.getId(), USER, "SKILL.md", validSkillMd(), "text/markdown", null, null); + + // cloud metadata endpoints are always blocked — even under the test + // profile where loopback/RFC1918 endpoints are deliberately allowed + assertBindingRejected(() -> bindingService.saveBinding(draft.getId(), USER, "local-script", + Map.of("interpreter", "sh"), null, + List.of(Map.of("name", "metadata", "transport", "http", + "endpoint", "http://169.254.169.254/latest/meta-data/")), null), + "link-local"); + + // envRefs outside the configured allowlist are rejected: bindings must + // not be able to read arbitrary server environment variables + assertBindingRejected(() -> bindingService.saveBinding(draft.getId(), USER, "local-script", + Map.of("interpreter", "sh"), null, + List.of(Map.of("name", "env-hog", "transport", "http", + "endpoint", "http://127.0.0.1:9/mcp", + "envRefs", List.of("HOME"))), null), + "env-allowlist"); + + // the LLM surface is guarded too — those requests carry the server API key + assertBindingRejected(() -> bindingService.saveBinding(draft.getId(), USER, "openai-compatible", + Map.of("endpoint", "http://169.254.169.254/v1", "model", "test-model"), null, null, null), + "link-local"); + + // the allowlisted env ref saves fine (loopback is allowed under the test profile) + bindingService.saveBinding(draft.getId(), USER, "local-script", + Map.of("interpreter", "sh"), null, + List.of(Map.of("name", "env-ok", "transport", "http", + "endpoint", "http://127.0.0.1:9/mcp", + "envRefs", List.of("TEST_ALLOWED_MCP_VAR"))), null); + } + + // ---------------------------------------------------------------- helpers + + /** Save-time rejection: a 400 whose first message arg explains the reason. */ + private static void assertBindingRejected(Runnable save, String reasonFragment) { + try { + save.run(); + fail("binding save should have been rejected: " + reasonFragment); + } catch (com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException exception) { + assertThat(String.valueOf(exception.messageArgs()[0])).contains(reasonFragment); + } + } + + private ValidationRun awaitTerminal(Long runId) { + long deadline = System.currentTimeMillis() + 60_000; + while (System.currentTimeMillis() < deadline) { + if (runService.getRun(runId).getStatus().isTerminal()) { + return runService.getRun(runId); + } + try { + Thread.sleep(200); + } catch (InterruptedException exception) { + Thread.currentThread().interrupt(); + fail("interrupted while waiting for the validation run to finish"); + } + } + fail("validation run " + runId + " did not reach a terminal state in time"); + return null; + } + + private static byte[] validSkillMd() { + return """ + --- + name: demo-flow + description: summarizes documents for the demo + --- + + # Demo flow + + ## Overview + + summarizes documents for the demo + """.getBytes(StandardCharsets.UTF_8); + } + + private static String validationYaml() { + return """ + version: 1 + tasks: + - name: smoke + description: script runs and prints OK + type: script + script: scripts/check.sh + args: [] + timeoutMs: 10000 + assertions: + - type: exit_code + equals: 0 + - type: stdout_contains + value: smoke OK + """; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/DraftSubmitService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/DraftSubmitService.java new file mode 100644 index 000000000..7d74b8f73 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/DraftSubmitService.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.domain.authoring.service; + +import com.iflytek.skillhub.domain.authoring.SkillDraft; +import com.iflytek.skillhub.domain.authoring.SkillDraftRepository; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRun; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRunStatus; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.service.SkillPublishService; +import com.iflytek.skillhub.domain.skill.validation.PackageEntry; +import java.time.Clock; +import java.util.Set; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Submits a validated draft into the existing SkillHub publish pipeline. The gate is + * strict: the draft's current revision must be bound to a validation run that + * succeeded with zero errors, so content can never be published without the exact + * bytes that were validated. Publishing reuses {@link SkillPublishService}, which + * re-validates the package, triggers the security scan, and opens the review flow. + */ +@Service +public class DraftSubmitService { + + private static final Logger log = LoggerFactory.getLogger(DraftSubmitService.class); + private static final String SUPER_ADMIN = "SUPER_ADMIN"; + + public record SubmitOutcome(Long skillId, Long versionId, String slug, String version, + SkillPublishService.PublishResult publishResult) {} + + private final SkillDraftRepository draftRepository; + private final SkillDraftService draftService; + private final ValidationRunService runService; + private final SkillPublishService publishService; + private final Clock clock; + + public DraftSubmitService(SkillDraftRepository draftRepository, + SkillDraftService draftService, + ValidationRunService runService, + SkillPublishService publishService, + Clock clock) { + this.draftRepository = draftRepository; + this.draftService = draftService; + this.runService = runService; + this.publishService = publishService; + this.clock = clock; + } + + /** + * Publishes the draft's current revision as a new skill version. Warnings that the + * validation run already surfaced to the author are confirmed here; errors were a + * hard gate and cannot reach this point. + */ + @Transactional + public SubmitOutcome submit(Long draftId, String userId, SkillVisibility visibility, + Set platformRoles) { + SkillDraft draft = draftRepository.findById(draftId) + .orElseThrow(() -> new DomainNotFoundException("error.authoring.draft.notFound", draftId)); + assertOwner(draft, userId, platformRoles); + + if (runService.listRuns(draftId).stream().anyMatch(ValidationRun::isActive)) { + throw new DomainConflictException("error.authoring.submit.runActive", draftId); + } + ValidationRun passingRun = runService.requirePassingRunForRevision( + draftId, draft.getRevision()); + + String namespaceSlug = draftService.resolveNamespaceSlug(draft); + java.util.List entries = draftService.materializeEntries(draftId); + + SkillPublishService.PublishResult result = publishService.publishFromEntries( + namespaceSlug, entries, userId, visibility, platformRoles, true); + + draft.markSubmitted(result.skillId(), result.version().getId(), clock.instant()); + draftRepository.save(draft); + log.info("Draft {} revision {} submitted as skill {} version {} (validation run {})", + draftId, draft.getRevision(), result.skillId(), + result.version().getVersion(), passingRun.getId()); + return new SubmitOutcome(result.skillId(), result.version().getId(), result.slug(), + result.version().getVersion(), result); + } + + private void assertOwner(SkillDraft draft, String userId, Set platformRoles) { + if (draft.getOwnerId().equals(userId) + || (platformRoles != null && platformRoles.contains(SUPER_ADMIN))) { + return; + } + throw new com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException( + "error.authoring.draft.forbidden", draft.getId()); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/FindingFixService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/FindingFixService.java new file mode 100644 index 000000000..8bc9a2c57 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/authoring/service/FindingFixService.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.domain.authoring.service; + +import com.iflytek.skillhub.domain.authoring.SkillDraft; +import com.iflytek.skillhub.domain.authoring.SkillDraftRepository; +import com.iflytek.skillhub.domain.authoring.validation.FindingStatus; +import com.iflytek.skillhub.domain.authoring.validation.ValidationFinding; +import com.iflytek.skillhub.domain.authoring.validation.ValidationFindingRepository; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRun; +import com.iflytek.skillhub.domain.authoring.validation.ValidationRunRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException; +import com.iflytek.skillhub.domain.shared.exception.DomainConflictException; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; +import java.util.Set; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Closes the fix loop for validation findings: an author previews a finding's + * machine-applicable suggestion, then confirms it. Applying the patch produces a new + * draft revision through {@link SkillDraftService#applyPatch} (which re-validates all + * sha256 guards), marks the finding APPLIED, and thereby invalidates the previous + * validation verdict — re-validation is always required afterwards. + */ +@Service +public class FindingFixService { + + private static final String SUPER_ADMIN = "SUPER_ADMIN"; + + private final ValidationFindingRepository findingRepository; + private final ValidationRunRepository runRepository; + private final SkillDraftRepository draftRepository; + private final SkillDraftService draftService; + + public FindingFixService(ValidationFindingRepository findingRepository, + ValidationRunRepository runRepository, + SkillDraftRepository draftRepository, + SkillDraftService draftService) { + this.findingRepository = findingRepository; + this.runRepository = runRepository; + this.draftRepository = draftRepository; + this.draftService = draftService; + } + + public record FixOutcome(ValidationFinding finding, SkillDraft draft) {} + + /** + * Applies the finding's fix suggestion to the draft. The whole operation is atomic: + * either every patch lands and the finding is marked APPLIED, or nothing changes. + */ + @Transactional + public FixOutcome applyFix(Long findingId, String userId, Set platformRoles) { + OwnedFinding owned = loadOwnedFinding(findingId, userId, platformRoles); + ValidationFinding finding = owned.finding(); + if (!finding.hasSuggestion()) { + throw new DomainBadRequestException("error.authoring.finding.noSuggestion", findingId); + } + if (finding.getStatus() == FindingStatus.APPLIED) { + throw new DomainConflictException("error.authoring.finding.applied", findingId); + } + + SkillDraft draft = draftService.applyPatch( + owned.draftId(), userId, finding.getSuggestion().safePatches(), null, platformRoles); + finding.markApplied(draft.getRevision()); + findingRepository.save(finding); + return new FixOutcome(finding, draft); + } + + /** Dismisses a finding the author considers not applicable. */ + @Transactional + public ValidationFinding dismissFinding(Long findingId, String userId, Set platformRoles) { + ValidationFinding finding = loadOwnedFinding(findingId, userId, platformRoles).finding(); + finding.markDismissed(); + return findingRepository.save(finding); + } + + private record OwnedFinding(ValidationFinding finding, Long draftId) {} + + private OwnedFinding loadOwnedFinding(Long findingId, String userId, Set platformRoles) { + ValidationFinding finding = findingRepository.findById(findingId) + .orElseThrow(() -> new DomainNotFoundException("error.authoring.finding.notFound", findingId)); + ValidationRun run = runRepository.findById(finding.getRunId()) + .orElseThrow(() -> new DomainNotFoundException( + "error.authoring.run.notFound", finding.getRunId())); + SkillDraft draft = draftRepository.findById(run.getDraftId()) + .orElseThrow(() -> new DomainNotFoundException( + "error.authoring.draft.notFound", run.getDraftId())); + if (draft.getOwnerId().equals(userId) + || (platformRoles != null && platformRoles.contains(SUPER_ADMIN))) { + return new OwnedFinding(finding, draft.getId()); + } + throw new DomainForbiddenException("error.authoring.draft.forbidden", draft.getId()); + } +} diff --git a/web/src/api/generated/schema.d.ts b/web/src/api/generated/schema.d.ts index 85e885699..37c4bc96d 100644 --- a/web/src/api/generated/schema.d.ts +++ b/web/src/api/generated/schema.d.ts @@ -2470,6 +2470,74 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/authoring/runs/{runId}/findings/{findingId}/dismiss": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Dismiss a finding as not applicable */ + post: operations["dismissFindingFix"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/authoring/runs/{runId}/findings/{findingId}/dismiss": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Dismiss a finding as not applicable */ + post: operations["dismissFindingFix_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/web/authoring/runs/{runId}/findings/{findingId}/apply": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Apply a finding's fix suggestion to the draft */ + post: operations["applyFindingFix"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/authoring/runs/{runId}/findings/{findingId}/apply": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Apply a finding's fix suggestion to the draft */ + post: operations["applyFindingFix_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/authoring/runs/{runId}/cancel": { parameters: { query?: never; @@ -2504,6 +2572,40 @@ export interface paths { patch?: never; trace?: never; }; + "/api/web/authoring/drafts/{draftId}/submit": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Submit a validated draft into the publish pipeline */ + post: operations["submitValidatedDraft"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/api/v1/authoring/drafts/{draftId}/submit": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** Submit a validated draft into the publish pipeline */ + post: operations["submitValidatedDraft_1"]; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/authoring/drafts/{draftId}/runs": { parameters: { query?: never; @@ -7199,6 +7301,43 @@ export interface components { createdAt?: string; readAt?: string; }; + ApiResponseValidationFindingResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["ValidationFindingResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + FilePatch: { + filePath?: string; + oldSha256?: string; + oldValue?: string; + newValue?: string; + }; + FixSuggestion: { + patches?: components["schemas"]["FilePatch"][]; + description?: string; + }; + ValidationFindingResponse: { + /** Format: int64 */ + id?: number; + /** Format: int64 */ + runId?: number; + layer?: string; + ruleCode?: string; + severity?: string; + filePath?: string; + location?: string; + message?: string; + suggestion?: components["schemas"]["FixSuggestion"]; + status?: string; + /** Format: int32 */ + appliedRevision?: number; + /** Format: date-time */ + createdAt?: string; + }; ApiResponseValidationRunResponse: { /** Format: int32 */ code?: number; @@ -7234,6 +7373,27 @@ export interface components { [key: string]: Record; }; }; + SubmitDraftRequest: { + visibility?: string; + platformRoles?: string[]; + }; + ApiResponseSubmitDraftResponse: { + /** Format: int32 */ + code?: number; + msg?: string; + data?: components["schemas"]["SubmitDraftResponse"]; + /** Format: date-time */ + timestamp?: string; + requestId?: string; + }; + SubmitDraftResponse: { + /** Format: int64 */ + skillId?: number; + /** Format: int64 */ + versionId?: number; + slug?: string; + version?: string; + }; CreateDraftRequest: { namespaceSlug: string; name: string; @@ -8763,34 +8923,6 @@ export interface components { timestamp?: string; requestId?: string; }; - FilePatch: { - filePath?: string; - oldSha256?: string; - oldValue?: string; - newValue?: string; - }; - FixSuggestion: { - patches?: components["schemas"]["FilePatch"][]; - description?: string; - }; - ValidationFindingResponse: { - /** Format: int64 */ - id?: number; - /** Format: int64 */ - runId?: number; - layer?: string; - ruleCode?: string; - severity?: string; - filePath?: string; - location?: string; - message?: string; - suggestion?: components["schemas"]["FixSuggestion"]; - status?: string; - /** Format: int32 */ - appliedRevision?: number; - /** Format: date-time */ - createdAt?: string; - }; SseEmitter: { /** Format: int64 */ timeout?: number; @@ -14564,6 +14696,98 @@ export interface operations { }; }; }; + dismissFindingFix: { + parameters: { + query?: never; + header?: never; + path: { + runId: number; + findingId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseValidationFindingResponse"]; + }; + }; + }; + }; + dismissFindingFix_1: { + parameters: { + query?: never; + header?: never; + path: { + runId: number; + findingId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseValidationFindingResponse"]; + }; + }; + }; + }; + applyFindingFix: { + parameters: { + query?: never; + header?: never; + path: { + runId: number; + findingId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseValidationFindingResponse"]; + }; + }; + }; + }; + applyFindingFix_1: { + parameters: { + query?: never; + header?: never; + path: { + runId: number; + findingId: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseValidationFindingResponse"]; + }; + }; + }; + }; cancelValidationRun: { parameters: { query?: never; @@ -14608,6 +14832,58 @@ export interface operations { }; }; }; + submitValidatedDraft: { + parameters: { + query?: never; + header?: never; + path: { + draftId: number; + }; + cookie?: never; + }; + requestBody?: { + content: { + "application/json": components["schemas"]["SubmitDraftRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSubmitDraftResponse"]; + }; + }; + }; + }; + submitValidatedDraft_1: { + parameters: { + query?: never; + header?: never; + path: { + draftId: number; + }; + cookie?: never; + }; + requestBody?: { + content: { + "application/json": components["schemas"]["SubmitDraftRequest"]; + }; + }; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "*/*": components["schemas"]["ApiResponseSubmitDraftResponse"]; + }; + }; + }; + }; listValidationRuns: { parameters: { query?: never;