Skip to content

deploy: modularize bin shell scripts with shared helpers#109

Merged
benvinegar merged 4 commits into
mainfrom
bentlegen/shell-modularization-pass
Feb 22, 2026
Merged

deploy: modularize bin shell scripts with shared helpers#109
benvinegar merged 4 commits into
mainfrom
bentlegen/shell-modularization-pass

Conversation

@benvinegar
Copy link
Copy Markdown
Member

@benvinegar benvinegar commented Feb 22, 2026

Summary

  • add shared shell helper modules under bin/lib/
    • shell-common.sh for strict mode/logging/errors/root+env/systemd helpers
    • deploy-common.sh for deploy-specific identity/env-source helpers
    • doctor-common.sh for doctor counters/output summary
  • migrate core operational scripts to shared helpers:
    • bin/deploy.sh, bin/update-release.sh, bin/rollback-release.sh
    • bin/config.sh, bin/env.sh, bin/doctor.sh, bin/security-audit.sh
  • update docs (README.md, AGENTS.md) with shell module conventions

Commit structure

  1. deploy: add shared shell helper modules
  2. deploy: migrate bin scripts to shared helpers
  3. docs: document shared shell module conventions

Validation

  • bash -n on updated scripts/modules
  • bash bin/env.test.sh
  • bash bin/update-release.test.sh
  • bash bin/rollback-release.test.sh

@benvinegar
Copy link
Copy Markdown
Member Author

benvinegar commented Feb 22, 2026

Superseded by the corrected review-guide comment below (with proper code formatting): #109 (comment)

@benvinegar
Copy link
Copy Markdown
Member Author

benvinegar commented Feb 22, 2026

Review guide (commit-by-commit):

  1. deploy: add shared shell helper modules
  • Adds new shared modules only:
bin/lib/shell-common.sh
bin/lib/deploy-common.sh
bin/lib/doctor-common.sh
  • No migration changes in this commit.
  1. deploy: migrate bin scripts to shared helpers
  • Migrates scripts to shared helpers:
bin/deploy.sh
bin/update-release.sh
bin/rollback-release.sh
bin/config.sh
bin/env.sh
bin/doctor.sh
bin/security-audit.sh
  • Main review focus: behavior parity and duplication reduction.
  1. docs: document shared shell module conventions
  • Docs-only update:
README.md
AGENTS.md

Validation run on this branch:

bash -n <updated scripts/modules>
bash bin/env.test.sh
bash bin/update-release.test.sh
bash bin/rollback-release.test.sh

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Feb 22, 2026

Greptile Summary

This PR successfully modularizes shell scripts by extracting common patterns into shared helper modules under bin/lib/, reducing duplication across operational scripts.

Key changes:

  • Created three new shared modules: shell-common.sh (strict mode, logging, errors, root checks, systemd helpers), deploy-common.sh (identity resolution, env sourcing), and doctor-common.sh (test counters and summary output)
  • Migrated 7 bin scripts (deploy.sh, doctor.sh, env.sh, update-release.sh, rollback-release.sh, config.sh, security-audit.sh) to use shared helpers
  • Removed ~166 lines of duplicated code while adding 273 lines total (net +107 lines, but with significantly better maintainability)
  • Updated documentation in README.md with new "Shell script architecture" section and AGENTS.md conventions

Code quality improvements:

  • Consistent error handling via bb_die instead of ad-hoc patterns
  • Standardized strict mode activation via bb_enable_strict_mode
  • Unified user home resolution, env value reading, systemd detection, and root privilege checks
  • Better testability through modular functions

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • This is a well-executed refactoring that follows established patterns (similar to existing bin/lib/release-common.sh and json-common.sh). The changes are purely structural, extracting existing logic into reusable modules without altering behavior. Testing includes bash -n syntax validation and existing test suites (env.test.sh, update-release.test.sh, rollback-release.test.sh). The PR adheres to documented conventions in AGENTS.md about keeping shell CLIs thin and using shared helpers.
  • No files require special attention - all changes are straightforward refactoring

Important Files Changed

Filename Overview
bin/lib/shell-common.sh new shared helper module with strict mode, logging, error handling, root checks, systemd detection, user home resolution, and env file parsing utilities
bin/lib/deploy-common.sh new deploy-specific helpers for running commands as user, resolving deploy user identity, and sourcing env values from render script or config files
bin/lib/doctor-common.sh new doctor script helpers for test counters (pass/fail/warn) and formatted summary output with exit status handling
bin/deploy.sh migrated to use shared helpers: sources shell-common.sh and deploy-common.sh, replaced inline strict mode with bb_enable_strict_mode, uses bb_as_user, bb_resolve_deploy_user, bb_source_env_value, and bb_log functions
bin/doctor.sh migrated to shared helpers: sources shell-common.sh and doctor-common.sh, replaced inline counter variables and summary logic with doctor_init_counters, doctor_pass/fail/warn, doctor_summary_and_exit, uses bb_resolve_user_home, bb_read_env_value, and bb_has_systemd
bin/env.sh migrated to shared helpers: sources shell-common.sh, removed duplicate resolve_user_home and read_env_value functions in favor of bb_resolve_user_home and bb_read_env_value, replaced inline require_root with bb_require_root, uses bb_restart_systemd_service_or_die
AGENTS.md added shell module conventions to the Conventions section explaining the new shared helper pattern, bb_enable_strict_mode, and bb_* helper functions; repo layout section not updated to list new modules
README.md added new "Shell script architecture" section documenting the shared module pattern, listing all bin/lib/* modules, and explaining conventions for sourcing modules and using shared helpers

Last reviewed commit: aa8b7a4

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Feb 22, 2026

Additional Comments (1)

AGENTS.md
repo layout section should list the new shared shell modules (shell-common.sh, deploy-common.sh, doctor-common.sh) for consistency with the actual bin/lib/ directory structure

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: AGENTS.md
Line: 33-36

Comment:
repo layout section should list the new shared shell modules (`shell-common.sh`, `deploy-common.sh`, `doctor-common.sh`) for consistency with the actual bin/lib/ directory structure

<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.

@benvinegar
Copy link
Copy Markdown
Member Author

benvinegar commented Feb 22, 2026

Superseded by the corrected coverage update comment below (with proper code formatting): #109 (comment)

@benvinegar
Copy link
Copy Markdown
Member Author

benvinegar commented Feb 22, 2026

Added the follow-up coverage in this branch:

  • New shell unit tests:
bin/lib/deploy-common.test.sh
bin/lib/doctor-common.test.sh
  • Wired into shell runners:
bin/test.sh shell
test/shell-scripts.test.mjs

Validation after adding tests:

bash bin/lib/deploy-common.test.sh   # 6/6
bash bin/lib/doctor-common.test.sh   # 5/5
bash bin/test.sh shell               # 8/8 suites passing

@benvinegar benvinegar merged commit 092d302 into main Feb 22, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant