build: v0.8.47 — deadlock fix, composer text selection, project context tracing#2233
Conversation
…lock Replace `Arc<RwLock<()>>` in ToolCallRuntime with `Arc<Semaphore>` to eliminate the risk of a tool re-entering and deadlocking on the same lock. Parallel tools now acquire then immediately drop the permit, allowing concurrent execution after any in-flight serial tool finishes. Serial tools hold the permit for the full duration. Fixes #2157. Harvested from #1856. Co-authored-by: Fire-dtx <58944505+Fire-dtx@users.noreply.github.com>
Add mouse drag selection and Shift+Arrow text selection in the composer input box. Ctrl+C copies selected text; Ctrl+X cuts (or toggles mode if no selection). Selection highlighting uses the theme's selection_bg color. Mouse coordinate mapping accounts for wrapping, scroll offset, and padding. Also fix Home, End, Ctrl+A, and Ctrl+E to clear the selection anchor before jumping, matching the existing Left/Right behavior. Without these calls a stale anchor silently reforms a selection and can cause unintended deletions on the next keystroke. Harvested from #2228. Co-authored-by: imkingjh999 <imkingjh999@users.noreply.github.com>
Emit a tracing::info! line with the file path and byte size when a project context file (AGENTS.md, CLAUDE.md, etc.) is successfully loaded. This helps users and maintainers verify which file was used during prompt assembly, addressing the confusion reported in #2227.
Add entries for deadlock fix (#1856), composer text selection (#2228), and project context loading tracing (#2227). Add community credits for @Fire-dtx and @imkingjh999.
There was a problem hiding this comment.
Code Review
This pull request introduces text selection capabilities (copy, cut, mouse drag, and Shift+Arrow selection) in the TUI composer, adds logging for project context loading, and replaces RwLock with Semaphore in ToolCallRuntime to prevent deadlocks during concurrent sub-agent execution. The review feedback highlights critical concurrency and race condition issues with the new semaphore-based serialization, suggests reusing existing wrapping logic to avoid rendering discrepancies, and recommends defensively clamping selection indices to prevent potential out-of-bounds panics.
| if configured.supports_parallel_tool_calls { | ||
| let _guard = self.runtime.parallel_execution.read().await; | ||
| // Parallel tools wait for any in-flight serial tool to finish, | ||
| // but do not hold the permit so other parallel tools may run concurrently. | ||
| drop(self.runtime.serial_semaphore.acquire().await | ||
| .map_err(|_| FunctionCallError::Cancelled { name: call.name })?); | ||
| self.execute_with_timeout(handler, configured.spec.timeout_ms, invocation) | ||
| .await | ||
| } else { | ||
| let _guard = self.runtime.parallel_execution.write().await; | ||
| // Serial tools hold the semaphore for the full execution duration, | ||
| // preventing other serial AND parallel tools from starting. | ||
| let _permit = self.runtime.serial_semaphore.acquire().await | ||
| .map_err(|_| FunctionCallError::Cancelled { name: call.name })?; | ||
| self.execute_with_timeout(handler, configured.spec.timeout_ms, invocation) | ||
| .await | ||
| // _permit dropped here, releasing the semaphore. | ||
| } | ||
| } |
There was a problem hiding this comment.
Concurrency & Deadlock Issues with Semaphore-based Serialization
The replacement of RwLock with Semaphore(1) introduces two major correctness and concurrency issues:
-
Loss of Mutual Exclusion (Race Condition):
When a parallel tool executes, it acquires and immediately drops the permit (lines 395-396). Because the permit is released back to the semaphore, a serial tool can subsequently acquire the permit and run concurrently with the already-running parallel tool. This violates the core guarantee that serial tools must run exclusively and never concurrently with other tools. -
Re-entrant Deadlocks Still Exist:
If a serial tool is currently running, it holds the semaphore permit (line 402). If this serial tool makes a re-entrant call (e.g., by spawning a sub-agent that executes another tool), the child tool call will attempt to acquire the permit. Since the permit is held by the parent serial tool (which is waiting for the child to finish), this results in an immediate deadlock.
To properly support re-entrant tool calls while maintaining mutual exclusion, consider using a task-local tracking mechanism or a custom async re-entrant lock rather than a simple semaphore.
| let line_ranges = visible_line_char_ranges( | ||
| &self.app.input, | ||
| &visible_lines, | ||
| content_width, | ||
| scroll_offset, | ||
| ); |
There was a problem hiding this comment.
Redundant and Inconsistent Wrapping Logic
visible_line_char_ranges re-implements a custom wrapping and grapheme-width loop that is redundant and potentially inconsistent with wrap_input_lines_for_mouse (which uses wrap_text). Any discrepancy between these two wrapping implementations will cause the text selection highlight to be misaligned with the actual rendered text.
Instead of maintaining a separate wrapping loop, we can reuse wrap_input_lines_for_mouse directly to compute the visible line character ranges in a single, consistent way.
| let line_ranges = visible_line_char_ranges( | |
| &self.app.input, | |
| &visible_lines, | |
| content_width, | |
| scroll_offset, | |
| ); | |
| let all_ranges = wrap_input_lines_for_mouse(&self.app.input, content_width); | |
| let line_ranges: Vec<(usize, usize)> = all_ranges | |
| .into_iter() | |
| .skip(scroll_offset) | |
| .take(visible_lines.len()) | |
| .map(|(start, text)| (start, start + text.chars().count())) | |
| .collect(); |
| fn visible_line_char_ranges( | ||
| input: &str, | ||
| visible_lines: &[String], | ||
| width: usize, | ||
| scroll_offset: usize, | ||
| ) -> Vec<(usize, usize)> { | ||
| if input.is_empty() || width == 0 { | ||
| return vec![(0, 0); visible_lines.len()]; | ||
| } | ||
|
|
||
| let mut ranges = Vec::new(); | ||
| let mut char_idx = 0usize; | ||
| let mut line_start = 0usize; | ||
| let mut line_width = 0usize; | ||
|
|
||
| for g in input.graphemes(true) { | ||
| if g == "\n" { | ||
| ranges.push((line_start, char_idx)); | ||
| char_idx += 1; | ||
| line_start = char_idx; | ||
| line_width = 0; | ||
| continue; | ||
| } | ||
|
|
||
| let gw = g.width(); | ||
| if line_width + gw > width && line_width > 0 { | ||
| ranges.push((line_start, char_idx)); | ||
| line_start = char_idx; | ||
| line_width = 0; | ||
| } | ||
| char_idx += g.chars().count(); | ||
| line_width += gw; | ||
| if line_width >= width { | ||
| ranges.push((line_start, char_idx)); | ||
| line_start = char_idx; | ||
| line_width = 0; | ||
| } | ||
| } | ||
| ranges.push((line_start, char_idx)); | ||
|
|
||
| // Use the actual scroll_offset to align with visible_lines. | ||
| let start = scroll_offset.min(ranges.len()); | ||
| ranges | ||
| .into_iter() | ||
| .skip(start) | ||
| .take(visible_lines.len()) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
| pub fn selection_range(&self) -> Option<(usize, usize)> { | ||
| let anchor = self.selection_anchor?; | ||
| let cursor = self.cursor_position; | ||
| if anchor == cursor { | ||
| return None; | ||
| } | ||
| Some(if anchor < cursor { | ||
| (anchor, cursor) | ||
| } else { | ||
| (cursor, anchor) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Defensive Clamping of Selection Indices
self.selection_anchor and self.cursor_position are not guaranteed to be within the bounds of self.input if the input is modified or cleared externally (e.g., during history navigation or autocomplete) without resetting the selection.
To prevent potential out-of-bounds panics or incorrect slicing, clamp both indices to char_count(&self.input) inside selection_range.
| pub fn selection_range(&self) -> Option<(usize, usize)> { | |
| let anchor = self.selection_anchor?; | |
| let cursor = self.cursor_position; | |
| if anchor == cursor { | |
| return None; | |
| } | |
| Some(if anchor < cursor { | |
| (anchor, cursor) | |
| } else { | |
| (cursor, anchor) | |
| }) | |
| } | |
| pub fn selection_range(&self) -> Option<(usize, usize)> { | |
| let len = char_count(&self.input); | |
| let anchor = self.selection_anchor?.min(len); | |
| let cursor = self.cursor_position.min(len); | |
| if anchor == cursor { | |
| return None; | |
| } | |
| Some(if anchor < cursor { | |
| (anchor, cursor) | |
| } else { | |
| (cursor, anchor) | |
| }) | |
| } |
| { | ||
| let area = chunks[3]; | ||
| let has_panel = app.composer_border && area.height >= 3 && area.width >= 12; | ||
| let inner = if has_panel { | ||
| ratatui::widgets::Block::default() | ||
| .borders(ratatui::widgets::Borders::ALL) | ||
| .inner(area) | ||
| } else { | ||
| area | ||
| }; | ||
| app.viewport.last_composer_content = Some(inner); |
There was a problem hiding this comment.
has_panel border check is duplicated and can silently diverge
ComposerWidget::has_panel() (in widgets/mod.rs) is the canonical gate for whether the composer border renders. The new render-time block duplicates its condition (app.composer_border && area.height >= 3 && area.width >= 12) to compute last_composer_content. If the widget's threshold is ever changed, the mouse coordinate mapping will use the wrong inner rect without any compiler warning.
There was a problem hiding this comment.
🔴 Up/Down arrow keys don't clear selection_anchor, causing unexpected text deletion after history navigation
The PR adds app.clear_selection() for Left/Right/Home/End keys to prevent stale selections from causing accidental deletions. However, the Up/Down arrow handlers at crates/tui/src/tui/ui.rs:3592-3598 were missed. When a user creates a selection (Shift+Arrow), then presses Up/Down to navigate history, history_up()/history_down() at crates/tui/src/tui/app.rs:4511 replace self.input with history content without clearing selection_anchor. The next typed character calls insert_char() → delete_selection(), which finds the stale anchor and deletes an unexpected range of the new text. For example: type "hello world", Shift+Left×5 to select "world" (anchor=11, cursor=6), press Up to load "previous input" (14 chars, cursor=14, anchor still=11), type 'x' → delete_selection() removes chars 11–14 ("put") → result is "previous inx" instead of the expected "previous inputx".
(Refers to lines 3592-3598)
Was this helpful? React with 👍 or 👎 to provide feedback.
- Add resolve_project_state_dir and ensure_project_state_dir to
codewhale-config, providing project-local .codewhale/.deepseek
resolution matching the home-directory pattern.
- Migrate snapshot paths to prefer ~/.codewhale/snapshots with
~/.deepseek/snapshots fallback (snapshot_base_with_home).
- Migrate skill_state.rs to use codewhale_config::ensure_state_dir
instead of hardcoded home.join(".deepseek").
- Update doc comments to reference canonical .codewhale paths.
Part of #2231 (state-root migration).
Steered/queued user messages were being inserted into app.history before the active cell (holding streaming thinking/tool content) was flushed, causing the user's message to render above (before) the thinking block that chronologically preceded it. Now call app.flush_active_cell() before app.add_message() in steer_user_message(), matching the pattern used in MessageStarted and MessageDelta handlers. Fixes #2225.
The /save command now writes to ~/.codewhale/sessions (or legacy ~/.deepseek/sessions) instead of the workspace root. Update the test to set CODEWHALE_HOME to a temp directory and pre-create the sessions subdirectory so resolve_state_dir picks the primary path. Fixes the first failing test in #2223.
Update all three READMEs (en, zh-CN, ja-JP) to use the canonical ~/.codewhale paths for config, skills, and Docker volume mounts, with legacy ~/.deepseek noted as a compatibility fallback. The state-root migration has been underway since v0.8.44 — the docs now reflect it.
Add entries for steer message ordering fix, state-root migration progress, README path updates, and session save test fix.
- Spillover directory (truncate.rs): prefer ~/.codewhale/tool_outputs - Memory storage (capacity_memory.rs): prefer ~/.codewhale/memory - Runtime logs (runtime_log.rs): prefer ~/.codewhale/logs - Crash dumps (utils.rs): prefer ~/.codewhale/crashes - Automations (automation_manager.rs): prefer ~/.codewhale/automations - TUI settings (settings.rs): prefer ~/.codewhale/tui.toml All paths fall back to the legacy ~/.deepseek location when the canonical path doesn't exist, preserving compatibility for existing installs. Part of #2231.
…codewhale - HANDOFF_RELATIVE_PATH → .codewhale/handoff.md with .deepseek fallback - load_handoff_block reads both paths, prefers .codewhale - ToolContext notes_path and mcp_config_path use resolve_project_state_dir - Sub-agent state path prefers .codewhale/state/ - Cycle archive (recall_archive) uses resolve_state_dir for sessions - Compaction anchors path prefers .codewhale/anchors.md - Updated marker constants and comments Part of #2231.
- Add CODEWHALE_RELEASE_BASE_URL as canonical env override for release asset base URL (DEEPSEEK_TUI_RELEASE_BASE_URL and DEEPSEEK_RELEASE_BASE_URL remain as legacy fallbacks). - Add CODEWHALE_USE_CNB_MIRROR env var to auto-select the CNB (cnb.cool) mirror for binary downloads, avoiding GitHub Releases timeouts in China. - Update npm install scripts (artifacts.js) with the same env checks. - Update Rust self-updater (update.rs) with new constants and env cascade. Fixes #2222.
Harvested and vetted — no malware, no external deps, no injection: - #1859 (@harvey2011888): loop guard now reports Failed on halt - #1870 (@victorcheng2333): honour DEEPSEEK_YOLO env on startup - #1935 (@IIzzaya): replace [x] with [✓] completion markers - #1837 (@PurplePulse): fix macOS title centering (pin to top) - #1967 (@cyq1017): show base_url in /config view - #1906 (@knqiufan): copy transcript without visual-wrap newlines Also fix cycle_manager archive_dir_for to use resolve_state_dir so recall_archive tests pass with the migrated sessions path. Co-authored-by: victorcheng2333 <victorcheng2333@users.noreply.github.com> Co-authored-by: IIzzaya <IIzzaya@users.noreply.github.com> Co-authored-by: PurplePulse <PurplePulse@users.noreply.github.com> Co-authored-by: cyq1017 <cyq1017@users.noreply.github.com> Co-authored-by: knqiufan <knqiufan@users.noreply.github.com>
- .gitignore: add deep-swe/ and all_preds.jsonl to prevent accidental commits - config.rs: home_config_path(), managed_config, requirements, mcp, notes, memory all prefer ~/.codewhale/config.toml with .deepseek fallback - commands/config.rs: config_toml_path() prefers .codewhale - commands/anchor.rs: anchors_path prefers .codewhale/anchors.md - commands/note.rs: notes_path prefers .codewhale/notes.md - skills/install.rs: cache defaults to .codewhale/cache/skills - skills/mod.rs: global skills discovery includes .codewhale/skills - file_frecency, clipboard, onboarding, audit, task_manager: all .codewhale - project-local paths (onboarding trust) still .deepseek for compat Closes #2231.
- Line-wrap long function signatures and format arguments - Fix bracket placement for early returns (consistent style) - Use [!] instead of [✓] for network-denied skill sync - Fix copy-selection ordering: clear after success, not always
… execution - Use OwnedRwLockReadGuard for parallel-safe tools, OwnedRwLockWriteGuard for serial - Add TOOL_EXECUTION_LOCK_HELD task-local for reentrancy detection - Add BlockingHandler test harness and parallel-vs-serial concurrency tests
…ut length - Replace visible_line_char_ranges with wrap_input_lines_for_mouse for accurate mouse selection - Clamp selection_anchor and cursor_position to char_count - Clear selection on history navigation to prevent stale highlights - Add test for history-navigation-clears-stale-selection
…talog PR #2076 deferred planning/checklist tools (checklist_write, update_plan, task_create, task_list, task_read) to reduce catalog tokens, but the system prompt actively instructs the model to use these tools. Without them in the active catalog, the model cannot call them until it first discovers them via tool_search, which it is not prompted to do for planning tools. Keep these tools in DEFAULT_ACTIVE_NATIVE_TOOLS so the model can follow the Constitution's Regulations (Tier 3) and the Mode: YOLO instructions.
| if let Some(home) = dirs::home_dir() { | ||
| return home.join(".deepseek").join("tasks"); | ||
| return home.join(".codewhale").join("tasks"); | ||
| } | ||
| PathBuf::from(".deepseek").join("tasks") | ||
| PathBuf::from(".codewhale").join("tasks") |
There was a problem hiding this comment.
Existing tasks become invisible on upgrade — no
.deepseek fallback. Every other migrated path in this PR uses a "prefer .codewhale, fall back to .deepseek" pattern (anchor.rs, note.rs, snapshot/paths.rs, etc.), but default_tasks_dir switches unconditionally to .codewhale without checking whether .deepseek/tasks exists first. A user who upgrades with pending tasks stored in ~/.deepseek/tasks/ will see an empty task list, and those records won't surface again unless they manually move the directory. The docstring still reads "~/.deepseek/tasks" which is now wrong.
| if let Some(home) = dirs::home_dir() { | |
| return home.join(".deepseek").join("tasks"); | |
| return home.join(".codewhale").join("tasks"); | |
| } | |
| PathBuf::from(".deepseek").join("tasks") | |
| PathBuf::from(".codewhale").join("tasks") | |
| if let Some(home) = dirs::home_dir() { | |
| let primary = home.join(".codewhale").join("tasks"); | |
| if primary.exists() { | |
| return primary; | |
| } | |
| return home.join(".deepseek").join("tasks"); | |
| } | |
| PathBuf::from(".codewhale").join("tasks") |
v0.8.47 — 16 commits
Community PRs harvested (9)
Issues closed (10)
#2157 #2229 #2227 #2225 #2221 #2165 #2223 #2232 #2222 #2231
State-root migration (#2231 — ✅ complete)
All 30+ runtime filesystem paths now prefer ~/.codewhale with ~/.deepseek fallback.
Test results
Remaining for v0.8.47
Greptile Summary
This release bundles 16 commits and 9 community PRs, fixing a deadlock in concurrent sub-agent tool dispatch (RwLock → proper read/write guard with task-local reentrant detection), adding composer text selection with mouse drag and Shift+Arrow support, and continuing a large filesystem migration from
~/.deepseekto~/.codewhaleacross 30+ paths.crates/tools/src/lib.rs): parallel tools acquire a read guard (allowing overlap), serial tools acquire a write guard (full exclusion), and reentrancy within the same Tokio task bypasses the lock via atask_local!flag. Two new concurrency tests cover both the exclusivity contract and the reentrant case.app.rs,mouse_ui.rs,ui.rs):selection_anchoradded toComposerState; all editing operations correctly apply or clear the selection; mouse drag and Shift+Arrow produce consistent char-indexed ranges..codewhale, fall back to.deepseek" pattern is applied consistently across most paths, butdefault_tasks_dirandfile_frecency::default_pathswitch unconditionally to.codewhalewith no fallback, silently hiding any data left in the legacy directory after an upgrade.Confidence Score: 4/5
Safe to merge with the task-dir fallback issue noted — the deadlock fix and composer selection are correct, but upgrading users will silently lose access to existing tasks stored under ~/.deepseek/tasks.
The task manager's default directory switches to .codewhale without checking whether .deepseek/tasks exists first. Any tasks persisted by a prior release become invisible after upgrade, with no recovery path short of manual directory renaming. Every other migrated path in this PR checks the legacy location as a fallback; this one does not, making it an implementation gap against the stated migration strategy. The file-frecency store has the same omission but its impact is limited to ranking scores.
crates/tui/src/task_manager.rs (no legacy fallback, existing tasks hidden on upgrade) and crates/tui/src/core/capacity_memory.rs (both .codewhale and .deepseek memory dirs always searched when primary exists).
Important Files Changed
Reviews (6): Last reviewed commit: "chore: add DeepSWE task verification scr..." | Re-trigger Greptile