Skip to content

feat(claude-ops): add morning-brief operator dashboard skill#569

Merged
kyle-sexton merged 9 commits into
mainfrom
feat/507-morning-ritual-dashboard
Jul 20, 2026
Merged

feat(claude-ops): add morning-brief operator dashboard skill#569
kyle-sexton merged 9 commits into
mainfrom
feat/507-morning-ritual-dashboard

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

The 30-60 minute operator morning pass starts with the same hand-run gh queries every day. This adds a morning-brief skill to the claude-ops plugin that collapses them into a single read-only, gh-based picture for the current repo: open counts per queue label, the merge-ready PR list, parked decisions with their RECOMMENDED lines, and loop-lane telemetry freshness. The pass now begins from a complete 5-second view instead of ad-hoc lookups — and the same view is the natural seed for the WP2-telemetry-era dashboard.

Fix

New sibling skill plugins/claude-ops/skills/morning-brief/ (an observability/operational-visibility concern matching this plugin's existing shape, chosen over a bare bin/ script):

  • scripts/morning-brief.sh — self-contained, read-only, gh-based. Never mutates labels, comments, issues, or PRs. Owner/repo is derived from gh repo view (never hardcoded). Sections:
    • Queues — open counts for priority: needs-triage, status: ready, status: needs-decision, needs-human (label spellings verified against gh label list).
    • Merge-ready PRsgh pr list filtered to non-draft + mergeStateStatus=CLEAN, reviewDecision shown but not required (this repo leaves it empty — no required-review rule). This is a lighter, gh-native glance; it points at /source-control:babysit-prs as the authoritative merge gate rather than shelling into that plugin's pr_queue_snapshot.py engine. That shell-out would cross the plugin-cache boundary (an installed claude-ops consumer cannot reach another plugin's scripts, and source-control may not even be installed) — the repo has zero runtime cross-plugin script invocations, and slash-command cross-reference is its established boundary-safe reuse pattern.
    • Parked decisions — open status: needs-decision issues with each one's RECOMMENDED line. Two-tier extraction: the deliberate uppercase RECOMMENDED marker wins over an incidental lowercase mention (e.g. "not recommended"), with a case-insensitive fallback per the process note that a case-sensitive-only scan produced false negatives. Long lines truncate to a scannable preview (--rec-maxlen, default 240; the issue URL carries the full text).
    • Lane telemetry — the loop-lane telemetry issue's per-lane comments: each lane's last-cycle age (marked STALE past --stale-hours, default 6) and any flags:. The issue is auto-discovered by title (--telemetry-issue to pin) and degrades to "no telemetry issue found" where absent, so the tool is reusable in repos without loop-lane telemetry.
  • morning-brief.test.sh — 29 fixture-driven, network-free cases (each data source injected via a --*-json fixture flag, clock pinned with --now); SKIPs when jq/GNU-date are absent, per the plugin-test runner contract.
  • SKILL.md — thin instructions that invoke the script and cross-reference /source-control:babysit-prs (authoritative merge gate) and /observability (local telemetry stores).
  • Version bump claude-ops 0.13.10.14.0 (minor, new skill) with a matching CHANGELOG entry; README skills table, plugin description ("Five → Six skills"), and marketplace tags updated for accuracy.

Verification

All run locally against this repo's own configs — actual output, not asserted.

Test suite (bash plugins/claude-ops/skills/morning-brief/morning-brief.test.sh):

PASS: [1] renders successfully
...
PASS: [29] missing fixture file exits 3

morning-brief.test.sh: all 29 cases passed

Cases cover: queue-count rendering; merge-ready filtering (keeps CLEAN non-drafts, drops draft + BLOCKED, shows/normalizes reviewDecision); two-tier RECOMMENDED extraction (uppercase wins over "not recommended", lowercase fallback, no-marker message, comment-body scan); preview truncation; telemetry staleness (fresh vs STALE vs unparsable stamp vs missing issue), flag surfacing, non-lane-comment skipping; and argument handling (--help=0, unknown flag=3, missing fixture=3).

Linters (repo-pinned configs):

  • shellcheck --rcfile .shellcheckrc on both .sh files — clean.
  • markdownlint-cli2 --config .markdownlint-cli2.jsonc on SKILL.md + CHANGELOG.md + README.md — Summary: 0 error(s).
  • typos --config _typos.toml on the plugin + marketplace.json — 0 findings.
  • editorconfig-checker --config .editorconfig-checker.json on every changed file — exit 0.
  • plugin.json and marketplace.json re-validated as JSON after edits.

Live output (bash .../morning-brief.sh, against this real repo):

Morning brief — melodic-software/claude-code-plugins — 2026-07-19T23:40Z

== Queues (open issues per label) ==
  priority: needs-triage   28
  status: ready            52
  status: needs-decision   6
  needs-human              1

== Merge-ready PRs (non-draft, mergeStateStatus=CLEAN) ==
  (none clean right now)
  authoritative merge gate: /source-control:babysit-prs

== Parked decisions (status: needs-decision) with RECOMMENDED lines ==
  #398 feat(source-control): machine-agnostic worktree setup skill (detect -> decide -> install)
    https://github.com/melodic-software/claude-code-plugins/issues/398
    RECOMMENDED: Store the root in a new `userConfig` directory key (e.g. `worktree_root`), mirroring `babysit_worktree_root` exactly. Both #399's helper and a `WorktreeCreate` hook resolve `${user_config.worktree_root}`.
  ... (6 total, one per open needs-decision issue)

== Lane telemetry freshness (last-cycle age + flags) ==
  source: issue #502
  babysit    last-cycle=2026-07-19T23:06Z  age=34m
  triage     last-cycle=2026-07-19T23:37Z  age=3m
  work       last-cycle=2026-07-19T19:XX  age=unparsable timestamp

