SonarCloud: misc cleanup (mixed findings) (×2) #24

Activity

1. don-petry

   added

   enhancementFeature requestsFeature requests

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jul 3, 2026
2. don-petry commented on Jul 3, 2026

   

   don-petry

   on Jul 3, 2026

   ContributorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #24 — SonarCloud: misc cleanup (mixed findings) (×2)

   Scope

   Resolve two SonarCloud shell findings in scripts/cron_backfill.sh with behavior-preserving fixes: use [[ ]] for the conditional test (S7688) and add an explicit return to the ts() function (S7682).

   Implementation Checklist

   • scripts/cron_backfill.sh line 23: [ -x "$PY" ] → [[ -x "$PY" ]] (S7688)
   • scripts/cron_backfill.sh line 25: add explicit return at end of ts() function (S7682)

   Tests to Write

   • None — this is a behavior-preserving shell refactor with no observable behavior change (Phase 2 TDD exception). Verification is via shellcheck and the existing Python contract suite staying green.

   Test Command

   pytest (existing suite) + bash .dev-lead/scripts/dev-lead-lint.sh (shellcheck)
3. don-petry commented on Jul 3, 2026

   

   don-petry

   on Jul 3, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Plan Execution

   • scripts/cron_backfill.sh line 23: [ -x "$PY" ] → [[ -x "$PY" ]] (S7688)
   • scripts/cron_backfill.sh line 25: added explicit return to ts() function (S7682)

   Test Results

   Lint (bash .dev-lead/scripts/dev-lead-lint.sh):

     [lint] shellcheck on 1 file(s)...
     [lint] validating .github/CODEOWNERS...
   OK: .github/CODEOWNERS
     [lint] all checks passed
   === lint exit 0 ===
   

   Tests (pytest):

   ============================= test session starts ==============================
   platform linux -- Python 3.12.3, pytest-9.1.1, pluggy-1.6.0
   testpaths: tests
   collected 15 items
   ...
   SKIPPED [15] tests/test_contract.py: BROODMINDER_API_KEY not set; live contract tests skipped
   ============================= 15 skipped in 0.03s ==============================
   === pytest exit 0 ===
   

   The 15 contract tests are live tests that skip without BROODMINDER_API_KEY — expected in CI without credentials. Suite exits 0.

   Files Changed

   • scripts/cron_backfill.sh: two behavior-preserving SonarCloud fixes — [[ ]] conditional test (S7688) and explicit return in the ts() helper (S7682).

   Notes

   Behavior-preserving refactor, so the TDD phase was skipped per the documented exception (no observable behavior change). [[ is safe here because the shebang is #!/usr/bin/env bash; a bare return propagates date's exit status and ts() is only used in command substitution, so both changes are semantically identical to the originals. No NOSONAR suppressions used — both fixes are real. Both listed findings for this workstream are resolved.
4. don-petry

   added

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   and removed

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jul 15, 2026
5. don-petry commented on Jul 15, 2026

   

   don-petry

   on Jul 15, 2026

   ContributorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #24 — SonarCloud: misc cleanup (mixed findings) (×2)

   Scope

   Resolve the two SonarCloud findings in scripts/cron_backfill.sh — shelldre:S7688 (use [[ instead of [) and shelldre:S7682 (explicit return at end of function) — with no behavior change.

   Implementation Checklist

   • scripts/cron_backfill.sh — S7688: ensure the -x "$PY" conditional test uses [[ … ]] (not [ … ])
   • scripts/cron_backfill.sh — S7682: ensure the ts() function ends with an explicit return
   • Confirm no other [ … ] single-bracket tests or return-less functions remain in the file

   Tests to Write

   • N/A — behavior-preserving shell lint fixes with no observable behavioral change. Verified instead via shellcheck --severity=warning (repo CI level) and the repo lint script. (Phase 2 TDD exception per the dev-lead workflow.)

   Test Command

   bash .dev-lead/scripts/dev-lead-lint.sh (shellcheck warning-level on scripts/**/*.sh + agent-profile validation); plus python -m pytest -q for the repo compliance suite.

   Note: On inspection the current tree already satisfies both findings ([[ on line 23, return in ts()), consistent with the "(×2)" duplicate designation. I will verify this holds, apply any gap, and report — without fabricating a no-op change.
6. don-petry commented on Jul 15, 2026

   

   don-petry

   on Jul 15, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Summary

   Both SonarCloud findings for scripts/cron_backfill.sh are already resolved in the current tree, so no source change was required. I verified each finding against the file rather than fabricating a no-op diff (consistent with the issue's "(×2)" duplicate designation, where the sibling workstream already landed the fix).

   Finding-by-finding

   • shelldre:S7688 (use [[ instead of [): the only conditional test in the file already uses [[ … ]]:

     # scripts/cron_backfill.sh:23
     [[ -x "$PY" ]] || PY="python3"   # fall back to system python if no venv

     A scan confirms there are no single-bracket [ … ] tests remaining in the file.
   • shelldre:S7682 (explicit return at end of function): the only function, ts(), already ends with an explicit return:

     # scripts/cron_backfill.sh:25-28
     ts() {
         date -u +%Y-%m-%dT%H:%M:%SZ
         return
     }

   Verification

   • Required repo lint (bash .dev-lead/scripts/dev-lead-lint.sh): passed — shellcheck --severity=warning clean on scripts/cron_backfill.sh, CODEOWNERS valid.
   • shellcheck (warning level, CI parity): clean. (One SC2129 style note about grouping redirects is out of scope for this issue and not flagged by the repo lint.)
   • Full test suite (python -m pytest -q):

   collected 23 items
   
   tests/test_ci_compliance.py ...                                          [ 13%]
   tests/test_contract.py sssssssssssssss                                   [ 78%]
   tests/test_repo_settings_compliance.py .....                             [100%]
   
   SKIPPED [15] tests/test_contract.py: BROODMINDER_API_KEY not set; live contract tests skipped
   =================== 8 passed, 15 skipped, 1 warning in 0.33s ===================
   

   (The 15 skips are live BroodMinder API contract tests, skipped without a key — expected.)

   Files Changed

   • None. Working tree matches HEAD; both findings were already satisfied.

   Acceptance

   • All listed SonarCloud findings (S7688, S7682) for this workstream resolved to zero
   • No behavior change; existing tests/CI green
   • Fixes are real ([[ and explicit return present in source — no NOSONAR used)

   Notes

   • Phase 2 (TDD) skipped per the documented behavior-preserving exception — these are shell lint conformance items with no observable behavior change and no shell test harness in the repo; verified via shellcheck + repo lint instead.
   • The out-of-scope SC2129 style note was intentionally left unchanged to avoid modifying beyond the issue's two findings.
7. don-petry commented on Oct 12, 2026

   

   don-petry

   on Oct 12, 2026

   ContributorAuthor

   More actions

   Closing as resolved. The shelldre:S7688 and shelldre:S7682 findings in scripts/cron_backfill.sh were addressed in PR #37 (commit fb0fbd2), which updated conditional tests to [[ and added an explicit return in ts().
8. don-petry

   closed this as completedon Oct 12, 2026