fix(kernel,tools,workflow): session-robustness QA sweep
Uncommitted work from the session-robustness-and-dox branch sweep
(docs/qa/QA-session-robustness-and-dox.md), verified alongside the
compression/context fixes:
- SandboxedToolExecutor: validate tool args centrally before dispatch. A
malformed/missing-arg call becomes a recoverable ERROR: (surfaced with the
tool's arg schema so the model can correct + retry) instead of stranding
the stage with no artifact. + validation test.
- PlanLinter: seed artifacts (analysis) produced by the planning phase count
as available producers, so a plan stage that `needs` them isn't flagged as
an unproduced-need; H1 unproduced-needs + trap-state checks. + tests.
- DefaultSessionOrchestrator: live-QA robustness fixes (event-tail /
per-stage budget + retry handling).
- workflow prompts/configs: DOX AGENTS.md alignment + freestyle/task/role
prompt tweaks.
- SessionOrchestratorIntegrationTest: coverage for the above.
- FreestylePlanningWorkflowTest: allow list_dir in analyst tools (follows the
list_dir wiring in 968cbfa).
- QA plan doc for the branch sweep.
This commit is contained in:
+14
@@ -18,6 +18,7 @@ import com.correx.core.tools.contract.FileAffectingTool
|
||||
import com.correx.core.tools.contract.Tool
|
||||
import com.correx.core.tools.contract.ToolExecutor
|
||||
import com.correx.core.tools.contract.ToolResult
|
||||
import com.correx.core.tools.contract.ValidationResult
|
||||
import com.correx.core.tools.registry.ToolRegistry
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.withContext
|
||||
@@ -55,6 +56,19 @@ class SandboxedToolExecutor(
|
||||
// 2. emit started
|
||||
emitStarted(sessionId, invocationId, toolName)
|
||||
|
||||
// 3. validate args centrally. A malformed/missing-arg call is the model's mistake, not a
|
||||
// fatal stage failure: return recoverable so the orchestrator surfaces it as ERROR: + the
|
||||
// tool's arg schema and the model can correct + retry (bounded by MAX_TOOL_ROUNDS), instead
|
||||
// of stranding the stage with no artifact ("no transition condition matched").
|
||||
(tool.validateRequest(request) as? ValidationResult.Invalid)?.let { invalid ->
|
||||
emitFailed(sessionId, invocationId, toolName, invalid.reason)
|
||||
return@withContext ToolResult.Failure(
|
||||
invocationId = invocationId,
|
||||
reason = invalid.reason,
|
||||
recoverable = true,
|
||||
)
|
||||
}
|
||||
|
||||
// 4. create working dir
|
||||
val workingDir = workDir.resolve(sessionId.value).resolve(invocationId.value)
|
||||
Files.createDirectories(workingDir)
|
||||
|
||||
+87
@@ -0,0 +1,87 @@
|
||||
package com.correx.infrastructure.tools
|
||||
|
||||
import com.correx.core.approvals.Tier
|
||||
import com.correx.core.events.EventDispatcher
|
||||
import com.correx.core.events.events.EventPayload
|
||||
import com.correx.core.events.events.NewEvent
|
||||
import com.correx.core.events.events.StoredEvent
|
||||
import com.correx.core.events.events.ToolExecutionFailedEvent
|
||||
import com.correx.core.events.events.ToolRequest
|
||||
import com.correx.core.events.stores.EventStore
|
||||
import com.correx.core.events.types.SessionId
|
||||
import com.correx.core.events.types.StageId
|
||||
import com.correx.core.events.types.ToolInvocationId
|
||||
import com.correx.core.tools.contract.ParamRole
|
||||
import com.correx.core.tools.contract.Tool
|
||||
import com.correx.core.tools.contract.ToolCapability
|
||||
import com.correx.core.tools.contract.ToolExecutor
|
||||
import com.correx.core.tools.contract.ToolResult
|
||||
import com.correx.core.tools.contract.ValidationResult
|
||||
import com.correx.core.tools.registry.ToolRegistry
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.emptyFlow
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.serialization.json.JsonObject
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
import java.nio.file.Files
|
||||
|
||||
class SandboxedToolExecutorValidationTest {
|
||||
|
||||
private class CapturingEventStore : EventStore {
|
||||
val payloads = mutableListOf<EventPayload>()
|
||||
override suspend fun append(event: NewEvent): StoredEvent {
|
||||
payloads += event.payload
|
||||
val seq = payloads.size.toLong()
|
||||
return StoredEvent(event.metadata, seq, seq, event.payload)
|
||||
}
|
||||
override suspend fun appendAll(events: List<NewEvent>): List<StoredEvent> = events.map { append(it) }
|
||||
override fun read(sessionId: SessionId): List<StoredEvent> = emptyList()
|
||||
override fun readFrom(sessionId: SessionId, fromSequence: Long): List<StoredEvent> = emptyList()
|
||||
override fun lastSequence(sessionId: SessionId): Long? = null
|
||||
override fun subscribe(sessionId: SessionId): Flow<StoredEvent> = emptyFlow()
|
||||
override fun subscribeAll(): Flow<StoredEvent> = emptyFlow()
|
||||
override suspend fun lastGlobalSequence(): Long = payloads.size.toLong()
|
||||
override fun allEvents(): Sequence<StoredEvent> = emptySequence()
|
||||
override fun allSessionIds(): Set<SessionId> = emptySet()
|
||||
}
|
||||
|
||||
private class SingleToolRegistry(private val tool: Tool) : ToolRegistry {
|
||||
override fun resolve(name: String): Tool? = if (name == tool.name) tool else null
|
||||
override fun all(): List<Tool> = listOf(tool)
|
||||
}
|
||||
|
||||
/** A tool that rejects every call at validation and would explode if execute() were reached. */
|
||||
private class AlwaysInvalidTool : Tool, ToolExecutor {
|
||||
override val name: String = "picky"
|
||||
override val description: String = "fake"
|
||||
override val tier: Tier = Tier.T2
|
||||
override val requiredCapabilities: Set<ToolCapability> = emptySet()
|
||||
override val paramRoles: Map<String, ParamRole> = emptyMap()
|
||||
override val parametersSchema: JsonObject = JsonObject(emptyMap())
|
||||
override fun validateRequest(request: ToolRequest): ValidationResult =
|
||||
ValidationResult.Invalid("Missing or empty 'argv' parameter. Expected List<String>.")
|
||||
override suspend fun execute(request: ToolRequest): ToolResult =
|
||||
error("execute must not run when validation fails")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `bad args fail recoverably and never reach execute`() {
|
||||
val tool = AlwaysInvalidTool()
|
||||
val events = CapturingEventStore()
|
||||
val exec = SandboxedToolExecutor(
|
||||
delegate = tool,
|
||||
registry = SingleToolRegistry(tool),
|
||||
eventDispatcher = EventDispatcher(events),
|
||||
workDir = Files.createTempDirectory("sbx-validate"),
|
||||
)
|
||||
val result = runBlocking {
|
||||
exec.execute(ToolRequest(ToolInvocationId("inv"), SessionId("s"), StageId("st"), "picky", emptyMap()))
|
||||
}
|
||||
|
||||
result as ToolResult.Failure
|
||||
assertTrue(result.recoverable, "validation failures must be recoverable so the model can correct + retry")
|
||||
assertEquals(1, events.payloads.filterIsInstance<ToolExecutionFailedEvent>().size)
|
||||
}
|
||||
}
|
||||
+13
-6
@@ -20,7 +20,7 @@ data class PlanLintResult(
|
||||
|
||||
/**
|
||||
* Deterministic, pure-Kotlin lint over a compiled plan graph (plan-pipeline-spec §5). Zero inference,
|
||||
* zero I/O — a function of the graph alone, so it is replay-safe by construction. It complements
|
||||
* zero I/O — a function of the graph and a fixed seed set, so it is replay-safe by construction. It complements
|
||||
* [ExecutionPlanCompiler] (which already throws on unreachable-from-start / unknown-kind / bad-edge):
|
||||
* the lint adds the checks the compiler does NOT make.
|
||||
*
|
||||
@@ -32,15 +32,22 @@ object PlanLinter {
|
||||
private const val STAGE_CEILING = 12
|
||||
private const val FAN_OUT_THRESHOLD = 4
|
||||
|
||||
fun lint(graph: WorkflowGraph): PlanLintResult =
|
||||
/**
|
||||
* Artifacts that pre-exist in the session before the execution plan runs — produced by the
|
||||
* planning phase, not by any plan stage. The architect prompt explicitly allows `needs` to
|
||||
* reference these (notably `analysis`), so the linter must treat them as available producers.
|
||||
*/
|
||||
private val seedArtifacts = setOf("analysis")
|
||||
|
||||
fun lint(graph: WorkflowGraph, seeds: Set<String> = seedArtifacts): PlanLintResult =
|
||||
PlanLintResult(
|
||||
hardFailures = unproducedNeeds(graph) + trapStates(graph),
|
||||
hardFailures = unproducedNeeds(graph, seeds) + trapStates(graph),
|
||||
softFindings = stageCount(graph) + fanOut(graph) + emptyBriefs(graph) + duplicateBriefs(graph),
|
||||
)
|
||||
|
||||
/** H1: a stage `needs` an artifact that no stage `produces`. */
|
||||
private fun unproducedNeeds(graph: WorkflowGraph): List<PlanLintFinding> {
|
||||
val produced = graph.stages.values.flatMap { it.produces }.map { it.name.value }.toSet()
|
||||
/** H1: a stage `needs` an artifact that neither a plan stage `produces` nor a seed provides. */
|
||||
private fun unproducedNeeds(graph: WorkflowGraph, seeds: Set<String>): List<PlanLintFinding> {
|
||||
val produced = graph.stages.values.flatMap { it.produces }.map { it.name.value }.toSet() + seeds
|
||||
return graph.stages.entries.flatMap { (id, stage) ->
|
||||
stage.needs.map { it.value }.filterNot { it in produced }.map { need ->
|
||||
PlanLintFinding(
|
||||
|
||||
+1
-1
@@ -73,7 +73,7 @@ class FreestylePlanningWorkflowTest {
|
||||
// analyst frames the work: search + open one task or decompose into a graph (the architect
|
||||
// threads the named task into the plan).
|
||||
assertEquals(
|
||||
setOf("file_read", "ShellTool", "task_search", "task_context", "task_create", "task_decompose"),
|
||||
setOf("file_read", "list_dir", "shell", "task_search", "task_context", "task_create", "task_decompose"),
|
||||
graph.stages[StageId("analyst")]!!.allowedTools,
|
||||
)
|
||||
}
|
||||
|
||||
+14
@@ -78,6 +78,20 @@ class PlanLinterTest {
|
||||
assertTrue(h.detail.contains("z"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a need satisfied by the analysis seed is not a failure`() {
|
||||
val g = graph(
|
||||
stages = mapOf(
|
||||
"a" to stage(produces = listOf("x"), needs = listOf("analysis"), prompt = "research"),
|
||||
"b" to stage(needs = listOf("x"), prompt = "build"),
|
||||
),
|
||||
edges = listOf("a" to "b", "b" to "done"),
|
||||
start = "a",
|
||||
)
|
||||
val result = PlanLinter.lint(g)
|
||||
assertTrue(result.passed, "analysis is a planning-phase seed, not an unproduced need")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a cycle with no exit is a trap-state hard failure`() {
|
||||
val g = graph(
|
||||
|
||||
Reference in New Issue
Block a user