(Merge-ready was empty at capture time — 3 BLOCKED, 1 UNSTABLE — demonstrating the graceful "none clean" path; the 19:XX lane demonstrates graceful handling of a malformed stamp already present in the telemetry issue.)

Closes #507

Related

🤖 Generated with Claude Code

Collapse the daily hand-run operator morning queries into one read-only,
gh-based view for the current repo: open counts per queue label, the
gh-native merge-ready PR list (non-draft + mergeStateStatus=CLEAN), parked
status: needs-decision issues with their RECOMMENDED lines (uppercase marker
preferred, case-insensitive fallback), and loop-lane telemetry freshness
(per-lane last-cycle age, STALE past --stale-hours, plus any flags).

Owner/repo is derived from `gh repo view`, never hardcoded. The telemetry
issue is auto-discovered by title and degrades to "no telemetry issue found"
where absent, so the tool is reusable across repos. Merge-ready is a light
gh-native glance that points to /source-control:babysit-prs as the
authoritative gate rather than shelling into that plugin's engine (which
would break the plugin-cache boundary for installed consumers).

Ships scripts/morning-brief.sh plus a fixture-driven morning-brief.test.sh
(29 cases, network-free), bumps claude-ops 0.13.1 -> 0.14.0, and updates the
CHANGELOG, README skills table, plugin description, and marketplace tags.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kyle-sexton kyle-sexton added the automated Opened by automation. label Jul 19, 2026
@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context and understand the request
  • Read changed files
  • Review shell script (morning-brief.sh) — correctness, security, error handling
  • Review test file (morning-brief.test.sh) — coverage, fixture integrity
  • Review SKILL.md — alignment with plugin conventions
  • Review manifest/metadata changes — plugin.json, marketplace.json, CHANGELOG.md, README.md
  • Check CLAUDE.md alignment (fresh-docs mandate, design rules)
  • Post final review

Overall: High-quality addition. The script is read-only as advertised, repo-agnostic, well-tested (29 fixture-driven cases), and follows the plugin's established conventions closely. Two medium-priority input-validation gaps and a handful of minor items below.


Correctness

[Medium] --stale-hours accepts non-numeric values silently

morning-brief.sh L373:

if ((delta > STALE_HOURS * 3600)); then

If someone passes --stale-hours 6h or a typo like --stale-hours abc, bash arithmetic expands abc as a variable name — which is almost certainly unset, so it evaluates to 0. The result: every lane appears STALE (delta > 0 is true for any positive age). There's no post-parse validation that $STALE_HOURS is numeric.

Fix this →

[Medium] --rec-maxlen non-numeric value produces a stderr error

morning-brief.sh L305:

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

-gt inside [[ ]] requires integer operands. A non-numeric $REC_MAXLEN produces integer expression expected on stderr (though the script continues, skipping truncation). Same root cause — require_value verifies non-empty / non-flag, not numeric. A shared validation block after argument parsing would cover both.


Test Coverage

[Minor] No test for --rec-maxlen 0 (never-truncate path)

