Skip to content

fix(claude-ops): parameterize morning-brief queue labels with repo-aware degradation - #2540

Merged
kyle-sexton merged 6 commits into
mainfrom
cursor/fix-610-morning-brief-taxonomy-2ae1
Aug 13, 2026
Merged

fix(claude-ops): parameterize morning-brief queue labels with repo-aware degradation#2540
kyle-sexton merged 6 commits into
mainfrom
cursor/fix-610-morning-brief-taxonomy-2ae1

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

The morning-brief Queues section no longer hardcodes the melodic-software queue label taxonomy on live runs. Default labels are unchanged, but the script now filters to labels that actually exist in the target repo (same degrade-when-absent pattern as the telemetry-issue path). Consuming repos with a different labeling scheme no longer see misleading 0/? count rows.

Changes

  • Live queue counts filter against gh label list (or --repo-labels-json in tests)
  • Degrade to "no queue labels found" when none of the configured defaults exist
  • Add --queue-labels (comma-separated pin) and --decision-label flags
  • Parked-decisions section degrades when the decision label is absent
  • Bump claude-ops to 0.31.12 (main already shipped 0.31.11 via fix(claude-ops): stop sync writing committed settings, restoring the invariant #2539)
  • Regression tests for degradation, partial overlap, and pinned labels

Test plan

  • bash plugins/claude-ops/skills/morning-brief/morning-brief.test.sh (81 cases)

Related

No linked issue

Fixes #610

…are degradation

Live runs filter default queue labels to those that exist in the target
repo, mirroring the telemetry-issue auto-discover/degrade pattern. Add
--queue-labels and --decision-label pins for consuming repos with a
different taxonomy.

Fixes #610

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@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: 3fa6f30c3b

ℹ️ 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
Comment thread plugins/claude-ops/skills/morning-brief/morning-brief.test.sh
@kyle-sexton kyle-sexton added the priority: needs-triage Default until a priority tier is assigned. label Aug 13, 2026
Rebase onto main (which shipped sync fix as 0.31.11) and carry the
morning-brief taxonomy fix as 0.31.12.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 3m 9s —— View job


Code review complete

  • Gather context (PR diff, changed files, CLAUDE.md)
  • Review morning-brief.sh changes for correctness
  • Review morning-brief.test.sh changes for correctness
  • Review docs/changelog/plugin.json changes
  • Post findings

Ran /review:code-review against origin/main...HEAD. Two correctness/coverage findings, posted inline:

  1. fetch_repo_label_names can't distinguish "lookup failed" from "lookup succeeded and found zero labels" (scripts/morning-brief.sh:333). Both callers treat an empty raw as "unavailable" and fall back to showing all configured labels — the exact misleading 0/? rendering this PR is meant to eliminate — for a repo whose actual label set is empty (or a --repo-labels-json of []). Confirmed with a direct jq check that [] in produces 0 bytes of raw out.
  2. Three new label-fixture test cases (OUT_NO_QL, OUT_CUSTOM_QL, OUT_NO_DEC) still hit the live gh repo view auto-resolution path (morning-brief.test.sh:320, also lines 343 and 353) because each omits --counts-json/--decisions-json, which trips ANY_LIVE regardless of --repo-labels-json being present. In a checkout without an authenticated, resolvable repo this fails with could not resolve owner/repo before the label short-circuit is ever reached — traced through the script's own ANY_LIVE gate at scripts/morning-brief.sh:209-223. Passing an explicit --repo placeholder to these three cases would keep them fully offline.

Both align with (and were independently traced/verified against the script logic, not just restated from) the two Codex review comments already on this PR.

Everything else — the --queue-labels/--decision-label flag wiring, docs, CHANGELOG.md, and the 0.31.12 version bump — looks correct and consistent.

Note: this lane's allowed-tools only grants gh pr diff/view/comment/review plus read-only file tools, so I could not execute morning-brief.test.sh or run gh/jq probes directly to reproduce the test failures live — the above is from static trace analysis of the script's control flow, cross-checked with one narrow jq probe that was permitted. The author's PR description's own "81 cases passing" claim is unverified by me for the same reason.

Branch: cursor/fix-610-morning-brief-taxonomy-2ae1

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 1m 12s —— View job


Security review (/review:security-review)

  • Gather context (PR diff, files, custom instructions)
  • Run /review:security-review per repo custom instructions
  • Post findings

No security issues found.

This PR is a read-only gh-based CLI script change (morning-brief). Reviewed the full diff of morning-brief.sh plus the test/doc/manifest changes for logic flaws, injection surfaces, and trust-boundary issues:

  • Command construction is injection-safe. New --queue-labels/--decision-label values and labels read via --repo-labels-json flow into gh issue list --label "$label" through quoted bash array elements (REPO_ARGS, per-iteration "$label"), never through eval or unquoted expansion. jq lookups use --arg/--argjson binding (e.g. label_exists_in_repo), not string interpolation, so operator/fixture-supplied label text can't break out into jq or shell syntax.
  • No new trust boundary. Label values originate from CLI flags (operator-controlled) or local fixture files gated by the existing require_file check — same trust level as the pre-existing --telemetry-issue/--counts-json flags this PR mirrors.
  • No secrets, auth, permissions, or GitHub Actions workflow changes — this PR touches only the skill script, its test suite, SKILL.md, CHANGELOG, and a plugin version bump.
  • The pre-existing GraphQL call in print_stranded (unchanged by this PR) correctly binds $owner/$name via -F rather than string-interpolating into the query.

Two correctness bugs were already flagged by Codex's review (empty label list mishandled as a lookup failure; new test fixtures missing --counts-json/--decisions-json causing live gh calls in offline CI) — these are functional/test issues, not security findings, so they're out of scope for this security-only lane (see /review:code-review for that class of feedback).

@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

Last security-reviewed head: a329ea6a1d0b970a1c998da38121b8cdb945dca6. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
Comment thread plugins/claude-ops/skills/morning-brief/morning-brief.test.sh
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude has reviewed this PR 5 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 4m 0s —— View job


Code review (/review:code-review)

  • Gather context (PR diff, files, custom instructions)
  • Run /review:code-review per repo custom instructions
  • Post findings

Reviewed the full diff against origin/mainmorning-brief.sh, morning-brief.test.sh, SKILL.md, CHANGELOG.md, plugin.json. Scope: correctness/tests/docs (security is owned by the separate /review:security-review lane per this repo's REVIEW.md, which already ran and found no issues here).

Two correctness findings, posted inline (both independently verified by reading the code paths, not just restated from Codex's earlier pass — though they land on the same two spots):

  1. morning-brief.sh:333fetch_repo_label_names returns failure whenever the parsed label list is empty, so a repo that genuinely has zero labels (or a --repo-labels-json of []) is treated the same as an unresolvable lookup. Both print_queues and print_decisions then fall back to showing/querying every default label instead of degrading — reproducing the exact "misleading counts for labels that don't exist" bug this PR sets out to fix.
  2. morning-brief.test.sh:320-358OUT_NO_QL, OUT_CUSTOM_QL, and OUT_NO_DEC omit --counts-json/--decisions-json, which trips ANY_LIVE=1 and forces a live gh repo view before the --repo-labels-json fixture can short-circuit anything — breaking the offline-fixture convention every other case in this file follows (OUT_PARTIAL_QL right above does it correctly). This fails in an environment without a resolvable/authenticated repo, and is nondeterministic even where gh does resolve.

No other high-signal issues found — the injection-safety/quoting the security lane already verified checked out on read-through, and the rest of the diff (SKILL.md, CHANGELOG, version bump) is documentation/metadata with no logic to review.

Note: this lane's allowed-tools don't include running the test suite directly, so the 81 cases test-plan claim above is author-reported and not independently re-run here; finding 2 is a static read of the control flow, not an observed failure.
· branch cursor/fix-610-morning-brief-taxonomy-2ae1

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh Outdated
Comment thread plugins/claude-ops/skills/morning-brief/morning-brief.test.sh
Treat a successful empty label inventory as [] rather than a failed lookup,
and keep the new label-fixture cases fully offline with --repo plus the
remaining JSON fixtures.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 4m 6s —— View job


Code review (/review:code-review)

  • Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
  • Review latest fix commit (32f443e) against prior findings
  • Check for new issues introduced by the fix
  • Post findings

Scope: correctness/tests/docs (security is owned by the separate /review:security-review lane per REVIEW.md, which already ran clean on this PR).

Both prior findings are resolved by 32f443e:

  1. Empty-label-list lookup vs. failed lookup (morning-brief.sh:326-341) — fetch_repo_label_names now emits a valid [] on a successful-but-empty parse (jq -R -s 'split("\n") | map(select(length > 0)) | unique'), and both callers (print_queues:360-364, print_decisions:434-437) now branch on the function's exit status, not on -n "$repo_labels". Confirmed a genuine gh/jq source failure still returns non-zero via pipefail. Regression tests OUT_EMPTY_LABELS/OUT_EMPTY_LABELS_DEC cover it directly.
  2. New label-fixture cases hitting the live gh pathOUT_NO_QL, OUT_CUSTOM_QL, and OUT_EMPTY_LABELS now supply all four of --counts-json/--pr-json/--decisions-json/--telemetry-json, so ANY_LIVE=0 and they're fully offline; no gh call at all.

One residual gap found and posted inline on morning-brief.test.sh:361-368: OUT_NO_DEC and OUT_EMPTY_LABELS_DEC still omit --decisions-json, which trips ANY_LIVE=1 and therefore still requires the gh binary to be installed (though no longer a live, resolvable repo, since --repo is now pinned). That's a smaller residual version of the same issue, and breaks this test file's own stated "no network, no gh required" invariant in a gh-less environment. Trivial fix: add --decisions-json "$TMP/empty.json" to both blocks — the label-not-found short circuit runs before DECISIONS_JSON is ever read, so this doesn't change what's being tested.

No other issues found in the fix commit — plugin.json/CHANGELOG.md/SKILL.md are unchanged by this commit and were already reviewed clean in earlier rounds.

Note: this lane's allowed-tools don't include running the test suite directly, so this is a static trace of the control flow (ANY_LIVE gate, pipefail semantics, jq behavior on [] input), not an observed test run.
· branch cursor/fix-610-morning-brief-taxonomy-2ae1

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 1m 43s —— View job


Security review (/review:security-review)

  • Gather context (PR diff, changed files, custom instructions)
  • Run /review:security-review per repo custom instructions
  • Post findings

No security issues found.

Per the lane's skip gate, this pass focuses on what changed since the last-reviewed head (60f4fa08ab15dfaf030f9c6b12a5ae08a3b3ff72, per the bot's "Last security-reviewed head" note above) — the diff to current HEAD (32f443e6a43a849211650d509ef10f5267d8c6f1) touches only:

  • morning-brief.sh — reworks fetch_repo_label_names/label_exists_in_repo to distinguish a genuinely-empty label list from a failed lookup (via return-code + a labels_available flag in print_queues/print_decisions), addressing the correctness bug flagged by both Codex and the prior Claude code-review passes.
  • morning-brief.test.sh — adds --repo "$FIXTURE_REPO" and the missing --counts-json/--decisions-json fixtures to the three cases that were tripping the live ANY_LIVE/gh repo view path, plus two new empty-label-inventory regression cases.

This is a control-flow/return-code fix with no new external input path: no new shell interpolation, no new gh/jq invocations beyond what was already reviewed, and no change to how label values reach the command line (still bound via quoted array elements and jq --arg, as confirmed in the earlier pass on this PR). No new trust boundary, no secrets/permissions/workflow changes.

Re-checked the full PR diff (origin/main...HEAD, +217/-11 across the 5 declared files) against the same criteria as the earlier full pass — quoting/--arg binding for --queue-labels/--decision-label/--repo-labels-json remains injection-safe, and no other security-relevant surface was introduced. This confirms the prior "no security issues found" result from the earlier review on this branch (60f4fa0) still applies to the current head.
· branch cursor/fix-610-morning-brief-taxonomy-2ae1

Comment thread plugins/claude-ops/skills/morning-brief/morning-brief.test.sh
Do not trip ANY_LIVE when --repo-labels-json can satisfy the parked-decisions
label probe without a live issue list.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 2m 30s —— View job


Code review (/review:code-review)

  • Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
  • Check skip gate (already reviewed this head?)
  • Review latest commit (54c78f5) against prior findings
  • Check for new issues
  • Post findings

Scope: correctness/tests/docs (security is owned by the separate /review:security-review lane per REVIEW.md, which already ran clean on this PR).

Prior findings from earlier rounds are resolved by 54c78f5: OUT_NO_DEC and OUT_EMPTY_LABELS_DEC no longer require the gh binary — the new NEEDS_LIVE_DECISIONS short-circuit (morning-brief.sh:217-221) correctly keeps ANY_LIVE=0 for those two cases, since both use --repo-labels-json fixtures where the decision label is genuinely absent.

One new finding, posted inline on morning-brief.sh:217-221: the fix's short-circuit is broader than its stated intent — it forces NEEDS_LIVE_DECISIONS=0 whenever --repo-labels-json is supplied at all, without checking whether the resolved decision label actually appears in that file. print_decisions only skips the live gh issue list call when the label is confirmed absent; if it's present, execution still falls through to a live call. So a caller who pins --repo-labels-json (documented as a general-purpose flag, not test-only — see line 31) while leaving --decisions-json unset and expecting live decisions data would silently lose the "gh required for live queries" fail-fast check on a machine without gh installed, instead getting a masked "(unable to read decision queue)". This PR's own regression tests don't exercise this because both fixtures used with the short-circuit happen to omit the decision label — a test with a label file that includes the decision label (and no --decisions-json, no gh on PATH) would catch it.

No other issues found — the rest of the diff since the last full review pass (32f443e) is limited to this one gate change; plugin.json/CHANGELOG.md/SKILL.md are unchanged and were already reviewed clean.

Note: this lane's allowed-tools don't include running the test suite directly, so this is a static trace of the control-flow change (ANY_LIVE/NEEDS_LIVE_DECISIONS gating, label_exists_in_repo branching), not an observed test run.

Branch: cursor/fix-610-morning-brief-taxonomy-2ae1

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 1m 6s —— View job


Security review (/review:security-review)

  • Gather context (PR diff, changed files, custom instructions)
  • Run /review:security-review per repo custom instructions
  • Post findings

Skip gate hit — no new review needed.

Per the lane's "last security-reviewed head already applies" skip condition: the prior security-review pass on this branch (posted against head 32f443e6a43a849211650d509ef10f5267d8c6f) already covered the full PR diff and found no security issues. The only change since that head, up to the current HEAD 54c78f56b85c97a5354a47c9c02e1312b0ccb923, is commit 54c78f5, which touches a single file:

plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh#L207-L226

That change splits the single ANY_LIVE flag into per-source NEEDS_LIVE_* flags and adds one short-circuit: when --decisions-json is unset but --repo-labels-json is supplied, NEEDS_LIVE_DECISIONS is forced to 0 so the offline decision-label-absent path doesn't require the gh binary. Verified this is safe:

  • No new external input path — REPO_LABELS_JSON/DECISIONS_JSON are the same pre-existing, already-reviewed CLI flags (quoted, no eval/interpolation).
  • No trust-boundary change — the short-circuit only skips a live gh call, it doesn't skip validation: print_decisions already returns from the label-not-found branch (line 450) before DECISIONS_JSON is ever read, so the gate change doesn't alter what gets executed — only whether gh is demanded up front.
  • No permissions, secrets, or GitHub Actions workflow changes.

This is a pure control-flow/test-determinism fix (addressing the residual "still requires gh" correctness gap from the prior code-review round), not a security-relevant change. No security issues found.

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh
…n label

Probe REPO_LABELS_JSON for the resolved decision label before skipping the
gh-required gate, so callers who include that label still fail fast without gh.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 2m 20s —— View job


Code review (/review:code-review)

  • Gather context (PR diff, changed files, CLAUDE.md/REVIEW.md)
  • Check skip gate (already reviewed this head?)
  • Review latest commit (a329ea6) against prior findings
  • Check for new issues
  • Post findings

Scope: correctness/tests/docs (security is owned by the separate /review:security-review lane per REVIEW.md, which already ran clean on every prior head of this PR).

The prior finding is resolved by a329ea6. The last round flagged that NEEDS_LIVE_DECISIONS was relaxed to 0 whenever --repo-labels-json was merely supplied, without checking whether the resolved decision label actually appeared in it — so a caller pinning --repo-labels-json with a label set that includes the decision label, and no --decisions-json, would silently lose the "gh required for live queries" fail-fast. This commit fixes that correctly: it now resolves _probe_decision_label (mirroring resolve_decision_label()'s DECISION_LABEL_ARG-else-DEFAULT_DECISION_LABEL precedence), fetches the fixture label set via fetch_repo_label_names (file-read path only, since the guard already requires REPO_LABELS_JSON non-empty — no live gh call is introduced here), and only relaxes NEEDS_LIVE_DECISIONS when label_exists_in_repo confirms the label is genuinely absent (morning-brief.sh:217-224).

One residual gap, posted inline on morning-brief.sh:221: the fix's own suggested regression test — a --repo-labels-json fixture that includes the decision label with no --decisions-json, asserting the gh-required fail-fast — wasn't added. All three label fixtures in this PR (repo-labels-alternate.json, repo-labels-partial.json, repo-labels-empty.json) omit status: needs-decision, so the new branch's "label present → still requires gh" side is untested. Not blocking — the production code path checks out correct by inspection — but worth closing before this lands, since it's the exact case the last two review rounds centered on.

No other issues found. morning-brief.test.sh, SKILL.md, CHANGELOG.md, and plugin.json are unchanged since the last-reviewed head (54c78f5) and were already reviewed clean in earlier rounds.

Note: this lane's allowed-tools don't include running the test suite directly, so the above is a static trace of the control-flow change (NEEDS_LIVE_DECISIONS gating, label_exists_in_repo branching, set -u var initialization), not an observed test run — the "81 cases" claim remains author-reported.

Branch: cursor/fix-610-morning-brief-taxonomy-2ae1

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Claude finished @cursor[bot]'s task in 2m 48s —— View job


Security review (/review:security-review)

  • Gather context (PR diff, changed files, custom instructions)
  • Run /review:security-review per repo custom instructions
  • Post findings

No security issues found.

Per the lane's skip gate, this pass reviews what changed since the last-reviewed head (54c78f56b85c97a5354a47c9c02e1312b0ccb923, confirmed clean by the prior security-review pass on this PR) through the current HEAD a329ea6a1d0b970a1c998da38121b8cdb945dca6. That range is a single commit touching one file:

plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh#L217-L224

This commit tightens the NEEDS_LIVE_DECISIONS relaxation added in the previous commit: instead of unconditionally skipping the live gh requirement whenever --repo-labels-json is supplied, it now attempts to probe the fixture-provided label list for the resolved decision label before relaxing the gate. Reviewed for security impact:

  • No new external input path — REPO_LABELS_JSON/DECISION_LABEL_ARG are the same pre-existing, already-reviewed CLI flags; no new gh/jq invocation shape, no eval, no unquoted expansion.
  • No trust-boundary change — this only affects whether the script demands the gh binary up front before a still-degraded/gh-free decision path executes; it doesn't change what data reaches a command or how it's quoted.
  • No secrets, permissions, or GitHub Actions workflow changes.

Note (out of scope for this lane, flagged for awareness): the probe this commit adds calls fetch_repo_label_names and label_exists_in_repo at morning-brief.sh:221-222, but those functions aren't defined until lines 341 and 358 — later in the script's linear execution. Bash has no function hoisting, so this call site will hit "command not found" at runtime rather than actually probing the label file, which looks like it silently defeats the fix's intent (the gh-required gate would stay stricter than needed, not looser — a correctness/availability issue, not a vulnerability). This lane's allowed-tools don't include running the test suite to confirm live, and per REVIEW.md this lane reports security findings only, so it's not filed as a finding here — worth a /review:code-review pass to confirm and fix before merge.

No other issues found; the rest of the diff (test file, docs, changelog, version bump) is unchanged since the prior clean security-review pass.
· branch cursor/fix-610-morning-brief-taxonomy-2ae1

Comment thread plugins/claude-ops/skills/morning-brief/scripts/morning-brief.sh
@kyle-sexton
kyle-sexton merged commit 6349d1a into main Aug 13, 2026
38 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/fix-610-morning-brief-taxonomy-2ae1 branch August 13, 2026 02:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: needs-triage Default until a priority tier is assigned.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

claude-ops:morning-brief: Queues section label set hardcoded to melodic-software taxonomy — parameterize or degrade like the telemetry-issue path

2 participants