refactor: phase 1 shell dedup foundation#151
Merged
benvinegar merged 6 commits intomainfrom Feb 24, 2026
Merged
Conversation
Greptile SummaryExtracted shared shell script helpers to reduce duplication across update, rollback, doctor, and security-audit scripts. Created three new library modules:
The refactoring successfully eliminates ~127 lines of duplicated code while maintaining behavioral compatibility. All changes are well-structured and improve maintainability. Tests are provided for Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Shell Scripts] --> B[bin/update-release.sh]
A --> C[bin/rollback-release.sh]
A --> D[bin/security-audit.sh]
A --> E[bin/doctor.sh]
B --> F[bin/lib/release-runtime-common.sh]
C --> F
B --> G[bin/lib/shell-common.sh]
C --> G
D --> G
E --> G
D --> H[bin/lib/check-report-common.sh]
E --> I[bin/lib/doctor-common.sh]
I --> H
F -->|restart_and_health| J[bb_run_release_restart_and_health]
F -->|version_check| K[bb_verify_deployed_release_sha]
G -->|arg_parsing| L[bb_require_option_value]
G -->|arg_parsing| M[bb_has_arg]
H -->|counters| N[bb_counter_reset_many]
H -->|counters| O[bb_counter_inc]
H -->|display| P[bb_summary_print_*]
style F fill:#e1f5ff
style G fill:#e1f5ff
style H fill:#e1f5ff
style I fill:#fff4e1
Last reviewed commit: e8d7e9e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+87
to
+108
| bb_require_option_value() { | ||
| local option="$1" | ||
| local argc="$2" | ||
|
|
||
| if [ "$argc" -lt 2 ]; then | ||
| bb_die "$option requires a value" | ||
| fi | ||
| } | ||
|
|
||
| bb_has_arg() { | ||
| local needle="$1" | ||
| shift || true | ||
|
|
||
| local arg | ||
| for arg in "$@"; do | ||
| if [ "$arg" = "$needle" ]; then | ||
| return 0 | ||
| fi | ||
| done | ||
|
|
||
| return 1 | ||
| } |
There was a problem hiding this comment.
Consider adding unit tests for these new helpers (bb_require_option_value and bb_has_arg). Other lib files with exported functions have corresponding .test.sh files.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: bin/lib/shell-common.sh
Line: 87-108
Comment:
Consider adding unit tests for these new helpers (`bb_require_option_value` and `bb_has_arg`). Other lib files with exported functions have corresponding `.test.sh` files.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Member
Author
There was a problem hiding this comment.
Clean follow-up: Added tests in bin/lib/shell-common.test.sh for bb_has_arg and bb_require_option_value, and wired them into test/shell-scripts.test.mjs so they run in npm run test:shell.
Responded by dev-agent using openai/gpt-5.
Comment on lines
+1
to
+74
| #!/bin/bash | ||
| # Shared runtime helpers for release update/rollback scripts. | ||
| # | ||
| # Expects caller to provide: | ||
| # - log() and die() functions | ||
| # - restart_baudbot_service_if_active() | ||
| # - json_get_string_stdin() | ||
| # - BAUDBOT_AGENT_USER and BAUDBOT_AGENT_HOME | ||
|
|
||
| bb_run_release_override_cmd() { | ||
| local description="$1" | ||
| local command="$2" | ||
| local env_array_name="$3" | ||
|
|
||
| [ -n "$command" ] || return 1 | ||
|
|
||
| local -n env_ref="$env_array_name" | ||
| log "running $description override" | ||
| env "${env_ref[@]}" bash -lc "$command" | ||
| } | ||
|
|
||
| bb_run_release_restart_and_health() { | ||
| local restart_cmd="$1" | ||
| local skip_restart="$2" | ||
| local health_cmd="$3" | ||
| local env_array_name="$4" | ||
|
|
||
| if ! bb_run_release_override_cmd "restart" "$restart_cmd" "$env_array_name"; then | ||
| if [ "$skip_restart" = "1" ]; then | ||
| log "skipping restart" | ||
| else | ||
| restart_baudbot_service_if_active | ||
| fi | ||
| fi | ||
|
|
||
| bb_run_release_override_cmd "health" "$health_cmd" "$env_array_name" || true | ||
| } | ||
|
|
||
| bb_verify_deployed_release_sha() { | ||
| local expected_sha="$1" | ||
| local skip_version_check="$2" | ||
| local verified_label="${3:-}" | ||
|
|
||
| if [ "$skip_version_check" = "1" ]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| if [ "$(id -u)" -ne 0 ]; then | ||
| log "non-root run: skipping deployed version verification" | ||
| return 0 | ||
| fi | ||
|
|
||
| if ! id "$BAUDBOT_AGENT_USER" >/dev/null 2>&1; then | ||
| log "agent user '$BAUDBOT_AGENT_USER' missing; skipping deployed version verification" | ||
| return 0 | ||
| fi | ||
|
|
||
| local version_file="$BAUDBOT_AGENT_HOME/.pi/agent/baudbot-version.json" | ||
| local deployed_sha | ||
|
|
||
| deployed_sha="$(sudo -u "$BAUDBOT_AGENT_USER" sh -c "cat '$version_file' 2>/dev/null" | json_get_string_stdin "sha" 2>/dev/null || true)" | ||
|
|
||
| if [ -z "$deployed_sha" ]; then | ||
| die "deployed version file missing or unreadable: $version_file" | ||
| fi | ||
|
|
||
| if [ "$deployed_sha" != "$expected_sha" ]; then | ||
| die "deployed sha mismatch (expected $expected_sha, got $deployed_sha)" | ||
| fi | ||
|
|
||
| if [ -n "$verified_label" ]; then | ||
| log "deployed version verified: $verified_label" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Consider adding unit tests for release-runtime-common.sh. These functions contain critical release logic (restart, health checks, version verification) that would benefit from test coverage.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: bin/lib/release-runtime-common.sh
Line: 1-74
Comment:
Consider adding unit tests for `release-runtime-common.sh`. These functions contain critical release logic (restart, health checks, version verification) that would benefit from test coverage.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Member
Author
There was a problem hiding this comment.
Added coverage in a new bin/lib/release-runtime-common.test.sh and wired it into test/shell-scripts.test.mjs.
The tests cover restart/health override behavior, including failure propagation and default restart fallback behavior when no override is provided.
Responded by dev-agent using openai/gpt-5.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
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.
Summary
Files
bin/lib/release-runtime-common.shbin/lib/check-report-common.shbin/lib/check-report-common.test.shbin/update-release.shbin/rollback-release.shbin/security-audit.shbin/lib/doctor-common.shbin/doctor.shbin/lib/shell-common.shtest/shell-scripts.test.mjsValidation
npm run test:shellbash bin/update-release.test.shbash bin/rollback-release.test.shbash -non modified scriptsNotes
npm run lint:shellis currently blocked in this environment becauseshellcheckis not installed in PATH.