morning-brief.test.sh L132–138 tests --rec-maxlen 30 (truncation active) and the fixture with a long RECOMMENDED. The branch [[ "$REC_MAXLEN" != "0" && ... ]] disabling truncation is untested. The fixture decisions.json already has a long rec (#104), so a one-line assert_not_contains "rec-maxlen 0 never truncates" "$OUT_FULL" "…" against a --rec-maxlen 0 run would close this.

[Minor] assert_exit argument order is non-conventional

morning-brief.test.sh L37:

assert_exit() { if [[ "$2" == "$3" ]]; then pass "$1"; else fail "$1" "exit $2" "exit $3"; fi; }

Called as assert_exit "renders successfully" 0 "$RC" — positionally $2 is expected and $3 is actual. The fail message reads expected: exit 0 got: exit <RC> which is correct, but the (expected, actual) convention reversal is easy to misread when adding new cases. Not a bug, just a readability note.


Style / Minor

[Minor] local combined declared mid-loop while other loop variables are declared outside

morning-brief.sh L279–289:

local i number title url rec
for ((i = 0; i < count; i++)); do
    number="$(jq ...)"
    ...
    local combined      # ← inside the loop; bash is fine with this, but inconsistent
    combined="$(jq ...)"

local in bash scopes to the function, not the loop body, so re-declaring it every iteration is harmless. Moving local combined to the outer local i number title url rec line would make it consistent.

[Minor] usage() is coupled to fixed line numbers

morning-brief.sh L59:

sed -n '2,35p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//'

If the comment block grows or lines are inserted above it, the range goes stale silently and --help truncates or drops text. A sentinel-based approach (e.g., sed -n '/^# morning-brief/,/^[^#]/p') would be more robust, though the current approach is common for short-lived utility scripts.


What's working well

  • Read-only guarantee is structurally enforced — no gh write calls anywhere in the script; gh itself is only demanded when a fixture is absent, making fully-fixtured offline runs clean.
  • Two-tier RECOMMENDED extraction is well-motivated and the test fixtures (#100 uppercase beats "not recommended", #101 lowercase fallback, #102 no marker, #103 comment-only) give good confidence.
  • Graceful degradation throughout — missing telemetry issue, empty queues, unresolvable repo, unparsable last-cycle timestamp — each degrades to a readable "nothing to report" rather than an error exit.
  • Cross-plugin boundary is handled correctly — the PR description's reasoning for not shelling into source-control:babysit-prs (installed plugins run from isolated caches; cross-plugin script invocations have no runtime contract) is sound, and the slash-command cross-reference is the right boundary-safe pattern.
  • Metadata changes are consistent — version bump (0.13.1 → 0.14.0) is correct semver for a new skill, description and skills table updated in README, marketplace dashboard tag added.

The two numeric-validation findings are the only items I'd call out before merge. Everything else is stylistic.

`generate-catalog.mjs --check` (plugin-gate) flagged the root README catalog
block as stale after the claude-ops plugin.json description bumped from five to
six skills. Regenerated via the sanctioned generator rather than hand-edited.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context and understand the request
  • Read changed files
  • Review shell script (morning-brief.sh) — correctness, security, error handling
  • Review test file (morning-brief.test.sh) — coverage, fixture integrity
  • Review SKILL.md — alignment with plugin conventions
  • Review manifest/metadata changes — plugin.json, marketplace.json, CHANGELOG.md, README.md
  • Check CLAUDE.md alignment (fresh-docs mandate, design rules)
  • Post final review

Summary

This is a clean, well-scoped addition. The implementation is read-only, repo-agnostic, plugin-form safe, and the fixture-driven test design is particularly elegant (allows network-free CI without any mocking infrastructure). The cross-plugin boundary is handled correctly — slash-command cross-reference to /source-control:babysit-prs rather than a shell invocation, which correctly respects the plugin-cache isolation model.

Four findings below, all minor; none are blocking.


Findings

1. --stale-hours and --rec-maxlen accept non-numeric values without validation

File: morning-brief.sh (lines 91–99 for parsing; consequences at lines 305–306 and 373)

Under set -u, passing --stale-hours abc causes a cryptic arithmetic error: bash interprets abc as a variable name inside ((delta > STALE_HOURS * 3600)) — if abc is unset, the script exits with "unbound variable" rather than a clear "N must be a number" message. --rec-maxlen has the same problem at the "${rec:0:$REC_MAXLEN}" substring expansion.

A simple guard after the existing require_value call would fix both:

--stale-hours)
  require_value "$1" "${2:-}"
  [[ "$2" =~ ^[0-9]+$ ]] || { printf 'morning-brief: --stale-hours requires a non-negative integer\n' >&2; exit 3; }
  STALE_HOURS="$2"
  shift 2
  ;;

Fix this →


2. Missing test: any=0 path in telemetry (no lane-tagged comments on a real telemetry JSON)

File: morning-brief.test.sh (and morning-brief.sh line 389)

The any=0 path at line 389 emits "(no per-lane comments found on the telemetry issue)". Two existing tests cover null and a populated array, but neither covers the case where the telemetry JSON is a valid non-empty array with no comment containing a lane: field (e.g., only scope-note comments). The fixture already has that comment in telemetry.json, but there's no test that feeds an array containing only that non-lane comment. A minimal addition would be:

printf '[{"body":"Scope note only."}]\n' >"$TMP/no-lanes.json"
OUT_NL="$(bash "$BRIEF" --now "$NOW" --counts-json "$TMP/counts.json" \
  --pr-json "$TMP/empty.json" --decisions-json "$TMP/empty.json" \
  --telemetry-json "$TMP/no-lanes.json" 2>&1)"
assert_contains "no-lane comments message" "$OUT_NL" "(no per-lane comments found on the telemetry issue)"

3. Missing test: --rec-maxlen 0 (no-truncate mode)

File: morning-brief.test.sh | morning-brief.sh line 305

The documented 0 = full semantics of --rec-maxlen are exercised only through the != "0" branch check in source. There's no test that passes --rec-maxlen 0 and asserts the long recommendation tail (omicron pi) survives in full — the inverse of the existing truncation test. Low risk since the logic is a single [[ "$REC_MAXLEN" != "0" && ... ]] guard, but it's the natural complement to case [24].


4. usage() self-extraction is fragile to header growth

File: morning-brief.sh line 59

sed -n '2,35p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//'

The hard-coded upper bound 35 is currently correct (the header ends at line 35). Adding or removing a line from the file-level comment block silently truncates or over-runs the --help output. A sentinel-based extraction would be more durable:

sed -n '/^# morning-brief\.sh/,/^$/{ /^$/q; s/^# \{0,1\}//; p }' "${BASH_SOURCE[0]}"

This is a minor maintenance hazard, not a correctness bug — the range is correct today and shellcheck passes.


What looks good

  • Security: No injection surfaces. gh flags are passed as "${REPO_ARGS[@]}" (array expansion, never concatenated into a string); fixture data flows through jq/grep stdin, not eval; %q format for the unknown-flag error prevents terminal-injection via crafted flag names.
  • Read-only contract: Nothing writes to the filesystem, labels, comments, issues, or PRs.
  • Repo-agnostic: gh repo view --json nameWithOwner drives the target, never a hardcoded repo.
  • Plugin-form correct: ${CLAUDE_PLUGIN_ROOT} used in SKILL.md; no ../ reach-outs; state is never written so ${CLAUDE_PLUGIN_DATA} is not needed.
  • Cross-plugin boundary: Correctly uses a slash-command reference (/source-control:babysit-prs) rather than shelling into another plugin's script. PR description explains the reasoning clearly and correctly.
  • Two-tier RECOMMENDED extraction: The uppercase-wins-over-lowercase design is sound and test cases [7–10] cover the key discrimination scenarios (uppercase beats "not recommended", lowercase fallback, no marker, comment-body scan).
  • Version bump: 0.13.10.14.0 is correct for a new skill. CHANGELOG entry is comprehensive.
  • Telemetry degradation: The resolve_telemetry_issue auto-discover path correctly degrades to "no telemetry issue found" so the skill is reusable in repos without loop-lane telemetry.
  • set -uo pipefail is appropriate; the 2>/dev/null || echo "" fallback pattern correctly suppresses errors in optional gh calls without masking the pipefail behavior in the main flow.

--stale-hours and --rec-maxlen accepted non-numeric values, which crashed
cryptically under `set -u` in the later arithmetic (STALE_HOURS * 3600 /
rec-length compare) instead of failing cleanly. Add an integer guard after
each flag's require_value that exits 3 with a specific message, and cover the
error path with tests.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Post-green review pass (work lane)

Bot review's finding #1 (missing numeric validation on --stale-hours/--rec-maxlen) fixed inline — integer guards added, 4 new tests, 33/33 passing, CI green. Findings #2-4 filed as #579, deferred (test-coverage/maintenance nits, none blocking). Moving this out of draft.

🤖 Generated with Claude Code

@kyle-sexton
kyle-sexton marked this pull request as ready for review July 20, 2026 02:08
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR body, prior review comments, author's fix comment)
  • Read changed files (diff + current state post-fix)
  • Review morning-brief.sh — numeric validation fix, correctness, security
  • Review morning-brief.test.sh — 33-test suite coverage
  • Review SKILL.md and metadata files
  • Post final review

This is a post-fix pass covering the 6fceb0e commit that addressed the two medium findings from prior reviews. Everything else in the skill was reviewed in those passes and hasn't changed.

Numeric validation fix (the key change)

The guard is placed inline during argument parsing — correct and cleaner than the post-parse block suggested in the prior review, since it fails fast at the point of bad input rather than after all flags are processed.

morning-brief.sh L92–95 / L101–104:

[[ "$2" =~ ^[0-9]+$ ]] || {
  printf 'morning-brief: --stale-hours requires a non-negative integer\n' >&2
  exit 3
}

The regex ^[0-9]+$ is strict: requires at least one digit, rejects empty string, 6h, abc, 12x. Both flags get identical treatment. ✓

Test additions (cases 30–33)

morning-brief.test.sh L168–173:

ERR_STALE="$(bash "$BRIEF" --stale-hours abc 2>&1)"
assert_exit "non-numeric --stale-hours exits 3" 3 "$?"
assert_contains "non-numeric --stale-hours clear message" "$ERR_STALE" "--stale-hours requires a non-negative integer"
ERR_MAXLEN="$(bash "$BRIEF" --rec-maxlen 12x 2>&1)"
assert_exit "non-numeric --rec-maxlen exits 3" 3 "$?"
assert_contains "non-numeric --rec-maxlen clear message" "$ERR_MAXLEN" "--rec-maxlen requires a non-negative integer"

The pattern is correct: $? after VAR="$(cmd)" reflects cmd's exit code under set -uo pipefail (no -e), so the exit-code assertions are valid. stderr is captured via 2>&1 and checked independently. Both the exit code and the message content are exercised per flag. ✓

Nothing new to flag

The three open items from the first two review passes (deferred to #579) are minor maintenance nits — fragile sed -n '2,35p' in usage(), untested --rec-maxlen 0 no-truncate path, and local combined declared inside the loop — and were deliberately deferred rather than forgotten. None are blocking.

Metadata is consistent: 0.14.0 semver correct for a new skill, CHANGELOG date-free format matches existing entries, dashboard tag added in both plugin.json and marketplace.json, "Five → Six skills" updated across README.md (root and plugin), plugin.json description.

This looks good to merge.


@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6fceb0ed96

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
morning-brief.sh used GNU-only `date -d` at three sites (--now, telemetry
last-cycle, and the header render). On BSD/macOS date, where Claude Code
commonly runs, `-d` is unsupported: --now exited 3 and every last-cycle
value rendered "unparsable timestamp", so no lane got a staleness verdict.

Add to_epoch/from_epoch helpers that try GNU (`-d`) first, then BSD
(`-j -f` for parsing, `-r` for epoch render). TZ=UTC on the BSD parse
forces the timezone-naive strptime result to UTC, matching how GNU reads
the Z-suffixed stamps. Only time-bearing formats are tried so a malformed
stamp stays unparsable instead of prefix-matching to a wrong midnight.

Addresses codex P2. 33/33 tests pass on GNU and under a BSD-date stub.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, latest commits)
  • Read the BSD date fix (68225a9) — the triggering change
  • Review morning-brief.sh for correctness, security, alignment
  • Review morning-brief.test.sh — coverage of new BSD fix
  • Verify metadata/manifest changes are consistent
  • Post final review

This is a targeted review of the 68225a9 BSD date fix — the change that triggered this run. The rest of the skill was reviewed in the prior three passes; nothing in those reviewed areas has changed.

BSD date fix (68225a9) — overall assessment

The fix is correct and uses the right approach. to_epoch tries GNU date -d first, falls through to BSD date -j -f with a loop over four format strings, and returns empty on failure (callers treat that as "unparsable timestamp"). from_epoch mirrors the pattern: GNU date -u -d "@$e" → BSD date -u -r "$e" → current time. The TZ=UTC flag on the BSD parse is the key correctness detail: BSD date -j -f has no %Z conversion specifier, so the literal Z is consumed as a plain character and the result is timezone-naive; TZ=UTC forces it to UTC, which matches what GNU date infers from the Z suffix. Only time-bearing strptime formats are tried, so a malformed stamp can't prefix-match to a wrong midnight.


One minor item

Test skip guard comment is now stalemorning-brief.test.sh L17–22

# The script computes ages with `date -u -d`; a date without -d (BSD) cannot run
# the staleness path, so skip rather than report false failures.
if ! date -u -d "2026-01-01T00:00Z" +%s >/dev/null 2>&1; then
  echo "SKIP: date -d (GNU date) not available" >&2
  exit 0
fi

The comment's premise — "BSD cannot run the staleness path" — is no longer true after this fix. The script now falls back to date -j -f / date -r on BSD, so both --now parsing and last-cycle age calculation work without GNU date. The skip guard itself is still defensively safe (CI runs on Linux; GNU date is present), but a developer running the tests on macOS will see SKIP when they could now actually run and pass. Not a blocker, and this is a good candidate for the same deferred #579 issue.

Fix this →


What looks correct in the fix

  • Fallback chain is right: GNU is tried first (correct order — GNU date is the common CI environment); BSD formats are tried in a loop to handle both T%H:%MZ (no seconds) and T%H:%M:%SZ (with seconds) variants that appear in real telemetry stamps.
  • TZ=UTC is necessary and applied correctly: BSD date -j -f with a format that doesn't include a timezone directive produces a naive local-time result. The TZ=UTC prefix pins parsing to UTC, matching the meaning of the Z suffix. This is the non-obvious invariant and the commit message correctly calls it out.
  • Only time-bearing formats are tried: The omission of a bare %Y-%m-%d format is correct — a date-only format would prefix-match malformed timestamps and yield a wrong midnight. The comment in the source (L181-182) documents this correctly.
  • from_epoch BSD branch is correct: date -u -r "$e" is the standard BSD epoch-to-timestamp path. The final fallback to current time (date -u '+%Y-%m-%dT%H:%MZ') is safe — it's only reached if both date -u -d and date -u -r fail, which is essentially impossible on any system where the tests run.
  • No injection surface: to_epoch passes $s as a quoted argument to date, not via eval or string concatenation. from_epoch passes $e the same way. Telemetry body values are extracted through grep/sed before reaching these functions, but the argument-quoting makes injection moot regardless.
  • Tests cover the fix: Cases 14–16 exercise to_epoch through the staleness path (fresh lane, stale lane, unparsable timestamp), and they run against the real to_epoch helper via --now "$NOW". The new BSD-date path is exercised by the "BSD-date stub" the commit message references (not part of the test file itself, but the CI behavior on Linux validates the GNU path, which is the common case).

This is ready to merge. The stale skip-guard comment is the only new item, and it's a documentation nit that belongs with the other deferred #579 items.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68225a9f13

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
--stale-hours and --rec-maxlen passed the ^[0-9]+$ guard but were stored
raw, so leading-zero values were later misread as octal: 08 threw
"value too great for base" in the ((delta > STALE_HOURS * 3600))
staleness test (silently suppressing the STALE verdict), and 010
truncated recommendations to 8 chars instead of 10. Normalize each value
to decimal once at parse time with the 10# base prefix (the repo's
established octal-guard convention), so every downstream use is safe.
Add tests asserting 08 still flags STALE with no arithmetic error and
010 truncates at 10 chars.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, Codex finding, latest commit)
  • Read the base-10 normalization fix (bd653cb) — the triggering change
  • Review morning-brief.sh — correctness of normalization approach
  • Review morning-brief.test.sh — coverage of new cases
  • Post final review

