[malibio]

Summary

When a PTY output subscriber fell behind the daemon's broadcast buffer, RecvError::Lagged was only logged server-side, so the terminal silently rendered a truncated or desynced stream. This PR makes the gap visible all the way to the user.

• Proto: OutputChunk gains uint64 dropped_chunks = 3. A non-zero value marks a loss marker: its data is empty, and that many chunks were dropped just before this point.
• Daemon (agent_session_service.rs): on Lagged(n), still warns and keeps draining, but now also yields a marker chunk with dropped_chunks: n. The stale TODO is removed.
• Tauri: PtyOutputPayload forwards droppedChunks.
• Desktop terminal (pty-terminal.svelte): shows [output truncated: N chunks dropped] in yellow, styled like the [Session closed] notice. The text comes from formatDroppedNotice.
• CLI (nodespace session): writes the same notice inline.

The unit is chunks rather than bytes because that is what broadcast reports. Each chunk is at most 4 KiB.

Unrelated fix, needed to get the pre-push gate green

methodology_linear_execution_test::rollover_moves_a_task_rather_than_leaving_it_in_both_cycles built end_date from Local::now(). CEL today() now reads Utc::now(), so on main the test fails every day between local midnight and UTC midnight in timezones east of UTC. It now uses UTC, in its own commit.

Tests

• New daemon gRPC integration test stream_output_reports_dropped_chunks_when_subscriber_lags. It floods 32 MiB through a real PTY while the client doesn't read, then asserts that a marker arrives with empty data. I confirmed it fails with the marker count forced to 0.
• New pty-terminal.test.ts: ordinary bytes are written unchanged, and a marker writes the truncation notice.
• The pre-push gate passed: test:all, daemon build, and e2e.

🤖 Generated with Claude Code

[malibio]

Pragmatic Code Review — PR #3057

Review Type: Initial Review (commits c5e58fb3, d2208372)

Requirements Check (#2985)

• ✅ Wire-level loss signal: OutputChunk gains uint64 dropped_chunks = 3. The field is additive, and proto3 defaults it to 0, so ordinary chunks need no special handling. The proto comment defines marker semantics (empty data, non-zero count).
• ✅ RecvError::Lagged surfaced to the client: the daemon now yields a marker chunk instead of continue, and the stale TODO(review) is removed.
• ✅ Terminal UI shows an "output truncated" indicator: Tauri forwards droppedChunks, and pty-terminal.svelte writes a yellow notice through formatDroppedNotice.
• ✅ All consumers updated: a grep shows exactly two StreamOutput clients (the Tauri command and CLI nodespace session). Both handle the marker. No other generated TS or proto consumers exist.
• ✅ Tests: a daemon gRPC integration test drives real lag through a PTY (the author confirmed it fails when the marker count is forced to 0), and a frontend component test covers the pass-through and marker paths.
• ✅ Unrelated test fix is justified: cel.rs today() reads the UTC clock, while origin/main still builds end_date from Local::now(). The test fails between local and UTC midnight. The commit is separate and small, and the push gate needs it.

Code Review Findings

🟢 Suggestion: notices and warn logs can repeat under sustained backpressure
📁 packages/daemon/src/services/agent_session_service.rs:270-287, packages/desktop-app/src/lib/components/agent/pty-terminal.svelte:52-58
After a Lagged, the receiver jumps to the oldest buffered chunk. If the consumer is still backpressured, the next few recvs can lag again, so markers get interleaved with data. Each marker produces two terminal lines, a daemon tracing::warn! and a frontend log.warn. The Tauri reader task drains quickly, so this is rare in practice. The flood test shows the conditions can still occur. If it ever looks noisy, coalesce the markers: sum skipped into a pending counter and flush one marker at most every N ms, or have the frontend suppress a second notice until non-marker output has rendered. Principle: signal-to-noise. One notice per burst tells the user what happened; a notice per chunk hides it. Not needed for this PR.

🟢 Suggestion: the notice clears the running program's text attributes
📁 packages/desktop-app/src/lib/components/agent/pty-output.ts:9, packages/cli/src/commands/session.rs:226
\x1b[0m resets whatever colors and attributes the program had active. In a full-screen alternate-screen TUI (the main PTY workload here is coding-agent TUIs), the \r\n lines are also written onto the TUI surface. The dropped bytes have already desynced the stream, so this adds little extra damage, and the next repaint fixes it. On partial escape sequences: xterm aborts an in-progress CSI when the notice's ESC arrives, and pending partial UTF-8 bytes turn into a replacement character on the next byte write. Both are acceptable given data is already missing. A longer-term option is an out-of-band UI badge (e.g. a "truncated" chip on the terminal header) instead of in-band text. Principle: keep presentation separate from the program's byte stream. Fine as is for now.

🟢 Suggestion: integration test timing
📁 packages/daemon/tests/agent_session_grpc.rs:417-470
I think the test is robust. 32 MiB of input becomes about 48 MiB after ONLCR, which far exceeds 256×4 KiB plus the HTTP/2 window. A late subscribe cannot make it pass falsely, because lag only needs the flood to still be running when the client stops reading. Closed arrives only after the buffered Lagged. The fixed sleep(2s) does add 2 s to every run of the suite and is the only mechanism creating backpressure. That is acceptable, but a comment saying the sleep is the backpressure (not just "time to flood") would stop a future edit from "optimizing" it away. The test could also assert dropped_chunks == 0 on an ordinary chunk to cover the negative case.

🟢 Nit: notice text exists in two languages
📁 packages/desktop-app/src/lib/components/agent/pty-output.ts, packages/cli/src/commands/session.rs:217-227
The same string is written in both TS and Rust. There is no shared runtime across that boundary, and the proto field is the actual contract, so this is an acceptable DRY exception. If the wording is ever meant to stay in sync, a cross-reference comment in each would help.

🟢 Nit: dropped_notice has no unit test
📁 packages/cli/src/commands/session.rs:217-227
It is trivial, but a two-line test for singular/plural and the leading \r\n would match the coverage on the TS side.

Positives

• The daemon keeps its "keep draining, don't tear down" behavior and adds only the signal. The change is small and correct.
• Marker semantics are documented in the proto, which is where every client reads the contract.
• The frontend test mocks at the right seams (xterm, Tauri listen, logger) and checks behavior, not implementation.
• Standards: no lint suppression, no any, createLogger used, no issue numbers in comments, no compat shims.

Recommendation

APPROVE. The PR fully resolves #2985 with a minimal, additive contract change, all consumers updated, and a real regression test. The suggestions above are optional polish.

[malibio]

Automated pragmatic-code-review: APPROVE (posted as a comment — GitHub blocks self-approval). Full findings in the review comment above.

[malibio]

Review addressed (6d0afb1)

✅ Addressed (3)

• Lag test: added a comment explaining that the 2 s no-read sleep is what creates the backpressure.
• Notice text: added cross-reference comments in pty-output.ts and the CLI's dropped_notice so the wording stays in sync.
• CLI: added a dropped_notice unit test covering the leading fresh line and singular/plural.

⏭️ Skipped (2)

• Coalescing repeated markers: repeated lag is speculative, because the Tauri reader drains quickly. Revisit if repeated notices show up in practice.
• Avoiding the SGR reset / a header badge: the notice follows the existing [Session closed] pattern, and the next TUI repaint fixes the screen. A header badge would be a separate UI change.

Re-review: not needed. The changes are comments plus one small unit test, with no logic change.