Conversation
The previous render_table_row truncated cell content with `…` whenever a cell's display width exceeded `(terminal_width - 7) / num_cols`. In narrow terminals or with verbose English/Chinese instructional tables (common in LLM responses), users would see only the first ~30 characters of meaningful content per cell with the rest silently lost — not visible by scrolling, not recoverable. Replace the truncation with a word-wrapping renderer that preserves the full cell content across multiple visual lines while keeping the column separators (`│`) aligned on every wrapped continuation line. The row's visual height becomes the height of the tallest column; shorter columns get blank-padded continuation rows so column edges stay aligned. Algorithm: - wrap_cell_text splits on whitespace and packs words greedily until the next word wouldn't fit; words wider than col_width are hard-broken at character boundaries so wrapping always makes progress (URLs, paths). - render_table_row pre-wraps every cell, computes the row height as max(cell_segments_len), then emits N visual lines with each cell's segment-or-empty padded to col_width and separated by `│`. Adds two regression tests covering: long cells preserve content (no `…`) and wrapped continuation lines retain column separators.
Extract the common "re-arm keyboard enhancement, mouse capture, bracketed paste, and focus events" sequence used by app startup, FocusGained recovery, and resume_terminal into a single best-effort helper. The FocusGained handler previously only re-pushed keyboard enhancement flags + (after v0.8.24) mouse capture, missing bracketed paste and focus events. Each new mode added at startup was a fresh gap waiting to be discovered when a user Cmd+Tabbed away. Co-locating the four flags in one canonical helper means future mode additions are one edit, not three. Adds a unit test that pins the gating (mouse + bracketed paste honor their booleans; keyboard + focus are unconditional) by writing into an in-memory buffer and asserting on the CSI sequences crossterm emits. Test: cargo test -p deepseek-tui --bin deepseek-tui --locked
There was a problem hiding this comment.
Code Review
This pull request refactors the CLI self-update mechanism to use reqwest and an aggregated checksum manifest, removes unused per-model context configurations, and integrates the recall_archive tool into the parent engine. Additionally, the MCP transport layer now handles raw bytes, and markdown table rendering has been enhanced with word-wrapping capabilities. Review feedback highlights a potential regression for Windows users where the update process might fail due to certificate revocation checks in reqwest. There is also a concern that removing JSON validation in the MCP transport layer could lead to connection failures when servers emit non-JSON output.
| fn update_http_client() -> Result<reqwest::blocking::Client> { | ||
| reqwest::blocking::Client::builder() | ||
| .user_agent(UPDATE_USER_AGENT) | ||
| .build() | ||
| .context("failed to build update HTTP client") | ||
| } |
There was a problem hiding this comment.
Switching from curl to reqwest for the self-update logic removes the ability to bypass certificate revocation checks on Windows (previously handled via --ssl-no-revoke). If the project's reqwest configuration uses the default native-tls backend, it will rely on Schannel, which performs mandatory OCSP/CRL checks. In environments where these checks are blocked (e.g., behind certain corporate firewalls), the update process will fail. Since this was an explicit fix in the previous implementation, consider ensuring rustls is used for the update client or providing a way to opt-out of revocation checks if native-tls is retained.
| if let Ok(value) = serde_json::from_str::<serde_json::Value>(trimmed) { | ||
| return Ok(value); | ||
| } | ||
| return Ok(trimmed.as_bytes().to_vec()); |
There was a problem hiding this comment.
The removal of the JSON validation check in StdioTransport::recv (and similarly in parse_sse_message_data) introduces a regression in robustness. Previously, non-JSON lines—such as debug logs or startup messages printed by some MCP servers to stdout—were silently ignored. Now, any non-empty line is returned as a message, which will cause McpConnection::recv to fail when it attempts to deserialize the bytes as JSON, likely terminating the connection. If performance is the concern, consider a lightweight check (e.g., ensuring the line starts with {) or restoring the validation to skip 'noisy' output from servers.
The "message" arm wrapped its work in `if !data.trim().is_empty()`, which clippy::collapsible_match flags. Move the predicate into a match guard so the inner branch is the only body. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Crossterm routes the same logical commands through the WinAPI console backend on Windows, so EnableFocusChange / Push keyboard flags / EnableMouseCapture / EnableBracketedPaste never reach the writer as ANSI bytes there. Gate the byte-level test with cfg(not(windows)) and add a windows-only smoke test that just exercises the function for panic-freedom. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Scope
This PR rolls up two related v0.8.25 efforts on
work/v0.8.25:Stabilization (Tier 1 from v0.8.25 handoff)
wrap_cell_texthelper + row-height-aware row rendering replace…truncationupdate.rsreqwest migration — drops thecurlshellout (and the Schannel--ssl-no-revokeWindows hack) forreqwest::blockingover rustls, plus actually-honored SHA256 verification from the aggregateddeepseek-artifacts-sha256.txtmanifestrecover_terminal_modes()is the canonical path used at startup, onFocusGained, and fromresume_terminal, so new mode flags only have to be wired in one placeDrift fixes (codemap follow-ups)
cycle_manager.rs[cycle.per_model]comments (no behavior change)ContextConfig.per_modelfield while preserving old-config load compatibility (serde ignores unknown keys; covered by a new test)recall_archivein the parent tool registry for Plan, Agent, and YOLO modes (read-only, investigative-friendly)CI fixes
"message"match arm into a guard to satisfyclippy::collapsible_matchrecover_terminal_modesANSI assertions to non-Windows targets (crossterm routes those commands through the WinAPI console backend on Windows, so no ANSI bytes reach the writer); add a Windows-only smoke test that exercises the function for panic-freedomVerification
cargo fmt --checkcargo clippy --workspace --all-targets --all-features -- -D warningscargo test --workspace --all-features(one pre-existing flaky HTTP integration test under heavy parallel load only; passes in isolation and in CI)cargo test -p deepseek-tui parent_turn_registry_includes_recall_archive_for_investigative_modes --all-featurescargo test -p deepseek-tui removed_context_per_model_table_is_ignored_for_compatibility --all-featurescargo test -p deepseek-tui --bin deepseek-tui recover_terminal_modes --all-featuresOut of scope (deferred to v0.8.26)
▏glyph in code blocks (White vertical line (▏) appears in code blocks after v0.8.20 update #1212), mouse selection crossing sidebar (When selecting text with the mouse, the selection area crosses the sidebar and also selects content in the right sidebar. No isolation is implemented, which is not expected behavior. #1169)Notes
recall_archivebecause the tool isReadOnlyand investigative-friendly; the existing Plan read-only registry test was updated to assert membership while still rejecting write/exec tools.🤖 Generated with Claude Code