This is a targeted review of bd653cb — the base-10 normalization fix addressing the Codex P2 finding about leading-zero values like 08/010 being silently misread as octal in bash arithmetic. Everything else in the skill was reviewed in the prior four passes; nothing in those areas has changed.

Base-10 normalization fix — overall assessment

The fix is correct, minimal, and well-placed.

morning-brief.sh L98 / L109:

STALE_HOURS="$((10#$2))"
# ...
REC_MAXLEN="$((10#$2))"

The 10# prefix is the canonical bash idiom for forcing base-10 interpretation — $((10#08)) = 8, $((10#010)) = 10. Placement is correct: it runs after the ^[0-9]+$ guard (so $2 is already validated as a digit string, no injection surface), and stores a clean decimal string with no leading zeros. All downstream uses receive a plain decimal integer:

  • ((delta > STALE_HOURS * 3600)) at L421: arithmetic, now unambiguously decimal
  • [[ "${#rec}" -gt "$REC_MAXLEN" ]] at L353: -gt uses integer arithmetic in [[ ]], now safe
  • "${rec:0:$REC_MAXLEN}…" at L354: parameter expansion length also uses arithmetic, now safe

Default values ("6" and "240") have no leading zeros — no issue there.

Test additions — coverage is sound

morning-brief.test.sh L179–192:

