fix: prepare v0.8.38 bug-bash fixes#1647
Conversation
Harvests the cleanup-guard portion of PR #1630 by @duanchao-lab while preserving provider-aware onboarding startup.
There was a problem hiding this comment.
Pull request overview
Bundles five v0.8.38 bug-bash fixes spanning the Rust TUI, npm wrapper, and marketing site: it tightens OpenAI-compatible chat payloads, prevents pnpm postinstall hangs, restores terminal modes on early TUI exits via an RAII guard, preserves full OSC 8 link targets across line wraps, and refreshes the website's update guidance.
Changes:
- Strip Anthropic-only tool metadata (
allowed_callers,defer_loading,input_examples) and stop sending top-levelthinkingto Fireworks; keepreasoning_effortfor OpenAI-compat shape. - Skip optional install-time downloads when invoked under pnpm's optional postinstall, deferring binary fetch to first run; add a
TerminalCleanupGuardso early TUI bail-outs restore raw mode/alt-screen/mouse/bracketed-paste/keyboard flags/cursor. - Refactor markdown wrap logic into
InlineToken { text, style, link_url }so each wrapped chunk re-emits the full OSC 8 hyperlink target; update homepage/install page to surfacedeepseek updateplus package-manager fallbacks.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/tui/src/client.rs | Split Fireworks out of the Anthropic-style thinking branches in apply_reasoning_effort; add tests for OpenAI-compatible shape and stripped tool metadata. |
| crates/tui/src/client/chat.rs | Drop allowed_callers/defer_loading/input_examples from the chat-completions tool wire shape (Anthropic-only fields). |
| crates/tui/src/tui/ui.rs | Introduce TerminalCleanupGuard after terminal init and defuse it on the normal cleanup path so early returns still restore terminal modes. |
| crates/tui/src/tui/markdown_render.rs | Replace (String, Style) tuples with InlineToken carrying the link URL so wrapped chunks re-wrap with the full OSC 8 target; add wrap regression test. |
| npm/deepseek-tui/scripts/install.js | Add shouldSkipOptionalPostinstall to bypass install-time download under pnpm optional postinstall; first-run downloader still fetches the binary. |
| npm/deepseek-tui/test/postinstall.test.js | Cover pnpm-vs-npm UA, runtime-vs-install context, and --optional flag combinations for the new gate. |
| CHANGELOG.md | Document the four fixes plus the website update guidance and credit #1630's idea. |
| web/app/[locale]/page.tsx | Replace npm/registry snippets with a concise install + deepseek update block on the homepage. |
| web/app/[locale]/install/page.tsx | Insert a new "Update" section (deepseek update plus brew/npm/cargo equivalents) and renumber subsequent sections. |
| web/lib/facts.generated.ts | Regenerated generatedAt timestamp. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request introduces several improvements and fixes, including a refactored markdown renderer to ensure OSC 8 links maintain their full target across wrapped lines, a new TerminalCleanupGuard to reliably restore terminal modes on exit, and stricter request body formatting for OpenAI-compatible providers like Fireworks. It also optimizes pnpm installations by skipping binary downloads during postinstall and updates the website documentation to highlight the update command. Review feedback focused on improving the robustness of these changes, specifically suggesting that spaces within links should inherit the link URL to maintain clickability, and recommending that the terminal cleanup guard be initialized earlier and used more consistently to prevent resource leaks during initialization failures.
| // The space consumed by split — attach as a plain space word | ||
| // so the wrap loop can decide whether to keep or break it. | ||
| words.push((" ".to_string(), style)); | ||
| words.push(InlineToken::new(" ".to_string(), token.style, None)); |
There was a problem hiding this comment.
Spaces within a link token should inherit the link_url from the parent token. Currently, they are initialized with None, which breaks the clickability of the space character in links containing multiple words (e.g., [click here](url)). This results in a fragmented link where only the individual words are clickable, but the spaces between them are not. By passing token.link_url.clone(), the entire link span remains clickable even when wrapped.
| words.push(InlineToken::new(" ".to_string(), token.style, None)); | |
| words.push(InlineToken::new(" ".to_string(), token.style, token.link_url.clone())); |
| let mut cleanup_guard = TerminalCleanupGuard { | ||
| use_alt_screen, | ||
| use_mouse_capture, | ||
| use_bracketed_paste, | ||
| defused: false, | ||
| }; |
There was a problem hiding this comment.
The TerminalCleanupGuard should be initialized earlier in run_tui, ideally before any terminal state is modified (e.g., before enable_raw_mode() on line 246). In its current position, if an error occurs during terminal initialization (like EnterAlternateScreen failing on line 268), the guard is never created, and the terminal is left in raw mode. Moving the guard to the start of the function ensures that cleanup happens reliably for all early exit paths.
| persistence_actor::persist(PersistRequest::ClearCheckpoint); | ||
| persistence_actor::persist(PersistRequest::Shutdown); | ||
|
|
||
| cleanup_guard.defused = true; |
There was a problem hiding this comment.
Defusing the cleanup_guard here makes the subsequent manual cleanup block (lines 533-545) redundant and potentially unsafe. If any of those manual steps (like disable_raw_mode()?) fail, the remaining cleanup operations (leaving the alternate screen, disabling mouse capture, etc.) will be skipped because the guard is already defused. It is safer to remove the manual cleanup block entirely and let the guard handle it automatically when it goes out of scope, or at least move the defusal to the very end of the function after all cleanup steps have succeeded.
Summary
deepseek updateplus package-manager fallbacks)Fixes #1637.
Fixes #1592.
Fixes #1577.
Fixes #1593.
Fixes #1582.
Tests
Notes