feat: shell execution correctness + artifact_field_equals condition

- Add field to ProcessResultArtifact ("success"/"failed") - Materialize ToolResult.Failure as a failed ProcessResult artifact in the orchestrator, with artifact lifecycle events emitted - Non-recoverable ToolResult.Failure now fails the stage ("FATAL:") instead of being silently fed back as LLM context - Fix enterStage to return Continue on non-retryable failures, enabling back-edge transitions (fix/retry loops) - Add ArtifactFieldEquals transition condition with flat and nested dot-notation field access + FieldOperator enum (eq, neq, gt, gte, lt, lte) - Wire artifact_field_equals through ConditionFactory and TomlWorkflowLoader (condition_field, condition_operator) - Cache ProcessResult artifact content for evaluation context - Add 16 tests covering all condition operators and error cases - DefaultTransitionResolver catches condition evaluation errors and returns Blocked instead of crashing
2026-05-26 17:28:51 +04:00
parent 9734eec63c
commit 3b24c7e91a
11 changed files with 304 additions and 10 deletions
@@ -4,6 +4,7 @@ import kotlinx.serialization.Serializable

@Serializable
 data class ProcessResultArtifact(
+    val status: String = "success",
    val command: String,
    val exitCode: Int,
    val stdout: String,
@@ -10,13 +10,14 @@ object ProcessResultKind : ArtifactKind {
    override fun deriveJsonSchema(): JsonSchema = JsonSchema(
        type = "object",
        properties = mapOf(
+            "status" to JsonSchemaProperty(type = "string", description = "\"success\" or \"failed\""),
            "command" to JsonSchemaProperty(type = "string"),
            "exitCode" to JsonSchemaProperty(type = "integer"),
            "stdout" to JsonSchemaProperty(type = "string"),
            "stderr" to JsonSchemaProperty(type = "string"),
            "durationMs" to JsonSchemaProperty(type = "integer"),
        ),
-        required = listOf("command", "exitCode", "stdout", "stderr", "durationMs"),
+        required = listOf("status", "command", "exitCode", "stdout", "stderr", "durationMs"),
        additionalProperties = false,
    )
 }
@@ -53,6 +53,7 @@ class DefaultSessionOrchestrator(
        }
    }