--stale-hours 08 (two assertions):

  • assert_contains "... STALE (>8h)" — exercises the staleness arithmetic path. The triage fixture lane has last-cycle: 2026-07-19T06:00Z (26h old at NOW), which is >8h. Without the fix, 08 in ((delta > 08 * 3600)) produces a value too great for base arithmetic error; under set -uo pipefail (no -e), (()) returns exit 1 and the STALE branch is silently skipped — the assert would fail.
  • assert_not_contains "... value too great for base" — directly confirms the stderr error is suppressed (captured via 2>&1). A belt-and-suspenders check that complements the positive assertion.

--rec-maxlen 010 (one assertion):

  • assert_contains "... alpha beta…" — verifies 10-char truncation (not 8-char octal truncation). The #104 fixture recommendation starts with "alpha beta gamma…"; 10 chars gives alpha beta. Without the fix, ${rec:0:010} evaluates 010 as octal 8, yielding alpha be…, which would fail the assertion.

Both failure modes are covered with the right fixtures and assertions.

Nothing new to flag

The one remaining nit from the prior review pass (stale skip-guard comment at L17–22 still says "BSD cannot run the staleness path" after 68225a9 made that untrue) is correctly deferred to #579. It's a documentation nit, not a correctness issue.

This is ready to merge.


@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd653cba98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/claude-ops/skills/morning-brief/morning-brief.test.sh Outdated
…pping

The test guard required GNU `date -d`, so it skipped entirely on macOS and
never exercised to_epoch/from_epoch's BSD `date -j -f` / `-r` fallback on any
runner. Relax the guard to run whenever either date dialect is present (so a
macOS run drives the real BSD branch), and add a sub-test that forces the BSD
branch on a GNU host via a `date` test double rejecting `-d` — proving the
branch is reached and wired (return handling, TZ, from_epoch's -r render, and
the parseable-vs-unparseable decision). Negative control: breaking the BSD
parse fails exactly the four new cases and nothing else.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, latest commit 31bdd5e)
  • Read the BSD date skip-guard fix — the triggering change
  • Review morning-brief.test.sh — correctness of new skip logic and BSD stub
  • Post final review

