diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt b/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt index 51aa1ff0..4566a94d 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/Main.kt @@ -34,6 +34,7 @@ import com.correx.core.inference.ModelCapability import com.correx.core.kernel.orchestration.DefaultOrchestrationReducer import com.correx.core.kernel.orchestration.DefaultSessionOrchestrator import com.correx.core.kernel.orchestration.OrchestrationConfig +import com.correx.core.kernel.orchestration.OrchestrationTuning import com.correx.core.kernel.orchestration.OrchestrationProjector import com.correx.core.kernel.orchestration.OrchestrationRepository import com.correx.core.kernel.orchestration.OrchestratorEngines @@ -109,9 +110,10 @@ import java.nio.file.Paths private val log = LoggerFactory.getLogger("com.correx.apps.server.Main") +private val loopbackHosts = setOf("localhost", "127.0.0.1", "::1") + fun main() { log.info("=== correx server starting ===") - log.info(" port : 8080") val artifactStore = InfrastructureModule.createArtifactStore() val eventStore = LoggingEventStore(InfrastructureModule.createEventStore(artifactStore)) @@ -218,16 +220,19 @@ fun main() { val workingDir = explicitWorkingDir ?: workspaceRoot // One shared HTTP client backs both the default and per-workspace registries' research tools // (web_search/web_fetch). Built only when research is enabled, so the static path stays offline. + // Lives for the process lifetime (shared across requests), so it's closed via shutdown hook + // below rather than `.use { }`. + val researchHttpClient = if (toolsConfig.research.enabled) { + io.ktor.client.HttpClient(io.ktor.client.engine.cio.CIO) + } else { + null + } val researchToolConfig = com.correx.infrastructure.tools.ResearchToolConfig( enabled = toolsConfig.research.enabled, searxngUrl = toolsConfig.research.searxngUrl, maxResults = toolsConfig.research.maxResults, maxFetchBytes = toolsConfig.research.maxFetchBytes, - httpClient = if (toolsConfig.research.enabled) { - io.ktor.client.HttpClient(io.ktor.client.engine.cio.CIO) - } else { - null - }, + httpClient = researchHttpClient, ) // Agents create/update/delete tasks through the tool system (tier-gated like any tool); // the service appends to the same event log. Project is derived from the task id prefix. @@ -255,6 +260,9 @@ fun main() { com.correx.apps.server.tasks.EventStoreSessionFactRecorder(eventStore), com.correx.apps.server.tasks.EventStoreSessionWrites(eventStore), ) + // Retrieves full tool output the kernel spilled to CAS on truncation (ref shown in-context). + val toolOutputTool = com.correx.infrastructure.tools.ToolOutputTool(artifactStore) + val extraTools = taskTools + toolOutputTool val toolRegistry = InfrastructureModule.createToolRegistry( buildToolConfig( workspaceRoot, @@ -263,7 +271,7 @@ fun main() { toolsConfig, researchToolConfig, ), - extraTools = taskTools, + extraTools = extraTools, ) val toolExecutor = InfrastructureModule.createToolExecutor( registry = toolRegistry, @@ -296,7 +304,7 @@ fun main() { val wsToolRegistryProvider = WorkspaceToolRegistryProvider { workspace -> val wsRegistry = InfrastructureModule.createToolRegistry( buildToolConfigForWorkspace(workspace, shellAllowedExecutables, toolsConfig, researchToolConfig), - extraTools = taskTools, + extraTools = extraTools, ) val wsExecutor = DispatchingToolExecutor(wsRegistry) WorkspaceTools(registry = wsRegistry, executor = wsExecutor) @@ -380,11 +388,45 @@ fun main() { // Plan-compile gate: wrap the ExecutionPlanCompiler as a post-stage check so a compile-invalid // architect plan is handed back through the retry-feedback loop instead of parking the session in // ACTIVE (post-planning compile dead-end). Returns null on success, the compiler message on failure. + // Operator sampling defaults ([sampling] config) sent on every stage inference request. maxTokens=0 + // is a placeholder; the loader/compiler pin it per-stage to the token budget via copy(). + val stageSamplingDefaults = with(correxConfig.sampling) { + GenerationConfig( + temperature = temperature, + topP = topP, + maxTokens = 0, + topK = topK, + minP = minP, + repeatPenalty = repeatPenalty, + ) + } val planCompiler = ExecutionPlanCompiler( artifactKindRegistry, toolRegistry.all().map { it.name }.toSet(), injectRecovery = true, + samplingDefaults = stageSamplingDefaults, ) + // Startup-load orchestration tuning from the [orchestration] config section into the kernel's + // OrchestrationTuning. ponytail: read once at boot — a config edit needs a restart to take effect + // (unlike stage_timeout_ms, which ServerModule re-reads per session). Add a supplier if these ever + // need hot-reload. + val orchestrationTuning = with(correxConfig.orchestration) { + OrchestrationTuning( + maxToolRounds = maxToolRounds, + readLoopNudgeThreshold = readLoopNudgeThreshold, + rejectionLoopNudgeThreshold = rejectionLoopNudgeThreshold, + maxFeedbackIssues = maxFeedbackIssues, + repoMapInjectTopK = repoMapInjectTopK, + repoMapFilesPerDir = repoMapFilesPerDir, + docsCatalogMax = docsCatalogMax, + maxClarificationRounds = maxClarificationRounds, + reviewBlockMinConfidence = reviewBlockMinConfidence, + reviewBlockRetryCap = reviewBlockRetryCap, + defaultMaxRefinement = defaultMaxRefinement, + recoveryRouteBudget = recoveryRouteBudget, + intentRouteBudget = intentRouteBudget, + ) + } val orchestrator = DefaultSessionOrchestrator( repositories = repositories, engines = engines.copy( @@ -420,9 +462,10 @@ fun main() { taskSessionResolver, ), ), + tuning = orchestrationTuning, ) val workflowRegistry = FileSystemWorkflowRegistry( - InfrastructureModule.createWorkflowLoader(configArtifactKindsEarly), + InfrastructureModule.createWorkflowLoader(configArtifactKindsEarly, stageSamplingDefaults), ) // Builds the router facade from a config snapshot, mapping the [router] block onto the domain // TalkieConfig. Reused by ConfigService's rebuild hook so router knob edits apply live (the @@ -534,6 +577,8 @@ fun main() { ) // observability-spec §4: continuous health watch. Seed the monitor's last-status from the // recorded system-session events so a restart doesn't re-emit a degraded already in the log. + // healthProbeHttpClient lives for the process lifetime; closed via shutdown hook below. + var healthProbeHttpClient: io.ktor.client.HttpClient? = null val healthMonitor = correxConfig.health.let { hc -> if (!hc.enabled) { null @@ -542,6 +587,7 @@ fun main() { val seeded = DefaultEventReplayer(eventStore, com.correx.apps.server.health.HealthProjection()) .rebuild(com.correx.apps.server.health.SYSTEM_SESSION) .subjects.mapValues { it.value.status } + healthProbeHttpClient = io.ktor.client.HttpClient(io.ktor.client.engine.cio.CIO) com.correx.apps.server.health.HealthMonitor( eventStore = eventStore, probes = listOfNotNull( @@ -552,7 +598,7 @@ fun main() { ), com.correx.apps.server.health.LlamaServerHealthProbe( llamaBaseUrl = llamaBaseUrl, - httpClient = io.ktor.client.HttpClient(io.ktor.client.engine.cio.CIO), + httpClient = healthProbeHttpClient!!, eventStore = eventStore, livenessTimeoutMs = hc.llamaLivenessTimeoutMs, tpsWarnBelow = hc.llamaTpsWarnBelow, @@ -567,6 +613,12 @@ fun main() { ) } } + Runtime.getRuntime().addShutdownHook( + Thread { + researchHttpClient?.close() + healthProbeHttpClient?.close() + }, + ) val module = ServerModule( orchestrator = orchestrator, eventStore = eventStore, @@ -609,7 +661,16 @@ fun main() { module.start() log.info("==============================") - embeddedServer(Netty, port = correxConfig.server.port, host = correxConfig.server.host) { + val serverConfig = correxConfig.server + log.info(" host:port : {}:{}", serverConfig.host, serverConfig.port) + if (serverConfig.host !in loopbackHosts) { + log.warn( + "Server host '{}' is not loopback — the unauthenticated HTTP/WS surface is exposed to the network", + serverConfig.host, + ) + } + + embeddedServer(Netty, host = serverConfig.host, port = serverConfig.port) { configureServer(module) }.start(wait = true) } @@ -674,9 +735,7 @@ private fun loadConfigArtifactKinds(config: CorrexConfig): List { val schema = runCatching { Json.decodeFromString(JsonSchema.serializer(), Files.readString(schemaPath)) }.getOrElse { e -> - System.err.println( - "Warning: artifact kind '${decl.id}' schema '$schemaPath' failed to load: ${e.message}", - ) + log.warn("artifact kind '{}' schema '{}' failed to load: {}", decl.id, schemaPath, e.message) return@mapNotNull null } ConfigArtifactKind(id = decl.id, schema = schema, llmEmitted = decl.llmEmitted) diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt b/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt index c0bacf8b..32dbb027 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/ServerModule.kt @@ -7,6 +7,9 @@ import com.correx.apps.server.narration.NarrationSubscriber import com.correx.apps.server.registry.ProviderRegistry import com.correx.apps.server.registry.WorkflowRegistry import com.correx.apps.server.workspace.WorkspaceResolver +import com.correx.apps.server.workspace.WorkspaceResolution +import com.correx.core.events.events.SessionWorkspaceBoundEvent +import com.correx.core.kernel.orchestration.WorkspaceContext import com.correx.core.approvals.ApprovalProjector import com.correx.core.approvals.DefaultApprovalReducer import com.correx.core.approvals.DefaultApprovalRepository @@ -183,10 +186,13 @@ class ServerModule( fun start() { if (subscriptionJob != null) return preRegisterPendingApprovals() + repairStuckApprovalPauses() resumeAbandonedSessions() subscriptionJob = eventStore.subscribeAll() .filter { it.payload is ApprovalRequestedEvent } - .onEach { approvalCoordinator.onApprovalRequested(it.payload as ApprovalRequestedEvent) } + .onEach { + approvalCoordinator.onApprovalRequested(it.payload as ApprovalRequestedEvent, it.sessionSequence) + } .launchIn(moduleScope) // When a PAUSED session's approval is resolved without a live orchestrator coroutine @@ -294,6 +300,48 @@ class ServerModule( * run job is invisible to the guard and a second `resume()` is spuriously launched, * duplicating inference calls (the root cause of the healthcheck script corruption). */ + /** + * Resolves [workingDir] into a trusted workspace and records the decision as a + * [SessionWorkspaceBoundEvent] (invariant #9), returning the bound context or null when no + * resolver is configured. Shared by the WS StartSession path and the REST POST /sessions + * launcher so both anchor a session's workspace identically — the REST route previously skipped + * this entirely, leaving sessions with no bound workspace. + */ + suspend fun bindWorkspace(sessionId: SessionId, workingDir: String?): WorkspaceContext? { + val resolver = workspaceResolver ?: return null + val workspace = when (val resolution = withContext(Dispatchers.IO) { resolver.resolve(workingDir) }) { + is WorkspaceResolution.Bound -> { + log.info("workspace bound: session={} root={}", sessionId.value, resolution.workspace.workspaceRoot) + resolution.workspace + } + is WorkspaceResolution.Rejected -> { + log.warn( + "workspace rejected: session={} reason={} fallback={}", + sessionId.value, resolution.reason, resolution.fallback.workspaceRoot, + ) + resolution.fallback + } + } + eventStore.append( + NewEvent( + metadata = EventMetadata( + eventId = EventId(java.util.UUID.randomUUID().toString()), + sessionId = sessionId, + timestamp = Clock.System.now(), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + payload = SessionWorkspaceBoundEvent( + sessionId = sessionId, + workspaceRoot = workspace.workspaceRoot.toString(), + allowedPaths = workspace.allowedPaths.map { it.toString() }, + ), + ), + ) + return workspace + } + fun launchSessionRun( sessionId: SessionId, graph: com.correx.core.transitions.graph.WorkflowGraph, @@ -576,13 +624,25 @@ class ServerModule( private val sessionSummaryProjector = SessionSummaryProjector() - fun listSessionSummaries(): List = - eventStore.allSessionIds() + // ponytail: memoized on lastGlobalSequence — GET /sessions was re-reading + re-projecting every + // session's full log on every call (S5). Any new event anywhere bumps the global sequence, so a + // stale cache is impossible: a mismatch always forces a full recompute. Volatile pair-swap is + // enough here (no lock) — worst case under a race is one redundant recompute, never a stale read. + @Volatile + private var sessionSummaryCache: Pair>? = null + + suspend fun listSessionSummaries(): List { + val currentSeq = eventStore.lastGlobalSequence() + sessionSummaryCache?.let { (seq, summaries) -> if (seq == currentSeq) return summaries } + val summaries = eventStore.allSessionIds() // The system session carries global health events, not a user workflow — hide it. .filter { it != com.correx.apps.server.health.SYSTEM_SESSION } .map { sessionId -> sessionSummaryProjector.project(sessionId, eventStore.readFrom(sessionId, fromSequence = 0L)) } + sessionSummaryCache = currentSeq to summaries + return summaries + } private fun preRegisterPendingApprovals() { val projector = ApprovalProjector(DefaultApprovalReducer()) @@ -595,7 +655,54 @@ class ServerModule( } approvalState.requests.values .filter { req -> approvalState.decisions.values.none { it.requestId == req.id } } - .forEach { req -> approvalCoordinator.registerPendingRequest(req.id, sessionId) } + .forEach { req -> approvalCoordinator.registerPendingRequest(req.id, sessionId, req.tier) } + } + } + + /** + * One-shot boot repair for sessions stuck PAUSED + pendingApproval with no actual unresolved + * approval request — the symptom of a pre-Feb-13 orchestrator bug where approval was resolved + * but OrchestrationResumedEvent was never emitted. Appends the missing resume so the projection + * corrects permanently. Previously this ran per WS connection inside SessionEventBridge. + * replaySnapshot, which put a racy write on a pure read path (two concurrent clients could + * double-append). Running it once at boot removes that hazard; the bug that creates the state is + * fixed, so no session newly enters it at runtime. + * + * Guard: skip when there are more OrchestrationPausedEvents than ApprovalRequestedEvents — the + * session just entered the gate and its ApprovalRequestedEvent isn't stored yet; a spurious + * resume would hide the pending approval. + */ + private fun repairStuckApprovalPauses() { + val projector = ApprovalProjector(DefaultApprovalReducer()) + eventStore.allSessionIds().forEach { sessionId -> + val orchState = orchestrationRepository.getState(sessionId) + if (!orchState.pendingApproval) return@forEach + val events = eventStore.read(sessionId) + val approvalState = events.fold(projector.initial()) { state, event -> projector.apply(state, event) } + val hasUnresolved = approvalState.requests.values + .any { req -> approvalState.decisions.values.none { it.requestId == req.id } } + val pauseCount = events.count { it.payload is OrchestrationPausedEvent } + val approvalRequestCount = events.count { it.payload is ApprovalRequestedEvent } + val hasUnpairedPause = pauseCount > approvalRequestCount + val alreadyResumed = events.any { it.payload is OrchestrationResumedEvent } + val stageId = orchState.currentStageId + if (hasUnresolved || hasUnpairedPause || alreadyResumed || stageId == null) return@forEach + log.info("repairStuckApprovalPauses: emitting missing resume for stuck session={}", sessionId.value) + moduleScope.launch { + eventStore.append( + NewEvent( + metadata = EventMetadata( + eventId = EventId(java.util.UUID.randomUUID().toString()), + sessionId = sessionId, + timestamp = Clock.System.now(), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + payload = OrchestrationResumedEvent(sessionId = sessionId, stageId = stageId), + ), + ) + } } } diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/approval/ApprovalCoordinator.kt b/apps/server/src/main/kotlin/com/correx/apps/server/approval/ApprovalCoordinator.kt index e249b678..f0192ea5 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/approval/ApprovalCoordinator.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/approval/ApprovalCoordinator.kt @@ -13,6 +13,7 @@ import com.correx.core.events.types.SessionId import com.correx.core.kernel.orchestration.ApprovalGateway import io.ktor.server.websocket.DefaultWebSocketServerSession import io.ktor.websocket.Frame +import org.slf4j.LoggerFactory import java.util.concurrent.ConcurrentHashMap open class ApprovalCoordinator( @@ -23,6 +24,7 @@ open class ApprovalCoordinator( private val globalClients: MutableSet = ConcurrentHashMap.newKeySet() private val resolved: ConcurrentHashMap = ConcurrentHashMap() private val requestSessions: ConcurrentHashMap = ConcurrentHashMap() + private val requestTiers: ConcurrentHashMap = ConcurrentHashMap() fun registerClient(sessionId: SessionId, session: DefaultWebSocketServerSession) { sessionClients.getOrPut(sessionId) { ConcurrentHashMap.newKeySet() }.add(session) @@ -40,8 +42,9 @@ open class ApprovalCoordinator( globalClients.remove(session) } - suspend fun onApprovalRequested(event: ApprovalRequestedEvent) { + suspend fun onApprovalRequested(event: ApprovalRequestedEvent, sessionSequence: Long = 0L) { requestSessions[event.requestId] = event.sessionId + requestTiers[event.requestId] = event.tier val msg = ServerMessage.ApprovalRequired( sessionId = event.sessionId, requestId = event.requestId, @@ -55,7 +58,7 @@ open class ApprovalCoordinator( toolName = event.toolName, preview = event.preview, sequence = 0L, - sessionSequence = 0L, + sessionSequence = sessionSequence, ) broadcast(event.sessionId, msg) } @@ -75,8 +78,9 @@ open class ApprovalCoordinator( * during snapshot replay. This makes [lookupSession] work for approvals that were created * before the current connection's live stream started. */ - fun registerPendingRequest(requestId: ApprovalRequestId, sessionId: SessionId) { + fun registerPendingRequest(requestId: ApprovalRequestId, sessionId: SessionId, tier: Tier) { requestSessions[requestId] = sessionId + requestTiers[requestId] = tier } open suspend fun handleResponse(msg: ClientMessage.ApprovalResponse, sessionId: SessionId): ServerMessage? { @@ -87,12 +91,20 @@ open class ApprovalCoordinator( sessionSequence = null, ) } - requestSessions.remove(msg.requestId) - val domain = msg.toDomain(sessionId, null, Tier.T2) + val tier = requestTiers[msg.requestId] ?: Tier.T2 + val domain = msg.toDomain(sessionId, null, tier) return runCatching { orchestrator.submitApprovalDecision(msg.requestId, domain) } .fold( - onSuccess = { null }, + onSuccess = { + // Only retire the request once the decision was actually recorded. If submit + // throws, we clear the resolved flag so the client can retry instead of the + // request being permanently unanswerable. + requestSessions.remove(msg.requestId) + requestTiers.remove(msg.requestId) + null + }, onFailure = { + resolved.remove(msg.requestId) ServerMessage.ProtocolError( message = it.message ?: "Unknown error", sequence = null, @@ -106,8 +118,29 @@ open class ApprovalCoordinator( val encoded = ProtocolSerializer.encodeServerMessage(msg) val sessionSubs = sessionClients[sessionId].orEmpty() val recipients: Set = sessionSubs + globalClients + var delivered = 0 recipients.forEach { client -> runCatching { client.send(Frame.Text(encoded)) } + .onSuccess { delivered++ } + .onFailure { err -> + // A dead socket must not silently swallow an approval prompt. Log it and evict + // the client from both registries so we don't keep sending into a closed socket + // (the client re-registers and gets the pending approval from the snapshot on + // reconnect). + log.warn("approval broadcast to a client failed, evicting: {}", err.message) + sessionClients[sessionId]?.remove(client) + globalClients.remove(client) + } + } + if (delivered == 0) { + log.error( + "approval prompt for session={} reached 0 live clients; session stays paused until a client reconnects", + sessionId.value, + ) } } + + private companion object { + private val log = LoggerFactory.getLogger(ApprovalCoordinator::class.java) + } } diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/DomainEventMapper.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/DomainEventMapper.kt index ee6fafa8..a6d2dfae 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/DomainEventMapper.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/DomainEventMapper.kt @@ -1,55 +1,12 @@ package com.correx.apps.server.bridge -import com.correx.apps.server.protocol.AssessedIssueDto -import com.correx.apps.server.protocol.PauseReason -import com.correx.apps.server.protocol.ReviewFindingDto -import com.correx.apps.server.protocol.RiskSummaryDto import com.correx.apps.server.protocol.ServerMessage -import com.correx.apps.server.protocol.toDto import com.correx.core.artifactstore.ArtifactStore -import com.correx.core.events.events.ApprovalDecisionResolvedEvent -import com.correx.core.events.events.ApprovalRequestedEvent -import com.correx.core.events.events.ArtifactContentStoredEvent -import com.correx.core.events.events.ArtifactCreatedEvent -import com.correx.core.events.events.ArtifactValidatedEvent -import com.correx.core.events.events.ArtifactValidatingEvent -import com.correx.core.events.events.ExecutionPlanLockedEvent -import com.correx.core.events.events.ChatSessionStartedEvent -import com.correx.core.events.events.ChatTurnEvent -import com.correx.core.events.events.ClarificationRequestedEvent -import com.correx.core.events.events.InferenceCompletedEvent -import com.correx.core.events.events.SessionNamedEvent -import com.correx.core.events.events.SessionWorkspaceBoundEvent -import com.correx.core.events.events.WorkflowStartedEvent -import com.correx.core.events.events.InferenceFailedEvent -import com.correx.core.events.events.InferenceStartedEvent -import com.correx.core.events.events.InferenceTimeoutEvent -import com.correx.core.events.events.ReviewFindingsRaisedEvent -import com.correx.core.events.events.RetryAttemptedEvent -import com.correx.core.events.events.ModelLoadedEvent -import com.correx.core.events.events.ModelUnloadedEvent -import com.correx.core.events.events.OrchestrationPausedEvent -import com.correx.core.events.events.OrchestrationResumedEvent -import com.correx.core.events.events.PreemptRedirectBlockedEvent -import com.correx.core.events.events.PreemptRedirectEvent -import com.correx.core.events.events.StageCompletedEvent -import com.correx.core.events.events.StageFailedEvent import com.correx.core.events.events.StoredEvent -import com.correx.core.events.events.ToolExecutionCompletedEvent -import com.correx.core.events.events.ToolExecutionFailedEvent -import com.correx.core.events.events.ToolExecutionRejectedEvent -import com.correx.core.events.events.ToolCallAssessedEvent -import com.correx.core.events.events.ToolInvocationRequestedEvent -import com.correx.core.events.events.TransitionExecutedEvent -import com.correx.core.events.events.WorkflowCompletedEvent -import com.correx.core.events.events.TalkieNarrationEvent -import com.correx.core.events.events.WorkflowFailedEvent -import com.correx.core.events.events.WorkflowProposedEvent -import com.correx.core.events.risk.RiskAction import com.correx.core.events.types.ArtifactId import org.slf4j.LoggerFactory -private val log = LoggerFactory.getLogger("DomainEventMapper") +internal val log = LoggerFactory.getLogger("DomainEventMapper") class DomainEventMapper(private val artifactStore: ArtifactStore = NoopArtifactStore) { suspend fun map(event: StoredEvent, sessionSequence: Long = 0L): ServerMessage? = @@ -62,374 +19,44 @@ private object NoopArtifactStore : ArtifactStore { override suspend fun flushBefore(commit: suspend () -> Unit) = commit() } -@Suppress("CyclomaticComplexMethod") +/** + * Outcome of one per-domain mapper. [Emit] means "this event is mine" — carrying either a + * [ServerMessage] or `null` (handled but deliberately not surfaced, e.g. transient bookkeeping + * events). [Skip] means "not my domain", so the dispatcher tries the next mapper. The distinction + * matters: chaining on a bare `null` would conflate suppression with non-ownership. + */ +internal sealed interface MapOutcome { + @JvmInline + value class Emit(val message: ServerMessage?) : MapOutcome + data object Skip : MapOutcome +} + +// Per-domain mappers, tried in order. Each owns a disjoint slice of the payload hierarchy and +// returns [MapOutcome.Skip] for anything outside it. Split across files by domain area to keep +// each mapper and its import list small — see StageInferenceEventMappers, ToolEventMappers, etc. +private val domainMappers: List MapOutcome> = listOf( + ::mapSessionEvent, + ::mapStageInferenceEvent, + ::mapToolEvent, + ::mapLifecycleEvent, +) + suspend fun domainEventToServerMessage( event: StoredEvent, artifactStore: ArtifactStore, sessionSequence: Long = 0L, ): ServerMessage? { - val seq = event.sequence - return when (val p = event.payload) { - is ChatSessionStartedEvent -> ServerMessage.SessionAnnounced( - sessionId = p.sessionId, - workflowId = "chat", - sequence = seq, - sessionSequence = sessionSequence, - ) - - is WorkflowStartedEvent -> ServerMessage.SessionAnnounced( - sessionId = p.sessionId, - workflowId = p.workflowId, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is SessionWorkspaceBoundEvent -> ServerMessage.SessionWorkspaceBound( - sessionId = p.sessionId, - workspaceRoot = p.workspaceRoot, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is SessionNamedEvent -> ServerMessage.SessionRenamed( - sessionId = p.sessionId, - name = p.name, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ChatTurnEvent -> ServerMessage.ChatTurn( - sessionId = p.sessionId, - turnId = p.turnId, - role = p.role.name, - content = p.content, - latencyMs = p.latencyMs, - totalTokens = p.tokensUsed?.totalTokens, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is WorkflowCompletedEvent -> ServerMessage.SessionCompleted( - sessionId = p.sessionId, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is WorkflowFailedEvent -> ServerMessage.SessionFailed( - sessionId = p.sessionId, - reason = p.reason, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is TransitionExecutedEvent -> ServerMessage.StageStarted( - sessionId = p.sessionId, - stageId = p.to, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is StageCompletedEvent -> ServerMessage.StageCompleted( - sessionId = p.sessionId, - stageId = p.stageId, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is StageFailedEvent -> ServerMessage.StageFailed( - sessionId = p.sessionId, - stageId = p.stageId, - reason = p.reason, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is OrchestrationPausedEvent -> mapOrchestrationPaused(p, seq, sessionSequence) - is OrchestrationResumedEvent -> ServerMessage.SessionResumed( - sessionId = p.sessionId, - stageId = p.stageId, - sequence = seq, - sessionSequence = sessionSequence, - ) - is InferenceStartedEvent -> ServerMessage.InferenceStarted( - sessionId = p.sessionId, - stageId = p.stageId, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is InferenceCompletedEvent -> mapInferenceCompleted(event, p, artifactStore, sessionSequence) - is InferenceTimeoutEvent -> ServerMessage.InferenceTimedOut( - sessionId = p.sessionId, - stageId = p.stageId, - elapsedMs = p.timeoutMs, - sequence = seq, - sessionSequence = sessionSequence, - ) - is InferenceFailedEvent -> ServerMessage.InferenceFailed( - sessionId = p.sessionId, - stageId = p.stageId, - reason = p.reason, - sequence = seq, - sessionSequence = sessionSequence, - ) - is RetryAttemptedEvent -> ServerMessage.RetryAttempted( - sessionId = p.sessionId, - stageId = p.stageId, - attemptNumber = p.attemptNumber, - maxAttempts = p.maxAttempts, - failureReason = p.failureReason, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ToolInvocationRequestedEvent -> ServerMessage.ToolStarted( - sessionId = p.sessionId, - toolName = p.toolName, - tier = p.tier, - params = prettyToolParams(p.request.parameters), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ToolExecutionCompletedEvent -> ServerMessage.ToolCompleted( - sessionId = p.sessionId, - toolName = p.toolName, - outputSummary = p.receipt.outputSummary, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - diff = p.receipt.diff, - affectedEntities = p.receipt.affectedEntities, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ToolExecutionFailedEvent -> ServerMessage.ToolFailed( - sessionId = p.sessionId, - toolName = p.toolName, - reason = p.reason, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ToolExecutionRejectedEvent -> ServerMessage.ToolRejected( - sessionId = p.sessionId, - toolName = p.toolName, - reason = p.reason, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ToolCallAssessedEvent -> ServerMessage.ToolAssessed( - sessionId = p.sessionId, - stageId = p.stageId, - toolName = p.toolName, - disposition = p.disposition.name, - issues = p.issues.map { AssessedIssueDto(it.code, it.message, it.severity) }, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ReviewFindingsRaisedEvent -> ServerMessage.ReviewFindings( - sessionId = p.sessionId, - stageId = p.stageId, - verdict = p.verdict.name, - findings = p.findings.map { - ReviewFindingDto( - severity = it.severity.name, - confidence = it.confidence, - category = it.category, - target = it.target, - message = it.message, - suggestedFix = it.suggestedFix, - correctness = it.correctness, - ) - }, - blocked = p.blocked, - sequence = seq, - sessionSequence = sessionSequence, - ) - - is ApprovalRequestedEvent -> mapApprovalRequested(p, seq, sessionSequence) - is ClarificationRequestedEvent -> ServerMessage.ClarificationRequired( - sessionId = p.sessionId, - requestId = p.requestId, - stageId = p.stageId, - questions = p.questions, - sequence = seq, - sessionSequence = sessionSequence, - ) - is WorkflowProposedEvent -> ServerMessage.WorkflowProposed( - sessionId = p.sessionId, - proposalId = p.proposalId, - prompt = p.prompt, - candidates = p.candidates, - originalRequest = p.originalRequest, - sequence = seq, - sessionSequence = sessionSequence, - ) - is ApprovalDecisionResolvedEvent -> ServerMessage.ApprovalResolved( - // ApprovalDecisionResolvedEvent has no sessionId on its payload — read it from the event envelope - sessionId = event.metadata.sessionId, - requestId = p.requestId, - outcome = p.outcome.name, - reason = p.reason, - sequence = seq, - sessionSequence = sessionSequence, - ) - is TalkieNarrationEvent -> ServerMessage.Narration( - sessionId = p.sessionId, - content = p.content, - stageId = p.stageId, - latencyMs = p.latencyMs, - totalTokens = p.tokensUsed?.totalTokens, - sequence = seq, - sessionSequence = sessionSequence, - ) - is ArtifactCreatedEvent -> ServerMessage.ArtifactCreated( - sessionId = p.sessionId, - stageId = p.stageId, - artifactId = p.artifactId, - sequence = seq, - sessionSequence = sessionSequence, - ) - is ArtifactValidatedEvent -> ServerMessage.ArtifactValidated( - sessionId = p.sessionId, - stageId = p.stageId, - artifactId = p.artifactId, - sequence = seq, - sessionSequence = sessionSequence, - ) - // Transient pre-validation marker, emitted microseconds before Validated or a - // stage failure — either of those carries the outcome the operator cares about. - is ArtifactValidatingEvent -> null - is ExecutionPlanLockedEvent -> ServerMessage.PlanLocked( - sessionId = p.sessionId, - workflowId = p.workflowId, - stageIds = p.stageIds, - sequence = seq, - sessionSequence = sessionSequence, - ) - is ModelLoadedEvent -> ServerMessage.ModelChanged( - modelId = p.modelId, - providerId = p.providerId.value, - loaded = true, - ) - is ModelUnloadedEvent -> ServerMessage.ModelChanged( - modelId = p.modelId, - providerId = p.providerId.value, - loaded = false, - ) - // Internal slot→CAS-hash bookkeeping (F-007 durable bridge); no operator-facing surface. - is ArtifactContentStoredEvent -> null - - // Freestyle graph-rerouting bookkeeping. The deterministic record is in place; a dedicated - // operator surface ships with the LLM-proposal + approval-confirm front-half. - is PreemptRedirectEvent -> null - is PreemptRedirectBlockedEvent -> null - - else -> { - log.debug( - "DomainEventMapper: unmapped payload type={} sessionId={} sequence={}", - p::class.simpleName, - event.metadata.sessionId, - event.sequence, - ) - null + for (mapper in domainMappers) { + when (val outcome = mapper(event, artifactStore, sessionSequence)) { + is MapOutcome.Emit -> return outcome.message + MapOutcome.Skip -> Unit } } -} - -private fun mapOrchestrationPaused( - p: OrchestrationPausedEvent, - seq: Long, - sessionSequence: Long, -): ServerMessage { - val reason = if (p.reason == "APPROVAL_PENDING") PauseReason.APPROVAL_PENDING else PauseReason.USER_REQUESTED - return ServerMessage.SessionPaused( - sessionId = p.sessionId, - reason = reason, - sequence = seq, - sessionSequence = sessionSequence, + log.debug( + "DomainEventMapper: unmapped payload type={} sessionId={} sequence={}", + event.payload::class.simpleName, + event.metadata.sessionId, + event.sequence, ) + return null } - -private suspend fun mapInferenceCompleted( - event: StoredEvent, - p: InferenceCompletedEvent, - artifactStore: ArtifactStore, - sessionSequence: Long, -): ServerMessage { - val response = runCatching { - artifactStore.get(p.responseArtifactId)?.toString(Charsets.UTF_8) ?: "" - }.getOrElse { "" } - val reasoning = p.reasoningArtifactId?.let { id -> - runCatching { artifactStore.get(id)?.toString(Charsets.UTF_8) ?: "" }.getOrElse { "" } - } ?: "" - return ServerMessage.InferenceCompleted( - sessionId = p.sessionId, - stageId = p.stageId, - outputSummary = response, - responseText = response, - reasoning = reasoning, - occurredAt = event.metadata.timestamp.toEpochMilliseconds(), - totalTokens = p.tokensUsed.totalTokens, - sequence = event.sequence, - sessionSequence = sessionSequence, - ) -} - -private fun mapApprovalRequested( - p: ApprovalRequestedEvent, - seq: Long, - sessionSequence: Long, -): ServerMessage = - ServerMessage.ApprovalRequired( - sessionId = p.sessionId, - requestId = p.requestId, - tier = p.tier, - riskSummary = p.riskSummary?.toDto() ?: RiskSummaryDto( - level = p.tier.name, - factors = emptyList(), - recommendedAction = RiskAction.PROMPT_USER.name, - rationale = emptyList(), - ), - toolName = p.toolName, - preview = p.preview, - sequence = seq, - sessionSequence = sessionSequence, - ) - -/** - * Render a tool call's raw parameter map into compact, human-readable "key=value" cells for the - * client's tool-call row. Primary keys (path/command/query/…) lead so the most salient argument is - * first; long values are truncated and multi-line values collapsed so the WS frame stays small. - */ -internal fun prettyToolParams(parameters: Map): List { - if (parameters.isEmpty()) return emptyList() - val primary = listOf("command", "path", "query", "url", "pattern", "content", "operation") - val ordered = parameters.entries.sortedWith( - compareBy({ primary.indexOf(it.key).let { i -> if (i < 0) primary.size else i } }, { it.key }), - ) - return ordered.take(MAX_PARAM_CELLS).map { (k, v) -> "$k=${formatParamValue(v)}" } -} - -private fun formatParamValue(value: Any?): String { - val raw = when (value) { - null -> "null" - is String -> value - is Collection<*> -> value.joinToString(", ", prefix = "[", postfix = "]") { formatParamValue(it) } - else -> value.toString() - } - val flattened = raw.replace('\n', ' ').replace('\r', ' ').trim() - val clipped = if (flattened.length > MAX_PARAM_VALUE_LEN) flattened.take(MAX_PARAM_VALUE_LEN) + "…" else flattened - // Quote strings that carry whitespace so the boundary of the value is unambiguous in the row. - return if (value is String && clipped.any { it.isWhitespace() }) "\"$clipped\"" else clipped -} - -private const val MAX_PARAM_CELLS = 5 -private const val MAX_PARAM_VALUE_LEN = 80 diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/LifecycleEventMappers.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/LifecycleEventMappers.kt new file mode 100644 index 00000000..91fe9202 --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/LifecycleEventMappers.kt @@ -0,0 +1,150 @@ +package com.correx.apps.server.bridge + +import com.correx.apps.server.protocol.RiskSummaryDto +import com.correx.apps.server.protocol.ServerMessage +import com.correx.apps.server.protocol.toDto +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.ApprovalDecisionResolvedEvent +import com.correx.core.events.events.ApprovalRequestedEvent +import com.correx.core.events.events.ArtifactContentStoredEvent +import com.correx.core.events.events.ArtifactCreatedEvent +import com.correx.core.events.events.ArtifactValidatedEvent +import com.correx.core.events.events.ArtifactValidatingEvent +import com.correx.core.events.events.ClarificationRequestedEvent +import com.correx.core.events.events.ExecutionPlanLockedEvent +import com.correx.core.events.events.ModelLoadedEvent +import com.correx.core.events.events.ModelUnloadedEvent +import com.correx.core.events.events.PreemptRedirectBlockedEvent +import com.correx.core.events.events.PreemptRedirectEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.TalkieNarrationEvent +import com.correx.core.events.events.WorkflowProposedEvent +import com.correx.core.events.risk.RiskAction + +/** + * Approval / clarification / workflow-proposal / narration / artifact / model / preempt events — + * the remaining operator-facing (or deliberately suppressed) lifecycle surface. Branches that + * return `null` are handled-but-not-surfaced (transient bookkeeping); wrapping them in + * [MapOutcome.Emit] keeps them from falling through to the dispatcher's "unmapped" log. + */ +@Suppress("UnusedParameter", "LongMethod", "CyclomaticComplexMethod") +internal suspend fun mapLifecycleEvent( + event: StoredEvent, + artifactStore: ArtifactStore, + sessionSequence: Long, +): MapOutcome { + val seq = event.sequence + val msg: ServerMessage? = when (val p = event.payload) { + is ApprovalRequestedEvent -> mapApprovalRequested(p, seq, sessionSequence) + is ClarificationRequestedEvent -> ServerMessage.ClarificationRequired( + sessionId = p.sessionId, + requestId = p.requestId, + stageId = p.stageId, + questions = p.questions, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is WorkflowProposedEvent -> ServerMessage.WorkflowProposed( + sessionId = p.sessionId, + proposalId = p.proposalId, + prompt = p.prompt, + candidates = p.candidates, + originalRequest = p.originalRequest, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ApprovalDecisionResolvedEvent -> ServerMessage.ApprovalResolved( + // ApprovalDecisionResolvedEvent has no sessionId on its payload — read it from the event envelope + sessionId = event.metadata.sessionId, + requestId = p.requestId, + outcome = p.outcome.name, + reason = p.reason, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is TalkieNarrationEvent -> ServerMessage.Narration( + sessionId = p.sessionId, + content = p.content, + stageId = p.stageId, + latencyMs = p.latencyMs, + totalTokens = p.tokensUsed?.totalTokens, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ArtifactCreatedEvent -> ServerMessage.ArtifactCreated( + sessionId = p.sessionId, + stageId = p.stageId, + artifactId = p.artifactId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ArtifactValidatedEvent -> ServerMessage.ArtifactValidated( + sessionId = p.sessionId, + stageId = p.stageId, + artifactId = p.artifactId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + // Transient pre-validation marker, emitted microseconds before Validated or a + // stage failure — either of those carries the outcome the operator cares about. + is ArtifactValidatingEvent -> null + + is ExecutionPlanLockedEvent -> ServerMessage.PlanLocked( + sessionId = p.sessionId, + workflowId = p.workflowId, + stageIds = p.stageIds, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ModelLoadedEvent -> ServerMessage.ModelChanged( + modelId = p.modelId, + providerId = p.providerId.value, + loaded = true, + ) + + is ModelUnloadedEvent -> ServerMessage.ModelChanged( + modelId = p.modelId, + providerId = p.providerId.value, + loaded = false, + ) + + // Internal slot→CAS-hash bookkeeping (F-007 durable bridge); no operator-facing surface. + is ArtifactContentStoredEvent -> null + + // Freestyle graph-rerouting bookkeeping. The deterministic record is in place; a dedicated + // operator surface ships with the LLM-proposal + approval-confirm front-half. + is PreemptRedirectEvent -> null + is PreemptRedirectBlockedEvent -> null + + else -> return MapOutcome.Skip + } + return MapOutcome.Emit(msg) +} + +private fun mapApprovalRequested( + p: ApprovalRequestedEvent, + seq: Long, + sessionSequence: Long, +): ServerMessage = + ServerMessage.ApprovalRequired( + sessionId = p.sessionId, + requestId = p.requestId, + tier = p.tier, + riskSummary = p.riskSummary?.toDto() ?: RiskSummaryDto( + level = p.tier.name, + factors = emptyList(), + recommendedAction = RiskAction.PROMPT_USER.name, + rationale = emptyList(), + ), + toolName = p.toolName, + preview = p.preview, + sequence = seq, + sessionSequence = sessionSequence, + ) diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventBridge.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventBridge.kt index 0eee9e72..cca340c2 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventBridge.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventBridge.kt @@ -14,18 +14,15 @@ import com.correx.core.approvals.ApprovalProjector import com.correx.core.approvals.DefaultApprovalReducer import com.correx.core.approvals.model.ApprovalState import com.correx.core.artifactstore.ArtifactStore -import com.correx.core.events.events.EventMetadata import com.correx.core.events.events.InferenceCompletedEvent import com.correx.core.events.events.InferenceStartedEvent import com.correx.core.events.events.InferenceTimeoutEvent -import com.correx.core.events.events.NewEvent import com.correx.core.events.events.ApprovalRequestedEvent import com.correx.core.events.events.ArtifactCreatedEvent import com.correx.core.events.events.ChatTurnEvent import com.correx.core.events.events.ClarificationAnsweredEvent import com.correx.core.events.events.ClarificationRequestedEvent import com.correx.core.events.events.OrchestrationPausedEvent -import com.correx.core.events.events.OrchestrationResumedEvent import com.correx.core.events.events.StageCompletedEvent import com.correx.core.events.events.SessionWorkspaceBoundEvent import com.correx.core.events.events.StageFailedEvent @@ -40,10 +37,7 @@ import com.correx.core.events.events.WorkflowCompletedEvent import com.correx.core.events.events.WorkflowStartedEvent import com.correx.core.events.events.WorkflowFailedEvent import com.correx.core.events.orchestration.OrchestrationStatus -import com.correx.core.events.types.EventId -import kotlinx.datetime.Clock import com.correx.core.events.stores.EventStore -import com.correx.core.events.types.ApprovalRequestId import com.correx.core.events.types.SessionId import com.correx.core.kernel.orchestration.OrchestrationRepository import com.correx.core.tools.registry.ToolRegistry @@ -109,39 +103,11 @@ class SessionEventBridge( .sortedWith(compareBy({ it.timestamp }, { it.id.value })) } ?: emptyList() - // Fix stuck sessions: orchestration says PAUSED + pendingApproval but there are no - // actual unresolved approval requests. This happens when approval was resolved but - // OrchestrationResumedEvent was not emitted (pre-Feb-13 orchestrator bug). Append the - // missing event permanently so the session state corrects on all future replays. - // - // Guard: if there are more OrchestrationPausedEvents than ApprovalRequestedEvents the - // session just entered the approval gate and ApprovalRequestedEvent has not been stored - // yet (tiny race window between two sequential emits). Do NOT fire in that case — - // appending a spurious resume would hide the pending approval from the TUI. - val pauseCount = events.count { it.payload is OrchestrationPausedEvent } - val approvalRequestCount = events.count { it.payload is ApprovalRequestedEvent } - val hasUnpairedPause = pauseCount > approvalRequestCount - if (orchState.pendingApproval && pendingApprovalRequests.isEmpty() && !hasUnpairedPause) { - val alreadyResumed = events.any { it.payload is OrchestrationResumedEvent } - val stageId = orchState.currentStageId - if (!alreadyResumed && stageId != null) { - val resumeEvent = NewEvent( - metadata = EventMetadata( - eventId = EventId(java.util.UUID.randomUUID().toString()), - sessionId = sessionId, - timestamp = Clock.System.now(), - schemaVersion = 1, - causationId = null, - correlationId = null, - ), - payload = OrchestrationResumedEvent( - sessionId = sessionId, - stageId = stageId, - ), - ) - eventStore.append(resumeEvent) - } - } + // NOTE: repairing stuck approval-pauses (PAUSED + pendingApproval but no unresolved + // request, from a pre-Feb-13 orchestrator bug) used to happen HERE, per connection — + // a write side effect on a pure read path that two concurrent clients could double-fire. + // It now runs once at boot in ServerModule.repairStuckApprovalPauses(). replaySnapshot is + // read-only again. val pendingApprovals = pendingApprovalRequests.map { ApprovalDto( @@ -164,8 +130,8 @@ class SessionEventBridge( // Re-register pending approvals so the ApprovalCoordinator can route responses // from clients that connected after the ApprovalRequestedEvent was emitted. - pendingApprovals.forEach { dto -> - approvalCoordinator?.registerPendingRequest(ApprovalRequestId(dto.requestId), sessionId) + pendingApprovalRequests.forEach { req -> + approvalCoordinator?.registerPendingRequest(req.id, sessionId, req.tier) } val toolRecords = rebuildTools(events) diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventMappers.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventMappers.kt new file mode 100644 index 00000000..b53f1984 --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/SessionEventMappers.kt @@ -0,0 +1,78 @@ +package com.correx.apps.server.bridge + +import com.correx.apps.server.protocol.ServerMessage +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.ChatSessionStartedEvent +import com.correx.core.events.events.ChatTurnEvent +import com.correx.core.events.events.SessionNamedEvent +import com.correx.core.events.events.SessionWorkspaceBoundEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.WorkflowCompletedEvent +import com.correx.core.events.events.WorkflowFailedEvent +import com.correx.core.events.events.WorkflowStartedEvent + +/** Session lifecycle + chat-turn events. [artifactStore] is unused here but kept for a uniform mapper signature. */ +@Suppress("UnusedParameter", "LongMethod") +internal suspend fun mapSessionEvent( + event: StoredEvent, + artifactStore: ArtifactStore, + sessionSequence: Long, +): MapOutcome { + val seq = event.sequence + val msg = when (val p = event.payload) { + is ChatSessionStartedEvent -> ServerMessage.SessionAnnounced( + sessionId = p.sessionId, + workflowId = "chat", + sequence = seq, + sessionSequence = sessionSequence, + ) + + is WorkflowStartedEvent -> ServerMessage.SessionAnnounced( + sessionId = p.sessionId, + workflowId = p.workflowId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is SessionWorkspaceBoundEvent -> ServerMessage.SessionWorkspaceBound( + sessionId = p.sessionId, + workspaceRoot = p.workspaceRoot, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is SessionNamedEvent -> ServerMessage.SessionRenamed( + sessionId = p.sessionId, + name = p.name, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ChatTurnEvent -> ServerMessage.ChatTurn( + sessionId = p.sessionId, + turnId = p.turnId, + role = p.role.name, + content = p.content, + latencyMs = p.latencyMs, + totalTokens = p.tokensUsed?.totalTokens, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is WorkflowCompletedEvent -> ServerMessage.SessionCompleted( + sessionId = p.sessionId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is WorkflowFailedEvent -> ServerMessage.SessionFailed( + sessionId = p.sessionId, + reason = p.reason, + sequence = seq, + sessionSequence = sessionSequence, + ) + + else -> return MapOutcome.Skip + } + return MapOutcome.Emit(msg) +} diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/StageInferenceEventMappers.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/StageInferenceEventMappers.kt new file mode 100644 index 00000000..ee0492ac --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/StageInferenceEventMappers.kt @@ -0,0 +1,141 @@ +package com.correx.apps.server.bridge + +import com.correx.apps.server.protocol.PauseReason +import com.correx.apps.server.protocol.ServerMessage +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.InferenceCompletedEvent +import com.correx.core.events.events.InferenceFailedEvent +import com.correx.core.events.events.InferenceStartedEvent +import com.correx.core.events.events.InferenceTimeoutEvent +import com.correx.core.events.events.OrchestrationPausedEvent +import com.correx.core.events.events.OrchestrationResumedEvent +import com.correx.core.events.events.RetryAttemptedEvent +import com.correx.core.events.events.StageCompletedEvent +import com.correx.core.events.events.StageFailedEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.TransitionExecutedEvent + +/** Stage transition + inference lifecycle events (some carry timestamps / need the artifact store). */ +@Suppress("LongMethod") +internal suspend fun mapStageInferenceEvent( + event: StoredEvent, + artifactStore: ArtifactStore, + sessionSequence: Long, +): MapOutcome { + val seq = event.sequence + val msg = when (val p = event.payload) { + is TransitionExecutedEvent -> ServerMessage.StageStarted( + sessionId = p.sessionId, + stageId = p.to, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is StageCompletedEvent -> ServerMessage.StageCompleted( + sessionId = p.sessionId, + stageId = p.stageId, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is StageFailedEvent -> ServerMessage.StageFailed( + sessionId = p.sessionId, + stageId = p.stageId, + reason = p.reason, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is OrchestrationPausedEvent -> mapOrchestrationPaused(p, seq, sessionSequence) + is OrchestrationResumedEvent -> ServerMessage.SessionResumed( + sessionId = p.sessionId, + stageId = p.stageId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is InferenceStartedEvent -> ServerMessage.InferenceStarted( + sessionId = p.sessionId, + stageId = p.stageId, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is InferenceCompletedEvent -> mapInferenceCompleted(event, p, artifactStore, sessionSequence) + is InferenceTimeoutEvent -> ServerMessage.InferenceTimedOut( + sessionId = p.sessionId, + stageId = p.stageId, + elapsedMs = p.timeoutMs, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is InferenceFailedEvent -> ServerMessage.InferenceFailed( + sessionId = p.sessionId, + stageId = p.stageId, + reason = p.reason, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is RetryAttemptedEvent -> ServerMessage.RetryAttempted( + sessionId = p.sessionId, + stageId = p.stageId, + attemptNumber = p.attemptNumber, + maxAttempts = p.maxAttempts, + failureReason = p.failureReason, + sequence = seq, + sessionSequence = sessionSequence, + ) + + else -> return MapOutcome.Skip + } + return MapOutcome.Emit(msg) +} + +private fun mapOrchestrationPaused( + p: OrchestrationPausedEvent, + seq: Long, + sessionSequence: Long, +): ServerMessage { + val reason = when (p.reason) { + "APPROVAL_PENDING" -> PauseReason.APPROVAL_PENDING + "CLARIFICATION_PENDING" -> PauseReason.CLARIFICATION_PENDING + "ABANDONED_STALE" -> PauseReason.ABANDONED_STALE + else -> PauseReason.USER_REQUESTED + } + return ServerMessage.SessionPaused( + sessionId = p.sessionId, + reason = reason, + sequence = seq, + sessionSequence = sessionSequence, + ) +} + +private suspend fun mapInferenceCompleted( + event: StoredEvent, + p: InferenceCompletedEvent, + artifactStore: ArtifactStore, + sessionSequence: Long, +): ServerMessage { + val response = runCatching { + artifactStore.get(p.responseArtifactId)?.toString(Charsets.UTF_8) ?: "" + }.getOrElse { "" } + val reasoning = p.reasoningArtifactId?.let { id -> + runCatching { artifactStore.get(id)?.toString(Charsets.UTF_8) ?: "" }.getOrElse { "" } + } ?: "" + return ServerMessage.InferenceCompleted( + sessionId = p.sessionId, + stageId = p.stageId, + outputSummary = response, + responseText = response, + reasoning = reasoning, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + totalTokens = p.tokensUsed.totalTokens, + sequence = event.sequence, + sessionSequence = sessionSequence, + ) +} diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/bridge/ToolEventMappers.kt b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/ToolEventMappers.kt new file mode 100644 index 00000000..ca94c6e8 --- /dev/null +++ b/apps/server/src/main/kotlin/com/correx/apps/server/bridge/ToolEventMappers.kt @@ -0,0 +1,125 @@ +package com.correx.apps.server.bridge + +import com.correx.apps.server.protocol.AssessedIssueDto +import com.correx.apps.server.protocol.ReviewFindingDto +import com.correx.apps.server.protocol.ServerMessage +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.ReviewFindingsRaisedEvent +import com.correx.core.events.events.StoredEvent +import com.correx.core.events.events.ToolCallAssessedEvent +import com.correx.core.events.events.ToolExecutionCompletedEvent +import com.correx.core.events.events.ToolExecutionFailedEvent +import com.correx.core.events.events.ToolExecutionRejectedEvent +import com.correx.core.events.events.ToolInvocationRequestedEvent + +/** Tool invocation / execution / assessment + review-findings events. */ +@Suppress("UnusedParameter", "LongMethod") +internal suspend fun mapToolEvent( + event: StoredEvent, + artifactStore: ArtifactStore, + sessionSequence: Long, +): MapOutcome { + val seq = event.sequence + val msg = when (val p = event.payload) { + is ToolInvocationRequestedEvent -> ServerMessage.ToolStarted( + sessionId = p.sessionId, + toolName = p.toolName, + tier = p.tier, + params = prettyToolParams(p.request.parameters), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ToolExecutionCompletedEvent -> ServerMessage.ToolCompleted( + sessionId = p.sessionId, + toolName = p.toolName, + outputSummary = p.receipt.outputSummary, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + diff = p.receipt.diff, + affectedEntities = p.receipt.affectedEntities, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ToolExecutionFailedEvent -> ServerMessage.ToolFailed( + sessionId = p.sessionId, + toolName = p.toolName, + reason = p.reason, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ToolExecutionRejectedEvent -> ServerMessage.ToolRejected( + sessionId = p.sessionId, + toolName = p.toolName, + reason = p.reason, + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ToolCallAssessedEvent -> ServerMessage.ToolAssessed( + sessionId = p.sessionId, + stageId = p.stageId, + toolName = p.toolName, + disposition = p.disposition.name, + issues = p.issues.map { AssessedIssueDto(it.code, it.message, it.severity) }, + occurredAt = event.metadata.timestamp.toEpochMilliseconds(), + sequence = seq, + sessionSequence = sessionSequence, + ) + + is ReviewFindingsRaisedEvent -> ServerMessage.ReviewFindings( + sessionId = p.sessionId, + stageId = p.stageId, + verdict = p.verdict.name, + findings = p.findings.map { + ReviewFindingDto( + severity = it.severity.name, + confidence = it.confidence, + category = it.category, + target = it.target, + message = it.message, + suggestedFix = it.suggestedFix, + correctness = it.correctness, + ) + }, + blocked = p.blocked, + sequence = seq, + sessionSequence = sessionSequence, + ) + + else -> return MapOutcome.Skip + } + return MapOutcome.Emit(msg) +} + +/** + * Render a tool call's raw parameter map into compact, human-readable "key=value" cells for the + * client's tool-call row. Primary keys (path/command/query/…) lead so the most salient argument is + * first; long values are truncated and multi-line values collapsed so the WS frame stays small. + */ +internal fun prettyToolParams(parameters: Map): List { + if (parameters.isEmpty()) return emptyList() + val primary = listOf("command", "path", "query", "url", "pattern", "content", "operation") + val ordered = parameters.entries.sortedWith( + compareBy({ primary.indexOf(it.key).let { i -> if (i < 0) primary.size else i } }, { it.key }), + ) + return ordered.take(MAX_PARAM_CELLS).map { (k, v) -> "$k=${formatParamValue(v)}" } +} + +private fun formatParamValue(value: Any?): String { + val raw = when (value) { + null -> "null" + is String -> value + is Collection<*> -> value.joinToString(", ", prefix = "[", postfix = "]") { formatParamValue(it) } + else -> value.toString() + } + val flattened = raw.replace('\n', ' ').replace('\r', ' ').trim() + val clipped = if (flattened.length > MAX_PARAM_VALUE_LEN) flattened.take(MAX_PARAM_VALUE_LEN) + "…" else flattened + // Quote strings that carry whitespace so the boundary of the value is unambiguous in the row. + return if (value is String && clipped.any { it.isWhitespace() }) "\"$clipped\"" else clipped +} + +private const val MAX_PARAM_CELLS = 5 +private const val MAX_PARAM_VALUE_LEN = 80 diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/narration/NarrationSubscriber.kt b/apps/server/src/main/kotlin/com/correx/apps/server/narration/NarrationSubscriber.kt index 4b3a9fe3..9fecf431 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/narration/NarrationSubscriber.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/narration/NarrationSubscriber.kt @@ -104,22 +104,28 @@ class NarrationSubscriber( stageId = p.stageId.value, ), ) - is WorkflowCompletedEvent -> enqueue( - sid, - NarrationTrigger( - kind = "workflow_completed", - instruction = "The workflow finished successfully. Provide a brief summary.", - stageId = p.terminalStageId.value, - ), - ) - is WorkflowFailedEvent -> enqueue( - sid, - NarrationTrigger( - kind = "workflow_failed", - instruction = "The workflow failed in stage ${p.stageId.value}: ${p.reason}. Explain the failure to the user.", - stageId = p.stageId.value, - ), - ) + is WorkflowCompletedEvent -> { + enqueue( + sid, + NarrationTrigger( + kind = "workflow_completed", + instruction = "The workflow finished successfully. Provide a brief summary.", + stageId = p.terminalStageId.value, + ), + ) + closeLane(sid) + } + is WorkflowFailedEvent -> { + enqueue( + sid, + NarrationTrigger( + kind = "workflow_failed", + instruction = "The workflow failed in stage ${p.stageId.value}: ${p.reason}. Explain the failure to the user.", + stageId = p.stageId.value, + ), + ) + closeLane(sid) + } // Surface the semantic reviewer's verdict conversationally instead of a raw findings // dump: the narrator turns "FAIL, 2 findings" into a plain-language explanation the // operator can act on. Only narrate when there's something to say (non-PASS or findings). @@ -221,6 +227,13 @@ class NarrationSubscriber( lanes[sessionId.value]?.pendingPauses?.clear() } + /** Closes a terminated session's lane so its worker drains any buffered narration (the terminal + * one enqueued just before this) and then exits, self-removing the lane from [lanes]. Without + * this, every session leaks its channel + worker coroutine + map entry forever. */ + private fun closeLane(sessionId: SessionId) { + lanes[sessionId.value]?.channel?.close() + } + private fun startLane(sessionId: SessionId): SessionLane { val channel = Channel(capacity = Channel.UNLIMITED) val lane = SessionLane(channel, used = 0) @@ -249,6 +262,9 @@ class NarrationSubscriber( ) } } + // Channel closed on workflow termination: drop the lane so it stops leaking. A later event + // for the same session (none expected post-terminal) would lazily start a fresh lane. + lanes.remove(sessionId.value) } return lane } diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/protocol/Dtos.kt b/apps/server/src/main/kotlin/com/correx/apps/server/protocol/Dtos.kt index 8c43090f..43eaa87a 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/protocol/Dtos.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/protocol/Dtos.kt @@ -61,6 +61,8 @@ data class ApprovalDto( @Serializable enum class PauseReason { APPROVAL_PENDING, + CLARIFICATION_PENDING, + ABANDONED_STALE, USER_REQUESTED, } diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt b/apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt index 512e42a8..777c50fb 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt @@ -121,10 +121,14 @@ private fun Route.startSessionRoute(module: ServerModule) { val graph = module.workflowRegistry.find(body.workflowId) ?: return@post call.respond(HttpStatusCode.BadRequest, "Unknown workflowId: ${body.workflowId}") val sessionId: SessionId = TypeId(UUID.randomUUID().toString()) + // Anchor the session's workspace (invariant #9) so path containment, project profile and + // grants resolve against a bound root. REST carries no cwd, so the resolver's default/ + // fallback root is bound — parity with the WS StartSession path. + val workspace = module.bindWorkspace(sessionId, null) body.intent?.takeIf { it.isNotBlank() }?.let { intent -> EventDispatcher(module.eventStore).emit(InitialIntentEvent(sessionId, intent), sessionId) } - module.launchSessionRun(sessionId, graph) + module.launchSessionRun(sessionId, graph, module.orchestrationConfig().copy(workspace = workspace)) call.respond(HttpStatusCode.Accepted, StartSessionResponse(sessionId.value)) } } diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/ws/GlobalStreamHandler.kt b/apps/server/src/main/kotlin/com/correx/apps/server/ws/GlobalStreamHandler.kt index ca08b3b8..d3f048a6 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/ws/GlobalStreamHandler.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/ws/GlobalStreamHandler.kt @@ -12,7 +12,6 @@ import com.correx.apps.server.protocol.ServerMessage import com.correx.apps.server.protocol.WorkflowDto import com.correx.apps.server.protocol.StageToolDecl import com.correx.apps.server.protocol.ToolDecl -import com.correx.apps.server.workspace.WorkspaceResolution import com.correx.core.approvals.GRANT_LEDGER_SESSION_ID import com.correx.core.approvals.GrantScope import com.correx.core.events.events.ApprovalGrantCreatedEvent @@ -66,6 +65,9 @@ import java.util.UUID private val log = LoggerFactory.getLogger(GlobalStreamHandler::class.java) private const val BUFFER_CAPACITY = 1024 + +/** Raised to tear down a WS connection whose forward buffer overflowed, so the client reconnects. */ +private class SlowClientException : RuntimeException("global WS stream buffer overflow") private const val RESOURCE_PUSH_INTERVAL_MS = 2500L private const val BYTES_PER_MB = 1024L * 1024L @@ -716,47 +718,10 @@ class GlobalStreamHandler(private val module: ServerModule) { val sessionId: SessionId = TypeId(UUID.randomUUID().toString()) log.info("starting session={} workflow={}", sessionId.value, msg.workflowId) - // Resolve workspace from the Hello-frame working directory (invariant #9: record the - // resolver's decision at handshake time as an event; replay reads the recorded fact). - val resolvedWorkspace: WorkspaceContext? = module.workspaceResolver?.let { resolver -> - val resolution = withContext(Dispatchers.IO) { resolver.resolve(workingDir) } - when (resolution) { - is WorkspaceResolution.Bound -> { - log.info("workspace bound: root={}", resolution.workspace.workspaceRoot) - resolution.workspace - } - is WorkspaceResolution.Rejected -> { - // Deliberate: a rejected client path still binds the resolver's fallback - // workspace. The fallback root is recorded in SessionWorkspaceBoundEvent so - // replay reproduces the same binding; the rejection reason is audit-logged - // here and does NOT become a field on the event (YAGNI). - log.warn( - "workspace rejected: session={} reason={} fallback={}", - sessionId.value, resolution.reason, resolution.fallback.workspaceRoot, - ) - resolution.fallback - } - } - } - - // Emit SessionWorkspaceBoundEvent when a workspace was resolved (invariant #9). - resolvedWorkspace?.let { ws -> - module.eventStore.append(NewEvent( - metadata = EventMetadata( - eventId = EventId(UUID.randomUUID().toString()), - sessionId = sessionId, - timestamp = Clock.System.now(), - schemaVersion = 1, - causationId = null, - correlationId = null, - ), - payload = SessionWorkspaceBoundEvent( - sessionId = sessionId, - workspaceRoot = ws.workspaceRoot.toString(), - allowedPaths = ws.allowedPaths.map { it.toString() }, - ), - )) - } + // Resolve + record the workspace from the Hello-frame working directory (invariant #9: + // record the resolver's decision as an event; replay reads the recorded fact). Shared with + // the REST launcher via module.bindWorkspace. + val resolvedWorkspace: WorkspaceContext? = module.bindWorkspace(sessionId, workingDir) // Send StageToolManifest FIRST. If the WS is already closed, this throws and the // orchestrator never launches — no silent data loss. The exception propagates to the @@ -826,22 +791,28 @@ internal suspend fun streamGlobal( is SharedFlow -> source.onSubscription { subscribed.complete(Unit) } else -> source.onStart { subscribed.complete(Unit) } } + // trySend, never send: a blocking send here would back-pressure globalFlow (SUSPEND overflow), + // suspending append() and stalling the whole kernel on one wedged client. On overflow we instead + // drop the connection (close the buffer); the client reconnects and re-syncs via replaySnapshot. val subscription = launch { - signaled.collect { buffer.send(it) } + signaled.collect { + val result = buffer.trySend(it) + if (result.isFailure && !result.isClosed) { + log.warn("global WS stream buffer overflow (slow client); dropping connection to re-sync") + buffer.close(SlowClientException()) + } + } } - // Per-session sequence counters so live events carry the correct sessionSequence - // and are not silently dropped by the TUI's SnapshotPhaseReducer dedup filter. - val sessionSequences = mutableMapOf() - try { subscribed.await() bridge.replaySnapshot() for (event in buffer) { - val sid = event.metadata.sessionId.value - val seq = sessionSequences.getOrDefault(sid, 0L) + 1 - sessionSequences[sid] = seq - val msg = mapper.map(event, sessionSequence = seq) ?: continue + // Use the event's own persisted sessionSequence, not a per-connection counter. The + // snapshot advertises lastSessionSequence = MAX(session_sequence); a counter restarting + // at 1 per connection would collide with that after reconnect and the TUI's dedup/ + // ordering filter (keyed on sessionSequence) would silently drop or misorder frames. + val msg = mapper.map(event, sessionSequence = event.sessionSequence) ?: continue sendFrame(msg) } } finally { diff --git a/apps/server/src/test/kotlin/com/correx/apps/server/approval/ApprovalCoordinatorWiringTest.kt b/apps/server/src/test/kotlin/com/correx/apps/server/approval/ApprovalCoordinatorWiringTest.kt index 5e83c312..2f4e0656 100644 --- a/apps/server/src/test/kotlin/com/correx/apps/server/approval/ApprovalCoordinatorWiringTest.kt +++ b/apps/server/src/test/kotlin/com/correx/apps/server/approval/ApprovalCoordinatorWiringTest.kt @@ -60,13 +60,56 @@ class ApprovalCoordinatorWiringTest { scope.cancel() } - private class RecordingGateway : ApprovalGateway { + private class RecordingGateway(@Volatile var failNext: Boolean = false) : ApprovalGateway { val submissions = CopyOnWriteArrayList>() override suspend fun submitApprovalDecision(requestId: ApprovalRequestId, decision: DomainApprovalDecision) { + if (failNext) { + failNext = false + error("submit boom") + } submissions.add(requestId to decision) } } + private fun approvalEvent(tier: Tier) = ApprovalRequestedEvent( + requestId = requestId, + tier = tier, + validationReportId = ValidationReportId("vr-1"), + riskSummaryId = null, + sessionId = sessionId, + stageId = null, + projectId = null, + ) + + @Test + fun `submit failure keeps request answerable and retry succeeds`(): Unit = runBlocking { + val gateway = RecordingGateway(failNext = true) + val coord = ApprovalCoordinator(gateway) + coord.onApprovalRequested(approvalEvent(Tier.T2)) + val msg = ClientMessage.ApprovalResponse(requestId, ApprovalDecision.APPROVE, steeringNote = null) + + val first = coord.handleResponse(msg, sessionId) + assertInstanceOf(ServerMessage.ProtocolError::class.java, first) + assertTrue(gateway.submissions.isEmpty()) + + // Retry must not be blocked by a stale resolved flag. + val second = coord.handleResponse(msg, sessionId) + assertNull(second) + assertEquals(1, gateway.submissions.size) + } + + @Test + fun `recorded decision carries the request's real tier`(): Unit = runBlocking { + val gateway = RecordingGateway() + val coord = ApprovalCoordinator(gateway) + coord.onApprovalRequested(approvalEvent(Tier.T3)) + val msg = ClientMessage.ApprovalResponse(requestId, ApprovalDecision.APPROVE, steeringNote = null) + + coord.handleResponse(msg, sessionId) + + assertEquals(Tier.T3, gateway.submissions[0].second.tier) + } + private fun storedEvent(payload: EventPayload, seq: Long): StoredEvent = StoredEvent( metadata = EventMetadata( eventId = EventId("evt-$seq"), diff --git a/apps/tui-go/internal/app/server.go b/apps/tui-go/internal/app/server.go index a6ae864b..6a193237 100644 --- a/apps/tui-go/internal/app/server.go +++ b/apps/tui-go/internal/app/server.go @@ -120,8 +120,13 @@ func (m *Model) applyServer(msg protocol.ServerMessage) { } case protocol.TypeSessionPaused: label := "PAUSED" - if msg.Reason == "APPROVAL_PENDING" { + switch msg.Reason { + case "APPROVAL_PENDING": label = "PAUSED awaiting approval" + case "CLARIFICATION_PENDING": + label = "PAUSED awaiting answer" + case "ABANDONED_STALE": + label = "PAUSED (stale)" } m.touch(msg.SessionID, label) if s := m.session(msg.SessionID); s != nil { diff --git a/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt b/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt index 41a8a674..b76312a2 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/ConfigLoader.kt @@ -630,6 +630,19 @@ object ConfigLoader { compressionLevel = asInt(orchestrationSection["compression_level"], 4), tokenPrunerUrl = asString(orchestrationSection["token_pruner_url"], "http://127.0.0.1:8199"), + maxToolRounds = asInt(orchestrationSection["max_tool_rounds"], 30), + readLoopNudgeThreshold = asInt(orchestrationSection["read_loop_nudge_threshold"], 3), + rejectionLoopNudgeThreshold = asInt(orchestrationSection["rejection_loop_nudge_threshold"], 3), + maxFeedbackIssues = asInt(orchestrationSection["max_feedback_issues"], 3), + repoMapInjectTopK = asInt(orchestrationSection["repo_map_inject_top_k"], 30), + repoMapFilesPerDir = asInt(orchestrationSection["repo_map_files_per_dir"], 8), + docsCatalogMax = asInt(orchestrationSection["docs_catalog_max"], 20), + maxClarificationRounds = asInt(orchestrationSection["max_clarification_rounds"], 3), + reviewBlockMinConfidence = asDouble(orchestrationSection["review_block_min_confidence"], 0.7), + reviewBlockRetryCap = asInt(orchestrationSection["review_block_retry_cap"], 20), + defaultMaxRefinement = asInt(orchestrationSection["default_max_refinement"], 3), + recoveryRouteBudget = asInt(orchestrationSection["recovery_route_budget"], 2), + intentRouteBudget = asInt(orchestrationSection["intent_route_budget"], 2), ) val modelsSettings = ModelsSettings( @@ -649,6 +662,15 @@ object ConfigLoader { ) } + val samplingSection = sections["sampling"] ?: emptyMap() + val sampling = SamplingConfig( + temperature = asDouble(samplingSection["temperature"], 0.7), + topP = asDouble(samplingSection["top_p"], 1.0), + topK = samplingSection["top_k"]?.let { asInt(it) }, + minP = samplingSection["min_p"]?.let { asDouble(it) }, + repeatPenalty = samplingSection["repeat_penalty"]?.let { asDouble(it) }, + ) + return CorrexConfig( server = server, tui = tui, @@ -662,6 +684,7 @@ object ConfigLoader { project = project, personalization = personalization, orchestration = orchestration, + sampling = sampling, ) } diff --git a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt index 1b529324..a53d85c6 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfig.kt @@ -16,9 +16,26 @@ data class CorrexConfig( val project: ProjectConfig = ProjectConfig(), val personalization: PersonalizationConfig = PersonalizationConfig(), val orchestration: OrchestrationKnobs = OrchestrationKnobs(), + val sampling: SamplingConfig = SamplingConfig(), val health: HealthConfig = HealthConfig(), ) +/** + * Sampling knobs sent to the llama-server on every *stage* inference request (the main agentic + * loop). [temperature] and [topP] default to the former hardcoded stage values. [topK], [minP] and + * [repeatPenalty] are null by default, meaning the request omits them and the model keeps its own + * default — set them to tighten a rambling local model. maxTokens is not here: it is pinned per-stage + * to the token budget. Talkie chat/narration have their own [GenerationSettings]/[NarrationSettings]. + */ +@Serializable +data class SamplingConfig( + val temperature: Double = 0.7, + val topP: Double = 1.0, + val topK: Int? = null, + val minP: Double? = null, + val repeatPenalty: Double? = null, +) + /** * Continuous health watch (observability-spec §4). When [enabled], a background monitor polls * the on-disk footprint every [intervalMs] and records a degraded/restored event on the edge. @@ -69,6 +86,25 @@ data class OrchestrationKnobs( */ val compressionLevel: Int = 4, val tokenPrunerUrl: String = "http://127.0.0.1:8199", + /** + * Orchestration loop/threshold/budget tuning, read once at server startup. Defaults equal the + * kernel's former hardcoded constants — an absent section reproduces prior behavior. Reach for + * these when a local model misbehaves (thrashes a read loop, re-asks clarifications, gets trapped + * by a stubborn reviewer). Mirrors kernel OrchestrationTuning. + */ + val maxToolRounds: Int = 30, + val readLoopNudgeThreshold: Int = 3, + val rejectionLoopNudgeThreshold: Int = 3, + val maxFeedbackIssues: Int = 3, + val repoMapInjectTopK: Int = 30, + val repoMapFilesPerDir: Int = 8, + val docsCatalogMax: Int = 20, + val maxClarificationRounds: Int = 3, + val reviewBlockMinConfidence: Double = 0.7, + val reviewBlockRetryCap: Int = 20, + val defaultMaxRefinement: Int = 3, + val recoveryRouteBudget: Int = 2, + val intentRouteBudget: Int = 2, ) @Serializable diff --git a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt index 0ffe77af..e24b676e 100644 --- a/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt +++ b/core/config/src/main/kotlin/com/correx/core/config/CorrexConfigWriter.kt @@ -86,6 +86,28 @@ object CorrexConfigWriter { b.kv("stage_timeout_ms", cfg.orchestration.stageTimeoutMs) b.kv("journal_compaction_token_threshold", cfg.orchestration.journalCompactionTokenThreshold) b.kv("resume_abandoned_max_age_minutes", cfg.orchestration.resumeAbandonedMaxAgeMinutes) + b.kv("compression_level", cfg.orchestration.compressionLevel) + b.kv("token_pruner_url", cfg.orchestration.tokenPrunerUrl) + b.kv("max_tool_rounds", cfg.orchestration.maxToolRounds) + b.kv("read_loop_nudge_threshold", cfg.orchestration.readLoopNudgeThreshold) + b.kv("rejection_loop_nudge_threshold", cfg.orchestration.rejectionLoopNudgeThreshold) + b.kv("max_feedback_issues", cfg.orchestration.maxFeedbackIssues) + b.kv("repo_map_inject_top_k", cfg.orchestration.repoMapInjectTopK) + b.kv("repo_map_files_per_dir", cfg.orchestration.repoMapFilesPerDir) + b.kv("docs_catalog_max", cfg.orchestration.docsCatalogMax) + b.kv("max_clarification_rounds", cfg.orchestration.maxClarificationRounds) + b.kv("review_block_min_confidence", cfg.orchestration.reviewBlockMinConfidence) + b.kv("review_block_retry_cap", cfg.orchestration.reviewBlockRetryCap) + b.kv("default_max_refinement", cfg.orchestration.defaultMaxRefinement) + b.kv("recovery_route_budget", cfg.orchestration.recoveryRouteBudget) + b.kv("intent_route_budget", cfg.orchestration.intentRouteBudget) + + b.section("sampling") + b.kv("temperature", cfg.sampling.temperature) + b.kv("top_p", cfg.sampling.topP) + cfg.sampling.topK?.let { b.kv("top_k", it) } + cfg.sampling.minP?.let { b.kv("min_p", it) } + cfg.sampling.repeatPenalty?.let { b.kv("repeat_penalty", it) } b.section("personalization") b.kv("enabled", cfg.personalization.enabled) diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/ToolReceipt.kt b/core/events/src/main/kotlin/com/correx/core/events/events/ToolReceipt.kt index 35698795..42dfe1ad 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/events/ToolReceipt.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/events/ToolReceipt.kt @@ -19,4 +19,8 @@ data class ToolReceipt( val tier: Tier, val timestamp: Instant, val diff: String? = null, + // Artifact-store hash of the FULL tool output when it was truncated for model context (the + // outputSummary above is a bounded preview). Null when nothing was truncated. Lets an agent + // retrieve everything via the tool_output tool without bloating the event log with raw output. + val fullOutputHash: String? = null, ) diff --git a/core/inference/src/main/kotlin/com/correx/core/inference/GenerationConfig.kt b/core/inference/src/main/kotlin/com/correx/core/inference/GenerationConfig.kt index f862be6a..f9fe6c78 100644 --- a/core/inference/src/main/kotlin/com/correx/core/inference/GenerationConfig.kt +++ b/core/inference/src/main/kotlin/com/correx/core/inference/GenerationConfig.kt @@ -13,4 +13,9 @@ data class GenerationConfig( val maxTokens: Int, val stopSequences: List = emptyList(), val seed: Long? = null, // null = non-deterministic; set for replay + // Sampling knobs. null = omit from the request so the provider/model keeps its own default, + // preserving prior behavior. Serialized only when set (top_k / min_p / repeat_penalty). + val topK: Int? = null, + val minP: Double? = null, + val repeatPenalty: Double? = null, ) diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt index b9cf320d..fb80df81 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/DefaultSessionOrchestrator.kt @@ -57,18 +57,14 @@ import java.util.concurrent.atomic.* ) private val log = LoggerFactory.getLogger(DefaultSessionOrchestrator::class.java) -// Fallback refinement-loop cap when a stage does not declare maxRetries. -private const val DEFAULT_MAX_REFINEMENT = 3 - // Own, small budget for the retry-agency recovery route: how many times a single stage may be // routed to a recovery stage before the run fails terminally. Deliberately smaller than the // per-gate retry budget — recovery is a purpose-built repair-then-reverify loop, not open-ended. -private const val RECOVERY_ROUTE_BUDGET = 2 +// RECOVERY_ROUTE_BUDGET / INTENT_ROUTE_BUDGET now in OrchestrationTuning. // Tier-2 escalation budget: how many NO-PROGRESS routes the arbiter (recovery-stage-as-intent-holder) // gets after the tier-1 owner loop exhausts, before the run fails terminally. Independent of the owner // budget (keyed with INTENT_BUDGET_SUFFIX) so escalation is a genuine second chance, not a shared pool. -private const val INTENT_ROUTE_BUDGET = 2 // Transition ids for the failure-ticket loop. A ticket routes the failing gate's control to the // repair owner (TICKET_ROUTE); the owner, once done, hands control straight back to the gate that @@ -109,7 +105,8 @@ class DefaultSessionOrchestrator( repoKnowledgeRetriever: RepoKnowledgeRetriever? = null, readyTaskCounter: ReadyTaskCounter? = null, taskClaimCoordinator: TaskClaimCoordinator? = null, -) : SessionOrchestrator(repositories, engines, artifactStore, decisionJournalRepository, artifactKindRegistry = artifactKindRegistry, repoKnowledgeRetriever = repoKnowledgeRetriever, readyTaskCounter = readyTaskCounter, taskClaimCoordinator = taskClaimCoordinator), ApprovalGateway { + tuning: OrchestrationTuning = OrchestrationTuning(), +) : SessionOrchestrator(repositories, engines, artifactStore, decisionJournalRepository, artifactKindRegistry = artifactKindRegistry, repoKnowledgeRetriever = repoKnowledgeRetriever, readyTaskCounter = readyTaskCounter, taskClaimCoordinator = taskClaimCoordinator, tuning = tuning), ApprovalGateway { override val tokenizer: Tokenizer? = tokenizer override val cancellations: ConcurrentHashMap = ConcurrentHashMap() @@ -340,10 +337,10 @@ class DefaultSessionOrchestrator( // (target, budgetKey, budget, escalated). Prefer the owner tier while it has budget; escalate to // the arbiter tier once the owner loop is spent; null = no tier available now. val route = when { - owner != null && !budgetExhausted(state, ownerKey, fingerprint, RECOVERY_ROUTE_BUDGET) -> - RouteTier(owner, ownerKey, RECOVERY_ROUTE_BUDGET, escalated = false) - arbiter != null && !budgetExhausted(state, intentKey, fingerprint, INTENT_ROUTE_BUDGET) -> - RouteTier(arbiter, intentKey, INTENT_ROUTE_BUDGET, escalated = true) + owner != null && !budgetExhausted(state, ownerKey, fingerprint, tuning.recoveryRouteBudget) -> + RouteTier(owner, ownerKey, tuning.recoveryRouteBudget, escalated = false) + arbiter != null && !budgetExhausted(state, intentKey, fingerprint, tuning.intentRouteBudget) -> + RouteTier(arbiter, intentKey, tuning.intentRouteBudget, escalated = true) else -> null } if (route == null) { @@ -621,7 +618,7 @@ class DefaultSessionOrchestrator( // terminal failure instead of looping forever. if (isBackEdge(repositories.eventStore.read(ctx.sessionId), nextStageId)) { val cycleKey = "${ctx.currentStageId.value}->${nextStageId.value}" - val maxIterations = ctx.graph.stages[nextStageId]?.maxRetries ?: DEFAULT_MAX_REFINEMENT + val maxIterations = ctx.graph.stages[nextStageId]?.maxRetries ?: tuning.defaultMaxRefinement val iteration = (orchestrationRepository.getState(ctx.sessionId).refinementIterations[cycleKey] ?: 0) + 1 emit(ctx.sessionId, RefinementIterationEvent(ctx.sessionId, cycleKey, iteration, maxIterations)) if (iteration > maxIterations) { @@ -661,8 +658,11 @@ class DefaultSessionOrchestrator( .map { it.requestId } .toSet() stageRequestIds.isNotEmpty() && events.any { - (it.payload as? ApprovalDecisionResolvedEvent) - ?.requestId in stageRequestIds + val decision = it.payload as? ApprovalDecisionResolvedEvent + // A REJECTED decision must NOT satisfy the gate on retry/resume, else the stage + // runs unapproved. Only APPROVED/AUTO_APPROVED count as a prior approval. + decision?.requestId in stageRequestIds && + decision?.outcome != ApprovalOutcome.REJECTED } } if (!alreadyApproved) { @@ -696,7 +696,7 @@ class DefaultSessionOrchestrator( compactionService?.let { svc -> val journalState = decisionJournalRepository.getJournal(ctx.sessionId) val journalText = DecisionJournalRenderer().render(journalState) - val tokenEstimate = journalText.length / 4 + val tokenEstimate = estimateTokens(journalText) svc.compactIfNeeded( sessionId = ctx.sessionId, state = journalState, diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt new file mode 100644 index 00000000..891dab48 --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/OrchestrationTuning.kt @@ -0,0 +1,44 @@ +package com.correx.core.kernel.orchestration + +/** + * Operator-tunable orchestration loop/threshold/budget knobs, loaded once at server startup from + * the `[orchestration]` config section and threaded into the orchestrator. Defaults equal the + * former hardcoded constants, so an absent config section reproduces prior behavior exactly. + * + * These are the knobs an operator reaches for when a *local* model misbehaves (thrashes a read + * loop, re-asks the same clarification, gets trapped by a stubborn reviewer). Pure output-shaping + * truncation caps (summary/evidence length limits) are intentionally left as constants — they don't + * change orchestration behavior, only display width. + * + * ponytail: startup-load, not hot-reload — orchestrator holds this by value. If per-session + * live-tuning is ever wanted, pass a `() -> OrchestrationTuning` supplier like the journal-compaction + * threshold does. + */ +data class OrchestrationTuning( + /** Max inference+tool rounds in a single stage's ReAct loop before it's forced to conclude. */ + val maxToolRounds: Int = 30, + /** Consecutive read-only rounds after which the model is nudged to write. */ + val readLoopNudgeThreshold: Int = 3, + /** Consecutive rejected-tool rounds after which the model is nudged off the blocked path. */ + val rejectionLoopNudgeThreshold: Int = 3, + /** Max validation issues surfaced back to the model as feedback. */ + val maxFeedbackIssues: Int = 3, + /** Top-K repo-map hits injected into a stage's context. */ + val repoMapInjectTopK: Int = 30, + /** Files listed per directory in the what-exists repo map. */ + val repoMapFilesPerDir: Int = 8, + /** Max docs listed in the always-on "docs available" catalog. */ + val docsCatalogMax: Int = 20, + /** Max times a single stage may re-ask clarification before it must proceed. */ + val maxClarificationRounds: Int = 3, + /** Minimum reviewer confidence for a correctness finding to block a stage. */ + val reviewBlockMinConfidence: Double = 0.7, + /** Pathological backstop: max review-driven retries before the stage is let through. */ + val reviewBlockRetryCap: Int = 20, + /** Max review→refine cycles for a stage (freestyle default refinement budget). */ + val defaultMaxRefinement: Int = 3, + /** Budget for routing a failed write-less stage to a recovery stage. */ + val recoveryRouteBudget: Int = 2, + /** Budget for tier-2 intent-holder arbiter re-routing. */ + val intentRouteBudget: Int = 2, +) diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt index b5d516a2..0f881618 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt @@ -181,14 +181,13 @@ import kotlin.coroutines.cancellation.CancellationException // re-investigating (list_dir/read/rebuild) before it reaches the write, and got bounced out before // acting on a correct diagnosis. The real runaway guards are the read-loop and rejection-loop // nudges (both fire after 3 unproductive rounds), so this ceiling only needs to be generous enough -// that productive stages aren't cut off mid-work. -private const val MAX_TOOL_ROUNDS = 30 +// that productive stages aren't cut off mid-work. Value now lives in OrchestrationTuning.maxToolRounds. // Consecutive read-only tool rounds (no file_write/file_edit) that still owe a file_written // artifact before we force the write nudge. A model that keeps calling read tools every round // trips neither the prose nudge nor the stage_complete nudge, so without this it silently burns // every round reading and never writes (2026-07-05: define_types read-looped 40 turns → failed). -private const val READ_LOOP_NUDGE_THRESHOLD = 3 +// Value now lives in OrchestrationTuning.readLoopNudgeThreshold. // Consecutive rounds in which EVERY tool call was rejected/denied (plane-2 BLOCKED or a stage/policy // ERROR) with nothing succeeding. Distinct from the read-loop breaker, which only fires for stages @@ -196,22 +195,44 @@ private const val READ_LOOP_NUDGE_THRESHOLD = 3 // rejected path (e.g. list/read of a not-yet-created dir) would otherwise thrash to MAX_TOOL_ROUNDS // with no progress. After this many all-rejected rounds we force the model to stop retrying and // produce its output; a single successful tool call resets the counter. -private const val REJECTION_LOOP_NUDGE_THRESHOLD = 3 +// Value now lives in OrchestrationTuning.rejectionLoopNudgeThreshold. private val WRITE_TOOL_NAMES = setOf("file_write", "file_edit") -private const val MAX_FEEDBACK_ISSUES = 3 private const val STAGE_COMPLETE_TOOL = "stage_complete" private const val EMIT_ARTIFACT_TOOL = "emit_artifact" private const val SCOPE_PROPOSAL_TOOL = "propose_scope" private const val READ_BEFORE_WRITE_CODE = "READ_BEFORE_WRITE" private const val REFERENCE_EXISTS_CODE = "REFERENCE_EXISTS" private const val OUTPUT_SUMMARY_LIMIT = 500 -private const val REPO_MAP_INJECT_TOP_K = 30 -private const val REPO_MAP_FILES_PER_DIR = 8 + +// Global floor on tool-result text entering model context, applied beneath each tool's own +// outputCompressor. A tool without a compressor (or one whose output survives compression) can still +// flood a small model's window, so any Success body over this many chars is shown head+tail with a +// marker and the FULL raw output spilled to the artifact store for retrieval via tool_output. +// ponytail: fixed constants; lift to OrchestrationTuning if operators need to tune per-deployment. +private const val TOOL_RESULT_MAX_CHARS = 8_000 +private const val TOOL_RESULT_HEAD_LINES = 60 +private const val TOOL_RESULT_TAIL_LINES = 60 +private const val TOOL_OUTPUT_TOOL = "tool_output" + +/** + * Frame an over-cap tool output as `header` + head lines + a truncation marker (naming the + * [tool_output] ref that retrieves the full text) + tail lines. Head and tail are each char-capped + * so a single pathological long line can't defeat the line-count bound. Pure — the caller spills the + * full output and supplies its [ref]. + */ +internal fun frameTruncatedToolResult(header: String, compressed: String, ref: String): String { + val lines = compressed.split("\n") + val head = lines.take(TOOL_RESULT_HEAD_LINES).joinToString("\n").take(TOOL_RESULT_MAX_CHARS / 2) + val tail = lines.takeLast(TOOL_RESULT_TAIL_LINES).joinToString("\n").takeLast(TOOL_RESULT_MAX_CHARS / 2) + val marker = "… output truncated (${lines.size} lines); " + + "call $TOOL_OUTPUT_TOOL(ref=\"$ref\") for the full output …" + return "$header\n$head\n$marker\n$tail" +} // Read-on-demand doc catalog: the top-N docs (by repo-map recency score) surfaced as // `path — descriptor` so a stage learns which docs exist and can file_read one when relevant, // instead of docs being force-fed (or excluded outright). One line each, hard-capped, so it can // stay always-on without re-poisoning context (2026-07-07 doc-injection rework). -private const val DOCS_CATALOG_MAX = 20 +// Value now lives in OrchestrationTuning.docsCatalogMax. // A stage prompt must explicitly ask about documentation for .md/docs paths to enter the repo // layout listing; otherwise docs only reach context through a real retrieval hit. @@ -241,8 +262,7 @@ private val REQUIRED_SOURCE_TYPES = setOf( private const val HTTP_TIMEOUT = 408 private const val HTTP_TOO_MANY_REQUESTS = 429 -// Cap on clarification rounds per stage, so a stage that keeps re-asking eventually proceeds. -private const val MAX_CLARIFICATION_ROUNDS = 3 +// Cap on clarification rounds per stage — OrchestrationTuning.maxClarificationRounds. // Static-analysis output caps: the tail retained in the recorded event vs. the (larger) slice fed // back to the model so it can fix the failure. Tails, because the error summary sits at the end. @@ -255,8 +275,7 @@ private const val CONTRACT_EVIDENCE_CAP = 400 // every attempt (so the budget never charges, i.e. the run looks like perpetual "progress"): after // this many blocks, findings surface without blocking so a stuck reviewer can never trap a stage // forever. Set well above any legitimate budget+salvage sequence. Objective text is capped too. -private const val REVIEW_BLOCK_MIN_CONFIDENCE = 0.7 -private const val REVIEW_BLOCK_RETRY_CAP = 20 +// REVIEW_BLOCK_MIN_CONFIDENCE / REVIEW_BLOCK_RETRY_CAP now in OrchestrationTuning. private const val REVIEW_OBJECTIVE_CAP = 4_000 private const val STATIC_ANALYSIS_SUMMARY_CAP = 2_000 private const val STATIC_ANALYSIS_FEEDBACK_CAP = 6_000 @@ -287,6 +306,7 @@ abstract class SessionOrchestrator( private val repoKnowledgeRetriever: RepoKnowledgeRetriever? = null, private val readyTaskCounter: ReadyTaskCounter? = null, private val taskClaimCoordinator: TaskClaimCoordinator? = null, + protected val tuning: OrchestrationTuning = OrchestrationTuning(), ) { private val log = LoggerFactory.getLogger(this::class.java) private val eventStore: EventStore = repositories.eventStore @@ -338,6 +358,14 @@ abstract class SessionOrchestrator( * Used by DefaultSessionOrchestrator.step() to populate EvaluationContext.artifactContent. */ protected val artifactContentCache: ConcurrentHashMap = ConcurrentHashMap() + /** Drops a terminated session's cached artifact contents (the heaviest per-session state — full + * file/JSON payloads). Safe: rehydrateArtifactContentCache rebuilds it from durable events if the + * session is ever resumed. Called on WorkflowCompleted/WorkflowFailed. */ + private fun evictArtifactContentCache(sessionId: SessionId) { + val prefix = "${sessionId.value}:" + artifactContentCache.keys.removeAll { it.startsWith(prefix) } + } + /** Deterministic extraction/repair ladder for near-miss LLM artifact text (prose-wrapped JSON, * code fences, trailing commas). Pure — recomputes on replay, records no events. */ private val artifactExtractionPipeline = ArtifactExtractionPipeline() @@ -511,7 +539,7 @@ abstract class SessionOrchestrator( content = journalText, sourceType = "decisionJournal", sourceId = "decision-journal", - tokenEstimate = journalText.length / 4, + tokenEstimate = estimateTokens(journalText), role = EntryRole.SYSTEM, ), ) @@ -682,7 +710,7 @@ abstract class SessionOrchestrator( while ( inferenceResult is InferenceResult.Success && - toolRounds < MAX_TOOL_ROUNDS && + toolRounds < tuning.maxToolRounds && (inferenceResult.response.finishReason is FinishReason.ToolCall || owesFileWrite()) ) { // Content turn (no tool call) but the stage still owes a file_written artifact: the @@ -761,7 +789,7 @@ abstract class SessionOrchestrator( consecutiveReadOnlyRounds = 0 } else { consecutiveReadOnlyRounds++ - if (consecutiveReadOnlyRounds >= READ_LOOP_NUDGE_THRESHOLD) { + if (consecutiveReadOnlyRounds >= tuning.readLoopNudgeThreshold) { consecutiveReadOnlyRounds = 0 inferenceResult = pushBack(readLoopNudge, forceWriteOnly = true) continue @@ -781,7 +809,7 @@ abstract class SessionOrchestrator( toolResults.all { it.content.startsWith("BLOCKED:") || it.content.startsWith("ERROR:") } if (allRejected) { consecutiveRejectedRounds++ - if (consecutiveRejectedRounds >= REJECTION_LOOP_NUDGE_THRESHOLD) { + if (consecutiveRejectedRounds >= tuning.rejectionLoopNudgeThreshold) { consecutiveRejectedRounds = 0 inferenceResult = pushBack( "STOP. Your last tool calls were all rejected and retrying the same paths will " + @@ -1098,6 +1126,38 @@ abstract class SessionOrchestrator( "\nAccepted parameters for '${it.name}': ${it.parametersSchema}" }.orEmpty() + private data class RenderedToolResult(val content: String, val fullOutputHash: String?) + + /** + * Render a tool result into the consistently-framed, bounded text the model sees. Success output + * is run through the tool's [ToolOutputCompressor], then framed with a uniform `[tool exit=N]` + * header. If it still exceeds [TOOL_RESULT_MAX_CHARS] the FULL raw output is spilled to the + * artifact store (CAS — durable, its hash recorded on the ToolReceipt in the event log) and only a + * head+tail preview is shown, with a marker telling the model to call `tool_output(ref=…)` for the + * rest. Failures keep their `ERROR:`/`FATAL:` sentinels unchanged — the all-rejected loop breaker + * keys on those prefixes. + */ + private suspend fun renderToolResult(toolName: String, tool: Tool?, result: ToolResult): RenderedToolResult = + when (result) { + is ToolResult.Failure -> RenderedToolResult( + if (!result.recoverable) "FATAL: ${result.reason}" else "ERROR: ${result.reason}${toolArgsHint(tool)}", + null, + ) + is ToolResult.Success -> { + val compressed = tool?.outputCompressor?.compress(result.output, ToolOutputContext(result.exitCode)) + ?: result.output + val header = "[$toolName exit=${result.exitCode}]" + if (compressed.length <= TOOL_RESULT_MAX_CHARS) { + RenderedToolResult("$header\n$compressed", null) + } else { + // Spill the full RAW output (most complete) so retrieval returns everything, not the + // already-compressed preview the model saw. + val ref = artifactStore.put(result.output.toByteArray()).value + RenderedToolResult(frameTruncatedToolResult(header, compressed, ref), ref) + } + } + } + private suspend fun dispatchToolCalls( sessionId: SessionId, stageId: StageId, @@ -1275,7 +1335,9 @@ abstract class SessionOrchestrator( mode = approvalMode, ) val requestId = ApprovalRequestId(UUID.randomUUID().toString()) - val toolPreview = computeToolPreview(toolCall.function.name, parameters) + val toolPreview = computeToolPreview( + toolCall.function.name, parameters, effectives.policy?.workspaceRoot, + ) val domainRequest = DomainApprovalRequest( id = requestId, tier = tier, @@ -1405,6 +1467,9 @@ abstract class SessionOrchestrator( ?.let { request.copy(grantedPaths = grantedOutside + it.toString()) } ?: request val result = executor.execute(executedRequest) + // Frame + bound the result once; on truncation this spills the full output to CAS and + // returns its hash, which we record on the receipt so the event log points at the full text. + val rendered = renderToolResult(toolCall.function.name, tool, result) // Store ProcessResult artifact for every shell execution outcome for (slot in processResultSlots) { @@ -1447,7 +1512,7 @@ abstract class SessionOrchestrator( // artifact carries the diff as evidence of the change. recordToolExecution( sessionId, stageId, toolCall, invocationId, tier, result, - tool as? FileAffectingTool, request, fileWrittenSlots, + tool as? FileAffectingTool, request, fileWrittenSlots, rendered.fullOutputHash, ) val sourceId = toolCall.id ?: invocationId.value @@ -1461,22 +1526,13 @@ abstract class SessionOrchestrator( role = EntryRole.ASSISTANT, reasoning = toolCallReasoning, ) - val resultContent = when (result) { - is ToolResult.Success -> - tool?.outputCompressor?.compress(result.output, ToolOutputContext(result.exitCode)) - ?: result.output - is ToolResult.Failure -> { - if (!result.recoverable) "FATAL: ${result.reason}" - else "ERROR: ${result.reason}${toolArgsHint(tool)}" - } - } val resultEntry = ContextEntry( id = ContextEntryId(UUID.randomUUID().toString()), layer = ContextLayer.L2, sourceType = "toolResult", sourceId = sourceId, - content = resultContent, - tokenEstimate = estimateTokens(resultContent), + content = rendered.content, + tokenEstimate = estimateTokens(rendered.content), role = EntryRole.TOOL, ) val steeringEntry = approvalNote?.takeIf { it.isNotBlank() }?.let { @@ -1764,7 +1820,7 @@ abstract class SessionOrchestrator( val priorRounds = eventStore.read(sessionId) .count { (it.payload as? ClarificationRequestedEvent)?.stageId == stageId } - if (priorRounds >= MAX_CLARIFICATION_ROUNDS) return false + if (priorRounds >= tuning.maxClarificationRounds) return false val requestId = ClarificationRequestId(UUID.randomUUID().toString()) val deferred = CompletableDeferred>() @@ -1804,7 +1860,7 @@ abstract class SessionOrchestrator( appendLine("## Repo layout (what exists — use file_read for content)") byDir.entries.sortedBy { it.key }.forEach { (dir, files) -> val names = files.map { it.substringAfterLast('/') } - val shown = names.take(REPO_MAP_FILES_PER_DIR) + val shown = names.take(tuning.repoMapFilesPerDir) val more = names.size - shown.size val suffix = if (more > 0) ", …(+$more more)" else "" appendLine("- $dir/ (${names.size}): ${shown.joinToString(", ")}$suffix") @@ -1905,7 +1961,7 @@ abstract class SessionOrchestrator( val retriever = repoKnowledgeRetriever ?: return buildRepoMapEntries(sessionId, stagePrompt) + buildDocsCatalogEntry(sessionId) - val hits = runCatching { retriever.retrieve(sessionId, stagePrompt, REPO_MAP_INJECT_TOP_K) } + val hits = runCatching { retriever.retrieve(sessionId, stagePrompt, tuning.repoMapInjectTopK) } .getOrElse { e -> if (e is CancellationException) throw e log.warn("repo-knowledge retrieval failed for stage {}: {}", stageId.value, e.message) @@ -1932,7 +1988,7 @@ abstract class SessionOrchestrator( val docs = map.entries .filter { isDocPath(it.path) } .sortedByDescending { it.score } - .take(DOCS_CATALOG_MAX) + .take(tuning.docsCatalogMax) if (docs.isEmpty()) return emptyList() val content = buildString { appendLine("## Docs available (file_read to open — do not assume contents)") @@ -2072,9 +2128,14 @@ abstract class SessionOrchestrator( ) { return@forEach } - emit(sessionId, ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1)) - emit(sessionId, ArtifactValidatingEvent(slot.name, sessionId, stageId)) - emit(sessionId, ArtifactValidatedEvent(slot.name, sessionId, stageId)) + emitAll( + sessionId, + listOf( + ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1), + ArtifactValidatingEvent(slot.name, sessionId, stageId), + ArtifactValidatedEvent(slot.name, sessionId, stageId), + ), + ) } } @@ -2088,6 +2149,7 @@ abstract class SessionOrchestrator( fileTool: FileAffectingTool?, request: ToolRequest, fileWrittenSlots: List, + fullOutputHash: String? = null, ) { // Invariant #5: every tool side effect is captured. This is the single authoritative // ToolExecutionCompleted/Failed record and the source the read-before-write gate replays. @@ -2126,6 +2188,7 @@ abstract class SessionOrchestrator( tier = tier, timestamp = Clock.System.now(), diff = diff, + fullOutputHash = fullOutputHash, ), ), ) @@ -2196,9 +2259,14 @@ abstract class SessionOrchestrator( ) return@forEach } - emit(sessionId, ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1)) - emit(sessionId, ArtifactValidatingEvent(slot.name, sessionId, stageId)) - emit(sessionId, ArtifactValidatedEvent(slot.name, sessionId, stageId)) + emitAll( + sessionId, + listOf( + ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1), + ArtifactValidatingEvent(slot.name, sessionId, stageId), + ArtifactValidatedEvent(slot.name, sessionId, stageId), + ), + ) } } @@ -2767,11 +2835,11 @@ abstract class SessionOrchestrator( .mapNotNull { it.payload as? ReviewFindingsRaisedEvent } .count { it.stageId == stageId && it.blocked } val blockingFinding = outcome.findings.firstOrNull { - it.correctness && it.confidence >= REVIEW_BLOCK_MIN_CONFIDENCE + it.correctness && it.confidence >= tuning.reviewBlockMinConfidence } val shouldBlock = outcome.verdict == ReviewVerdict.FAIL && blockingFinding != null && - priorBlocks < REVIEW_BLOCK_RETRY_CAP + priorBlocks < tuning.reviewBlockRetryCap emit(sessionId, ReviewFindingsRaisedEvent(sessionId, stageId, outcome.verdict, outcome.findings, blocked = shouldBlock)) @@ -2808,9 +2876,14 @@ abstract class SessionOrchestrator( // Emit artifact lifecycle events for process_result slots on failure branches. // Content was already stored in CAS + cache by dispatchToolCalls for every outcome. stageConfig.produces.filter { it.kind.id == "process_result" }.forEach { slot -> - emit(sessionId, ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1)) - emit(sessionId, ArtifactValidatingEvent(slot.name, sessionId, stageId)) - emit(sessionId, ArtifactValidatedEvent(slot.name, sessionId, stageId)) + emitAll( + sessionId, + listOf( + ArtifactCreatedEvent(slot.name, sessionId, stageId, schemaVersion = 1), + ArtifactValidatingEvent(slot.name, sessionId, stageId), + ArtifactValidatedEvent(slot.name, sessionId, stageId), + ), + ) } } @@ -2859,7 +2932,7 @@ abstract class SessionOrchestrator( return if (errors.isEmpty()) { "validation failed" } else { - "validation failed: " + errors.take(MAX_FEEDBACK_ISSUES).joinToString("; ") + "validation failed: " + errors.take(tuning.maxFeedbackIssues).joinToString("; ") } } @@ -2904,11 +2977,14 @@ abstract class SessionOrchestrator( tools = if (!withTools) { emptyList() } else { + // Read the log ONCE for the read-only check instead of once per tool inside the filter + // (the flag is tool-independent) — this filter runs per tool per inference round. + val readOnlyMode = isReadOnlyMode(sessionId) stageConfig.effectiveAllowedTools .mapNotNull { effectives.registry?.resolve(it) } .filter { tool -> // ponytail: filter write tools while read-before-write block is active; restored once a read completes - !isReadOnlyMode(sessionId) || ToolCapability.FILE_WRITE !in tool.requiredCapabilities + !readOnlyMode || ToolCapability.FILE_WRITE !in tool.requiredCapabilities } .filter { tool -> // Read-loop break: keep only write tools (drop file_read/list_dir/shell) so @@ -3071,6 +3147,7 @@ abstract class SessionOrchestrator( correlateCritiqueOutcomes(sessionId) emit(sessionId, WorkflowCompletedEvent(sessionId, terminalStageId, stageCount, workflowId)) cancellations.remove(sessionId) + evictArtifactContentCache(sessionId) return WorkflowResult.Completed(sessionId, terminalStageId) } @@ -3114,6 +3191,7 @@ abstract class SessionOrchestrator( ) } cancellations.remove(sessionId) + evictArtifactContentCache(sessionId) return WorkflowResult.Failed(sessionId, reason, retryExhausted) } @@ -3187,6 +3265,29 @@ abstract class SessionOrchestrator( ) } + /** Appends [payloads] as one transaction (one flow-publish pass) for events that belong together. */ + internal suspend fun emitAll(sessionId: SessionId, payloads: List) { + if (payloads.size <= 1) { + payloads.firstOrNull()?.let { emit(sessionId, it) } + return + } + eventStore.appendAll( + payloads.map { payload -> + NewEvent( + metadata = EventMetadata( + eventId = EventId(UUID.randomUUID().toString()), + sessionId = sessionId, + timestamp = Clock.System.now(), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + payload = payload, + ) + }, + ) + } + // --- token estimation --- protected open suspend fun estimateTokens(content: String): Int { @@ -3382,10 +3483,14 @@ abstract class SessionOrchestrator( * This function performs blocking I/O (file reads) and must be called from a suspend context * that will dispatch it on [Dispatchers.IO]. */ -private suspend fun computeToolPreview(toolName: String, parameters: Map): String? { +private suspend fun computeToolPreview( + toolName: String, + parameters: Map, + workspaceRoot: java.nio.file.Path?, +): String? { if (toolName == "shell") return shellCommandPreview(parameters) if (toolName == "task_decompose") return renderDecomposePreview(parameters) - if (toolName == "file_edit") return computeFileEditPreview(parameters) + if (toolName == "file_edit") return computeFileEditPreview(parameters, workspaceRoot) if (toolName != "file_write") return null val path = parameters["path"] as? String ?: return null // file_write no longer carries an `operation` param (delete was split into file_delete), so the @@ -3393,18 +3498,23 @@ private suspend fun computeToolPreview(toolName: String, parameters: Map): String? { +private suspend fun computeFileEditPreview( + parameters: Map, + workspaceRoot: java.nio.file.Path?, +): String? { val path = parameters["path"] as? String ?: return null val operation = parameters["operation"] as? String ?: return null - val existingContent = readFileIfExists(path) ?: return null + val existingContent = readFileIfExists(path, workspaceRoot) ?: return null val proposedContent = when (operation) { "append" -> existingContent + (parameters["content"] as? String ?: return null) diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/FrameTruncatedToolResultTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/FrameTruncatedToolResultTest.kt new file mode 100644 index 00000000..af1c182f --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/FrameTruncatedToolResultTest.kt @@ -0,0 +1,30 @@ +package com.correx.core.kernel.orchestration + +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class FrameTruncatedToolResultTest { + + @Test + fun `keeps header, head, tail and a marker naming the retrieval ref`() { + val body = (1..300).joinToString("\n") { "line-$it" } + val out = frameTruncatedToolResult("[grep exit=0]", body, "abc123") + + assertTrue(out.startsWith("[grep exit=0]"), out) + assertTrue(out.contains("line-1\n"), "head lines kept") + assertTrue(out.contains("line-300"), "tail lines kept") + assertTrue(out.contains("output truncated (300 lines)"), out) + assertTrue(out.contains("tool_output(ref=\"abc123\")"), out) + // Middle is dropped — a line well inside the elided span must be absent. + assertTrue(!out.contains("line-150"), "middle elided") + } + + @Test + fun `char-caps a single pathological long line so the bound is not defeated`() { + val giant = "x".repeat(1_000_000) + val out = frameTruncatedToolResult("[shell exit=0]", giant, "ref") + // One line means head==tail==giant; each is char-capped, so output can't be ~2MB. + assertTrue(out.length < 20_000, "length was ${out.length}") + assertTrue(out.contains("tool_output(ref=\"ref\")"), out) + } +} diff --git a/core/talkie/src/main/kotlin/com/correx/core/talkie/IdeaReader.kt b/core/talkie/src/main/kotlin/com/correx/core/talkie/IdeaReader.kt index d24755c2..5ab0411a 100644 --- a/core/talkie/src/main/kotlin/com/correx/core/talkie/IdeaReader.kt +++ b/core/talkie/src/main/kotlin/com/correx/core/talkie/IdeaReader.kt @@ -1,46 +1,61 @@ package com.correx.core.talkie +import com.correx.core.events.events.EventPayload import com.correx.core.events.events.IdeaCapturedEvent import com.correx.core.events.events.IdeaDiscardedEvent import com.correx.core.events.events.IdeaPromotedEvent import com.correx.core.events.stores.EventStore import com.correx.core.events.types.SessionId import com.correx.core.talkie.model.Idea +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.launch +import java.util.concurrent.ConcurrentHashMap /** - * Rebuilds the operator's idea board from the event log. Cross-session by design: it folds over - * [EventStore.allEvents] rather than a single session's replay, so an idea captured in one session - * shows on the board (and feeds the router) in every session. A captured idea is dropped once a - * matching [IdeaDiscardedEvent] (or [IdeaPromotedEvent]) tombstones it (invariant #1 — the capture - * stays in the log). + * Holds the operator's idea board in memory. Cross-session by design: it folds over the whole event + * log (not a single session's replay), so an idea captured in one session shows on the board (and + * feeds the router) in every session. A captured idea is dropped once a matching [IdeaDiscardedEvent] + * (or [IdeaPromotedEvent]) tombstones it (invariant #1 — the capture stays in the log). + * + * The board is folded ONCE at construction from [EventStore.allEvents] (a full, eager table load) and + * then kept current incrementally via [EventStore.subscribeAll]. This replaces the previous + * fold-the-entire-log-per-call design, which ran a full deserializing scan on every CHAT context + * build. The fold is idempotent (map put / set add), so the seed⇄live overlap needs no dedupe. */ -class IdeaReader(private val eventStore: EventStore) { +class IdeaReader( + eventStore: EventStore, + scope: CoroutineScope = CoroutineScope(Dispatchers.Default + SupervisorJob()), +) { + // id -> its capture event (source for text/sessionId); tombstoned = discarded/promoted ids. + private val captures = ConcurrentHashMap() + private val tombstoned: MutableSet = ConcurrentHashMap.newKeySet() - /** Active ideas (captured, not later discarded or promoted) across all sessions, newest first. */ - fun activeIdeas(): List { - val tombstoned = mutableSetOf() - val captured = mutableListOf() - eventStore.allEvents().forEach { stored -> - when (val payload = stored.payload) { - is IdeaDiscardedEvent -> tombstoned += payload.ideaId - is IdeaPromotedEvent -> tombstoned += payload.ideaId - is IdeaCapturedEvent -> captured += Idea(payload.ideaId, payload.text, payload.timestampMs) - else -> Unit - } - } - return captured.filterNot { it.id in tombstoned }.sortedByDescending { it.capturedAtMs } + init { + eventStore.allEvents().forEach { apply(it.payload) } + scope.launch { eventStore.subscribeAll().collect { apply(it.payload) } } } + private fun apply(payload: EventPayload) { + when (payload) { + is IdeaDiscardedEvent -> tombstoned += payload.ideaId + is IdeaPromotedEvent -> tombstoned += payload.ideaId + is IdeaCapturedEvent -> captures[payload.ideaId] = payload + else -> Unit + } + } + + /** Active ideas (captured, not later discarded or promoted) across all sessions, newest first. */ + fun activeIdeas(): List = + captures.values + .filterNot { it.ideaId in tombstoned } + .map { Idea(it.ideaId, it.text, it.timestampMs) } + .sortedByDescending { it.capturedAtMs } + /** The session that captured [ideaId], so a tombstone lands alongside its capture. */ - fun sessionOf(ideaId: String): SessionId? = - capturedOf(ideaId)?.sessionId + fun sessionOf(ideaId: String): SessionId? = captures[ideaId]?.sessionId /** The captured text of [ideaId] (so a promotion can write it into the project profile), or null. */ - fun textOf(ideaId: String): String? = - capturedOf(ideaId)?.text - - private fun capturedOf(ideaId: String): IdeaCapturedEvent? = - eventStore.allEvents() - .mapNotNull { it.payload as? IdeaCapturedEvent } - .firstOrNull { it.ideaId == ideaId } + fun textOf(ideaId: String): String? = captures[ideaId]?.text } diff --git a/core/transitions/src/main/kotlin/com/correx/core/transitions/graph/StageConfig.kt b/core/transitions/src/main/kotlin/com/correx/core/transitions/graph/StageConfig.kt index 6204be57..b178d1fd 100644 --- a/core/transitions/src/main/kotlin/com/correx/core/transitions/graph/StageConfig.kt +++ b/core/transitions/src/main/kotlin/com/correx/core/transitions/graph/StageConfig.kt @@ -66,6 +66,7 @@ data class StageConfig( companion object { /** Read-only tools every tool-granting stage may call regardless of its declared set. */ - val ALWAYS_AVAILABLE_READ_TOOLS: Set = setOf("file_read", "list_dir") + val ALWAYS_AVAILABLE_READ_TOOLS: Set = + setOf("file_read", "list_dir", "glob", "grep", "tool_output") } } diff --git a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/DefaultModelManager.kt b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/DefaultModelManager.kt index b5013b49..5d978899 100644 --- a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/DefaultModelManager.kt +++ b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/DefaultModelManager.kt @@ -151,10 +151,12 @@ class DefaultModelManager( currentDescriptor = null } - @Suppress("UnusedParameter") private suspend fun waitForHealthy(process: LlamaProcess): Boolean { val endTime = Clock.System.now().toEpochMilliseconds() + healthTimeoutMs while (Clock.System.now().toEpochMilliseconds() < endTime) { + // A crashed process (bad --model, port clash) never gets healthy; polling the full + // timeout is pure dead time. Bail as soon as it's gone. + if (!process.isAlive) return false try { val response = httpClient.get("http://$host:$port/health").body() if (response.contains("\"status\":\"healthy\"") || response.contains("ok")) { diff --git a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppApiModels.kt b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppApiModels.kt index e71a579e..48d6c846 100644 --- a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppApiModels.kt +++ b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppApiModels.kt @@ -3,6 +3,7 @@ package com.correx.infrastructure.inference.llama.cpp import com.correx.core.inference.ChatMessage import com.correx.core.inference.ToolCallRequest import com.correx.core.inference.ToolDefinition +import kotlinx.serialization.EncodeDefault import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable @@ -15,6 +16,12 @@ data class ChatCompletionRequest( @SerialName("max_tokens") val maxTokens: Int, @SerialName("stop") val stopSequences: List = emptyList(), val seed: Long? = null, + // EncodeDefault.NEVER overrides the class-level encodeDefaults=true so an unset (null) sampling + // knob is omitted from the JSON entirely, letting the model keep its own default (rather than + // sending "top_k": null, which llama.cpp may reject or misread). + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("top_k") val topK: Int? = null, + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("min_p") val minP: Double? = null, + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("repeat_penalty") val repeatPenalty: Double? = null, val stream: Boolean = false, val grammar: String? = null, val tools: List? = null, diff --git a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppInferenceProvider.kt b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppInferenceProvider.kt index f0712812..1dbeb9ae 100644 --- a/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppInferenceProvider.kt +++ b/infrastructure/inference/llama_cpp/src/main/kotlin/com/correx/infrastructure/inference/llama/cpp/LlamaCppInferenceProvider.kt @@ -42,22 +42,14 @@ private fun defaultHttpClient(): HttpClient = HttpClient(CIO) { Json { ignoreUnknownKeys = true isLenient = true - encodeDefaults = false + explicitNulls = false + encodeDefaults = true }, ) } install(HttpTimeout) { requestTimeoutMillis = DEFAULT_REQUEST_TIMEOUT_MS } - install(ContentNegotiation) { - json( - Json { - ignoreUnknownKeys = true - explicitNulls = false - encodeDefaults = true - }, - ) - } } private val json = Json { ignoreUnknownKeys = true @@ -178,6 +170,9 @@ class LlamaCppInferenceProvider( maxTokens = request.generationConfig.maxTokens, stopSequences = request.generationConfig.stopSequences, seed = request.generationConfig.seed, + topK = request.generationConfig.topK, + minP = request.generationConfig.minP, + repeatPenalty = request.generationConfig.repeatPenalty, stream = false, grammar = grammar, tools = tools, @@ -199,7 +194,9 @@ class LlamaCppInferenceProvider( log.debug("got response from llm: {}", response) - val message = response.choices.first().message + val choice = response.choices.firstOrNull() + ?: error("llama-server returned no choices in the completion response") + val message = choice.message // Some local models emit the tool call as a JSON blob in `content` instead of the native // tool_calls array; salvage it so the orchestrator gets a real call rather than treating // the blob as a (failing) artifact and retry-looping to exhaustion. @@ -207,7 +204,7 @@ class LlamaCppInferenceProvider( val toolCalls = message.toolCalls.ifEmpty { salvaged } val finishReason = when { toolCalls.isNotEmpty() -> FinishReason.ToolCall - response.choices.first().finishReason.lowercase() == "length" -> FinishReason.Length + choice.finishReason.lowercase() == "length" -> FinishReason.Length else -> FinishReason.Stop } diff --git a/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/SamplingRequestSerializationTest.kt b/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/SamplingRequestSerializationTest.kt new file mode 100644 index 00000000..2a5aead1 --- /dev/null +++ b/infrastructure/inference/llama_cpp/src/test/kotlin/com/correx/infrastructure/inference/llama/cpp/SamplingRequestSerializationTest.kt @@ -0,0 +1,37 @@ +package com.correx.infrastructure.inference.llama.cpp + +import kotlinx.serialization.encodeToString +import kotlinx.serialization.json.Json +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +// Guards the EncodeDefault.NEVER behavior on the sampling knobs: with encodeDefaults=true (matching +// the provider's Json), an unset (null) top_k/min_p/repeat_penalty must be OMITTED from the body so +// the model keeps its own default; a set value must appear. +class SamplingRequestSerializationTest { + private val json = Json { encodeDefaults = true } + + @Test + fun `unset sampling knobs are omitted from the request body`() { + val body = ChatCompletionRequest( + model = "m", messages = emptyList(), temperature = 0.7, topP = 1.0, maxTokens = 16, + ) + val out = json.encodeToString(body) + assertFalse("top_k" in out, out) + assertFalse("min_p" in out, out) + assertFalse("repeat_penalty" in out, out) + } + + @Test + fun `set sampling knobs are serialized`() { + val body = ChatCompletionRequest( + model = "m", messages = emptyList(), temperature = 0.7, topP = 1.0, maxTokens = 16, + topK = 40, minP = 0.05, repeatPenalty = 1.1, + ) + val out = json.encodeToString(body) + assertTrue("\"top_k\":40" in out, out) + assertTrue("\"min_p\":0.05" in out, out) + assertTrue("\"repeat_penalty\":1.1" in out, out) + } +} diff --git a/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiApiModels.kt b/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiApiModels.kt index fb6cf427..9509403a 100644 --- a/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiApiModels.kt +++ b/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiApiModels.kt @@ -2,6 +2,7 @@ package com.correx.infrastructure.inference.openai import com.correx.core.inference.ToolCallRequest import com.correx.core.inference.ToolDefinition +import kotlinx.serialization.EncodeDefault import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable @@ -20,6 +21,11 @@ data class OpenAiChatCompletionRequest( @SerialName("max_tokens") val maxTokens: Int, @SerialName("stop") val stopSequences: List? = null, val seed: Long? = null, + // Non-standard OpenAI params accepted by local backends (vLLM, llama.cpp --api). EncodeDefault.NEVER + // omits them when unset so a strict endpoint never sees an unknown key unless the operator opts in. + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("top_k") val topK: Int? = null, + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("min_p") val minP: Double? = null, + @EncodeDefault(EncodeDefault.Mode.NEVER) @SerialName("repeat_penalty") val repeatPenalty: Double? = null, val stream: Boolean = false, val tools: List? = null, ) diff --git a/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiCompatInferenceProvider.kt b/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiCompatInferenceProvider.kt index 34c26d4a..3027392c 100644 --- a/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiCompatInferenceProvider.kt +++ b/infrastructure/inference/openai_compat/src/main/kotlin/com/correx/infrastructure/inference/openai/OpenAiCompatInferenceProvider.kt @@ -104,6 +104,9 @@ class OpenAiCompatInferenceProvider( maxTokens = request.generationConfig.maxTokens, stopSequences = request.generationConfig.stopSequences.ifEmpty { null }, seed = request.generationConfig.seed, + topK = request.generationConfig.topK, + minP = request.generationConfig.minP, + repeatPenalty = request.generationConfig.repeatPenalty, stream = false, tools = tools, ) diff --git a/infrastructure/persistence/build.gradle b/infrastructure/persistence/build.gradle index 14e1144c..0e0af0e6 100644 --- a/infrastructure/persistence/build.gradle +++ b/infrastructure/persistence/build.gradle @@ -11,6 +11,7 @@ dependencies { implementation(project(":core:artifacts")) implementation(project(":core:artifacts-store")) implementation "org.xerial:sqlite-jdbc" + implementation "org.slf4j:slf4j-api:2.0.16" testImplementation(testFixtures(project(":testing:contracts"))) testImplementation(project(":testing:fixtures")) testImplementation "org.junit.jupiter:junit-jupiter" diff --git a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/InMemoryEventStore.kt b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/InMemoryEventStore.kt index efdf655a..054b0c4c 100644 --- a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/InMemoryEventStore.kt +++ b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/InMemoryEventStore.kt @@ -9,9 +9,12 @@ import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.asSharedFlow +import org.slf4j.LoggerFactory import java.util.concurrent.* import java.util.concurrent.atomic.* +private val log = LoggerFactory.getLogger(InMemoryEventStore::class.java) + class InMemoryEventStore : EventStore { private val streams = ConcurrentHashMap>() private val sequences = ConcurrentHashMap() @@ -33,7 +36,7 @@ class InMemoryEventStore : EventStore { } stored = doAppend(event, stream) } - subscriptions[event.metadata.sessionId]?.tryEmit(stored) + subscriptions[event.metadata.sessionId]?.let { warnIfDropped(it.tryEmit(stored), stored) } globalFlow.emit(stored) return stored } @@ -47,7 +50,7 @@ class InMemoryEventStore : EventStore { stored = events.map { doAppend(it, stream) } } val flow = subscriptions[sessionId] - if (flow != null) stored.forEach { flow.tryEmit(it) } + if (flow != null) stored.forEach { warnIfDropped(flow.tryEmit(it), it) } stored.forEach { globalFlow.emit(it) } return stored } @@ -77,6 +80,19 @@ class InMemoryEventStore : EventStore { override fun allSessionIds(): Set = sequences.keys + /** A lagging per-session collector's buffer (extraBufferCapacity 64) is full; the event is durably + * stored but dropped from the live SharedFlow. Surface it rather than swallow the false return. */ + private fun warnIfDropped(emitted: Boolean, event: StoredEvent) { + if (!emitted) { + log.warn( + "dropped live subscription emit for session {} seq {} ({}): buffer full, collector lagging", + event.metadata.sessionId.value, + event.sequence, + event.payload::class.simpleName, + ) + } + } + private fun doAppend(event: NewEvent, stream: MutableList): StoredEvent { val sessionSeq = sequences.computeIfAbsent(event.metadata.sessionId) { AtomicLong(0) } .incrementAndGet() diff --git a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/SqliteEventStore.kt b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/SqliteEventStore.kt index 9a9167cf..7b6175ee 100644 --- a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/SqliteEventStore.kt +++ b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/SqliteEventStore.kt @@ -21,10 +21,13 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import kotlinx.datetime.Instant +import org.slf4j.LoggerFactory import java.sql.Connection import java.sql.ResultSet import java.util.concurrent.* +private val log = LoggerFactory.getLogger(SqliteEventStore::class.java) + class SqliteEventStore( private val connection: Connection, private val jsonSerializer: JsonEventSerializer = JsonEventSerializer(eventJson), @@ -72,25 +75,24 @@ class SqliteEventStore( appendMutex.withLock { artifactStore.flushBefore { withContext(Dispatchers.IO) { - stored = connection.transaction { - val existing = findByEventId(event.metadata.eventId) - check(existing == null) { "duplicate event_id: ${event.metadata.eventId}" } - val globalSeqVal = nextGlobalSequence() - val sessionSeqVal = nextSessionSequence(event.metadata.sessionId) - val s = StoredEvent( - metadata = event.metadata, - sequence = globalSeqVal, - sessionSequence = sessionSeqVal, - payload = event.payload, - ) - insert(s) - s + stored = withConnection { + connection.transaction { + val existing = findByEventId(event.metadata.eventId) + check(existing == null) { "duplicate event_id: ${event.metadata.eventId}" } + val (globalSeqVal, sessionSeqVal) = insertAssigningSequences(event) + StoredEvent( + metadata = event.metadata, + sequence = globalSeqVal, + sessionSequence = sessionSeqVal, + payload = event.payload, + ) + } } } } } val result = checkNotNull(stored) - subscriptions[event.metadata.sessionId]?.tryEmit(result) + subscriptions[event.metadata.sessionId]?.let { warnIfDropped(it.tryEmit(result), result) } globalFlow.emit(result) return result } @@ -102,32 +104,31 @@ class SqliteEventStore( appendMutex.withLock { artifactStore.flushBefore { withContext(Dispatchers.IO) { - stored = connection.transaction { - events.map { event -> - val existing = findByEventId(event.metadata.eventId) - check(existing == null) { "duplicate event_id: ${event.metadata.eventId}" } - val globalSeqVal = nextGlobalSequence() - val sessionSeqVal = nextSessionSequence(sessionId) - val s = StoredEvent( - metadata = event.metadata, - sequence = globalSeqVal, - sessionSequence = sessionSeqVal, - payload = event.payload, - ) - insert(s) - s + stored = withConnection { + connection.transaction { + events.map { event -> + val existing = findByEventId(event.metadata.eventId) + check(existing == null) { "duplicate event_id: ${event.metadata.eventId}" } + val (globalSeqVal, sessionSeqVal) = insertAssigningSequences(event) + StoredEvent( + metadata = event.metadata, + sequence = globalSeqVal, + sessionSequence = sessionSeqVal, + payload = event.payload, + ) + } } } } } } val flow = subscriptions[sessionId] - if (flow != null) stored.forEach { flow.tryEmit(it) } + if (flow != null) stored.forEach { warnIfDropped(flow.tryEmit(it), it) } stored.forEach { globalFlow.emit(it) } return stored } - override fun read(sessionId: SessionId): List = + override fun read(sessionId: SessionId): List = withConnection { connection.prepareStatement( """ SELECT * FROM events @@ -144,8 +145,9 @@ class SqliteEventStore( } } } + } - override fun readFrom(sessionId: SessionId, fromSequence: Long): List = + override fun readFrom(sessionId: SessionId, fromSequence: Long): List = withConnection { connection.prepareStatement( """ SELECT * FROM events @@ -164,8 +166,9 @@ class SqliteEventStore( } } } + } - override fun lastSequence(sessionId: SessionId): Long? = + override fun lastSequence(sessionId: SessionId): Long? = withConnection { connection.prepareStatement( "SELECT MAX(session_sequence) FROM events WHERE session_id = ?" ).use { ps -> @@ -175,6 +178,7 @@ class SqliteEventStore( else null } } + } override fun subscribe(sessionId: SessionId): Flow = subscriptions.computeIfAbsent(sessionId) { MutableSharedFlow(replay = 0, extraBufferCapacity = 64) } @@ -183,17 +187,19 @@ class SqliteEventStore( override suspend fun lastGlobalSequence(): Long = withContext(Dispatchers.IO) { - connection.prepareStatement( - "SELECT COALESCE(MAX(sequence), 0) FROM events" - ).use { ps -> - ps.executeQuery().use { rs -> - rs.next() - rs.getLong(1) + withConnection { + connection.prepareStatement( + "SELECT COALESCE(MAX(sequence), 0) FROM events" + ).use { ps -> + ps.executeQuery().use { rs -> + rs.next() + rs.getLong(1) + } } } } - override fun allEvents(): Sequence = + override fun allEvents(): Sequence = withConnection { connection.prepareStatement( """ SELECT * FROM events @@ -208,8 +214,9 @@ class SqliteEventStore( } } }.asSequence() + } - override fun allSessionIds(): Set = + override fun allSessionIds(): Set = withConnection { mutableSetOf().apply { connection.prepareStatement("SELECT DISTINCT session_id FROM events").use { ps -> ps.executeQuery().use { rs -> @@ -217,32 +224,47 @@ class SqliteEventStore( } } } + } // ---------- helpers ---------- - private fun nextGlobalSequence(): Long = - connection.prepareStatement( - "SELECT COALESCE(MAX(sequence), 0) FROM events" - ).use { ps -> - ps.executeQuery().use { rs -> - rs.next() - rs.getLong(1) + 1 - } - } + /** + * Serializes all JDBC access to the shared [connection] on a single JVM monitor. Reads run on the + * caller thread while append's transaction runs on Dispatchers.IO; the coroutine [appendMutex] only + * excludes append-vs-append, so without this a read could hit the connection mid-transaction + * (SQLite JDBC is not thread-safe → corrupted tx state). ponytail: store-wide lock, one session at a + * time is fine here; shard per-connection only if read throughput ever matters. + */ + private inline fun withConnection(block: () -> T): T = synchronized(connection) { block() } - private fun nextSessionSequence(sessionId: SessionId): Long = - connection.prepareStatement( - "SELECT COALESCE(MAX(session_sequence), 0) FROM events WHERE session_id = ?" - ).use { ps -> - ps.setString(1, sessionId.value) - ps.executeQuery().use { rs -> - rs.next() - rs.getLong(1) + 1 - } + /** + * A per-session subscription's buffer (extraBufferCapacity 64) is full and this event was dropped. + * The event is durably persisted (the drop is only on the live SharedFlow), but a lagging in-process + * collector — e.g. LiveArtifactRepository — silently diverges from the log. Surface it instead of + * swallowing the false return. ponytail: log-only; give that collector a bounded rebuild-on-lag if + * divergence ever bites in practice. + */ + private fun warnIfDropped(emitted: Boolean, event: StoredEvent) { + if (!emitted) { + log.warn( + "dropped live subscription emit for session {} seq {} ({}): buffer full, collector lagging", + event.metadata.sessionId.value, + event.sequence, + event.payload::class.simpleName, + ) } + } + /** + * Inserts [event], letting SQLite assign both the global `sequence` and the per-session + * `session_sequence` as `MAX+1` subqueries evaluated inside the INSERT itself, and RETURNs the + * assigned values. SQLite serializes writers (one write transaction at a time), so the subqueries + * are atomic against other processes on the same DB — closing the CLI+server MAX+1 race that the + * old read-then-insert had (the unique index on `sequence` is the backstop). Within one appendAll + * transaction, each INSERT sees the prior rows, so a batch increments correctly. + */ @Suppress("MagicNumber") - private fun insert(event: StoredEvent) { + private fun insertAssigningSequences(event: NewEvent): Pair = connection.prepareStatement( """ INSERT INTO events ( @@ -255,21 +277,28 @@ class SqliteEventStore( causation_id, correlation_id, payload - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES ( + ?, ?, + (SELECT COALESCE(MAX(sequence), 0) + 1 FROM events), + (SELECT COALESCE(MAX(session_sequence), 0) + 1 FROM events WHERE session_id = ?), + ?, ?, ?, ?, ? + ) + RETURNING sequence, session_sequence """.trimIndent(), ).use { ps -> ps.setString(1, event.metadata.eventId.value) ps.setString(2, event.metadata.sessionId.value) - ps.setLong(3, event.sequence) - ps.setLong(4, event.sessionSequence) - ps.setString(5, event.metadata.timestamp.toString()) - ps.setInt(6, event.metadata.schemaVersion) - ps.setString(7, event.metadata.causationId?.value) - ps.setString(8, event.metadata.correlationId?.value) - ps.setString(9, jsonSerializer.serialize(event.payload)) - ps.executeUpdate() + ps.setString(3, event.metadata.sessionId.value) + ps.setString(4, event.metadata.timestamp.toString()) + ps.setInt(5, event.metadata.schemaVersion) + ps.setString(6, event.metadata.causationId?.value) + ps.setString(7, event.metadata.correlationId?.value) + ps.setString(8, jsonSerializer.serialize(event.payload)) + ps.executeQuery().use { rs -> + rs.next() + rs.getLong("sequence") to rs.getLong("session_sequence") + } } - } private fun findByEventId(eventId: EventId): StoredEvent? = connection.prepareStatement( diff --git a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepository.kt b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepository.kt index d2d6862f..a294e52e 100644 --- a/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepository.kt +++ b/infrastructure/persistence/src/main/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepository.kt @@ -12,6 +12,7 @@ import com.correx.core.events.stores.EventStore import com.correx.core.events.types.ArtifactId import com.correx.core.events.types.SessionId import com.correx.core.events.types.StageId +import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job @@ -55,11 +56,27 @@ class LiveArtifactRepository( private fun ensureSubscribed(sessionId: SessionId) { subscriptions.computeIfAbsent(sessionId) { - scope.launch { + // Rebuild from persisted history so a cold (post-restart) or first mid-session consumer + // sees prior artifact state immediately, not empty (invariant #1). The reducer's phase + // transitions are strictly ordered, so application must stay single-threaded and in + // sequence order — hence the gate below rather than a concurrent replay. + val rebuildWatermark = CompletableDeferred() + val job = scope.launch { + // Start collecting immediately so events appended during the rebuild are captured (flow + // back-pressure buffers them), but hold each until the rebuild's high-water mark is + // known, then drop anything already folded by the rebuild (sequence <= watermark). + // Live delivery is in append order, so post-watermark events fold in order too. eventStore.subscribe(sessionId).collect { event -> - processEvent(sessionId, event) + if (event.sequence > rebuildWatermark.await()) processEvent(sessionId, event) } } + var watermark = 0L + eventStore.read(sessionId).forEach { event -> + watermark = maxOf(watermark, event.sequence) + processEvent(sessionId, event) + } + rebuildWatermark.complete(watermark) + job } } diff --git a/infrastructure/persistence/src/test/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepositoryTest.kt b/infrastructure/persistence/src/test/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepositoryTest.kt new file mode 100644 index 00000000..a78ab9b0 --- /dev/null +++ b/infrastructure/persistence/src/test/kotlin/com/correx/infrastructure/persistence/artifact/LiveArtifactRepositoryTest.kt @@ -0,0 +1,41 @@ +package com.correx.infrastructure.persistence.artifact + +import com.correx.core.artifacts.DefaultArtifactReducer +import com.correx.core.events.events.ArtifactCreatedEvent +import com.correx.core.events.events.ArtifactValidatedEvent +import com.correx.core.events.events.ArtifactValidatingEvent +import com.correx.core.events.types.ArtifactId +import com.correx.core.events.types.ArtifactLifecyclePhase +import com.correx.core.events.types.EventId +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import com.correx.infrastructure.persistence.InMemoryEventStore +import com.correx.testing.fixtures.EventFixtures +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +class LiveArtifactRepositoryTest { + + @Test + fun `cold access rebuilds artifact state from the persisted log, in order`(): Unit = runBlocking { + val store = InMemoryEventStore() + val session = SessionId("s1") + val artifact = ArtifactId("a1") + val stage = StageId("stage1") + + // Persist the full lifecycle BEFORE any repository/consumer exists — the pre-fix repo + // subscribed with replay=0 and never read() this history, so a cold consumer saw null. + store.append(EventFixtures.newEvent(EventId("e1"), session, ArtifactCreatedEvent(artifact, session, stage, 1))) + store.append(EventFixtures.newEvent(EventId("e2"), session, ArtifactValidatingEvent(artifact, session, stage))) + store.append(EventFixtures.newEvent(EventId("e3"), session, ArtifactValidatedEvent(artifact, session, stage))) + + // Fresh repository = cold consumer (mimics a restart / first mid-session access). + val repo = LiveArtifactRepository(store, DefaultArtifactReducer()) + + // Rebuilt, and folded in sequence order — VALIDATED only reachable via CREATED→VALIDATING→VALIDATED. + assertEquals(ArtifactLifecyclePhase.VALIDATED, repo.getById(session, artifact)?.phase) + assertNull(repo.getById(session, ArtifactId("does-not-exist"))) + } +} diff --git a/infrastructure/router/turbovec/src/main/kotlin/com/correx/infrastructure/router/turbovec/TurboVecL3MemoryStore.kt b/infrastructure/router/turbovec/src/main/kotlin/com/correx/infrastructure/router/turbovec/TurboVecL3MemoryStore.kt index acf3d7d8..ab986077 100644 --- a/infrastructure/router/turbovec/src/main/kotlin/com/correx/infrastructure/router/turbovec/TurboVecL3MemoryStore.kt +++ b/infrastructure/router/turbovec/src/main/kotlin/com/correx/infrastructure/router/turbovec/TurboVecL3MemoryStore.kt @@ -8,8 +8,10 @@ import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.Json +import java.io.BufferedReader import java.nio.file.Paths import kotlin.io.path.createDirectories +import kotlin.io.path.exists /** * Adapter to TurboVec sidecar process for cross-session vector-based memory. @@ -24,6 +26,9 @@ class TurboVecL3MemoryStore(private val config: TurboVecSidecarConfig) : Rehydra private val json = Json { ignoreUnknownKeys = true } private val mutex = Mutex() private var process: Process? = null + // One reader owned by the store for the process's lifetime. A fresh bufferedReader() per request + // would strand read-ahead bytes in the discarded reader's buffer, desyncing the stream. + private var reader: BufferedReader? = null private val metadata = java.util.concurrent.ConcurrentHashMap() override suspend fun rehydrateMetadata(entries: List) { @@ -72,10 +77,12 @@ class TurboVecL3MemoryStore(private val config: TurboVecSidecarConfig) : Rehydra override suspend fun close() { mutex.withLock { process?.let { p -> - val shutdownRequest = SidecarRequest(op = "shutdown") - runCatching { sendRequest(shutdownRequest) } + // Persist vectors before exit so the next process can `load` them (only save() puts + // the quantized index on disk — init alone loses everything between runs). + config.persistPath?.let { runCatching { sendRequestLocked(SidecarRequest(op = "save", path = it.toString())) } } + runCatching { sendRequestLocked(SidecarRequest(op = "shutdown")) } try { - withTimeout(2000) { + withTimeout(SHUTDOWN_TIMEOUT_MS) { p.waitFor() } } catch (e: Exception) { @@ -85,27 +92,41 @@ class TurboVecL3MemoryStore(private val config: TurboVecSidecarConfig) : Rehydra throw e } } + reader = null + process = null } } } - private suspend fun sendRequest(request: SidecarRequest): SidecarResponse { - mutex.withLock { - ensureProcessStarted() - val process = process ?: throw RuntimeException("Process failed to start") + private suspend fun sendRequest(request: SidecarRequest): SidecarResponse = + mutex.withLock { sendRequestLocked(request) } - val requestLine = json.encodeToString(SidecarRequest.serializer(), request) + "\n" - process.outputStream.write(requestLine.toByteArray()) - process.outputStream.flush() + /** Caller must hold [mutex]. */ + private fun sendRequestLocked(request: SidecarRequest): SidecarResponse { + ensureProcessStarted() + val process = process ?: throw RuntimeException("Process failed to start") + val reader = reader ?: throw RuntimeException("Sidecar reader not initialized") - val responseLine = runCatching { - withTimeout(config.requestTimeoutMs) { - process.inputStream.bufferedReader().readLine() - } - }.getOrNull() ?: throw RuntimeException("No response from sidecar") + val requestLine = json.encodeToString(SidecarRequest.serializer(), request) + "\n" + process.outputStream.write(requestLine.toByteArray()) + process.outputStream.flush() - return json.decodeFromString(SidecarResponse.serializer(), responseLine) + // A blocking readLine can't be cancelled by a coroutine timeout, so a slow/late response + // would land in the NEXT request's read and desync the stream permanently. We don't wrap it + // in withTimeout at all: instead the null/failure path below tears the process down so the + // next request restarts from a clean, empty stream rather than reading a stale line. + val responseLine = runCatching { reader.readLine() }.getOrNull() + if (responseLine == null) { + killProcess() + throw RuntimeException("No response from sidecar (process reset)") } + return json.decodeFromString(SidecarResponse.serializer(), responseLine) + } + + private fun killProcess() { + runCatching { process?.destroyForcibly() } + reader = null + process = null } private fun ensureProcessStarted() { @@ -122,19 +143,24 @@ class TurboVecL3MemoryStore(private val config: TurboVecSidecarConfig) : Rehydra val started = pb.start() process = started + reader = started.inputStream.bufferedReader() - // The sidecar rejects add/search until the index is created. Send init once on startup; - // without this every store/query fails with "Index not initialized". - val initLine = json.encodeToString( - SidecarRequest.serializer(), - SidecarRequest(op = "init", dim = config.dim, bitWidth = config.bitWidth) - ) + "\n" - started.outputStream.write(initLine.toByteArray()) + // Reload a persisted index if one exists, else create a fresh one. Either way the sidecar + // rejects add/search until initialized; without this every store/query fails "not initialized". + val bootstrap = config.persistPath + ?.takeIf { it.exists() } + ?.let { SidecarRequest(op = "load", path = it.toString()) } + ?: SidecarRequest(op = "init", dim = config.dim, bitWidth = config.bitWidth) + + started.outputStream.write((json.encodeToString(SidecarRequest.serializer(), bootstrap) + "\n").toByteArray()) started.outputStream.flush() - val response = started.inputStream.bufferedReader().readLine() - ?.let { json.decodeFromString(SidecarResponse.serializer(), it) } + val response = reader!!.readLine()?.let { json.decodeFromString(SidecarResponse.serializer(), it) } if (response?.ok != true) { - throw RuntimeException("TurboVec init failed: ${response?.error ?: "no response"}") + throw RuntimeException("TurboVec ${bootstrap.op} failed: ${response?.error ?: "no response"}") } } + + private companion object { + const val SHUTDOWN_TIMEOUT_MS = 2000L + } } diff --git a/infrastructure/src/main/kotlin/com/correx/infrastructure/InfrastructureModule.kt b/infrastructure/src/main/kotlin/com/correx/infrastructure/InfrastructureModule.kt index 98ea2fb8..309aac21 100644 --- a/infrastructure/src/main/kotlin/com/correx/infrastructure/InfrastructureModule.kt +++ b/infrastructure/src/main/kotlin/com/correx/infrastructure/InfrastructureModule.kt @@ -17,6 +17,7 @@ import com.correx.core.events.EventDispatcher import com.correx.core.events.stores.EventStore import com.correx.core.inference.CapabilityScore import com.correx.core.inference.Embedder +import com.correx.core.inference.GenerationConfig import com.correx.core.inference.InferenceProvider import com.correx.core.inference.InferenceRouter import com.correx.core.inference.ModelCapability @@ -210,10 +211,13 @@ object InfrastructureModule { ), ) - fun createWorkflowLoader(extraKinds: List = emptyList()): WorkflowLoader { + fun createWorkflowLoader( + extraKinds: List = emptyList(), + samplingDefaults: GenerationConfig = GenerationConfig(temperature = 0.7, topP = 1.0, maxTokens = 0), + ): WorkflowLoader { val registry = DefaultArtifactKindRegistry() extraKinds.forEach { registry.register(it) } - return TomlWorkflowLoader(registry) + return TomlWorkflowLoader(registry, samplingDefaults) } fun createPromptLoader(): PromptLoader = FileSystemPromptLoader() diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt index 37d3f3f4..cfe0d179 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileEditTool.kt @@ -24,7 +24,6 @@ import java.nio.file.Files import java.nio.file.InvalidPathException import java.nio.file.Path import java.nio.file.Paths -import java.nio.file.StandardOpenOption @Suppress("TooManyFunctions") class FileEditTool( @@ -242,7 +241,9 @@ class FileEditTool( private fun append(path: Path, request: ToolRequest): ToolResult { val content = request.parameters["content"] as String - Files.writeString(path, content, StandardOpenOption.APPEND) + // Read-modify-atomic-swap instead of an in-place APPEND: a crash mid-append leaves the file + // whole (old content), never truncated. Same crash-safety FileWriteTool gets. + AtomicFileWriter.write(path, (Files.readString(path) + content).toByteArray(Charsets.UTF_8)) return ToolResult.Success( invocationId = request.invocationId, output = "Content appended to ${path.toAbsolutePath()}", @@ -258,7 +259,7 @@ class FileEditTool( return when (occurrences) { 1 -> { val newContent = currentContent.replace(target, replacement) - Files.writeString(path, newContent) + AtomicFileWriter.write(path, newContent.toByteArray(Charsets.UTF_8)) ToolResult.Success( invocationId = request.invocationId, output = "Target replaced in $pathString", @@ -338,7 +339,18 @@ class FileEditTool( return try { process.outputStream.use { it.write(patchContent.toByteArray()) } val output = process.inputStream.bufferedReader().use { it.readText() } - val exitCode = process.waitFor() + if (!process.waitFor(PATCH_TIMEOUT_SECONDS, java.util.concurrent.TimeUnit.SECONDS)) { + // A hung `patch` (e.g. prompting for a reject-file name it never gets) would block the + // stage forever. Kill it and let the model fall back to replace/file_write. + process.destroyForcibly() + return ToolResult.Failure( + invocationId = request.invocationId, + reason = "Patch timed out after ${PATCH_TIMEOUT_SECONDS}s. " + + "Consider using operation 'replace' (exact string swap) or file_write instead.", + recoverable = true, + ) + } + val exitCode = process.exitValue() if (exitCode == 0) { ToolResult.Success( @@ -389,5 +401,6 @@ class FileEditTool( const val MIN_ANCHOR = 3 const val MAX_CMP = 400 const val MIN_SIMILARITY = 0.5 + const val PATCH_TIMEOUT_SECONDS = 30L } } diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt index 2b4d6868..9c5c6277 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/FileReadTool.kt @@ -141,7 +141,10 @@ class FileReadTool( } private fun readFile(path: Path, startLine: Int?, endLine: Int?, request: ToolRequest): ToolResult = runCatching { - val lines = Files.readAllLines(path) + // Read the file once. Lines and the (whole-read) content hash both derive from these bytes, + // so a full read no longer hits the disk twice (readAllLines + readAllBytes for the hash). + val bytes = Files.readAllBytes(path) + val lines = bytes.inputStream().bufferedReader().readLines() val start = ((startLine ?: 1) - 1).coerceAtLeast(0) val end = (endLine ?: lines.size).coerceAtMost(lines.size) val selected = lines.subList(start, end) @@ -166,7 +169,7 @@ class FileReadTool( // against what the agent actually saw. A partial or truncated view establishes no baseline — // the agent must read the relevant range before editing. val sawWholeFile = startLine == null && endLine == null && !lineCapped && !charCapped - val metadata = if (sawWholeFile) mapOf("contentHash" to sha256(path)) else emptyMap() + val metadata = if (sawWholeFile) mapOf("contentHash" to sha256(bytes)) else emptyMap() ToolResult.Success( invocationId = request.invocationId, output = content + note, @@ -180,8 +183,8 @@ class FileReadTool( ) } - private fun sha256(path: Path): String = - java.security.MessageDigest.getInstance("SHA-256").digest(Files.readAllBytes(path)) + private fun sha256(bytes: ByteArray): String = + java.security.MessageDigest.getInstance("SHA-256").digest(bytes) .joinToString("") { "%02x".format(it) } private fun listDir(path: Path, request: ToolRequest): ToolResult = runCatching { diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt index 649d490e..8811e162 100644 --- a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/ListDirTool.kt @@ -111,7 +111,15 @@ class ListDirTool( ToolResult.Failure(request.invocationId, msg, recoverable = true) } !Files.isDirectory(root) -> - ToolResult.Failure(request.invocationId, "Not a directory: $pathString", recoverable = true) + // Explicit file-vs-dir mismatch: a bare "Not a directory" left models re-issuing the + // identical list_dir until cancel (session a60c54b0). Name the remedy so the next + // action is unambiguous — file_read it, or delete/convert it first. + ToolResult.Failure( + request.invocationId, + "Not a directory: $pathString is a FILE, not a directory. " + + "Use file_read to read it, or delete/convert it before listing it as a directory.", + recoverable = true, + ) else -> runCatching { walk(root, recursive, request) }.getOrElse { ToolResult.Failure(request.invocationId, "Failed to list dir: ${it.message}", recoverable = false) } diff --git a/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt new file mode 100644 index 00000000..626bc690 --- /dev/null +++ b/infrastructure/tools/filesystem/src/main/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchTools.kt @@ -0,0 +1,246 @@ +package com.correx.infrastructure.tools.filesystem + +import com.correx.core.approvals.Tier +import com.correx.core.events.events.ToolRequest +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 kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject +import java.nio.file.FileSystems +import java.nio.file.FileVisitResult +import java.nio.file.Files +import java.nio.file.InvalidPathException +import java.nio.file.Path +import java.nio.file.Paths +import java.nio.file.SimpleFileVisitor +import java.nio.file.attribute.BasicFileAttributes +import kotlin.io.path.name +import kotlin.io.path.readText + +// Shared read-only search cap. The list_dir incident (a 400-entry dump burying the answer) is the +// cautionary tale: a search that floods the context is worse than useless to a small model, so cap +// aggressively and tell it to narrow rather than returning everything. +private const val MAX_RESULTS = 200 + +// Skip files larger than this when grepping — a multi-MB minified bundle or lockfile is never what a +// symbol search wants, and reading it line-by-line stalls the walk. ponytail: fixed byte cap; make it +// a param if a real use case needs to grep large generated files. +private const val GREP_MAX_FILE_BYTES = 2_000_000L + +/** + * `.gitignore`-aware path search by glob pattern — the affordance a model should reach for to answer + * "does `frontend/` exist?" / "where are the `.tsx` files?" instead of `shell find`, which is unjailed + * and dumps ignored trees. Read-only (Tier T1, FILE_READ + DIRECTORY_LIST — like [ListDirTool], a glob + * is a survey, so anti-hallucination read gates exempt it: globbing a not-yet-created path truthfully + * reports "no matches"). Shares [FileReadTool]'s path jail + workspace anchor. Results are relative + * paths, sorted, capped at [MAX_RESULTS]. + */ +class GlobTool( + private val allowedPaths: Set = emptySet(), + private val workingDir: Path? = null, +) : Tool, ToolExecutor { + + override val name: String = "glob" + override val description: String = + "Find files by glob pattern (e.g. '**/*.tsx', 'src/**/use*.ts'), skipping .gitignore'd paths. " + + "Prefer this over shell 'find' to locate files. Returns matching relative paths." + override val parametersSchema: JsonObject = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("pattern") { + put("type", "string") + put("description", "Glob pattern, e.g. '**/*.kt' or 'src/**/use*.ts'. Matched against paths relative to 'path'.") + } + putJsonObject("path") { + put("type", "string") + put("description", "Relative directory to search under. Omit or '.' for the workspace root.") + } + } + put("required", buildJsonArray { add(kotlinx.serialization.json.JsonPrimitive("pattern")) }) + } + override val tier: Tier = Tier.T1 + override val requiredCapabilities: Set = + setOf(ToolCapability.FILE_READ, ToolCapability.DIRECTORY_LIST) + override val paramRoles: Map = mapOf("path" to ParamRole.PATH) + + override fun validateRequest(request: ToolRequest): ValidationResult = + validateSearchPath(request, allowedPaths, workingDir) + + override suspend fun execute(request: ToolRequest): ToolResult = withContext(Dispatchers.IO) { + (validateRequest(request) as? ValidationResult.Invalid)?.let { + return@withContext ToolResult.Failure(request.invocationId, it.reason, recoverable = false) + } + val pattern = (request.parameters["pattern"] as? String)?.takeIf { it.isNotBlank() } + ?: return@withContext ToolResult.Failure(request.invocationId, "glob requires a non-empty 'pattern'", recoverable = true) + val base = resolveSearchPath(request, workingDir) + if (!Files.isDirectory(base)) { + return@withContext ToolResult.Failure(request.invocationId, "Not a directory: ${request.parameters["path"] ?: "."}", recoverable = true) + } + val matcher = runCatching { FileSystems.getDefault().getPathMatcher("glob:$pattern") } + .getOrElse { return@withContext ToolResult.Failure(request.invocationId, "Invalid glob pattern: ${it.message}", recoverable = true) } + + val hits = ArrayList() + var truncated = false + walkGitignored(base) { rel, _ -> + if (matcher.matches(Paths.get(rel))) { + hits += rel + if (hits.size >= MAX_RESULTS) truncated = true + } + !truncated + } + hits.sort() + ToolResult.Success(request.invocationId, output = renderResults("glob '$pattern'", hits, truncated)) + } +} + +/** + * `.gitignore`-aware content search — find lines matching a regex across the workspace, so a model can + * locate a symbol/string ("where's `web-ui-design-spec`?") in one call instead of `shell grep -r`. + * Read-only (Tier T1, FILE_READ). Shares the reader's jail + anchor. Returns `path:line: text`, capped + * at [MAX_RESULTS] total matches; an optional `glob` narrows which files are scanned. + */ +class GrepTool( + private val allowedPaths: Set = emptySet(), + private val workingDir: Path? = null, +) : Tool, ToolExecutor { + + override val name: String = "grep" + override val description: String = + "Search file contents for a regex across the workspace (skips .gitignore'd paths). Prefer this " + + "over shell 'grep -r' to find a symbol or string. Returns 'path:line: matched text'." + override val parametersSchema: JsonObject = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("pattern") { + put("type", "string") + put("description", "Regular expression to search for in file contents.") + } + putJsonObject("path") { + put("type", "string") + put("description", "Relative directory to search under. Omit or '.' for the workspace root.") + } + putJsonObject("glob") { + put("type", "string") + put("description", "Optional glob to limit which files are scanned, e.g. '**/*.kt'.") + } + } + put("required", buildJsonArray { add(kotlinx.serialization.json.JsonPrimitive("pattern")) }) + } + override val tier: Tier = Tier.T1 + override val requiredCapabilities: Set = setOf(ToolCapability.FILE_READ) + override val paramRoles: Map = mapOf("path" to ParamRole.PATH) + + override fun validateRequest(request: ToolRequest): ValidationResult = + validateSearchPath(request, allowedPaths, workingDir) + + override suspend fun execute(request: ToolRequest): ToolResult = withContext(Dispatchers.IO) { + (validateRequest(request) as? ValidationResult.Invalid)?.let { + return@withContext ToolResult.Failure(request.invocationId, it.reason, recoverable = false) + } + val patternStr = (request.parameters["pattern"] as? String)?.takeIf { it.isNotBlank() } + ?: return@withContext ToolResult.Failure(request.invocationId, "grep requires a non-empty 'pattern'", recoverable = true) + val regex = runCatching { Regex(patternStr) } + .getOrElse { return@withContext ToolResult.Failure(request.invocationId, "Invalid regex: ${it.message}", recoverable = true) } + val base = resolveSearchPath(request, workingDir) + if (!Files.isDirectory(base)) { + return@withContext ToolResult.Failure(request.invocationId, "Not a directory: ${request.parameters["path"] ?: "."}", recoverable = true) + } + val fileFilter = (request.parameters["glob"] as? String)?.takeIf { it.isNotBlank() }?.let { + runCatching { FileSystems.getDefault().getPathMatcher("glob:$it") } + .getOrElse { e -> return@withContext ToolResult.Failure(request.invocationId, "Invalid glob: ${e.message}", recoverable = true) } + } + + val hits = ArrayList() + var truncated = false + walkGitignored(base) { rel, file -> + if (fileFilter == null || fileFilter.matches(Paths.get(rel))) { + if (Files.size(file) <= GREP_MAX_FILE_BYTES) { + val text = runCatching { file.readText() }.getOrNull() + // Skip apparent binaries (NUL byte) — a symbol search never wants them. + if (text != null && !text.contains('\u0000')) { + text.lineSequence().forEachIndexed { idx, line -> + if (!truncated && regex.containsMatchIn(line)) { + hits += "$rel:${idx + 1}: ${line.trim().take(200)}" + if (hits.size >= MAX_RESULTS) truncated = true + } + } + } + } + } + !truncated + } + ToolResult.Success(request.invocationId, output = renderResults("grep '$patternStr'", hits, truncated)) + } +} + +// ---------- shared helpers ---------- + +private fun resolveSearchPath(request: ToolRequest, workingDir: Path?): Path { + val pathString = (request.parameters["path"] as? String)?.takeIf { it.isNotBlank() } ?: "." + val raw = Paths.get(pathString) + return when { + raw.isAbsolute -> raw.normalize() + workingDir != null -> workingDir.resolve(raw).normalize() + else -> raw.toAbsolutePath().normalize() + } +} + +private fun validateSearchPath(request: ToolRequest, allowedPaths: Set, workingDir: Path?): ValidationResult = + runCatching { + val path = resolveSearchPath(request, workingDir) + val effectiveRoots = allowedPaths + request.grantedPaths.map { Paths.get(it) } + if (!PathJail.isContained(path, effectiveRoots)) { + ValidationResult.Invalid("Path '${request.parameters["path"] ?: "."}' is not in the allowed list") + } else { + ValidationResult.Valid + } + }.getOrElse { + when (it) { + is InvalidPathException -> ValidationResult.Invalid("Invalid path format: ${it.message}") + else -> ValidationResult.Invalid(it.message ?: "Unknown error occurred") + } + } + +/** + * Walk [base] gitignore-pruned (reusing [GitignoreMatcher] — same rules as list_dir), invoking [onFile] + * for every non-ignored regular file with its base-relative path and absolute [Path]. [onFile] returns + * false to stop the walk early (used to terminate once a result cap is hit). `.git` is always skipped. + */ +private fun walkGitignored(base: Path, onFile: (rel: String, file: Path) -> Boolean) { + val matcher = GitignoreMatcher(base, base) + Files.walkFileTree(base, object : SimpleFileVisitor() { + override fun preVisitDirectory(dir: Path, attrs: BasicFileAttributes): FileVisitResult { + if (dir != base && dir.name == ".git") return FileVisitResult.SKIP_SUBTREE + if (dir != base && matcher.ignored(dir, isDir = true)) return FileVisitResult.SKIP_SUBTREE + matcher.loadFrom(dir) + return FileVisitResult.CONTINUE + } + + override fun visitFile(file: Path, attrs: BasicFileAttributes): FileVisitResult { + if (attrs.isDirectory || matcher.ignored(file, isDir = false)) return FileVisitResult.CONTINUE + val rel = base.relativize(file).toString().replace('\\', '/') + return if (onFile(rel, file)) FileVisitResult.CONTINUE else FileVisitResult.TERMINATE + } + + override fun visitFileFailed(file: Path, exc: java.io.IOException) = FileVisitResult.CONTINUE + }) +} + +private fun renderResults(query: String, hits: List, truncated: Boolean): String = buildString { + appendLine("$query") + appendLine("") + appendLine(hits.joinToString("\n").ifEmpty { "(no matches)" }) + if (truncated) appendLine("truncated at $MAX_RESULTS matches; narrow the pattern or scope with 'path'/'glob'") + append("(${hits.size} ${if (hits.size == 1) "match" else "matches"})") + appendLine() + append("") +} diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/ListDirToolTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/ListDirToolTest.kt index 66a986db..6b0cda12 100644 --- a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/ListDirToolTest.kt +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/ListDirToolTest.kt @@ -58,6 +58,19 @@ class ListDirToolTest { assertFalse(out.contains("deep"), out) } + @Test + fun `listing a file gives a recoverable file-vs-dir hint, not a bare error`(): Unit = runBlocking { + // Regression (session a60c54b0): list_dir on a plain file returned a bare "Not a directory" + // and models re-issued the identical call until cancel. The failure must name the remedy. + val root = Files.createTempDirectory("listdir") + Files.writeString(root.resolve("pages"), "i am a file") + val tool = ListDirTool(allowedPaths = setOf(root), workingDir = root) + val result = tool.execute(request(path = "pages")) as ToolResult.Failure + assertTrue(result.recoverable, "file-vs-dir mismatch is recoverable") + assertTrue(result.reason.contains("FILE"), result.reason) + assertTrue(result.reason.contains("file_read"), result.reason) + } + @Test fun `non-recursive lists only immediate children`(): Unit = runBlocking { val root = Files.createTempDirectory("listdir") diff --git a/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchToolsTest.kt b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchToolsTest.kt new file mode 100644 index 00000000..64f51ec0 --- /dev/null +++ b/infrastructure/tools/filesystem/src/test/kotlin/com/correx/infrastructure/tools/filesystem/WorkspaceSearchToolsTest.kt @@ -0,0 +1,94 @@ +package com.correx.infrastructure.tools.filesystem + +import com.correx.core.events.events.ToolRequest +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.ToolResult +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertFalse +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.nio.file.Files +import java.nio.file.Path +import java.util.* + +class WorkspaceSearchToolsTest { + + private fun request(toolName: String, params: Map) = ToolRequest( + invocationId = ToolInvocationId(UUID.randomUUID().toString()), + sessionId = SessionId(UUID.randomUUID().toString()), + stageId = StageId(UUID.randomUUID().toString()), + toolName = toolName, + parameters = params, + ) + + private fun fixture(): Path { + val root = Files.createTempDirectory("search") + Files.writeString(root.resolve(".gitignore"), "node_modules\ndist\n") + Files.createDirectories(root.resolve("src/hooks")) + Files.writeString(root.resolve("src/hooks/useAuth.ts"), "export const useAuth = () => {}\n") + Files.writeString(root.resolve("src/main.ts"), "import { useAuth } from './hooks/useAuth'\n") + Files.writeString(root.resolve("README.md"), "# web-ui-design-spec lives here\n") + Files.createDirectories(root.resolve("node_modules/pkg")) + Files.writeString(root.resolve("node_modules/pkg/useAuth.ts"), "junk useAuth\n") + Files.createDirectories(root.resolve("dist")) + Files.writeString(root.resolve("dist/bundle.js"), "useAuth\n") + return root + } + + @Test + fun `glob finds matching paths and prunes gitignored trees`(): Unit = runBlocking { + val root = fixture() + val tool = GlobTool(allowedPaths = setOf(root), workingDir = root) + val out = (tool.execute(request("glob", mapOf("pattern" to "**/*.ts"))) as ToolResult.Success).output + + assertTrue(out.contains("src/hooks/useAuth.ts"), out) + assertTrue(out.contains("src/main.ts"), out) + assertFalse(out.contains("node_modules"), out) // gitignored subtree never matched + } + + @Test + fun `glob with a leading path segment scopes the search`(): Unit = runBlocking { + val root = fixture() + val tool = GlobTool(allowedPaths = setOf(root), workingDir = root) + val out = (tool.execute(request("glob", mapOf("pattern" to "src/**/use*.ts"))) as ToolResult.Success).output + assertTrue(out.contains("src/hooks/useAuth.ts"), out) + assertFalse(out.contains("main.ts"), out) + } + + @Test + fun `grep finds a literal across files, skipping gitignored ones`(): Unit = runBlocking { + val root = fixture() + val tool = GrepTool(allowedPaths = setOf(root), workingDir = root) + val out = (tool.execute(request("grep", mapOf("pattern" to "web-ui-design-spec"))) as ToolResult.Success).output + assertTrue(out.contains("README.md:1:"), out) + } + + @Test + fun `grep respects an optional file glob and prunes gitignore`(): Unit = runBlocking { + val root = fixture() + val tool = GrepTool(allowedPaths = setOf(root), workingDir = root) + val out = (tool.execute(request("grep", mapOf("pattern" to "useAuth", "glob" to "**/*.ts"))) as ToolResult.Success).output + assertTrue(out.contains("src/hooks/useAuth.ts:"), out) + assertTrue(out.contains("src/main.ts:"), out) + assertFalse(out.contains("node_modules"), out) // gitignored + assertFalse(out.contains("bundle.js"), out) // not matched by *.ts glob (and gitignored) + } + + @Test + fun `grep rejects an invalid regex recoverably`(): Unit = runBlocking { + val root = fixture() + val tool = GrepTool(allowedPaths = setOf(root), workingDir = root) + val result = tool.execute(request("grep", mapOf("pattern" to "[unterminated"))) + assertTrue(result is ToolResult.Failure && result.recoverable, result.toString()) + } + + @Test + fun `search outside the jail is rejected`(): Unit = runBlocking { + val root = fixture() + val tool = GlobTool(allowedPaths = setOf(root), workingDir = root) + val result = tool.execute(request("glob", mapOf("pattern" to "*", "path" to "/etc"))) + assertTrue(result is ToolResult.Failure, result.toString()) + } +} diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/SandboxedToolExecutor.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/SandboxedToolExecutor.kt index 5ef90181..1abaf99b 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/SandboxedToolExecutor.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/SandboxedToolExecutor.kt @@ -119,7 +119,12 @@ class SandboxedToolExecutor( for (original in affectedPaths) { // A1: skip files that don't yet exist (new-file tools have nothing to back up) if (!Files.exists(original)) continue - val backupPath = workingDir.resolve("${original.fileName}.bak") + // Name the backup by the full source path, not just the basename — two affected paths + // sharing a filename in different dirs would otherwise overwrite each other's backup and + // restore the wrong content on failure. Hash keeps the name filesystem-safe and unique. + val stem = original.fileName.toString() + val hash = Integer.toHexString(original.toAbsolutePath().normalize().toString().hashCode()) + val backupPath = workingDir.resolve("$stem.$hash.bak") Files.copy(original, backupPath, StandardCopyOption.REPLACE_EXISTING) put(original, backupPath) } diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt index 1890663e..d1a53945 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolConfig.kt @@ -5,6 +5,8 @@ import com.correx.infrastructure.tools.filesystem.FileDeleteTool import com.correx.infrastructure.tools.filesystem.FileEditTool import com.correx.infrastructure.tools.filesystem.FileReadTool import com.correx.infrastructure.tools.filesystem.FileWriteTool +import com.correx.infrastructure.tools.filesystem.GlobTool +import com.correx.infrastructure.tools.filesystem.GrepTool import com.correx.infrastructure.tools.filesystem.ListDirTool import com.correx.infrastructure.tools.shell.ShellTool import com.correx.infrastructure.tools.web.WebFetchTool @@ -82,6 +84,10 @@ fun ToolConfig.buildTools(): List = buildList { workingDir = fileRead.workingDir, ), ) + // glob (find by pattern) + grep (search contents) — read-only search, same jail/anchor/toggle; + // the affordance weak models need instead of shell find/grep -r. + add(GlobTool(allowedPaths = fileRead.allowedPaths, workingDir = fileRead.workingDir)) + add(GrepTool(allowedPaths = fileRead.allowedPaths, workingDir = fileRead.workingDir)) } if (fileWrite.enabled) { add( diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolOutputTool.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolOutputTool.kt new file mode 100644 index 00000000..918a00c6 --- /dev/null +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/ToolOutputTool.kt @@ -0,0 +1,68 @@ +package com.correx.infrastructure.tools + +import com.correx.core.approvals.Tier +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.types.ArtifactId +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 kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject + +/** + * Retrieves the full text of an earlier tool result that was truncated for model context. When a + * Success output exceeds the context floor, the kernel spills the full raw output to the artifact + * store and shows a marker containing its `ref` (the content hash, also recorded on the + * ToolExecutionCompleted receipt in the event log). This tool resolves that `ref` back to the full + * content. Read-only (Tier T1, no filesystem capability — it reads the content-addressed store by a + * hash the model was explicitly handed, never arbitrary paths). + */ +class ToolOutputTool(private val artifactStore: ArtifactStore) : Tool, ToolExecutor { + + override val name: String = "tool_output" + override val description: String = + "Retrieve the FULL output of an earlier tool call that was truncated. Pass the ref shown in " + + "the truncation marker (e.g. tool_output(ref=\"\"))." + override val parametersSchema: JsonObject = buildJsonObject { + put("type", "object") + putJsonObject("properties") { + putJsonObject("ref") { + put("type", "string") + put("description", "The ref/hash from a '… output truncated …' marker.") + } + } + put("required", buildJsonArray { add(JsonPrimitive("ref")) }) + } + override val tier: Tier = Tier.T1 + override val requiredCapabilities: Set = emptySet() + + override fun validateRequest(request: ToolRequest): ValidationResult = + if ((request.parameters["ref"] as? String).isNullOrBlank()) { + ValidationResult.Invalid("tool_output requires a non-empty 'ref'") + } else { + ValidationResult.Valid + } + + override suspend fun execute(request: ToolRequest): ToolResult { + val ref = (request.parameters["ref"] as? String)?.takeIf { it.isNotBlank() } + ?: return ToolResult.Failure( + request.invocationId, + "tool_output requires a non-empty 'ref'", + recoverable = true, + ) + return artifactStore.get(ArtifactId(ref)) + ?.let { ToolResult.Success(request.invocationId, output = it.toString(Charsets.UTF_8)) } + ?: ToolResult.Failure( + request.invocationId, + "No stored output for ref '$ref' — it may have expired or the ref is wrong.", + recoverable = true, + ) + } +} diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt index b01d0d1d..85faa146 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/shell/ShellTool.kt @@ -16,10 +16,9 @@ import com.correx.core.tools.contract.ToolResult import com.correx.core.tools.contract.ValidationResult import com.correx.core.tools.process.ChildProcess import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.async +import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeout import kotlinx.serialization.json.Json import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive @@ -28,6 +27,7 @@ import kotlinx.serialization.json.buildJsonObject import kotlinx.serialization.json.put import kotlinx.serialization.json.putJsonObject import java.nio.file.Path +import java.util.concurrent.TimeUnit class ShellTool( private val allowedExecutables: Set = emptySet(), @@ -78,8 +78,15 @@ class ShellTool( // is not enough — an auto-approve loop waves these straight through. deniedReason(parsed.argv) != null -> ValidationResult.Invalid(deniedReason(parsed.argv)!!) - allowedExecutables.isNotEmpty() && parsed.argv[0] !in allowedExecutables -> - ValidationResult.Invalid("Executable '${parsed.argv[0]}' is not in the allowed list.") + // Check EVERY command position, not just argv[0]. An operator form runs via `sh -c` + // (below), so `["ls", "&&", "rm", "-rf", "/"]` would slip past an argv[0]-only check + // with allowlist {ls} and then execute `rm`. Shell builtins (cd, export…) are allowed + // implicitly — they navigate, they don't exec an external program. + allowedExecutables.isNotEmpty() -> + commandPositions(parsed.argv) + .firstOrNull { it !in SHELL_BUILTINS && it !in allowedExecutables } + ?.let { ValidationResult.Invalid("Executable '$it' is not in the allowed list.") } + ?: ValidationResult.Valid else -> ValidationResult.Valid } } @@ -140,6 +147,19 @@ class ShellTool( private fun looksLikeShellCommand(argv: List): Boolean = argv[0] in SHELL_BUILTINS || argv.any { it in SHELL_OPERATORS } + // The tokens that land in a command position once the argv is joined and run via `sh -c`: + // argv[0], plus the token right after each command-separator (&&, ||, |, ;, &). Redirect + // operators (>, >>, <, 2>…) are followed by a filename, not a command, so they open no new + // command position. For a plain (non-operator) argv this is just [argv[0]]. + private fun commandPositions(argv: List): List = buildList { + var expectCommand = true + for (tok in argv) when { + tok in COMMAND_SEPARATORS -> expectCommand = true + tok in SHELL_OPERATORS -> expectCommand = false // redirect target follows, not a command + expectCommand -> { add(tok); expectCommand = false } + } + } + // argv[0] is the program name; embedded quotes/commas/whitespace mean the array collapsed into // one string element and there is no real executable to run. private fun checkExecutable(argv: List): ArgvParse = @@ -175,20 +195,12 @@ class ShellTool( runCatching { runCmd(request, process) }.getOrElse { - process.destroyForcibly() - if (it is TimeoutCancellationException) { - ToolResult.Failure( - invocationId = request.invocationId, - reason = "Process timed out after ${timeoutMs}ms", - recoverable = false, - ) - } else { - ToolResult.Failure( - invocationId = request.invocationId, - reason = it.message ?: "Unknown error occurred during execution", - recoverable = false, - ) - } + killTree(process) + ToolResult.Failure( + invocationId = request.invocationId, + reason = it.message ?: "Unknown error occurred during execution", + recoverable = false, + ) } } ?: ToolResult.Failure( invocationId = request.invocationId, @@ -204,6 +216,9 @@ class ShellTool( const val EMPTY_MSG = "Missing or empty 'argv' parameter. Expected a JSON array of strings." val SHELL_BUILTINS = setOf("cd", "export", "source", ".", "pushd", "popd", "umask", "ulimit") val SHELL_OPERATORS = setOf("&&", "||", "|", ">", ">>", "<", ";", "&", "2>", "2>&1") + // Operators that chain a new command (its following token is a command position); the rest + // of SHELL_OPERATORS are redirects whose following token is a filename. + val COMMAND_SEPARATORS = setOf("&&", "||", "|", ";", "&") // Runners that download and execute an arbitrary remote package in one shot. val REMOTE_EXEC_RUNNERS = setOf("npx", "bunx", "pnpx") } @@ -231,11 +246,32 @@ class ShellTool( .joinToString("\n") } - private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = withTimeout(timeoutMs) { - val stdoutDeferred = async { process.inputStream.bufferedReader().use { it.readText() } } - val stderrDeferred = async { process.errorStream.bufferedReader().use { it.readText() } } + // Kill the whole process tree. destroyForcibly() signals only the direct child, so an `sh -c` + // command's grandchildren (the actual npm/tsc/…) would survive as orphans; descendants() reaches + // them. Snapshot descendants BEFORE destroying the parent — reparenting can hide them afterwards. + private fun killTree(process: Process) { + process.toHandle().descendants().forEach { it.destroyForcibly() } + process.destroyForcibly() + } - val exitCode = process.waitFor() + private suspend fun runCmd(request: ToolRequest, process: Process): ToolResult = coroutineScope { + val stdoutDeferred = async(Dispatchers.IO) { process.inputStream.bufferedReader().use { it.readText() } } + val stderrDeferred = async(Dispatchers.IO) { process.errorStream.bufferedReader().use { it.readText() } } + + // waitFor(timeout) enforces the deadline on the clock — the plain blocking waitFor() is not + // coroutine-cancellable, so a withTimeout around it can't actually interrupt a hung process. + val finished = withContext(Dispatchers.IO) { process.waitFor(timeoutMs, TimeUnit.MILLISECONDS) } + if (!finished) { + killTree(process) + stdoutDeferred.cancel() + stderrDeferred.cancel() + return@coroutineScope ToolResult.Failure( + invocationId = request.invocationId, + reason = "Process timed out after ${timeoutMs}ms", + recoverable = false, + ) + } + val exitCode = process.exitValue() val stdout = stdoutDeferred.await() val stderr = stderrDeferred.await() diff --git a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/web/WebFetchTool.kt b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/web/WebFetchTool.kt index 3b990c70..5757d329 100644 --- a/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/web/WebFetchTool.kt +++ b/infrastructure/tools/src/main/kotlin/com/correx/infrastructure/tools/web/WebFetchTool.kt @@ -43,6 +43,13 @@ class WebFetchTool( private val maxBytes: Long = DEFAULT_MAX_BYTES, ) : Tool, ToolExecutor { + // Do NOT follow redirects. Plane-2 egress (NetworkHostRule) validates the REQUESTED host before + // this tool runs; ktor's default redirect-following would then let a server 302 the fetch to an + // internal host that was never validated (SSRF). With redirects off, a 3xx never opens that + // connection — the model must re-issue web_fetch for the new URL, which re-triggers egress + // validation. Derived client shares the injected engine (no separate resource to close). + private val client: HttpClient = httpClient.config { followRedirects = false } + override val name: String = "web_fetch" override val description: String = "Fetch a URL and return its main content as clean markdown." override val tier: Tier = Tier.T2 @@ -77,10 +84,19 @@ class WebFetchTool( } private suspend fun fetch(invocationId: ToolInvocationId, url: String): ToolResult = - httpClient.prepareGet(url).execute { response -> + client.prepareGet(url).execute { response -> val contentType = response.headers[HttpHeaders.ContentType] val declaredLength = response.headers[HttpHeaders.ContentLength]?.toLongOrNull() when { + response.status.value in 300..399 -> + fail( + invocationId, + "URL redirected (HTTP ${response.status.value}) to " + + "${response.headers[HttpHeaders.Location] ?: "an unspecified location"}. Redirects are " + + "not followed automatically — re-issue web_fetch with that URL so egress is validated " + + "for its host.", + recoverable = true, + ) !response.status.isSuccess() -> fail(invocationId, "HTTP ${response.status.value} for $url", recoverable = true) declaredLength != null && declaredLength > maxBytes -> @@ -123,13 +139,13 @@ class WebFetchTool( /** Reads the channel up to [maxBytes]; returns null if the body exceeds the cap. */ private suspend fun readBounded(channel: io.ktor.utils.io.ByteReadChannel): ByteArray? { - val out = ArrayList() + val out = java.io.ByteArrayOutputStream() val buffer = ByteArray(READ_CHUNK) while (true) { val read = channel.readAvailable(buffer, 0, buffer.size) if (read == -1) break - if (out.size + read > maxBytes) return null - for (i in 0 until read) out.add(buffer[i]) + if (out.size() + read > maxBytes) return null + out.write(buffer, 0, read) } return out.toByteArray() } diff --git a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/ToolOutputToolTest.kt b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/ToolOutputToolTest.kt new file mode 100644 index 00000000..d003dc16 --- /dev/null +++ b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/ToolOutputToolTest.kt @@ -0,0 +1,52 @@ +package com.correx.infrastructure.tools + +import com.correx.core.artifactstore.ArtifactStore +import com.correx.core.events.events.ToolRequest +import com.correx.core.events.types.ArtifactId +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.ToolResult +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import java.util.* + +class ToolOutputToolTest { + + private class FakeStore(private val entries: Map) : ArtifactStore { + override suspend fun put(bytes: ByteArray): ArtifactId = error("unused") + override suspend fun get(id: ArtifactId): ByteArray? = entries[id.value] + override suspend fun flushBefore(commit: suspend () -> Unit) = commit() + } + + private fun request(ref: String?) = ToolRequest( + invocationId = ToolInvocationId(UUID.randomUUID().toString()), + sessionId = SessionId(UUID.randomUUID().toString()), + stageId = StageId(UUID.randomUUID().toString()), + toolName = "tool_output", + parameters = ref?.let { mapOf("ref" to it) } ?: emptyMap(), + ) + + @Test + fun `returns the full stored output for a known ref`(): Unit = runBlocking { + val tool = ToolOutputTool(FakeStore(mapOf("hash1" to "the full output".toByteArray()))) + val out = (tool.execute(request("hash1")) as ToolResult.Success).output + assertEquals("the full output", out) + } + + @Test + fun `unknown ref fails recoverably`(): Unit = runBlocking { + val tool = ToolOutputTool(FakeStore(emptyMap())) + val result = tool.execute(request("missing")) + assertTrue(result is ToolResult.Failure && result.recoverable, result.toString()) + } + + @Test + fun `missing ref fails recoverably`(): Unit = runBlocking { + val tool = ToolOutputTool(FakeStore(emptyMap())) + val result = tool.execute(request(null)) + assertTrue(result is ToolResult.Failure && result.recoverable, result.toString()) + } +} diff --git a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt index 14dd79ed..3d6bd8f9 100644 --- a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt +++ b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/shell/ShellToolTest.kt @@ -46,6 +46,32 @@ class ShellToolTest { assertEquals("Executable 'ls' is not in the allowed list.", (result as ValidationResult.Invalid).reason) } + @Test + fun `allowlist rejects a disallowed command hidden after a shell operator`(): Unit = runBlocking { + // Bypass (#61): ["echo","&&","rm"] joins to `sh -c "echo && rm"`; an argv[0]-only allowlist + // check would wave it through with {echo} and then run rm. Every command position is checked. + val tool = ShellTool(allowedExecutables = setOf("echo")) + val result = tool.validateRequest(createRequest(listOf("echo", "hi", "&&", "rm", "-rf", "x"))) + assertTrue(result is ValidationResult.Invalid) + assertEquals("Executable 'rm' is not in the allowed list.", (result as ValidationResult.Invalid).reason) + } + + @Test + fun `allowlist allows a chain where every command is allowed, builtins implicit`(): Unit = runBlocking { + // `cd dir && echo x` — cd is a builtin (implicitly allowed), echo is allowlisted, `dir`/`x` + // are arguments (not command positions), so the chain is valid. + val tool = ShellTool(allowedExecutables = setOf("echo")) + val result = tool.validateRequest(createRequest(listOf("cd", "dir", "&&", "echo", "x"))) + assertEquals(ValidationResult.Valid, result) + } + + @Test + fun `allowlist treats a redirect target as a filename, not a command`(): Unit = runBlocking { + // `echo x > rm` — the token after `>` is a redirect FILE named "rm", not a command to run. + val tool = ShellTool(allowedExecutables = setOf("echo")) + assertEquals(ValidationResult.Valid, tool.validateRequest(createRequest(listOf("echo", "x", ">", "rm")))) + } + @Test fun `validateRequest returns Invalid for empty argv`(): Unit = runBlocking { val tool = ShellTool(allowedExecutables = setOf("echo")) diff --git a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/web/WebFetchToolTest.kt b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/web/WebFetchToolTest.kt index 4ffb820a..fe97ab7d 100644 Binary files a/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/web/WebFetchToolTest.kt and b/infrastructure/tools/src/test/kotlin/com/correx/infrastructure/tools/web/WebFetchToolTest.kt differ diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt index 68fa7198..eb60f2f3 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/ExecutionPlanCompiler.kt @@ -59,12 +59,6 @@ private const val DEFAULT_STAGE_TOKEN_BUDGET = 16384 // default, or the model is truncated (finishReason=length) mid-artifact — and a degenerating local // model burns the whole 2048 on garbage (e.g. a `<|channel>thought` repetition loop) before it can // stop. Mirror the static TomlWorkflowLoader path: pin the completion cap to the stage token budget. -private val DEFAULT_STAGE_GENERATION = GenerationConfig( - temperature = 0.7, - topP = 1.0, - maxTokens = DEFAULT_STAGE_TOKEN_BUDGET, -) - class ExecutionPlanCompiler( private val registry: ArtifactKindRegistry, // Names of every registered tool. A stage that references a tool the runtime can't resolve @@ -79,7 +73,11 @@ class ExecutionPlanCompiler( // retries (Vikunja #41). Off by default so the compiler's own unit tests see only plan stages; // the server's freestyle path (Main.kt) turns it on. private val injectRecovery: Boolean = false, + // Operator sampling defaults for freestyle-compiled stages; maxTokens pinned to the stage budget. + // Default reproduces the former hardcoded temperature=0.7/topP=1.0. + private val samplingDefaults: GenerationConfig = GenerationConfig(temperature = 0.7, topP = 1.0, maxTokens = 0), ) { + private val defaultStageGeneration = samplingDefaults.copy(maxTokens = DEFAULT_STAGE_TOKEN_BUDGET) private val mapper = JsonMapper.builder() .addModule(kotlinModule()) .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) @@ -150,7 +148,7 @@ class ExecutionPlanCompiler( autoBuildGate = s.id == autoGateStageId, semanticReview = s.semanticReview, tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET, - generationConfig = DEFAULT_STAGE_GENERATION, + generationConfig = defaultStageGeneration, metadata = mapOf("promptInline" to s.prompt), ) } @@ -168,7 +166,7 @@ class ExecutionPlanCompiler( StageId(RECOVERY_STAGE) to StageConfig( allowedTools = knownTools.ifEmpty { setOf("file_write", "file_edit", "shell") }, tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET, - generationConfig = DEFAULT_STAGE_GENERATION, + generationConfig = defaultStageGeneration, metadata = mapOf("role" to "recovery", "promptInline" to RECOVERY_PROMPT), ) } diff --git a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/TomlWorkflowLoader.kt b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/TomlWorkflowLoader.kt index 4e848825..5791b129 100644 --- a/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/TomlWorkflowLoader.kt +++ b/infrastructure/workflow/src/main/kotlin/com/correx/infrastructure/workflow/TomlWorkflowLoader.kt @@ -76,6 +76,9 @@ private val mapper = TomlMapper.builder() class TomlWorkflowLoader( private val registry: ArtifactKindRegistry = DefaultArtifactKindRegistry(), + // Operator sampling defaults applied to every stage's inference request (maxTokens is still pinned + // per-stage to the token budget). Defaults reproduce the former hardcoded temperature=0.7/topP=1.0. + private val samplingDefaults: GenerationConfig = GenerationConfig(temperature = 0.7, topP = 1.0, maxTokens = 0), ) : WorkflowLoader { override fun load(path: Path): WorkflowGraph { val raw = path.readText() @@ -116,11 +119,7 @@ class TomlWorkflowLoader( // Propagate the declared token budget to the inference completion cap. // Without this the StageConfig default (maxTokens=2048) is used, truncating // larger artifacts (finishReason=length) → invalid JSON → validation failure. - generationConfig = GenerationConfig( - temperature = 0.7, - topP = 1.0, - maxTokens = s.tokenBudget, - ), + generationConfig = samplingDefaults.copy(maxTokens = s.tokenBudget), maxRetries = s.maxRetries, metadata = s.toMetadata(workflowDir), ) diff --git a/testing/integration/src/test/kotlin/FreestyleApprovalGateTest.kt b/testing/integration/src/test/kotlin/FreestyleApprovalGateTest.kt index 76d3d7f1..797a6696 100644 --- a/testing/integration/src/test/kotlin/FreestyleApprovalGateTest.kt +++ b/testing/integration/src/test/kotlin/FreestyleApprovalGateTest.kt @@ -209,6 +209,71 @@ class FreestyleApprovalGateTest { runJob.join() } + @Test + fun `a prior REJECTED decision does not satisfy the gate — it re-prompts, not runs unapproved`(): Unit = + runBlocking { + val sessionId = SessionId("freestyle-gate-reject") + // Pre-seed the log as if the architect gate had already been prompted and REJECTED (the + // retry/resume situation): a matching request + a REJECTED decision for it. The pre-fix + // predicate matched any decision for the request and skipped the gate, running unapproved. + val requestId = com.correx.core.events.types.ApprovalRequestId("seeded-req") + eventStore.append( + com.correx.testing.fixtures.EventFixtures.newEvent( + com.correx.core.events.types.EventId("seed-req"), + sessionId, + ApprovalRequestedEvent( + requestId = requestId, + tier = Tier.T2, + validationReportId = com.correx.core.events.types.ValidationReportId("vr"), + riskSummaryId = null, + sessionId = sessionId, + stageId = architectStage, + projectId = null, + toolName = null, + ), + ), + ) + eventStore.append( + com.correx.testing.fixtures.EventFixtures.newEvent( + com.correx.core.events.types.EventId("seed-dec"), + sessionId, + com.correx.core.events.events.ApprovalDecisionResolvedEvent( + decisionId = com.correx.core.events.types.ApprovalDecisionId("seeded-dec"), + requestId = requestId, + outcome = ApprovalOutcome.REJECTED, + status = ApprovalStatus.COMPLETED, + tier = Tier.T2, + resolutionTimestamp = Clock.System.now(), + reason = "operator said no", + ), + ), + ) + + val orchestrator = buildOrchestrator() + val config = OrchestrationConfig(retryPolicy = RetryPolicy(maxAttempts = 1, backoffMs = 0)) + val runJob = launch { orchestrator.run(sessionId, freestyleGraph(), config) } + + // The gate must re-prompt: a SECOND architect approval request appears (the seeded one is + // the first). With the bug the seeded REJECTED decision satisfied the gate and no new + // request was emitted — the architect ran and the workflow completed unapproved. + withTimeout(5_000) { + while ( + eventStore.read(sessionId) + .mapNotNull { it.payload as? ApprovalRequestedEvent } + .count { it.stageId == architectStage && it.toolName == null } < 2 + ) { + yield() + } + } + + assertTrue( + eventStore.read(sessionId).none { it.payload is WorkflowCompletedEvent }, + "Workflow must not complete off a REJECTED decision", + ) + runJob.cancel() + runJob.join() + } + @Test fun `approval resumes workflow and architect produces execution plan`(): Unit = runBlocking { val sessionId = SessionId("freestyle-gate-2")