fix: address code review feedback from post-release audit#106
Merged
Conversation
- query.rs: remove dead AsyncResponse.status field (was the source of the dead_code compiler warning; field was printed pre-polling but is unused now that the loop reads QueryRunResponse.status) - query.rs: reorder polling loop — poll first, then check deadline and sleep; eliminates the mandatory 500ms delay before the first check and ensures timeout is detected without an extra sleep cycle - skill.rs: add 120s timeout to download_and_extract_from_url HTTP client, matching the timeout used in perform_update; prevents an indefinite hang during hotdata update if the skills download stalls - main.rs: replace chained .unwrap() with .expect() in the databases tables help path for clearer panic messages if subcommand names change Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Instead of printing instructions and returning, run_update() now shells out to `brew upgrade <formula>` when a Homebrew install is detected. brew is located by checking common install paths (/opt/homebrew/bin, /usr/local/bin) before falling back to PATH. If brew is not found or the upgrade fails, falls back to printing the manual command. On success, busts the update-available cache so the notice clears on the next run, matching the behaviour of the non-Homebrew upgrade path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four fixes from the review of changes since v0.2.9, validated by both manual review and Codex CLI.
Changes
query.rs— remove deadAsyncResponse.statusfieldThis field was printed in the old exit-code-2 path but is unused since the polling loop was introduced. Was the source of the persistent
dead_codecompiler warning.query.rs— reorder polling loop (poll → match → deadline → sleep)The previous order (sleep → deadline → poll) added an unconditional 500ms delay before the first check and caused timeout detection to lag by up to 500ms. New order polls immediately on entry, checks deadline only after an in-progress result, then sleeps before the next iteration.
skill.rs— add 120s timeout to skills download HTTP clientdownload_and_extract_from_urlwas usingClient::new()with no timeout. Withinstall_for_versionnow called fromrun_update(), a stalled connection would hang the entire upgrade. Timeout matches the 120s used inperform_update.main.rs— replace chained.unwrap()with.expect()in tables help pathThree unwraps on string-matched subcommand lookups replaced with descriptive
.expect()messages.Not fixed (by design)
hotdata updateinstead ofbrew upgrade— intentional UX simplification, tracked separately.Generated with Claude Code