fix: complete all P2 audit findings — dead code, NPE guard, hygiene
P2-1: Guard SessionId NPE with safe-call in SessionsReducer
P2-2: Remove 16 dead event classes + reducer branches; replace with live orchestration events in tests
P2-3: Standardize ChatInput divergences — guard CancelSession, single-arg ProtocolError
P2-4: Remove dead TuiToolRecord.diff and ToolDisplayStatus.REQUESTED
P2-5: Remove dead streamLive from SessionEventBridge
P2-6: Extract SessionsReducerContext data class for 6+ param method
P2-7: Replace StageId("none") sentinel with TypeId.NONE
P2-9: Add TypeId.random() factory on all type-alias IDs
P2-10: Add KDoc on schemaVersion documenting reserved-for-migration
P2-8: Verified RouterReducer string-template already correct (TypeId value class toString)
This commit is contained in:
@@ -244,12 +244,4 @@ class SessionEventBridge(
|
||||
return orderedIds.mapNotNull { byInvocation[it] }
|
||||
}
|
||||
|
||||
suspend fun streamLive(sessionId: SessionId) {
|
||||
var sessionSequence = 0L
|
||||
eventStore.subscribe(sessionId).collect { event ->
|
||||
sessionSequence++
|
||||
domainEventToServerMessage(event, artifactStore, sessionSequence = sessionSequence)
|
||||
?.let { send(it) }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -88,11 +88,7 @@ class GlobalStreamHandler(private val module: ServerModule) {
|
||||
}
|
||||
.onFailure {
|
||||
log.warn("decode error: {}", it.message)
|
||||
val error = ServerMessage.ProtocolError(
|
||||
message = "Unknown message: ${it.message}",
|
||||
sequence = null,
|
||||
sessionSequence = null,
|
||||
)
|
||||
val error = ServerMessage.ProtocolError("Unknown message: ${it.message}")
|
||||
session.send(Frame.Text(ProtocolSerializer.encodeServerMessage(error)))
|
||||
}
|
||||
}
|
||||
@@ -144,7 +140,10 @@ class GlobalStreamHandler(private val module: ServerModule) {
|
||||
is ClientMessage.Ping -> Unit
|
||||
is ClientMessage.StartSession -> handleStartSession(session, msg, sendFrame)
|
||||
is ClientMessage.StartChatSession -> handleStartChatSession(session, msg, sendFrame)
|
||||
is ClientMessage.CancelSession -> module.orchestrator.cancel(msg.sessionId)
|
||||
is ClientMessage.CancelSession -> {
|
||||
runCatching { module.orchestrator.cancel(msg.sessionId) }
|
||||
.onFailure { log.error("cancel failed for session={}: {}", msg.sessionId.value, it.message, it) }
|
||||
}
|
||||
is ClientMessage.ResumeSession -> session.send(encodeError("ResumeSession not supported"))
|
||||
is ClientMessage.ApprovalResponse -> handleApprovalResponse(msg, sendFrame)
|
||||
is ClientMessage.CreateGrant -> handleCreateGrant(msg, sendFrame)
|
||||
@@ -159,11 +158,7 @@ class GlobalStreamHandler(private val module: ServerModule) {
|
||||
))
|
||||
}.onFailure {
|
||||
log.error("routerFacade.onUserInput failed: {}", it.message)
|
||||
sendFrame(ServerMessage.ProtocolError(
|
||||
message = "Router error: ${it.message}",
|
||||
sequence = null,
|
||||
sessionSequence = null,
|
||||
))
|
||||
sendFrame(ServerMessage.ProtocolError("Router error: ${it.message}"))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -176,34 +171,16 @@ class GlobalStreamHandler(private val module: ServerModule) {
|
||||
val scopeSessionId = module.approvalCoordinator.lookupSession(msg.requestId)
|
||||
if (scopeSessionId == null) {
|
||||
log.warn("handleApprovalResponse: no session found for requestId={}", msg.requestId.value)
|
||||
sendFrame(
|
||||
ServerMessage.ProtocolError(
|
||||
message = "Unknown approval request",
|
||||
sequence = null,
|
||||
sessionSequence = null,
|
||||
),
|
||||
)
|
||||
sendFrame(ServerMessage.ProtocolError("Unknown approval request"))
|
||||
return
|
||||
}
|
||||
module.approvalCoordinator.handleResponse(msg, scopeSessionId)?.let { sendFrame(it) }
|
||||
}
|
||||
|
||||
private fun errorResponse(message: String) = ServerMessage.ProtocolError(
|
||||
message = message,
|
||||
sequence = null,
|
||||
sessionSequence = null,
|
||||
)
|
||||
private fun errorResponse(message: String) = ServerMessage.ProtocolError(message)
|
||||
|
||||
private fun encodeError(message: String): Frame.Text =
|
||||
Frame.Text(
|
||||
ProtocolSerializer.encodeServerMessage(
|
||||
ServerMessage.ProtocolError(
|
||||
message = message,
|
||||
sequence = null,
|
||||
sessionSequence = null,
|
||||
),
|
||||
),
|
||||
)
|
||||
Frame.Text(ProtocolSerializer.encodeServerMessage(ServerMessage.ProtocolError(message)))
|
||||
|
||||
private suspend fun handleCreateGrant(
|
||||
msg: ClientMessage.CreateGrant,
|
||||
|
||||
@@ -10,7 +10,7 @@ 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.OrchestrationPausedEvent
|
||||
import com.correx.core.events.events.SessionStartedEvent
|
||||
import com.correx.core.events.events.SteeringNoteAddedEvent
|
||||
import com.correx.core.events.events.StageCompletedEvent
|
||||
import com.correx.core.events.events.StageFailedEvent
|
||||
import com.correx.core.events.events.StoredEvent
|
||||
@@ -371,7 +371,7 @@ class DomainEventMapperTest {
|
||||
|
||||
@Test
|
||||
fun `unmapped event returns null`(): Unit = runTest {
|
||||
val event = storedEvent(SessionStartedEvent(sessionId = sessionId))
|
||||
val event = storedEvent(SteeringNoteAddedEvent(sessionId = sessionId, content = "test"))
|
||||
val result = domainEventToServerMessage(event, noopStore, sessionSequence = 0L)
|
||||
assertNull(result)
|
||||
}
|
||||
@@ -392,12 +392,12 @@ class DomainEventMapperTest {
|
||||
val origLevel = logger.level
|
||||
logger.level = Level.DEBUG
|
||||
try {
|
||||
val event = storedEvent(SessionStartedEvent(sessionId = sessionId))
|
||||
val event = storedEvent(SteeringNoteAddedEvent(sessionId = sessionId, content = "test"))
|
||||
val result = domainEventToServerMessage(event, noopStore, sessionSequence = 0L)
|
||||
assertNull(result)
|
||||
assertEquals(1, appender.events.size)
|
||||
assertEquals(Level.DEBUG, appender.events[0].level)
|
||||
assertTrue(appender.events[0].message.formattedMessage.contains("SessionStartedEvent"))
|
||||
assertTrue(appender.events[0].message.formattedMessage.contains("SteeringNoteAddedEvent"))
|
||||
} finally {
|
||||
logger.removeAppender(appender)
|
||||
appender.stop()
|
||||
|
||||
@@ -25,10 +25,8 @@ import com.correx.core.kernel.orchestration.OrchestrationRepository
|
||||
import com.correx.core.tools.contract.Tool
|
||||
import com.correx.core.tools.registry.ToolRegistry
|
||||
import com.correx.core.sessions.projections.replay.EventReplayer
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.flow
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.datetime.Instant
|
||||
import org.junit.jupiter.api.Assertions.assertEquals
|
||||
@@ -186,36 +184,6 @@ class SessionEventBridgeTest {
|
||||
assertEquals(ServerMessage.SnapshotComplete, sent[1])
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `streamLive sends mapped messages from live flow`() = runTest {
|
||||
val events = listOf(
|
||||
storedEvent(WorkflowStartedEvent(sessionId, workflowId, stageId), seq = 1L),
|
||||
storedEvent(WorkflowCompletedEvent(sessionId, stageId, 1), seq = 2L),
|
||||
)
|
||||
val liveFlow: Flow<StoredEvent> = flow { events.forEach { emit(it) } }
|
||||
val store = fakeEventStore(liveFlow = liveFlow)
|
||||
val sent = mutableListOf<ServerMessage>()
|
||||
val bridge = SessionEventBridge(
|
||||
store,
|
||||
noopArtifactStore,
|
||||
activeOrchestrationRepository(),
|
||||
noopWorkflowRegistry,
|
||||
noopToolRegistry,
|
||||
) { sent.add(it) }
|
||||
|
||||
bridge.streamLive(sessionId)
|
||||
|
||||
assertEquals(2, sent.size)
|
||||
assertEquals(
|
||||
ServerMessage.SessionStarted(sessionId, workflowId, 1L, 1L),
|
||||
sent[0],
|
||||
)
|
||||
assertEquals(
|
||||
ServerMessage.SessionCompleted(sessionId, 2L, 2L),
|
||||
sent[1],
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `replaySnapshot with no sessions emits only SnapshotComplete`() = runTest {
|
||||
val store = fakeEventStore(allEventsList = emptyList())
|
||||
@@ -266,29 +234,4 @@ class SessionEventBridgeTest {
|
||||
assertEquals(3L, snapshot.lastSessionSequence)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `streamLive cancellation stops collection`() = runTest {
|
||||
val channel = Channel<StoredEvent>(Channel.UNLIMITED)
|
||||
val liveFlow: Flow<StoredEvent> = flow {
|
||||
for (event in channel) emit(event)
|
||||
}
|
||||
val store = fakeEventStore(liveFlow = liveFlow)
|
||||
val sent = mutableListOf<ServerMessage>()
|
||||
val bridge = SessionEventBridge(
|
||||
store,
|
||||
noopArtifactStore,
|
||||
activeOrchestrationRepository(),
|
||||
noopWorkflowRegistry,
|
||||
noopToolRegistry,
|
||||
) { sent.add(it) }
|
||||
|
||||
channel.send(storedEvent(WorkflowStartedEvent(sessionId, workflowId, stageId), seq = 1L))
|
||||
|
||||
val job = launch { bridge.streamLive(sessionId) }
|
||||
channel.send(storedEvent(WorkflowCompletedEvent(sessionId, stageId, 1), seq = 2L))
|
||||
channel.close()
|
||||
job.join()
|
||||
|
||||
assertEquals(2, sent.size)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
package com.correx.apps.tui.components
|
||||
|
||||
import com.correx.apps.tui.state.ToolDisplayStatus
|
||||
import com.correx.apps.tui.state.TuiState
|
||||
import dev.tamboui.style.Style
|
||||
import dev.tamboui.text.Line
|
||||
|
||||
@@ -62,12 +62,14 @@ object RootReducer {
|
||||
|
||||
log.debug("sessions state before reducers={}", state.sessions)
|
||||
val (sessions, se) = SessionsReducer.reduce(
|
||||
sessions = afterInput.sessions,
|
||||
displayState = afterInput.displayState,
|
||||
inputMode = prevInputMode,
|
||||
inputText = prevInputBuffer,
|
||||
action = action,
|
||||
clock = clock,
|
||||
SessionsReducerContext(
|
||||
sessions = afterInput.sessions,
|
||||
displayState = afterInput.displayState,
|
||||
inputMode = prevInputMode,
|
||||
inputText = prevInputBuffer,
|
||||
action = action,
|
||||
clock = clock,
|
||||
),
|
||||
)
|
||||
log.debug("connection state before reducers={}", state.connection)
|
||||
val (connection, ce) = ConnectionReducer.reduce(afterInput.connection, action)
|
||||
|
||||
@@ -22,40 +22,42 @@ import java.time.format.DateTimeFormatter
|
||||
|
||||
private val log = LoggerFactory.getLogger(SessionsReducer::class.simpleName)
|
||||
|
||||
data class SessionsReducerContext(
|
||||
val sessions: SessionsState,
|
||||
val displayState: DisplayState,
|
||||
val inputMode: InputMode,
|
||||
val inputText: String,
|
||||
val action: Action,
|
||||
val clock: () -> Long = System::currentTimeMillis,
|
||||
)
|
||||
|
||||
object SessionsReducer {
|
||||
private val timeFormatter = DateTimeFormatter.ofPattern("HH:mm:ss").withZone(ZoneOffset.UTC)
|
||||
|
||||
fun reduce(
|
||||
sessions: SessionsState,
|
||||
displayState: DisplayState,
|
||||
inputMode: InputMode,
|
||||
inputText: String,
|
||||
action: Action,
|
||||
clock: () -> Long = System::currentTimeMillis,
|
||||
): Pair<SessionsState, List<Effect>> = when (action) {
|
||||
fun reduce(ctx: SessionsReducerContext): Pair<SessionsState, List<Effect>> = when (ctx.action) {
|
||||
is Action.NavigateUp -> when {
|
||||
inputMode == InputMode.FILTER -> navigateUp(sessions) to emptyList()
|
||||
displayState == DisplayState.IDLE -> navigateUp(sessions) to emptyList()
|
||||
displayState == DisplayState.IN_SESSION -> sessions to emptyList()
|
||||
else -> sessions to emptyList()
|
||||
ctx.inputMode == InputMode.FILTER -> navigateUp(ctx.sessions) to emptyList()
|
||||
ctx.displayState == DisplayState.IDLE -> navigateUp(ctx.sessions) to emptyList()
|
||||
ctx.displayState == DisplayState.IN_SESSION -> ctx.sessions to emptyList()
|
||||
else -> ctx.sessions to emptyList()
|
||||
}
|
||||
|
||||
is Action.NavigateDown -> when {
|
||||
inputMode == InputMode.FILTER -> navigateDown(sessions) to emptyList()
|
||||
displayState == DisplayState.IDLE -> navigateDown(sessions) to emptyList()
|
||||
displayState == DisplayState.IN_SESSION -> sessions to emptyList()
|
||||
else -> sessions to emptyList()
|
||||
ctx.inputMode == InputMode.FILTER -> navigateDown(ctx.sessions) to emptyList()
|
||||
ctx.displayState == DisplayState.IDLE -> navigateDown(ctx.sessions) to emptyList()
|
||||
ctx.displayState == DisplayState.IN_SESSION -> ctx.sessions to emptyList()
|
||||
else -> ctx.sessions to emptyList()
|
||||
}
|
||||
|
||||
is Action.SubmitInput -> when {
|
||||
inputMode == InputMode.FILTER -> sessions.copy(filter = inputText) to emptyList()
|
||||
displayState == DisplayState.IDLE -> {
|
||||
val text = inputText.trim()
|
||||
val wfIndex = sessions.selectedWorkflowIndex
|
||||
ctx.inputMode == InputMode.FILTER -> ctx.sessions.copy(filter = ctx.inputText) to emptyList()
|
||||
ctx.displayState == DisplayState.IDLE -> {
|
||||
val text = ctx.inputText.trim()
|
||||
val wfIndex = ctx.sessions.selectedWorkflowIndex
|
||||
when {
|
||||
wfIndex in sessions.workflows.indices -> {
|
||||
val wf = sessions.workflows[wfIndex]
|
||||
sessions.copy(selectedWorkflowIndex = -1) to listOf(
|
||||
wfIndex in ctx.sessions.workflows.indices -> {
|
||||
val wf = ctx.sessions.workflows[wfIndex]
|
||||
ctx.sessions.copy(selectedWorkflowIndex = -1) to listOf(
|
||||
Effect.SendWs(ClientMessage.StartSession(workflowId = wf.workflowId, config = null)),
|
||||
)
|
||||
}
|
||||
@@ -67,56 +69,56 @@ object SessionsReducer {
|
||||
status = "STARTING",
|
||||
workflowId = "chat",
|
||||
name = "chat",
|
||||
lastEventAt = clock(),
|
||||
lastEventAt = ctx.clock(),
|
||||
)
|
||||
sessions.copy(
|
||||
sessions = sessions.sessions + optimistic,
|
||||
ctx.sessions.copy(
|
||||
sessions = ctx.sessions.sessions + optimistic,
|
||||
selectedId = sessionId.value,
|
||||
) to listOf(
|
||||
Effect.SendWs(ClientMessage.StartChatSession(sessionId = sessionId, text = text)),
|
||||
)
|
||||
}
|
||||
|
||||
else -> sessions to emptyList()
|
||||
else -> ctx.sessions to emptyList()
|
||||
}
|
||||
}
|
||||
|
||||
displayState == DisplayState.IN_SESSION -> {
|
||||
sessions to listOf(
|
||||
ctx.displayState == DisplayState.IN_SESSION -> ctx.sessions.selectedId?.let { sid ->
|
||||
ctx.sessions to listOf(
|
||||
Effect.SendWs(
|
||||
ClientMessage.ChatInput(
|
||||
sessionId = SessionId(sessions.selectedId!!),
|
||||
text = inputText,
|
||||
sessionId = SessionId(sid),
|
||||
text = ctx.inputText,
|
||||
mode = ChatMode.CHAT,
|
||||
),
|
||||
),
|
||||
)
|
||||
}
|
||||
} ?: (ctx.sessions to emptyList())
|
||||
|
||||
else -> sessions to emptyList()
|
||||
else -> ctx.sessions to emptyList()
|
||||
}
|
||||
|
||||
is Action.CancelInput -> if (inputMode == InputMode.FILTER) {
|
||||
sessions.copy(filter = "") to emptyList()
|
||||
is Action.CancelInput -> if (ctx.inputMode == InputMode.FILTER) {
|
||||
ctx.sessions.copy(filter = "") to emptyList()
|
||||
} else {
|
||||
sessions to emptyList()
|
||||
ctx.sessions to emptyList()
|
||||
}
|
||||
|
||||
is Action.CancelSelectedSession -> {
|
||||
val id = sessions.selectedId
|
||||
val id = ctx.sessions.selectedId
|
||||
if (id != null) {
|
||||
sessions to listOf(Effect.SendWs(ClientMessage.CancelSession(sessionId = SessionId(id))))
|
||||
ctx.sessions to listOf(Effect.SendWs(ClientMessage.CancelSession(sessionId = SessionId(id))))
|
||||
} else {
|
||||
sessions to emptyList()
|
||||
ctx.sessions to emptyList()
|
||||
}
|
||||
}
|
||||
|
||||
is Action.ToggleWorkflows -> sessions.copy(
|
||||
workflowsVisible = !sessions.workflowsVisible,
|
||||
is Action.ToggleWorkflows -> ctx.sessions.copy(
|
||||
workflowsVisible = !ctx.sessions.workflowsVisible,
|
||||
) to emptyList()
|
||||
|
||||
is Action.ServerEventReceived -> applyServerMessage(sessions, action.message, clock)
|
||||
else -> sessions to emptyList()
|
||||
is Action.ServerEventReceived -> applyServerMessage(ctx.sessions, ctx.action.message, ctx.clock)
|
||||
else -> ctx.sessions to emptyList()
|
||||
}
|
||||
|
||||
private fun filteredSessions(sessions: SessionsState): List<SessionSummary> =
|
||||
@@ -311,7 +313,6 @@ object SessionsReducer {
|
||||
tier = tool.tier,
|
||||
status = displayStatus,
|
||||
argsPreview = null,
|
||||
diff = tool.diff,
|
||||
)
|
||||
}
|
||||
val summary = SessionSummary(
|
||||
@@ -409,7 +410,7 @@ object SessionsReducer {
|
||||
if (s.id == msg.sessionId.value) {
|
||||
val updatedTools = s.tools.map { t ->
|
||||
if (t.name == msg.toolName && t.status == ToolDisplayStatus.STARTED) {
|
||||
t.copy(status = ToolDisplayStatus.COMPLETED, diff = msg.diff)
|
||||
t.copy(status = ToolDisplayStatus.COMPLETED)
|
||||
} else {
|
||||
t
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@ import com.correx.apps.server.protocol.ServerMessage
|
||||
|
||||
enum class InputMode { ROUTER, FILTER }
|
||||
|
||||
enum class ToolDisplayStatus { REQUESTED, STARTED, COMPLETED, FAILED, REJECTED }
|
||||
enum class ToolDisplayStatus { STARTED, COMPLETED, FAILED, REJECTED }
|
||||
|
||||
enum class ProviderType { LOCAL, REMOTE }
|
||||
|
||||
@@ -14,7 +14,6 @@ data class TuiToolRecord(
|
||||
val tier: Int,
|
||||
val status: ToolDisplayStatus,
|
||||
val argsPreview: String?,
|
||||
val diff: String? = null,
|
||||
)
|
||||
|
||||
data class TuiEventEntry(
|
||||
|
||||
@@ -41,7 +41,16 @@ class SessionsReducerTest {
|
||||
inputMode: InputMode = InputMode.ROUTER,
|
||||
inputText: String = "",
|
||||
action: Action,
|
||||
) = SessionsReducer.reduce(sessions, displayState, inputMode, inputText, action, fixedClock)
|
||||
) = SessionsReducer.reduce(
|
||||
SessionsReducerContext(
|
||||
sessions = sessions,
|
||||
displayState = displayState,
|
||||
inputMode = inputMode,
|
||||
inputText = inputText,
|
||||
action = action,
|
||||
clock = fixedClock,
|
||||
),
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `NavigateUp wraps from top to bottom`() {
|
||||
|
||||
Reference in New Issue
Block a user