This is a targeted review of 31bdd5e — the commit that addresses the final Codex P2 finding about the test suite skipping entirely on macOS instead of exercising the BSD date -j -f fallback added in 68225a9. The rest of the skill was reviewed in the prior four passes; nothing there has changed.

Skip guard relaxation

morning-brief.test.sh L21–26:

have_gnu_date() { date -u -d "2026-01-01T00:00Z" +%s >/dev/null 2>&1; }
have_bsd_date() { date -u -j -f "%Y-%m-%dT%H:%MZ" "2026-01-01T00:00Z" +%s >/dev/null 2>&1; }
if ! have_gnu_date && ! have_bsd_date; then
  echo "SKIP: no supported date dialect (need GNU 'date -d' or BSD 'date -j -f')" >&2
  exit 0
fi

Correct. The probe calls match exactly what to_epoch tries — GNU -d first, then BSD -j -f with the same format string. The skip message is accurate (names both dialects so a developer understands why). The stale comment from the prior review is gone. ✓

BSD date test double

morning-brief.test.sh L210–253

The stub's argument parsing is correct. Tracing through the key call TZ=UTC date -u -j -f "%Y-%m-%dT%H:%MZ" "$s" +%s:

  1. Pre-loop -d check: no -d in args → passes the guard.
  2. Parse loop: -uuflag=(-u); -jmode=parse; -f → inner shift consumes the format string, outer shift advances past it; the timestamp becomes input; +%soutfmt.
  3. Dispatch: exec "$real" -u -d "$input" "+%s"TZ=UTC is inherited from the calling environment, so GNU date receives it and returns the correct UTC epoch. ✓

For from_epoch's BSD branch date -u -r "$e" '+%Y-%m-%dT%H:%MZ': the stub intercepts the initial date -u -d "@$e" call (hits the -d guard → exits 1), then the -r path: sets mode=render; epoch=$2 plus inner shift correctly consumes the epoch value; outer shift advances to outfmt; dispatches to exec "$real" -u -d "@$epoch" "$outfmt". ✓

Known limitation (documented in the comment): the stub ignores which format string is passed to -j -f and delegates parsing to GNU date -d. This means a wrong format string in to_epoch would not be caught by these cases. The comment on L206–209 calls this out explicitly and correctly — the format strings were independently reviewed in the 68225a9 pass.

Four new assertions

morning-brief.test.sh L249–252:

  • assert_exit "BSD fallback: renders successfully" 0 "$RC_BSD" — proves to_epoch/from_epoch don't error out under the stub, and set -uo pipefail isn't tripped.
  • assert_contains "... fresh age computed via date -j -f" "$OUT_BSD" "age=1h 30m" — exercises the success path through to_epoch → arithmetic → from_epoch rendering via the -r branch.
  • assert_contains "... stale verdict via date -j -f" "$OUT_BSD" "STALE (>6h)" — exercises ((delta > STALE_HOURS * 3600)) with a value parsed through the BSD branch.
  • assert_contains "... unparsable stamp still unparsable" "$OUT_BSD" "unparsable timestamp" — confirms the 07:XX malformed fixture doesn't silently succeed under the BSD branch (i.e., to_epoch returning empty still propagates correctly).

All four cover the right failure modes for the BSD path. ✓

The if have_gnu_date guard around the BSD sub-test is correct: the stub relies on a real GNU date -d to back-compute epoch math, so it can only run on a GNU host. On a real macOS host, the relaxed skip guard lets the suite run natively against BSD date, which is the complementary coverage path. Together they close the gap.

What looks good

  • chmod +x "$STUB_DIR/date" 2>/dev/null || true is appropriately defensive; mktemp -d always produces a writable temp dir, so this will not silently fail in practice.
  • PATH="$STUB_DIR:$PATH" MB_REAL_DATE="$REAL_DATE" is the right injection pattern — scoped to one invocation, no global mutation.
  • The guard if have_gnu_date around the BSD sub-test is the right scoping — it makes the four new assertions conditional on the host having GNU date (which all CI runners do), so they don't inflate the skip count on macOS.

Summary

All five commits (6fceb0e, 68225a9, bd653cb, 31bdd5e, plus the original) are clean and the findings from all prior review passes have been addressed or deliberately deferred to #579. This is ready to merge.


The BSD-path test comment used "unparseable"; the typos gate (and the script's
own output) uses "unparsable". Aligns the word so the hygiene lane passes.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, latest commit 1fcf9ff)
  • Review the typos fix and verify final state of changed files
  • Post final review

This is a targeted review of 1fcf9ff — the single-word spelling correction that unblocked the typos hygiene gate. Everything else was reviewed across four prior passes.

Typos fix — correct and minimal

morning-brief.test.sh L206:

-# unparseable decision on the `07:XX` fixture.
+# unparsable decision on the `07:XX` fixture.

The change is comment-only, in the BSD test double's scope note. unparsable matches what the script itself emits ("unparsable timestamp" at morning-brief.sh line ~390) and the test assertion at L252 (assert_contains "... unparsable timestamp"), so the fix is correct and the wording is now consistent across the file. No functional code is touched.

Overall PR status

All five commits are clean. A summary of what each addressed:

