test(claude-ops): cover morning-brief telemetry any=0 and rec-maxlen=0; harden usage()

[kyle-sexton]

Summary

Closes the three non-blocking test-coverage/maintenance nits deferred from the claude[bot] review on PR #569 (the morning-brief dashboard skill).

Fix

• Added a regression test for the any=0 telemetry path: a non-empty telemetry-comments array where none carry a lane: field (only scope-note comments) — previously only the null/no-issue-found case and the populated-with-lanes case were covered.
• Added a regression test for --rec-maxlen 0 (full, untruncated RECOMMENDED preview) — previously the 0 = full semantics were only exercised implicitly via the != "0" source check, with no assertion that a long recommendation tail survives untruncated.
• Hardened usage()'s header self-extraction: replaced the hardcoded sed -n '2,35p' line range with a sentinel-based awk extraction (shebang line skipped, then every comment line printed up to the first non-comment/blank line). The header comment can now grow or shrink without silently truncating or over-running --help output.
• Bumped plugins/claude-ops/.claude-plugin/plugin.json to 0.17.2 (patch — test/maintenance only, no behavior change) and added a matching CHANGELOG.md entry.

Verification

Ran the plugin's own test suite locally (bash + jq, both GNU-date and BSD-date-stub paths):

$ bash plugins/claude-ops/skills/morning-brief/morning-brief.test.sh
...
PASS: [28] rec-maxlen 0 keeps the full tail untruncated
PASS: [29] rec-maxlen 0 never inserts an ellipsis
PASS: [30] telemetry with no lane-tagged comments reports none found
...
morning-brief.test.sh: all 47 cases passed

All 47 cases pass (44 pre-existing + 3 new: rec-maxlen=0 truncation-skip, rec-maxlen=0 no-ellipsis, and the any=0 no-lane-comments case). Also manually diffed --help output before/after the usage() change — byte-identical.

Closes #579

Related

• claude-ops:morning-brief: 3 minor test-coverage/maintenance nits (no-lanes telemetry case, rec-maxlen=0 case, usage() header extraction fragility) #579 (this issue)
• feat(claude-ops): add morning-brief operator dashboard skill #569 (origin PR — the morning-brief dashboard skill; claude[bot] review surfaced these as deferred, non-blocking nits)

Note on serialization

PR #841 (fix/812-jq-guard) is currently open and also touches plugins/claude-ops/ (a different skill, lanes). Opening this as draft with do-not-merge per the repo's serialization convention for concurrent claude-ops changes; this PR's version bump (0.17.1 → 0.17.2) may need to be re-based once #841 lands.

[claude]

Claude finished @kyle-sexton's task in 2m 44s —— View job

---

PR Review

• Gather context and read changed files
• Review morning-brief.sh changes (usage() hardening)
• Review morning-brief.test.sh additions
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Post final review findings

---

Summary: Clean, targeted patch. All three deferred nits from #569 are addressed correctly, the new tests exercise the right code paths, and the usage() rewrite is a genuine improvement. No bugs or security issues found.

---

morning-brief.sh — usage() hardening

The awk sentinel approach is correct and an improvement over the hardcoded sed -n '2,35p'.

NR == 1 { next }          # skip shebang
/^#/ { sub(/^# ?/, ""); print; next }  # strip prefix, print comment
{ exit }                  # stop at first non-comment line (blank or code)

