[scealiontach]

Sprint Plan — 2026-04-28 sprint-loop-7

Sprint Goal

Knock down a coherent batch of correctness defects and small improvements across bash/ command scripts and libraries: fix two silently-broken integrations (PagerDuty alerting, bb Bitbucket PR queries), repair the k8s-support-collector archive pipeline, finish two follow-ups left dangling from the previous sprint (fetch-all-pod-logs -n, plantuml-from-k8s options/help), and document/initialise the log.sh level system so default-verbosity scripts behave predictably. Every selected item is a single- or two-file edit with static verification (shellcheck, shfmt, bash -n, -h smoke) — fits a short sprint and reduces operational risk in tools that SREs actually rely on.

Selected Issues

1. SUR-1856 — Follow-up to SUR-1829: fetch-all-pod-logs — add -n NAMESPACE flag

• Description summary: SUR-1829's DoD listed namespace support as in-scope-but-optional. The merged PR did not add it and did not call the deferral out in the commit body. The issue description explicitly requests adding -n NAMESPACE so the script works outside the kubectl current-context default namespace. Suggested skeleton uses options::add -o n -d "namespace …" -a -e namespace and threads (-n "$namespace") into both k8s::ctl get pods and k8s::ctl logs calls.
• Rationale: Small, single-file edit. Closes the loop on a deferred bullet from SUR-1829. Restores parity between issue DoD and shipped behaviour.
• Definition of Done:
  • bash/fetch-all-pod-logs -h lists -n NAMESPACE.
  • k8s::ctl get pods and k8s::ctl logs calls receive -n "$namespace" when supplied.
  • Default behaviour unchanged when -n is omitted (current-context namespace).
  • pre-commit run shellcheck shfmt --files bash/fetch-all-pod-logs is clean.
  • Conventional-commit message references SUR-1856 and SUR-1829.
• Dependencies / ordering: None. Independent of other selected issues. Recommend tackling first as a warm-up.

2. SUR-1855 — Follow-up to SUR-1832: plantuml-from-k8s — getRs unused arg + missing options::parse/help

• Description summary: Two defects from SUR-1832's "Other defects in the same file" section that the merged PR did not address: (a) getRs is invoked with an unused replicaset literal (copy-paste leftover from a getItems "$ns" replicaset pattern); (b) the script never calls options::standard/options::add/options::parse, so it has no -h, no flag validation, and consumes the cluster name from a bare positional. Suggested: drop the unused arg or thread it through getRs, and convert the positional cluster arg into a mandatory -c CLUSTER (or keep positional but add the standard option boilerplate).
• Rationale: Brings plantuml-from-k8s in line with the rest of the repo's command-script convention (every other command uses options::*) and removes a confusing dead argument.
• Definition of Done:
  • getRs no longer receives an unused argument (or accepts and uses it consistently).
  • Script calls options::standard/options::add/options::parse.
  • bash/plantuml-from-k8s -h produces help text.
  • bash -n bash/plantuml-from-k8s succeeds.
  • pre-commit run shellcheck shfmt --files bash/plantuml-from-k8s is clean.
  • Conventional-commit message references SUR-1855 and SUR-1832.
• Dependencies / ordering: None. Independent.

3. SUR-1847 — Docs/Improvement: log.sh default LOG_LEVEL=0 silently disables WARNING/INFO/DEBUG/TRACE

