diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowController.java b/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowController.java index 0e41e917..8762d710 100644 --- a/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowController.java +++ b/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowController.java @@ -14,8 +14,10 @@ import vip.mate.workflow.compiler.WorkflowCompileFailedException; import vip.mate.workflow.compiler.WorkflowCompiler; import vip.mate.workflow.model.WorkflowEntity; import vip.mate.workflow.model.WorkflowRunEntity; +import vip.mate.workflow.model.WorkflowRunPauseEntity; import vip.mate.workflow.model.WorkflowRunStepEntity; import vip.mate.workflow.repository.WorkflowRunMapper; +import vip.mate.workflow.repository.WorkflowRunPauseMapper; import vip.mate.workflow.repository.WorkflowRunStepMapper; import vip.mate.workflow.service.WorkflowService; @@ -36,50 +38,63 @@ public class WorkflowController { private final WorkflowService workflowService; private final WorkflowRunMapper runMapper; private final WorkflowRunStepMapper stepMapper; + private final WorkflowRunPauseMapper pauseMapper; private final WorkflowCompiler compiler; private final WorkflowAclPort aclPort; @Operation(summary = "List workflows in the workspace") @GetMapping - public R> list(@RequestParam("workspaceId") long workspaceId) { + public R> list(@RequestHeader("X-Workspace-Id") long workspaceId) { return R.ok(workflowService.listByWorkspace(workspaceId)); } @Operation(summary = "Get a workflow by id (includes inline draft).") @GetMapping("/{id}") - public R get(@PathVariable long id) { - WorkflowEntity row = workflowService.get(id); + public R get(@PathVariable long id, + @RequestHeader("X-Workspace-Id") long workspaceId) { + WorkflowEntity row = workflowService.get(id, workspaceId); if (row == null) return R.fail("workflow not found: " + id); return R.ok(row); } @Operation(summary = "Create a workflow row (draft starts empty).") @PostMapping - public R create(@RequestBody WorkflowEntity workflow) { + public R create(@RequestBody WorkflowEntity workflow, + @RequestHeader("X-Workspace-Id") long workspaceId) { + // Force the workspace from the trusted header — the request body + // can't choose a workspace for the new row, otherwise a caller + // could plant rows into another tenant. + workflow.setWorkspaceId(workspaceId); return R.ok(workflowService.create(workflow)); } @Operation(summary = "Update workflow metadata (name / description / enabled).") @PutMapping("/{id}") public R update(@PathVariable long id, - @RequestBody WorkflowEntity workflow) { - workflow.setId(id); - return R.ok(workflowService.update(workflow)); + @RequestBody WorkflowMetadataRequest body, + @RequestHeader("X-Workspace-Id") long workspaceId) { + return R.ok(workflowService.updateMetadata(id, workspaceId, + body.name(), body.description(), body.enabled())); } @Operation(summary = "Save the inline draft graph_json without compiling.") @PutMapping("/{id}/draft") public R saveDraft(@PathVariable long id, @RequestBody WorkflowDraftRequest body, - @RequestParam(value = "userId", required = false) Long userId) { - return R.ok(workflowService.saveDraft(id, body.draftJson(), userId)); + @RequestParam(value = "userId", required = false) Long userId, + @RequestHeader("X-Workspace-Id") long workspaceId) { + return R.ok(workflowService.saveDraft(id, workspaceId, body.draftJson(), userId)); } @Operation(summary = "Compile the draft and surface diagnostics without persisting a revision.") @PostMapping("/{id}/compile") - public ResponseEntity compileDraft(@PathVariable long id) { - WorkflowEntity row = workflowService.get(id); - if (row == null || row.getDraftJson() == null) { + public ResponseEntity compileDraft(@PathVariable long id, + @RequestHeader("X-Workspace-Id") long workspaceId) { + WorkflowEntity row = workflowService.get(id, workspaceId); + if (row == null) { + return ResponseEntity.badRequest().body(R.fail("workflow not found: " + id)); + } + if (row.getDraftJson() == null) { return ResponseEntity.badRequest() .body(R.fail("workflow has no draft to compile: " + id)); } @@ -96,9 +111,10 @@ public class WorkflowController { @PostMapping("/{id}/publish") public ResponseEntity publish(@PathVariable long id, @RequestBody(required = false) WorkflowPublishRequest body, - @RequestParam(value = "userId", required = false) Long userId) { + @RequestParam(value = "userId", required = false) Long userId, + @RequestHeader("X-Workspace-Id") long workspaceId) { try { - WorkflowService.PublishOutcome outcome = workflowService.publish(id, userId, + WorkflowService.PublishOutcome outcome = workflowService.publish(id, workspaceId, userId, body == null ? null : body.note()); return ResponseEntity.ok(R.ok(outcome)); } catch (WorkflowCompileFailedException e) { @@ -110,36 +126,97 @@ public class WorkflowController { @Operation(summary = "Soft-delete a workflow row.") @DeleteMapping("/{id}") - public R delete(@PathVariable long id) { - workflowService.delete(id); + public R delete(@PathVariable long id, + @RequestHeader("X-Workspace-Id") long workspaceId) { + workflowService.delete(id, workspaceId); return R.ok(); } @Operation(summary = "List the most recent runs for a workflow.") @GetMapping("/{id}/runs") public R> listRuns(@PathVariable long id, - @RequestParam(value = "limit", defaultValue = "50") int limit) { + @RequestParam(value = "limit", defaultValue = "50") int limit, + @RequestHeader("X-Workspace-Id") long workspaceId) { + // Verify the parent workflow belongs to the caller's workspace + // before listing run rows, otherwise a caller could enumerate + // every workspace's runs by guessing workflow ids. + if (workflowService.get(id, workspaceId) == null) { + return R.fail("workflow not found: " + id); + } int capped = Math.min(Math.max(limit, 1), 200); List rows = runMapper.selectList(new LambdaQueryWrapper() .eq(WorkflowRunEntity::getWorkflowId, id) + .eq(WorkflowRunEntity::getWorkspaceId, workspaceId) .orderByDesc(WorkflowRunEntity::getStartedAt) .last("LIMIT " + capped)); return R.ok(rows); } + @Operation(summary = "List paused runs across the workspace so operators can resume them.") + @GetMapping("/runs/paused") + public R> listPausedRuns(@RequestParam(value = "limit", defaultValue = "50") int limit, + @RequestHeader("X-Workspace-Id") long workspaceId) { + // Without this listing surface, an await_approval pause is only + // recoverable by a caller that already happens to know the runId + // and pauseToken — i.e. orphaned for any human operator. The + // shape is small (run + active pause token) because operator UIs + // primarily need to know "which runs are blocked, and how do I + // resume them". + int capped = Math.min(Math.max(limit, 1), 200); + List paused = runMapper.selectList(new LambdaQueryWrapper() + .eq(WorkflowRunEntity::getWorkspaceId, workspaceId) + .eq(WorkflowRunEntity::getState, "paused") + .orderByDesc(WorkflowRunEntity::getStartedAt) + .last("LIMIT " + capped)); + if (paused.isEmpty()) return R.ok(List.of()); + List out = new java.util.ArrayList<>(paused.size()); + for (WorkflowRunEntity run : paused) { + WorkflowRunPauseEntity pause = pauseMapper.selectOne( + new LambdaQueryWrapper() + .eq(WorkflowRunPauseEntity::getRunId, run.getId()) + .isNull(WorkflowRunPauseEntity::getResumedAt) + .orderByDesc(WorkflowRunPauseEntity::getPausedAt) + .last("LIMIT 1")); + out.add(new PausedRunSummary(run, pause)); + } + return R.ok(out); + } + @Operation(summary = "Inspect a single run with its step rows for replay / debugging.") @GetMapping("/runs/{runId}") - public R getRun(@PathVariable long runId) { + public R getRun(@PathVariable long runId, + @RequestHeader("X-Workspace-Id") long workspaceId) { WorkflowRunEntity run = runMapper.selectById(runId); if (run == null) return R.fail("run not found: " + runId); + if (run.getWorkspaceId() == null || run.getWorkspaceId() != workspaceId) { + // Same surface as "not found" — don't leak run id existence + // to non-owning workspaces. + return R.fail("run not found: " + runId); + } List steps = stepMapper.selectList(new LambdaQueryWrapper() .eq(WorkflowRunStepEntity::getRunId, runId) .orderByAsc(WorkflowRunStepEntity::getStepIndex) .orderByAsc(WorkflowRunStepEntity::getIterationIndex)); - return R.ok(new RunDetail(run, steps)); + // Include the most recent unresolved pause so the caller can wire + // a "resume" button without a second roundtrip. + WorkflowRunPauseEntity activePause = pauseMapper.selectOne( + new LambdaQueryWrapper() + .eq(WorkflowRunPauseEntity::getRunId, runId) + .isNull(WorkflowRunPauseEntity::getResumedAt) + .orderByDesc(WorkflowRunPauseEntity::getPausedAt) + .last("LIMIT 1")); + return R.ok(new RunDetail(run, steps, activePause)); } - public record RunDetail(WorkflowRunEntity run, List steps) {} + /** Narrow patch shape for {@link #update}; keeps the metadata path + * from accepting fields that would clobber the draft. */ + public record WorkflowMetadataRequest(String name, String description, Boolean enabled) {} + + public record RunDetail(WorkflowRunEntity run, + List steps, + WorkflowRunPauseEntity activePause) {} + + public record PausedRunSummary(WorkflowRunEntity run, WorkflowRunPauseEntity pause) {} private static R buildCompileFailure(List errors) { R r = new R<>(); diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowResumeController.java b/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowResumeController.java new file mode 100644 index 00000000..8289ad5e --- /dev/null +++ b/mateclaw-server/src/main/java/vip/mate/workflow/api/WorkflowResumeController.java @@ -0,0 +1,135 @@ +package vip.mate.workflow.api; + +import io.swagger.v3.oas.annotations.Operation; +import io.swagger.v3.oas.annotations.tags.Tag; +import lombok.RequiredArgsConstructor; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.*; +import vip.mate.common.result.R; +import vip.mate.workflow.compiler.PublishContext; +import vip.mate.workflow.compiler.WorkflowAclPort; +import vip.mate.workflow.compiler.WorkflowCompiler; +import vip.mate.workflow.model.WorkflowEntity; +import vip.mate.workflow.model.WorkflowRevisionEntity; +import vip.mate.workflow.model.WorkflowRunEntity; +import vip.mate.workflow.model.WorkflowRunPauseEntity; +import vip.mate.workflow.repository.WorkflowRevisionMapper; +import vip.mate.workflow.repository.WorkflowRunMapper; +import vip.mate.workflow.repository.WorkflowRunPauseMapper; +import vip.mate.workflow.runtime.WorkflowResumer; +import vip.mate.workflow.service.WorkflowService; + +import java.nio.charset.StandardCharsets; + +/** + * HTTP surface for resuming an {@code await_approval} pause. + * + *

The pause itself is opened by {@code AwaitApprovalStepAdapter} when a + * step transitions to PAUSED; this controller is what advances the run + * once a human (operator UI / approval webhook / timeout sweeper) + * decides the outcome. Without a public endpoint here, every paused run + * would be stuck until someone called {@code WorkflowResumer} from + * inside the JVM — exactly the gap the design called out as + * "v0 functionally broken". + * + *

v0 supports the operator-driven path: an authorised user in the + * owning workspace POSTs the pauseToken and an outcome. v1 will add the + * webhook callback that {@code ApprovalWorkflowService.requestWorkflowApproval} + * fires once the platform has a real workflow-approval pending row. + */ +@Tag(name = "工作流恢复") +@RestController +@RequestMapping("/api/v1/workflows/runs") +@RequiredArgsConstructor +public class WorkflowResumeController { + + private final WorkflowResumer resumer; + private final WorkflowRunMapper runMapper; + private final WorkflowRunPauseMapper pauseMapper; + private final WorkflowRevisionMapper revisionMapper; + private final WorkflowService workflowService; + private final WorkflowCompiler compiler; + private final WorkflowAclPort aclPort; + + @Operation(summary = "Resume a paused workflow run with the given outcome.") + @PostMapping("/{runId}/resume") + public ResponseEntity resume(@PathVariable long runId, + @RequestBody ResumeRequest body, + @RequestHeader("X-Workspace-Id") long workspaceId) { + if (body == null || body.pauseToken() == null || body.pauseToken().isBlank()) { + return ResponseEntity.badRequest().body(R.fail("pauseToken is required")); + } + WorkflowResumer.ResumeOutcome outcome = parseOutcome(body.outcome()); + if (outcome == null) { + return ResponseEntity.badRequest() + .body(R.fail("outcome must be one of: approved / rejected / timeout / cancelled")); + } + + WorkflowRunEntity run = runMapper.selectById(runId); + if (run == null + || run.getWorkspaceId() == null + || run.getWorkspaceId() != workspaceId) { + // Same surface as "not found" so tenants can't probe foreign run ids. + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(R.fail("run not found: " + runId)); + } + + // Validate the pause token belongs to this run before doing anything. + // Without this, a token leaked from one workspace could resume a run + // in another workspace just because the resumer doesn't itself check + // the workspace-vs-token coupling. + WorkflowRunPauseEntity pause = pauseMapper.selectOne( + new com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper() + .eq(WorkflowRunPauseEntity::getPauseToken, body.pauseToken()) + .last("LIMIT 1")); + if (pause == null || pause.getRunId() == null || pause.getRunId() != runId) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(R.fail("pause not found for run " + runId)); + } + + // Re-compile the locked revision to materialize a graph the resumer + // can walk. Compile errors here would mean a published revision is + // unparseable — should never happen in practice but we surface 500 + // explicitly rather than crashing inside the resumer. + WorkflowEntity workflow = workflowService.get(run.getWorkflowId(), workspaceId); + if (workflow == null) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(R.fail("workflow not found: " + run.getWorkflowId())); + } + WorkflowRevisionEntity revision = revisionMapper.selectById(run.getRevisionId()); + if (revision == null) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(R.fail("revision " + run.getRevisionId() + " missing for run " + runId)); + } + WorkflowCompiler.Result compiled = compiler.compile(revision.getGraphJson(), + new PublishContext(0L, run.getWorkspaceId()), aclPort); + if (!compiled.ok()) { + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) + .body(R.fail("revision graph failed to recompile on resume")); + } + + byte[] payload = (body.payload() == null || body.payload().isEmpty()) + ? null + : body.payload().getBytes(StandardCharsets.UTF_8); + + WorkflowResumer.Outcome result = resumer.resume(compiled.graph(), body.pauseToken(), outcome, payload); + return ResponseEntity.ok(R.ok(new ResumeResponse(result.kind().name(), + result.runId(), result.errorMessage()))); + } + + private static WorkflowResumer.ResumeOutcome parseOutcome(String token) { + if (token == null) return null; + String t = token.trim().toLowerCase(); + return switch (t) { + case "approved" -> WorkflowResumer.ResumeOutcome.APPROVED; + case "rejected" -> WorkflowResumer.ResumeOutcome.REJECTED; + case "timeout" -> WorkflowResumer.ResumeOutcome.TIMEOUT; + case "cancelled" -> WorkflowResumer.ResumeOutcome.CANCELLED; + default -> null; + }; + } + + public record ResumeRequest(String pauseToken, String outcome, String payload) {} + public record ResumeResponse(String kind, Long runId, String errorMessage) {} +} diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/repository/WorkflowMapper.java b/mateclaw-server/src/main/java/vip/mate/workflow/repository/WorkflowMapper.java index c4c6b662..2d90ddc9 100644 --- a/mateclaw-server/src/main/java/vip/mate/workflow/repository/WorkflowMapper.java +++ b/mateclaw-server/src/main/java/vip/mate/workflow/repository/WorkflowMapper.java @@ -2,8 +2,22 @@ package vip.mate.workflow.repository; import com.baomidou.mybatisplus.core.mapper.BaseMapper; import org.apache.ibatis.annotations.Mapper; +import org.apache.ibatis.annotations.Param; +import org.apache.ibatis.annotations.Select; import vip.mate.workflow.model.WorkflowEntity; @Mapper public interface WorkflowMapper extends BaseMapper { + + /** + * Row-locking lookup used by the publish path. Two concurrent publishes + * for the same workflow would otherwise both compute the same + * {@code max(revision)+1} and the second would crash on the + * {@code uk_workflow_revision} unique constraint, leaving + * {@code latest_revision_id} pointing at the first while the second + * caller saw a 500. Locking the workflow row in a single transaction + * serializes the two publishes cleanly. + */ + @Select("SELECT * FROM mate_workflow WHERE id = #{id} AND deleted = 0 FOR UPDATE") + WorkflowEntity selectByIdForUpdate(@Param("id") long id); } diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/runtime/mode/AwaitApprovalStepAdapter.java b/mateclaw-server/src/main/java/vip/mate/workflow/runtime/mode/AwaitApprovalStepAdapter.java index 8e236855..4a5d9e16 100644 --- a/mateclaw-server/src/main/java/vip/mate/workflow/runtime/mode/AwaitApprovalStepAdapter.java +++ b/mateclaw-server/src/main/java/vip/mate/workflow/runtime/mode/AwaitApprovalStepAdapter.java @@ -19,15 +19,28 @@ import java.util.UUID; * {@code await_approval} — pauses the run pending an external approval * decision. Inserts a {@code mate_workflow_run_pause} row keyed by a fresh * {@code pauseToken}, then returns {@link StepResult.State#PAUSED} so the - * runner can short-circuit and mark the run row {@code paused}. Resume is - * orchestrated by {@code WorkflowResumer} once the external approval (or - * timeout) lands. + * runner can short-circuit and mark the run row {@code paused}. * - *

v0 stores no extra approval metadata — the workflow's {@code mate_tool_approval} - * link will be wired in once the approval-driven resume callback exists. - * The pause row's {@code resume_deadline} is honoured when the step + *

Resolution path (v0): + *

    + *
  1. Operator UI lists paused runs via {@code GET /api/v1/workflows/runs/paused}, + * which returns the run + the active pause record (including the + * {@code pauseToken}).
    2. + *
  2. Operator picks an outcome and POSTs to + * {@code /api/v1/workflows/runs/{runId}/resume} with the + * {@code pauseToken} and {@code outcome ∈ {approved, rejected, timeout, cancelled}}.
    3. + *
  3. {@code WorkflowResumer} marks the pause row resolved and advances + * the run state machine.
    4. + *
+ * + *

The pause row's {@code resume_deadline} is honoured when the step * declares a {@code timeoutSecs}; otherwise it stays {@code null} and the * resumer treats the pause as open-ended. + * + *

The {@code external_approval_id} column on the pause row is reserved + * for a future integration that bridges workflow pauses into the + * tool-approval inbox; v0 leaves it null and routes resolution through the + * pause-token path above. */ @Component public class AwaitApprovalStepAdapter implements StepAdapter { diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/service/DefaultWorkflowAclPort.java b/mateclaw-server/src/main/java/vip/mate/workflow/service/DefaultWorkflowAclPort.java index 41fc6803..e1cc97de 100644 --- a/mateclaw-server/src/main/java/vip/mate/workflow/service/DefaultWorkflowAclPort.java +++ b/mateclaw-server/src/main/java/vip/mate/workflow/service/DefaultWorkflowAclPort.java @@ -30,7 +30,14 @@ public class DefaultWorkflowAclPort implements WorkflowAclPort { @Override public boolean agentExists(long workspaceId, String agentName) { if (agentName == null || agentName.isBlank()) return false; + // Workspace-scoped lookup. Without this clause a workflow in + // workspace A could reference an agent that lives in workspace B, + // which would silently bypass the per-workspace ACL the rest of + // the platform enforces. Reject cross-workspace agent references + // at publish time so the failure is visible to authors instead of + // surfacing as a runtime "agent not found". Long count = agentMapper.selectCount(new LambdaQueryWrapper() + .eq(AgentEntity::getWorkspaceId, workspaceId) .eq(AgentEntity::getName, agentName.trim()) .eq(AgentEntity::getEnabled, true)); return count != null && count > 0; @@ -39,13 +46,19 @@ public class DefaultWorkflowAclPort implements WorkflowAclPort { @Override public boolean agentIdExists(long workspaceId, long agentId) { AgentEntity row = agentMapper.selectById(agentId); - return row != null && Boolean.TRUE.equals(row.getEnabled()); + return row != null + && Boolean.TRUE.equals(row.getEnabled()) + && row.getWorkspaceId() != null + && row.getWorkspaceId() == workspaceId; } @Override public boolean channelAllowed(long workspaceId, String channelName) { if (channelName == null || channelName.isBlank()) return false; + // Same workspace constraint as above: a channel adapter enabled in + // another workspace should not satisfy this workflow's allowlist. Long count = channelMapper.selectCount(new LambdaQueryWrapper() + .eq(ChannelEntity::getWorkspaceId, workspaceId) .eq(ChannelEntity::getChannelType, channelName.trim()) .eq(ChannelEntity::getEnabled, true)); return count != null && count > 0; diff --git a/mateclaw-server/src/main/java/vip/mate/workflow/service/WorkflowService.java b/mateclaw-server/src/main/java/vip/mate/workflow/service/WorkflowService.java index 2a100778..bbc51b10 100644 --- a/mateclaw-server/src/main/java/vip/mate/workflow/service/WorkflowService.java +++ b/mateclaw-server/src/main/java/vip/mate/workflow/service/WorkflowService.java @@ -37,8 +37,30 @@ public class WorkflowService { .orderByDesc(WorkflowEntity::getUpdateTime)); } - public WorkflowEntity get(long id) { - return workflowMapper.selectById(id); + /** + * Workspace-scoped lookup. All read paths that take a raw {@code id} + * must use this so callers can't fetch a row from another tenant just + * by guessing a numeric id. Returns {@code null} when the row exists + * but lives in a different workspace (treated as "not found" so the + * caller doesn't get a side-channel signal that the id is real). + */ + public WorkflowEntity get(long id, long workspaceId) { + WorkflowEntity row = workflowMapper.selectById(id); + if (row == null) return null; + if (row.getWorkspaceId() == null || row.getWorkspaceId() != workspaceId) return null; + return row; + } + + /** + * Same as {@link #get(long, long)} but throws when the row is missing. + * Used by mutation paths that can fail loudly instead of returning null. + */ + private WorkflowEntity getOrThrow(long id, long workspaceId) { + WorkflowEntity row = get(id, workspaceId); + if (row == null) { + throw new IllegalArgumentException("workflow not found: " + id); + } + return row; } @Transactional @@ -48,22 +70,30 @@ public class WorkflowService { return workflow; } + /** + * Update workflow metadata (name / description / enabled). The patch + * shape is deliberately narrow: the caller cannot replace + * {@code draftJson}, {@code latest_revision_id}, or {@code workspace_id} + * through this path. Without that narrowing, a metadata-only save from + * the UI would clobber the draft because the request body wouldn't + * carry it. + */ @Transactional - public WorkflowEntity update(WorkflowEntity workflow) { - WorkflowEntity existing = workflowMapper.selectById(workflow.getId()); - if (existing == null) { - throw new IllegalArgumentException("workflow not found: " + workflow.getId()); - } - // Preserve revision pointer — only the publish path moves it. - workflow.setLatestRevisionId(existing.getLatestRevisionId()); - workflowMapper.updateById(workflow); - return workflow; + public WorkflowEntity updateMetadata(long id, long workspaceId, String name, + String description, Boolean enabled) { + WorkflowEntity existing = getOrThrow(id, workspaceId); + if (name != null) existing.setName(name); + if (description != null) existing.setDescription(description); + if (enabled != null) existing.setEnabled(enabled); + // draftJson / latest_revision_id / workspace_id are intentionally + // left untouched here — those move only through saveDraft / publish. + workflowMapper.updateById(existing); + return existing; } @Transactional - public WorkflowEntity saveDraft(long id, String draftJson, Long updatedBy) { - WorkflowEntity row = workflowMapper.selectById(id); - if (row == null) throw new IllegalArgumentException("workflow not found: " + id); + public WorkflowEntity saveDraft(long id, long workspaceId, String draftJson, Long updatedBy) { + WorkflowEntity row = getOrThrow(id, workspaceId); row.setDraftJson(draftJson); row.setDraftUpdatedAt(LocalDateTime.now()); row.setDraftUpdatedBy(updatedBy); @@ -72,7 +102,8 @@ public class WorkflowService { } @Transactional - public void delete(long id) { + public void delete(long id, long workspaceId) { + getOrThrow(id, workspaceId); workflowMapper.deleteById(id); } @@ -83,11 +114,21 @@ public class WorkflowService { * the compiler reports any errors. */ @Transactional - public PublishOutcome publish(long workflowId, Long publisherId, String publishedNote) { - WorkflowEntity workflow = workflowMapper.selectById(workflowId); + public PublishOutcome publish(long workflowId, long workspaceId, Long publisherId, String publishedNote) { + // Row-lock the workflow for the entire publish transaction so two + // concurrent publishes serialize on the same monotonic next revision + // — without this, both compute max+1 and the second one trips + // uk_workflow_revision while leaving the latest_revision_id pointer + // ambiguous. + WorkflowEntity workflow = workflowMapper.selectByIdForUpdate(workflowId); if (workflow == null) { throw new IllegalArgumentException("workflow not found: " + workflowId); } + if (workflow.getWorkspaceId() == null || workflow.getWorkspaceId() != workspaceId) { + // Cross-workspace publish attempt — same surface as "not found" + // so the caller can't probe id existence by error message. + throw new IllegalArgumentException("workflow not found: " + workflowId); + } String draft = workflow.getDraftJson(); if (draft == null || draft.isBlank()) { throw new IllegalStateException("cannot publish workflow " + workflowId