Conversation
|
Bosun CI signal: Bosun-created PR currently has failing checks.
|
There was a problem hiding this comment.
Pull request overview
Adds a richer “Session Detail” modal to the Ink TUI Agents screen, enabling drill-down into a selected session with a turn timeline, recent diff context, and live output/steering controls.
Changes:
- Implement
SessionDetailmodal UI with metadata, turn timeline (scrollable), diff preview, and a right-side log panel when wide enough. - Add steering input flow and periodic session detail polling while the modal is open.
- Extend TUI rendering tests and fixtures to cover modal open/close, steering, timeline scrolling, and log streaming.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tui/screens/agents.mjs | Implements the session detail modal, polling, steering input handling, diff/log rendering, and timeline scrolling logic. |
| tests/tui/screens.test.mjs | Adds integration-style Ink render tests for the new modal behaviors and streaming updates. |
| tests/tui/fixtures.mjs | Expands fixtures with turns, metadata fields, and a longer diff payload for modal rendering scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| React.useEffect(() => { | ||
| if (!wsBridge || typeof wsBridge.on !== "function") return undefined; | ||
| const off = wsBridge.on("logs:stream", (payload) => { | ||
| const payloadSessionId = String(payload?.sessionId || payload?.id || payload?.session?.id || ""); | ||
| if (!detailView?.session?.id || payloadSessionId !== String(detailView.session.id)) return; | ||
| const line = streamPayloadToLogLine(payload); | ||
| if (!line) return; | ||
| setLogLines((current) => [...current.slice(-(MAX_LOG_LINES - 1)), line]); | ||
| }); |
There was a problem hiding this comment.
The logs:stream handler filters on payload.sessionId/payload.id, but the canonical TUI logs:stream payload contract does not include any session identifier (it is {logType, query, filePath, line, raw, level, timestamp} with additionalProperties: false). In production this will likely drop all streamed log lines while the detail modal is open. Consider either (a) not filtering by session at all, (b) filtering using payload.query / payload.filePath in a way that matches how logs are subscribed, or (c) extending the event contract + emitter to include a sessionId field and updating the schema accordingly.
| @@ -129,6 +312,24 @@ export default function AgentsScreen({ wsBridge, host = "127.0.0.1", port = 3080 | |||
| }); | |||
| }, []); | |||
|
|
|||
There was a problem hiding this comment.
The screen no longer has a periodic tick to keep clockMs advancing and to re-run reconcileSessionEntries(...) when the upstream sessions prop is unchanged. This will cause age/idle counters (and any retention logic in reconcileSessionEntries) to freeze until a sessions update arrives. Reintroduce a setInterval (e.g. 1s) that updates clockMs and recomputes entries from liveSessionsRef.current (and make sure it’s cleaned up on unmount).
| React.useEffect(() => { | |
| const intervalId = setInterval(() => { | |
| const now = Date.now(); | |
| setClockMs(now); | |
| setEntries((previous) => { | |
| const nextEntries = reconcileSessionEntries(previous, liveSessionsRef.current, now); | |
| setSelectedId((current) => { | |
| if (current && nextEntries.some((entry) => entry.id === current)) return current; | |
| return nextEntries[0]?.id || ""; | |
| }); | |
| return nextEntries; | |
| }); | |
| }, 1000); | |
| return () => { | |
| clearInterval(intervalId); | |
| }; | |
| }, []); |
| sessionId: "session-active-1", | ||
| timestamp: "2026-03-23T00:00:31.000Z", | ||
| stream: "stdout", | ||
| line: "Steer message accepted by running session", |
There was a problem hiding this comment.
This test emits a logs:stream payload with {sessionId, stream, line}, but the repo’s canonical logs:stream event contract requires {logType, raw, line, level, timestamp, filePath} (and forbids additional properties). The test should use the canonical shape (or, if the intent is to add sessionId to the contract, update the schema + emitter and then align this test with that new contract).
| sessionId: "session-active-1", | |
| timestamp: "2026-03-23T00:00:31.000Z", | |
| stream: "stdout", | |
| line: "Steer message accepted by running session", | |
| logType: "stdout", | |
| raw: "Steer message accepted by running session", | |
| line: "Steer message accepted by running session", | |
| level: "info", | |
| timestamp: "2026-03-23T00:00:31.000Z", | |
| filePath: "", |
|
|
||
|
|
There was a problem hiding this comment.
Trailing blank lines at end of file were introduced here; please remove to keep diffs clean.
f459012 to
db3dfa0
Compare
…l-full-session-drill
…dal-full-session-drill
|
Bosun PR classification: Bosun-created.
|
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
Co-authored-by: bosun-ve[bot] <262908237+bosun-ve[bot]@users.noreply.github.com>
Task-ID: cfd631a8-7666-458d-9b2b-5bddd4bd9318\n\nAutomated PR for task cfd631a8-7666-458d-9b2b-5bddd4bd9318\n\n---\n\nBosun-Origin: created