• Description summary: bash/log.sh initialises LOG_LEVEL=${LOG_LEVEL:-0} but never calls log::level at source time, so the LOG_DISABLE_* flags remain unset. Scripts that don't pass -v end up with empty disable flags; the test [ "$LOG_DISABLE_INFO" = "false" ] then evaluates false (empty ≠ "false"), so log::info and below silently no-op. The level→enabled-set mapping is documented nowhere. Suggested fix: (1) document the level table at the top of log.sh; (2) call log::level "$LOG_LEVEL" once at source time so the disable flags are deterministic; (3) mention the contract in options.sh::options::standard and/or the README.
• Rationale: Tiny, low-risk change with high discoverability payoff. Removes a footgun every new contributor hits.
• Definition of Done:
  • bash/log.sh calls log::level "$LOG_LEVEL" at source time after the LOG_LEVEL=${LOG_LEVEL:-0} line.
  • Top-of-file comment in log.sh documents the LOG_LEVEL→enabled-levels table.
  • options::standard's @doc annotation in bash/options.sh references the level mapping (or links back to log.sh).
  • pre-commit run shellcheck shfmt --files bash/log.sh bash/options.sh is clean.
  • Conventional-commit message references SUR-1847.
• Dependencies / ordering: None. Independent.

4. SUR-1833 — Bug: bb shadows function arg repo with local project repo; sources untrusted CONFIG file

• Description summary: Three defects in bash/bb: (1) pr-data redeclares its repo parameter via local project repo, clobbering $1 to empty — every API call ends up projects//repos//pull-requests?..., so pr-count and pr-list (which delegate through pr-data) always return empty; (2) the script sources $HOME/.${CONFIG}-config from a user-controlled $CONFIG value with no validation, allowing path-separator escapes; (3) the trailing "$@" is unbounded function dispatch — any name visible in the shell (including those sourced from the config file) is callable. Suggested: drop the redeclaration and use a distinct repo_name local; validate $CONFIG against ^[a-zA-Z0-9_-]+$; replace "$@" with an explicit allow-list case over pr-data|pr-count|pr-list.
• Rationale: Restores the bb script to working order (currently broken in the most-used path) and tightens a real security smell. Single-file change.
• Definition of Done:
  • pr-data no longer shadows its repo argument; HTTP path includes a non-empty project and repo.
  • $CONFIG is validated against a sane identifier regex before sourcing; sourcing rejects names with /, .., etc.
  • The trailing dispatch is an explicit allow-list over pr-data, pr-count, pr-list only; unknown subcommands error::exit cleanly.
  • pre-commit run shellcheck shfmt --files bash/bb is clean.
  • Smoke: bb -c <name> pr-list <project>/<repo> returns a non-empty body for a known repo (or, where API access is unavailable, the call URL is correct in set -x).
  • Conventional-commit message references SUR-1833.
• Dependencies / ordering: None. Independent.

5. SUR-1827 — Bug: k8s-support-collector — sed in null-delimited pipe and single-slash filename replace

