fix(scripts): preserve successful peer query exit status - #703
Merged
Merged
Conversation
Collaborator
|
Ok sure ... our scripts really are dev tools, and rough |
n13
approved these changes
Sep 19, 2026
n13
left a comment
Collaborator
There was a problem hiding this comment.
Reviewer model: GPT 5.6 Sol
APPROVE — the assignment-form arithmetic fixes both set -e exit paths without changing the intended counting behavior.
No blocking findings. success_count=$((success_count + 1)) cannot inherit the zero result status of the old post-increment command, and total_peers=$((total_peers + peer_count)) likewise keeps a valid zero-peer response from terminating text mode. The regression coverage exercises JSON/text success, zero peers, multiple hosts, both partial-failure orders, and the all-failed status.
Validation at a0c9c8c270d3d56d8948ebf5b4aabe115313a6eb:
python3 -B scripts/testing/test_query_peers.py— 6 tests passed.bash -n scripts/query_peers.sh— passed.git diff --check 662ef6d1dea8f3572776b8db9da1f57283744fd2...a0c9c8c270d3d56d8948ebf5b4aabe115313a6eb— passed.- GitHub format, dependency-cooldown, and Clippy/doc checks passed; the Linux and macOS build/test matrix jobs were still running at review time.
Collaborator
|
Safe to merge - just the script bugfix |
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
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.
Summary
Fixes #702.
Use arithmetic expansion assignments for the successful-host count and total peer count in
scripts/query_peers.sh. Unlike standalone arithmetic commands, these assignments do not return failure when the expression evaluates to zero underset -e.This prevents both the first successful query from terminating the script and a successful text-mode query with zero peers from triggering the same early-exit behavior. All-failed queries continue to return exit status 1.
Tests
python3 scripts/testing/test_query_peers.py— 6 tests passed, covering JSON/text output, zero peers, multiple hosts, partial success in both host orders, and all-failed queries.bash -n scripts/query_peers.sh— passed.git diff --check— passed.The regression suite runs the actual project script with only
curlstubbed;bashandjqremain real dependencies. No live node or public network is used. Run the Python suite explicitly; it is not added to the Rust CI workflow.Scope and risk
Two shell expressions changed, plus an offline regression test file. No node/runtime code, RPC protocol, or dependency changes. No full Rust build was needed or run for this shell-only fix.
AI assistance
AI assistance was used to investigate, implement, and test this change. Test results above come from actual local execution.