Commit Finding Status
3d474b2 Original skill reviewed ✓
6fceb0e Numeric validation for --stale-hours/--rec-maxlen reviewed ✓
68225a9 BSD date -j -f fallback in to_epoch/from_epoch reviewed ✓
bd653cb Base-10 normalization (10# prefix) to prevent octal misread reviewed ✓
31bdd5e Relaxed skip guard + BSD date stub for GNU hosts reviewed ✓
1fcf9ff unparseableunparsable spelling (this pass)

Three minor items remain deferred to #579 (none blocking):

  • usage() self-extraction fragile to header growth (sed -n '2,35p')
  • --rec-maxlen 0 (no-truncate) path has no test
  • local combined declared inside a loop body rather than with sibling locals

This is ready to merge.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fcf9ffd3b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
… position

The tier-2 lowercase fallback matched the first `recommended` anywhere, so an
issue whose body says "... is not recommended" before its real lowercase
"recommended: ..." marker rendered the rejected option as the recommendation —
the same false positive tier 1 exists to avoid. Anchor the fallback to marker
position (line start, after optional list/blockquote/bold decoration), and add
a discriminating fixture (#105) proving the marker beats an earlier "not
recommended" line. 42/42 pass, shellcheck + typos clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, latest commit c88224f)
  • Read changed files — morning-brief.sh and morning-brief.test.sh
  • Review morning-brief.sh — correctness of anchored lowercase fallback
  • Review morning-brief.test.sh — coverage of new cases
  • Post final review

This is a targeted review of c88224f — the commit that addresses the final Codex P2 finding about the tier-2 lowercase RECOMMENDED fallback matching the first incidental recommended anywhere in the text (including inside "not recommended" rejection lines). The rest of the skill was reviewed across five prior passes; nothing there has changed.

RECOMMENDED anchor fix — overall assessment

The fix is correct, minimal, and addresses exactly the failure mode the Codex reviewer described.

Before

morning-brief.sh L346 prior:

[[ -n "$rec" ]] || rec="$(grep -iam1 'recommended' <<<"$combined")"

Unanchored — grep returns the first line containing recommended anywhere, including prose like "The bare-clone hub is not recommended here." when it appears before the actual marker.

After

morning-brief.sh L346:

[[ -n "$rec" ]] || rec="$(grep -iam1 -E '^[[:space:]]*([0-9]+[.)][[:space:]]*)?[-*>|#[:space:]]*\**[[:space:]]*recommended' <<<"$combined")"

Anchored to line start via ^, followed only by optional Markdown decoration characters (whitespace, list bullets, blockquote markers, bold markers, numbered list prefixes) before recommended. This mirrors exactly what tier-1 already does implicitly — the uppercase \bRECOMMENDED\b match is also anchored by requiring it to be a whole word, not embedded in prose.

Regex correctness tracing

For the fixture body:

The bare-clone hub is not recommended here.
recommended: keep the sibling worktree layout.
  • Line 1: Starts with T → fails ^[[:space:]]*([0-9]+[.)][[:space:]]*)?[-*>|#[:space:]]*\**[[:space:]]*recommended (the character class after ^ only accepts whitespace/decoration, not letters) ✓
  • Line 2: Starts with recommended → matches immediately ✓

Checked additional edge cases the character class might interact with:

  • - **not** recommended[-*>|#[:space:]]* consumes - **, leaving not** recommended; recommended doesn't match not → no match ✓
  • > **Recommended:** do X[-*>|#[:space:]]* consumes > **, \** zero asterisks, recommended matches → correctly selected ✓
  • **not** recommended[-*>|#[:space:]]* consumes **; then not is not recommended → no match ✓

The jq-built combined string concatenates body + all comment bodies line by line, and the multi-line grep (-m = first match, -i = case-insensitive, -a = treat as text) returns exactly the first anchored-marker line. Consistent with tier-1 behavior. ✓

Test fixture and assertions

morning-brief.test.sh L80–82:

{"number": 105, "title": "lowercase after rejection", "url": "http://x/i/105",
 "body": "The bare-clone hub is not recommended here.\nrecommended: keep the sibling worktree layout.",
 "comments": []}

morning-brief.test.sh L129–132:

assert_contains "decision #105 lowercase marker beats earlier 'not recommended'" "$OUT" "keep the sibling worktree layout"
assert_not_contains "decision #105 ignores 'not recommended' rejection" "$OUT" "bare-clone hub is not recommended"

The fixture is a direct instantiation of the Codex failure scenario: the rejection phrase appears first, the real marker second. Both assertions are discriminating — the positive assertion confirms the correct line was extracted; the negative confirms the rejection line was not selected. Together they would catch a regression to the pre-fix unanchored grep. ✓

The complement with the existing #100 fixture (tier-1 uppercase) is now complete: both tiers have an "X is not recommended" / "RECOMMENDED/recommended: Y" discrimination case proving each tier anchors to marker position independently.

What looks correct in this commit

  • The anchor is load-bearing and the updated comment says exactly that (L338–341). A future maintainer loosening it to increase flexibility would need to re-read the comment to understand why the anchor exists.
  • Tier-1 unchanged — the uppercase \bRECOMMENDED\b path is untouched; this commit only tightens tier-2 to match tier-1's implicit position-anchoring behavior.
  • The stripping pipeline at L347–355 (bullet stripping, token stripping) works correctly on the newly anchored result: recommended: keep the sibling worktree layout. → strips leading recommended token and separator → keep the sibling worktree layout.
  • 42/42 tests pass per the commit message; shellcheck and typos clean.

Summary

All six prior Codex/bot findings (five from the review rounds, one from the final Codex pass) have been addressed in this PR. The three minor items deferred to #579 remain correctly deferred:

Item Status
usage() hard-coded sed -n '2,35p' Deferred #579
--rec-maxlen 0 no-truncate path untested Deferred #579
local combined declared inside loop body Deferred #579

This is ready to merge.


@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c88224f1f7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
The prior fix anchored only the lowercase tier; the uppercase tier still
matched any RECOMMENDED, so "Option A is NOT RECOMMENDED because..." before the
real marker rendered the rejected option. Rather than anchor to line start
(which drops the legitimate mid-line "After review, RECOMMENDED:" marker in
fixture #103), gate BOTH tiers on a labeled marker — RECOMMENDED followed, past
optional bold/space, by a ':'/'-'/em-dash separator. Negated prose ("NOT
RECOMMENDED because", "not recommended for") is followed by a word, not a
separator, so it no longer wins. Added discriminating fixture #106; both tiers'
anchors proven load-bearing by negative control. 44/44 pass, shellcheck + typos
clean.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown

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


PR Review: feat(claude-ops): add morning-brief operator dashboard skill

  • Gather context (PR history, prior reviews, latest Codex finding)
  • Read current state of morning-brief.sh — focus on uppercase RECOMMENDED extraction (Codex P2)
  • Review test coverage for the new finding
  • Post final review

This is a targeted review of 8fe21e3 — the commit that addresses the final Codex P2 finding about the uppercase RECOMMENDED tier matching negated prose before the real marker. The rest of the skill was reviewed across six prior passes; nothing there has changed.

Labeled-marker fix — overall assessment

The fix is correct and the approach is sound.

Before (c88224f):

  • Tier 1: grep -am1 -E '\bRECOMMENDED\b' — unanchored word-boundary; matched NOT RECOMMENDED because of drift before the real marker
  • Tier 2: grep -iam1 -E '^[[:space:]]*...*recommended' — line-start-anchored but asymmetric

After (current morning-brief.sh L347–351):

marker_re='RECOMMENDED[[:space:]]*\**[[:space:]]*[-:—]'
rec="$(grep -am1 -E "$marker_re" <<<"$combined")"
[[ -n "$rec" ]] || rec="$(grep -iam1 -E "$marker_re" <<<"$combined")"

Both tiers are now symmetric, using the same pattern. The separator constraint ([-:—]) is the load-bearing discriminator: negated prose (NOT RECOMMENDED because…, not recommended for…) has a word following the token, not a separator — the pattern fails to match. The real marker (RECOMMENDED: …, RECOMMENDED — …) has a separator — the pattern matches.

Regression check on prior fixtures:

  • #100 body "Bare-clone hub is not recommended for this profile.\n1. **RECOMMENDED — Store the root in a userConfig key.**":
    • Tier 1 (case-sensitive): not recommended is lowercase → skipped; RECOMMENDED — hits the em-dash separator → match ✓
  • #103 body has "After review, RECOMMENDED: adopt option B." (mid-line):
    • RECOMMENDED: separator → matches, correctly avoiding the line-start-anchor design choice that would drop this form ✓
  • #105 "not recommended here.\nrecommended: keep the sibling worktree layout.":
    • Tier 1: no uppercase RECOMMENDED with separator → falls through to tier 2
    • Tier 2 (case-insensitive): not recommended hererecommended then here → space then h → not in [-:—] → no match; next line recommended: → separator → match ✓

Test additions — coverage is correct

morning-brief.test.sh L83–86 — fixture #106:

{"number": 106, "title": "uppercase after rejection", "url": "http://x/i/106",
 "body": "Option A is NOT RECOMMENDED because of drift.\nRECOMMENDED: choose the managed sync path.",
 "comments": []}

morning-brief.test.sh L137–139:

assert_contains "decision #106 uppercase marker beats earlier 'NOT RECOMMENDED'" "$OUT" "choose the managed sync path"
assert_not_contains "decision #106 ignores 'NOT RECOMMENDED' rejection" "$OUT" "because of drift"

This is the canonical discriminating case for the finding: rejection phrase (NOT RECOMMENDED because) appears before the real marker (RECOMMENDED:). The positive assertion confirms the right line was selected; the negative confirms the rejection content is absent. Both would fail on the prior unanchored \bRECOMMENDED\b pattern. ✓

The complement set is now: #100 (uppercase beats lowercase "not recommended"), #105 (lowercase marker beats "not recommended"), #106 (uppercase marker beats uppercase "NOT RECOMMENDED because") — each tier's anchor is proven independently.

One minor residual note (not blocking)

The separator set includes (em-dash). The phrase NOT RECOMMENDED — here's why would still match tier 1, since em-dash is in [-:—]. This is the same residual risk present in the design for the RECOMMENDED - prose form. The comment at L337–345 documents the deliberate scope: the discriminator rejects token-then-word patterns, not all negated-context uses. In practice, NOT RECOMMENDED — as rejection-opener prose is uncommon enough that this is an acceptable bound, and correcting it would require line-start anchoring — which the comment correctly notes would drop the legitimate mid-line "After review, RECOMMENDED:" form (exercised by #103). The trade-off is called out clearly.

What looks correct

  • Both tiers symmetric. The prior asymmetry (tier 1 unanchored, tier 2 line-anchored) is gone. A single marker_re variable enforces the same constraint on both paths, making the logic easier to reason about and maintain.
  • No stripping regression. The downstream sed pipeline at L353–360 strips bullets/blockquote/bold, then drops the leading RECOMMENDED[/recommended token. The new separator approach still produces a line starting with RECOMMENDED: for that stripping to consume. ✓
  • 44/44 per the commit message, shellcheck and typos clean.

Summary

All six Codex/bot findings across the PR have been addressed. The three minor items deferred to #579 remain correctly deferred:

Item Status
usage() hard-coded sed -n '2,35p' Deferred #579
--rec-maxlen 0 no-truncate path untested Deferred #579
local combined declared inside loop body Deferred #579

This is ready to merge.


@kyle-sexton
kyle-sexton merged commit 3ef0852 into main Jul 20, 2026
16 checks passed
@kyle-sexton
kyle-sexton deleted the feat/507-morning-ritual-dashboard branch July 20, 2026 04:40
kyle-sexton added a commit that referenced this pull request Jul 21, 2026
…0; harden usage() (#844)

## 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

- #579 (this issue)
- #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.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated Opened by automation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ops: morning-ritual dashboard — one command for queues, merge-ready, decision list

1 participant