• Description summary: Two bugs in bash/k8s-support-collector that produce mangled support tarballs: (1) find … -print0 | sed -e "s@$OUT_DIR/@@g" | xargs -0 tar cfz — sed is line-oriented and strips/garbles the NUL delimiters between find -print0 and xargs -0, so xargs receives one giant blob and tar silently misses files or fails; (2) file=${file/\//_} uses single replace (only fires on the first /), so cluster contexts containing multiple slashes (e.g. arn:aws:eks:.../cluster/my-cluster) end up with embedded slashes that tar interprets as directories. Bonus: find … -exec shred {} \; runs one shred per file instead of +-form. Suggested fix: move path stripping to tar --transform and pipe find -print0 straight to tar --null --files-from=-; switch to ${file//\//_}; switch to -exec shred {} +.
• Rationale: This is the SRE incident-response tool. A bug that produces an incomplete archive only surfaces during a real incident. Worth fixing pre-emptively.
• Definition of Done:
  • The archive pipeline no longer pipes sed between find -print0 and xargs -0; uses tar --null --files-from=- --transform (or cd + relative paths).
  • Filename slash collapse uses ${file//\//_}.
  • shred invocation uses -exec … +.
  • pre-commit run shellcheck shfmt --files bash/k8s-support-collector is clean.
  • Smoke: produced tarball expands to the same set of files written under $OUT_DIR (verified by tar -tzf | wc -l vs. find $OUT_DIR -type f | wc -l in a dry run).
  • Conventional-commit message references SUR-1827.
• Dependencies / ordering: None. Independent.

6. SUR-1821 — Bug: pagerduty-alert event path is fully commented out and send_incident misses -X POST

• Description summary: Two related defects: (1) bash/pagerduty.sh::send_event has its entire _curl body commented out, but the function still logs "Sent PagerDuty event" — invoking pagerduty-alert -i event … returns 0 having sent nothing, a silent-failure mode of an alerting tool; (2) bash/pagerduty.sh::send_incident calls _curl POST --header … rather than _curl -X POST --header … — POST is consumed as a positional URL argument and the request method is wrong. The pagerduty-alert script's event arm is also commented out, so even fixing send_event requires un-commenting the call site. Suggested: fix -X POST for incidents; either properly implement send_event against the PagerDuty Events API v2 (different endpoint and payload) or have it error::exit "send_event not implemented" so callers fail loudly; un-comment the pagerduty-alert event arm to match; namespace the functions to pagerduty::send_incident/pagerduty::send_event per AGENTS.md.
• Rationale: Silent failure of an alerting tool is the single worst class of bug we have on the board. The fix is small. Even the "fail loudly" stub is a strict improvement over the current state.
• Definition of Done:
  • pagerduty.sh::send_incident (or its renamed pagerduty::send_incident) calls curl -X POST (not bare POST).
  • pagerduty.sh::send_event either implements the Events API v2 call or fails loudly (error::exit) — never silently returns 0.
  • pagerduty-alert's event) arm is no longer commented out — it either calls the implemented send_event or routes through the loud-fail stub.
  • Functions renamed to pagerduty::send_incident / pagerduty::send_event (AGENTS.md namespacing); back-compat shims via annotations::deprecated if any in-tree caller still uses the bare names.
  • pre-commit run shellcheck shfmt --files bash/pagerduty.sh bash/pagerduty-alert is clean.
  • Conventional-commit message references SUR-1821.
• Dependencies / ordering: None. Independent. (Recommended last because the namespace rename touches both files.)

Risks + Mitigations

• R1 — bb smoke verification needs Bitbucket credentials: The pr-data fix is provable by inspecting the resolved URL via set -x, but a true integration smoke needs a Bitbucket token. Mitigation: validate the URL string in a captured set -x trace; do not require live API access in DoD.
• R2 — k8s-support-collector smoke requires a populated $OUT_DIR: The pipeline fix is best validated end-to-end against a real collected bundle. Mitigation: synthesise a small fake $OUT_DIR with a few files (including paths containing spaces and multiple slashes in the leading prefix) and verify the produced tarball roundtrips.
• R3 — send_event PagerDuty Events API v2 implementation is non-trivial: The Events API uses a different endpoint (events.pagerduty.com/v2/enqueue), a different auth model (routing key, not API token), and a different payload schema. Mitigation: scope the sprint deliverable to the loud-fail stub plus the -X POST fix; track full Events-API implementation as a follow-up issue if no driver exists.
• R4 — log::level at source time changes default behaviour for callers that already manipulate LOG_DISABLE_* directly: Calling log::level "$LOG_LEVEL" at source time will overwrite any LOG_DISABLE_* a caller pre-set before sourcing log.sh. Mitigation: only call log::level if the LOG_DISABLE_* flags are unset (if [ -z "${LOG_DISABLE_NOTICE+set}" ]; then log::level "$LOG_LEVEL"; fi), or document the precedence change explicitly.
• R5 — plantuml-from-k8s switching from positional cluster arg to -c CLUSTER is a breaking API change: Anyone with plantuml-from-k8s my-cluster in a wrapper will break. Mitigation: keep the positional accepted as a fallback (parse -c first, then fall back to ${1:-}); or commit the breaking change but call it out in the conventional-commit body and changelog.
• R6 — Sprint scope creep into adjacent anti-patterns: SUR-1842 ("$@" dispatch) overlaps SUR-1833's bonus item; touching bash/bb may tempt tackling SUR-1842 too. Mitigation: hold the line — fix only the bb instance under SUR-1833; defer kind, minikube to SUR-1842.
• R7 — Branch-naming hook will reject some natural names: pre-commit's no-commit-to-branch enforces ^(fix|feature|refactor)/[a-zA-Z0-9\-]+$ and rejects underscores/dots. Mitigation: branch names use fix/sur-XXXX-short-slug or feature/sur-XXXX-short-slug only.
• R8 — No regression test suite: Repo has no tests today (AGENTS.md acknowledges this). Mitigation: rely on pre-commit (shellcheck, shfmt) plus targeted manual smokes documented in the implementation plan.

Out of Scope

• SUR-1850 (introduce bats-core test suite) — foundational but a sprint of its own; defer.
• SUR-1851 (semver test coverage) — depends on SUR-1850 landing first.
• SUR-1854 (daml-export hygiene: set -x, exit-trap scope, java arg arrays, Ctrl-C trap) — separate file, separate sprint.
• SUR-1842 (unbounded "$COMMAND"/"$@" dispatch in kind/minikube/bb) — bb is covered by SUR-1833; defer the kind/minikube instances.
• SUR-1841 (brittle kubectl … | grep -v NAME | awk '{print $1}' parsing) — touches five command scripts and needs a shared helper; defer.
• SUR-1840 (bare non-namespaced functions across libraries) — large mechanical refactor; defer.
• SUR-1848 (registry-cp duplicates docker::cp; release-images iterates extras) — separate refactor; defer.
• SUR-1853 (replace Jenkinsfile with GitHub Actions CI) — large infra change with credential migration; defer.
• Implementing the full PagerDuty Events API v2 driver (see R3 above) — sprint covers only the loud-fail stub + -X POST fix.
• Switching plantuml-from-k8s to a non-backward-compatible CLI (see R5) — sprint keeps the positional fallback.
• Any work not in the six selected issues.

Linear Evidence

• Linear team verified: Surinis (id ce9ebfde-ff2b-4f54-90f1-c388591ca110)
• Linear project used: shell-scripts (id a43901a0-b02b-4009-aae1-a6e8903d127d); project's owning team is Surinis — match confirmed.
• Query/filter used: list_issues(team="Surinis", project="shell-scripts", state="Backlog", limit=100) plus per-candidate get_issue(includeRelations=true), list_issues(parentId=…) for sub-issues, list_comments(issueId=…) for comment threads, and list_issue_labels(team="Surinis", name="manual") to enumerate manual-labelled issues.
• Approx count of Backlog issues reviewed: 14 (full page returned, hasNextPage=false).
• Approx count of manual-labelled Backlog issues skipped: 0. The manual label exists in the workspace (id 91344244-aedf-462c-ae9e-c68f8622bcf7) but no Backlog issue in the shell-scripts project currently carries it. All 14 Backlog issues had label sets in {Improvement, Bug, testing, documentation, 3 - medium} only.

Sub-issue Status

None of the six selected parents has any sub-issues. Verified by list_issues(parentId=<each>) returning an empty array for all six. No parent was skipped due to incomplete sub-issues.

Parent Issue	Sub-issue	Sub-issue Status	Eligible?
SUR-1856	(none)	n/a	yes
SUR-1855	(none)	n/a	yes
SUR-1847	(none)	n/a	yes
SUR-1833	(none)	n/a	yes
SUR-1827	(none)	n/a	yes
SUR-1821	(none)	n/a	yes

Linear State Transitions

Issue ID	Previous State	New State
SUR-1856	Backlog	Todo
SUR-1855	Backlog	Todo
SUR-1847	Backlog	Todo
SUR-1833	Backlog	Todo
SUR-1827	Backlog	Todo
SUR-1821	Backlog	Todo