+    @Suppress("LongMethod")
    private tailrec suspend fun step(ctx: EnrichedExecutionContext): WorkflowResult {
        log.debug(
            "[Orchestrator] step session={} stage={} stageCount={}",
@@ -82,7 +83,14 @@ class DefaultSessionOrchestrator(
            validated.associateWith { ArtifactState(phase = ArtifactLifecyclePhase.VALIDATED) }
        }

-        val decision = resolveTransition(enriched.graph, enriched.sessionId, enriched.currentStageId, stageArtifacts)
+        val artifactContent: Map<ArtifactId, String> = targetIds.mapNotNull { id ->
+            val cacheKey = "${enriched.sessionId.value}:${id.value}"
+            artifactContentCache[cacheKey]?.let { content -> id to content }
+        }.toMap()
+
+        val decision = resolveTransition(
+            enriched.graph, enriched.sessionId, enriched.currentStageId, stageArtifacts, artifactContent,
+        )
        log.debug(
            "[Orchestrator] transition session={} stage={} decision={}",
            enriched.sessionId.value, enriched.currentStageId.value, decision::class.simpleName,
@@ -174,6 +182,11 @@ class DefaultSessionOrchestrator(
                        "[Orchestrator] stage failed session=${ctx.sessionId.value} " +
                                "stage=${stageId.value} reason=${result.reason} retryable=${result.retryable}",
                    )
+                    if (!result.retryable) {
+                        // Non-retryable: let the transition resolver handle the outcome
+                        // (e.g., back-edge via artifact_field_equals on failure)
+                        return StepResult.Continue(ctx.copy(stageCount = ctx.stageCount + 1))
+                    }
                    val refreshedState = orchestrationRepository.getState(ctx.sessionId)
                    val shouldRetry = retryCoordinator.shouldRetry(
                        sessionId = ctx.sessionId,
@@ -188,7 +201,7 @@ class DefaultSessionOrchestrator(
                                ctx.sessionId,
                                stageId,
                                result.reason,
-                                retryExhausted = result.retryable,
+                                retryExhausted = true,
                            ),
                        )
                    }
@@ -7,6 +7,7 @@ import com.correx.core.approvals.model.ApprovalDecision
 import com.correx.core.approvals.model.DomainApprovalRequest
 import com.correx.core.artifacts.ArtifactState
 import com.correx.core.artifacts.kind.JsonSchema
+import com.correx.core.artifacts.kind.ProcessResultArtifact
 import com.correx.core.artifactstore.ArtifactStore
 import com.correx.core.context.builder.ContextPackBuilder
 import com.correx.core.context.model.ContextEntry
@@ -117,6 +118,11 @@ abstract class SessionOrchestrator(
    internal val pendingApprovals: ConcurrentHashMap<ApprovalRequestId, CompletableDeferred<ApprovalDecision>> =
        ConcurrentHashMap()

+    /** Cache mapping "sessionId:artifactId" to JSON-serialized artifact payload content.
+     *  Populated when ProcessResult artifacts are stored on tool failure.
+     *  Used by DefaultSessionOrchestrator.step() to populate EvaluationContext.artifactContent. */
+    protected val artifactContentCache: ConcurrentHashMap<String, String> = ConcurrentHashMap()
+
    abstract suspend fun run(
        sessionId: SessionId,
        graph: WorkflowGraph,
@@ -234,9 +240,21 @@ abstract class SessionOrchestrator(
            toolRounds < MAX_TOOL_ROUNDS
        ) {
            val toolEntries = dispatchToolCalls(sessionId, stageId, inferenceResult.response.toolCalls)
-            val firstError = toolEntries.firstOrNull { it.content.startsWith("ERROR:") }
-            if (firstError != null) {
-                return StageExecutionResult.Failure(firstError.content.removePrefix("ERROR: "), retryable = true)
+            val fatalEntry = toolEntries.firstOrNull { it.content.startsWith("FATAL:") }
+            if (fatalEntry != null) {
+                storeFailedProcessResult(sessionId, stageId, stageConfig,
+                    inferenceResult.response.toolCalls, fatalEntry)
+                return StageExecutionResult.Failure(
+                    fatalEntry.content.removePrefix("FATAL: "),
+                    retryable = false,
+                )
+            }
+            val errorEntry = toolEntries.firstOrNull { it.content.startsWith("ERROR:") }
+            if (errorEntry != null) {
+                return StageExecutionResult.Failure(
+                    errorEntry.content.removePrefix("ERROR: "),
+                    retryable = true,
+                )
            }
            val allEntries = currentContext.layers.values.flatten() + toolEntries
            currentContext = contextPackBuilder.build(
@@ -276,6 +294,7 @@ abstract class SessionOrchestrator(
    ): String = "[SessionOrchestrator] stage=${stageId.value}: " +
            "failed to load prompt '$path': ${throwable.message}"

+    @Suppress("CyclomaticComplexMethod")
    private suspend fun dispatchToolCalls(
        sessionId: SessionId,
        stageId: StageId,
@@ -383,7 +402,10 @@ abstract class SessionOrchestrator(
            )
            val resultContent = when (result) {
                is ToolResult.Success -> result.output
-                is ToolResult.Failure -> "ERROR: ${result.reason}"
+                is ToolResult.Failure -> {
+                    if (!result.recoverable) "FATAL: ${result.reason}"
+                    else "ERROR: ${result.reason}"
+                }
            }
            val resultEntry = ContextEntry(
                id = ContextEntryId(UUID.randomUUID().toString()),
@@ -455,6 +477,34 @@ abstract class SessionOrchestrator(
        )
    }

+    private suspend fun storeFailedProcessResult(
+        sessionId: SessionId,
+        stageId: StageId,
+        stageConfig: StageConfig,
+        toolCalls: List<ToolCallRequest>,
+        fatalEntry: ContextEntry,
+    ) {
+        stageConfig.produces.filter { it.kind.id == "process_result" }.forEach { slot ->
+            val cmd = toolCalls.firstOrNull()?.function?.arguments ?: ""
+            val reason = fatalEntry.content.removePrefix("FATAL: ")
+            val processResult = ProcessResultArtifact(
+                status = "failed",
+                command = cmd,
+                exitCode = -1,
+                stdout = "",
+                stderr = reason,
+                durationMs = 0,
+            )
+            val jsonContent = Json.encodeToString(ProcessResultArtifact.serializer(), processResult)
+            artifactStore.put(jsonContent.toByteArray())
+            val cacheKey = "${sessionId.value}:${slot.name.value}"
+            artifactContentCache[cacheKey] = jsonContent
+            emit(sessionId, ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1))
+            emit(sessionId, ArtifactValidatingEvent(slot.name, sessionId, stageId))
+            emit(sessionId, ArtifactValidatedEvent(slot.name, sessionId, stageId))
+        }
+    }
+
    internal open suspend fun mapValidationOutcome(
        sessionId: SessionId,
        stageId: StageId,
@@ -568,8 +618,14 @@ abstract class SessionOrchestrator(
        sessionId: SessionId,
        currentStageId: StageId,
        artifacts: Map<ArtifactId, ArtifactState> = emptyMap(),
+        artifactContent: Map<ArtifactId, String> = emptyMap(),
    ): TransitionDecision {
-        val ctx = EvaluationContext(sessionId, currentStageId, artifacts = artifacts)
+        val ctx = EvaluationContext(
+            sessionId = sessionId,
+            currentStage = currentStageId,
+            artifacts = artifacts,
+            artifactContent = artifactContent,
+        )
        return transitionResolver.resolve(graph, ctx)
    }

@@ -0,0 +1,78 @@
+package com.correx.core.transitions.conditions
+
+import com.correx.core.events.types.ArtifactId
+import com.correx.core.transitions.evaluation.EvaluationContext
+import com.correx.core.transitions.graph.TransitionCondition
+import kotlinx.serialization.json.Json
+import kotlinx.serialization.json.JsonElement
+import kotlinx.serialization.json.JsonObject
+import kotlinx.serialization.json.JsonPrimitive
+
+enum class FieldOperator(val symbol: String) {
+    EQ("eq"),
+    NEQ("neq"),
+    GT("gt"),
+    GTE("gte"),
+    LT("lt"),
+    LTE("lte");
+
+    companion object {
+        fun fromString(s: String): FieldOperator =
+            entries.firstOrNull { it.symbol == s }
+                ?: error("Unknown field operator '$s'. Supported: ${entries.joinToString(", ") { it.symbol }}")
+    }
+}
+
+data class ArtifactFieldEquals(
+    val artifactId: ArtifactId,
+    val field: String,
+    val value: String,
+    val operator: FieldOperator = FieldOperator.EQ,
+) : TransitionCondition {
+
+    override fun evaluate(context: EvaluationContext): Boolean {
+        val contentJson = context.artifactContent[artifactId]
+            ?: error("Artifact '${artifactId.value}' not found in evaluation context")
+
+        val element = Json.parseToJsonElement(contentJson)
+        val resolved = resolveField(element, field)
+            ?: error("Field '$field' not found in artifact '${artifactId.value}' content. " +
+                    "Available top-level fields: ${topLevelFields(element)}")
+
+        return compareValues(resolved, value, operator)
+    }
+
+    @Suppress("ReturnCount")
+    private fun resolveField(element: JsonElement, fieldPath: String): JsonPrimitive? {
+        var current: JsonElement? = element
+        for (part in fieldPath.split('.')) {
+            current = (current as? JsonObject)?.get(part)
+            if (current == null) return null
+        }
+        return current as? JsonPrimitive
+    }
+
+    private fun compareValues(actual: JsonPrimitive, expected: String, op: FieldOperator): Boolean = when (op) {
+        FieldOperator.EQ -> actual.content == expected
+        FieldOperator.NEQ -> actual.content != expected
+        FieldOperator.GT, FieldOperator.GTE, FieldOperator.LT, FieldOperator.LTE -> {
+            val actualNum = actual.content.toDoubleOrNull()
+                ?: error("Numeric operator '${op.symbol}' applied to non-numeric field value '${actual.content}'")
+            val expectedNum = expected.toDoubleOrNull()
+                ?: error("Numeric operator '${op.symbol}' requires numeric expected value, got '$expected'")
+            when (op) {
+                FieldOperator.GT -> actualNum > expectedNum
+                FieldOperator.GTE -> actualNum >= expectedNum
+                FieldOperator.LT -> actualNum < expectedNum
+                FieldOperator.LTE -> actualNum <= expectedNum
+                else -> false
+            }
+        }
+    }
+
+    private fun topLevelFields(element: JsonElement): String =
+        when (element) {
+            is JsonObject -> element.keys.joinToString(", ")
+            else -> "<not an object>"
+        }
+}
@@ -10,4 +10,5 @@ data class EvaluationContext(
    val currentStage: StageId,
    val artifacts: Map<ArtifactId, ArtifactState> = emptyMap(),
    val variables: Map<String, String> = emptyMap(),
+    val artifactContent: Map<ArtifactId, String> = emptyMap(),
 )
@@ -8,7 +8,7 @@ class DefaultTransitionResolver(
    private val evaluator: TransitionConditionEvaluator
 ) : TransitionResolver {

-    @Suppress("ReturnCount")
+    @Suppress("ReturnCount", "TooGenericExceptionCaught")
    override fun resolve(
        graph: WorkflowGraph,
        context: EvaluationContext
@@ -22,7 +22,13 @@ class DefaultTransitionResolver(
        if (outgoing.isEmpty()) return TransitionDecision.NoMatch

        for (edge in outgoing) {
-            val result = evaluator.evaluate(edge.condition, context)
+            val result = try {
+                evaluator.evaluate(edge.condition, context)
+            } catch (e: Exception) {
+                return TransitionDecision.Blocked(
+                    "condition evaluation failed on '${edge.id.value}': ${e.message}",
+                )
+            }
            if (result) {
                return TransitionDecision.Move(
                    transitionId = edge.id,
@@ -7,6 +7,7 @@ import com.correx.core.events.types.SessionId
 import com.correx.core.events.types.StageId
 import com.correx.core.transitions.evaluation.EvaluationContext
 import org.junit.jupiter.api.Assertions.assertFalse
+import org.junit.jupiter.api.Assertions.assertThrows
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test

@@ -156,4 +157,124 @@ class TransitionConditionTest {
    fun `Not inverts false to true`() {
        assertTrue(Not(Not(AlwaysTrue)).evaluate(baseContext))
    }
+
+    // ArtifactFieldEquals — flat field access
+
+    @Test
+    fun `ArtifactFieldEquals matches flat field with eq`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"failed","command":"ls","exitCode":1,"stdout":"","stderr":"error"}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "status", "failed").evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals does not match different value`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"success","exitCode":0}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertFalse(ArtifactFieldEquals(id, "status", "failed").evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals with neq matches different value`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"success","exitCode":0}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "status", "failed", operator = FieldOperator.NEQ).evaluate(ctx))
+    }
+
+    // ArtifactFieldEquals — nested field access (dot notation)
+
+    @Test
+    fun `ArtifactFieldEquals resolves nested fields via dot notation`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"execution":{"result":{"exitCode":1}}}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "execution.result.exitCode", "1").evaluate(ctx))
+    }
+
+    // ArtifactFieldEquals — numeric operators
+
+    @Test
+    fun `ArtifactFieldEquals gt matches larger number`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"exitCode":5}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "exitCode", "0", operator = FieldOperator.GT).evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals gt does not match smaller number`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"exitCode":0}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertFalse(ArtifactFieldEquals(id, "exitCode", "1", operator = FieldOperator.GT).evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals gte matches equal number`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"exitCode":1}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "exitCode", "1", operator = FieldOperator.GTE).evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals lt matches smaller number`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"exitCode":0}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "exitCode", "1", operator = FieldOperator.LT).evaluate(ctx))
+    }
+
+    @Test
+    fun `ArtifactFieldEquals lte matches equal number`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"exitCode":1}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "exitCode", "1", operator = FieldOperator.LTE).evaluate(ctx))
+    }
+
+    // ArtifactFieldEquals — error cases
+
+    @Test
+    fun `ArtifactFieldEquals throws on missing artifact`() {
+        val ctx = baseContext.copy(artifactContent = emptyMap())
+        assertThrows(IllegalStateException::class.java) {
+            ArtifactFieldEquals(ArtifactId("nonexistent"), "status", "failed").evaluate(ctx)
+        }
+        // Also catches the error() call
+    }
+
+    @Test
+    fun `ArtifactFieldEquals throws on missing field`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"success"}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        val ex = assertThrows(IllegalStateException::class.java) {
+            ArtifactFieldEquals(id, "missingField", "value").evaluate(ctx)
+        }
+        assertTrue(ex.message?.contains("not found") == true)
+    }
+
+    @Test
+    fun `ArtifactFieldEquals throws numeric operator on non-numeric field`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"success"}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        val ex = assertThrows(IllegalStateException::class.java) {
+            ArtifactFieldEquals(id, "status", "0", operator = FieldOperator.GT).evaluate(ctx)
+        }
+        assertTrue(ex.message?.contains("non-numeric") == true)
+    }
+
+    @Test
+    fun `ArtifactFieldEquals neq with string works correctly`() {
+        val id = ArtifactId("execution_result")
+        val json = """{"status":"success"}"""
+        val ctx = baseContext.copy(artifactContent = mapOf(id to json))
+        assertTrue(ArtifactFieldEquals(id, "status", "failed", operator = FieldOperator.NEQ).evaluate(ctx))
+        assertFalse(ArtifactFieldEquals(id, "status", "success", operator = FieldOperator.NEQ).evaluate(ctx))
+    }
 }
@@ -5,8 +5,10 @@ import com.correx.core.transitions.conditions.AllOf
 import com.correx.core.transitions.conditions.AlwaysTrue
 import com.correx.core.transitions.conditions.AnyOf
 import com.correx.core.transitions.conditions.ArtifactAbsent
+import com.correx.core.transitions.conditions.ArtifactFieldEquals
 import com.correx.core.transitions.conditions.ArtifactPresent
 import com.correx.core.transitions.conditions.ArtifactValidated
+import com.correx.core.transitions.conditions.FieldOperator
 import com.correx.core.transitions.conditions.Not
 import com.correx.core.transitions.conditions.VariableEquals
 import com.correx.core.transitions.graph.TransitionCondition
@@ -17,6 +19,7 @@ fun ConditionSpec.toCondition(): TransitionCondition = when (type) {
    "artifact_absent" -> buildArtifactAbsent()
    "artifact_validated" -> buildArtifactValidated()
    "variable_equals" -> buildVariableEquals()
+    "artifact_field_equals" -> buildArtifactFieldEquals()
    "all_of" -> AllOf(conditions.map { it.toCondition() })
    "any_of" -> AnyOf(conditions.map { it.toCondition() })
    "not" -> Not((condition ?: error("condition required for not")).toCondition())
@@ -37,3 +40,11 @@ private fun ConditionSpec.buildVariableEquals() =
        key ?: error("key required for variable_equals"),
        value ?: error("value required for variable_equals"),
    )
+
+private fun ConditionSpec.buildArtifactFieldEquals() =
+    ArtifactFieldEquals(
+        artifactId = ArtifactId(artifactId ?: error("artifact_id required for artifact_field_equals")),
+        field = field ?: error("field required for artifact_field_equals"),
+        value = value ?: error("value required for artifact_field_equals"),
+        operator = operator?.let { FieldOperator.fromString(it) } ?: FieldOperator.EQ,
+    )
@@ -5,6 +5,8 @@ data class ConditionSpec(
    val artifactId: String? = null,
    val key: String? = null,
    val value: String? = null,
+    val field: String? = null,
+    val operator: String? = null,
    val conditions: List<ConditionSpec> = emptyList(),
    val condition: ConditionSpec? = null,
 )
@@ -49,6 +49,8 @@ private data class TransitionSection(
    @param:JsonProperty("condition_artifact_id") val conditionArtifactId: String? = null,
    @param:JsonProperty("condition_key") val conditionKey: String? = null,
    @param:JsonProperty("condition_value") val conditionValue: String? = null,
+    @param:JsonProperty("condition_field") val conditionField: String? = null,
+    @param:JsonProperty("condition_operator") val conditionOperator: String? = null,
 )

 private const val TERMINAL = "done"
@@ -112,6 +114,8 @@ class TomlWorkflowLoader(
                    artifactId = t.conditionArtifactId,
                    key = t.conditionKey,
                    value = t.conditionValue,
+                    field = t.conditionField,
+                    operator = t.conditionOperator,
                ).toCondition(),
            )
        }.toSet()