The sub(/^# ?/, "") pattern is semantically identical to the old sed 's/^# \{0,1\}//' — both strip # or # from the line start. The stop sentinel ({ exit } on the first non-# line) correctly terminates at line 36's blank separator before set -uo pipefail. The extraction is now size-independent.

One constraint to be aware of: Blank lines within the header comment block would prematurely terminate extraction since they don't match /^#/. Currently no blank lines exist in the header, and # comment lines (which awk does match) would render as blank help output — so the convention is already correct. Not a bug, but an implicit invariant.

Permalink: morning-brief.sh L58–68

---

morning-brief.test.sh — New test cases

any=0 telemetry path (lines 103–110, 176–182):

The fixture correctly contains only non-lane comments (no lane: field on any body), which causes any to remain 0 throughout the loop, triggering:

((any)) || echo "  (no per-lane comments found on the telemetry issue)"

at morning-brief.sh:454. The assertion string matches exactly. Well-targeted.

--rec-maxlen 0 (lines 167–174):

The source condition at line 370:

if [[ "$REC_MAXLEN" != "0" && -n "$rec" && "${#rec}" -gt "$REC_MAXLEN" ]]; then

…means REC_MAXLEN="0" short-circuits the entire truncation block. The tests verify both the survival of the tail text ("omicron pi") and the absence of the ellipsis character (…). The ellipsis check against a full render is sound — with --rec-maxlen 0, no decision can produce a …, so no false pass risk.

Permalink: morning-brief.test.sh L167–182

---

Minor observations (non-blocking)

1. CHANGELOG dates: The [0.17.2] header omits a date, but this is consistent with all prior entries in this file ([0.17.1], [0.17.0], [0.16.0], etc.). Not a defect given the project's established convention.

2. telemetry-no-lanes.json fixture placement: The fixture is created at line 103 (before $TMP/empty.json's first use block) and consumed at line 177. Ordering is correct. No issue.

3. Version bump note: PR body correctly flags that 0.17.1 → 0.17.2 may need a rebase once fix(claude-ops): use type -P for jq prereq check to avoid wrapper shadowing #841 lands. Nothing to address here until that merge.

---

Verdict: Approved as-is. The changes are correct, the tests are well-scoped, and the usage() rewrite removes a real fragility. Ready to merge once #841 serialization is resolved.

[claude]

Claude finished @kyle-sexton's task in 1m 14s —— View job

---

Security Review — PR #844

• Gather PR diff and changed files
• Review morning-brief.sh for injection surfaces and unsafe shell patterns
• Review morning-brief.test.sh for unsafe test patterns
• Review plugin.json and CHANGELOG.md for supply-chain / config risks
• Post security findings

---

No security issues found. This PR's changes are limited to three areas; none introduce new attack surfaces or worsen existing ones.

usage() awk replacement (morning-brief.sh lines 62–66)

The old sed -n '2,35p' "${BASH_SOURCE[0]}" and the new sentinel-based awk block both read the script's own source file via ${BASH_SOURCE[0]}. No user input flows into the awk program string or into the path operand — ${BASH_SOURCE[0]} is set by the shell from the filesystem, not from an argument. The sub(/^# ?/, "") substitution operates on the literal content of the script's header comment, not on any user-supplied string. Confidence: high. No finding.

Test fixture additions (morning-brief.test.sh lines 103–110, 167–186)

Both new fixture blocks use single-quoted heredocs (<<'EOF'), so no variable expansion occurs during fixture construction. The fixture JSON is written to paths under $TMP (the test's mktemp -d scratch directory) and consumed only within the same test run. No user-controlled data reaches jq or the shell through this path. Confidence: high. No finding.

plugin.json version bump and CHANGELOG.md

The version field changes from 0.17.1 to 0.17.2 — a semver patch bump with no new permissions, no new MCP servers, no new userConfig keys, and no change to the plugin's trust boundary. No supply-chain or privilege-escalation risk. Confidence: high. No finding.

---

[kyle-sexton]

Post-green review classification (work lane):

Finding	Source	Classification	Disposition
Everything	PR review + security review	"Approved as-is" / "No security issues found"	No action needed
Blank-line-in-header implicit invariant (usage() awk sentinel)	PR review	Non-blocking observation, not currently exploitable (no blank lines exist in the header)	Left alone
Version-rebase-once-#841-lands note	PR review	Already tracked in PR body/do-not-merge serialization	No new action

No DEFER-FORBIDDEN class applies. No inline threads to resolve (PR-level comments only).

Leaving do-not-merge in place — this PR is correctly serialized behind sibling #841 (same plugin, different skill) per rule 6d. Not flipping to ready yet; will rebase + re-bump past #841 once it merges, then flip.

[claude]

Claude finished @kyle-sexton's task in 2m 11s —— View job

---

PR Review — #844

• Gather context and read changed files
• Review morning-brief.sh changes (usage() hardening)
• Review morning-brief.test.sh additions
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Post final review findings

---

Summary: Clean, well-targeted patch. All three deferred nits from #569 are addressed correctly. No bugs, no security issues, no CLAUDE.md violations.

---

morning-brief.sh — usage() awk sentinel (L58–68)

The sentinel-based extraction is correct and a genuine improvement. The script's header runs lines 2–35 (all ^#-prefixed); line 36 is the blank separator before set -uo pipefail. The awk { exit } fires on that blank line, extracting exactly the right range without depending on a hardcoded count.

35  #   4  prerequisite missing (gh or jq), or repo could not be resolved
36  <blank>   ← awk exit fires here
37  set -uo pipefail

sub(/^# ?/, "") is semantically identical to the old sed 's/^# \{0,1\}//'. Behavior is preserved and the extraction is now size-independent.

Minor nit (non-blocking): The inline comment says "shebang to first blank line" — more precisely it's "shebang skipped, then every ^# line until the first non-^# line (which happens to be a blank)". The existing constraint (noted by the prior review) that blank lines within the header would prematurely terminate is real but the current file has none, and the # convention is established.

---

morning-brief.test.sh — New test cases (L103–110, L167–182)

--rec-maxlen 0 (lines 167–174): Uses the existing decisions.json fixture which includes the long-rec item #104 (RECOMMENDED: alpha beta gamma … omicron pi.). With REC_MAXLEN="0", the condition at L370 short-circuits ("$REC_MAXLEN" != "0" is false), so the … substitution never runs. Both assertions are correct:

• assert_contains … "omicron pi" — tail survives untruncated
• assert_not_contains … "…" — no ellipsis in the full render

any=0 telemetry path (lines 176–182): The telemetry-no-lanes.json fixture has two comments with no lane: field, so any stays 0 through the loop and L454 emits (no per-lane comments found on the telemetry issue). The assertion string matches exactly.

Both fixtures use single-quoted heredocs (<<'EOF'), no variable expansion risk, and are written under $TMP (the mktemp -d scratch dir). No issues.

---

plugin.json version bump (L4)

0.17.3 → 0.17.4. The PR body anticipated a re-base from 0.17.2 once #841 landed; the branch has correctly been rebased past that and 0.17.3, incrementing to 0.17.4. Patch bump is appropriate — no behavior change, test/maintenance only. No new permissions, MCP servers, or userConfig keys.

---

CHANGELOG.md (L6–17)

The [0.17.4] entry accurately describes all three changes. "Fixed" category is appropriate. Consistent with the repo's convention of omitting dates from version headers.

---

Verdict: Approved. Ready to merge once the do-not-merge serialization constraint is cleared.

[claude]

Claude finished @kyle-sexton's task in 1m 1s —— View job

---

Security Review — PR #844

• Fetch PR diff and read changed files
• Review morning-brief.sh changes
• Review morning-brief.test.sh changes
• Review plugin.json and CHANGELOG.md
• Post findings

---

No security issues found. The four changed files introduce no new attack surfaces and do not worsen any existing ones.

morning-brief.sh — usage() awk rewrite (lines 62–66)

awk '
  NR == 1 { next }
  /^#/ { sub(/^# ?/, ""); print; next }
  { exit }
' "${BASH_SOURCE[0]}"

The only operand is ${BASH_SOURCE[0]} — bash sets this to the script's own filesystem path; no user input touches it. The awk program string is a hardcoded literal with no concatenation. The sub(/^# ?/, "") substitution operates on the literal content of the script's header comment, not on any external data. Confidence: high. No finding.

morning-brief.test.sh — new fixture blocks (lines 103–110, 167–182)

Both new blocks use single-quoted heredocs (<<'EOF'), so no variable expansion occurs during fixture construction. Written paths are all under $TMP (a per-run mktemp -d directory), and consumed only within the same test invocation. No user-controlled data reaches bash, jq, or awk through these fixtures. Confidence: high. No finding.

plugin.json version bump and CHANGELOG.md

The version field advances with no new permissions, no new MCP servers, no new userConfig keys, and no change to the plugin's trust boundary or data-egress surface. CHANGELOG.md is inert documentation. No supply-chain or privilege-escalation risk. Confidence: high. No finding.

[kyle-sexton]

Post-rebase re-review classification (work lane):

Rebased twice now (past #841, then past #860), re-derived to 0.17.4, do-not-merge dropped after confirming no live collision with sibling #853 (which serializes past this PR instead).

Finding	Source	Classification	Disposition
Everything substantive	PR review + security review	"Approved" / "No security issues found"	No action needed
usage() inline comment slightly imprecise ("shebang to first blank line" vs. exact mechanics)	PR review (nit)	Non-blocking wording suggestion	Left alone

No DEFER-FORBIDDEN class applies. No inline review threads to resolve (PR-level comments only). Verified comment content directly (not just the green check) per this cycle's standing practice.

Flipping to ready, handing off to babysit.

[claude]

Claude finished @kyle-sexton's task in 1m 20s —— View job

---

PR Review — #844

• Gather context and read changed files
• Review morning-brief.sh changes (usage() hardening)
• Review morning-brief.test.sh additions
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Post final review findings

---

Summary: Clean, targeted patch. All three deferred nits from #569 are addressed correctly. No bugs, no security issues, no CLAUDE.md violations. Approved.

---

morning-brief.sh — usage() awk sentinel (L58–68)

The sentinel-based extraction is correct and a genuine improvement over the hardcoded sed -n '2,35p'. The { exit } on the first non-^# line (the blank separator at line 36 before set -uo pipefail) extracts exactly the right range. sub(/^# ?/, "") is semantically identical to the old sed substitution; behavior is preserved and the extraction is now size-independent.

Invariant to keep: Blank lines within the header comment would prematurely terminate extraction (they don't match /^#/). The current file has none, and # comment lines (which awk does match) render as blank help output — so the convention is already correct. Not a defect, just an implicit constraint to maintain.

---

morning-brief.test.sh — New test cases (L103–110, L167–182)

--rec-maxlen 0 (lines 167–174): The source condition at L370 — [[ "$REC_MAXLEN" != "0" && ... ]] — short-circuits the entire truncation block when REC_MAXLEN="0". Both assertions are correct:

• assert_contains … "omicron pi" — tail survives untruncated
• assert_not_contains … "…" — ellipsis never emitted

any=0 telemetry path (lines 103–110, 176–182): The telemetry-no-lanes.json fixture has two comments with no lane: field, so any stays 0 through the loop and L454 emits (no per-lane comments found on the telemetry issue). The assertion string matches exactly. Well-targeted — this path was previously untested.

Both fixtures use single-quoted heredocs (<<'EOF'), no variable expansion risk, written to $TMP (per-run mktemp -d). No issues.

---

plugin.json version bump (L4)

0.17.3 → 0.17.4 — correctly re-derived after rebasing past #841 and #860. Patch bump is appropriate (test/maintenance only, no behavior change). No new permissions, MCP servers, or userConfig keys.

---

CHANGELOG.md (L6–17)

The [0.17.4] entry accurately describes all three changes. "Fixed" category is appropriate. Date omission is consistent with the project's established convention for all prior entries.

---

Verdict: Approved. Ready to merge.

[claude]

Claude finished @kyle-sexton's task in 1m 43s —— View job

---

Security Review — PR #844

• Gather context
• Fetch PR diff and read changed files
• Review morning-brief.sh changes (usage() awk rewrite)
• Review morning-brief.test.sh additions
• Review plugin.json and CHANGELOG.md
• Post findings

---

No security issues found. The four changed files introduce no new attack surfaces and do not worsen any existing ones.

---

morning-brief.sh — usage() sentinel rewrite (L62–66)

awk '
  NR == 1 { next }
  /^#/ { sub(/^# ?/, ""); print; next }
  { exit }
' "${BASH_SOURCE[0]}"

The sole operand is ${BASH_SOURCE[0]} — the shell sets this from the filesystem, not from any argument or external input. The awk program string is a hardcoded literal; no user-controlled data reaches the awk program or the path operand. sub(/^# ?/, "") operates on literal lines from the script header. No injection surface. Confidence: high.

---

morning-brief.test.sh — new fixture blocks (L103–110, L167–182)

Both new fixture blocks use single-quoted heredocs (<<'EOF'), so no variable expansion occurs during fixture construction. All fixture paths are under $TMP (a per-run mktemp -d directory with a trap ... EXIT cleanup). No user-controlled data reaches bash, jq, grep, or awk through these fixtures. Confidence: high.

---

plugin.json version bump (L4) and CHANGELOG.md

0.17.3 → 0.17.4. No new permissions, no new MCP servers, no new userConfig keys, no change to the plugin's trust boundary or data-egress surface. CHANGELOG.md is inert documentation. No supply-chain or privilege-escalation risk. Confidence: high.

---

GitHub Actions scope

No workflow files are touched by this PR. No pull_request_target, workflow_run, script injection via github context, or permission-widening changes. Nothing to flag.