From 7ea5b6229e2d6ac165dab14668a867f7dcfe01d5 Mon Sep 17 00:00:00 2001 From: Aleksandr Suvorov Date: Sun, 19 Apr 2026 01:53:22 +0400 Subject: [PATCH 1/3] feat(sub-workflow): hierarchical workflow composition via SubWorkflowNode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces the Sub-workflow implementation, enabling nested workflow execution with tenant-aware propagation and push-time graph validation. Core & Execution: - Added SubWorkflowNodeExecutor with a MAX_DEPTH = 16 safety cap. - Implemented SubWorkflowGraphValidator for cycle detection and dangling reference checks (single-pass DFS). - Added _tenant_id and context propagation for nested execution. DSL & CLI: - Introduced subWorkflow(id) { } builder with imports() and writes() mappings. - Added --with flags on run, validate, and visualize for local sub-workflow seeding; build does not accept --with — sub-workflows push independently. Server & Persistence: - Added WorkflowPushLock using pg_advisory_xact_lock for cluster-wide concurrency control during workflow updates. - Integrated push-time validation in WorkflowRegistryService. - Refactored server services into specialized domain modules (Registry, Execution, Query, State). Includes a comprehensive update to all module READMEs and developer guides to reflect the new architecture and package structures. Resolve: #64 Signed-off-by: Aleksandr Suvorov --- .claude/rules/00-master.md | 1 - .claude/rules/10-java-standards.md | 13 - .cursor/rules/00-master.mdc | 1 - .cursor/rules/10-java-standards.mdc | 13 - AGENTS.md | 13 +- docs/developer-guide-core.md | 37 ++ docs/developer-guide-server.md | 78 ++- docs/dsl-reference.md | 75 ++- docs/unified-architecture.md | 29 +- hensu-cli/README.md | 42 +- .../cli/commands/WorkflowBuildCommand.java | 10 + .../hensu/cli/commands/WorkflowCommand.java | 40 +- .../cli/commands/WorkflowRunCommand.java | 32 +- .../cli/commands/WorkflowValidateCommand.java | 10 +- .../commands/WorkflowVisualizeCommand.java | 11 +- .../java/io/hensu/cli/daemon/DaemonFrame.java | 9 + .../io/hensu/cli/daemon/DaemonServer.java | 21 + .../producers/HensuEnvironmentProducer.java | 18 + .../hensu/cli/workflow/SubWorkflowLoader.java | 282 +++++++++ .../cli/commands/BaseWorkflowCommandTest.java | 14 + .../commands/WorkflowBuildCommandTest.java | 1 + .../cli/commands/WorkflowRunCommandTest.java | 1 + .../commands/WorkflowValidateCommandTest.java | 1 + .../WorkflowVisualizeCommandTest.java | 1 + .../cli/workflow/SubWorkflowLoaderTest.java | 231 +++++++ hensu-core/README.md | 6 +- .../executor/SubWorkflowNodeExecutor.java | 61 +- .../core/workflow/node/SubWorkflowNode.java | 38 +- .../validation/SubWorkflowGraphValidator.java | 164 +++++ .../validation/WorkflowValidator.java | 37 +- .../WorkflowExecutorSubWorkflowTest.java | 403 ++++++++++++ .../SubWorkflowGraphValidatorTest.java | 184 ++++++ .../WorkflowValidatorSubWorkflowTest.java | 90 +++ hensu-dsl/README.md | 3 +- .../io/hensu/dsl/builders/BaseNodeBuilder.kt | 2 +- .../io/hensu/dsl/builders/GraphBuilder.kt | 29 + .../dsl/builders/SubWorkflowNodeBuilder.kt | 115 ++++ .../builders/SubWorkflowNodeBuilderTest.kt | 72 +++ .../hensu/serialization/NodeDeserializer.java | 1 + .../hensu/serialization/NodeSerializer.java | 1 + hensu-server/README.md | 74 ++- .../hensu/server/api/ExecutionResource.java | 9 +- .../io/hensu/server/api/WorkflowResource.java | 47 +- .../persistence/ExecutionLeaseManager.java | 72 +++ .../server/persistence/WorkflowPushLock.java | 107 ++++ .../workflow/ExecutionNotFoundException.java | 12 + .../server/workflow/ExecutionOutput.java | 15 + .../workflow/ExecutionQueryService.java | 113 ++++ .../server/workflow/ExecutionStartResult.java | 7 + .../workflow/ExecutionStateService.java | 143 +++++ .../server/workflow/ExecutionStatus.java | 15 + .../server/workflow/ExecutionSummary.java | 12 + .../io/hensu/server/workflow/PlanInfo.java | 8 + .../hensu/server/workflow/ResumeDecision.java | 18 + .../workflow/WorkflowExecutionException.java | 12 + .../workflow/WorkflowExecutionService.java | 242 ++++++++ .../workflow/WorkflowNotFoundException.java | 12 + .../server/workflow/WorkflowRecoveryJob.java | 27 +- .../workflow/WorkflowRegistryService.java | 119 ++++ .../server/workflow/WorkflowService.java | 585 +----------------- .../server/api/ExecutionResourceTest.java | 14 +- .../server/api/WorkflowResourceTest.java | 108 ++-- .../ApprovalRoutingIntegrationTest.java | 2 +- .../integration/IntegrationTestBase.java | 2 +- .../integration/McpActionIntegrationTest.java | 2 +- .../ParallelNodeIntegrationTest.java | 2 +- .../PlanExecutionIntegrationTest.java | 2 +- .../ReviewAndBacktrackIntegrationTest.java | 2 +- .../RubricEvaluationIntegrationTest.java | 17 +- .../SpecializedNodeIntegrationTest.java | 2 +- .../StandardNodeIntegrationTest.java | 2 +- .../StatePersistenceIntegrationTest.java | 4 +- .../SubWorkflowIntegrationTest.java | 2 +- .../WorkflowLifecycleIntegrationTest.java | 4 +- .../WorkflowRegistryIntegrationTest.java | 59 ++ .../workflow/ExecutionQueryServiceTest.java | 184 ++++++ .../workflow/ExecutionStateServiceTest.java | 220 +++++++ .../server/workflow/WorkflowServiceTest.java | 329 ---------- working-dir/workflows/main-research.kt | 78 +++ working-dir/workflows/sub-summarizer.kt | 46 ++ 80 files changed, 3824 insertions(+), 1096 deletions(-) create mode 100644 hensu-cli/src/main/java/io/hensu/cli/workflow/SubWorkflowLoader.java create mode 100644 hensu-cli/src/test/java/io/hensu/cli/workflow/SubWorkflowLoaderTest.java create mode 100644 hensu-core/src/main/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidator.java create mode 100644 hensu-core/src/test/java/io/hensu/core/execution/WorkflowExecutorSubWorkflowTest.java create mode 100644 hensu-core/src/test/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidatorTest.java create mode 100644 hensu-core/src/test/java/io/hensu/core/workflow/validation/WorkflowValidatorSubWorkflowTest.java create mode 100644 hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilder.kt create mode 100644 hensu-dsl/src/test/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilderTest.kt create mode 100644 hensu-server/src/main/java/io/hensu/server/persistence/WorkflowPushLock.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionNotFoundException.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionOutput.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionQueryService.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStartResult.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStateService.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStatus.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ExecutionSummary.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/PlanInfo.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/ResumeDecision.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionException.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionService.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/WorkflowNotFoundException.java create mode 100644 hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java create mode 100644 hensu-server/src/test/java/io/hensu/server/integration/WorkflowRegistryIntegrationTest.java create mode 100644 hensu-server/src/test/java/io/hensu/server/workflow/ExecutionQueryServiceTest.java create mode 100644 hensu-server/src/test/java/io/hensu/server/workflow/ExecutionStateServiceTest.java delete mode 100644 hensu-server/src/test/java/io/hensu/server/workflow/WorkflowServiceTest.java create mode 100644 working-dir/workflows/main-research.kt create mode 100644 working-dir/workflows/sub-summarizer.kt diff --git a/.claude/rules/00-master.md b/.claude/rules/00-master.md index b8c377e..2a86525 100644 --- a/.claude/rules/00-master.md +++ b/.claude/rules/00-master.md @@ -13,5 +13,4 @@ MANDATORY: Before every session: - Adhere to the "Model Coordination" in `01-model-coordination.md`. - Adhere to the "GraalVM Native Image" in `10-java-standards.md`. - Adhere to the "Hensu Java & Kotlin Standards" in `20-native-safety.md`. -- All diagrams and badges must follow the [Visual Style Guide](../../docs/visual-style-guide.md). - Document all public APIs following the [Javadoc Guide](../../docs/javadoc-guide.md). diff --git a/.claude/rules/10-java-standards.md b/.claude/rules/10-java-standards.md index 0a423c7..170f5a2 100644 --- a/.claude/rules/10-java-standards.md +++ b/.claude/rules/10-java-standards.md @@ -77,19 +77,6 @@ Hensu leverages the latest JVM features to reduce boilerplate and increase type --- -## Native-Image Constraints (The "No-Go" List) - -Since Hensu is optimized for **GraalVM**, you must avoid the following "dynamic" patterns: - -* **No Unregistered Reflection:** Never use `Class.forName()` or `method.invoke()` unless the class is explicitly - registered in Quarkus build-time metadata. -* **No Dynamic Proxies:** Avoid libraries that generate bytecode at runtime (e.g., standard CGLIB or Hibernate - lazy-loading). -* **No Classpath Scanning:** All `AgentProvider` and `NodeExecutor` instances must be wired explicitly via - `HensuFactory.builder()`. - ---- - ## Testing Integrity * **Unit Tests:** Must be pure JVM and use `StubAgentProvider` to avoid API costs and network latency. diff --git a/.cursor/rules/00-master.mdc b/.cursor/rules/00-master.mdc index 333c6d3..7efe22f 100644 --- a/.cursor/rules/00-master.mdc +++ b/.cursor/rules/00-master.mdc @@ -18,5 +18,4 @@ MANDATORY: Before every session: - Adhere to the "Model Coordination" in `01-model-coordination.mdc`. - Adhere to the "GraalVM Native Image" in `10-java-standards.mdc`. - Adhere to the "Hensu Java & Kotlin Standards" in `20-native-safety.mdc`. -- All diagrams and badges must follow the [Visual Style Guide](../../docs/visual-style-guide.md). - Document all public APIs following the [Javadoc Guide](../../docs/javadoc-guide.md). diff --git a/.cursor/rules/10-java-standards.mdc b/.cursor/rules/10-java-standards.mdc index 70ea702..25cbe61 100644 --- a/.cursor/rules/10-java-standards.mdc +++ b/.cursor/rules/10-java-standards.mdc @@ -77,19 +77,6 @@ Hensu leverages the latest JVM features to reduce boilerplate and increase type --- -## Native-Image Constraints (The "No-Go" List) - -Since Hensu is optimized for **GraalVM**, you must avoid the following "dynamic" patterns: - -* **No Unregistered Reflection:** Never use `Class.forName()` or `method.invoke()` unless the class is explicitly - registered in Quarkus build-time metadata. -* **No Dynamic Proxies:** Avoid libraries that generate bytecode at runtime (e.g., standard CGLIB or Hibernate - lazy-loading). -* **No Classpath Scanning:** All `AgentProvider` and `NodeExecutor` instances must be wired explicitly via - `HensuFactory.builder()`. - ---- - ## Testing Integrity * **Unit Tests:** Must be pure JVM and use `StubAgentProvider` to avoid API costs and network latency. diff --git a/AGENTS.md b/AGENTS.md index 797d363..9440a57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -121,6 +121,8 @@ concerns (e.g., adding a new rubric) from the core agent execution logic. - `ExecutionLeaseManager` - Distributed lease manager; generates `server_node_id`, bumps heartbeats, atomically claims stale executions (`@ApplicationScoped`) - `ExecutionHeartbeatJob` - Scheduled heartbeat; runs every `hensu.lease.heartbeat-interval` (default 30s) (`@ApplicationScoped`) - `WorkflowRecoveryJob` - Scheduled sweeper; claims orphaned executions older than `hensu.lease.stale-threshold` (default 90s) and resumes them (`@ApplicationScoped`) +- `WorkflowRegistryService` - Push pipeline: wraps save in `WorkflowPushLock` and invokes `SubWorkflowGraphValidator` lazily resolving sub-workflow ids through the repository; rejects cycles and dangling refs before any row is written (`@ApplicationScoped`) +- `WorkflowPushLock` - Cluster-wide push mutex (`pg_advisory_xact_lock` with JVM `ReentrantLock` fallback) so two concurrent pushes on different nodes cannot together introduce a cycle (`@ApplicationScoped`) **AI Provider Interface**: @@ -151,7 +153,7 @@ Workflow - `GenericNode` - Custom execution logic via registered handlers - `ActionNode` - Execute commands mid-workflow (git, deploy, notify) - `EndNode` - Workflow termination (SUCCESS/FAILURE/CANCELLED) -- `SubWorkflowNode` - Nested workflows +- `SubWorkflowNode` - Delegation to another workflow by id with input/output mapping; cycle + dangling-ref validation at push (`SubWorkflowGraphValidator`); depth cap `MAX_DEPTH = 16`; tenant isolation preserved via `_tenant_id` propagation **Transitions** (`TransitionRule` interface): @@ -229,6 +231,7 @@ the recovery sweeper's scope. - `_tenant_id` — tenant identifier, read by `SubWorkflowNodeExecutor` for loading child workflows - `_execution_id` — ensures `WorkflowExecutor` uses the same ID the service layer tracks +- `_sub_workflow_depth` — recursion depth counter enforced by `SubWorkflowNodeExecutor.MAX_DEPTH = 16` ### Patterns & Conventions @@ -384,6 +387,8 @@ CRUD operations, UPSERT semantics, FK constraints, tenant isolation, and seriali - `hensu-core/.../execution/enricher/YieldsVariableInjector.java` - Injects yield format instructions for branch prompts - `hensu-core/.../workflow/transition/ApprovalTransition.java` - Boolean approval routing via `approved` engine variable - `hensu-core/.../workflow/validation/WorkflowValidator.java` - Load-time validator for `writes` and prompt variable references +- `hensu-core/.../workflow/validation/SubWorkflowGraphValidator.java` - Cycle + dangling-ref detection across the sub-workflow reference graph (single DFS, `globallyVisited`); CLI overload `validate(Collection)` for local cycle-only checks, server overload `validate(Workflow, Function)` for push with lazy repository resolution +- `hensu-core/.../execution/executor/SubWorkflowNodeExecutor.java` - Child workflow execution with `MAX_DEPTH = 16` recursion cap and `_tenant_id` propagation **DSL:** @@ -414,8 +419,10 @@ CRUD operations, UPSERT semantics, FK constraints, tenant isolation, and seriali - `hensu-server/src/main/resources/db/migration/V1__create_persistence_tables.sql` - Flyway schema migration - `hensu-server/src/main/resources/db/migration/V2__add_execution_leases.sql` - Lease columns migration - `hensu-server/.../persistence/ExecutionLeaseManager.java` - Distributed lease manager (heartbeat + atomic claim) -- `hensu-server/.../service/ExecutionHeartbeatJob.java` - Heartbeat emission (@Scheduled) -- `hensu-server/.../service/WorkflowRecoveryJob.java` - Recovery sweeper (@Scheduled) +- `hensu-server/.../persistence/WorkflowPushLock.java` - Cluster-wide push mutex (pg_advisory_xact_lock + JVM fallback) +- `hensu-server/.../workflow/ExecutionHeartbeatJob.java` - Heartbeat emission (@Scheduled) +- `hensu-server/.../workflow/WorkflowRecoveryJob.java` - Recovery sweeper (@Scheduled) +- `hensu-server/.../workflow/WorkflowRegistryService.java` - Push pipeline: `WorkflowPushLock` + `SubWorkflowGraphValidator` **CLI:** diff --git a/docs/developer-guide-core.md b/docs/developer-guide-core.md index 198116d..f1152ea 100644 --- a/docs/developer-guide-core.md +++ b/docs/developer-guide-core.md @@ -15,6 +15,7 @@ This guide covers API usage, adapter development, extension points, and testing - [Agentic Output Validation](#agentic-output-validation) - [Creating Custom Adapters](#creating-custom-adapters) - [Generic Nodes](#generic-nodes) +- [Sub-Workflows](#sub-workflows) - [Action Handlers](#action-handlers) - [Rubric Engine](#rubric-engine) - [Score-Based Routing](#score-based-routing) @@ -486,6 +487,30 @@ private void registerGenericHandlers() { 4. **Return meaningful metadata**: Include relevant info in `NodeResult` metadata map 5. **Handle errors gracefully**: Return `NodeResult.failure()` with clear error messages +## Sub-Workflows + +`SubWorkflowNode` delegates execution to a nested workflow. The parent pauses on the boundary, the child runs to completion, and control returns to the parent with selected outputs mapped back into its state. + +### Context propagation and depth limit + +- `_tenant_id` is copied from parent into child context, preserving multi-tenant isolation across the boundary. +- Nested invocation is capped at depth 16 (`SubWorkflowNodeExecutor.MAX_DEPTH`) via `_sub_workflow_depth`. The executor throws before invoking a child beyond the cap – this is a hard guard against runaway recursion, not a tunable. + +### Input and output mappings + +Data crosses the boundary only through explicit mappings declared on `SubWorkflowNode`: + +| Mapping | Direction | Semantics | +|-----------------|------------------------|------------------------------------------------------------------------------------------------------------------------------| +| `inputMapping` | `childKey → parentKey` | Before the child starts, the executor reads `parentKey` from parent state and writes it as `childKey` in child state. | +| `outputMapping` | `parentKey → childKey` | On successful child completion, the executor reads `childKey` from child state and writes it as `parentKey` in parent state. | + +Anything not covered by these mappings stays on its side of the boundary. There is no implicit state leakage between parent and child. + +### Reference graph validation + +`SubWorkflowGraphValidator` rejects cycles and dangling references at graph-load time, before any node executes. See [SubWorkflowGraphValidator checks](#subworkflowgraphvalidator-checks) under State Schema. + ## Action Handlers Action handlers send data from workflow actions to external systems. Handlers can implement any integration: HTTP calls, messaging (Slack, email), event publishing (Kafka, RabbitMQ), database operations, or custom logic. @@ -729,6 +754,17 @@ WorkflowValidator.validate(workflow); // throws IllegalStateException on violati Validation is a no-op when no schema is declared. Legacy workflows always pass through unchanged. +### `SubWorkflowGraphValidator` checks + +`SubWorkflowGraphValidator` runs at graph-load time over the sub-workflow reference graph. It rejects cycles and – on the server push path – unresolved references, so neither can surface mid-execution. Two overloads serve the two entry points: + +| Overload | Caller | Detects | Unknown targets | +|----------------------------------|------------------|---------------------------------------|------------------------------------------------------------------------------------------------| +| `validate(Collection)` | CLI batch loader | Cycles only | Silently skipped – loader reports missing `--with` declarations separately with richer context | +| `validate(Workflow, Function)` | Server push path | Cycles **and** unknown referenced ids | Aggregated into a single `IllegalStateException` alongside any cycles, in a single DFS pass | + +The `(Workflow, Function)` overload shadows the incoming workflow for its own id so re-push/update sees the post-push graph without an intermediate write. The resolver is queried lazily and only for ids forward-reachable from the root – bounded by a `globallyVisited` set so each id costs at most one repository lookup. + ## Engine Variable Injection Before each agent call, `AgentLifecycleRunner` runs `EngineVariablePromptEnricher` to append @@ -1283,6 +1319,7 @@ Environment variables matching `*_API_KEY`, `*_KEY`, `*_SECRET`, or `*_TOKEN` pa | `workflow/state/StateVariableDeclaration.java` | Single variable declaration record (name, type, isInput) | | `workflow/state/VarType.java` | Variable type enum: STRING, NUMBER, BOOLEAN, LIST_STRING | | `workflow/transition/ApprovalTransition.java` | Boolean approval routing via the `approved` engine variable | +| `workflow/validation/SubWorkflowGraphValidator.java` | Load-time cycle + dangling-reference detector for sub-workflow graphs | | `workflow/validation/WorkflowValidator.java` | Load-time validator for `writes` and prompt `{variable}` references | | `rubric/RubricEngine.java` | Quality evaluation engine | | `rubric/model/Rubric.java` | Rubric definition model | diff --git a/docs/developer-guide-server.md b/docs/developer-guide-server.md index 3aeba0f..6358175 100644 --- a/docs/developer-guide-server.md +++ b/docs/developer-guide-server.md @@ -10,6 +10,7 @@ This guide covers the architecture, patterns, and best practices for developing - [Package Structure](#package-structure) - [Multi-Tenancy](#multi-tenancy) - [REST API Development](#rest-api-development) + - [Sub-Workflow Validation on Push](#sub-workflow-validation-on-push) - [SSE Streaming](#sse-streaming) - [MCP Integration](#mcp-integration) - [Dynamic Tool Discovery](#dynamic-tool-discovery) @@ -296,9 +297,16 @@ io.hensu.server/ │ └── ConstraintViolationExceptionMapper # Global 400 error mapper │ ├── config/ # CDI configuration -│ ├── HensuEnvironmentProducer # HensuFactory → HensuEnvironment -│ ├── ServerBootstrap # Startup registrations -│ └── ServerConfiguration # CDI delegation + server beans +│ ├── HensuEnvironmentProducer # HensuFactory → HensuEnvironment +│ ├── NativeImageConfig # @RegisterForReflection — Hensu domain model +│ ├── LangChain4jNativeConfig # @RegisterForReflection — JDK HTTP transport +│ ├── LangChain4jAnthropicNativeConfig # @RegisterForReflection — Anthropic DTOs +│ ├── LangChain4jGeminiNativeConfig # @RegisterForReflection — Gemini DTOs +│ ├── ServerBootstrap # Startup registrations +│ └── ServerConfiguration # CDI delegation + server beans +│ +├── dev/ # Dev-only handlers (excluded from prod image) +│ └── SleepHandler # Simulates long-running node for crash-recovery tests │ ├── execution/ # Server-side execution listeners │ ├── LoggingExecutionListener # Logs plan/step lifecycle events @@ -308,21 +316,37 @@ io.hensu.server/ │ ├── JsonRpc # JSON-RPC 2.0 message helper │ ├── McpSessionManager # SSE session management │ ├── McpConnection # Connection interface +│ ├── McpConnectionFactory # Factory for per-tenant connections │ ├── McpConnectionPool # Connection pooling +│ ├── McpException # Checked MCP protocol errors │ ├── McpSidecar # ActionHandler for MCP tools -│ └── SseMcpConnection # SSE-based connection impl +│ ├── McpToolDiscovery # Runtime tool schema discovery + cache +│ ├── SseMcpConnection # SSE-based connection impl +│ └── TenantToolRegistry # Merges base + tenant MCP tools (MCP precedence) +│ +├── security/ # JWT + tenant resolution + error mapping +│ ├── GlobalExceptionMapper # Global @Provider — normalizes errors to JSON +│ └── RequestTenantResolver # Extracts tenant_id claim from JWT │ ├── persistence/ # PostgreSQL persistence (plain JDBC) │ ├── JdbcWorkflowRepository # Workflow definitions (JSONB) │ ├── JdbcWorkflowStateRepository # Execution state snapshots (JSONB + lease columns) │ ├── ExecutionLeaseManager # Distributed lease management (@ApplicationScoped) +│ ├── WorkflowPushLock # Cluster-wide push mutex (pg_advisory_xact_lock + JVM fallback) │ ├── JdbcSupport # JDBC helper (queryList, update) │ └── PersistenceException # Unchecked wrapper for SQLException │ -├── service/ # Business logic layer -│ ├── WorkflowService # Workflow operations +├── workflow/ # Business logic layer +│ ├── WorkflowService # Facade over registry + execution + query services +│ ├── WorkflowRegistryService # Push pipeline: WorkflowPushLock + SubWorkflowGraphValidator +│ ├── WorkflowExecutionService # Start/resume orchestration +│ ├── ExecutionQueryService # Read-side: status, plan, output, paused list +│ ├── ExecutionStateService # Snapshot load/save with split-brain guard │ ├── ExecutionHeartbeatJob # Periodic heartbeat emission (@Scheduled) -│ └── WorkflowRecoveryJob # Orphaned execution sweeper (@Scheduled) +│ ├── WorkflowRecoveryJob # Orphaned execution sweeper (@Scheduled) +│ ├── ExecutionStartResult / ExecutionOutput / ExecutionSummary / PlanInfo / ResumeDecision # DTOs +│ ├── ExecutionStatus # Enum: COMPLETED / PAUSED / RUNNING +│ └── {Execution,Workflow}{NotFound,Execution}Exception # Domain exceptions │ ├── streaming/ # Execution event streaming │ ├── ExecutionEvent # Event DTOs (sealed interface) @@ -547,6 +571,46 @@ public Response push(@ValidWorkflow Workflow workflow) { } ``` +#### Sub-Workflow Validation on Push + +`ValidWorkflowValidator` checks the incoming workflow in isolation. Cross-workflow +consistency — cycles and dangling sub-workflow references — is a second pass handled by +`WorkflowRegistryService` before the row is written: + +```java +// WorkflowRegistryService.push() +pushLock.runExclusive(tenantId, workflowId, () -> { + SubWorkflowGraphValidator.validate( + workflow, + id -> id.equals(workflow.getId()) + ? Optional.of(workflow) // incoming overrides repo + : repository.findById(tenantId, id)); // lazy resolution + repository.save(tenantId, workflow); +}); +``` + +Key properties: + +- **Cluster-wide mutex** — `WorkflowPushLock` wraps the save in `pg_advisory_xact_lock` + (JVM `ReentrantLock` fallback when `quarkus.datasource.active=false`). Without this, + two concurrent pushes on different nodes could each observe a clean graph and together + introduce a cycle. +- **Post-push view** — the resolver short-circuits to the incoming workflow for its own + id, so the DFS sees the graph *as it will exist* after the push — no intermediate write + is needed, and a push that would fix an existing cycle validates correctly. +- **Single DFS** — `SubWorkflowGraphValidator.validate(Workflow, Function)` uses one + `globallyVisited` set for both cycle detection and dangling-ref detection. +- **Tenant-scoped resolution** — `repository.findById(tenantId, id)` never looks up + workflows from another tenant, so a sub-workflow target that exists under a different + tenant is rejected as dangling. +- **Runtime bounds** — even if validation passes, recursion is capped at + `SubWorkflowNodeExecutor.MAX_DEPTH = 16` (tracked via `_sub_workflow_depth`) and + `_tenant_id` is propagated into the child context, so a malicious deep chain cannot + exhaust the stack and a child cannot escape its tenant boundary. + +See the [hensu-core Developer Guide — `SubWorkflowGraphValidator` checks](developer-guide-core.md#subworkflowgraphvalidator-checks) +for the core-side algorithm and the CLI-only `validate(Collection)` overload. + #### Log Sanitizer (Defense-in-Depth) `LogSanitizer.sanitize()` strips CR/LF characters from user-derived values before they reach log diff --git a/docs/dsl-reference.md b/docs/dsl-reference.md index 49c8b14..8300daf 100644 --- a/docs/dsl-reference.md +++ b/docs/dsl-reference.md @@ -14,6 +14,7 @@ This document provides a complete reference for the Hensu Kotlin DSL used to def - [Join Node](#join-node) - [Generic Node](#generic-node) - [Action Node](#action-node) + - [Sub-Workflow Node](#sub-workflow-node) - [End Node](#end-node) - [Transitions](#transitions) - [State Variables (`writes`)](#state-variables-writes) @@ -132,16 +133,17 @@ graph { ### Graph Functions -| Function | Description | -|---------------------|------------------------------------------------------------| -| `start at "nodeId"` | Sets the workflow entry point node | -| `node(id) { }` | Defines a standard agent-based node | -| `parallel(id) { }` | Defines a parallel node with consensus | -| `fork(id) { }` | Defines a fork node for parallel execution | -| `join(id) { }` | Defines a join node to await forked paths | -| `generic(id) { }` | Defines a generic node with custom executor | -| `action(id) { }` | Defines an action node for executing commands mid-workflow | -| `end(id)` | Defines an end node (workflow termination) | +| Function | Description | +|-----------------------|------------------------------------------------------------| +| `start at "nodeId"` | Sets the workflow entry point node | +| `node(id) { }` | Defines a standard agent-based node | +| `parallel(id) { }` | Defines a parallel node with consensus | +| `fork(id) { }` | Defines a fork node for parallel execution | +| `join(id) { }` | Defines a join node to await forked paths | +| `generic(id) { }` | Defines a generic node with custom executor | +| `action(id) { }` | Defines an action node for executing commands mid-workflow | +| `subWorkflow(id) { }` | Defines a sub-workflow delegation node | +| `end(id)` | Defines an end node (workflow termination) | ## Nodes @@ -446,6 +448,59 @@ graph { } ``` +### Sub-Workflow Node + +Sub-workflow nodes delegate execution to a nested workflow. The parent pauses at the boundary, the child runs to completion, and control returns with selected state variables mirrored back into the parent. + +```kotlin +subWorkflow("delegate_summary") { + target = "sub-summarizer" + targetVersion = "1.0.0" // Optional, forward-compat with workflow versioning + + imports("draft") // Copy from parent state into child under the same name + writes("tl_dr") // Mirror from child state back into parent under the same name + + onSuccess goto "publish" + onFailure goto "handle_error" +} +``` + +#### Sub-Workflow Node Properties + +| Property | Type | Required | Description | +|-----------------|---------|----------|--------------------------------------------------------------------------------------------------------| +| `target` | String | Yes | Id of the child workflow to invoke | +| `targetVersion` | String? | No | Pinned version of the child workflow. Round-tripped through serialization; not enforced at runtime yet | + +#### Sub-Workflow Node Functions + +| Function | Description | +|-----------------------|-------------------------------------------------------------------------------------------------| +| `imports("a", "b")` | Parent state variables copied into the child under the same name | +| `writes("x", "y")` | Child state variables mirrored back into the parent under the same name | +| `onSuccess goto` | Route on successful child completion | +| `onFailure goto` | Route on child failure | + +#### Same-Name Discipline + +The DSL enforces identity mapping across the boundary – a name in `imports` or `writes` refers to the same key in both parent and child. The child's `state { }` schema is the contract; parents adapt their state variable names to match. This keeps sub-workflow integration explicit and greppable. + +Validation rules (checked by `WorkflowBuilder.build()`): + +- Every name in `imports` and `writes` must be declared in the parent workflow's `state { }` block. +- Engine variables (`score`, `approved`, `recommendation`) are rejected. +- `imports` and `writes` must not overlap. +- Duplicate names within either list are rejected. + +#### Reference Graph Constraints + +Sub-workflow references are validated before any node executes: + +- **Cycles** – `SubWorkflowGraphValidator` rejects any cycle in the sub-workflow reference graph at load time (CLI) and push time (server). +- **Dangling references** – On the server push path, unresolved child ids are rejected alongside cycles in a single pass. +- **Depth cap** – Nested invocation is capped at depth 16 (`SubWorkflowNodeExecutor.MAX_DEPTH`). The executor throws before invoking a child beyond the cap; this is a hard guard against runaway recursion, not a tunable. +- **Tenant isolation** – `_tenant_id` is propagated from parent to child context, preserving multi-tenant isolation across the boundary. + ### End Node End nodes terminate workflow execution with a status. They contain no actions — use action nodes for command execution. diff --git a/docs/unified-architecture.md b/docs/unified-architecture.md index ab87670..b425163 100644 --- a/docs/unified-architecture.md +++ b/docs/unified-architecture.md @@ -98,16 +98,16 @@ Unicode manipulation, and excessive payload size before the output is written to Workflows are not limited to linear chains. The graph engine supports: -| Capability | Mechanism | -|:--------------------------|:--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| **Conditional branching** | `ScoreTransition` routes based on rubric scores; `SuccessTransition` / `FailureTransition` route on result | -| **Loops** | `LoopNode` with configurable break conditions and max iterations | -| **Parallel fan-out** | `ParallelNode` executes branches concurrently on virtual threads | -| **Fork / Join** | `ForkNode` spawns independent parallel paths; `JoinNode` awaits and merges results | -| **Consensus** | Majority vote, unanimous, weighted vote, or judge-decides strategies. Branches declare domain output via `yields()`. Vote strategies merge all branch yields; JUDGE_DECIDES merges only the winning branch's yields | -| **Backtracking** | Review decisions can jump to any previous node, restoring state from execution history | -| **Sub-workflows** | `SubWorkflowNode` with input/output mapping for hierarchical composition | -| **Pause / Resume** | Any node returning `PENDING` checkpoints state (including `PlanSnapshot` — micro-plan step index — alongside node position); `executeFrom()` resumes from snapshot | +| Capability | Mechanism | +|:--------------------------|:---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| **Conditional branching** | `ScoreTransition` routes based on rubric scores; `SuccessTransition` / `FailureTransition` route on result | +| **Loops** | `LoopNode` with configurable break conditions and max iterations | +| **Parallel fan-out** | `ParallelNode` executes branches concurrently on virtual threads | +| **Fork / Join** | `ForkNode` spawns independent parallel paths; `JoinNode` awaits and merges results | +| **Consensus** | Majority vote, unanimous, weighted vote, or judge-decides strategies. Branches declare domain output via `yields()`. Vote strategies merge all branch yields; JUDGE_DECIDES merges only the winning branch's yields | +| **Backtracking** | Review decisions can jump to any previous node, restoring state from execution history | +| **Sub-workflows** | `SubWorkflowNode` delegates to another workflow by id with input/output mapping; `SubWorkflowGraphValidator` rejects cycles and dangling refs at push, `SubWorkflowNodeExecutor.MAX_DEPTH = 16` bounds recursion, `_tenant_id` propagates into the child | +| **Pause / Resume** | Any node returning `PENDING` checkpoints state (including `PlanSnapshot` — micro-plan step index — alongside node position); `executeFrom()` resumes from snapshot | For non-agent steps, `GenericNode` runs custom synchronous logic registered by `executorType`; `ActionNode` dispatches asynchronous tasks to external systems via a registered `ActionHandler` @@ -221,7 +221,7 @@ Violations return `400 Bad Request`. See [Server Developer Guide — Input Valid ``` /api/v1/workflows → WorkflowResource (definition management - CLI integration) -├── POST / Push workflow (create/update) +├── POST / Push workflow (create/update; validates sub-workflow graph under push lock) ├── GET / List workflows ├── GET /{workflowId} Pull workflow └── DELETE /{workflowId} Delete workflow @@ -334,6 +334,7 @@ Zero-dependency Java library. Contains: - `WorkflowRepository` / `WorkflowStateRepository` — Tenant-scoped storage interfaces with in-memory defaults - `HensuState` / `HensuSnapshot` / `ExecutionHistory` — Mutable runtime state, immutable checkpoints, execution trace - Workflow model, Node types (including `SubWorkflowNode`), Transition rules +- `SubWorkflowGraphValidator` (`workflow/validation`) — cycle + dangling-ref detection across the sub-workflow reference graph, single-DFS with `globallyVisited`; `SubWorkflowNodeExecutor` enforces `MAX_DEPTH = 16` and propagates `_tenant_id` into the child context ### hensu-dsl (Kotlin DSL) @@ -353,7 +354,9 @@ Extends core with HTTP, MCP, and multi-tenancy: - `HensuEnvironmentProducer` — CDI producer using `HensuFactory.builder()` - `ServerConfiguration` — Delegates core components from `HensuEnvironment` via `@Produces @Singleton` - `ServerActionExecutor` — Send-action dispatcher (routes to registered handlers, falls back to MCP; rejects `Action.Execute`) -- `WorkflowService` — Service layer: start/resume executions, snapshot management +- `WorkflowService` — Service layer facade: start/resume executions, snapshot management +- `WorkflowRegistryService` — Push pipeline: wraps save in `WorkflowPushLock` and invokes `SubWorkflowGraphValidator` lazily resolving sub-workflow ids through the repository +- `WorkflowPushLock` — Cluster-wide push mutex (`pg_advisory_xact_lock` with JVM `ReentrantLock` fallback) preventing concurrent pushes on different nodes from introducing cycles - `WorkflowResource` — Workflow definition management (push/pull/delete/list) - `ExecutionResource` — Execution runtime (start/resume/status/plan) - `McpSidecar` / `McpGateway` — MCP protocol integration @@ -771,7 +774,7 @@ The unified architecture provides: 7. **Rubric Evaluation** — Quality gates that score outputs and route on thresholds for self-correcting loops 8. **Pause / Resume** — Workflows checkpoint at any node (including micro-plan step index via `PlanSnapshot`) and resume; the lease protocol protects against data races when the owning node crashes 9. **Distributed Recovery** — Heartbeat/sweeper lease protocol for crashed-node detection; atomic PostgreSQL `UPDATE…RETURNING` claim -10. **Sub-Workflows** — Hierarchical composition via `SubWorkflowNode` with input/output mapping +10. **Sub-Workflows** — Hierarchical composition via `SubWorkflowNode` with input/output mapping; `SubWorkflowGraphValidator` rejects cycles and dangling refs at push (under `WorkflowPushLock`); recursion bounded by `MAX_DEPTH = 16`; tenant isolation preserved across the boundary via `_tenant_id` propagation 11. **Flexible Planning** — Static (predefined) or Dynamic (LLM-generated) execution plans within nodes 12. **Human Review** — Checkpoints for manual approval at both plan and execution levels 13. **Multi-Tenancy** — Java 25 `ScopedValues` for safe tenant context propagation and isolation diff --git a/hensu-cli/README.md b/hensu-cli/README.md index b679467..3fb3038 100644 --- a/hensu-cli/README.md +++ b/hensu-cli/README.md @@ -16,6 +16,7 @@ and re-attach at any time without interrupting execution. - [`hensu validate`](#hensu-validate) - [`hensu visualize`](#hensu-visualize) - [`hensu build`](#hensu-build) + - [Sub-Workflows](#sub-workflows) - [Daemon Commands](#daemon-commands) - [`hensu daemon`](#hensu-daemon) - [`hensu ps`](#hensu-ps) @@ -115,6 +116,7 @@ execution when the daemon is not available. ``` hensu run [] [-d ] [-v] [-i] [--no-color] [--no-daemon] [-c ] + [--with ]... options: Workflow name from workflows/ (default: hensu.workflow.file property) @@ -122,6 +124,7 @@ options: -c, --context Context as a JSON string '{"key":"value"}' or path to a JSON/YAML file -v, --verbose Show agent inputs/outputs and fork/join execution structure -i, --interactive Enable interactive human review mode with manual backtracking + --with Sub-workflow to load alongside the root (repeatable); see Sub-Workflows below --no-color Disable ANSI colored output --no-daemon Force inline execution even if the daemon is running ``` @@ -144,11 +147,12 @@ the decision. A 30-minute fallback timeout applies. ### `hensu validate` -Parse the workflow and perform static analysis: syntax errors, missing node references, and -unreachable nodes. +Parse the workflow and perform static analysis: syntax errors, missing node references, +unreachable nodes, and – when `--with` supplies sub-workflows – cycles in the sub-workflow +reference graph. ``` -hensu validate [] [-d ] +hensu validate [] [-d ] [--with ]... ``` ### `hensu visualize` @@ -156,10 +160,11 @@ hensu validate [] [-d ] Render the workflow graph to the terminal. ``` -hensu visualize [] [-d ] [--format ] +hensu visualize [] [-d ] [--format ] [--with ]... options: --format Output format (default: text) + --with Sub-workflow to include in the rendered graph (repeatable) ``` ### `hensu build` @@ -171,6 +176,35 @@ and is required by `hensu push`. hensu build [] [-d ] ``` +`build` does not accept `--with`. Sub-workflows are compiled and pushed independently; +the server resolves references at runtime via its own registry and rejects dangling refs +at push time. + +--- + +## Sub-Workflows + +A workflow can delegate to another workflow via a `subWorkflow` node. For commands that +execute or analyze the graph locally – `run`, `validate`, `visualize` – referenced children +must be supplied explicitly with `--with`. The flag is repeatable. + +```bash +hensu run parent --with child-a --with child-b -d ./my-project +``` + +Under the hood: + +- Each `--with ` resolves to `workflows/.kt` in the working directory. +- The root plus all `--with` workflows are registered under the CLI tenant + (`SubWorkflowLoader.CLI_TENANT`) so `SubWorkflowNodeExecutor` can resolve children at runtime. +- When the daemon handles the execution, the full set is serialized and shipped across the + wire – the daemon JVM does not share your working directory. +- `validate` additionally runs `SubWorkflowGraphValidator` over the supplied set, rejecting + cycles before execution. + +`build` and `push` do not use `--with` – each workflow is compiled and pushed independently, +and the server validates the full reference graph (cycles + dangling refs) at push time. + --- ## Daemon Commands diff --git a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowBuildCommand.java b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowBuildCommand.java index dcf4e23..538ac8a 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowBuildCommand.java +++ b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowBuildCommand.java @@ -2,10 +2,12 @@ import io.hensu.cli.exception.UnsupportedWorkflowException; import io.hensu.core.workflow.Workflow; +import io.hensu.dsl.WorkingDirectory; import io.hensu.serialization.WorkflowSerializer; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import picocli.CommandLine.Command; import picocli.CommandLine.Parameters; @@ -49,4 +51,12 @@ protected void execute() { System.err.println("I/O error: " + e.getMessage()); } } + + /// Sub-workflows referenced by the root are built and pushed independently; + /// the server resolves them at runtime. Local `--with` resolution is not required. + @Override + protected List resolveSubWorkflows( + WorkingDirectory workingDir, Workflow root, List withNames) { + return List.of(); + } } diff --git a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowCommand.java b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowCommand.java index 2cbe6b0..f4edd7e 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowCommand.java +++ b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowCommand.java @@ -1,11 +1,13 @@ package io.hensu.cli.commands; import io.hensu.cli.exception.UnsupportedWorkflowException; +import io.hensu.cli.workflow.SubWorkflowLoader; import io.hensu.core.workflow.Workflow; import io.hensu.dsl.WorkingDirectory; import io.hensu.dsl.parsers.KotlinScriptParser; import jakarta.inject.Inject; import java.nio.file.Path; +import java.util.List; import java.util.Objects; import java.util.Optional; import org.eclipse.microprofile.config.inject.ConfigProperty; @@ -49,6 +51,16 @@ public abstract class WorkflowCommand extends HensuCommand { @Inject private KotlinScriptParser kotlinParser; + @Inject private SubWorkflowLoader subWorkflowLoader; + + /// Sub-workflows loaded alongside the root by the most recent + /// {@link #getWorkflow(String, List)} call, in declaration order and deduplicated. + /// Empty when the root was loaded without `--with` or has no sub-workflow references. + /// Subclasses that forward execution off-JVM (e.g. to the background daemon) must + /// transport these alongside the root — the loader only registers them in the CLI's + /// in-memory repository. + protected List loadedSubWorkflows = List.of(); + /// Loads and parses a workflow by name from the working directory. /// /// Resolves the workflow name using CLI argument or config default, then parses @@ -59,6 +71,22 @@ public abstract class WorkflowCommand extends HensuCommand { /// @throws UnsupportedWorkflowException if workflow name is not specified and /// no default configured protected Workflow getWorkflow(String workflowName) throws UnsupportedWorkflowException { + return getWorkflow(workflowName, List.of()); + } + + /// Loads and parses the root workflow plus every sub-workflow declared via `--with`. + /// + /// Each `withName` is resolved through the same {@link WorkingDirectory} as the root + /// (shared `workflows/`, `prompts/`, `rubrics/`). The compiled workflows are saved to + /// the shared repository under the CLI tenant so `SubWorkflowNodeExecutor` can resolve + /// them at runtime. + /// + /// @param workflowName root workflow name, may be null + /// @param withNames sub-workflow names declared via `--with`, may be empty + /// @return parsed root workflow, never null + /// @throws UnsupportedWorkflowException if no root workflow name is configured + protected Workflow getWorkflow(String workflowName, List withNames) + throws UnsupportedWorkflowException { String effectiveWorkflowName = resolveWorkflowName(workflowName); if (effectiveWorkflowName == null) { System.err.println( @@ -74,7 +102,17 @@ protected Workflow getWorkflow(String workflowName) throws UnsupportedWorkflowEx System.out.println("Using working directory: " + workingDir.root()); System.out.println("Compiling Kotlin DSL workflow: " + effectiveWorkflowName); - return kotlinParser.parse(workingDir, effectiveWorkflowName); + Workflow root = kotlinParser.parse(workingDir, effectiveWorkflowName); + loadedSubWorkflows = resolveSubWorkflows(workingDir, root, withNames); + return root; + } + + /// Hook for subclasses to override sub-workflow resolution behavior. + /// + /// @return loaded sub-workflows in declaration order; empty when none are required + protected List resolveSubWorkflows( + WorkingDirectory workingDir, Workflow root, List withNames) { + return subWorkflowLoader.resolveDeclared(workingDir, root, withNames); } /// Returns the effective working directory for workflow resolution. diff --git a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowRunCommand.java b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowRunCommand.java index b699248..97f1524 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowRunCommand.java +++ b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowRunCommand.java @@ -8,6 +8,7 @@ import io.hensu.cli.review.CLIReviewHandler; import io.hensu.cli.review.DaemonClientReviewer; import io.hensu.cli.ui.AnsiStyles; +import io.hensu.cli.workflow.SubWorkflowLoader; import io.hensu.core.HensuEnvironment; import io.hensu.core.agent.stub.StubResponseRegistry; import io.hensu.core.execution.ExecutionListener; @@ -21,11 +22,13 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.UUID; import java.util.function.Consumer; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import picocli.CommandLine.Command; import picocli.CommandLine.Option; import picocli.CommandLine.Parameters; @@ -93,6 +96,14 @@ class WorkflowRunCommand extends WorkflowCommand { description = "Force inline execution even if daemon is running") private boolean noDaemon = false; + @Option( + names = {"--with"}, + description = + """ + Sub-workflow to load alongside the root \ + (repeatable, resolved from working-dir/workflows/, .kt optional)""") + private List withNames = List.of(); + @Inject private HensuEnvironment environment; @Inject private VerboseExecutionListenerFactory listenerFactory; @@ -105,7 +116,7 @@ protected void execute() { System.setProperty("hensu.review.interactive", "true"); } - Workflow workflow = getWorkflow(workflowName); + Workflow workflow = getWorkflow(workflowName, withNames); ExecutionSink sink = LocalExecutionSink.INSTANCE; @SuppressWarnings("resource") // wraps System.out — must not be closed @@ -114,9 +125,20 @@ protected void execute() { "%n%s %s%n", styles.checkmark(), styles.bold(workflow.getMetadata().getName() + " loaded")); sinkOut.printf(" agents %d%n", workflow.getAgents().size()); - sinkOut.printf(" nodes %d%n%n", workflow.getNodes().size()); + sinkOut.printf(" nodes %d%n", workflow.getNodes().size()); + if (!loadedSubWorkflows.isEmpty()) { + String subIds = + loadedSubWorkflows.stream() + .map(w -> w.getMetadata().getName()) + .collect(Collectors.joining(", ")); + sinkOut.printf(" subs %d (%s)%n", loadedSubWorkflows.size(), subIds); + } + sinkOut.println(); Map context = loadContext(contextInput); + // Seed the tenant so SubWorkflowNodeExecutor resolves children from the + // repository slot the loader just populated. + context.putIfAbsent("_tenant_id", SubWorkflowLoader.CLI_TENANT); // — Daemon mode —————————————————————————————————————————————— if (!noDaemon && DaemonClient.isAlive()) { @@ -150,6 +172,12 @@ private void runViaDaemon(Workflow workflow, Map context, AnsiSt req.execId = execId; req.workflowId = workflow.getMetadata().getName(); req.workflowJson = workflowJson; + // Ship `--with` subs across the wire — the daemon runs in a separate JVM with its + // own WorkflowRepository, so the loader's in-process registration doesn't reach it. + if (!loadedSubWorkflows.isEmpty()) { + req.subWorkflowsJson = + loadedSubWorkflows.stream().map(WorkflowSerializer::toJson).toList(); + } req.context = context; req.verbose = verbose; req.color = color; diff --git a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowValidateCommand.java b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowValidateCommand.java index 3bc8996..ec9d680 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowValidateCommand.java +++ b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowValidateCommand.java @@ -33,10 +33,18 @@ class WorkflowValidateCommand extends WorkflowCommand { arity = "0..1") private String workflowName; + @CommandLine.Option( + names = {"--with"}, + description = + """ + Sub-workflow to load and validate alongside the root \ + (repeatable, resolved from working-dir/workflows/, .kt optional)""") + private List withNames = List.of(); + @Override protected void execute() { try { - Workflow workflow = getWorkflow(workflowName); + Workflow workflow = getWorkflow(workflowName, withNames); System.out.println(" [OK] Workflow is valid!"); System.out.println(" Name: " + workflow.getMetadata().getName()); diff --git a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowVisualizeCommand.java b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowVisualizeCommand.java index f7872db..aacea83 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowVisualizeCommand.java +++ b/hensu-cli/src/main/java/io/hensu/cli/commands/WorkflowVisualizeCommand.java @@ -3,6 +3,7 @@ import io.hensu.cli.visualizer.WorkflowVisualizer; import io.hensu.core.workflow.Workflow; import jakarta.inject.Inject; +import java.util.List; import picocli.CommandLine; /// CLI command for rendering workflow graphs in various formats. @@ -34,12 +35,20 @@ class WorkflowVisualizeCommand extends WorkflowCommand { description = "Output format: text, mermaid") private String format; + @CommandLine.Option( + names = {"--with"}, + description = + """ + Sub-workflow to load alongside the root \ + (repeatable, resolved from working-dir/workflows/, .kt optional)""") + private List withNames = List.of(); + @Inject WorkflowVisualizer visualizer; @Override protected void execute() { try { - Workflow workflow = getWorkflow(workflowName); + Workflow workflow = getWorkflow(workflowName, withNames); String output = visualizer.visualize(workflow, format); System.out.println(output); } catch (Exception e) { diff --git a/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonFrame.java b/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonFrame.java index 5da53e3..6f06b99 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonFrame.java +++ b/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonFrame.java @@ -133,6 +133,15 @@ public final class DaemonFrame { @JsonProperty("workflow_json") public String workflowJson; + /// Pre-compiled sub-workflow JSONs from `--with` (run frames). + /// Each entry is a standalone {@link io.hensu.core.workflow.Workflow} serialized via + /// {@link io.hensu.serialization.WorkflowSerializer}. The daemon registers them in its + /// own {@link io.hensu.core.workflow.WorkflowRepository} under the run's tenant before + /// the executor starts so {@code SubWorkflowNodeExecutor} can resolve them. Empty or + /// null when the run has no sub-workflow references. + @JsonProperty("sub_workflows") + public List subWorkflowsJson; + /// Initial context map (run frames). @JsonProperty("context") public Map context; diff --git a/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonServer.java b/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonServer.java index ffb0878..5fa8ba1 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonServer.java +++ b/hensu-cli/src/main/java/io/hensu/cli/daemon/DaemonServer.java @@ -5,6 +5,7 @@ import io.hensu.cli.execution.DaemonExecutionSink; import io.hensu.cli.execution.VerboseExecutionListenerFactory; import io.hensu.cli.review.DaemonReviewHandler; +import io.hensu.cli.workflow.SubWorkflowLoader; import io.hensu.core.HensuEnvironment; import io.hensu.core.execution.ExecutionListener; import io.hensu.core.execution.result.ExecutionResult; @@ -28,6 +29,7 @@ import java.util.Base64; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; @@ -392,6 +394,12 @@ private void runExecution( // DaemonReviewManager cannot correlate the review to this execution. context.put("_execution_id", execId); + // Register `--with` subs in the daemon's repository under the run's tenant + // before the executor starts — SubWorkflowNodeExecutor looks them up there. + // The CLI process registered them in its own JVM; nothing crossed the wire + // until the frame added `sub_workflows`. + registerSubWorkflows(req, context); + var sink = new DaemonExecutionSink(execution, mapper); ExecutionListener listener = verbose @@ -417,6 +425,19 @@ private void runExecution( } } + /// Deserializes every `--with` sub-workflow from the run frame and saves it into the + /// daemon's {@link io.hensu.core.workflow.WorkflowRepository} under the context's + /// {@code _tenant_id}. No-op when the frame has no subs. The tenant defaults to + /// {@code io.hensu.cli.workflow.SubWorkflowLoader#CLI_TENANT} to match the CLI seed. + private void registerSubWorkflows(DaemonFrame req, Map context) { + if (req.subWorkflowsJson == null || req.subWorkflowsJson.isEmpty()) return; + String tenantId = (String) context.getOrDefault("_tenant_id", SubWorkflowLoader.CLI_TENANT); + for (String json : req.subWorkflowsJson) { + var sub = WorkflowSerializer.fromJson(json); + environment.getWorkflowRepository().save(tenantId, sub); + } + } + // — Queue drain ———————————————————————————————————————————————————————— private void drainQueueToWriter( diff --git a/hensu-cli/src/main/java/io/hensu/cli/producers/HensuEnvironmentProducer.java b/hensu-cli/src/main/java/io/hensu/cli/producers/HensuEnvironmentProducer.java index 47c1271..68399ed 100644 --- a/hensu-cli/src/main/java/io/hensu/cli/producers/HensuEnvironmentProducer.java +++ b/hensu-cli/src/main/java/io/hensu/cli/producers/HensuEnvironmentProducer.java @@ -9,6 +9,8 @@ import io.hensu.core.HensuFactory; import io.hensu.core.execution.executor.GenericNodeHandler; import io.hensu.core.review.ReviewHandler; +import io.hensu.core.workflow.InMemoryWorkflowRepository; +import io.hensu.core.workflow.WorkflowRepository; import io.hensu.serialization.WorkflowSerializer; import io.hensu.serialization.plan.JacksonPlanResponseParser; import jakarta.enterprise.context.ApplicationScoped; @@ -53,6 +55,21 @@ public class HensuEnvironmentProducer { private HensuEnvironment hensuEnvironment; + /// Shared in-memory workflow repository for the CLI runtime. The same instance is wired + /// into {@link HensuEnvironment} and exposed for CDI injection so callers can populate or + /// query workflows through the lifetime of the process. On-demand sub-workflow resolution + /// is one current consumer; any other component is free to use it the same way. + private final InMemoryWorkflowRepository workflowRepository = new InMemoryWorkflowRepository(); + + /// Exposes the shared in-memory workflow repository for CDI injection. + /// + /// @return the singleton repository instance backing {@link HensuEnvironment}, never null + @Produces + @ApplicationScoped + public WorkflowRepository workflowRepository() { + return workflowRepository; + } + @Inject Config config; @Inject Instance genericNodeHandlers; @@ -80,6 +97,7 @@ public HensuEnvironment hensuEnvironment() { .agentProviders(List.of(new LangChain4jProvider())) .reviewHandler(reviewHandler) .actionExecutor(actionExecutor) + .workflowRepository(workflowRepository) .planResponseParser( new JacksonPlanResponseParser(WorkflowSerializer.createMapper())) .build(); diff --git a/hensu-cli/src/main/java/io/hensu/cli/workflow/SubWorkflowLoader.java b/hensu-cli/src/main/java/io/hensu/cli/workflow/SubWorkflowLoader.java new file mode 100644 index 0000000..271d503 --- /dev/null +++ b/hensu-cli/src/main/java/io/hensu/cli/workflow/SubWorkflowLoader.java @@ -0,0 +1,282 @@ +package io.hensu.cli.workflow; + +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.WorkflowRepository; +import io.hensu.core.workflow.node.JoinNode; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.StandardNode; +import io.hensu.core.workflow.node.SubWorkflowNode; +import io.hensu.core.workflow.state.WorkflowStateSchema; +import io.hensu.core.workflow.validation.SubWorkflowGraphValidator; +import io.hensu.dsl.WorkingDirectory; +import io.hensu.dsl.parsers.KotlinScriptParser; +import io.hensu.serialization.WorkflowSerializer; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/// Loads sub-workflows supplied via the `--with` CLI option and wires them into the +/// shared {@link WorkflowRepository} so {@code SubWorkflowNodeExecutor} can resolve +/// children at runtime. +/// +/// ### Declaration model +/// Every `--with ` value is a workflow name resolved through the same +/// {@link WorkingDirectory} as the root, via {@link WorkingDirectory#resolveWorkflow}. +/// The `.kt` suffix is optional. Prompts and rubrics are shared with the root. +/// +/// ```bash +/// hensu run main-research --with sub-summarizer --with sub-translator +/// ``` +/// +/// ### Validation +/// After every declared sub is compiled and saved, the loader walks every +/// {@link SubWorkflowNode} reference in the root and in each loaded sub and verifies: +/// - the `target` id resolves to a workflow the user supplied via `--with` +/// - any `targetVersion` pin matches the child's declared version +/// - same-name `imports` and `writes` are declared in the child's state schema +/// - the child actually writes every variable the parent expects +/// +/// Duplicate ids (root id clashing with a sub, or two subs with the same id) and hash +/// divergence for the same `(id, version)` are hard errors. All errors are aggregated. +/// +/// ### Tenancy +/// The CLI uses a single tenant across its runtime. The same constant is injected into +/// the execution context by `WorkflowRunCommand` so the executor's repository lookups +/// resolve to the workflows saved here. +/// +/// ### Hashing +/// SHA-256 over `WorkflowSerializer.toJson(child)` with all whitespace stripped. Used +/// now to detect divergent duplicate loads; forward compatible with the +/// workflow-versioning ticket's `targetHash` enforcement. +/// +/// @implNote Stateless `@ApplicationScoped` bean. All traversal state is local. +@ApplicationScoped +public class SubWorkflowLoader { + + /// Tenant id used by the CLI runtime for workflow repository storage and lookups. + /// `WorkflowRunCommand` seeds the execution context `_tenant_id` with the same value + /// so sub-workflow executor lookups hit the workflows saved here. + public static final String CLI_TENANT = "test-tenant"; + + @Inject KotlinScriptParser kotlinParser; + + @Inject WorkflowRepository workflowRepository; + + /// Compiles every `--with` workflow, saves the root and each sub to the repository, + /// and validates every {@link SubWorkflowNode} binding reachable from the loaded set. + /// + /// @param workingDir the shared working directory for root and every declared sub + /// @param root the already-compiled root workflow, not null + /// @param withNames workflow names supplied via `--with`; may be empty if the root + /// has no sub-workflow references + /// @return the unique sub-workflows loaded (root excluded), in declaration order; + /// empty when {@code withNames} is empty or only declares duplicates of root + /// @throws IllegalStateException if any sub fails to compile, duplicate ids are + /// declared, the same `(id, version)` loads with divergent content, a pinned + /// `targetVersion` mismatches, or a parent/child binding is invalid + public List resolveDeclared( + WorkingDirectory workingDir, Workflow root, List withNames) { + List errors = new ArrayList<>(); + Map hashesByCoord = new HashMap<>(); + Map loadedById = new LinkedHashMap<>(); + + saveWorkflow(root, hashesByCoord, loadedById, errors, "(root)"); + + if (withNames != null) { + for (String name : withNames) { + compileDeclaredSub(workingDir, name, hashesByCoord, loadedById, errors); + } + } + + // Reject parent→child cycles before runtime can blow the stack. + try { + SubWorkflowGraphValidator.validate(loadedById.values()); + } catch (IllegalStateException e) { + errors.add(e.getMessage()); + } + + // Validate every SubWorkflowNode reference reachable from root + loaded subs. + for (Workflow wf : new ArrayList<>(loadedById.values())) { + for (Node node : wf.getNodes().values()) { + if (!(node instanceof SubWorkflowNode sub)) continue; + validateReference(wf, sub, loadedById, errors); + } + } + + if (!errors.isEmpty()) { + throw new IllegalStateException( + "Sub-workflow resolution failed for '" + + root.getId() + + "':\n - " + + String.join("\n - ", errors)); + } + + List subs = new ArrayList<>(loadedById.values()); + subs.removeIf(wf -> wf.getId().equals(root.getId())); + return List.copyOf(subs); + } + + private void compileDeclaredSub( + WorkingDirectory workingDir, + String name, + Map hashesByCoord, + Map loadedById, + List errors) { + Workflow compiled; + try { + compiled = kotlinParser.parse(workingDir, name); + } catch (RuntimeException e) { + errors.add("--with '" + name + "' failed to compile: " + e.getMessage()); + return; + } + saveWorkflow(compiled, hashesByCoord, loadedById, errors, "--with " + name); + } + + private void saveWorkflow( + Workflow workflow, + Map hashesByCoord, + Map loadedById, + List errors, + String source) { + String id = workflow.getId(); + String coord = coord(id, workflow.getVersion()); + byte[] newHash = trimmedHash(workflow); + + Workflow priorById = loadedById.get(id); + if (priorById != null) { + byte[] previous = hashesByCoord.get(coord); + if (previous == null || !Arrays.equals(previous, newHash)) { + errors.add( + "Workflow id '" + + id + + "' declared twice with conflicting definitions" + + (priorById.getVersion().equals(workflow.getVersion()) + ? "" + : " (versions '" + + priorById.getVersion() + + "' and '" + + workflow.getVersion() + + "')") + + " (source: " + + source + + ")"); + } + return; + } + + workflowRepository.save(CLI_TENANT, workflow); + hashesByCoord.put(coord, newHash); + loadedById.put(id, workflow); + } + + private static void validateReference( + Workflow parent, + SubWorkflowNode sub, + Map loadedById, + List errors) { + String targetId = sub.getWorkflowId(); + Workflow child = loadedById.get(targetId); + if (child == null) { + errors.add( + "Parent '" + + parent.getId() + + "' references sub-workflow '" + + targetId + + "' which was not supplied via --with"); + return; + } + String pinned = sub.getTargetVersion(); + if (pinned != null && !pinned.equals(child.getVersion())) { + errors.add( + "Parent '" + + parent.getId() + + "' pins sub-workflow '" + + targetId + + "' to version '" + + pinned + + "' but child declares '" + + child.getVersion() + + "'"); + return; + } + validateBinding(parent, sub, child, errors); + } + + private static void validateBinding( + Workflow parent, SubWorkflowNode sub, Workflow child, List errors) { + WorkflowStateSchema childSchema = child.getStateSchema(); + if (childSchema == null) { + return; + } + for (String name : sub.getInputMapping().keySet()) { + if (!childSchema.contains(name)) { + errors.add( + "Parent '" + + parent.getId() + + "' imports '" + + name + + "' into sub-workflow '" + + child.getId() + + "' but child schema does not declare it"); + } + } + Set childWrites = collectChildWrites(child); + for (String name : sub.getOutputMapping().keySet()) { + if (!childSchema.contains(name)) { + errors.add( + "Parent '" + + parent.getId() + + "' expects sub-workflow '" + + child.getId() + + "' to write '" + + name + + "' but child schema does not declare it"); + } else if (!childWrites.contains(name)) { + errors.add( + "Parent '" + + parent.getId() + + "' expects sub-workflow '" + + child.getId() + + "' to write '" + + name + + "' but no node in the child writes that variable"); + } + } + } + + private static Set collectChildWrites(Workflow child) { + Set writes = new HashSet<>(); + for (Node n : child.getNodes().values()) { + if (n instanceof StandardNode sn) { + writes.addAll(sn.getWrites()); + } else if (n instanceof JoinNode jn) { + writes.addAll(jn.getWrites()); + } + } + return writes; + } + + private static byte[] trimmedHash(Workflow workflow) { + String json = WorkflowSerializer.toJson(workflow).replaceAll("\\s", ""); + try { + return MessageDigest.getInstance("SHA-256") + .digest(json.getBytes(StandardCharsets.UTF_8)); + } catch (NoSuchAlgorithmException e) { + throw new IllegalStateException("SHA-256 not available", e); + } + } + + private static String coord(String id, String version) { + return id + "@" + (version == null ? "" : version); + } +} diff --git a/hensu-cli/src/test/java/io/hensu/cli/commands/BaseWorkflowCommandTest.java b/hensu-cli/src/test/java/io/hensu/cli/commands/BaseWorkflowCommandTest.java index 86c4118..7ed618a 100644 --- a/hensu-cli/src/test/java/io/hensu/cli/commands/BaseWorkflowCommandTest.java +++ b/hensu-cli/src/test/java/io/hensu/cli/commands/BaseWorkflowCommandTest.java @@ -1,5 +1,6 @@ package io.hensu.cli.commands; +import io.hensu.cli.workflow.SubWorkflowLoader; import io.hensu.core.agent.AgentConfig; import io.hensu.core.execution.executor.NodeResult; import io.hensu.core.execution.result.BacktrackEvent; @@ -8,12 +9,14 @@ import io.hensu.core.execution.result.ExecutionStep; import io.hensu.core.execution.result.ExitStatus; import io.hensu.core.state.HensuState; +import io.hensu.core.workflow.InMemoryWorkflowRepository; import io.hensu.core.workflow.Workflow; import io.hensu.core.workflow.WorkflowMetadata; import io.hensu.core.workflow.node.EndNode; import io.hensu.core.workflow.node.Node; import io.hensu.core.workflow.node.StandardNode; import io.hensu.core.workflow.transition.SuccessTransition; +import io.hensu.dsl.parsers.KotlinScriptParser; import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.lang.reflect.Field; @@ -55,6 +58,17 @@ protected void injectField(Object target, String fieldName, Object value) throws field.set(target, value); } + /// Creates a real {@link SubWorkflowLoader} wired with the given parser and an + /// in-memory repository. Safe for command tests that pass no `--with` names: the + /// loader only saves the root and is otherwise a no-op. + protected SubWorkflowLoader createSubWorkflowLoader(KotlinScriptParser parser) + throws Exception { + SubWorkflowLoader loader = new SubWorkflowLoader(); + injectField(loader, "kotlinParser", parser); + injectField(loader, "workflowRepository", new InMemoryWorkflowRepository()); + return loader; + } + private Field findField(Class clazz, String fieldName) throws NoSuchFieldException { Class current = clazz; while (current != null) { diff --git a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowBuildCommandTest.java b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowBuildCommandTest.java index 737c02e..8863ef6 100644 --- a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowBuildCommandTest.java +++ b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowBuildCommandTest.java @@ -30,6 +30,7 @@ class WorkflowBuildCommandTest extends BaseWorkflowCommandTest { void setUp() throws Exception { command = new WorkflowBuildCommand(); injectField(command, "kotlinParser", kotlinParser); + injectField(command, "subWorkflowLoader", createSubWorkflowLoader(kotlinParser)); injectField(command, "workingDirPath", tempDir); } diff --git a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowRunCommandTest.java b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowRunCommandTest.java index 82b5eaf..9f87772 100644 --- a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowRunCommandTest.java +++ b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowRunCommandTest.java @@ -48,6 +48,7 @@ void setUp() throws Exception { command = new WorkflowRunCommand(); injectField(command, "kotlinParser", kotlinParser); + injectField(command, "subWorkflowLoader", createSubWorkflowLoader(kotlinParser)); injectField(command, "environment", environment); injectField(command, "workingDirPath", tempDir); injectField(command, "noDaemon", true); // force inline — tests run alongside a live daemon diff --git a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowValidateCommandTest.java b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowValidateCommandTest.java index b8c3509..ea1294d 100644 --- a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowValidateCommandTest.java +++ b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowValidateCommandTest.java @@ -35,6 +35,7 @@ void setUp() throws Exception { command = new WorkflowValidateCommand(); injectField(command, "kotlinParser", kotlinParser); + injectField(command, "subWorkflowLoader", createSubWorkflowLoader(kotlinParser)); injectField(command, "workingDirPath", tempDir); } diff --git a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowVisualizeCommandTest.java b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowVisualizeCommandTest.java index 2f07a1c..7c89134 100644 --- a/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowVisualizeCommandTest.java +++ b/hensu-cli/src/test/java/io/hensu/cli/commands/WorkflowVisualizeCommandTest.java @@ -38,6 +38,7 @@ void setUp() throws Exception { command = new WorkflowVisualizeCommand(); injectField(command, "kotlinParser", kotlinParser); + injectField(command, "subWorkflowLoader", createSubWorkflowLoader(kotlinParser)); injectField(command, "visualizer", visualizer); injectField(command, "workingDirPath", tempDir); } diff --git a/hensu-cli/src/test/java/io/hensu/cli/workflow/SubWorkflowLoaderTest.java b/hensu-cli/src/test/java/io/hensu/cli/workflow/SubWorkflowLoaderTest.java new file mode 100644 index 0000000..1c85b30 --- /dev/null +++ b/hensu-cli/src/test/java/io/hensu/cli/workflow/SubWorkflowLoaderTest.java @@ -0,0 +1,231 @@ +package io.hensu.cli.workflow; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.hensu.core.execution.result.ExitStatus; +import io.hensu.core.workflow.InMemoryWorkflowRepository; +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.WorkflowRepository; +import io.hensu.core.workflow.node.EndNode; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.StandardNode; +import io.hensu.core.workflow.node.SubWorkflowNode; +import io.hensu.core.workflow.state.StateVariableDeclaration; +import io.hensu.core.workflow.state.VarType; +import io.hensu.core.workflow.state.WorkflowStateSchema; +import io.hensu.core.workflow.transition.SuccessTransition; +import io.hensu.dsl.WorkingDirectory; +import io.hensu.dsl.parsers.KotlinScriptParser; +import java.lang.reflect.Field; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/// Aggregator-level tests for {@link SubWorkflowLoader}. Happy path plus every +/// distinct error mode — duplicate-id divergence, missing declarations, pin +/// mismatches, and cross-workflow binding violations. +class SubWorkflowLoaderTest { + + private KotlinScriptParser kotlinParser; + private WorkflowRepository workflowRepository; + private WorkingDirectory workingDir; + private SubWorkflowLoader loader; + + @BeforeEach + void setUp() throws Exception { + kotlinParser = mock(KotlinScriptParser.class); + workflowRepository = new InMemoryWorkflowRepository(); + workingDir = mock(WorkingDirectory.class); + loader = new SubWorkflowLoader(); + inject(loader, "kotlinParser", kotlinParser); + inject(loader, "workflowRepository", workflowRepository); + } + + @Test + void shouldSaveRootAndEveryDeclaredSubToRepository() { + Workflow root = parentWithSubRef("root", "sub-a", Map.of("draft", "draft"), Map.of()); + Workflow subA = childProducing("sub-a", "1.0.0", "draft", "draft"); + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + + assertThatCode(() -> loader.resolveDeclared(workingDir, root, List.of("sub-a"))) + .doesNotThrowAnyException(); + + assertThat(workflowRepository.findById(SubWorkflowLoader.CLI_TENANT, "root")) + .contains(root); + assertThat(workflowRepository.findById(SubWorkflowLoader.CLI_TENANT, "sub-a")) + .contains(subA); + } + + @Test + void shouldFailWhenParentReferencesSubNotSuppliedViaWith() { + Workflow root = parentWithSubRef("root", "sub-missing", Map.of(), Map.of()); + + assertThatThrownBy(() -> loader.resolveDeclared(workingDir, root, List.of())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("sub-missing") + .hasMessageContaining("not supplied via --with"); + } + + @Test + void shouldRejectDuplicateIdWithDivergentContent() { + Workflow root = parentWithSubRef("root", "sub-a", Map.of(), Map.of()); + Workflow subA = childProducing("sub-a", "1.0.0", "draft", "draft"); + // Same id + version, different schema/writes → different hash. + Workflow subADivergent = childProducing("sub-a", "1.0.0", "summary", "summary"); + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + when(kotlinParser.parse(eq(workingDir), eq("sub-a-dup"))).thenReturn(subADivergent); + + assertThatThrownBy( + () -> + loader.resolveDeclared( + workingDir, root, List.of("sub-a", "sub-a-dup"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("declared twice") + .hasMessageContaining("conflicting definitions"); + } + + @Test + void shouldAllowDuplicateIdWithIdenticalContent() { + Workflow root = parentWithSubRef("root", "sub-a", Map.of(), Map.of()); + Workflow subA = childProducing("sub-a", "1.0.0", "draft", "draft"); + // Same instance returned for both --with names → same serialized hash. + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + when(kotlinParser.parse(eq(workingDir), eq("sub-a-again"))).thenReturn(subA); + + assertThatCode( + () -> + loader.resolveDeclared( + workingDir, root, List.of("sub-a", "sub-a-again"))) + .doesNotThrowAnyException(); + } + + @Test + void shouldRejectTargetVersionPinMismatch() { + Workflow root = parentWithPinnedSubRef("root", "sub-a", "1.0.0", Map.of(), Map.of()); + Workflow subA = childProducing("sub-a", "2.0.0", "draft", "draft"); + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + + assertThatThrownBy(() -> loader.resolveDeclared(workingDir, root, List.of("sub-a"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("pins sub-workflow 'sub-a' to version '1.0.0'") + .hasMessageContaining("child declares '2.0.0'"); + } + + @Test + void shouldRejectImportsNotDeclaredInChildSchema() { + // Parent imports 'draft' but child's schema declares only 'summary'. + Workflow root = parentWithSubRef("root", "sub-a", Map.of("draft", "draft"), Map.of()); + Workflow subA = childProducing("sub-a", "1.0.0", "summary", "summary"); + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + + assertThatThrownBy(() -> loader.resolveDeclared(workingDir, root, List.of("sub-a"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("imports 'draft'") + .hasMessageContaining("child schema does not declare it"); + } + + @Test + void shouldRejectWritesDeclaredInChildSchemaButNotProducedByAnyChildNode() { + // Child schema declares 'tl_dr' but no node writes it — parent's expectation + // can never be satisfied at runtime. + Workflow root = parentWithSubRef("root", "sub-a", Map.of(), Map.of("tl_dr", "tl_dr")); + Workflow subA = childProducing("sub-a", "1.0.0", "tl_dr", /* writeVar */ null); + when(kotlinParser.parse(eq(workingDir), eq("sub-a"))).thenReturn(subA); + + assertThatThrownBy(() -> loader.resolveDeclared(workingDir, root, List.of("sub-a"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("expects sub-workflow 'sub-a' to write 'tl_dr'") + .hasMessageContaining("no node in the child writes that variable"); + } + + // ——— helpers ——— + + private static Workflow parentWithSubRef( + String id, + String targetWorkflowId, + Map inputMapping, + Map outputMapping) { + return buildParent(id, targetWorkflowId, null, inputMapping, outputMapping); + } + + private static Workflow parentWithPinnedSubRef( + String id, + String targetWorkflowId, + String pinnedVersion, + Map inputMapping, + Map outputMapping) { + return buildParent(id, targetWorkflowId, pinnedVersion, inputMapping, outputMapping); + } + + private static Workflow buildParent( + String id, + String targetWorkflowId, + String pinnedVersion, + Map inputMapping, + Map outputMapping) { + Map nodes = new LinkedHashMap<>(); + nodes.put( + "delegate", + SubWorkflowNode.builder() + .id("delegate") + .workflowId(targetWorkflowId) + .targetVersion(pinnedVersion) + .inputMapping(inputMapping) + .outputMapping(outputMapping) + .build()); + return Workflow.builder() + .id(id) + .version("1.0.0") + .startNode("delegate") + .nodes(nodes) + .build(); + } + + /// Builds a child workflow with {@code schemaVar} in its state schema. If {@code + /// writeVar} is non-null, a {@link StandardNode} that writes that variable is added + /// before the exit node; otherwise only the schema declares the variable. + private static Workflow childProducing( + String id, String version, String schemaVar, String writeVar) { + Map nodes = new LinkedHashMap<>(); + String startNode; + if (writeVar != null) { + nodes.put( + "produce", + StandardNode.builder() + .id("produce") + .agentId("agent-stub") + .prompt("produce " + writeVar) + .writes(List.of(writeVar)) + .transitionRules(List.of(new SuccessTransition("end"))) + .build()); + startNode = "produce"; + } else { + startNode = "end"; + } + nodes.put("end", EndNode.builder().id("end").status(ExitStatus.SUCCESS).build()); + return Workflow.builder() + .id(id) + .version(version) + .startNode(startNode) + .nodes(nodes) + .stateSchema( + new WorkflowStateSchema( + List.of( + new StateVariableDeclaration( + schemaVar, VarType.STRING, false)))) + .build(); + } + + private static void inject(Object target, String fieldName, Object value) throws Exception { + Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } +} diff --git a/hensu-core/README.md b/hensu-core/README.md index fb281f8..ba36843 100644 --- a/hensu-core/README.md +++ b/hensu-core/README.md @@ -31,7 +31,7 @@ flowchart TD end subgraph nodes["Node Executors"] direction LR - std(["Standard"]) ~~~ par(["Parallel"]) ~~~ fj(["Fork/Join"]) ~~~ loop(["Loop"]) ~~~ act(["Action"]) ~~~ gen(["Generic"]) + std(["Standard"]) ~~~ par(["Parallel"]) ~~~ fj(["Fork/Join"]) ~~~ loop(["Loop"]) ~~~ act(["Action"]) ~~~ gen(["Generic"]) ~~~ sub(["SubWorkflow"]) end subgraph mid["Infrastructure"] direction LR @@ -64,6 +64,7 @@ flowchart TD style loop fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px style act fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px style gen fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px + style sub fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px style af fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px style pe fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px style ae fill:#2c2c2e, stroke:#48484a, color:#ebebf5, stroke-width:1px @@ -342,6 +343,7 @@ hensu-core/src/main/java/io/hensu/core/ │ │ ├── StateVariableDeclaration.java # Variable declaration record (name, type, isInput) │ │ └── VarType.java # Type enum: STRING, NUMBER, BOOLEAN, LIST_STRING │ └── validation/ +│ ├── SubWorkflowGraphValidator.java # Load-time cycle + dangling-ref detection │ └── WorkflowValidator.java # Load-time validator for writes + prompt {variable} refs ├── rubric/ # Quality evaluation engine │ ├── RubricEngine.java # Evaluation orchestrator @@ -402,6 +404,8 @@ hensu-core/src/main/java/io/hensu/core/ | `SubWorkflowNode` | Delegates to another workflow | | `EndNode` | Terminal node | +**Sub-workflow constraints.** Nested execution is capped at depth 16 (`SubWorkflowNodeExecutor.MAX_DEPTH`) to prevent unbounded recursion. The executor propagates `_tenant_id` into the child context so multi-tenant isolation holds across the boundary. Reference graphs are validated at load time (CLI) and push time (server) by `SubWorkflowGraphValidator` – both cycles and dangling references are rejected before execution starts. + ## Transition Rules | Rule | Description | diff --git a/hensu-core/src/main/java/io/hensu/core/execution/executor/SubWorkflowNodeExecutor.java b/hensu-core/src/main/java/io/hensu/core/execution/executor/SubWorkflowNodeExecutor.java index 0627117..300e2e3 100644 --- a/hensu-core/src/main/java/io/hensu/core/execution/executor/SubWorkflowNodeExecutor.java +++ b/hensu-core/src/main/java/io/hensu/core/execution/executor/SubWorkflowNodeExecutor.java @@ -18,13 +18,25 @@ /// and maps outputs back to the parent state. /// /// @implNote Reads tenant ID from `_tenant_id` in the execution state context -/// for tenant-scoped workflow lookup. +/// for tenant-scoped workflow lookup, and propagates it into the child so deeper +/// recursion stays under the same tenant. Enforces a static recursion-depth cap +/// as defence-in-depth — load-time cycle validation +/// ({@link io.hensu.core.workflow.validation.SubWorkflowGraphValidator}) covers +/// the normal entry paths, but in-memory workflows pushed straight into the +/// repository would bypass it. /// /// @see WorkflowRepository for tenant-scoped workflow storage public class SubWorkflowNodeExecutor implements NodeExecutor { private static final Logger logger = Logger.getLogger(SubWorkflowNodeExecutor.class.getName()); + /// Hard cap on nested sub-workflow recursion. Cheap stack-depth guard for + /// any workflow that bypasses the load-time cycle validator. + public static final int MAX_DEPTH = 16; + + public static final String DEPTH_KEY = "_sub_workflow_depth"; + public static final String TENANT_KEY = "_tenant_id"; + @Override public Class getNodeType() { return SubWorkflowNode.class; @@ -37,11 +49,18 @@ public NodeResult execute(SubWorkflowNode node, ExecutionContext context) throws logger.info("Executing sub-workflow: " + node.getWorkflowId()); - // Load sub-workflow from repository using tenant ID from state context - String tenantId = (String) state.getContext().get("_tenant_id"); + int parentDepth = readDepth(state.getContext()); + if (parentDepth >= MAX_DEPTH) { + throw new IllegalStateException( + "Sub-workflow recursion depth exceeded " + + MAX_DEPTH + + " at: " + + node.getWorkflowId()); + } + + String tenantId = (String) state.getContext().get(TENANT_KEY); Workflow subWorkflow = loadSubWorkflow(node.getWorkflowId(), tenantId, context); - // Map input context Map subContext = new HashMap<>(); for (Map.Entry entry : node.getInputMapping().entrySet()) { String targetKey = entry.getKey(); @@ -53,10 +72,16 @@ public NodeResult execute(SubWorkflowNode node, ExecutionContext context) throws subContext.put(targetKey, value); } - // Execute sub-workflow - ExecutionResult subResult = workflowExecutor.execute(subWorkflow, subContext); + // Propagate engine vars so depth-2+ sub-workflows stay tenant-scoped + // and continue to enforce the recursion cap. + if (tenantId != null) { + subContext.put(TENANT_KEY, tenantId); + } + subContext.put(DEPTH_KEY, parentDepth + 1); + + ExecutionResult subResult = + workflowExecutor.execute(subWorkflow, subContext, context.getListener()); - // Map output back to parent state if (subResult instanceof ExecutionResult.Completed completed) { for (Map.Entry entry : node.getOutputMapping().entrySet()) { String targetKey = entry.getKey(); @@ -70,12 +95,26 @@ public NodeResult execute(SubWorkflowNode node, ExecutionContext context) throws return new NodeResult( ResultStatus.SUCCESS, completed.getFinalState().getContext(), metadata); - } else { - Map metadata = new HashMap<>(); - metadata.put("sub_workflow", node.getWorkflowId()); + } - return new NodeResult(ResultStatus.FAILURE, "Sub-workflow failed", metadata); + // Paused: child stopped for human review or async wait. Resume across the + // sub-workflow boundary is not yet implemented — fail loudly instead of + // silently masking the pause as a parent FAILURE. + if (subResult instanceof ExecutionResult.Paused) { + throw new UnsupportedOperationException( + "Sub-workflow '" + + node.getWorkflowId() + + "' paused; resume across sub-workflow boundary is not supported"); } + + Map metadata = new HashMap<>(); + metadata.put("sub_workflow", node.getWorkflowId()); + return new NodeResult(ResultStatus.FAILURE, "Sub-workflow failed", metadata); + } + + private static int readDepth(Map ctx) { + Object raw = ctx.get(DEPTH_KEY); + return raw instanceof Integer i ? i : 0; } private Workflow loadSubWorkflow(String workflowId, String tenantId, ExecutionContext context) { diff --git a/hensu-core/src/main/java/io/hensu/core/workflow/node/SubWorkflowNode.java b/hensu-core/src/main/java/io/hensu/core/workflow/node/SubWorkflowNode.java index 9f900d6..c0531b8 100644 --- a/hensu-core/src/main/java/io/hensu/core/workflow/node/SubWorkflowNode.java +++ b/hensu-core/src/main/java/io/hensu/core/workflow/node/SubWorkflowNode.java @@ -16,6 +16,7 @@ public final class SubWorkflowNode extends Node { private final NodeType nodeType = NodeType.SUB_WORKFLOW; private final String workflowId; + private final String targetVersion; private final Map inputMapping; private final Map outputMapping; private final List transitionRules; @@ -23,6 +24,7 @@ public final class SubWorkflowNode extends Node { private SubWorkflowNode(Builder builder) { super(builder.id); this.workflowId = builder.workflowId; + this.targetVersion = builder.targetVersion; this.inputMapping = builder.inputMapping != null ? Map.copyOf(builder.inputMapping) : Map.of(); this.outputMapping = @@ -45,9 +47,20 @@ public String getWorkflowId() { return workflowId; } + /// Returns the pinned version of the target workflow, or null if unpinned. + /// + /// Stored for forward compatibility with workflow versioning; not enforced at runtime yet. + /// + /// @return target workflow version, or null + public String getTargetVersion() { + return targetVersion; + } + /// Returns the input variable mapping from parent context to sub-workflow context. /// - /// Keys are parent context keys; values are sub-workflow context keys. + /// Keys are sub-workflow (child) context keys; values are parent context keys. + /// At execution time the executor reads `parent[value]` and writes it to + /// `child[key]`. /// /// @return unmodifiable mapping, never null (may be empty) public Map getInputMapping() { @@ -56,7 +69,9 @@ public Map getInputMapping() { /// Returns the output variable mapping from sub-workflow context back to parent context. /// - /// Keys are sub-workflow context keys; values are parent context keys. + /// Keys are parent context keys; values are sub-workflow (child) context keys. + /// At execution time the executor reads `child[value]` and writes it to + /// `parent[key]`. /// /// @return unmodifiable mapping, never null (may be empty) public Map getOutputMapping() { @@ -93,6 +108,7 @@ public String getRubricId() { public static final class Builder { private String id; private String workflowId; + private String targetVersion; private Map inputMapping; private Map outputMapping; private List transitionRules; @@ -117,6 +133,17 @@ public Builder workflowId(String workflowId) { return this; } + /// Sets the pinned target workflow version (optional). + /// + /// Stored for forward compatibility with workflow versioning; not enforced yet. + /// + /// @param targetVersion version string, may be null + /// @return this builder for chaining + public Builder targetVersion(String targetVersion) { + this.targetVersion = targetVersion; + return this; + } + /// Sets the input variable mapping from parent to sub-workflow context. /// /// @param inputMapping key mapping, may be null (treated as empty) @@ -166,6 +193,7 @@ public boolean equals(Object obj) { var that = (SubWorkflowNode) obj; return Objects.equals(this.id, that.id) && Objects.equals(this.workflowId, that.workflowId) + && Objects.equals(this.targetVersion, that.targetVersion) && Objects.equals(this.inputMapping, that.inputMapping) && Objects.equals(this.outputMapping, that.outputMapping) && Objects.equals(this.transitionRules, that.transitionRules); @@ -173,7 +201,8 @@ public boolean equals(Object obj) { @Override public int hashCode() { - return Objects.hash(id, workflowId, inputMapping, outputMapping, transitionRules); + return Objects.hash( + id, workflowId, targetVersion, inputMapping, outputMapping, transitionRules); } @Override @@ -185,6 +214,9 @@ public String toString() { + "workflowId=" + workflowId + ", " + + "targetVersion=" + + targetVersion + + ", " + "inputMapping=" + inputMapping + ", " diff --git a/hensu-core/src/main/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidator.java b/hensu-core/src/main/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidator.java new file mode 100644 index 0000000..b52b2bb --- /dev/null +++ b/hensu-core/src/main/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidator.java @@ -0,0 +1,164 @@ +package io.hensu.core.workflow.validation; + +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.SubWorkflowNode; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Function; + +/// Load-time validator for cross-workflow sub-workflow reference graphs. +/// +/// Builds a directed graph where each vertex is a workflow id and each edge is a +/// `SubWorkflowNode` reference, then rejects cycles via DFS with a recursion stack. +/// +/// ### Modes +/// - {@link #validate(Collection)} — CLI batch path: cycles only; unknown targets are +/// silently skipped because the loader reports missing-declaration errors separately. +/// - {@link #validate(Workflow, Function)} — server push path: cycles **and** unknown +/// referenced ids, in a single DFS pass (single repository lookup per id). +/// +/// ### Why load time +/// Without this check, a cycle like `a → b → a` would only surface at runtime inside +/// `SubWorkflowNodeExecutor`, potentially after the outer workflow has already executed +/// several nodes. Failing at load time gives the user a single, actionable error before +/// any side effects occur. +/// +/// @implNote **Thread-safe.** Stateless utility class; all traversal state is local. +/// +/// @see WorkflowValidator for single-workflow schema validation +public final class SubWorkflowGraphValidator { + + private SubWorkflowGraphValidator() {} + + /// Validates that the sub-workflow reference graph over the given workflows is acyclic. + /// + /// Used by the CLI loader which has the full set of compiled workflows in hand after + /// every `--with` parse completes. + /// + /// @param workflows every workflow in the loaded set (root + every declared sub), not null + /// @throws IllegalStateException if one or more cycles are found; the message lists every + /// distinct cycle in the form `a -> b -> a` + public static void validate(Collection workflows) { + Map byId = new HashMap<>(); + for (Workflow wf : workflows) { + byId.put(wf.getId(), wf); + } + Function resolver = byId::get; + + List> cycles = new ArrayList<>(); + Set globallyVisited = new HashSet<>(); + + for (String id : byId.keySet()) { + if (globallyVisited.contains(id)) continue; + LinkedHashSet stack = new LinkedHashSet<>(); + dfs(id, resolver, globallyVisited, stack, cycles, null); + } + + if (!cycles.isEmpty()) { + throw new IllegalStateException(cyclesMessage(cycles)); + } + } + + /// Validates that pushing {@code root} into an existing workflow set does not create a + /// cycle and that every referenced sub-workflow id can be resolved. + /// + /// Walks forward from {@code root} only, calling {@code resolver} lazily for each + /// referenced target. The invariant is that any cycle introduced by this push must pass + /// through {@code root} — any cycle not touching it existed before and should have been + /// caught earlier. + /// + /// The caller is expected to provide a resolver that shadows the stored version of + /// {@code root} with the incoming one, so re-push/update flows see the post-push graph + /// without touching the repository twice: + /// + /// ```java + /// validate(newWf, id -> id.equals(newWf.getId()) + /// ? newWf + /// : repo.findById(tenant, id).orElse(null)); + /// ``` + /// + /// @param root the incoming workflow about to be saved, not null + /// @param resolver lazy lookup of referenced workflow ids, not null; returns null for unknowns + /// @throws IllegalStateException if any referenced sub-workflow is missing or a cycle is found + public static void validate(Workflow root, Function resolver) { + Objects.requireNonNull(root, "root"); + Objects.requireNonNull(resolver, "resolver"); + + Function shadowed = + id -> id.equals(root.getId()) ? root : resolver.apply(id); + + List> cycles = new ArrayList<>(); + List missing = new ArrayList<>(); + Set globallyVisited = new HashSet<>(); + LinkedHashSet stack = new LinkedHashSet<>(); + dfs(root.getId(), shadowed, globallyVisited, stack, cycles, missing); + + List errors = new ArrayList<>(); + if (!missing.isEmpty()) { + errors.add( + "Workflow '" + + root.getId() + + "' references unknown sub-workflows: " + + String.join(", ", missing)); + } + if (!cycles.isEmpty()) { + errors.add(cyclesMessage(cycles)); + } + if (!errors.isEmpty()) { + throw new IllegalStateException(String.join("\n", errors)); + } + } + + private static String cyclesMessage(List> cycles) { + StringBuilder msg = new StringBuilder("Sub-workflow reference cycle detected:"); + for (List cycle : cycles) { + msg.append("\n - ").append(String.join(" -> ", cycle)); + } + return msg.toString(); + } + + private static void dfs( + String current, + Function resolver, + Set globallyVisited, + LinkedHashSet stack, + List> cycles, + List missing) { + if (stack.contains(current)) { + List cycle = new ArrayList<>(); + boolean collecting = false; + for (String id : stack) { + if (!collecting && id.equals(current)) collecting = true; + if (collecting) cycle.add(id); + } + cycle.add(current); + cycles.add(cycle); + return; + } + if (globallyVisited.contains(current)) return; + + Workflow wf = resolver.apply(current); + if (wf == null) { + if (missing != null) missing.add(current); + return; + } + + stack.add(current); + for (Node node : wf.getNodes().values()) { + if (!(node instanceof SubWorkflowNode sub)) continue; + String target = sub.getWorkflowId(); + if (target == null || target.isBlank()) continue; + dfs(target, resolver, globallyVisited, stack, cycles, missing); + } + stack.remove(current); + globallyVisited.add(current); + } +} diff --git a/hensu-core/src/main/java/io/hensu/core/workflow/validation/WorkflowValidator.java b/hensu-core/src/main/java/io/hensu/core/workflow/validation/WorkflowValidator.java index 7e315df..7e1882c 100644 --- a/hensu-core/src/main/java/io/hensu/core/workflow/validation/WorkflowValidator.java +++ b/hensu-core/src/main/java/io/hensu/core/workflow/validation/WorkflowValidator.java @@ -3,6 +3,7 @@ import io.hensu.core.workflow.Workflow; import io.hensu.core.workflow.node.Node; import io.hensu.core.workflow.node.StandardNode; +import io.hensu.core.workflow.node.SubWorkflowNode; import io.hensu.core.workflow.state.WorkflowStateSchema; import java.util.ArrayList; import java.util.List; @@ -52,7 +53,13 @@ public static void validate(Workflow workflow) { for (Map.Entry entry : workflow.getNodes().entrySet()) { String nodeId = entry.getKey(); - if (!(entry.getValue() instanceof StandardNode standardNode)) continue; + Node node = entry.getValue(); + + if (node instanceof SubWorkflowNode sub) { + validateSubWorkflow(nodeId, sub, schema, errors); + continue; + } + if (!(node instanceof StandardNode standardNode)) continue; for (String name : standardNode.getWrites()) { if (!schema.contains(name)) { @@ -89,4 +96,32 @@ public static void validate(Workflow workflow) { + String.join("\n", errors)); } } + + private static void validateSubWorkflow( + String nodeId, SubWorkflowNode sub, WorkflowStateSchema schema, List errors) { + if (sub.getWorkflowId() == null || sub.getWorkflowId().isBlank()) { + errors.add("Sub-workflow node '" + nodeId + "' has no target workflow id"); + } + // Same-name discipline: input/output maps are identity. Check declared in parent schema. + for (String name : sub.getInputMapping().keySet()) { + if (!schema.contains(name)) { + errors.add( + "Sub-workflow node '" + + nodeId + + "' imports '" + + name + + "' which is not declared in parent state schema"); + } + } + for (String name : sub.getOutputMapping().keySet()) { + if (!schema.contains(name)) { + errors.add( + "Sub-workflow node '" + + nodeId + + "' writes '" + + name + + "' which is not declared in parent state schema"); + } + } + } } diff --git a/hensu-core/src/test/java/io/hensu/core/execution/WorkflowExecutorSubWorkflowTest.java b/hensu-core/src/test/java/io/hensu/core/execution/WorkflowExecutorSubWorkflowTest.java new file mode 100644 index 0000000..863a6c2 --- /dev/null +++ b/hensu-core/src/test/java/io/hensu/core/execution/WorkflowExecutorSubWorkflowTest.java @@ -0,0 +1,403 @@ +package io.hensu.core.execution; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.hensu.core.agent.Agent; +import io.hensu.core.agent.AgentConfig; +import io.hensu.core.agent.AgentResponse; +import io.hensu.core.execution.executor.DefaultNodeExecutorRegistry; +import io.hensu.core.execution.executor.ExecutionContext; +import io.hensu.core.execution.executor.SubWorkflowNodeExecutor; +import io.hensu.core.execution.result.ExecutionResult; +import io.hensu.core.execution.result.ExitStatus; +import io.hensu.core.review.ReviewHandler; +import io.hensu.core.state.HensuState; +import io.hensu.core.template.SimpleTemplateResolver; +import io.hensu.core.workflow.InMemoryWorkflowRepository; +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.StandardNode; +import io.hensu.core.workflow.node.SubWorkflowNode; +import io.hensu.core.workflow.transition.SuccessTransition; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/// Tests for {@link io.hensu.core.execution.executor.SubWorkflowNodeExecutor}. +/// +/// Covers parent↔child data flow semantics: +/// inputMapping (childKey ← parentKey), outputMapping (parentKey ← childKey), +/// boundary filtering, multi-tenant lookup, and error surfaces. Scope-fenced: +/// does NOT duplicate DSL builder, validator, loader, or registry test coverage. +class WorkflowExecutorSubWorkflowTest extends WorkflowExecutorTestBase { + + private InMemoryWorkflowRepository repository; + + @BeforeEach + void setUpSubWorkflowRepository() { + repository = new InMemoryWorkflowRepository(); + executor = + new WorkflowExecutor( + new DefaultNodeExecutorRegistry(), + agentRegistry, + rubricEngine, + ReviewHandler.AUTO_APPROVE, + null, + new SimpleTemplateResolver(), + repository); + } + + @Test + void shouldMapInputsToChildAndReadOnlyMappedOutputsBack() throws Exception { + // Child writes both "summary" and "scratch"; parent maps only "summary" → "result". + // Asserts the boundary filter: unmapped child writes must NOT leak to parent. + var childAgent = mock(Agent.class); + when(agentRegistry.getAgent("child-agent")).thenReturn(Optional.of(childAgent)); + when(childAgent.execute(any(), any())) + .thenReturn( + AgentResponse.TextResponse.of( + "{\"summary\":\"distilled\",\"scratch\":\"draft\"}")); + repository.save( + "default", + buildChildWriting( + "child-summarizer", "child-agent", List.of("summary", "scratch"))); + + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("child-summarizer") + .inputMapping(Map.of("query", "topic")) + .outputMapping(Map.of("result", "summary")) + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + var ctx = new HashMap(); + ctx.put("topic", "AI workflows"); + + var result = executor.execute(buildParent(sub), ctx); + + assertThat(result).isInstanceOf(ExecutionResult.Completed.class); + var finalCtx = ((ExecutionResult.Completed) result).getFinalState().getContext(); + assertThat(finalCtx) + .containsEntry("result", "distilled") + .containsEntry("topic", "AI workflows"); + assertThat(finalCtx) + .doesNotContainKey("scratch") + .doesNotContainKey("summary") + .doesNotContainKey("query"); + } + + @Test + void shouldThrowWhenInputMappingSourceKeyMissingInParent() { + repository.save("default", buildEmptyChild("child-x")); + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("child-x") + .inputMapping(Map.of("query", "topic")) + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + assertThatThrownBy(() -> executor.execute(buildParent(sub), new HashMap<>())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Missing input") + .hasMessageContaining("topic"); + } + + @Test + void shouldResolveSubWorkflowUnderParentTenant() throws Exception { + // Parent state carries _tenant_id="acme"; child saved ONLY under "acme". + // Successful execution proves tenant routing in loadSubWorkflow. + repository.save("acme", buildEmptyChild("tenant-child")); + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("tenant-child") + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + var ctx = new HashMap(); + ctx.put("_tenant_id", "acme"); + + var result = executor.execute(buildParent(sub), ctx); + + assertThat(result).isInstanceOf(ExecutionResult.Completed.class); + assertThat(((ExecutionResult.Completed) result).getExitStatus()) + .isEqualTo(ExitStatus.SUCCESS); + } + + @Test + void shouldNotLeakGrandchildOutputThroughChildWithEmptyOutputMapping() throws Exception { + // Grandchild writes "deep_var"="leak-me". Child wraps grandchild with EMPTY + // outputMapping (filters). Parent asks for "deep_var" via outputMapping → + // resolves to null, NOT "leak-me". Proves the boundary filter applies at every + // hop, not just the root. + var grandchildAgent = mock(Agent.class); + when(agentRegistry.getAgent("gc-agent")).thenReturn(Optional.of(grandchildAgent)); + when(grandchildAgent.execute(any(), any())) + .thenReturn(AgentResponse.TextResponse.of("{\"deep_var\":\"leak-me\"}")); + + repository.save( + "default", buildChildWriting("grandchild", "gc-agent", List.of("deep_var"))); + + var childToGrandchild = + SubWorkflowNode.builder() + .id("sub-gc") + .workflowId("grandchild") + .transitionRules(List.of(new SuccessTransition("child-end"))) + .build(); + var nodes = new HashMap(); + nodes.put("sub-gc", childToGrandchild); + nodes.put("child-end", end("child-end")); + var childWf = + Workflow.builder() + .id("child") + .agents(Map.of()) + .nodes(nodes) + .startNode("sub-gc") + .build(); + repository.save("default", childWf); + + var parentSub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("child") + .outputMapping(Map.of("echo", "deep_var")) + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + var result = executor.execute(buildParent(parentSub), new HashMap<>()); + + var finalCtx = ((ExecutionResult.Completed) result).getFinalState().getContext(); + // Key is present (outputMapping always puts) but value is null — + // grandchild's "leak-me" did not pierce the child boundary. + assertThat(finalCtx).containsKey("echo"); + assertThat(finalCtx.get("echo")).isNull(); + } + + @Test + void shouldOverwriteParentContextOnOutputKeyCollision() throws Exception { + var childAgent = mock(Agent.class); + when(agentRegistry.getAgent("c-agent")).thenReturn(Optional.of(childAgent)); + when(childAgent.execute(any(), any())) + .thenReturn(AgentResponse.TextResponse.of("{\"summary\":\"ChildValue\"}")); + repository.save("default", buildChildWriting("collide", "c-agent", List.of("summary"))); + + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("collide") + .outputMapping(Map.of("topic", "summary")) + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + var ctx = new HashMap(); + ctx.put("topic", "ParentValue"); + + var result = executor.execute(buildParent(sub), ctx); + + assertThat(((ExecutionResult.Completed) result).getFinalState().getContext()) + .containsEntry("topic", "ChildValue"); + } + + @Test + void shouldThrowWhenSubWorkflowNotInRepository() { + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("ghost-workflow") + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + assertThatThrownBy(() -> executor.execute(buildParent(sub), new HashMap<>())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Sub-workflow not found") + .hasMessageContaining("ghost-workflow"); + } + + @Test + void shouldPropagateTenantThroughDepthTwoSubWorkflows() throws Exception { + // grandchild + intermediate child are both stored ONLY under tenant "acme". + // If _tenant_id were not propagated into the depth-1 subContext, the + // depth-2 lookup would fall through to "default" and throw "not found". + // Successful Completed proves the engine var rides the recursion. + var gcAgent = mock(Agent.class); + when(agentRegistry.getAgent("gc-agent")).thenReturn(Optional.of(gcAgent)); + when(gcAgent.execute(any(), any())) + .thenReturn(AgentResponse.TextResponse.of("{\"deep_var\":\"v\"}")); + repository.save("acme", buildChildWriting("grandchild", "gc-agent", List.of("deep_var"))); + + var middleSub = + SubWorkflowNode.builder() + .id("sub-gc") + .workflowId("grandchild") + .transitionRules(List.of(new SuccessTransition("child-end"))) + .build(); + var middleNodes = new HashMap(); + middleNodes.put("sub-gc", middleSub); + middleNodes.put("child-end", end("child-end")); + repository.save( + "acme", + Workflow.builder() + .id("middle") + .agents(Map.of()) + .nodes(middleNodes) + .startNode("sub-gc") + .build()); + + var topSub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("middle") + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + var ctx = new HashMap(); + ctx.put("_tenant_id", "acme"); + + var result = executor.execute(buildParent(topSub), ctx); + + assertThat(result).isInstanceOf(ExecutionResult.Completed.class); + } + + @Test + void shouldThrowWhenRecursionDepthExceedsCap() { + // Pre-seed _sub_workflow_depth at the cap so the very first sub-workflow + // entry trips the guard. Proves the static stack-depth fence catches + // any cycle that bypasses the load-time validator. + repository.save("default", buildEmptyChild("any-child")); + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("any-child") + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + var ctx = new HashMap(); + ctx.put(SubWorkflowNodeExecutor.DEPTH_KEY, SubWorkflowNodeExecutor.MAX_DEPTH); + + assertThatThrownBy(() -> executor.execute(buildParent(sub), ctx)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("recursion depth") + .hasMessageContaining("any-child"); + } + + @Test + void shouldThrowWhenChildExecutionPauses() throws Exception { + // Drives SubWorkflowNodeExecutor directly with a mocked WorkflowExecutor + // so the child returns Paused without standing up the review pipeline. + // Asserts we surface the unsupported combination loudly instead of + // masking it as a parent FAILURE (the original silent-swallow bug). + repository.save("default", buildEmptyChild("paused-child")); + + var pausedChildState = + new HensuState.Builder() + .executionId("child-exec") + .workflowId("paused-child") + .currentNode("end") + .context(new HashMap<>()) + .build(); + var pausingExecutor = mock(WorkflowExecutor.class); + when(pausingExecutor.execute(any(Workflow.class), any(Map.class), any())) + .thenReturn(new ExecutionResult.Paused(pausedChildState)); + + var parentState = + new HensuState.Builder() + .executionId("parent-exec") + .workflowId("parent") + .currentNode("sub") + .context(new HashMap<>()) + .build(); + var sub = SubWorkflowNode.builder().id("sub").workflowId("paused-child").build(); + + var ctx = + ExecutionContext.builder() + .state(parentState) + .workflow(buildParent(sub)) + .workflowExecutor(pausingExecutor) + .workflowRepository(repository) + .build(); + + assertThatThrownBy(() -> new SubWorkflowNodeExecutor().execute(sub, ctx)) + .isInstanceOf(UnsupportedOperationException.class) + .hasMessageContaining("paused-child") + .hasMessageContaining("paused"); + } + + @Test + void shouldThrowWhenWorkflowRepositoryIsNull() { + // Re-build executor without a repository (4-arg ctor leaves it null). + executor = + new WorkflowExecutor( + new DefaultNodeExecutorRegistry(), + agentRegistry, + rubricEngine, + ReviewHandler.AUTO_APPROVE); + + var sub = + SubWorkflowNode.builder() + .id("sub") + .workflowId("any-child") + .transitionRules(List.of(new SuccessTransition("end"))) + .build(); + + assertThatThrownBy(() -> executor.execute(buildParent(sub), new HashMap<>())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("No WorkflowRepository configured"); + } + + // — Helpers ———————————————————————————————————————————————————————————— + + private Workflow buildParent(SubWorkflowNode sub) { + var nodes = new HashMap(); + nodes.put("sub", sub); + nodes.put("end", end("end")); + return Workflow.builder() + .id("parent-" + sub.getWorkflowId()) + .agents(Map.of()) + .nodes(nodes) + .startNode("sub") + .build(); + } + + private Workflow buildChildWriting(String id, String agentId, List writes) { + var nodes = new HashMap(); + nodes.put( + "step", + StandardNode.builder() + .id("step") + .agentId(agentId) + .prompt("Process") + .writes(writes) + .transitionRules(List.of(new SuccessTransition("end"))) + .build()); + nodes.put("end", end("end")); + return Workflow.builder() + .id(id) + .agents( + Map.of( + agentId, + AgentConfig.builder() + .id(agentId) + .role("Worker") + .model("test") + .build())) + .nodes(nodes) + .startNode("step") + .build(); + } + + private Workflow buildEmptyChild(String id) { + return Workflow.builder() + .id(id) + .agents(Map.of()) + .nodes(Map.of("end", end("end"))) + .startNode("end") + .build(); + } +} diff --git a/hensu-core/src/test/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidatorTest.java b/hensu-core/src/test/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidatorTest.java new file mode 100644 index 0000000..573edad --- /dev/null +++ b/hensu-core/src/test/java/io/hensu/core/workflow/validation/SubWorkflowGraphValidatorTest.java @@ -0,0 +1,184 @@ +package io.hensu.core.workflow.validation; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.hensu.core.execution.result.ExitStatus; +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.node.EndNode; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.SubWorkflowNode; +import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import org.junit.jupiter.api.Test; + +/// Contract tests for {@link SubWorkflowGraphValidator}. +/// +/// The validator runs at load time over the full set of workflows the CLI loader (or any +/// future bundle loader) has compiled — root plus every `--with` declared sub. Its job is +/// to reject parent→child reference cycles before execution starts so the runtime executor +/// never blows the stack on infinite recursion. +/// +/// Contract: +/// - passes silently when the reference graph is acyclic +/// - throws {@link IllegalStateException} listing every distinct cycle when any are found +/// - `validate(Collection)` silently skips targets not present in the input — the CLI loader +/// reports missing `--with` declarations separately +/// - `validate(Workflow, Function)` reports unresolved references alongside cycles (server +/// push path) +class SubWorkflowGraphValidatorTest { + + @Test + void diamondPasses() { + Workflow a = wf("a", List.of("b", "c")); + Workflow b = wf("b", List.of("d")); + Workflow c = wf("c", List.of("d")); + Workflow d = wf("d", List.of()); + assertThatCode(() -> SubWorkflowGraphValidator.validate(List.of(a, b, c, d))) + .doesNotThrowAnyException(); + } + + @Test + void unknownTargetIsIgnored() { + Workflow a = wf("a", List.of("ghost")); + assertThatCode(() -> SubWorkflowGraphValidator.validate(List.of(a))) + .doesNotThrowAnyException(); + } + + @Test + void selfCycleIsRejected() { + Workflow a = wf("a", List.of("a")); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(List.of(a))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("cycle") + .hasMessageContaining("a"); + } + + @Test + void twoNodeCycleIsRejected() { + Workflow a = wf("a", List.of("b")); + Workflow b = wf("b", List.of("a")); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(List.of(a, b))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("cycle") + .hasMessageContaining("a") + .hasMessageContaining("b"); + } + + @Test + void disconnectedCycleIsRejected() { + // Root 'main' is clean; an isolated 'x' ↔ 'y' cycle still blocks the load. + Workflow main = wf("main", List.of()); + Workflow x = wf("x", List.of("y")); + Workflow y = wf("y", List.of("x")); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(List.of(main, x, y))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("x") + .hasMessageContaining("y"); + } + + @Test + void multipleDistinctCyclesAreAllReported() { + // a ↔ b and c ↔ d — both must show up in the error message. + Workflow a = wf("a", List.of("b")); + Workflow b = wf("b", List.of("a")); + Workflow c = wf("c", List.of("d")); + Workflow d = wf("d", List.of("c")); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(List.of(a, b, c, d))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("a") + .hasMessageContaining("b") + .hasMessageContaining("c") + .hasMessageContaining("d"); + } + + // — Resolver overload — simulates the server push path —————————————————— + + @Test + void pushingWorkflowWithUnknownReferenceIsRejected() { + // Pushing A→ghost-b when the repo has no ghost-b must fail at push time, not at + // runtime. Otherwise, the server stores a workflow with a dangling reference that + // only blows up when an execution reaches the SubWorkflowNode. + Workflow incoming = wf("a", List.of("ghost-b")); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(incoming, _ -> null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("unknown sub-workflows") + .hasMessageContaining("ghost-b"); + } + + @Test + void pushingWorkflowThatClosesCycleIsRejected() { + // Repo already has A→B. User pushes B→A — the new push is what closes the loop. + Workflow incoming = wf("b", List.of("a")); + Map repo = new HashMap<>(); + repo.put("a", wf("a", List.of("b"))); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(incoming, repo::get)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("a") + .hasMessageContaining("b"); + } + + @Test + void rePushShadowsStoredVersion() { + // Repo has a stale B with no outgoing refs. Repo also has A→B. User re-pushes B + // with B→A. The validator must see the INCOMING B, not the stored one — otherwise + // the cycle would be missed. + Workflow staleB = wf("b", List.of()); + Workflow newB = wf("b", List.of("a")); + Map repo = new HashMap<>(); + repo.put("a", wf("a", List.of("b"))); + repo.put("b", staleB); + assertThatThrownBy(() -> SubWorkflowGraphValidator.validate(newB, repo::get)) + .isInstanceOf(IllegalStateException.class); + } + + @Test + void resolverIsOnlyCalledForReachableIds() { + // Repo has unrelated noise (n1, n2, n3). Push A→B→C. Walk must fetch only B and C, + // never the unrelated ids and never 'a' itself (shadowed by the incoming). + Workflow incoming = wf("a", List.of("b")); + Map repo = new HashMap<>(); + repo.put("b", wf("b", List.of("c"))); + repo.put("c", wf("c", List.of())); + repo.put("n1", wf("n1", List.of("n2"))); + repo.put("n2", wf("n2", List.of("n3"))); + repo.put("n3", wf("n3", List.of())); + Set touched = new HashSet<>(); + Function tracking = + id -> { + touched.add(id); + return repo.get(id); + }; + SubWorkflowGraphValidator.validate(incoming, tracking); + // 'a' is shadowed and never queried. 'n1/n2/n3' are unrelated and must not be + // touched. + assertThat(touched).doesNotContain("a", "n1", "n2", "n3"); + assertThat(touched).contains("b", "c"); + } + + // — helpers —————————————————————————————————————————————————————————————— + + /// Builds a minimal Workflow with one SubWorkflowNode per declared target. + /// If `targets` is empty, a single {@link EndNode} is added instead so the workflow has + /// no outgoing sub-workflow references (a true leaf). + private static Workflow wf(String id, List targets) { + Map nodes = new LinkedHashMap<>(); + if (targets.isEmpty()) { + nodes.put("end", EndNode.builder().id("end").status(ExitStatus.SUCCESS).build()); + return Workflow.builder().id(id).nodes(nodes).startNode("end").build(); + } + for (int i = 0; i < targets.size(); i++) { + String nodeId = "sub_" + i; + nodes.put( + nodeId, + SubWorkflowNode.builder().id(nodeId).workflowId(targets.get(i)).build()); + } + return Workflow.builder().id(id).nodes(nodes).startNode("sub_0").build(); + } +} diff --git a/hensu-core/src/test/java/io/hensu/core/workflow/validation/WorkflowValidatorSubWorkflowTest.java b/hensu-core/src/test/java/io/hensu/core/workflow/validation/WorkflowValidatorSubWorkflowTest.java new file mode 100644 index 0000000..e42da61 --- /dev/null +++ b/hensu-core/src/test/java/io/hensu/core/workflow/validation/WorkflowValidatorSubWorkflowTest.java @@ -0,0 +1,90 @@ +package io.hensu.core.workflow.validation; + +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.SubWorkflowNode; +import io.hensu.core.workflow.state.StateVariableDeclaration; +import io.hensu.core.workflow.state.VarType; +import io.hensu.core.workflow.state.WorkflowStateSchema; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/// Schema-consistency tests for the {@code SubWorkflowNode} branch of +/// {@link WorkflowValidator#validate(Workflow)}. +/// +/// Standard-node and schema-absent branches are exercised indirectly through +/// {@code WorkflowBuilderTest}; this suite targets only the sub-workflow specific +/// imports/writes checks introduced with the sub-workflow DSL. +class WorkflowValidatorSubWorkflowTest { + + @Test + void shouldPassWhenSubWorkflowImportsAndWritesAreDeclared() { + Workflow workflow = + workflowWithSchema( + List.of( + new StateVariableDeclaration("draft", VarType.STRING, true), + new StateVariableDeclaration("tl_dr", VarType.STRING, false)), + subWorkflowNode(Map.of("draft", "draft"), Map.of("tl_dr", "tl_dr"))); + + assertThatCode(() -> WorkflowValidator.validate(workflow)).doesNotThrowAnyException(); + } + + @Test + void shouldRejectImportsNotDeclaredInParentSchema() { + // Parent schema declares only 'tl_dr'. Importing 'draft' would silently copy a + // non-existent variable into the child, giving it an empty value with no warning. + Workflow workflow = + workflowWithSchema( + List.of(new StateVariableDeclaration("tl_dr", VarType.STRING, false)), + subWorkflowNode(Map.of("draft", "draft"), Map.of("tl_dr", "tl_dr"))); + + assertThatThrownBy(() -> WorkflowValidator.validate(workflow)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("delegate") + .hasMessageContaining("imports 'draft'") + .hasMessageContaining("not declared in parent state schema"); + } + + @Test + void shouldRejectWritesNotDeclaredInParentSchema() { + // Parent schema declares only 'draft'. Child writes 'tl_dr' would land in the + // parent state under an undeclared key — silently lost or, worse, colliding. + Workflow workflow = + workflowWithSchema( + List.of(new StateVariableDeclaration("draft", VarType.STRING, true)), + subWorkflowNode(Map.of("draft", "draft"), Map.of("tl_dr", "tl_dr"))); + + assertThatThrownBy(() -> WorkflowValidator.validate(workflow)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("delegate") + .hasMessageContaining("writes 'tl_dr'") + .hasMessageContaining("not declared in parent state schema"); + } + + private static Workflow workflowWithSchema( + List variables, SubWorkflowNode node) { + Map nodes = new LinkedHashMap<>(); + nodes.put(node.getId(), node); + return Workflow.builder() + .id("parent") + .startNode(node.getId()) + .nodes(nodes) + .stateSchema(new WorkflowStateSchema(variables)) + .build(); + } + + private static SubWorkflowNode subWorkflowNode( + Map inputs, Map outputs) { + return SubWorkflowNode.builder() + .id("delegate") + .workflowId("sub-target") + .inputMapping(inputs) + .outputMapping(outputs) + .build(); + } +} diff --git a/hensu-dsl/README.md b/hensu-dsl/README.md index d06add6..249761f 100644 --- a/hensu-dsl/README.md +++ b/hensu-dsl/README.md @@ -88,7 +88,7 @@ See [DSL Reference](../docs/dsl-reference.md) for the complete syntax reference - Workflow structure and configuration - Agent definitions and model constants -- Node types (standard, parallel, fork/join, action, generic, end) +- Node types (standard, parallel, fork/join, action, generic, sub-workflow, end) - Transition rules (success, failure, score-based, approval, consensus) - State variables (`writes()` + `{placeholder}` syntax), branch outputs (`yields()`), join boundary filter (`exports()`) - State schema (`state { }` block with typed `input`/`variable` declarations and load-time validation) @@ -115,6 +115,7 @@ hensu-dsl/src/main/kotlin/io/hensu/dsl/ │ ├── ForkJoinBuilders.kt # Fork/join node builders │ ├── ActionNodeBuilder.kt # Action node builder │ ├── GenericNodeBuilder.kt # Custom node type builder +│ ├── SubWorkflowNodeBuilder.kt # Sub-workflow delegation builder │ ├── EndNodeBuilder.kt # Terminal node builder │ ├── BaseNodeBuilder.kt # Shared node builder base │ ├── StateSchemaBuilder.kt # Typed state schema builder (`state { }` block) diff --git a/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/BaseNodeBuilder.kt b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/BaseNodeBuilder.kt index 3ab115e..612268e 100644 --- a/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/BaseNodeBuilder.kt +++ b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/BaseNodeBuilder.kt @@ -7,7 +7,7 @@ import io.hensu.core.workflow.node.Node * * Sealed to ensure all node types are known at compile time. Implementations: * [StandardNodeBuilder], [EndNodeBuilder], [ParallelNodeBuilder], [GenericNodeBuilder], - * [ForkNodeBuilder], [JoinNodeBuilder], [ActionNodeBuilder] + * [ForkNodeBuilder], [JoinNodeBuilder], [ActionNodeBuilder], [SubWorkflowNodeBuilder] */ sealed interface BaseNodeBuilder { fun build(): Node diff --git a/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/GraphBuilder.kt b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/GraphBuilder.kt index 9f12301..ee1fb0b 100644 --- a/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/GraphBuilder.kt +++ b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/GraphBuilder.kt @@ -186,6 +186,35 @@ class GraphBuilder(private val workingDirectory: WorkingDirectory) { nodeBuilders[id] = EndNodeBuilder(id, status) } + /** + * Defines a sub-workflow delegation node. + * + * The node invokes a nested workflow identified by `target`. Parent state vars named in + * `imports` are copied to the child under the same name; the child's `writes` are mirrored back + * to the parent under the same name. + * + * Usage: + * ```kotlin + * subWorkflow("delegate_summary") { + * target = "sub-summarizer" + * targetVersion = "1.0.0" + * imports("draft") + * writes("tl_dr") + * onSuccess goto "publish" + * onFailure goto "publish" + * } + * ``` + * + * @param id unique node identifier + * @param block sub-workflow configuration block + * @see SubWorkflowNodeBuilder for configuration options + */ + fun subWorkflow(id: String, block: SubWorkflowNodeBuilder.() -> Unit) { + val builder = SubWorkflowNodeBuilder(id) + builder.apply(block) + nodeBuilders[id] = builder + } + /** * Define an action node for executing commands mid-workflow. * diff --git a/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilder.kt b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilder.kt new file mode 100644 index 0000000..4e37c80 --- /dev/null +++ b/hensu-dsl/src/main/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilder.kt @@ -0,0 +1,115 @@ +package io.hensu.dsl.builders + +import io.hensu.core.execution.EngineVariables +import io.hensu.core.workflow.node.SubWorkflowNode + +/** + * DSL builder for sub-workflow delegation nodes. + * + * A sub-workflow node delegates execution to a nested workflow identified by [target]. State + * variables named in [imports] are copied from parent to child under the same name; variables named + * in [writes] are mirrored back from child to parent under the same name. Same-name discipline: the + * child's state schema is the contract; parents adapt. + * + * Example: + * ```kotlin + * subWorkflow("delegate_summary") { + * target = "sub-summarizer" + * targetVersion = "1.0.0" // optional, forward-compat with workflow versioning + * imports("draft") + * writes("tl_dr") + * onSuccess goto "publish" + * onFailure goto "publish" + * } + * ``` + * + * @property id unique node identifier within the parent workflow + * @see io.hensu.core.workflow.node.SubWorkflowNode for the compiled node + */ +@WorkflowDsl +class SubWorkflowNodeBuilder(private val id: String) : BaseNodeBuilder, TransitionMarkers { + /** Required: id of the child workflow to invoke. */ + var target: String? = null + + /** + * Optional pinned version of the child workflow. + * + * Stored on the compiled [SubWorkflowNode] and round-tripped through serialization for forward + * compatibility with the workflow-versioning feature. Not enforced at runtime yet. + */ + var targetVersion: String? = null + + private var imports: List = emptyList() + private var writes: List = emptyList() + private val transitionBuilder = TransitionBuilder() + + /** + * Declares parent state variables to copy into the child workflow under the same name. + * + * Each name must be declared in the parent workflow's `state {}` block, must not be an engine + * variable, and must not overlap with [writes]. + */ + fun imports(vararg names: String) { + for (name in names) { + require(name.isNotBlank()) { "subWorkflow('$id'): imports name must not be blank" } + require(!EngineVariables.isEngineVar(name)) { + "subWorkflow('$id'): imports field '$name' is a reserved engine variable." + } + } + require(names.toSet().size == names.size) { + "subWorkflow('$id'): duplicate names in imports(${names.joinToString()})" + } + imports = names.toList() + } + + /** + * Declares state variables the child workflow writes, mirrored back into the parent state. + * + * Each name must be declared in the parent's `state {}` block, must not be an engine variable, + * and must not overlap with [imports]. + */ + fun writes(vararg names: String) { + for (name in names) { + require(name.isNotBlank()) { "subWorkflow('$id'): writes name must not be blank" } + require(!EngineVariables.isEngineVar(name)) { + "subWorkflow('$id'): writes field '$name' is a reserved engine variable." + } + } + require(names.toSet().size == names.size) { + "subWorkflow('$id'): duplicate names in writes(${names.joinToString()})" + } + writes = names.toList() + } + + /** Define transition on success. Usage: `onSuccess goto "next_node"` */ + infix fun onSuccess.goto(targetNode: String) { + transitionBuilder.addSuccessTransition(targetNode) + } + + /** Define transition on failure. Usage: `onFailure goto "error_node"` */ + infix fun onFailure.goto(targetNode: String) { + transitionBuilder.addFailureTransition(targetNode) + } + + override fun build(): SubWorkflowNode { + val resolvedTarget = + target ?: throw IllegalStateException("subWorkflow('$id'): target is required") + require(resolvedTarget.isNotBlank()) { "subWorkflow('$id'): target must not be blank" } + val overlap = imports.toSet().intersect(writes.toSet()) + require(overlap.isEmpty()) { + "subWorkflow('$id'): imports and writes must not overlap: $overlap" + } + // Same-name discipline: identity maps both directions. + val inputMapping = imports.associateWith { it } + val outputMapping = writes.associateWith { it } + + return SubWorkflowNode.builder() + .id(id) + .workflowId(resolvedTarget) + .targetVersion(targetVersion) + .inputMapping(inputMapping) + .outputMapping(outputMapping) + .transitionRules(transitionBuilder.build()) + .build() + } +} diff --git a/hensu-dsl/src/test/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilderTest.kt b/hensu-dsl/src/test/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilderTest.kt new file mode 100644 index 0000000..127ad5d --- /dev/null +++ b/hensu-dsl/src/test/kotlin/io/hensu/dsl/builders/SubWorkflowNodeBuilderTest.kt @@ -0,0 +1,72 @@ +package io.hensu.dsl.builders + +import io.hensu.core.workflow.node.NodeType +import io.hensu.core.workflow.transition.FailureTransition +import io.hensu.core.workflow.transition.SuccessTransition +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.assertThatThrownBy +import org.junit.jupiter.api.Test + +class SubWorkflowNodeBuilderTest { + + @Test + fun `should build node with identity mappings and round-trip all optional fields`() { + val builder = SubWorkflowNodeBuilder("delegate_summary") + builder.apply { + target = "sub-summarizer" + targetVersion = "1.0.0" + imports("draft") + writes("tl_dr") + onSuccess goto "publish" + onFailure goto "fallback" + } + + val node = builder.build() + + assertThat(node.id).isEqualTo("delegate_summary") + assertThat(node.nodeType).isEqualTo(NodeType.SUB_WORKFLOW) + assertThat(node.workflowId).isEqualTo("sub-summarizer") + assertThat(node.targetVersion).isEqualTo("1.0.0") + // Same-name discipline: both mappings must be identity. Any future "optimization" + // that turns `associateWith { it }` into an asymmetric or empty map breaks the + // parent↔child variable contract silently. + assertThat(node.inputMapping).containsExactlyEntriesOf(mapOf("draft" to "draft")) + assertThat(node.outputMapping).containsExactlyEntriesOf(mapOf("tl_dr" to "tl_dr")) + assertThat(node.transitionRules).hasAtLeastOneElementOfType(SuccessTransition::class.java) + assertThat(node.transitionRules).hasAtLeastOneElementOfType(FailureTransition::class.java) + } + + @Test + fun `should fail fast at DSL level when target is missing`() { + val builder = SubWorkflowNodeBuilder("delegate_summary") + + // Without the DSL guard the user sees the core builder's generic "workflowId is + // required" message far from the subWorkflow{} block, which is exactly what this + // contextual message prevents. + assertThatThrownBy { builder.build() } + .isInstanceOf(IllegalStateException::class.java) + .hasMessageContaining("target is required") + } + + @Test + fun `should reject engine variable name in imports`() { + val builder = SubWorkflowNodeBuilder("delegate_summary") + + // Importing 'score' would pre-populate the child's consensus-scoring variable + // from the parent, corrupting any downstream ScoreTransition or consensus gate. + assertThatThrownBy { builder.imports("score") } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("reserved engine variable") + } + + @Test + fun `should reject engine variable name in writes`() { + val builder = SubWorkflowNodeBuilder("delegate_summary") + + // Mirror-back direction: child's 'approved' must not leak into parent, where + // it would drive an ApprovalTransition the parent never declared. + assertThatThrownBy { builder.writes("approved") } + .isInstanceOf(IllegalArgumentException::class.java) + .hasMessageContaining("reserved engine variable") + } +} diff --git a/hensu-serialization/src/main/java/io/hensu/serialization/NodeDeserializer.java b/hensu-serialization/src/main/java/io/hensu/serialization/NodeDeserializer.java index a9646ae..70f23be 100644 --- a/hensu-serialization/src/main/java/io/hensu/serialization/NodeDeserializer.java +++ b/hensu-serialization/src/main/java/io/hensu/serialization/NodeDeserializer.java @@ -223,6 +223,7 @@ private SubWorkflowNode deserializeSubWorkflow(ObjectMapper mapper, JsonNode roo return SubWorkflowNode.builder() .id(id) .workflowId(root.get("workflowId").asText()) + .targetVersion(textOrNull(root, "targetVersion")) .inputMapping( root.has("inputMapping") ? mapper.convertValue(root.get("inputMapping"), STRING_MAP) diff --git a/hensu-serialization/src/main/java/io/hensu/serialization/NodeSerializer.java b/hensu-serialization/src/main/java/io/hensu/serialization/NodeSerializer.java index 232e0c2..eb76f31 100644 --- a/hensu-serialization/src/main/java/io/hensu/serialization/NodeSerializer.java +++ b/hensu-serialization/src/main/java/io/hensu/serialization/NodeSerializer.java @@ -152,6 +152,7 @@ private void writeJoinNode(JoinNode n, JsonGenerator gen, SerializerProvider pro private void writeSubWorkflowNode( SubWorkflowNode n, JsonGenerator gen, SerializerProvider provider) throws IOException { gen.writeStringField("workflowId", n.getWorkflowId()); + writeIfNotNull(gen, "targetVersion", n.getTargetVersion()); provider.defaultSerializeField("inputMapping", n.getInputMapping(), gen); provider.defaultSerializeField("outputMapping", n.getOutputMapping(), gen); provider.defaultSerializeField("transitionRules", n.getTransitionRules(), gen); diff --git a/hensu-server/README.md b/hensu-server/README.md index 2a372e8..856c6f1 100644 --- a/hensu-server/README.md +++ b/hensu-server/README.md @@ -144,12 +144,17 @@ is disabled and a default tenant is used (see [Local Development](#local-develop Terraform/kubectl-style operations for managing workflow definitions (CLI integration). -| Method | Path | Description | -|----------|----------------------------------|----------------------------------| -| `POST` | `/api/v1/workflows` | Push workflow (create or update) | -| `GET` | `/api/v1/workflows` | List all workflows for tenant | -| `GET` | `/api/v1/workflows/{workflowId}` | Pull workflow definition | -| `DELETE` | `/api/v1/workflows/{workflowId}` | Delete workflow | +| Method | Path | Description | +|----------|----------------------------------|---------------------------------------------------------------| +| `POST` | `/api/v1/workflows` | Push workflow (create or update; validates sub-workflow refs) | +| `GET` | `/api/v1/workflows` | List all workflows for tenant | +| `GET` | `/api/v1/workflows/{workflowId}` | Pull workflow definition | +| `DELETE` | `/api/v1/workflows/{workflowId}` | Delete workflow | + +Push is serialized cluster-wide by `WorkflowPushLock` (pg_advisory_xact_lock with JVM +fallback) and runs `SubWorkflowGraphValidator` over the forward-reachable sub-workflow +graph — rejecting cycles and dangling `target` references before any row is written. +See [Sub-Workflows](#sub-workflows) below. ### Execution Operations @@ -306,6 +311,27 @@ when a DataSource is available, otherwise falls back to in-memory. Repositories - **`inmem` profile**: `quarkus.datasource.active=false` disables PostgreSQL; `quarkus.scheduler.enabled=false` disables all lease jobs for in-memory-only operation +### Sub-Workflows + +`SubWorkflowNode` lets a parent workflow delegate to another workflow by id. The server +adds three guarantees on top of core execution: + +- **Push-time validation** (`WorkflowRegistryService` → `SubWorkflowGraphValidator`): a single + DFS walks the forward-reachable sub-workflow graph from the incoming workflow, rejecting + cycles and dangling `target` references. Unresolved ids are looked up lazily via + `WorkflowRepository.findById(tenant, id)`, so validation always reflects the post-push + graph without an intermediate write. +- **Cluster-wide serialization** (`WorkflowPushLock`): push acquires `pg_advisory_xact_lock` + (JVM-lock fallback when the datasource is inactive) so two concurrent nodes cannot each + observe a clean graph and together introduce a cycle. +- **Runtime isolation**: `_tenant_id` is propagated into the child context and child lookup + uses the tenant-scoped `WorkflowRepository`, so a workflow can never resolve a child from + a different tenant. Recursion depth is bounded by `SubWorkflowNodeExecutor.MAX_DEPTH = 16` + (tracked via `_sub_workflow_depth`). + +See [Sub-Workflow Validation on Push](../docs/developer-guide-server.md#sub-workflow-validation-on-push) +in the Server Developer Guide for the push pipeline. + ## Configuration ### application.properties @@ -380,31 +406,51 @@ hensu-server/ │ ├── execution/ # Server-side execution listeners │ │ ├── LoggingExecutionListener.java # Structured log output for plan/step events │ │ └── CompositeExecutionListener.java # Combines multiple ExecutionListeners +│ ├── dev/ # Dev-only handlers (excluded from prod image) +│ │ └── SleepHandler.java # Simulates long-running node for crash-recovery tests │ ├── mcp/ # MCP integration (SSE split-pipe transport) │ │ ├── JsonRpc.java │ │ ├── McpConnection.java │ │ ├── McpConnectionFactory.java │ │ ├── McpConnectionPool.java +│ │ ├── McpException.java │ │ ├── McpSessionManager.java -│ │ ├── McpSidecar.java -│ │ └── SseMcpConnection.java +│ │ ├── McpSidecar.java # ActionHandler dispatching to MCP tools +│ │ ├── McpToolDiscovery.java # Runtime tool schema discovery + cache +│ │ ├── SseMcpConnection.java +│ │ └── TenantToolRegistry.java # Merges base + tenant MCP tools (MCP precedence) +│ ├── security/ # JWT + tenant resolution + error mapping +│ │ ├── GlobalExceptionMapper.java # Global @Provider — normalizes errors to JSON +│ │ └── RequestTenantResolver.java # Extracts tenant_id claim from JWT │ ├── persistence/ # PostgreSQL persistence (plain JDBC) │ │ ├── JdbcWorkflowRepository.java # Workflow definitions (JSONB) │ │ ├── JdbcWorkflowStateRepository.java # Execution state snapshots (JSONB + lease columns) │ │ ├── ExecutionLeaseManager.java # Distributed lease management (@ApplicationScoped) +│ │ ├── WorkflowPushLock.java # Cluster-wide push mutex (pg_advisory_xact_lock + JVM fallback) │ │ ├── JdbcSupport.java # JDBC helper (queryList, update) │ │ └── PersistenceException.java # Unchecked wrapper for SQLException │ ├── workflow/ # Business logic -│ │ ├── WorkflowService.java -│ │ ├── ExecutionHeartbeatJob.java # Periodic heartbeat emission (@Scheduled) -│ │ └── WorkflowRecoveryJob.java # Orphaned execution sweeper (@Scheduled) +│ │ ├── WorkflowService.java # Facade over registry + execution + query services +│ │ ├── WorkflowRegistryService.java # Push pipeline: WorkflowPushLock + SubWorkflowGraphValidator +│ │ ├── WorkflowExecutionService.java # Start/resume orchestration +│ │ ├── ExecutionQueryService.java # Read-side: status, plan, output, paused list +│ │ ├── ExecutionStateService.java # Snapshot load/save with split-brain guard +│ │ ├── ExecutionHeartbeatJob.java # Periodic heartbeat emission (@Scheduled) +│ │ ├── WorkflowRecoveryJob.java # Orphaned execution sweeper (@Scheduled) +│ │ ├── ExecutionStartResult.java # DTO +│ │ ├── ExecutionOutput.java # DTO +│ │ ├── ExecutionStatus.java # Enum: COMPLETED / PAUSED / RUNNING +│ │ ├── ExecutionSummary.java # DTO for paused-list +│ │ ├── PlanInfo.java # DTO for plan review +│ │ ├── ResumeDecision.java # DTO +│ │ ├── ExecutionNotFoundException.java +│ │ ├── WorkflowNotFoundException.java +│ │ └── WorkflowExecutionException.java │ ├── streaming/ # SSE event streaming │ │ ├── ExecutionEvent.java │ │ └── ExecutionEventBroadcaster.java │ └── tenant/ # Multi-tenancy -│ ├── TenantContext.java -│ ├── TenantAware.java -│ └── TenantResolutionInterceptor.java +│ └── TenantContext.java # ScopedValue-based tenant context └── src/test/java/ └── io/hensu/server/ ├── action/ diff --git a/hensu-server/src/main/java/io/hensu/server/api/ExecutionResource.java b/hensu-server/src/main/java/io/hensu/server/api/ExecutionResource.java index 0e96eaf..bec4609 100644 --- a/hensu-server/src/main/java/io/hensu/server/api/ExecutionResource.java +++ b/hensu-server/src/main/java/io/hensu/server/api/ExecutionResource.java @@ -3,8 +3,15 @@ import io.hensu.server.security.RequestTenantResolver; import io.hensu.server.validation.LogSanitizer; import io.hensu.server.validation.ValidId; +import io.hensu.server.workflow.ExecutionNotFoundException; +import io.hensu.server.workflow.ExecutionOutput; +import io.hensu.server.workflow.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStatus; +import io.hensu.server.workflow.ExecutionSummary; +import io.hensu.server.workflow.PlanInfo; +import io.hensu.server.workflow.ResumeDecision; +import io.hensu.server.workflow.WorkflowNotFoundException; import io.hensu.server.workflow.WorkflowService; -import io.hensu.server.workflow.WorkflowService.*; import jakarta.inject.Inject; import jakarta.validation.Valid; import jakarta.validation.constraints.NotBlank; diff --git a/hensu-server/src/main/java/io/hensu/server/api/WorkflowResource.java b/hensu-server/src/main/java/io/hensu/server/api/WorkflowResource.java index 4f5e699..ed6c77d 100644 --- a/hensu-server/src/main/java/io/hensu/server/api/WorkflowResource.java +++ b/hensu-server/src/main/java/io/hensu/server/api/WorkflowResource.java @@ -1,13 +1,15 @@ package io.hensu.server.api; import io.hensu.core.workflow.Workflow; -import io.hensu.core.workflow.WorkflowRepository; import io.hensu.server.security.RequestTenantResolver; import io.hensu.server.validation.LogSanitizer; import io.hensu.server.validation.ValidId; import io.hensu.server.validation.ValidWorkflow; +import io.hensu.server.workflow.WorkflowNotFoundException; +import io.hensu.server.workflow.WorkflowRegistryService; import jakarta.inject.Inject; import jakarta.validation.constraints.NotNull; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.GET; @@ -42,7 +44,7 @@ /// hensu delete → DELETE /api/v1/workflows/{id} /// ``` /// -/// @see WorkflowRepository for persistence +/// @see WorkflowRegistryService for definition CRUD and cross-workflow cycle validation /// @see ExecutionResource for execution operations @Path("/api/v1/workflows") @Produces(MediaType.APPLICATION_JSON) @@ -51,18 +53,23 @@ public class WorkflowResource { private static final Logger LOG = Logger.getLogger(WorkflowResource.class); - private final WorkflowRepository workflowRepository; + private final WorkflowRegistryService registryService; private final RequestTenantResolver tenantResolver; @Inject public WorkflowResource( - WorkflowRepository workflowRepository, RequestTenantResolver tenantResolver) { - this.workflowRepository = workflowRepository; + WorkflowRegistryService registryService, RequestTenantResolver tenantResolver) { + this.registryService = registryService; this.tenantResolver = tenantResolver; } /// Pushes a workflow definition (idempotent create or update). /// + /// The registry service atomically validates the cross-workflow reference graph + /// for cycles before persisting. If the incoming workflow would close a cycle + /// with already-stored workflows (e.g. A→B pushed, then B→A), the push is + /// rejected with 400 Bad Request and no state change occurs. + /// /// ### Request /// ``` /// POST /api/v1/workflows @@ -96,17 +103,24 @@ public Response pushWorkflow( LogSanitizer.sanitize(workflow.getVersion()), tenantId); - boolean exists = workflowRepository.exists(tenantId, workflow.getId()); - workflowRepository.save(tenantId, workflow); + boolean created; + try { + created = registryService.pushWorkflow(tenantId, workflow); + } catch (IllegalStateException e) { + LOG.warnv( + "Rejected push for workflow {0}: {1}", + LogSanitizer.sanitize(workflow.getId()), e.getMessage()); + throw new BadRequestException(e.getMessage()); + } - Response.Status status = exists ? Response.Status.OK : Response.Status.CREATED; + Response.Status status = created ? Response.Status.CREATED : Response.Status.OK; return Response.status(status) .entity( Map.of( "id", workflow.getId(), "version", workflow.getVersion(), - "created", !exists)) + "created", created)) .build(); } @@ -138,11 +152,12 @@ public Response pullWorkflow(@PathParam("workflowId") @ValidId String workflowId LOG.debugv( "Pull workflow: id={0}, tenant={1}", LogSanitizer.sanitize(workflowId), tenantId); - Workflow workflow = - workflowRepository - .findById(tenantId, workflowId) - .orElseThrow( - () -> new NotFoundException("Workflow not found: " + workflowId)); + Workflow workflow; + try { + workflow = registryService.getWorkflow(tenantId, workflowId); + } catch (WorkflowNotFoundException e) { + throw new NotFoundException(e.getMessage()); + } return Response.ok().entity(workflow).build(); } @@ -169,7 +184,7 @@ public Response listWorkflows() { LOG.debugv("List workflows: tenant={0}", tenantId); - List workflows = workflowRepository.findAll(tenantId); + List workflows = registryService.listWorkflows(tenantId); List> summaries = workflows.stream() @@ -197,7 +212,7 @@ public Response deleteWorkflow(@PathParam("workflowId") @ValidId String workflow LOG.infov( "Delete workflow: id={0}, tenant={1}", LogSanitizer.sanitize(workflowId), tenantId); - boolean deleted = workflowRepository.delete(tenantId, workflowId); + boolean deleted = registryService.deleteWorkflow(tenantId, workflowId); if (!deleted) { throw new NotFoundException("Workflow not found: " + workflowId); diff --git a/hensu-server/src/main/java/io/hensu/server/persistence/ExecutionLeaseManager.java b/hensu-server/src/main/java/io/hensu/server/persistence/ExecutionLeaseManager.java index b92c920..785b98b 100644 --- a/hensu-server/src/main/java/io/hensu/server/persistence/ExecutionLeaseManager.java +++ b/hensu-server/src/main/java/io/hensu/server/persistence/ExecutionLeaseManager.java @@ -81,6 +81,29 @@ public class ExecutionLeaseManager { RETURNING tenant_id, execution_id """; + /// Conditional claim of a single execution. Succeeds when the row is unowned + /// (`server_node_id IS NULL`) or already owned by this node (re-entrant for the + /// recovery → resume hand-off). + static final String SQL_TRY_CLAIM = + """ + UPDATE runtime.execution_states + SET server_node_id = ?, + last_heartbeat_at = NOW() + WHERE tenant_id = ? + AND execution_id = ? + AND (server_node_id IS NULL OR server_node_id = ?) + """; + + /// Releases a lease held by this node. No-op if another node owns the row. + static final String SQL_RELEASE = + """ + UPDATE runtime.execution_states + SET server_node_id = NULL + WHERE tenant_id = ? + AND execution_id = ? + AND server_node_id = ? + """; + // --- CDI-injected fields --- @Inject Config config; @@ -168,6 +191,55 @@ public void updateHeartbeats() { LOG.debugv("Heartbeat updated for node: {0}", serverNodeId); } + /// Attempts to claim a single execution for this node before resuming it. + /// + /// Returns `true` when the row is unowned or already owned by this node — the + /// recovery sweeper claims via {@link #claimStaleExecutions} and then drives + /// `resumeExecution` against the same row, so re-entry must succeed. + /// + /// Returns `true` unconditionally when leasing is inactive (e.g., the + /// `inmem` profile) so single-node and test setups bypass the check. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution identifier, not null + /// @return `true` if this node now holds (or already held) the lease + public boolean tryClaim(String tenantId, String executionId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(executionId, "executionId must not be null"); + if (!active) return true; + int updated = + jdbc.update( + SQL_TRY_CLAIM, + ps -> { + ps.setString(1, serverNodeId); + ps.setString(2, tenantId); + ps.setString(3, executionId); + ps.setString(4, serverNodeId); + }, + "Failed to claim execution: " + executionId); + return updated > 0; + } + + /// Releases a lease held by this node. Safe to call from a `finally` after a + /// successful or failed `tryClaim` — the WHERE clause filters by this node id, + /// so a release after another node took over is a no-op. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution identifier, not null + public void release(String tenantId, String executionId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(executionId, "executionId must not be null"); + if (!active) return; + jdbc.update( + SQL_RELEASE, + ps -> { + ps.setString(1, tenantId); + ps.setString(2, executionId); + ps.setString(3, serverNodeId); + }, + "Failed to release execution: " + executionId); + } + /// Atomically claims all executions whose heartbeat is older than the given threshold. /// /// Uses a single `UPDATE … RETURNING` statement. Under PostgreSQL's default diff --git a/hensu-server/src/main/java/io/hensu/server/persistence/WorkflowPushLock.java b/hensu-server/src/main/java/io/hensu/server/persistence/WorkflowPushLock.java new file mode 100644 index 0000000..1cff40e --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/persistence/WorkflowPushLock.java @@ -0,0 +1,107 @@ +package io.hensu.server.persistence; + +import jakarta.annotation.PostConstruct; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.inject.Instance; +import jakarta.inject.Inject; +import java.sql.SQLException; +import java.util.Objects; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Supplier; +import javax.sql.DataSource; +import org.eclipse.microprofile.config.Config; +import org.jboss.logging.Logger; + +/// Cluster-wide mutex for workflow definition push operations. +/// +/// Backed by PostgreSQL advisory transaction locks (`pg_advisory_xact_lock`) when a +/// DataSource is active so pushes from multiple server nodes for the same +/// `(tenant, workflow)` pair serialize across the cluster. Falls back to a JVM-local +/// {@link ReentrantLock} map for the {@code inmem} profile. +/// +/// ### Why advisory locks +/// Postgres advisory locks are independent of row locks and have zero effect on +/// regular queries — pure coordination primitives. Acquired inside an explicit +/// transaction, the lock auto-releases on commit/rollback, so a crashed claimant +/// never wedges a tenant. +/// +/// @implNote Thread-safe. Active mode is decided once at {@link PostConstruct}; the +/// JVM-local map is only consulted when JDBC is unavailable. +@ApplicationScoped +public class WorkflowPushLock { + + private static final Logger LOG = Logger.getLogger(WorkflowPushLock.class); + + static final String SQL_ADVISORY_LOCK = "SELECT pg_advisory_xact_lock(?, ?)"; + + @Inject Config config; + @Inject Instance dataSourceInstance; + + private final ConcurrentMap jvmLocks = new ConcurrentHashMap<>(); + private DataSource dataSource; + private boolean distributed; + + @PostConstruct + void init() { + boolean dsActive = + config.getOptionalValue("quarkus.datasource.active", Boolean.class).orElse(true); + distributed = dsActive && dataSourceInstance.isResolvable(); + if (distributed) { + dataSource = dataSourceInstance.get(); + } + LOG.infov("Workflow push lock initialized: distributed={0}", distributed); + } + + /// Runs `work` while holding an exclusive lock keyed on `(tenant, workflow)`. + /// + /// @param tenantId tenant scope, not null + /// @param workflowId workflow scope, not null + /// @param work critical section, not null + /// @return whatever `work` returns + public T withLock(String tenantId, String workflowId, Supplier work) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(workflowId, "workflowId must not be null"); + Objects.requireNonNull(work, "work must not be null"); + return distributed + ? withAdvisoryLock(tenantId, workflowId, work) + : withJvmLock(tenantId, workflowId, work); + } + + private T withJvmLock(String tenantId, String workflowId, Supplier work) { + ReentrantLock lock = + jvmLocks.computeIfAbsent(tenantId + ":" + workflowId, _ -> new ReentrantLock()); + lock.lock(); + try { + return work.get(); + } finally { + lock.unlock(); + } + } + + private T withAdvisoryLock(String tenantId, String workflowId, Supplier work) { + try (var conn = dataSource.getConnection()) { + boolean priorAutoCommit = conn.getAutoCommit(); + conn.setAutoCommit(false); + try { + try (var ps = conn.prepareStatement(SQL_ADVISORY_LOCK)) { + ps.setInt(1, tenantId.hashCode()); + ps.setInt(2, workflowId.hashCode()); + ps.execute(); + } + T result = work.get(); + conn.commit(); + return result; + } catch (RuntimeException e) { + conn.rollback(); + throw e; + } finally { + conn.setAutoCommit(priorAutoCommit); + } + } catch (SQLException e) { + throw new PersistenceException( + "Failed to acquire advisory lock for " + tenantId + ":" + workflowId, e); + } + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionNotFoundException.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionNotFoundException.java new file mode 100644 index 0000000..add0b11 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionNotFoundException.java @@ -0,0 +1,12 @@ +package io.hensu.server.workflow; + +import java.io.Serial; + +/// Thrown when an execution cannot be found. +public class ExecutionNotFoundException extends RuntimeException { + @Serial private static final long serialVersionUID = -4295834705797331331L; + + public ExecutionNotFoundException(String message) { + super(message); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionOutput.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionOutput.java new file mode 100644 index 0000000..f53f03d --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionOutput.java @@ -0,0 +1,15 @@ +package io.hensu.server.workflow; + +import java.util.Map; + +/// The public output of a completed or paused execution. +/// +/// Contains the workflow context at termination with internal system keys +/// (prefixed with `_`) excluded. +/// +/// @param executionId the execution identifier, never null +/// @param workflowId the workflow definition identifier, never null +/// @param status `COMPLETED` or `PAUSED`, never null +/// @param output public context variables produced by the workflow, never null, may be empty +public record ExecutionOutput( + String executionId, String workflowId, String status, Map output) {} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionQueryService.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionQueryService.java new file mode 100644 index 0000000..c08d3ec --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionQueryService.java @@ -0,0 +1,113 @@ +package io.hensu.server.workflow; + +import io.hensu.core.state.HensuSnapshot; +import io.hensu.core.state.WorkflowStateRepository; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.stream.Collectors; + +/// Read-only service for querying execution state. +/// +/// Fetches status, pending plan info, public output, and paused execution summaries from +/// the {@link WorkflowStateRepository}. Does not mutate any state — any change to an +/// execution goes through {@link WorkflowExecutionService} or {@link ExecutionStateService}. +@ApplicationScoped +public class ExecutionQueryService { + + private final WorkflowStateRepository stateRepository; + + @Inject + public ExecutionQueryService(WorkflowStateRepository stateRepository) { + this.stateRepository = + Objects.requireNonNull(stateRepository, "stateRepository must not be null"); + } + + /// Gets the current plan for an execution awaiting review. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution ID, not null + /// @return the plan if one is pending review, empty if no plan is active + /// @throws ExecutionNotFoundException if execution not found + public Optional getPendingPlan(String tenantId, String executionId) { + HensuSnapshot snapshot = loadSnapshot(tenantId, executionId); + if (!snapshot.hasActivePlan()) { + return Optional.empty(); + } + return Optional.of( + new PlanInfo( + snapshot.activePlan().nodeId(), + snapshot.activePlan().steps().size(), + snapshot.activePlan().currentStepIndex())); + } + + /// Gets execution status by ID. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution ID, not null + /// @return the execution status, never null + /// @throws ExecutionNotFoundException if execution not found + public ExecutionStatus getExecutionStatus(String tenantId, String executionId) { + HensuSnapshot snapshot = loadSnapshot(tenantId, executionId); + String status = snapshot.isCompleted() ? "COMPLETED" : "PAUSED"; + return new ExecutionStatus( + executionId, + snapshot.workflowId(), + status, + snapshot.currentNodeId(), + snapshot.hasActivePlan()); + } + + /// Gets the public output of a completed or paused execution. + /// + /// Returns the workflow context at the time of the last checkpoint with internal + /// system keys (prefixed with `_`) filtered out. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution ID, not null + /// @return the execution output, never null + /// @throws ExecutionNotFoundException if execution not found + public ExecutionOutput getExecutionResult(String tenantId, String executionId) { + HensuSnapshot snapshot = loadSnapshot(tenantId, executionId); + String status = snapshot.isCompleted() ? "COMPLETED" : "PAUSED"; + return new ExecutionOutput( + executionId, snapshot.workflowId(), status, publicContext(snapshot.context())); + } + + /// Lists all paused executions for a tenant. + /// + /// @param tenantId the tenant ID, not null + /// @return list of paused execution summaries, never null, may be empty + public List listPausedExecutions(String tenantId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + return stateRepository.findPaused(tenantId).stream() + .map( + s -> + new ExecutionSummary( + s.executionId(), + s.workflowId(), + s.currentNodeId(), + s.createdAt())) + .toList(); + } + + private HensuSnapshot loadSnapshot(String tenantId, String executionId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(executionId, "executionId must not be null"); + return stateRepository + .findByExecutionId(tenantId, executionId) + .orElseThrow( + () -> + new ExecutionNotFoundException( + "Execution not found: " + executionId)); + } + + private static Map publicContext(Map context) { + return context.entrySet().stream() + .filter(e -> !e.getKey().startsWith("_")) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStartResult.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStartResult.java new file mode 100644 index 0000000..4f69a91 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStartResult.java @@ -0,0 +1,7 @@ +package io.hensu.server.workflow; + +/// Result of accepting a new workflow execution request. +/// +/// @param executionId the assigned execution identifier, never null +/// @param workflowId the workflow that was started, never null +public record ExecutionStartResult(String executionId, String workflowId) {} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStateService.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStateService.java new file mode 100644 index 0000000..efbbd97 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStateService.java @@ -0,0 +1,143 @@ +package io.hensu.server.workflow; + +import io.hensu.core.execution.ExecutionListener; +import io.hensu.core.execution.WorkflowExecutor; +import io.hensu.core.execution.result.ExecutionResult; +import io.hensu.core.state.HensuSnapshot; +import io.hensu.core.state.HensuState; +import io.hensu.core.state.WorkflowStateRepository; +import io.hensu.core.workflow.Workflow; +import io.hensu.server.persistence.ExecutionLeaseManager; +import io.hensu.server.tenant.TenantContext; +import io.hensu.server.tenant.TenantContext.TenantInfo; +import io.hensu.server.validation.LogSanitizer; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.util.Objects; +import org.jboss.logging.Logger; + +/// Service for resuming paused workflow executions and managing checkpoint persistence. +/// +/// Loads a previously paused execution from the state repository, applies any context +/// modifications from the resume decision, and hands the state back to the core executor +/// via {@link WorkflowExecutor#executeFrom}. Handles the same four {@link ExecutionResult} +/// variants as {@link WorkflowExecutionService} and persists the resulting snapshot. +@ApplicationScoped +public class ExecutionStateService { + + private static final Logger LOG = Logger.getLogger(ExecutionStateService.class); + + private final WorkflowExecutor workflowExecutor; + private final WorkflowStateRepository stateRepository; + private final WorkflowRegistryService registryService; + private final ExecutionLeaseManager leaseManager; + + @Inject + public ExecutionStateService( + WorkflowExecutor workflowExecutor, + WorkflowStateRepository stateRepository, + WorkflowRegistryService registryService, + ExecutionLeaseManager leaseManager) { + this.workflowExecutor = + Objects.requireNonNull(workflowExecutor, "workflowExecutor must not be null"); + this.stateRepository = + Objects.requireNonNull(stateRepository, "stateRepository must not be null"); + this.registryService = + Objects.requireNonNull(registryService, "registryService must not be null"); + this.leaseManager = Objects.requireNonNull(leaseManager, "leaseManager must not be null"); + } + + /// Resumes a paused workflow execution. + /// + /// Acquires the execution lease before driving the executor and releases it in a + /// `finally` block — closes the split-brain window where an API-edge resume could + /// run concurrently with a recovery sweep on another node. The claim is re-entrant + /// for this node, so the recovery → resume hand-off works without releasing in + /// between. + /// + /// @param tenantId the tenant owning the execution, not null + /// @param executionId the execution to resume, not null + /// @param decision the resume decision (approve/modify plan), may be null + /// @throws ExecutionNotFoundException if execution not found + /// @throws IllegalStateException if another node currently owns the execution lease + /// @throws WorkflowExecutionException if resume fails + public void resumeExecution(String tenantId, String executionId, ResumeDecision decision) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(executionId, "executionId must not be null"); + + LOG.infov( + "Resuming workflow execution: executionId={0}, tenant={1}", + LogSanitizer.sanitize(executionId), tenantId); + + if (!leaseManager.tryClaim(tenantId, executionId)) { + throw new IllegalStateException("Execution owned by another node: " + executionId); + } + + try { + HensuSnapshot snapshot = + stateRepository + .findByExecutionId(tenantId, executionId) + .orElseThrow( + () -> + new ExecutionNotFoundException( + "Execution not found: " + executionId)); + + TenantInfo tenant = TenantInfo.simple(tenantId); + + try { + TenantContext.runAs( + tenant, + () -> { + HensuState state = snapshot.toState(); + + if (decision != null && decision.modifications() != null) { + state.getContext().putAll(decision.modifications()); + } + + Workflow workflow = + registryService.getWorkflow(tenantId, snapshot.workflowId()); + + ExecutionResult result = + workflowExecutor.executeFrom( + workflow, state, checkpointListener(tenantId)); + + if (result instanceof ExecutionResult.Completed completed) { + stateRepository.save( + tenantId, + HensuSnapshot.from(completed.finalState(), "completed")); + } else if (result + instanceof ExecutionResult.Paused(HensuState pausedState)) { + stateRepository.save( + tenantId, HensuSnapshot.from(pausedState, "paused")); + } else if (result instanceof ExecutionResult.Rejected rejected) { + stateRepository.save( + tenantId, HensuSnapshot.from(rejected.state(), "rejected")); + } else if (result + instanceof ExecutionResult.Failure(HensuState failedState, _)) { + stateRepository.save( + tenantId, HensuSnapshot.from(failedState, "failed")); + } + + return null; + }); + } catch (Exception e) { + LOG.errorv( + e, + "Resume execution failed: executionId={0}", + LogSanitizer.sanitize(executionId)); + throw new WorkflowExecutionException("Resume failed: " + e.getMessage(), e); + } + } finally { + leaseManager.release(tenantId, executionId); + } + } + + private ExecutionListener checkpointListener(String tenantId) { + return new ExecutionListener() { + @Override + public void onCheckpoint(HensuState state) { + stateRepository.save(tenantId, HensuSnapshot.from(state, "checkpoint")); + } + }; + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStatus.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStatus.java new file mode 100644 index 0000000..8bab849 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionStatus.java @@ -0,0 +1,15 @@ +package io.hensu.server.workflow; + +/// Status of an execution. +/// +/// @param executionId the execution identifier, never null +/// @param workflowId the workflow definition identifier, never null +/// @param status `COMPLETED` or `PAUSED`, never null +/// @param currentNodeId the node where execution is positioned, may be null if completed +/// @param hasPendingPlan true if a plan is awaiting review +public record ExecutionStatus( + String executionId, + String workflowId, + String status, + String currentNodeId, + boolean hasPendingPlan) {} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionSummary.java b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionSummary.java new file mode 100644 index 0000000..b1c1f77 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ExecutionSummary.java @@ -0,0 +1,12 @@ +package io.hensu.server.workflow; + +import java.time.Instant; + +/// Summary of an execution. +/// +/// @param executionId the execution identifier, never null +/// @param workflowId the workflow definition identifier, never null +/// @param currentNodeId the node where execution is paused, may be null +/// @param createdAt when the execution was created, never null +public record ExecutionSummary( + String executionId, String workflowId, String currentNodeId, Instant createdAt) {} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/PlanInfo.java b/hensu-server/src/main/java/io/hensu/server/workflow/PlanInfo.java new file mode 100644 index 0000000..ee4b571 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/PlanInfo.java @@ -0,0 +1,8 @@ +package io.hensu.server.workflow; + +/// Information about a pending plan. +/// +/// @param planId the plan identifier, never null +/// @param totalSteps total number of steps in the plan +/// @param currentStep index of the step currently executing +public record PlanInfo(String planId, int totalSteps, int currentStep) {} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/ResumeDecision.java b/hensu-server/src/main/java/io/hensu/server/workflow/ResumeDecision.java new file mode 100644 index 0000000..c2465d8 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/ResumeDecision.java @@ -0,0 +1,18 @@ +package io.hensu.server.workflow; + +import java.util.Map; + +/// Decision for resuming a paused execution. +/// +/// @param approved whether the pending plan is approved +/// @param modifications optional context modifications to apply before resuming, may be null +public record ResumeDecision(boolean approved, Map modifications) { + + public static ResumeDecision approve() { + return new ResumeDecision(true, Map.of()); + } + + public static ResumeDecision modify(Map mods) { + return new ResumeDecision(true, mods); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionException.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionException.java new file mode 100644 index 0000000..057b7f8 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionException.java @@ -0,0 +1,12 @@ +package io.hensu.server.workflow; + +import java.io.Serial; + +/// Thrown when workflow execution fails at a layer above the core executor. +public class WorkflowExecutionException extends RuntimeException { + @Serial private static final long serialVersionUID = 2286066982933451717L; + + public WorkflowExecutionException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionService.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionService.java new file mode 100644 index 0000000..251e09c --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowExecutionService.java @@ -0,0 +1,242 @@ +package io.hensu.server.workflow; + +import io.hensu.core.execution.ExecutionListener; +import io.hensu.core.execution.WorkflowExecutor; +import io.hensu.core.execution.result.ExecutionResult; +import io.hensu.core.state.HensuSnapshot; +import io.hensu.core.state.HensuState; +import io.hensu.core.state.WorkflowStateRepository; +import io.hensu.core.workflow.Workflow; +import io.hensu.server.execution.CompositeExecutionListener; +import io.hensu.server.execution.LoggingExecutionListener; +import io.hensu.server.streaming.ExecutionEvent; +import io.hensu.server.streaming.ExecutionEventBroadcaster; +import io.hensu.server.tenant.TenantContext; +import io.hensu.server.tenant.TenantContext.TenantInfo; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jboss.logging.Logger; + +/// Service for dispatching new workflow executions. +/// +/// Accepts an execution request, assigns an execution id, and launches the execution +/// on a dedicated virtual thread. Handles all four {@link ExecutionResult} variants, +/// persists the final snapshot, and publishes SSE events for completion, failure, and +/// pause. +/// +/// This service is the **write path** for new executions. Resuming paused executions +/// lives in {@link ExecutionStateService}; read-model queries live in +/// {@link ExecutionQueryService}. +@ApplicationScoped +public class WorkflowExecutionService { + + private static final Logger LOG = Logger.getLogger(WorkflowExecutionService.class); + + @ConfigProperty(name = "hensu.verbose.enabled", defaultValue = "false") + boolean verboseEnabled; + + private final WorkflowExecutor workflowExecutor; + private final WorkflowStateRepository stateRepository; + private final ExecutionEventBroadcaster eventBroadcaster; + private final WorkflowRegistryService registryService; + + @Inject + public WorkflowExecutionService( + WorkflowExecutor workflowExecutor, + WorkflowStateRepository stateRepository, + ExecutionEventBroadcaster eventBroadcaster, + WorkflowRegistryService registryService) { + this.workflowExecutor = + Objects.requireNonNull(workflowExecutor, "workflowExecutor must not be null"); + this.stateRepository = + Objects.requireNonNull(stateRepository, "stateRepository must not be null"); + this.eventBroadcaster = + Objects.requireNonNull(eventBroadcaster, "eventBroadcaster must not be null"); + this.registryService = + Objects.requireNonNull(registryService, "registryService must not be null"); + } + + /// Accepts a new workflow execution and dispatches it asynchronously. + /// + /// Validates that the workflow exists, then immediately returns an execution ID while + /// running the workflow on a background virtual thread. Progress and outcome are + /// delivered via SSE events. + /// + /// @param tenantId the tenant requesting execution, not null + /// @param workflowId the workflow to execute, not null + /// @param context initial context variables, not null + /// @return execution result containing the assigned execution ID, never null + /// @throws WorkflowNotFoundException if the workflow does not exist + public ExecutionStartResult startExecution( + String tenantId, String workflowId, Map context) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(workflowId, "workflowId must not be null"); + Objects.requireNonNull(context, "context must not be null"); + + LOG.infov("Accepting workflow execution: workflow={0}, tenant={1}", workflowId, tenantId); + + String executionId = UUID.randomUUID().toString(); + + TenantInfo tenant; + if (TenantContext.isBound() && TenantContext.current().tenantId().equals(tenantId)) { + tenant = TenantContext.current(); + } else { + tenant = TenantInfo.withMcp(tenantId, "sse://" + tenantId); + } + + // Fail fast before accepting the request if the workflow is not registered. + registryService.getWorkflow(tenantId, workflowId); + + Map executionContext = new HashMap<>(context); + executionContext.put("_tenant_id", tenantId); + executionContext.put("_execution_id", executionId); + + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionStarted.now(executionId, workflowId, tenantId)); + + Thread.ofVirtual() + .name("wf-exec-" + executionId) + .start( + () -> + runExecutionAsync( + executionId, + workflowId, + tenantId, + tenant, + executionContext)); + + LOG.infov("Workflow execution accepted: executionId={0}", executionId); + return new ExecutionStartResult(executionId, workflowId); + } + + private void runExecutionAsync( + String executionId, + String workflowId, + String tenantId, + TenantInfo tenant, + Map executionContext) { + AtomicReference lastCheckpoint = new AtomicReference<>(); + try { + eventBroadcaster.runAs( + executionId, + () -> { + TenantContext.runAs( + tenant, + () -> { + Workflow workflow = + registryService.getWorkflow(tenantId, workflowId); + + ExecutionListener checkpoint = + trackingCheckpointListener(tenantId, lastCheckpoint); + ExecutionListener listener = + verboseEnabled + ? new CompositeExecutionListener( + checkpoint, + new LoggingExecutionListener()) + : checkpoint; + ExecutionResult result = + workflowExecutor.execute( + workflow, executionContext, listener); + + if (result instanceof ExecutionResult.Completed completed) { + HensuSnapshot snapshot = + HensuSnapshot.from( + completed.finalState(), "completed"); + stateRepository.save(tenantId, snapshot); + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionCompleted.success( + executionId, + workflowId, + completed.finalState().getCurrentNode(), + publicContext( + completed + .finalState() + .getContext()))); + } else if (result + instanceof ExecutionResult.Rejected rejected) { + HensuSnapshot snapshot = + HensuSnapshot.from(rejected.state(), "rejected"); + stateRepository.save(tenantId, snapshot); + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionCompleted.failure( + executionId, + workflowId, + rejected.state().getCurrentNode(), + publicContext( + rejected.state().getContext()))); + } else if (result + instanceof ExecutionResult.Paused(HensuState state)) { + HensuSnapshot snapshot = + HensuSnapshot.from(state, "paused"); + stateRepository.save(tenantId, snapshot); + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionCompleted.failure( + executionId, + workflowId, + state.getCurrentNode(), + publicContext(state.getContext()))); + } else if (result + instanceof + ExecutionResult.Failure( + HensuState currentState, + IllegalStateException e)) { + HensuSnapshot snapshot = + HensuSnapshot.from(currentState, "failed"); + stateRepository.save(tenantId, snapshot); + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionError.now( + executionId, + "ExecutionFailure", + e.getMessage(), + currentState.getCurrentNode())); + } + return null; + }); + return null; + }); + LOG.infov("Workflow execution completed: executionId={0}", executionId); + } catch (Exception e) { + LOG.errorv( + e, "Workflow execution failed: workflow={0}, tenant={1}", workflowId, tenantId); + eventBroadcaster.publish( + executionId, + ExecutionEvent.ExecutionError.now( + executionId, e.getClass().getSimpleName(), e.getMessage(), null)); + HensuState ls = lastCheckpoint.get(); + if (ls != null) { + stateRepository.save(tenantId, HensuSnapshot.from(ls, "failed")); + } + } finally { + eventBroadcaster.complete(executionId); + } + } + + private ExecutionListener trackingCheckpointListener( + String tenantId, AtomicReference lastCheckpoint) { + return new ExecutionListener() { + @Override + public void onCheckpoint(HensuState state) { + lastCheckpoint.set(state); + stateRepository.save(tenantId, HensuSnapshot.from(state, "checkpoint")); + } + }; + } + + private static Map publicContext(Map context) { + return context.entrySet().stream() + .filter(e -> !e.getKey().startsWith("_")) + .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowNotFoundException.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowNotFoundException.java new file mode 100644 index 0000000..e7924ec --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowNotFoundException.java @@ -0,0 +1,12 @@ +package io.hensu.server.workflow; + +import java.io.Serial; + +/// Thrown when a workflow definition cannot be found. +public class WorkflowNotFoundException extends RuntimeException { + @Serial private static final long serialVersionUID = 6275906519265300703L; + + public WorkflowNotFoundException(String message) { + super(message); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRecoveryJob.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRecoveryJob.java index 1f83dc9..53bd5c3 100644 --- a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRecoveryJob.java +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRecoveryJob.java @@ -32,7 +32,8 @@ /// /// @implNote Thread-safe. `claimStaleExecutions` is atomic under PostgreSQL /// `READ COMMITTED` — two concurrent sweepers cannot claim the same row. -/// Each `resumeExecution` call runs synchronously on a virtual thread. +/// Each claimed execution is dispatched to its own virtual thread so a slow +/// recovered workflow cannot block the scheduler tick or starve heartbeats. /// /// @see ExecutionLeaseManager#claimStaleExecutions(Instant) /// @see ExecutionHeartbeatJob @@ -69,15 +70,21 @@ void tick() { LOG.infov("Recovering {0} orphaned execution(s)", stale.size()); for (ExecutionRef ref : stale) { - try { - workflowService.resumeExecution(ref.tenantId(), ref.executionId(), null); - LOG.infov("Recovered execution: {0}", LogSanitizer.sanitize(ref.executionId())); - } catch (Exception e) { - LOG.errorv( - e, - "Failed to recover execution: {0}", - LogSanitizer.sanitize(ref.executionId())); - } + Thread.ofVirtual() + .name("wf-recover-" + ref.executionId()) + .start(() -> resumeRecovered(ref)); + } + } + + private void resumeRecovered(ExecutionRef ref) { + try { + workflowService.resumeExecution(ref.tenantId(), ref.executionId(), null); + LOG.infov("Recovered execution: {0}", LogSanitizer.sanitize(ref.executionId())); + } catch (Exception e) { + LOG.errorv( + e, + "Failed to recover execution: {0}", + LogSanitizer.sanitize(ref.executionId())); } } } diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java new file mode 100644 index 0000000..3bbea82 --- /dev/null +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java @@ -0,0 +1,119 @@ +package io.hensu.server.workflow; + +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.WorkflowRepository; +import io.hensu.core.workflow.validation.SubWorkflowGraphValidator; +import io.hensu.server.persistence.WorkflowPushLock; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.inject.Inject; +import java.util.List; +import java.util.Objects; +import org.jboss.logging.Logger; + +/// Service for workflow **definition** CRUD operations. +/// +/// Owns all static workflow metadata concerns — push, pull, list, delete — separate from +/// runtime execution concerns (which live in {@link WorkflowExecutionService} and friends). +/// +/// ### Push semantics +/// Push is idempotent create-or-update. Before saving, the service runs +/// {@link SubWorkflowGraphValidator} over the forward reachable graph from the incoming +/// workflow, lazily resolving referenced sub-workflow ids through the repository. A cycle +/// detected here fails the push before any persistence — the repository never holds a +/// cyclic state, even transiently. +/// +/// ### Per-(tenant, workflow) atomicity +/// The validate→save critical section runs under {@link WorkflowPushLock}, which +/// serializes pushes for the same `(tenant, workflow)` cluster-wide via Postgres +/// advisory locks (or a JVM-local fallback in `inmem` mode). Two concurrent pushes +/// cannot each observe a clean graph and together introduce a cycle. +/// +/// @see SubWorkflowGraphValidator +/// @see WorkflowPushLock +@ApplicationScoped +public class WorkflowRegistryService { + + private static final Logger LOG = Logger.getLogger(WorkflowRegistryService.class); + + private final WorkflowRepository workflowRepository; + private final WorkflowPushLock pushLock; + + @Inject + public WorkflowRegistryService( + WorkflowRepository workflowRepository, WorkflowPushLock pushLock) { + this.workflowRepository = + Objects.requireNonNull(workflowRepository, "workflowRepository must not be null"); + this.pushLock = Objects.requireNonNull(pushLock, "pushLock must not be null"); + } + + /// Pushes a workflow definition (create or update) after validating the sub-workflow + /// reference graph is acyclic. + /// + /// @param tenantId the owning tenant, not null + /// @param workflow the workflow definition to save, not null + /// @return {@code true} if this was a create, {@code false} if an update of an existing id + /// @throws IllegalStateException if pushing this workflow would introduce a cycle in the + /// sub-workflow reference graph + public boolean pushWorkflow(String tenantId, Workflow workflow) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(workflow, "workflow must not be null"); + + return pushLock.withLock( + tenantId, + workflow.getId(), + () -> { + // Validate reference existence and cycles BEFORE save — the repository never + // holds a broken or cyclic state. The resolver shadows the incoming workflow + // for its own id so the check sees the post-push graph without an intermediate + // write. + SubWorkflowGraphValidator.validate( + workflow, + id -> + id.equals(workflow.getId()) + ? workflow + : workflowRepository + .findById(tenantId, id) + .orElse(null)); + + boolean existed = workflowRepository.exists(tenantId, workflow.getId()); + workflowRepository.save(tenantId, workflow); + return !existed; + }); + } + + /// Looks up a workflow definition by id. + /// + /// @param tenantId the owning tenant, not null + /// @param workflowId the workflow id, not null + /// @return the workflow + /// @throws WorkflowNotFoundException if no workflow with that id exists for the tenant + public Workflow getWorkflow(String tenantId, String workflowId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(workflowId, "workflowId must not be null"); + return workflowRepository + .findById(tenantId, workflowId) + .orElseThrow( + () -> new WorkflowNotFoundException("Workflow not found: " + workflowId)); + } + + /// Lists every workflow definition for a tenant. + /// + /// @param tenantId the owning tenant, not null + /// @return list of workflow definitions, never null, may be empty + public List listWorkflows(String tenantId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + return workflowRepository.findAll(tenantId); + } + + /// Deletes a workflow definition. + /// + /// @param tenantId the owning tenant, not null + /// @param workflowId the workflow id to delete, not null + /// @return {@code true} if a workflow was deleted, {@code false} if none matched + public boolean deleteWorkflow(String tenantId, String workflowId) { + Objects.requireNonNull(tenantId, "tenantId must not be null"); + Objects.requireNonNull(workflowId, "workflowId must not be null"); + LOG.infov("Delete workflow: id={0}, tenant={1}", workflowId, tenantId); + return workflowRepository.delete(tenantId, workflowId); + } +} diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowService.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowService.java index cb724d5..970d196 100644 --- a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowService.java +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowService.java @@ -1,589 +1,66 @@ package io.hensu.server.workflow; -import io.hensu.core.execution.ExecutionListener; -import io.hensu.core.execution.WorkflowExecutor; -import io.hensu.core.execution.result.ExecutionResult; -import io.hensu.core.state.HensuSnapshot; -import io.hensu.core.state.HensuState; -import io.hensu.core.state.WorkflowStateRepository; -import io.hensu.core.workflow.Workflow; -import io.hensu.core.workflow.WorkflowRepository; -import io.hensu.server.execution.CompositeExecutionListener; -import io.hensu.server.execution.LoggingExecutionListener; -import io.hensu.server.streaming.ExecutionEvent; -import io.hensu.server.streaming.ExecutionEventBroadcaster; -import io.hensu.server.tenant.TenantContext; -import io.hensu.server.tenant.TenantContext.TenantInfo; -import io.hensu.server.validation.LogSanitizer; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; -import java.io.Serial; -import java.time.Instant; -import java.util.*; -import java.util.concurrent.atomic.AtomicReference; -import java.util.stream.Collectors; -import org.eclipse.microprofile.config.inject.ConfigProperty; -import org.jboss.logging.Logger; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; -/// Service for workflow execution and state management. +/// Facade over the execution-side workflow services. /// -/// Handles business logic for workflow operations: -/// - Starting new workflow executions -/// - Resuming paused executions -/// - Retrieving execution state, plans, and final output +/// Provides a single injection point for `ExecutionResource` and other callers that want +/// the full execution API surface without depending on the internal split between +/// {@link WorkflowExecutionService} (dispatch), {@link ExecutionStateService} (resume), and +/// {@link ExecutionQueryService} (reads). /// -/// All operations are tenant-scoped via {@link TenantContext}. +/// This class holds no state and contains no business logic — every method is a one-line +/// delegation. It exists solely to preserve a stable controller-facing API while keeping +/// the underlying services small and single-responsibility. /// -/// @see io.hensu.server.api.WorkflowResource for REST API endpoints -/// @see WorkflowExecutor for core execution logic +/// For workflow **definition** CRUD (push/pull/list/delete), use +/// {@link WorkflowRegistryService} directly — it has a different lifetime and concern +/// boundary and does not belong behind this facade. @ApplicationScoped public class WorkflowService { - private static final Logger LOG = Logger.getLogger(WorkflowService.class); - - @ConfigProperty(name = "hensu.verbose.enabled", defaultValue = "false") - boolean verboseEnabled; - - private final WorkflowExecutor workflowExecutor; - private final WorkflowStateRepository stateRepository; - private final ExecutionEventBroadcaster eventBroadcaster; - private final WorkflowRepository workflowRepository; + private final WorkflowExecutionService executionService; + private final ExecutionStateService stateService; + private final ExecutionQueryService queryService; @Inject public WorkflowService( - WorkflowExecutor workflowExecutor, - WorkflowStateRepository stateRepository, - ExecutionEventBroadcaster eventBroadcaster, - WorkflowRepository workflowRepository) { - this.workflowExecutor = - Objects.requireNonNull(workflowExecutor, "workflowExecutor must not be null"); - this.stateRepository = - Objects.requireNonNull(stateRepository, "stateRepository must not be null"); - this.eventBroadcaster = - Objects.requireNonNull(eventBroadcaster, "eventBroadcaster must not be null"); - this.workflowRepository = - Objects.requireNonNull(workflowRepository, "workflowRepository must not be null"); + WorkflowExecutionService executionService, + ExecutionStateService stateService, + ExecutionQueryService queryService) { + this.executionService = + Objects.requireNonNull(executionService, "executionService must not be null"); + this.stateService = Objects.requireNonNull(stateService, "stateService must not be null"); + this.queryService = Objects.requireNonNull(queryService, "queryService must not be null"); } - /// Accepts a new workflow execution and dispatches it asynchronously. - /// - /// Validates that the workflow exists, then immediately returns an execution ID while - /// running the workflow on a background virtual thread. The caller does not block on - /// workflow completion — progress and outcome are delivered via SSE events. - /// - /// ### Contracts - /// - **Precondition**: workflow identified by `workflowId` must be registered for the tenant - /// - **Postcondition**: `ExecutionStarted` SSE event is published before this method returns - /// - /// @apiNote **Side effects**: - /// - Publishes an `ExecutionStarted` event to the SSE stream synchronously - /// - Dispatches a virtual thread that persists checkpoints and publishes completion events - /// - Background thread publishes `ExecutionCompleted` on success (with `output` — the - /// public workflow context, all keys prefixed with `_` stripped) or `ExecutionError` - /// on unrecoverable failure - /// - /// @implNote Execution runs on a dedicated `Thread.ofVirtual()` thread; the HTTP response - /// is returned before the workflow completes. Both {@link TenantContext} and the - /// execution-scoped broadcaster are explicitly rebound inside the background thread. - /// - /// @param tenantId the tenant requesting execution; typically the JWT {@code tenant_id} - /// claim resolved by {@code RequestTenantResolver} at the API layer, not null - /// @param workflowId the workflow to execute, not null - /// @param context initial context variables, not null - /// @return execution result containing the assigned execution ID, never null - /// @throws WorkflowNotFoundException if the workflow does not exist public ExecutionStartResult startExecution( String tenantId, String workflowId, Map context) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - Objects.requireNonNull(workflowId, "workflowId must not be null"); - Objects.requireNonNull(context, "context must not be null"); - - LOG.infov("Accepting workflow execution: workflow={0}, tenant={1}", workflowId, tenantId); - - String executionId = UUID.randomUUID().toString(); - - // Preserve existing tenant context (e.g., already bound with MCP) if it matches; - // otherwise create a tenant context with the SSE MCP endpoint keyed by tenantId. - // The MCP client must connect via /mcp/connect?clientId= for routing to work. - TenantInfo tenant; - if (TenantContext.isBound() && TenantContext.current().tenantId().equals(tenantId)) { - tenant = TenantContext.current(); - } else { - tenant = TenantInfo.withMcp(tenantId, "sse://" + tenantId); - } - - // Validate workflow existence synchronously — fail fast before accepting the request. - workflowRepository - .findById(tenantId, workflowId) - .orElseThrow( - () -> new WorkflowNotFoundException("Workflow not found: " + workflowId)); - - Map executionContext = new HashMap<>(context); - executionContext.put("_tenant_id", tenantId); - executionContext.put("_execution_id", executionId); - - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionStarted.now(executionId, workflowId, tenantId)); - - // Dispatch execution on a virtual thread — HTTP response returns immediately. - Thread.ofVirtual() - .name("wf-exec-" + executionId) - .start( - () -> - runExecutionAsync( - executionId, - workflowId, - tenantId, - tenant, - executionContext)); - - LOG.infov("Workflow execution accepted: executionId={0}", executionId); - return new ExecutionStartResult(executionId, workflowId); + return executionService.startExecution(tenantId, workflowId, context); } - /// Runs a workflow execution on the calling (virtual) thread. - /// - /// Rebinds both {@link TenantContext} and execution-scoped broadcaster before executing, - /// then handles all result variants by persisting state and publishing the appropriate event. - /// Always calls {@link ExecutionEventBroadcaster#complete} in a `finally` block to - /// close the SSE stream regardless of outcome. - /// - /// @param executionId the execution identifier, not null - /// @param workflowId the workflow to execute, not null - /// @param tenantId the owning tenant, not null - /// @param tenant the tenant info for scoped context, not null - /// @param executionContext the pre-built execution context, not null - private void runExecutionAsync( - String executionId, - String workflowId, - String tenantId, - TenantInfo tenant, - Map executionContext) { - AtomicReference lastCheckpoint = new AtomicReference<>(); - try { - eventBroadcaster.runAs( - executionId, - () -> { - TenantContext.runAs( - tenant, - () -> { - Workflow workflow = loadWorkflow(workflowId); - - ExecutionListener checkpoint = - trackingCheckpointListener(tenantId, lastCheckpoint); - ExecutionListener listener = - verboseEnabled - ? new CompositeExecutionListener( - checkpoint, - new LoggingExecutionListener()) - : checkpoint; - ExecutionResult result = - workflowExecutor.execute( - workflow, executionContext, listener); - - if (result instanceof ExecutionResult.Completed completed) { - HensuSnapshot snapshot = - HensuSnapshot.from( - completed.finalState(), "completed"); - stateRepository.save(tenantId, snapshot); - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionCompleted.success( - executionId, - workflowId, - completed.finalState().getCurrentNode(), - publicContext( - completed - .finalState() - .getContext()))); - } else if (result - instanceof ExecutionResult.Rejected rejected) { - HensuSnapshot snapshot = - HensuSnapshot.from(rejected.state(), "rejected"); - stateRepository.save(tenantId, snapshot); - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionCompleted.failure( - executionId, - workflowId, - rejected.state().getCurrentNode(), - publicContext( - rejected.state().getContext()))); - } else if (result - instanceof ExecutionResult.Paused(HensuState state)) { - HensuSnapshot snapshot = - HensuSnapshot.from(state, "paused"); - stateRepository.save(tenantId, snapshot); - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionCompleted.failure( - executionId, - workflowId, - state.getCurrentNode(), - publicContext(state.getContext()))); - } else if (result - instanceof - ExecutionResult.Failure( - HensuState currentState, - IllegalStateException e)) { - HensuSnapshot snapshot = - HensuSnapshot.from(currentState, "failed"); - stateRepository.save(tenantId, snapshot); - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionError.now( - executionId, - "ExecutionFailure", - e.getMessage(), - currentState.getCurrentNode())); - } - return null; - }); - return null; - }); - LOG.infov("Workflow execution completed: executionId={0}", executionId); - } catch (Exception e) { - LOG.errorv( - e, "Workflow execution failed: workflow={0}, tenant={1}", workflowId, tenantId); - eventBroadcaster.publish( - executionId, - ExecutionEvent.ExecutionError.now( - executionId, e.getClass().getSimpleName(), e.getMessage(), null)); - // Persist a terminal "failed" snapshot so that status queries return "failed" - // rather than a stale "checkpoint" or a missing record, and so that - // WorkflowRecoveryJob does not attempt to re-claim an execution that ended - // with an unrecoverable error. Mirrors the ExecutionResult.Failure branch above, - // which handles failures that the executor wraps; this handles those that escape. - HensuState ls = lastCheckpoint.get(); - if (ls != null) { - stateRepository.save(tenantId, HensuSnapshot.from(ls, "failed")); - } - } finally { - eventBroadcaster.complete(executionId); - } - } - - /// Creates a checkpoint listener that persists state after each node and tracks the latest. - /// - /// @param tenantId the owning tenant, not null - /// @param lastCheckpoint holder updated with every checkpoint state, not null - /// @return listener that saves checkpoints and updates the reference, never null - private ExecutionListener trackingCheckpointListener( - String tenantId, AtomicReference lastCheckpoint) { - return new ExecutionListener() { - @Override - public void onCheckpoint(HensuState state) { - lastCheckpoint.set(state); - stateRepository.save(tenantId, HensuSnapshot.from(state, "checkpoint")); - } - }; - } - - /// Resumes a paused workflow execution. - /// - /// @param tenantId the tenant owning the execution, not null - /// @param executionId the execution to resume, not null - /// @param decision the resume decision (approve/modify plan), may be null - /// @throws ExecutionNotFoundException if execution not found - /// @throws WorkflowExecutionException if resume fails public void resumeExecution(String tenantId, String executionId, ResumeDecision decision) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - Objects.requireNonNull(executionId, "executionId must not be null"); - - LOG.infov( - "Resuming workflow execution: executionId={0}, tenant={1}", - LogSanitizer.sanitize(executionId), tenantId); - - HensuSnapshot snapshot = - stateRepository - .findByExecutionId(tenantId, executionId) - .orElseThrow( - () -> - new ExecutionNotFoundException( - "Execution not found: " + executionId)); - - TenantInfo tenant = TenantInfo.simple(tenantId); - - try { - TenantContext.runAs( - tenant, - () -> { - HensuState state = snapshot.toState(); - - // Apply modifications from resume decision - if (decision != null && decision.modifications() != null) { - state.getContext().putAll(decision.modifications()); - } - - // Load the workflow for re-execution - Workflow workflow = loadWorkflow(snapshot.workflowId()); - - // Resume execution from saved state - ExecutionResult result = - workflowExecutor.executeFrom( - workflow, state, checkpointListener(tenantId)); - - // Save final state - if (result instanceof ExecutionResult.Completed completed) { - HensuSnapshot finalSnapshot = - HensuSnapshot.from(completed.finalState(), "completed"); - stateRepository.save(tenantId, finalSnapshot); - } else if (result - instanceof ExecutionResult.Paused(HensuState pausedState)) { - HensuSnapshot pausedSnapshot = - HensuSnapshot.from(pausedState, "paused"); - stateRepository.save(tenantId, pausedSnapshot); - } else if (result instanceof ExecutionResult.Rejected rejected) { - HensuSnapshot rejectedSnapshot = - HensuSnapshot.from(rejected.state(), "rejected"); - stateRepository.save(tenantId, rejectedSnapshot); - } else if (result - instanceof ExecutionResult.Failure(HensuState failedState, _)) { - HensuSnapshot failedSnapshot = - HensuSnapshot.from(failedState, "failed"); - stateRepository.save(tenantId, failedSnapshot); - } - - return null; - }); - } catch (Exception e) { - LOG.errorv( - e, - "Resume execution failed: executionId={0}", - LogSanitizer.sanitize(executionId)); - throw new WorkflowExecutionException("Resume failed: " + e.getMessage(), e); - } + stateService.resumeExecution(tenantId, executionId, decision); } - /// Gets the current plan for an execution awaiting review. - /// - /// @param tenantId the tenant owning the execution, not null - /// @param executionId the execution ID, not null - /// @return the plan if one is pending review, empty if no plan is active - /// @throws ExecutionNotFoundException if execution not found public Optional getPendingPlan(String tenantId, String executionId) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - Objects.requireNonNull(executionId, "executionId must not be null"); - - HensuSnapshot snapshot = - stateRepository - .findByExecutionId(tenantId, executionId) - .orElseThrow( - () -> - new ExecutionNotFoundException( - "Execution not found: " + executionId)); - - if (!snapshot.hasActivePlan()) { - return Optional.empty(); - } - - return Optional.of( - new PlanInfo( - snapshot.activePlan().nodeId(), - snapshot.activePlan().steps().size(), - snapshot.activePlan().currentStepIndex())); + return queryService.getPendingPlan(tenantId, executionId); } - /// Gets execution status by ID. - /// - /// @param tenantId the tenant owning the execution, not null - /// @param executionId the execution ID, not null - /// @return the execution status, never null - /// @throws ExecutionNotFoundException if execution not found public ExecutionStatus getExecutionStatus(String tenantId, String executionId) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - Objects.requireNonNull(executionId, "executionId must not be null"); - - HensuSnapshot snapshot = - stateRepository - .findByExecutionId(tenantId, executionId) - .orElseThrow( - () -> - new ExecutionNotFoundException( - "Execution not found: " + executionId)); - - String status = snapshot.isCompleted() ? "COMPLETED" : "PAUSED"; - return new ExecutionStatus( - executionId, - snapshot.workflowId(), - status, - snapshot.currentNodeId(), - snapshot.hasActivePlan()); + return queryService.getExecutionStatus(tenantId, executionId); } - /// Gets the public output of a completed or paused execution. - /// - /// Returns the workflow context at the time of the last checkpoint with - /// internal system keys (prefixed with `_`) filtered out. This is the - /// primary endpoint for retrieving the final result of a workflow run. - /// - /// @param tenantId the tenant owning the execution, not null - /// @param executionId the execution ID, not null - /// @return the execution output, never null - /// @throws ExecutionNotFoundException if execution not found public ExecutionOutput getExecutionResult(String tenantId, String executionId) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - Objects.requireNonNull(executionId, "executionId must not be null"); - - HensuSnapshot snapshot = - stateRepository - .findByExecutionId(tenantId, executionId) - .orElseThrow( - () -> - new ExecutionNotFoundException( - "Execution not found: " + executionId)); - - String status = snapshot.isCompleted() ? "COMPLETED" : "PAUSED"; - return new ExecutionOutput( - executionId, snapshot.workflowId(), status, publicContext(snapshot.context())); + return queryService.getExecutionResult(tenantId, executionId); } - /// Lists all paused executions for a tenant. - /// - /// @param tenantId the tenant ID, not null - /// @return list of paused execution summaries, never null, may be empty public List listPausedExecutions(String tenantId) { - Objects.requireNonNull(tenantId, "tenantId must not be null"); - - return stateRepository.findPaused(tenantId).stream() - .map( - s -> - new ExecutionSummary( - s.executionId(), - s.workflowId(), - s.currentNodeId(), - s.createdAt())) - .toList(); - } - - /// Returns only the public context entries — keys starting with `_` are internal - /// system variables and are excluded from client-facing output. - /// - /// @param context the raw execution context, not null - /// @return unmodifiable map of public context entries, never null, may be empty - private static Map publicContext(Map context) { - return context.entrySet().stream() - .filter(e -> !e.getKey().startsWith("_")) - .collect(Collectors.toUnmodifiableMap(Map.Entry::getKey, Map.Entry::getValue)); - } - - /// Creates a listener that persists workflow state after each node completes. - /// - /// @param tenantId the tenant owning the execution, not null - /// @return listener that saves checkpoints to the state repository, never null - private ExecutionListener checkpointListener(String tenantId) { - return new ExecutionListener() { - @Override - public void onCheckpoint(HensuState state) { - stateRepository.save(tenantId, HensuSnapshot.from(state, "checkpoint")); - } - }; - } - - /// Loads a workflow by ID. - /// - /// @param workflowId the workflow ID to load, not null - /// @return the workflow, never null - /// @throws WorkflowNotFoundException if not found - private Workflow loadWorkflow(String workflowId) { - String tenantId = TenantContext.current().tenantId(); - return workflowRepository - .findById(tenantId, workflowId) - .orElseThrow( - () -> new WorkflowNotFoundException("Workflow not found: " + workflowId)); - } - - // --- Result types --- - - /// Result of starting an execution. - /// - /// @param executionId the assigned execution identifier, never null - /// @param workflowId the workflow that was started, never null - public record ExecutionStartResult(String executionId, String workflowId) {} - - /// Decision for resuming a paused execution. - /// - /// @param approved whether the pending plan is approved - /// @param modifications optional context modifications to apply before resuming, may be null - public record ResumeDecision(boolean approved, Map modifications) { - public static ResumeDecision approve() { - return new ResumeDecision(true, Map.of()); - } - - public static ResumeDecision modify(Map mods) { - return new ResumeDecision(true, mods); - } - } - - /// Information about a pending plan. - /// - /// @param planId the plan identifier, never null - /// @param totalSteps total number of steps in the plan - /// @param currentStep index of the step currently executing - public record PlanInfo(String planId, int totalSteps, int currentStep) {} - - /// Status of an execution. - /// - /// @param executionId the execution identifier, never null - /// @param workflowId the workflow definition identifier, never null - /// @param status `COMPLETED` or `PAUSED`, never null - /// @param currentNodeId the node where execution is positioned, may be null if completed - /// @param hasPendingPlan true if a plan is awaiting review - public record ExecutionStatus( - String executionId, - String workflowId, - String status, - String currentNodeId, - boolean hasPendingPlan) {} - - /// The public output of a completed or paused execution. - /// - /// Contains the workflow context at termination with internal system keys - /// (prefixed with `_`) excluded. - /// - /// @param executionId the execution identifier, never null - /// @param workflowId the workflow definition identifier, never null - /// @param status `COMPLETED` or `PAUSED`, never null - /// @param output public context variables produced by the workflow, never null, may be empty - public record ExecutionOutput( - String executionId, String workflowId, String status, Map output) {} - - /// Summary of an execution. - /// - /// @param executionId the execution identifier, never null - /// @param workflowId the workflow definition identifier, never null - /// @param currentNodeId the node where execution is paused, may be null - /// @param createdAt when the execution was created, never null - public record ExecutionSummary( - String executionId, String workflowId, String currentNodeId, Instant createdAt) {} - - // --- Exceptions --- - - /// Thrown when a workflow is not found. - public static class WorkflowNotFoundException extends RuntimeException { - @Serial private static final long serialVersionUID = 6275906519265300703L; - - public WorkflowNotFoundException(String message) { - super(message); - } - } - - /// Thrown when an execution is not found. - public static class ExecutionNotFoundException extends RuntimeException { - @Serial private static final long serialVersionUID = -4295834705797331331L; - - public ExecutionNotFoundException(String message) { - super(message); - } - } - - /// Thrown when workflow execution fails. - public static class WorkflowExecutionException extends RuntimeException { - @Serial private static final long serialVersionUID = 2286066982933451717L; - - public WorkflowExecutionException(String message, Throwable cause) { - super(message, cause); - } + return queryService.listPausedExecutions(tenantId); } } diff --git a/hensu-server/src/test/java/io/hensu/server/api/ExecutionResourceTest.java b/hensu-server/src/test/java/io/hensu/server/api/ExecutionResourceTest.java index 0950988..9fb099d 100644 --- a/hensu-server/src/test/java/io/hensu/server/api/ExecutionResourceTest.java +++ b/hensu-server/src/test/java/io/hensu/server/api/ExecutionResourceTest.java @@ -10,14 +10,14 @@ import static org.mockito.Mockito.when; import io.hensu.server.security.RequestTenantResolver; +import io.hensu.server.workflow.ExecutionNotFoundException; +import io.hensu.server.workflow.ExecutionOutput; +import io.hensu.server.workflow.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStatus; +import io.hensu.server.workflow.ExecutionSummary; +import io.hensu.server.workflow.ResumeDecision; +import io.hensu.server.workflow.WorkflowNotFoundException; import io.hensu.server.workflow.WorkflowService; -import io.hensu.server.workflow.WorkflowService.ExecutionNotFoundException; -import io.hensu.server.workflow.WorkflowService.ExecutionOutput; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; -import io.hensu.server.workflow.WorkflowService.ExecutionStatus; -import io.hensu.server.workflow.WorkflowService.ExecutionSummary; -import io.hensu.server.workflow.WorkflowService.ResumeDecision; -import io.hensu.server.workflow.WorkflowService.WorkflowNotFoundException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import java.time.Instant; diff --git a/hensu-server/src/test/java/io/hensu/server/api/WorkflowResourceTest.java b/hensu-server/src/test/java/io/hensu/server/api/WorkflowResourceTest.java index 7c303b7..308955b 100644 --- a/hensu-server/src/test/java/io/hensu/server/api/WorkflowResourceTest.java +++ b/hensu-server/src/test/java/io/hensu/server/api/WorkflowResourceTest.java @@ -2,36 +2,39 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import io.hensu.core.execution.result.ExitStatus; import io.hensu.core.workflow.Workflow; -import io.hensu.core.workflow.WorkflowRepository; import io.hensu.core.workflow.node.EndNode; import io.hensu.server.security.RequestTenantResolver; +import io.hensu.server.workflow.WorkflowNotFoundException; +import io.hensu.server.workflow.WorkflowRegistryService; +import jakarta.ws.rs.BadRequestException; import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.core.Response; import java.util.List; import java.util.Map; -import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; class WorkflowResourceTest { - private WorkflowRepository workflowRepository; + private WorkflowRegistryService registryService; private WorkflowResource resource; @BeforeEach void setUp() { - workflowRepository = mock(WorkflowRepository.class); + registryService = mock(WorkflowRegistryService.class); RequestTenantResolver tenantResolver = mock(RequestTenantResolver.class); when(tenantResolver.tenantId()).thenReturn("tenant-1"); - resource = new WorkflowResource(workflowRepository, tenantResolver); + resource = new WorkflowResource(registryService, tenantResolver); } - private Workflow createTestWorkflow(String id, String version) { + private Workflow workflow(String id, String version) { return Workflow.builder() .id(id) .version(version) @@ -48,37 +51,50 @@ class PushWorkflow { @Test void shouldReturn201WhenCreatingNewWorkflow() { - Workflow workflow = createTestWorkflow("wf-1", "1.0.0"); - when(workflowRepository.exists("tenant-1", "wf-1")).thenReturn(false); + Workflow wf = workflow("wf-1", "1.0.0"); + when(registryService.pushWorkflow("tenant-1", wf)).thenReturn(true); Map entity; - try (Response response = resource.pushWorkflow(workflow)) { - + try (Response response = resource.pushWorkflow(wf)) { assertThat(response.getStatus()).isEqualTo(201); - verify(workflowRepository).save("tenant-1", workflow); - entity = (Map) response.getEntity(); } assertThat(entity.get("id")).isEqualTo("wf-1"); assertThat(entity.get("version")).isEqualTo("1.0.0"); assertThat(entity.get("created")).isEqualTo(true); + verify(registryService).pushWorkflow("tenant-1", wf); } @Test void shouldReturn200WhenUpdatingExistingWorkflow() { - Workflow workflow = createTestWorkflow("wf-1", "2.0.0"); - when(workflowRepository.exists("tenant-1", "wf-1")).thenReturn(true); + Workflow wf = workflow("wf-1", "2.0.0"); + when(registryService.pushWorkflow("tenant-1", wf)).thenReturn(false); Map entity; - try (Response response = resource.pushWorkflow(workflow)) { - + try (Response response = resource.pushWorkflow(wf)) { assertThat(response.getStatus()).isEqualTo(200); - verify(workflowRepository).save("tenant-1", workflow); - entity = (Map) response.getEntity(); } assertThat(entity.get("created")).isEqualTo(false); } + + @Test + void shouldReturn400WhenRegistryRejectsCycle() { + Workflow wf = workflow("wf-1", "1.0.0"); + when(registryService.pushWorkflow("tenant-1", wf)) + .thenThrow( + new IllegalStateException( + "Sub-workflow reference cycle detected: wf-1 -> wf-2 -> wf-1")); + + assertThatThrownBy( + () -> { + try (var _ = resource.pushWorkflow(wf)) { + // closed by try + } + }) + .isInstanceOf(BadRequestException.class) + .hasMessageContaining("cycle"); + } } @Nested @@ -86,24 +102,24 @@ class PullWorkflow { @Test void shouldReturnWorkflowWhenFound() { - Workflow workflow = createTestWorkflow("wf-1", "1.0.0"); - when(workflowRepository.findById("tenant-1", "wf-1")).thenReturn(Optional.of(workflow)); + Workflow wf = workflow("wf-1", "1.0.0"); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(wf); try (Response response = resource.pullWorkflow("wf-1")) { - assertThat(response.getStatus()).isEqualTo(200); - assertThat(response.getEntity()).isEqualTo(workflow); + assertThat(response.getEntity()).isEqualTo(wf); } } @Test - void shouldReturn404WhenWorkflowNotFound() { - when(workflowRepository.findById("tenant-1", "wf-1")).thenReturn(Optional.empty()); + void shouldReturn404WhenRegistryThrowsWorkflowNotFound() { + when(registryService.getWorkflow("tenant-1", "wf-1")) + .thenThrow(new WorkflowNotFoundException("Workflow not found: wf-1")); assertThatThrownBy( () -> { try (var _ = resource.pullWorkflow("wf-1")) { - // No-op + // closed by try } }) .isInstanceOf(NotFoundException.class) @@ -115,35 +131,18 @@ void shouldReturn404WhenWorkflowNotFound() { class ListWorkflows { @Test - void shouldReturnAllWorkflowsForTenant() { - List workflows = - List.of( - createTestWorkflow("wf-1", "1.0.0"), - createTestWorkflow("wf-2", "2.0.0")); - when(workflowRepository.findAll("tenant-1")).thenReturn(workflows); + void shouldMapWorkflowsToIdVersionSummaries() { + when(registryService.listWorkflows("tenant-1")) + .thenReturn(List.of(workflow("wf-1", "1.0.0"), workflow("wf-2", "2.0.0"))); List> entity; try (Response response = resource.listWorkflows()) { - assertThat(response.getStatus()).isEqualTo(200); entity = (List>) response.getEntity(); } assertThat(entity).hasSize(2); - assertThat(entity.get(0).get("id")).isEqualTo("wf-1"); - assertThat(entity.get(1).get("id")).isEqualTo("wf-2"); - } - - @Test - void shouldReturnEmptyListWhenNoWorkflows() { - when(workflowRepository.findAll("tenant-1")).thenReturn(List.of()); - - List> entity; - try (Response response = resource.listWorkflows()) { - - assertThat(response.getStatus()).isEqualTo(200); - entity = (List>) response.getEntity(); - } - assertThat(entity).isEmpty(); + assertThat(entity.get(0)).containsEntry("id", "wf-1").containsEntry("version", "1.0.0"); + assertThat(entity.get(1)).containsEntry("id", "wf-2").containsEntry("version", "2.0.0"); } } @@ -151,24 +150,23 @@ void shouldReturnEmptyListWhenNoWorkflows() { class DeleteWorkflow { @Test - void shouldReturn204WhenWorkflowDeleted() { - when(workflowRepository.delete("tenant-1", "wf-1")).thenReturn(true); + void shouldReturn204WhenRegistryDeletes() { + when(registryService.deleteWorkflow("tenant-1", "wf-1")).thenReturn(true); try (Response response = resource.deleteWorkflow("wf-1")) { - assertThat(response.getStatus()).isEqualTo(204); } - verify(workflowRepository).delete("tenant-1", "wf-1"); + verify(registryService).deleteWorkflow("tenant-1", "wf-1"); } @Test - void shouldReturn404WhenWorkflowNotFound() { - when(workflowRepository.delete("tenant-1", "wf-1")).thenReturn(false); + void shouldReturn404WhenRegistryReportsNothingDeleted() { + when(registryService.deleteWorkflow("tenant-1", "wf-1")).thenReturn(false); assertThatThrownBy( () -> { try (var _ = resource.deleteWorkflow("wf-1")) { - // No-op + // closed by try } }) .isInstanceOf(NotFoundException.class) diff --git a/hensu-server/src/test/java/io/hensu/server/integration/ApprovalRoutingIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/ApprovalRoutingIntegrationTest.java index 99cb5c7..47761a7 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/ApprovalRoutingIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/ApprovalRoutingIntegrationTest.java @@ -4,7 +4,7 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import java.util.List; import java.util.Map; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/IntegrationTestBase.java b/hensu-server/src/test/java/io/hensu/server/integration/IntegrationTestBase.java index b8a2518..e003bf6 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/IntegrationTestBase.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/IntegrationTestBase.java @@ -12,8 +12,8 @@ import io.hensu.serialization.WorkflowSerializer; import io.hensu.server.tenant.TenantContext; import io.hensu.server.tenant.TenantContext.TenantInfo; +import io.hensu.server.workflow.ExecutionStartResult; import io.hensu.server.workflow.WorkflowService; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.junit.TestProfile; import io.smallrye.mutiny.Uni; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/McpActionIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/McpActionIntegrationTest.java index 0d59a2d..29f205e 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/McpActionIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/McpActionIntegrationTest.java @@ -8,7 +8,7 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; import io.hensu.server.mcp.McpSessionManager; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import io.smallrye.mutiny.Multi; import io.smallrye.mutiny.Uni; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/ParallelNodeIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/ParallelNodeIntegrationTest.java index eab5c7c..7e0ca7d 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/ParallelNodeIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/ParallelNodeIntegrationTest.java @@ -7,7 +7,7 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import java.util.List; import java.util.Map; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/PlanExecutionIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/PlanExecutionIntegrationTest.java index 92403e3..c6e359c 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/PlanExecutionIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/PlanExecutionIntegrationTest.java @@ -6,7 +6,7 @@ import io.hensu.core.execution.action.ActionExecutor.ActionResult; import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import jakarta.inject.Inject; import java.util.List; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/ReviewAndBacktrackIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/ReviewAndBacktrackIntegrationTest.java index ca8160c..bac559d 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/ReviewAndBacktrackIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/ReviewAndBacktrackIntegrationTest.java @@ -6,7 +6,7 @@ import io.hensu.core.review.ReviewDecision; import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import jakarta.inject.Inject; import java.util.List; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/RubricEvaluationIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/RubricEvaluationIntegrationTest.java index 596252e..f96d45f 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/RubricEvaluationIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/RubricEvaluationIntegrationTest.java @@ -7,7 +7,7 @@ import io.hensu.core.rubric.model.Rubric; import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import java.nio.file.Path; import java.util.List; @@ -30,7 +30,7 @@ /// `registerRubricIfAbsent()` skip file parsing entirely. /// /// ### Score Normalization -/// The [DefaultRubricEvaluator][io.hensu.core.rubric.evaluator.DefaultRubricEvaluator] +/// The [ScoreExtractingEvaluator][io.hensu.core.rubric.evaluator.ScoreExtractingEvaluator] /// extracts a self-reported `score` from the agent's JSON output and returns it /// directly. The [RubricEngine][io.hensu.core.rubric.RubricEngine] then computes /// the final score as `(weightedSum / maxWeight) * 100`. With a single default @@ -44,7 +44,7 @@ /// /// @see IntegrationTestBase for shared test infrastructure /// @see io.hensu.core.rubric.RubricEngine for evaluation logic -/// @see io.hensu.core.rubric.evaluator.DefaultRubricEvaluator for score extraction +/// @see io.hensu.core.rubric.evaluator.ScoreExtractingEvaluator for score extraction @QuarkusTest class RubricEvaluationIntegrationTest extends IntegrationTestBase { @@ -52,14 +52,14 @@ class RubricEvaluationIntegrationTest extends IntegrationTestBase { /// self-reported score exceeds the rubric pass threshold. /// /// The stub response includes a JSON `score` field that the - /// [DefaultRubricEvaluator][io.hensu.core.rubric.evaluator.DefaultRubricEvaluator] + /// [ScoreExtractingEvaluator][io.hensu.core.rubric.evaluator.ScoreExtractingEvaluator] /// extracts as the self-evaluation score. A normalized score of `0.85` /// yields a final score of 85, which exceeds the default rubric pass /// threshold (70). No backtracking occurs and the workflow transitions /// directly to the end node. @Test void shouldCompleteWhenRubricPasses() { - parseAndRegisterRubric("quality", "quality-high.md"); + parseAndRegisterRubric("quality-high.md"); Workflow workflow = loadWorkflow("rubric-evaluation-pass.json"); registerStub( @@ -97,7 +97,7 @@ void shouldCompleteWhenRubricPasses() { /// execution history, confirming the retry mechanism was triggered. @Test void shouldRetryAndCompleteOnMinorRubricFailure() { - parseAndRegisterRubric("quality", "quality-low.md"); + parseAndRegisterRubric("quality-low.md"); Workflow workflow = loadWorkflow("rubric-backtrack-critical.json"); registerStub("research", "Research findings about quantum computing."); @@ -134,16 +134,15 @@ void shouldRetryAndCompleteOnMinorRubricFailure() { /// is replaced by building a new rubric with the desired `rubricId`, /// preserving all criteria and thresholds from the original file. /// - /// @param rubricId the ID the workflow expects (e.g. `"quality"`), not null /// @param resourceName rubric file under `/rubrics/` (e.g. `"quality-high.md"`), not null - private void parseAndRegisterRubric(String rubricId, String resourceName) { + private void parseAndRegisterRubric(String resourceName) { String rubricPath = resolveRubricPath(resourceName); Rubric parsed = RubricParser.parse(Path.of(rubricPath)); // Rebuild with the rubricId the workflow expects Rubric rubric = Rubric.builder() - .id(rubricId) + .id("quality") .name(parsed.getName()) .version(parsed.getVersion()) .type(parsed.getType()) diff --git a/hensu-server/src/test/java/io/hensu/server/integration/SpecializedNodeIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/SpecializedNodeIntegrationTest.java index fe11bd3..942ca56 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/SpecializedNodeIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/SpecializedNodeIntegrationTest.java @@ -5,7 +5,7 @@ import io.hensu.core.execution.action.ActionExecutor; import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import jakarta.inject.Inject; import java.util.List; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/StandardNodeIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/StandardNodeIntegrationTest.java index 75bc313..ebd981e 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/StandardNodeIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/StandardNodeIntegrationTest.java @@ -4,7 +4,7 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import java.util.List; import java.util.Map; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/StatePersistenceIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/StatePersistenceIntegrationTest.java index 38d5b62..71b149b 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/StatePersistenceIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/StatePersistenceIntegrationTest.java @@ -4,9 +4,9 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; +import io.hensu.server.workflow.ExecutionStartResult; +import io.hensu.server.workflow.ResumeDecision; import io.hensu.server.workflow.WorkflowService; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; -import io.hensu.server.workflow.WorkflowService.ResumeDecision; import io.quarkus.test.junit.QuarkusTest; import jakarta.inject.Inject; import java.util.List; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/SubWorkflowIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/SubWorkflowIntegrationTest.java index 1154a7f..acd47d1 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/SubWorkflowIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/SubWorkflowIntegrationTest.java @@ -4,7 +4,7 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; +import io.hensu.server.workflow.ExecutionStartResult; import io.quarkus.test.junit.QuarkusTest; import java.util.List; import java.util.Map; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/WorkflowLifecycleIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/WorkflowLifecycleIntegrationTest.java index 52accda..5b9bcf5 100644 --- a/hensu-server/src/test/java/io/hensu/server/integration/WorkflowLifecycleIntegrationTest.java +++ b/hensu-server/src/test/java/io/hensu/server/integration/WorkflowLifecycleIntegrationTest.java @@ -5,8 +5,8 @@ import io.hensu.core.state.HensuSnapshot; import io.hensu.core.workflow.Workflow; -import io.hensu.server.workflow.WorkflowService.ExecutionStartResult; -import io.hensu.server.workflow.WorkflowService.WorkflowNotFoundException; +import io.hensu.server.workflow.ExecutionStartResult; +import io.hensu.server.workflow.WorkflowNotFoundException; import io.quarkus.test.junit.QuarkusTest; import java.util.List; import java.util.Map; diff --git a/hensu-server/src/test/java/io/hensu/server/integration/WorkflowRegistryIntegrationTest.java b/hensu-server/src/test/java/io/hensu/server/integration/WorkflowRegistryIntegrationTest.java new file mode 100644 index 0000000..25647a2 --- /dev/null +++ b/hensu-server/src/test/java/io/hensu/server/integration/WorkflowRegistryIntegrationTest.java @@ -0,0 +1,59 @@ +package io.hensu.server.integration; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.hensu.core.execution.result.ExitStatus; +import io.hensu.core.workflow.Workflow; +import io.hensu.core.workflow.node.EndNode; +import io.hensu.core.workflow.node.Node; +import io.hensu.core.workflow.node.SubWorkflowNode; +import io.hensu.server.workflow.WorkflowRegistryService; +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +/// Full-stack verification that {@link WorkflowRegistryService} rejects cyclic pushes +/// atomically with the live CDI-wired repository. +/// +/// The validator logic is covered by {@code SubWorkflowGraphValidatorTest} and REST +/// exception mapping by {@code WorkflowResourceTest}. The gap only real wiring can +/// expose is: a rejected push must leave the existing workflow byte-identical to its +/// pre-push state — validate-then-save must not silently swap order in production. +@QuarkusTest +class WorkflowRegistryIntegrationTest extends IntegrationTestBase { + + @Inject WorkflowRegistryService registryService; + + @Test + void shouldRejectClosingCycleAndPreserveExistingWorkflow() { + Workflow cleanA = cleanWorkflow(); + Workflow bReferencingA = withSubWorkflowNode("b", "a"); + registryService.pushWorkflow(TEST_TENANT, cleanA); + registryService.pushWorkflow(TEST_TENANT, bReferencingA); + + Workflow aClosingCycle = withSubWorkflowNode("a", "b"); + assertThatThrownBy(() -> registryService.pushWorkflow(TEST_TENANT, aClosingCycle)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("cycle"); + + Workflow storedA = workflowRepository.findById(TEST_TENANT, "a").orElseThrow(); + assertThat(storedA.getNodes().values()) + .as("stored A must not have been overwritten by the rejected cyclic push") + .noneMatch(SubWorkflowNode.class::isInstance); + } + + private static Workflow cleanWorkflow() { + Map nodes = new LinkedHashMap<>(); + nodes.put("end", EndNode.builder().id("end").status(ExitStatus.SUCCESS).build()); + return Workflow.builder().id("a").startNode("end").nodes(nodes).build(); + } + + private static Workflow withSubWorkflowNode(String id, String targetWorkflowId) { + Map nodes = new LinkedHashMap<>(); + nodes.put("sub", SubWorkflowNode.builder().id("sub").workflowId(targetWorkflowId).build()); + return Workflow.builder().id(id).startNode("sub").nodes(nodes).build(); + } +} diff --git a/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionQueryServiceTest.java b/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionQueryServiceTest.java new file mode 100644 index 0000000..081be83 --- /dev/null +++ b/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionQueryServiceTest.java @@ -0,0 +1,184 @@ +package io.hensu.server.workflow; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.hensu.core.plan.PlanSnapshot; +import io.hensu.core.plan.PlanSnapshot.PlannedStepSnapshot; +import io.hensu.core.state.HensuSnapshot; +import io.hensu.core.state.WorkflowStateRepository; +import java.time.Instant; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +class ExecutionQueryServiceTest { + + private WorkflowStateRepository stateRepository; + private ExecutionQueryService service; + + @BeforeEach + void setUp() { + stateRepository = mock(WorkflowStateRepository.class); + service = new ExecutionQueryService(stateRepository); + } + + private HensuSnapshot snapshot(String workflowId, String executionId, String currentNodeId) { + return new HensuSnapshot( + workflowId, executionId, currentNodeId, Map.of(), null, null, Instant.now(), null); + } + + @Nested + class GetExecutionStatus { + + @Test + void shouldMapSnapshotFieldsIntoStatus() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(snapshot("wf-1", "exec-1", "node-1"))); + + ExecutionStatus status = service.getExecutionStatus("tenant-1", "exec-1"); + + assertThat(status.executionId()).isEqualTo("exec-1"); + assertThat(status.workflowId()).isEqualTo("wf-1"); + assertThat(status.status()).isEqualTo("PAUSED"); + assertThat(status.currentNodeId()).isEqualTo("node-1"); + } + + @Test + void shouldReportCompletedWhenSnapshotHasNoCurrentNode() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(snapshot("wf-1", "exec-1", null))); + + ExecutionStatus status = service.getExecutionStatus("tenant-1", "exec-1"); + + assertThat(status.status()).isEqualTo("COMPLETED"); + } + + @Test + void shouldThrowExecutionNotFoundWhenSnapshotMissing() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.getExecutionStatus("tenant-1", "exec-1")) + .isInstanceOf(ExecutionNotFoundException.class) + .hasMessageContaining("exec-1"); + } + } + + @Nested + class ListPausedExecutions { + + @Test + void shouldMapSnapshotsIntoSummaries() { + when(stateRepository.findPaused("tenant-1")) + .thenReturn( + List.of( + snapshot("wf-1", "exec-1", "node-1"), + snapshot("wf-2", "exec-2", "node-2"))); + + List paused = service.listPausedExecutions("tenant-1"); + + assertThat(paused).hasSize(2); + assertThat(paused.getFirst().executionId()).isEqualTo("exec-1"); + assertThat(paused.get(0).workflowId()).isEqualTo("wf-1"); + assertThat(paused.get(0).currentNodeId()).isEqualTo("node-1"); + assertThat(paused.get(1).executionId()).isEqualTo("exec-2"); + } + } + + @Nested + class GetPendingPlan { + + @Test + void shouldReturnEmptyWhenSnapshotHasNoActivePlan() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(snapshot("wf-1", "exec-1", "node-1"))); + + Optional plan = service.getPendingPlan("tenant-1", "exec-1"); + + assertThat(plan).isEmpty(); + } + + @Test + void shouldReturnPlanInfoWithCorrectFieldOrderWhenPlanActive() { + PlanSnapshot active = + new PlanSnapshot( + "review-node", + List.of( + new PlannedStepSnapshot( + 0, "tool-a", Map.of(), "first", "PENDING"), + new PlannedStepSnapshot( + 1, "tool-b", Map.of(), "second", "PENDING"), + new PlannedStepSnapshot( + 2, "tool-c", Map.of(), "third", "PENDING")), + 1, + List.of()); + HensuSnapshot withPlan = + new HensuSnapshot( + "wf-1", + "exec-1", + "review-node", + Map.of(), + null, + active, + Instant.now(), + null); + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(withPlan)); + + PlanInfo info = service.getPendingPlan("tenant-1", "exec-1").orElseThrow(); + + assertThat(info.planId()).isEqualTo("review-node"); + assertThat(info.totalSteps()).isEqualTo(3); + assertThat(info.currentStep()).isEqualTo(1); + } + } + + @Nested + class GetExecutionResult { + + private HensuSnapshot withContext(Map context) { + return new HensuSnapshot( + "wf-1", "exec-1", null, context, null, null, Instant.now(), "completed"); + } + + @Test + void shouldFilterUnderscorePrefixedKeysFromOutput() { + Map mixed = new HashMap<>(); + mixed.put("_tenant_id", "tenant-1"); + mixed.put("_execution_id", "exec-1"); + mixed.put("_last_output", "internal routing value"); + mixed.put("summary", "Order processed successfully"); + mixed.put("approved", true); + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(withContext(mixed))); + + ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); + + assertThat(output.output()).doesNotContainKey("_tenant_id"); + assertThat(output.output()).doesNotContainKey("_execution_id"); + assertThat(output.output()).doesNotContainKey("_last_output"); + assertThat(output.output()).containsEntry("summary", "Order processed successfully"); + assertThat(output.output()).containsEntry("approved", true); + } + + @Test + void shouldReturnEmptyMapWhenAllKeysAreInternal() { + Map internalOnly = new HashMap<>(); + internalOnly.put("_tenant_id", "tenant-1"); + internalOnly.put("_execution_id", "exec-1"); + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(withContext(internalOnly))); + + ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); + + assertThat(output.output()).isEmpty(); + } + } +} diff --git a/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionStateServiceTest.java b/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionStateServiceTest.java new file mode 100644 index 0000000..96ef0db --- /dev/null +++ b/hensu-server/src/test/java/io/hensu/server/workflow/ExecutionStateServiceTest.java @@ -0,0 +1,220 @@ +package io.hensu.server.workflow; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.hensu.core.execution.WorkflowExecutor; +import io.hensu.core.execution.result.ExecutionResult; +import io.hensu.core.execution.result.ExitStatus; +import io.hensu.core.state.HensuSnapshot; +import io.hensu.core.state.HensuState; +import io.hensu.core.state.WorkflowStateRepository; +import io.hensu.core.workflow.Workflow; +import io.hensu.server.persistence.ExecutionLeaseManager; +import java.time.Instant; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +class ExecutionStateServiceTest { + + private WorkflowExecutor workflowExecutor; + private WorkflowStateRepository stateRepository; + private WorkflowRegistryService registryService; + private ExecutionLeaseManager leaseManager; + private ExecutionStateService service; + + @BeforeEach + void setUp() { + workflowExecutor = mock(WorkflowExecutor.class); + stateRepository = mock(WorkflowStateRepository.class); + registryService = mock(WorkflowRegistryService.class); + leaseManager = mock(ExecutionLeaseManager.class); + when(leaseManager.tryClaim(any(), any())).thenReturn(true); + service = + new ExecutionStateService( + workflowExecutor, stateRepository, registryService, leaseManager); + } + + private HensuSnapshot pausedSnapshot() { + return new HensuSnapshot( + "wf-1", + "exec-1", + "node-1", + new HashMap<>(Map.of("k", "v")), + null, + null, + Instant.now(), + "paused"); + } + + private HensuState completedState() { + return new HensuState.Builder() + .workflowId("wf-1") + .executionId("exec-1") + .currentNode("done") + .build(); + } + + @Test + void shouldThrowExecutionNotFoundWhenSnapshotMissing() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")).thenReturn(Optional.empty()); + + assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) + .isInstanceOf(ExecutionNotFoundException.class) + .hasMessageContaining("exec-1"); + } + + @Test + void shouldApplyDecisionModificationsToContextBeforeExecuting() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenReturn(new ExecutionResult.Completed(completedState(), ExitStatus.SUCCESS)); + + ResumeDecision decision = + new ResumeDecision(true, Map.of("approved", true, "reviewer_note", "looks good")); + + service.resumeExecution("tenant-1", "exec-1", decision); + + ArgumentCaptor stateCaptor = ArgumentCaptor.forClass(HensuState.class); + verify(workflowExecutor).executeFrom(any(), stateCaptor.capture(), any()); + Map ctx = stateCaptor.getValue().getContext(); + assertThat(ctx).containsEntry("approved", true); + assertThat(ctx).containsEntry("reviewer_note", "looks good"); + assertThat(ctx).containsEntry("k", "v"); + } + + @Test + void shouldPersistCompletedSnapshotWhenResumeReturnsCompleted() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenReturn(new ExecutionResult.Completed(completedState(), ExitStatus.SUCCESS)); + + service.resumeExecution("tenant-1", "exec-1", null); + + ArgumentCaptor savedCaptor = ArgumentCaptor.forClass(HensuSnapshot.class); + verify(stateRepository).save(eq("tenant-1"), savedCaptor.capture()); + assertThat(savedCaptor.getValue().checkpointReason()).isEqualTo("completed"); + } + + @Test + void shouldPersistPausedSnapshotWhenResumeReturnsPaused() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenReturn(new ExecutionResult.Paused(completedState())); + + service.resumeExecution("tenant-1", "exec-1", null); + + ArgumentCaptor savedCaptor = ArgumentCaptor.forClass(HensuSnapshot.class); + verify(stateRepository).save(eq("tenant-1"), savedCaptor.capture()); + assertThat(savedCaptor.getValue().checkpointReason()).isEqualTo("paused"); + } + + @Test + void shouldPersistRejectedSnapshotWhenResumeReturnsRejected() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenReturn(new ExecutionResult.Rejected("policy violation", completedState())); + + service.resumeExecution("tenant-1", "exec-1", null); + + ArgumentCaptor savedCaptor = ArgumentCaptor.forClass(HensuSnapshot.class); + verify(stateRepository).save(eq("tenant-1"), savedCaptor.capture()); + assertThat(savedCaptor.getValue().checkpointReason()).isEqualTo("rejected"); + } + + @Test + void shouldPersistFailureSnapshotWhenResumeReturnsFailure() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenReturn( + new ExecutionResult.Failure( + completedState(), new IllegalStateException("boom"))); + + service.resumeExecution("tenant-1", "exec-1", null); + + ArgumentCaptor savedCaptor = ArgumentCaptor.forClass(HensuSnapshot.class); + verify(stateRepository).save(eq("tenant-1"), savedCaptor.capture()); + assertThat(savedCaptor.getValue().checkpointReason()).isEqualTo("failed"); + } + + @Test + void shouldWrapExecutorRuntimeExceptionAsWorkflowExecutionException() throws Exception { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenThrow(new RuntimeException("executor crashed")); + + assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) + .isInstanceOf(WorkflowExecutionException.class) + .hasMessageContaining("Resume failed") + .hasRootCauseMessage("executor crashed"); + } + + @Test + void shouldWrapRegistryNotFoundAsWorkflowExecutionException() { + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")) + .thenThrow(new WorkflowNotFoundException("Workflow not found: wf-1")); + + assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) + .isInstanceOf(WorkflowExecutionException.class) + .hasCauseInstanceOf(WorkflowNotFoundException.class); + } + + @Test + void shouldRejectResumeAndSkipExecutorWhenLeaseHeldByAnotherNode() throws Exception { + // Closes the split-brain window: an API-edge resume must refuse to drive the + // executor when the lease row is owned by a different node. tryClaim returning + // false is the only signal the service has — verify we throw and never touch + // the state repo, executor, or release path. + when(leaseManager.tryClaim("tenant-1", "exec-1")).thenReturn(false); + + assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Execution owned by another node") + .hasMessageContaining("exec-1"); + + verify(stateRepository, never()).findByExecutionId(any(), any()); + verify(workflowExecutor, never()).executeFrom(any(), any(), any()); + verify(leaseManager, never()).release(any(), any()); + } + + @Test + void shouldReleaseLeaseEvenWhenExecutorThrows() throws Exception { + // Defends against lease leakage on failure. Without the finally block a + // crashed execution stays locked to this node until the next 60 s recovery + // sweep, blocking manual retries and masking real outages. + when(stateRepository.findByExecutionId("tenant-1", "exec-1")) + .thenReturn(Optional.of(pausedSnapshot())); + when(registryService.getWorkflow("tenant-1", "wf-1")).thenReturn(mock(Workflow.class)); + when(workflowExecutor.executeFrom(any(), any(), any())) + .thenThrow(new RuntimeException("executor crashed")); + + assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) + .isInstanceOf(WorkflowExecutionException.class); + + verify(leaseManager).release("tenant-1", "exec-1"); + } +} diff --git a/hensu-server/src/test/java/io/hensu/server/workflow/WorkflowServiceTest.java b/hensu-server/src/test/java/io/hensu/server/workflow/WorkflowServiceTest.java deleted file mode 100644 index d128a93..0000000 --- a/hensu-server/src/test/java/io/hensu/server/workflow/WorkflowServiceTest.java +++ /dev/null @@ -1,329 +0,0 @@ -package io.hensu.server.workflow; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import io.hensu.core.execution.WorkflowExecutor; -import io.hensu.core.state.HensuSnapshot; -import io.hensu.core.state.WorkflowStateRepository; -import io.hensu.core.workflow.Workflow; -import io.hensu.core.workflow.WorkflowRepository; -import io.hensu.server.streaming.ExecutionEventBroadcaster; -import io.hensu.server.workflow.WorkflowService.ExecutionNotFoundException; -import io.hensu.server.workflow.WorkflowService.ExecutionOutput; -import io.hensu.server.workflow.WorkflowService.ExecutionStatus; -import io.hensu.server.workflow.WorkflowService.ExecutionSummary; -import java.time.Instant; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; - -class WorkflowServiceTest { - - private WorkflowExecutor workflowExecutor; - private WorkflowStateRepository stateRepository; - private ExecutionEventBroadcaster eventBroadcaster; - private WorkflowRepository workflowRepository; - private WorkflowService service; - - @BeforeEach - void setUp() { - workflowExecutor = mock(WorkflowExecutor.class); - stateRepository = mock(WorkflowStateRepository.class); - eventBroadcaster = mock(ExecutionEventBroadcaster.class); - workflowRepository = mock(WorkflowRepository.class); - service = - new WorkflowService( - workflowExecutor, stateRepository, eventBroadcaster, workflowRepository); - } - - private HensuSnapshot createSnapshot( - String workflowId, String executionId, String currentNodeId) { - return new HensuSnapshot( - workflowId, executionId, currentNodeId, Map.of(), null, null, Instant.now(), null); - } - - @Nested - class GetExecutionStatus { - - @Test - void shouldReturnStatusForExistingExecution() { - HensuSnapshot snapshot = createSnapshot("wf-1", "exec-1", "node-1"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshot)); - - ExecutionStatus status = service.getExecutionStatus("tenant-1", "exec-1"); - - assertThat(status.executionId()).isEqualTo("exec-1"); - assertThat(status.workflowId()).isEqualTo("wf-1"); - assertThat(status.status()).isEqualTo("PAUSED"); - assertThat(status.currentNodeId()).isEqualTo("node-1"); - } - - @Test - void shouldReturnCompletedStatusWhenNoCurrentNode() { - HensuSnapshot snapshot = createSnapshot("wf-1", "exec-1", null); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshot)); - - ExecutionStatus status = service.getExecutionStatus("tenant-1", "exec-1"); - - assertThat(status.status()).isEqualTo("COMPLETED"); - } - - @Test - void shouldThrowWhenExecutionNotFound() { - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> service.getExecutionStatus("tenant-1", "exec-1")) - .isInstanceOf(ExecutionNotFoundException.class) - .hasMessageContaining("exec-1"); - } - - @Test - void shouldRejectNullTenantId() { - assertThatThrownBy(() -> service.getExecutionStatus(null, "exec-1")) - .isInstanceOf(NullPointerException.class); - } - - @Test - void shouldRejectNullExecutionId() { - assertThatThrownBy(() -> service.getExecutionStatus("tenant-1", null)) - .isInstanceOf(NullPointerException.class); - } - } - - @Nested - class ListPausedExecutions { - - @Test - void shouldReturnPausedExecutions() { - List snapshots = - List.of( - createSnapshot("wf-1", "exec-1", "node-1"), - createSnapshot("wf-2", "exec-2", "node-2")); - when(stateRepository.findPaused("tenant-1")).thenReturn(snapshots); - - List paused = service.listPausedExecutions("tenant-1"); - - assertThat(paused).hasSize(2); - assertThat(paused.get(0).executionId()).isEqualTo("exec-1"); - assertThat(paused.get(1).executionId()).isEqualTo("exec-2"); - } - - @Test - void shouldReturnEmptyListWhenNoPausedExecutions() { - when(stateRepository.findPaused("tenant-1")).thenReturn(List.of()); - - List paused = service.listPausedExecutions("tenant-1"); - - assertThat(paused).isEmpty(); - } - } - - @Nested - class ResumeExecution { - - @Test - void shouldLoadStateForResume() throws Exception { - HensuSnapshot snapshot = createSnapshot("wf-1", "exec-1", "node-1"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshot)); - when(workflowRepository.findById("tenant-1", "wf-1")) - .thenReturn(Optional.of(mock(Workflow.class))); - when(workflowExecutor.executeFrom(any(), any())).thenReturn(null); - - service.resumeExecution("tenant-1", "exec-1", null); - - verify(stateRepository).findByExecutionId("tenant-1", "exec-1"); - } - - @Test - void shouldThrowWhenExecutionNotFoundForResume() { - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> service.resumeExecution("tenant-1", "exec-1", null)) - .isInstanceOf(ExecutionNotFoundException.class); - } - } - - @Nested - class GetPendingPlan { - - @Test - void shouldReturnEmptyWhenNoActivePlan() { - HensuSnapshot snapshot = createSnapshot("wf-1", "exec-1", "node-1"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshot)); - - Optional plan = service.getPendingPlan("tenant-1", "exec-1"); - - assertThat(plan).isEmpty(); - } - - @Test - void shouldThrowWhenExecutionNotFound() { - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> service.getPendingPlan("tenant-1", "exec-1")) - .isInstanceOf(ExecutionNotFoundException.class); - } - } - - @Nested - class GetExecutionResult { - - private HensuSnapshot snapshotWithContext(Map context, String reason) { - return new HensuSnapshot( - "wf-1", "exec-1", null, context, null, null, Instant.now(), reason); - } - - @Test - void shouldFilterInternalKeysFromOutput() { - Map mixedContext = new HashMap<>(); - mixedContext.put("_tenant_id", "tenant-1"); - mixedContext.put("_execution_id", "exec-1"); - mixedContext.put("_last_output", "internal routing value"); - mixedContext.put("summary", "Order processed successfully"); - mixedContext.put("approved", true); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshotWithContext(mixedContext, "completed"))); - - ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); - - assertThat(output.output()).doesNotContainKey("_tenant_id"); - assertThat(output.output()).doesNotContainKey("_execution_id"); - assertThat(output.output()).doesNotContainKey("_last_output"); - assertThat(output.output()).containsEntry("summary", "Order processed successfully"); - assertThat(output.output()).containsEntry("approved", true); - } - - @Test - void shouldReturnEmptyOutputWhenAllContextKeysAreInternal() { - Map internalOnly = new HashMap<>(); - internalOnly.put("_tenant_id", "tenant-1"); - internalOnly.put("_execution_id", "exec-1"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshotWithContext(internalOnly, "completed"))); - - ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); - - assertThat(output.output()).isEmpty(); - } - - @Test - void shouldReturnCorrectStatusAndIdentifiers() { - Map context = new HashMap<>(); - context.put("result", "done"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(snapshotWithContext(context, "completed"))); - - ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); - - assertThat(output.executionId()).isEqualTo("exec-1"); - assertThat(output.workflowId()).isEqualTo("wf-1"); - assertThat(output.status()).isEqualTo("COMPLETED"); - assertThat(output.output()).containsKey("result"); - } - - @Test - void shouldReportPausedStatusForNonCompletedSnapshot() { - Map context = new HashMap<>(); - context.put("step", "validation"); - // currentNodeId is non-null and reason is not "completed" → PAUSED - HensuSnapshot paused = - new HensuSnapshot( - "wf-1", - "exec-1", - "validation-node", - context, - null, - null, - Instant.now(), - "checkpoint"); - when(stateRepository.findByExecutionId("tenant-1", "exec-1")) - .thenReturn(Optional.of(paused)); - - ExecutionOutput output = service.getExecutionResult("tenant-1", "exec-1"); - - assertThat(output.status()).isEqualTo("PAUSED"); - } - - @Test - void shouldThrowWhenExecutionNotFound() { - when(stateRepository.findByExecutionId("tenant-1", "missing")) - .thenReturn(Optional.empty()); - - assertThatThrownBy(() -> service.getExecutionResult("tenant-1", "missing")) - .isInstanceOf(ExecutionNotFoundException.class) - .hasMessageContaining("missing"); - } - } - - @Nested - class ConstructorValidation { - - @Test - void shouldRejectNullWorkflowExecutor() { - assertThatThrownBy( - () -> - new WorkflowService( - null, - stateRepository, - eventBroadcaster, - workflowRepository)) - .isInstanceOf(NullPointerException.class) - .hasMessageContaining("workflowExecutor"); - } - - @Test - void shouldRejectNullStateRepository() { - assertThatThrownBy( - () -> - new WorkflowService( - workflowExecutor, - null, - eventBroadcaster, - workflowRepository)) - .isInstanceOf(NullPointerException.class) - .hasMessageContaining("stateRepository"); - } - - @Test - void shouldRejectNullEventBroadcaster() { - assertThatThrownBy( - () -> - new WorkflowService( - workflowExecutor, - stateRepository, - null, - workflowRepository)) - .isInstanceOf(NullPointerException.class) - .hasMessageContaining("eventBroadcaster"); - } - - @Test - void shouldRejectNullWorkflowRepository() { - assertThatThrownBy( - () -> - new WorkflowService( - workflowExecutor, - stateRepository, - eventBroadcaster, - null)) - .isInstanceOf(NullPointerException.class) - .hasMessageContaining("workflowRepository"); - } - } -} diff --git a/working-dir/workflows/main-research.kt b/working-dir/workflows/main-research.kt new file mode 100644 index 0000000..1d87daf --- /dev/null +++ b/working-dir/workflows/main-research.kt @@ -0,0 +1,78 @@ +/** + * Parent workflow: `main-research`. + * + * Flow: + * 1. `write_article` – researcher drafts an article about {topic} + * 2. `delegate_summary` – delegates to the `sub-summarizer` child workflow, + * passing `draft` as `article_text` and receiving + * `summary` back as `tl_dr` + * 3. `publish` – composes the final deliverable using both vars + * + * Run (once DSL + CLI support exist): + * hensu run main-research --with sub-summarizer -d working-dir -v -c "{\"topic\": \"Project Loom\"}" + */ +fun mainResearch() = workflow("main-research") { + description = "Parent workflow that delegates summarization to a child sub-workflow" + version = "1.0.0" + + agents { + agent("researcher") { role = "Research Writer"; model = Models.GEMINI_3_1_PRO } + agent("editor") { role = "Publishing Editor"; model = Models.GEMINI_3_1_FLASH_LITE } + } + + state { + input("topic", VarType.STRING) + variable("draft", VarType.STRING, "full article draft produced by the researcher") + variable("tl_dr", VarType.STRING, "three-sentence summary returned by the sub-workflow") + variable("published", VarType.STRING, "final publishable deliverable (title + tl;dr + body)") + } + + graph { + start at "write_article" + + // 1. Draft the article in the parent context. + node("write_article") { + agent = "researcher" + prompt = "Write a detailed 4-paragraph article about {topic}." + writes("draft") + onSuccess goto "delegate_summary" + } + + // 2. Delegate to the child workflow. + // + // imports(...) – vars copied from parent state into the child context. + // Names must exist in both state schemas (same discipline + // as `writes` on a regular node — no renaming). + // writes(...) – mirrors what the sub-workflow writes. These vars land + // in parent state under the same names. + subWorkflow("delegate_summary") { + target = "sub-summarizer" + + imports("draft") + writes("tl_dr") + + onSuccess goto "publish" + onFailure goto "publish" // fall through; editor will handle a missing tl_dr + } + + // 3. Compose the final deliverable using parent + returned child var. + node("publish") { + agent = "editor" + prompt = """ + Compose a publishable post about {topic}. + + TL;DR (from summarizer): + {tl_dr} + + FULL DRAFT: + {draft} + + Output the final post with a title line, the TL;DR, then the body. + """.trimIndent() + writes("published") + onSuccess goto "done" + } + + end("done", ExitStatus.SUCCESS) + } +} diff --git a/working-dir/workflows/sub-summarizer.kt b/working-dir/workflows/sub-summarizer.kt new file mode 100644 index 0000000..65bee26 --- /dev/null +++ b/working-dir/workflows/sub-summarizer.kt @@ -0,0 +1,46 @@ +/** + * Child sub-workflow: `sub-summarizer`. + * + * Invoked by `main-research.kt` via a `subWorkflow { }` node. State vars are + * passed by name — the parent `imports("draft")` expects the child to declare + * `draft` as an input, and `writes("tl_dr")` expects the child to write `tl_dr`. + * No renaming; names are the contract. + */ +fun subSummarizer() = workflow("sub-summarizer") { + description = "Summarizes a draft passed in from a parent workflow" + version = "1.0.0" + + agents { + agent("summarizer") { + role = "Technical Summarizer" + model = Models.GEMINI_3_1_FLASH_LITE + temperature = 0.2 + } + } + + state { + // `draft` is populated by the parent via `imports("draft")`. + input("draft", VarType.STRING) + // `tl_dr` is read back by the parent via `writes("tl_dr")`. + variable("tl_dr", VarType.STRING, "three-sentence summary of the draft") + } + + graph { + start at "summarize" + + node("summarize") { + agent = "summarizer" + prompt = """ + Summarize the following article in exactly three sentences. + Preserve the key claims. No preamble, no bullet points. + + ARTICLE: + {draft} + """.trimIndent() + writes("tl_dr") + onSuccess goto "done" + } + + end("done", ExitStatus.SUCCESS) + } +} From 6291fc78d749b94558d568c6367bddc002e1ffd9 Mon Sep 17 00:00:00 2001 From: Aleksandr Suvorov Date: Sun, 19 Apr 2026 02:09:34 +0400 Subject: [PATCH 2/3] Potential fix for pull request finding 'CodeQL / Log Injection' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Aleksandr Suvorov --- .../io/hensu/server/workflow/WorkflowRegistryService.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java index 3bbea82..2bc55c3 100644 --- a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java @@ -4,6 +4,7 @@ import io.hensu.core.workflow.WorkflowRepository; import io.hensu.core.workflow.validation.SubWorkflowGraphValidator; import io.hensu.server.persistence.WorkflowPushLock; +import io.hensu.server.validation.LogSanitizer; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; import java.util.List; @@ -113,7 +114,10 @@ public List listWorkflows(String tenantId) { public boolean deleteWorkflow(String tenantId, String workflowId) { Objects.requireNonNull(tenantId, "tenantId must not be null"); Objects.requireNonNull(workflowId, "workflowId must not be null"); - LOG.infov("Delete workflow: id={0}, tenant={1}", workflowId, tenantId); + LOG.infov( + "Delete workflow: id={0}, tenant={1}", + LogSanitizer.sanitize(workflowId), + LogSanitizer.sanitize(tenantId)); return workflowRepository.delete(tenantId, workflowId); } } From 4520ce02d8e24275d8dde9e694e83ce771b38665 Mon Sep 17 00:00:00 2001 From: Aleksandr Suvorov Date: Sun, 19 Apr 2026 02:13:54 +0400 Subject: [PATCH 3/3] feat(sub-workflow): apply spotless Resolve: #64 Signed-off-by: Aleksandr Suvorov --- .../java/io/hensu/server/workflow/WorkflowRegistryService.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java index 2bc55c3..878e646 100644 --- a/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java +++ b/hensu-server/src/main/java/io/hensu/server/workflow/WorkflowRegistryService.java @@ -116,8 +116,7 @@ public boolean deleteWorkflow(String tenantId, String workflowId) { Objects.requireNonNull(workflowId, "workflowId must not be null"); LOG.infov( "Delete workflow: id={0}, tenant={1}", - LogSanitizer.sanitize(workflowId), - LogSanitizer.sanitize(tenantId)); + LogSanitizer.sanitize(workflowId), LogSanitizer.sanitize(tenantId)); return workflowRepository.delete(tenantId, workflowId); } }