[scealiontach]

Sprint Plan — 2026-04-28 Sprint Loop 1

Sprint Goal

Restore correctness in the foundational bash/*.sh libraries that every other script and command in this repository sources. Five high-confidence, low-risk bug fixes target the include mechanism (includer.sh), the command-path cache (commands.sh), the logging dispatch (log.sh), the directory-safety helper (dirs.sh), and the options parser (options.sh). Each issue ships with a precise reproduction, file/line location, and a suggested fix; the work is independent except for a soft ordering preference (fix includer.sh first because the rest are sourced through it). Outcome: the core libraries behave as their docstrings and AGENTS.md already promise, with shellcheck/shfmt clean and no new regressions in the existing pre-commit suite.

Selected Issues

1. SUR-1813 — Bug: includer.sh @include calls exit 1 from a sourced library, killing parent shells

• Linear ID: SUR-1813
• URL: https://linear.app/surinis/issue/SUR-1813
• Description summary: @include in bash/includer.sh (lines 22–40, line 38 in particular) calls exit 1 when an include cannot be found. Because library files are sourced (not executed), a missing-include in an interactive shell or a script-with-cleanup kills the parent shell. The fix is to return 1 from @include and let the caller decide whether to escalate via error::exit. Stderr should also be used for the diagnostic.
• Rationale: Highest-priority correctness issue — it can take down the user's interactive shell from a typo. It also constrains the test harness for the rest of this sprint: any sanity checks we add depend on @include returning to the test runner cleanly on failure. Doing this first unblocks faster iteration on the other four fixes.
• Definition of Done:
  • bash/includer.sh @include uses return 1 (not exit 1) on missing-file, with the diagnostic written to stderr (>&2).
  • A focused test confirms a missing include in a sourced context returns non-zero without exiting the runner shell.
  • Existing dedupe behavior (cksum-derived guard variable, idempotent re-include) is unchanged — verified by including the same file twice in a test and asserting the body runs once.
  • pre-commit run --all-files is clean (shellcheck, shfmt, commitizen subjects).
  • make package still produces all three tarballs (doc-*, bin-*, lib-*) under dist/.
• Dependencies / ordering: None. Should be merged first to simplify failure-mode testing of the other four fixes.

2. SUR-1810 — Bug: commands::use cache check uses wrong variable, defeating PATH lookup caching

• Linear ID: SUR-1810
• URL: https://linear.app/surinis/issue/SUR-1810
• Description summary: bash/commands.sh lines 33–46. commands::use checks $_cmd (a literal name that is never assigned) instead of the dynamic $_${cmd} (which the function does set on the next line). # shellcheck disable=SC2154 on the line above had been hiding the typo. Result: every $(commands::use git|jq|aws|kubectl|…) call re-runs command -v in a subshell. Fix: dereference the indirect variable via local var=_${cmd}; [ -z "${!var}" ], and remove the SC2154 disable.
• Rationale: Foundational — the cache is consumed by git.sh, aws.sh, docker.sh, k8s.sh, and many command scripts, on hot paths inside loops. Fix is one-function, surgical, and independently verifiable.
• Definition of Done:
  • commands::use caches via local var=_${cmd}; [ -z "${!var}" ] and the no-op # shellcheck disable=SC2154 is removed.
  • commands::err_not_found path still triggers when the binary is absent (covered by an added test that stubs command -v to fail).
  • Repeated calls demonstrably hit the cache: a test calls commands::use bash twice and asserts the cached value is used on the second call.
  • pre-commit run --all-files is clean.
  • No public API change — return value/echo contract preserved for existing callers.
• Dependencies / ordering: Independent of SUR-1811/1812/1819. Soft-depends on SUR-1813 only because tests source commands.sh via @include.

3. SUR-1811 — Bug: log.sh FORMAT_LOG emits literal \n and LOG() dispatcher is broken

• Linear ID: SUR-1811
• URL: https://linear.app/surinis/issue/SUR-1811
• Description summary: Two defects in bash/log.sh. (1) Line 197 echo "$formatted_log\n" emits a literal \n because echo does not interpret backslash escapes by default; the file handler masks this by stripping \n (line 256), but the colorterm/stderr path emits the literal sequence. Fix: use printf '%s\n' "$formatted_log". (2) Lines 203–209 LOG() does indirect variable expansion ${!level} expecting INFO, DEBUG, etc. to be variables; they are functions, so the dispatcher always invokes the empty string and silently drops messages routed through log() / log::log. Fix: convert to a case statement on the upper-cased level, dispatching to log::trace|debug|info|warn|error|critical|notice|alert|emergency.
• Rationale: Logging is used everywhere; both bugs are silent failures (stderr corruption + dropped messages) that mask other problems. The fix is mechanical and the issue ships with the exact replacement code.
• Definition of Done:
  • FORMAT_LOG uses printf '%s\n' "$formatted_log" and the LOG_HANDLER_LOGFILE workaround that stripped \\n is either removed or kept and verified harmless.
  • LOG() dispatches via case on ${1^^} to the appropriate log::* function, with an *) log::error "Unknown log level: $level" default branch.
  • Tests assert: (a) log::log INFO "hello" produces hello followed by a real newline (not \n); (b) log::log NOTAREALLEVEL "x" returns successfully via the error branch.
  • Colorterm output verified to no longer emit literal \n (manual eyeball check + automated assertion via printf capture).
  • pre-commit run --all-files is clean.
• Dependencies / ordering: Independent. Can land in parallel with SUR-1810/1812/1819.

4. SUR-1812 — Bug: dirs::noreplace is a no-op despite docstring promising existence check

• Linear ID: SUR-1812
• URL: https://linear.app/surinis/issue/SUR-1812
• Description summary: bash/dirs.sh lines 84–89. dirs::noreplace is documented as "Return an error if the directory exists and is not empty, otherwise ensure that it exists" but the body only emits a misleading log message ("This command will replace the contents …" — wording lifted from the replace function) and unconditionally returns success. Combined with dirs::replace (which does safe_rmrf), calling the wrong function silently allows clobbering. Fix: implement the documented behavior — [ -d "$d" ] && [ -n "$(ls -A "$d" 2>/dev/null)" ] && error::exit …; else dirs::ensure "$d" — and correct the log message.
• Rationale: A safety helper that is silently a no-op is worse than no helper at all. The fix is short, self-contained, and the docstring already specifies the intended contract — minimal interpretation required.
• Definition of Done:
  • dirs::noreplace errors when the target exists and is non-empty (via error::exit with a clear message naming the directory).
  • dirs::noreplace creates the directory via dirs::ensure when it does not exist, OR exits 0 when it exists but is empty.
  • The "will replace the contents" log line is removed or rewritten to match the function's actual semantics.
  • Tests cover the three cases: non-existent dir → created; empty dir → success no-op; non-empty dir → non-zero exit with diagnostic on stderr.
  • pre-commit run --all-files is clean.
• Dependencies / ordering: Independent.

5. SUR-1819 — Bug: options::add leaks opt and resets OPTARG globally

• Linear ID: SUR-1819
• URL: https://linear.app/surinis/issue/SUR-1819
• Description summary: bash/options.sh. options::add declares most of its variables local but uses opt (in the parsing loop ~line 74) and writes OPTARG= (clobbering the caller's getopts state) without local. Same pattern in options::syntax (items=() at ~164) and options::doc (count=0 at 181, mandatory, args at 186/191). Because options.sh calls options::add -o h … at source time (line 154), the leak fires on every include. Fix: add local opt; drop the OPTARG= reset entirely (getopts manages it); add local to items, count, mandatory, args.
• Rationale: Footgun for every command script. A leaking OPTARG mid-parse, or a top-level $opt variable, can corrupt subsequent argument parsing in subtle and intermittent ways. Fix is purely additive (local) — zero behavioral change to the documented contract, but eliminates a class of heisenbugs.
• Definition of Done:
  • local opt added to the options::add parsing loop; redundant OPTARG= removed.
  • local items, local count, local mandatory, local args added to options::syntax / options::doc as appropriate.
  • A test sources options.sh and asserts that none of opt, items, count, mandatory, args are set in the caller's environment afterward (using declare -p VAR and checking exit status).
  • At least one existing command-script smoke run (e.g. bash/clean-branches -h or bash/changelog -h) still prints help correctly.
  • pre-commit run --all-files is clean.
• Dependencies / ordering: Independent.

Risks + Mitigations

• Risk: No existing test suite means every fix introduces both a fix and its first-ever test.
  Mitigation: Use plain bash test scripts under tests/ at the repo root, invoked by a single tests/run.sh entry point. Do NOT block sprint completion on adopting bats-core (separate Backlog issue, SUR-1850); use set -e; trap plus [[ … ]] assertions for now. If bats-core is already installed locally, prefer it.
• Risk: commands::use cache fix changes hot-path behavior in many libraries; subtle regressions could surface only in long-running scripts (e.g. aws.sh, k8s.sh).
  Mitigation: The fix is a pure correctness win (cache now actually caches), so the only behavioral delta is fewer command -v invocations. Add a smoke test that calls commands::use git, commands::use jq twice each and inspects $_git, $_jq are populated and stable.
• Risk: log.sh LOG() dispatcher fix may unmask previously-hidden noisy log statements that were silently dropped.
  Mitigation: Run make package and a manual exercise of any command-script that uses log::log / log(); if a flood of unexpected log output appears, treat it as a separate finding and file a follow-up issue rather than re-suppressing the dispatcher. Default LOG_LEVEL=0 (per SUR-1847, out of scope here) will continue to gate verbose levels.
• Risk: includer.sh exit → return change shifts failure-handling responsibility to callers; some command scripts may rely on the abort behavior.
  Mitigation: Grep for @include callers (grep -rn '@include' bash/) and confirm none rely on exit. Where command scripts want hard-fail behavior, have the caller use @include foo || error::exit "Failed to load foo". Update at most a handful of call sites; if more than three need adjustment, file a follow-up rather than expanding sprint scope.
• Risk: Five issues × test-authoring + lint cycles could exceed the sprint window.
  Mitigation: Issues are ordered (1813 first); 1810/1811/1812/1819 are mutually independent and can be parallelized across separate feature/sur-XXXX branches using each issue's gitBranchName. Each PR is small and reviewable in isolation.
• Risk: shellcheck / shfmt configuration drift between local and CI.
  Mitigation: Run pre-commit run --all-files locally before pushing each PR. Repo CI (.github/workflows/pre-commit.yaml) only runs pre-commit hooks, so local-clean = CI-clean.
• Risk: Removing # shellcheck disable=SC2154 in commands.sh might surface unrelated SC2154 findings in nearby code.
  Mitigation: Scope the removal to the single line documented in SUR-1810. If shellcheck flags additional warnings, fix them in-scope only if trivial; otherwise file a follow-up.
• Risk: Branch name validation rejects pushes that don't match ^(fix|feature|refactor)/[a-zA-Z0-9\-]+$.
  Mitigation: Use the Linear-provided gitBranchName (feature/sur-1810 etc.); they already conform.

Out of Scope

• Introducing bats-core as the standard test framework (SUR-1850 — separate Backlog item).
• Test coverage for semver (SUR-1851) or pack-script produced binaries (SUR-1852).
• All other Bug-labelled Backlog items (SUR-1814 through SUR-1849) not in this five-issue selection.
• Documentation issues (SUR-1844 through SUR-1847).
• The Jenkinsfile → GitHub Actions migration (SUR-1853).
• Any changes to the @doc / bashadoc / pack-script mechanisms beyond what is strictly required by this sprint's tests.
• Changes to Makefile, standard_defs.mk, or dist/ build outputs except as incidental side effects of make package validation.
• Refactors to convert bare functions to namespaced package::name form (SUR-1840).
• Renaming, restructuring, or relocating libraries.

Linear Evidence

• Linear team verified: Surinis (id ce9ebfde-ff2b-4f54-90f1-c388591ca110, key SUR) — confirmed via mcp__linear__get_team and re-confirmed from each issue's team field.
• Linear project used: shell-scripts (id a43901a0-b02b-4009-aae1-a6e8903d127d) — confirmed via mcp__linear__get_project; teams[0].name == "Surinis".
• Query/filter used: mcp__linear__list_issues team="Surinis" project="shell-scripts" state="Backlog" limit=250 includeArchived=true (default in this MCP client).
• Approx count of Backlog issues reviewed: 44 (full set returned with hasNextPage=false).
• Approx count of manual-labelled Backlog issues skipped: 0 (no issue in the result set carries the team-scoped manual label).

Sub-issue Status

None of the five selected parent issues has any sub-issues (verified via mcp__linear__list_issues parentId=<ID> returning empty list for each):

Parent Issue	Sub-issue	Sub-issue Status	Eligible?
SUR-1813	(none)	n/a	Yes
SUR-1810	(none)	n/a	Yes
SUR-1811	(none)	n/a	Yes
SUR-1812	(none)	n/a	Yes
SUR-1819	(none)	n/a	Yes

No parent was skipped due to incomplete sub-issues.

Linear State Transitions

Issue ID	Previous State	New State
SUR-1813	Backlog	Todo
SUR-1810	Backlog	Todo
SUR-1811	Backlog	Todo
SUR-1812	Backlog	Todo
SUR-1819	Backlog	Todo