Summary
scripts/program-lib.sh has grown to ~870 lines and now spans many unrelated concerns in a single file: workspace/tempdir management, scan-ID generation, JSON schema validation, path validation, GitHub API backoff, issue-existence/lifecycle helpers, fork + cross-fork PR creation, report/history writing, and (newly) repo selection. This is a "does too much" module. We should decompose it into composable, single-purpose pieces and, at the same time, make its portability guarantees explicit and consistent.
This is tracked separately from the rename work — it is a structural refactor, not a rename fix — but it is a natural follow-up since the rename phases keep touching this file.
Motivation
- Modularity: each concern should be understandable and testable in isolation. A ~870-line grab-bag is hard to reason about and edit reliably. Smaller, well-bounded units compose better and are safer to change.
- Portability: the library must run on any Unix-like system — at minimum Linux and macOS — making no assumptions about GNU-specific tool behavior. Today portability is handled well in some places and unaddressed in others (see below), i.e. it is inconsistent rather than guaranteed.
Proposed decomposition (sketch — refine during design)
A thin program-lib.sh that sources single-purpose modules, e.g.:
lib/workspace.sh — setup_workspace, tempdir + cleanuplib/dates.sh — iso_to_epoch, scan-ID/date helperslib/json.sh — schema validation, atomic writeslib/github.sh — gh_with_backoff, issue existence/close, PR-open checkslib/fork.sh — ensure_fork, create_fork_prlib/reports.sh — write_report_latest, append_history_row, diff_against_previouslib/repos.sh — get_core_repos, core_repo_names, canonical_repo_for_dir (added in the allowlist work)
program-lib.sh becomes an aggregator that sources the modules, preserving the existing single-source entrypoint so callers do not change.
Portability audit (current state — evidence)
Good (keep as the model):
iso_to_epoch already tries GNU date -d, then BSD date -j -f, then a pure-awk fallback. This is the correct portable pattern.wc -l | tr -d ' ' guards against BSD wc's leading-space padding.
To verify/fix during the refactor:
- Establish an explicit portability contract: POSIX-ish tools only; any GNU-only flag must have a BSD branch or a pure-fallback (as
iso_to_epoch does).
- Audit every external command for GNU-only flags (
stat -c vs stat -f, sed -i, readlink -f, grep -P, date -d, sort -V, etc.). None are used bare in the lib today, but the audit should be systematic and enforced going forward.
- Add a CI/test matrix that runs the existing
tests/ suite on BOTH Linux and macOS so regressions in portability are caught automatically.
Acceptance
program-lib.sh decomposed into single-purpose modules with the existing entrypoint preserved (no caller changes required).- A documented portability contract, plus tests passing on both Linux and macOS.
- No behavior change to the programs (pure refactor); existing
tests/ continue to pass.
Design input: collapse the duplicated allowlist filter (from PR #38 review)
Review feedback on #38 (discussion) raised a concrete, security-relevant refinement for the lib/repos.sh module above:
The allowlist + canonical-remap + dedup block is now copy-pasted ~8 times across the five scanners (3× in dep-bump-fixer.sh alone). For security-sensitive automation this is a real drift risk — the repo-selection policy can silently diverge between scanners if one copy is edited and others are not.
Proposed shape: a single program-lib/lib/repos.sh helper that performs the canonical_repo_for_dir remap + is_core_repo gate and echoes the canonical name, so each call site collapses to roughly:
canon=$(repo_filter "$dir") || continue
leaving only the per-loop SEEN_CANON dedup inline. This puts the allowlist policy in exactly one place. The decomposition should treat de-duplicating this filter as a first-class goal of lib/repos.sh, not just a file move.
Summary
scripts/program-lib.shhas grown to ~870 lines and now spans many unrelated concerns in a single file: workspace/tempdir management, scan-ID generation, JSON schema validation, path validation, GitHub API backoff, issue-existence/lifecycle helpers, fork + cross-fork PR creation, report/history writing, and (newly) repo selection. This is a "does too much" module. We should decompose it into composable, single-purpose pieces and, at the same time, make its portability guarantees explicit and consistent.This is tracked separately from the rename work — it is a structural refactor, not a rename fix — but it is a natural follow-up since the rename phases keep touching this file.
Motivation
Proposed decomposition (sketch — refine during design)
A thin
program-lib.shthat sources single-purpose modules, e.g.:lib/workspace.sh—setup_workspace, tempdir + cleanuplib/dates.sh—iso_to_epoch, scan-ID/date helperslib/json.sh— schema validation, atomic writeslib/github.sh—gh_with_backoff, issue existence/close, PR-open checkslib/fork.sh—ensure_fork,create_fork_prlib/reports.sh—write_report_latest,append_history_row,diff_against_previouslib/repos.sh—get_core_repos,core_repo_names,canonical_repo_for_dir(added in the allowlist work)program-lib.shbecomes an aggregator that sources the modules, preserving the existing single-source entrypoint so callers do not change.Portability audit (current state — evidence)
Good (keep as the model):
iso_to_epochalready tries GNUdate -d, then BSDdate -j -f, then a pure-awk fallback. This is the correct portable pattern.wc -l | tr -d ' 'guards against BSDwc's leading-space padding.To verify/fix during the refactor:
iso_to_epochdoes).stat -cvsstat -f,sed -i,readlink -f,grep -P,date -d,sort -V, etc.). None are used bare in the lib today, but the audit should be systematic and enforced going forward.tests/suite on BOTH Linux and macOS so regressions in portability are caught automatically.Acceptance
program-lib.shdecomposed into single-purpose modules with the existing entrypoint preserved (no caller changes required).tests/continue to pass.Design input: collapse the duplicated allowlist filter (from PR #38 review)
Review feedback on #38 (discussion) raised a concrete, security-relevant refinement for the
lib/repos.shmodule above:The allowlist + canonical-remap + dedup block is now copy-pasted ~8 times across the five scanners (3× in
dep-bump-fixer.shalone). For security-sensitive automation this is a real drift risk — the repo-selection policy can silently diverge between scanners if one copy is edited and others are not.Proposed shape: a single
program-lib/lib/repos.shhelper that performs thecanonical_repo_for_dirremap +is_core_repogate and echoes the canonical name, so each call site collapses to roughly:leaving only the per-loop
SEEN_CANONdedup inline. This puts the allowlist policy in exactly one place. The decomposition should treat de-duplicating this filter as a first-class goal oflib/repos.sh, not just a file move.