[codex] v0.8.50 triage harvest#2504
Conversation
(cherry picked from commit cb22c7b)
(cherry picked from commit fb77cf1)
…ility When MCP servers return tool schemas, the field order within each schema object and the order of entries in required / dependentRequired arrays can vary across reconnections. This causes the serialized tool catalog bytes to change even when the logical schema is unchanged, busting DeepSeek's KV prefix cache. Add schema_canonicalize::canonicalize_schema which recursively: - Sorts every required array alphabetically - Sorts every dependentRequired sub-array alphabetically - Rebuilds object keys in alphabetical order - Recurses into all nested objects and arrays The canonicalize step runs after schema_sanitize in build_api_tools, so each tool's input_schema is first cleaned then byte-stabilized. The existing OnceLock api_cache pins the result, ensuring the tool catalog bytes are identical across reads and across process restarts. 8 unit tests cover: required sorting, dependentRequired sorting, equivalent-ordering byte match, recursive nesting, empty schemas, deeply nested schemas, non-required array preservation, and key ordering. (cherry picked from commit 7cee9cd)
(cherry picked from commit 6cdea32)
(cherry picked from commit 3ab06d9)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several updates, including support for an optional sibling permissions.toml file for ask-only typed permission rules, byte-level JSON Schema canonicalization for prefix-cache stability, and Windows job object integration to prevent orphaned descendant processes. It also refactors the state store migration query to correctly handle same-second messages. Feedback is provided regarding the migration query, which may suffer from messages tables if an appropriate index is missing.
| SET parent_entry_id = ( | ||
| SELECT m2.id | ||
| FROM messages m2 | ||
| WHERE m2.created_at < messages.created_at AND m2.thread_id = messages.thread_id | ||
| ORDER BY m2.id DESC | ||
| WHERE m2.thread_id = messages.thread_id | ||
| AND ( | ||
| m2.created_at < messages.created_at | ||
| OR ( | ||
| m2.created_at = messages.created_at | ||
| AND m2.id < messages.id | ||
| ) | ||
| ) | ||
| ORDER BY m2.created_at DESC, m2.id DESC | ||
| LIMIT 1 | ||
| ); |
There was a problem hiding this comment.
The correlated subquery used to update parent_entry_id performs a lookup on the messages table for every single row during migration. If the messages table contains a large number of rows and lacks an index on (thread_id, created_at, id), this migration will have
Consider ensuring that an index on (thread_id, created_at, id) exists before running this migration, or creating a temporary index specifically for this migration and dropping it afterwards to optimize performance.
(cherry picked from commit 5f01dda)
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Thanks for harvesting #2498 into this v0.8.50 slice. I compared the current #2504 branch with the final #2498 head (96adffb) and noticed that #2504 appears to include only the earlier Windows shell commits: The final review-driven #2498 head also includes two later commits that are important for the 5/5-confidence version:
The second one is not only tests: it applies the close-before-reader-join fallback to the foreground execute_sync_sandboxed path as well as the background path. Without it, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE can still be unreachable in the foreground stdout/stderr reader join path if explicit job termination fails. Can you please harvest the final #2498 head (96adffb) or apply the equivalent diff before merging #2504? Paulo Aboim Pinto |
Override `supports_parallel()` to return `true` in `WebSearchTool`, allowing the engine to batch multiple concurrent web_search calls into a `FuturesUnordered` parallel group instead of serializing them. The tool is already read-only, auto-approved, and non-interactive — parallel-safe by all other criteria. This change removes the final gate (`supports_parallel() -> false` default) so co-issued searches run concurrently rather than one-at-a-time. Closes the ~55s serial wall-clock for 3 simultaneous web searches (now ~20s, the slowest individual call). Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit a7dcf63)
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
There was a problem hiding this comment.
Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Thanks @aboimpinto. Confirmed your read was correct: #2504 had only the first two #2498 commits at the time of your comment. I have now harvested the remaining final #2498 commits into #2504:
Local verification on the updated harvest branch:
The branch is pushed; Windows CI is the platform proof for the new Windows-only tests. |
…ltiple_of, dead unwrap)
This reverts commit 6bc5e62.
|
Too many files changed for review. ( |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Users copying text from the output area and right-clicking in the composer expect Paste to be the first, most accessible action. Previously Paste appeared after all cell-specific actions (Open Details, Copy Message, etc.), requiring extra mouse travel or keyboard navigation. Reported by a WeChat/Chinese UX user during the v0.8.50 triage pass.
|
Thanks for harvesting these into the v0.8.50 branch, and for keeping the individual PR references/credit in the notes. I’ll keep the remaining follow-ups narrow and issue-scoped. |
10 participants
Summary
v0.8.50 triage harvest after v0.8.48/v0.8.49 release stewardship. This branch does not tag, publish, create a GitHub Release, or republish
deepseek-tui.Newly Harvested In This Push
exec_shellreader drains so timeout/pipe-handle cleanup cannot wedge later tools behind the global tool lock. Local harvest also covers normal-exit inherited-pipe cleanup with a foreground orphan-pipe regression.CodeWhaleSetup.exefrom the real Windows binaries, the installer compiles under NSIS 3.12, docs use live asset names, and PATH add/remove is exact-entry based. Refs Feature Request: NSIS Windows Installer for CodeWhale #1987./contextuses the effective routed model window, so DeepSeek V4 routes show the 1M-token budget while legacy DeepSeek routes keep the 128K fallback.--versionprefers the installed binary version before falling back to package metadata.vendor/model-idpass-through, without freezing a volatile provider catalog or blanket capability flags.Additional Local Fixes In This Push
baltobalance, after Incorrect total cost display in TUI: shows much higher amount than actual DeepSeek platform consumption (possible saved/cache hit calculation bug) #2567 showed it could be confused with session spend.Earlier Harvested Work Still In Branch
Includes #2476, #2491, #2499, #2498, #2505, #2509, #2511, #1722/#1723, #2529, #2538, #2530, #2540, #2524, #2534, #2516, #2552, #2553, #2550, #2549, #2547, #2526, #2555, #2551, and #2557. Contributor authorship and relevant reporter credit are preserved in commits, changelog, and maintainer comments.
Local Verification
cargo fmt --all -- --check./scripts/release/check-versions.shbash -n scripts/release/check-published.shgit diff --checknode --test npm/codewhale/test/run.test.jsmakensis -DVERSION=0.8.50 scripts/installer/codewhale.nsiwith staged dummy binariescargo test -p codewhale-agent --all-features --locked atlascloud -- --nocapturecargo test -p codewhale-agent --all-features --lockedcargo clippy -p codewhale-agent --all-targets --all-features --locked -- -D warningscargo test -p codewhale-tui --all-features --locked foreground_shell_does_not_block_on_orphaned_subprocess_pipe -- --nocapturecargo test -p codewhale-tui --all-features --locked test_orphaned_subprocess_does_not_block_collect_output -- --nocapturecargo test -p codewhale-tui --all-features --locked test_timeout -- --nocapturecargo test -p codewhale-tui --all-features --locked test_exec_shell_foreground_timeout_guides_background_rerun -- --nocapturecargo clippy -p codewhale-tui --all-targets --all-features --locked -- -D warningscargo test -p codewhale-tui --all-features --locked composer_arrow -- --nocapturecargo test -p codewhale-tui --all-features --locked footer_balance -- --nocapturecargo test -p codewhale-tui --all-features --locked v4_pro -- --nocapturecargo test -p codewhale-tui --all-features --locked cost_estimate -- --nocapturecargo test -p codewhale-tui --all-features --locked context_inspectorcargo test -p codewhale-tui --all-features --locked truncated_subagentcargo test -p codewhale-config --all-features --locked migrate_config_reports_copied_legacy_pathcargo test -p codewhale-tui --all-features --locked absolute_updated_timestampcargo test -p codewhale-tui --all-features --locked current_modelcargo test -p codewhale-tui --all-features --locked turn_metadatacargo test -p codewhale-config --all-features --lockedorigin/maincompile check forcodewhale-tuifails to compile onmain: methodlines_with_copy_metadata_foldednot found on&HistoryCell#2570:cargo check -p codewhale-tui --lockedat31f34c5dcargo install --path crates/cli --locked --forceandcargo install --path crates/tui --locked --force; verifiedcodewhale,codewhale-tui,codew,deepseek, anddeepseek-tuireport 0.8.50 from PATH, with legacy shim warnings intact.Recent Triage Notes
4a0919743; commented with exact delta and verification.buglabel.codewhale-tuifails to compile onmain: methodlines_with_copy_metadata_foldednot found on&HistoryCell#2570: closed as not reproducible/current-main-valid after cleanorigin/maincompile evidence.Instantnext steps.Skipped Or Still Needs Follow-Up
path_suffix: useful idea, but current revision applies the suffix too broadly. Commented with exact next steps.External Blocker
npm view deepseek-tui deprecatedstill returns an empty value. Registry deprecation for the legacy package still needs npm OTP/browser auth. Do not republishdeepseek-tui; DeepSeek provider/model support remains first-class.