Skip to content

Comments

refactor: reduce doctor/audit shell LOC via shared node checks#156

Open
benvinegar wants to merge 4 commits intomainfrom
refactor/phase3-loc-reduction-doctor-audit
Open

refactor: reduce doctor/audit shell LOC via shared node checks#156
benvinegar wants to merge 4 commits intomainfrom
refactor/phase3-loc-reduction-doctor-audit

Conversation

@benvinegar
Copy link
Member

Summary:

  • add bin/checks/slack-allowed-users.mjs to normalize SLACK_ALLOWED_USERS checks into machine-readable JSON
  • refactor bin/doctor.sh and bin/security-audit.sh to consume shared Node check contracts
  • add reusable shell helpers in bin/lib/check-report-common.sh for JSON field extraction and Node check execution
  • extend tests with test/slack-allowed-users-check.test.mjs and helper coverage in bin/lib/check-report-common.test.sh

LOC impact:
Targeted shell files (doctor.sh + security-audit.sh):

  • bin/doctor.sh: +14 / -24
  • bin/security-audit.sh: +23 / -32
  • Net: -19 lines

Repository-wide for this PR (including new checks/tests/helpers):

  • +276 / -57 (net +219)

Validation:

  • npm run test:shell
  • npm run test:js
  • npm run lint:js (existing unrelated heartbeat warnings)
  • npm run lint:shell (could not run locally: shellcheck missing)

Todo:

  • completes TODO-2a84d6a5

@greptile-apps
Copy link

greptile-apps bot commented Feb 24, 2026

Greptile Summary

Extracted SLACK_ALLOWED_USERS parsing into a reusable Node check (bin/checks/slack-allowed-users.mjs) that outputs machine-readable JSON. Added three shell helper functions (bb_json_field_or_default, bb_pick_node_bin, bb_run_node_check_payload) to bin/lib/check-report-common.sh for standardized Node check execution. Refactored both bin/doctor.sh and bin/security-audit.sh to use these shared contracts, eliminating 19 lines of duplicated shell logic across the targeted files. Includes comprehensive test coverage for both Node and shell components.

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • Well-architected refactoring with comprehensive test coverage for both new Node checks and shell helpers. Changes follow established patterns from the existing integrity-status.mjs check. All edge cases are properly handled (missing files, empty values, parse errors). Shell helpers include proper validation and safe fallbacks. Net reduction in LOC while improving maintainability.
  • No files require special attention

Important Files Changed

Filename Overview
bin/checks/slack-allowed-users.mjs New Node check that parses SLACK_ALLOWED_USERS from .env files and outputs machine-readable JSON
bin/lib/check-report-common.sh Added three reusable shell helpers: bb_json_field_or_default, bb_pick_node_bin, bb_run_node_check_payload for executing Node checks from shell
bin/doctor.sh Refactored to use shared helpers for integrity and SLACK_ALLOWED_USERS checks, reducing 10 lines of repetitive Node invocation code
bin/security-audit.sh Refactored to use shared helpers for both integrity and SLACK_ALLOWED_USERS checks, eliminating shell-based user counting logic

Last reviewed commit: fe0ceea

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