Skip to content

feat(session-flow): add show-options, a nothing-hidden skill menu - #3011

Merged
kyle-sexton merged 16 commits into
mainfrom
claude/skill-recommendation-system-opdhxd
Aug 19, 2026
Merged

feat(session-flow): add show-options, a nothing-hidden skill menu#3011
kyle-sexton merged 16 commits into
mainfrom
claude/skill-recommendation-system-opdhxd

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

No linked issue

Summary

Adds /session-flow:show-options — a human-facing menu answering "what should I run next?" over a
~200-skill catalog the operator cannot hold in their head. It resolves the installed catalog, sorts
it into five buckets, and presents ranked options for the human to choose from. Its defining
constraint is that it never withholds an option because it judges the step already done or
unnecessary.

Fix

The skill. Five buckets — Now, Next, Skipped upstream (artifact-grounded), Later (the in-domain
remainder beyond the near horizon), and a rotating Spotlight of three — rendered in two tiers: at
most five ranked options per bucket in full treatment (invocation name, what it adds to this
conversation
, when you would skip it), then the entire remainder as bare invocation names with an
explicit count. Later renders tier-2 only, which is what lets a catch-all exist without becoming a
dumping ground. Ships manual-only for V1, so it costs no listing-budget description and cannot
collide with workflow's auto-fire triggers.

The contract is two rules, because one was insufficient. Never omit a candidate's name; never
invent one. Rule 1 alone creates pressure to fill buckets from a thin source, and a menu that
confidently routes to a nonexistent skill is worse than a short menu. Model judgment reaches rank and
annotations — a skill believed to have run is ranked normally and annotated — never presence. Rule 1
cites this plugin's existing "every section is always present" doctrine rather than inventing a
contract.

Candidates do not come from the in-context skill listing. That listing omits every
disable-model-invocation: true skill (56 of 207 here) and drops descriptions starting with the
least-invoked ones — i.e. the forgotten skills this exists to surface. Measured with this repo's own
check-listing-budget.sh: 101,563 chars against an 8,000-char budget, 12.7× over. Sourcing from it
would have reinstated, invisibly, the exact gatekeeping the contract bans. The ladder is
/claude-ops:inventory (gated on install; it owns fleet enumeration, so no second cache-walker),
else a project-declared catalog, else the listing with its truncation disclosed per
docs/conventions/liveness-assertion/. Inventory's output is read from stdout, never via --out
into the consumer's repo, and installed_plugins is reconciled against enabled_plugins so a
disabled plugin's skills are annotated rather than offered as runnable.

workflow cedes option surfacing. Its "route to exactly ONE owner… never present both" mandate
now says it governs stage routing. Without that carve the plugin would ship two contradictory
routing doctrines.

reference/gather.md. Seven skills each inlined a near-identical durable-state probe block;
point-dont-copy pins the threshold at two. Extracted to one owner doc, with per-consumer
differences preserved and documented as deliberate — orient reads git log -8 where save-point
skills read -5, retro alone takes git diff --name-only HEAD, find-handoff takes no git state
beyond the branch, workflow takes no session id. check-shell-portability.sh was widened to scan
plugins/*/reference/*.md so the extraction does not move executable snippets out of gate scope.

Two fixes outside this feature. audit-fleet.test.sh's unreadable-root case reported a false
FAIL as uid 0 — root bypasses the permission bits it asserts on, so the run legitimately succeeded
and the test blamed a defect that does not exist; it now skips as root with the reason named, and CI
is unaffected because GitHub-hosted runners are non-root. show-options' ladder gained per-rung
done-conditions, clearing skill-quality check 23.

Reviewer note: incidental formatter churn in audit-fleet.test.sh

The diff for that file contains hunks I did not author — <<PY heredocs gaining ; then on the
if line, and one blank line removed. A shfmt formatter hook normalised the file after my edit.
It is a formatting change, not a behavioural one (both heredoc forms are equivalent bash), and it is
a net improvement: shfmt -d exits 1 on origin/main's copy and 0 on this branch's, so the
file was already non-conformant and is now clean. Flagging it because the automated security review
described it as fixing a "heredoc placement bug", which overstates it — nothing was broken, the file
was merely unformatted. Happy to revert those hunks to keep the diff minimal if you would rather the
formatting fix landed separately.

Verification

CI is green on feb54cc: ci-status (the aggregate over all required jobs) success, including
all four Windows lanes, skill-quality-gate, contract-slice-prune-gate, changelog-parity-gate,
shell-portability-lint, portability-lint, silent-skip-gate, zizmor, and GitGuardian.

Also run locally against origin/main:

  • check-changed-skills.sh origin/main — 10 skills checked, 0 failed
  • check-contract-slice-prune.sh --check-diff origin/main — passed (slice pruned)
  • check-skill-leaf-names.sh --check, check-skill-portability.sh, check-shell-portability.sh — clean
  • check-changelog-parity.sh --check-bump / --check-preserved / --check-order — all clean
  • generate-catalog.mjs --check, generate-cheatsheet.mjs --check — in sync
  • audit-fleet.test.sh — exits 0
  • check-evals-quality.sh — PASS, 0 warnings; check-jsonschema validates the 9-case eval set
  • markdownlint-cli2 over all new and changed files — 0 issues

Empirical checks behind the design claims: /claude-ops:inventory's bundled script enumerates
education:teach (manual-only, absent from every in-context listing), proving the ladder's top rung
closes the completeness gap; the listing-budget aggregate stayed at 101,563 chars after adding the
skill, confirming manual-only costs no description budget; and the shell-portability diff scan grew
from 6 files to 13, confirming the widened scope covers the extracted reference.

Review history

Three independent reviewers raised nine findings; all nine are fixed and their threads resolved. The
substantive one was a contract hole: with only four buckets, an in-domain but downstream skill fit
nowhere and would have had to be dropped, breaking the never-omit rule from the inside — Later
closes it. Security review passed on every head.

Related

  • ADR docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md — the graduated
    decision record, carrying the candidate-source rationale, the success criterion and its recheck
    trigger, the CATALOG-TAXONOMY placement adjudication, and the use-your-skills revisit-trigger
    adjudication.
  • Pre-prune contract slice: 9f8721a7 holds the Brief, the seven-phase plan, and the Tier-B design
    resolution, pruned per docs/conventions/topic-docs/.
  • melodic-software/claude-code-plugins#1687 — the worktree-isolation $-expansion behavior the
    gather seam preserves.

claude added 11 commits August 18, 2026 04:19
…kill

Records the interview contract for a new /session-flow:what-next skill: a
human-facing option menu that reads session trajectory and buckets candidate
skills as Backfill / Now / Next / Standing under a no-gatekeeping presentation
contract, drawing candidates from the live in-context listing rather than an
embedded inventory.

Usage-metrics-informed surfacing and a sibling phase-boundary decision tree are
deferred with named arbiters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
Three fresh-context validators, an exploration pass, external research, and a
fresh-eyes verifier challenged five of eight auditable decisions independently.
Two were factually wrong and are replaced.

The candidate source was the in-context skill listing. That listing omits 56 of
207 skills outright and measures 101,563 chars against an 8,000-char budget,
dropping descriptions from the least-invoked skills first — reinstating the
gatekeeping the contract bans, biased toward the forgotten skills the skill
exists to surface. Replaced with a completeness ladder that routes to
claude-ops:inventory rather than walking the plugin cache, which
skill-quality:check refuses on documented grounds.

The output contract measured 139 options across 275 lines and ~4,100 tokens per
invocation — the generated cheat sheet with an extra column, and a shape the
repo's own adhd:shape rule 9 forbids. Replaced with a two-tier shortlist plus a
counted complete remainder, and the no-gatekeeping rule split in two so ranking
is permitted and only suppression is banned.

Also: V1 ships manual-only to cost no listing budget and avoid a verbatim
trigger collision with session-flow:workflow; the name moves to an imperative
verb phrase per the naming grammar; a fourth boundary neighbor is named; and the
contract-slice prune-before-merge obligation is recorded after the gate was
confirmed failing on this branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
…exit

Populates the Plan section with phases, per-phase mechanically verifiable
sanity checks, a per-phase execution-surface routing table, and the
scope-fencing tables for the one parallel opportunity.

Phase 1 probes the candidate ladder before any authoring, since whether
claude-ops:inventory can enumerate the manual-only skills decides the
contract Phase 3 writes. Phase 2 extracts the shared gather seam ahead of the
new skill because point-dont-copy pins the duplication threshold at two and
orient plus workflow are already two. Phase 7 carries the contract-slice
prune-with-pointer the audit surfaced.

The design gate is evaluated, not skipped: Tier B with a documented early-exit
recording all nine design threads and where each was resolved.

Plan is a draft pending fresh-context plan-reviewer findings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
… phase

A fresh-context plan review returned 38 findings (3 critical, 10 high). All
three criticals were re-verified against the tree before being applied.

Three phases gated on check-skill.sh invoked with a file path; the script takes
a skill name resolved under a skills root, so the gate would never have run.
Phase 7 invoked check-changed-skills.sh with no base ref, which exits on a
usage error. Both fixed to their CI-equivalent forms.

The third correction is a design change. Phase 2 extracted a shared gather seam
on the premise that orient and workflow were its only two consumers; seven
session-flow skills carry that block. Extracting for two would have left five
divergent copies beside a new seam — two sources of truth for one probe. The
extraction is withdrawn: show-options now routes durable state to orient, and
the seven-way cleanup is promoted to its own topic so it does not ride this PR.

Also: the HTML rendering tier is cut from V1 as absent from the locked contract;
the Spotlight ledger location is decided rather than deferred, since plugin-data
keying leaks one file across every repository on a machine; three previously
unmapped Brief obligations gain phases and eval cases; two owner conventions the
plan had missed are loaded and cited; and every count target in the plugin
surface phase is stated explicitly, because the existing "eleven" is already off
by one.

Blast radius revised down to LOW-to-MEDIUM now that no shipped skill's gather
block is touched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
Answers "what should I run next?" by resolving the installed catalog and
rendering it as a ranked menu the operator decides from, rather than routing to
one owner. Buckets are Now, Next, Skipped upstream, and a rotating Spotlight.

The no-gatekeeping contract is two rules, because one is insufficient: never
omit a candidate's name, and never invent one. Rule 1 alone creates pressure to
fill buckets from a thin source; rule 2 closes it. A skill believed to have run
already is ranked normally and annotated, so model judgment reaches rank and
annotations but never presence. Rule 1 cites this plugin's existing
"every section is always present" doctrine rather than inventing a contract.

Candidates resolve through two ladders, not one. A probe of
claude-ops:inventory established that it supplies complete names including
manual-only skills — education:teach, absent from every in-context listing, is
enumerated — but no descriptions and no stage metadata. Enrichment is therefore
a separate ladder, and absent enrichment lands a skill in tier 2 by name rather
than omitting it, so a thin catalog cannot break rule 1. A pool sourced from the
truncated listing discloses that in the output, per liveness-assertion.

Durable state is the primary signal and the conversation secondary, because the
long compacted session is when the skill is most needed and the conversation is
least reliable. It builds no probe of its own: seven session-flow skills already
inline near-identical blocks, so it routes to orient instead.

Two-tier rendering caps presentation, never the candidate set: five ranked per
bucket in full, the entire remainder by name with an explicit count that makes
the omission-free claim checkable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
…mp to 0.24.0

Evals cover the seven behaviors the contract actually rests on, refusal first:
that a thin catalog yields a short menu rather than an invented one; that an
already-run stage is annotated instead of dropped; the two-tier shape and its
line budget; disclosure of a degraded pool; buckets without stage metadata;
refusing to infer skipped stages when the memory root is unreadable; and durable
state outranking a thin post-compaction conversation.

workflow's precedence section now says its route-to-exactly-one rule governs
stage routing and cedes option surfacing. Without that carve the two skills read
as contradictory doctrine in one plugin — one required never to present both
candidates, the other existing to present the whole set.

Counts: the total moves to fourteen in plugin.json and the README; the "other N
skills are zero-config" sites move to thirteen, correcting a count that was
already off by one before this change. Both generated docs regenerated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
… the listing

Graduates the durable decision record for show-options out of the branch-lived
contract slice, ahead of the prune. Earns an ADR on all three admission tests:
hard to reverse (the candidate-source and output contracts), surprising without
context ("why not just read the skill listing?"), and a real trade-off with three
rejected alternatives.

Carries the two things that would otherwise die with the slice: the success
criterion with its recheck trigger, and the written adjudications for the
taxonomy placement challenge and the deferred routing hook.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
…ad of failing

The unreadable-discovery-root assertion made a directory with chmod a-rx and
expected discovery to refuse it. Root bypasses those permission bits, so the
directory stays traversable, the run legitimately succeeds, and the case reported
a product defect that does not exist. Root containers are a common local and
cloud-session shape, so this surfaced as a standing false FAIL.

The case now skips as uid 0 with its reason named. CI is unaffected: GitHub-hosted
Ubuntu runners execute as a non-root user, so the assertion still discriminates
where it matters, and only root containers see the skip. A check that silently
reports the wrong verdict is what the liveness-assertion convention forbids —
naming the skip is the conforming shape.

Also merges origin/main (45 commits) and renumbers this branch's ADR from 0014 to
0016: main landed 0014 and 0015 while this work was in progress. The merge clears
five reported trigger-keyword regressions in improve, decompose, track, work, and
draft-goal-condition, none of which this branch touched — they were artifacts of
diffing a stale branch against an advanced main, and main had already added the
keywords and the missing evals file.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
… owner doc

Seven skills each inlined a near-identical probe list, the one-command-per-call
and treat-failure-as-unknown rules, and the #1687 no-precompute rationale.
point-dont-copy pins the duplication threshold at two, so this was sevenfold
duplication with no single source.

reference/gather.md now owns the probe set. Each consumer names the subset it
takes and cites the seam. The per-consumer differences are preserved and
documented as deliberate rather than normalised away — orient reads git log -8
where the save-point skills read -5, retro alone takes git diff --name-only HEAD,
find-handoff takes no git state beyond the branch, and workflow takes no session
id. A naive merge would have silently changed orient's commit depth.

Two local caveats stay at their call sites because they are not general:
continue-in-background's warning that this block is never the dirty-tree gate
(generalised in the seam as well, since the reasoning applies to any consumer),
and find-handoff's note that bare $HOME is the one expansion form observed to
survive the worktree-isolation guard.

check-shell-portability.sh now scans plugins/*/reference/*.md. Its own rationale
for covering skill markdown (#2704 — agents execute shell snippets from those
files) applies identically to a shared engine doc a skill body cites, and without
the widening this extraction would have moved executable snippets out of the
gate's scope. Verified: the diff-mode scan grew from 6 files to 13, and the
gate's fixture for "non-skill .md stays out" sits directly under plugins/alpha/,
so that assertion is unaffected.

Also gives show-options' candidate ladder per-rung done-conditions, clearing the
advisory completion-criteria warning from skill-quality check 23.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
…d to ADR 0016

docs/topics/<slug>/ is Contract tier per docs/conventions/topic-docs: committed
on the task branch only, pruned before merge. Pre-prune state is 9f8721a —
the Brief, the seven-phase plan, and the Tier-B design resolution are recoverable
from there and from the PR body.

Durable outcomes graduated first, so nothing load-bearing dies with the slice:
the candidate-source and output-contract decisions, the success criterion with
its recheck trigger, the CATALOG-TAXONOMY placement adjudication, and the
use-your-skills revisit-trigger adjudication all live in
docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Load /review:security-review instructions and gather PR context
  • Run skip-gate check
  • Review diff for security-relevant surface
  • Report findings

No security issues found.

This PR is documentation-and-test-fixture in nature — no security-relevant surface:

  • plugins/session-flow/skills/show-options/** (new skill) and plugins/session-flow/reference/gather.md (extracted probe doc) are prompt/instruction markdown only. The gather probes are fixed, non-parameterized read-only git commands (git branch --show-current, git status --porcelain, git log --oneline -N, git diff --name-only HEAD) with no user input concatenated into them — no injection surface.
  • plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh — the change adds an id -u check to skip an assertion when running as root, and fixes a pre-existing <<PY ... then heredoc placement bug. Test-only, no exploitable path.
  • scripts/check-shell-portability.sh — widens the glob the portability gate scans (plugins/*/reference/*.md) to keep the newly-extracted gather.md in scope. No permission or trust-boundary change.
  • plugins/*/.claude-plugin/plugin.json, CHANGELOG.md, README.md, docs/CATALOG.md, docs/SKILL-CHEAT-SHEET.md, the ADR — metadata/docs only.
  • No allowed-tools frontmatter changes grant new capabilities; show-options/SKILL.md declares no allowed-tools override and is disable-model-invocation: true (manual-only), consistent with the PR's stated design.
  • No auth, secrets, credentials, GitHub Actions workflow files, or dependency/pin changes are touched.

@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

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

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md, skill instructions)
  • Run /review:code-review lane over changed files
  • Post inline findings
  • Post summary

This repo carries a dedicated claude-security-review.yml workflow, so this pass stayed in code-review scope (correctness, design, docs/consistency) and left security to that lane.

Findings

🔴 Important — ADR contradicts the extraction this same PR performs.
docs/adr/0016-...md#L102-L106
The "Known follow-up" section states the seven-way durable-state probe extraction "remains outstanding and is tracked separately." But this PR performs exactly that extraction: plugins/session-flow/reference/gather.md is added and all seven named consumers (continue-in-background, find-handoff, handoff, orient, retro, running-retro, workflow) are updated to cite it — confirmed by the diff and by the session-flow CHANGELOG's own 0.24.0 entry ("the durable-state probe block is extracted to one owner doc"). As merged, the ADR is factually wrong about the state of the codebase it ships beside. Posted inline with a suggested fix (drop the paragraph, or reword it to describe only genuinely-open follow-up, e.g. the two-consumer attempt-and-withdrawal history).

What I checked and found clean

  • The reference/gather.md probe table (session-id/branch/status/recent-commits/changed-files, per-consumer subsets and -N values) matches every one of the seven consumer SKILL.md "Context — gather first" sections exactly, including the deliberate divergences called out in the PR description (orient at -8, retro's changed-files, find-handoff and workflow's omissions).
  • workflow's new "governs STAGE routing, not option surfacing" carve and show-options's reciprocal boundary section are consistent and non-contradictory.
  • Skill-count updates (README, setup description, docs/CATALOG.md, plugin manifest descriptions) are internally consistent: 14 skills installed, "other thirteen" for setup is arithmetically correct (this also fixes a pre-existing off-by-one noted in the CHANGELOG).
  • docs/SKILL-CHEAT-SHEET.md and docs/CATALOG.md additions match the generated-doc conventions and the new skill.
  • audit-fleet.test.sh's heredoc then-placement change (<<PY; then on the opening line vs. a bare then after the heredoc terminator) is valid bash and behavior-preserving; the new uid-0 skip is scoped correctly and doesn't weaken the non-root (CI) assertion.
  • check-shell-portability.sh's widened scan (plugins/*/reference/*.md) correctly closes the gap the extraction would otherwise have opened.
  • show-options/evals/evals.json's 7 cases exercise the two core rules (never omit / never invent) plus the degraded-pool, no-metadata, and durable-state-over-conversation branches.

Not independently re-run: the shell/markdown/JSON lint and test commands the PR description lists as passing locally (check-changed-skills.sh, check-contract-slice-prune.sh, check-evals-quality.sh, generate-catalog.mjs --check, etc.) — noted as author-claimed, not verified by me.

Comment thread docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md Outdated
@github-actions

github-actions Bot commented Aug 19, 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.

…claim

Two CI/review findings, both real.

The hygiene job's shellcheck lane failed on SC2292: the uid-0 guard added in the
previous commit used [ ] where this repo's shellcheck severity requires [[ ]].
Local runs had used -S warning, which does not surface a style-class finding —
the gate runs at default severity. Fixed and re-verified at default severity;
the test still exits 0 and still emits its named SKIP as root.

The automated code review caught a genuine contradiction: ADR 0016's follow-up
paragraph said the seven-way probe extraction "remains outstanding and is tracked
separately", but this same PR performs it. The ADR was written before the
extraction was pulled forward into this change and was never updated, so as
merged it would have described the codebase shipping beside it incorrectly. The
paragraph now records what actually happened — the extraction is done for all
seven consumers, the two-consumer version was attempted and withdrawn because it
would have left five divergent copies beside a new seam, and the per-consumer
divergences are preserved deliberately so a later tidying pass does not normalise
them away.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Load /review:security-review instructions and gather PR context
  • Run skip-gate check
  • Review diff for security-relevant surface
  • Report findings

No security issues found.

Per the skip gate: only one commit landed since the last reviewed head (16bddf6) — 308db46. Reviewed that diff in isolation:

  • docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md — prose-only rewrite of the ADR's "Known follow-up" section to correct a stale claim (the seven-way probe extraction is described as done rather than outstanding). No code, no executable content.
  • plugins/repo-fleet-hygiene/skills/audit/scripts/audit-fleet.test.sh — one-character change, [ "$(id -u)" -eq 0 ][[ "$(id -u)" -eq 0 ]], to satisfy shellcheck SC2292. Same fixed, non-parameterized uid check as before; no behavioral or trust-boundary change.

This matches the prior full review's conclusion (no security-relevant surface anywhere in this PR: prompt/instruction markdown, a read-only git-probe extraction, a test-only root-skip guard, and a portability-scan glob widening). Nothing in this incremental diff changes that assessment.

@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: 16bddf6bae

ℹ️ 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/session-flow/skills/show-options/SKILL.md Outdated
Comment thread plugins/session-flow/skills/show-options/SKILL.md Outdated
Comment thread plugins/session-flow/skills/show-options/SKILL.md Outdated
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md, prior review comments)
  • Run /review:code-review lane over changed files (new head: 308db46)
  • Post inline findings
  • Post summary

This pass reviewed the full PR at the new head. The prior review's one Important finding — ADR
0016's "Known follow-up" contradicting the seven-way probe extraction this PR itself performs — is
now fixed in 308db46: the paragraph was rewritten to describe the extraction as done, including the
withdrawn two-consumer attempt and the deliberate per-consumer divergences. Verified against the
current ADR text and against every consumer SKILL.md's citation of reference/gather.md — they
match, including orient's -8 vs. the save-point skills' -5, retro's unique changed-files
probe, find-handoff's branch-only subset, and workflow's no-session-id. The 308db46 shellcheck
fix ([ ][[ ]] for the uid-0 guard) is also correct and behavior-preserving.

No new findings. Everything else checked out clean:

  • plugins/session-flow/skills/show-options/{SKILL.md,context/buckets.md,context/candidate-ladder.md}
    — internally consistent: the two-rule contract (never omit / never invent), the two-ladder candidate
    resolution, the tier-1/tier-2 rendering, and the bucket definitions all agree with each other and
    with the ADR's narrative.
  • evals/evals.json — well-formed, 7 cases, each exercising a distinct rule from the SKILL.md
    (invent-refusal, annotate-not-omit, tier budget, degraded-pool disclosure, no-metadata heuristic
    bucketing, ungroundable-memory-root non-inference, durable-state-over-conversation).
  • workflow/SKILL.md's new "governs STAGE routing, not option surfacing" carve is reciprocal and
    non-contradictory with show-options's boundary section.
  • docs/CATALOG.md, docs/SKILL-CHEAT-SHEET.md, both plugin.json manifests, and both CHANGELOG.md
    files are mutually consistent (14 skills, "other thirteen" fixes a pre-existing off-by-one, version
    bumps match Added/Fixed sections: session-flow 0.23.9→0.24.0, repo-fleet-hygiene 0.23.4→0.23.5).
  • scripts/check-shell-portability.sh's widened plugins/*/reference/*.md scan correctly closes the
    gap the extraction would otherwise leave (minor: the updated comment block runs two sentences
    together on one long line — cosmetic only, not flagging as a finding).
  • audit-fleet.test.sh's full diff (uid-0 skip + the shfmt-driven heredoc then-placement changes
    the author called out) is behavior-preserving; the description's characterization of the formatter
    churn as incidental rather than a bug fix is accurate.

Not independently re-run: the shell/markdown/JSON lint and test commands the PR description lists as
passing locally — author-claimed, not verified by me in this pass.

An external reviewer (Codex) found one contract hole and three unspecified
behaviors. All four verified against the code before acting; all four were real.

P1 — the bucket set could not honor its own never-omit rule. Now, Next, Skipped
upstream, and a three-entry Spotlight leave nowhere for a skill that is in-domain
but downstream: testing and review skills early in a session fit none of them, so
the only ways to render were to stretch a definition or drop the name. Adds a
fifth bucket, Later, holding the in-domain remainder beyond the near horizon. It
renders tier 2 only — bare names with a count, about one line — which is what
lets a catch-all exist without recreating the 60-row dumping ground an earlier
cut of this design measured and rejected. Out-of-domain skills are still omitted
under the irrelevant test; Later catches relevance, not everything.

P2 — the ladder treated installation as invocability. claude-ops:inventory
reports installed_plugins and enabled_plugins as distinct sets and asks callers
to say which they used. A skill in an installed-but-disabled plugin is now named
with a "(plugin not enabled)" annotation rather than listed as runnable or
silently dropped, consistent with the annotate-never-omit contract.

P2 — the ladder would have littered the consuming repo. inventory's own
documented invocation writes ./claude-inventory.json into the working directory,
which contradicts this skill advertising the Spotlight ledger as its only write.
Now specifies reading stdout and never passing --out into a consumer's project.

P2 — the Spotlight ledger had no fixed path or record shape, so two sessions
could choose different ones and never recover what the other surfaced, defeating
least-recently-surfaced ordering. Path, JSON shape, sort rule, and
missing-or-unparsable handling are now pinned in context/buckets.md. The ledger
is deliberately not per-topic-slug: rotation is a property of the operator's
catalog, and a per-slug ledger would restart it on every new piece of work.

Two eval cases added for the new behaviors, and the bucket count corrected in the
skill body, the spoke, the plugin README, and the CHANGELOG entry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Load /review:security-review instructions and gather PR context
  • Run skip-gate check
  • Review diff for security-relevant surface
  • Report findings

No security issues found.

Per the skip gate: only one commit landed since the last reviewed head (308db46) — 0cd8263. Reviewed that diff in isolation:

  • plugins/session-flow/skills/show-options/SKILL.md, context/buckets.md — adds a fifth bucket (Later), pins the Spotlight ledger's path/record shape, reconciles installed_plugins vs enabled_plugins, and directs the skill to read claude-ops:inventory output from stdout rather than passing --out into the consumer's project. All prompt/instruction markdown — no executable content, no new file I/O behavior introduced by this skill itself (it now explicitly avoids writing into the consumer's repo, which narrows prior surface rather than widening it).
  • plugins/session-flow/skills/show-options/evals/evals.json — two new eval cases (JSON fixtures, no code).
  • plugins/session-flow/CHANGELOG.md, plugins/session-flow/README.md — doc-only updates reflecting the bucket-count change.

This matches every prior review's conclusion for this PR: no security-relevant surface anywhere (prompt/instruction markdown, a read-only git-probe extraction, a test-only root-skip guard, a portability-scan glob widening, and now a contract-hardening pass over the same markdown skill). Nothing in this incremental diff changes that assessment.

@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md, /review:code-review skill instructions)
  • Review the delta since the last code-review pass against the skip gate and criteria
  • Post inline findings
  • Post summary

Per the skip gate, this pass reviewed the delta since the last full code-review pass (head 16bddf6): commits 308db46 and 0cd8263 (current HEAD).

What changed since the last review

  • 308db46 fixes the two findings from the prior pass: the ADR's "Known follow-up" no longer claims the seven-way probe extraction is outstanding (it now correctly describes the extraction as done and records the withdrawn two-consumer attempt), and audit-fleet.test.sh's uid-0 guard is now [[ ]] to satisfy shellcheck SC2292. Both confirmed correct by reading the diffs directly.
  • 0cd8263 addresses all four chatgpt-codex-connector review comments by adding a fifth bucket, Later (in-domain, beyond-Next remainder, tier-2-only rendering), reconciling installed_plugins against enabled_plugins in the candidate ladder, routing claude-ops:inventory output through stdout instead of --out to avoid littering the consumer's repo, and pinning the Spotlight ledger's path/schema/ordering rules in context/buckets.md. I checked each fix against the finding it targets — all four are real fixes, not just addressed in prose. Two new eval cases (8, 9) cover the Later-routing and installed-vs-enabled behaviors.

Findings (posted inline)

Both are doc-consistency nits left over from the Later-bucket addition — neither blocks, both are cheap to fix:

  1. plugins/session-flow/skills/show-options/SKILL.md#L2 — the frontmatter description still lists four buckets, omitting Later, contradicting ## The five buckets later in the same file.
  2. plugins/session-flow/skills/show-options/context/buckets.md#L6 — the ## Why four, and why not the obvious four / ## The four headers weren't updated when Later was added, contradicting the file's own opening line ("five buckets") three lines above.

What I checked and found clean

  • README.md, CHANGELOG.md, and the eval file's new cases all correctly say "five buckets" / describe Later consistently.
  • docs/CATALOG.md and docs/SKILL-CHEAT-SHEET.md don't need regeneration from this change — they source from metadata.summary, not the stale frontmatter description, and that field is unaffected.
  • The --out/stdout fix and installed-vs-enabled reconciliation in SKILL.md are consistent with plugins/claude-ops/skills/inventory/SKILL.md's documented contract (verified by reading it).
  • Eval case ids land out of numeric order in the array (8, 9, then 7) — cosmetic only; check-evals-quality.sh only rejects duplicate ids, not ordering, so not reported as a finding.

Not independently re-run: the shell/markdown/JSON lint and test commands the PR description lists as passing locally — author-claimed, not verified by me in this pass.

The sentence explaining plugins/*/reference/ coverage ran 122 characters where
the surrounding comment block wraps near 80, leaving two sentences joined on one
long line. Cosmetic only — no gate enforces a line length on .sh here, and a
reviewer noted it without raising it as a finding. Rewrapped so the block reads
uniformly; shellcheck, shfmt, and the gate's own diff scan are unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, CLAUDE.md, prior review comments)
  • Run skip-gate check
  • Review diff for security-relevant surface
  • Report findings

No security issues found.

Per the skip gate: only one commit landed since the last reviewed head (0cd8263) — 93fdde1. Reviewed that diff in isolation:

  • scripts/check-shell-portability.sh#L164-L172 — rewraps a comment block to fit the surrounding line width (the over-long line a prior review pass had flagged as cosmetic-only). Pure whitespace/comment reflow, no code or logic change, no executable content.

This matches every prior review's conclusion for this PR: no security-relevant surface anywhere across the full diff (git diff origin/main...HEAD, 23 files) — prompt/instruction markdown for the new show-options skill and its reference/gather.md extraction, a test-only root-skip guard plus formatter-driven heredoc changes in audit-fleet.test.sh, a portability-scan glob widening, and now a comment-only rewrap of that same widening's documentation. Nothing in this incremental diff changes that assessment.

Comment thread plugins/session-flow/skills/show-options/SKILL.md Outdated
Comment thread plugins/session-flow/skills/show-options/context/buckets.md Outdated
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md, prior review comments)
  • Run /review:code-review lane over changed files (new head: 93fdde1)
  • Post inline findings
  • Post summary

Per the skip gate, only two commits landed since the last code-review pass (308db46): 0cd8263
("close four review findings in show-options" — addressing the four Codex P1/P2 comments on this
PR) and 93fdde1 (a pure comment rewrap in check-shell-portability.sh, no behavioral surface).
Reviewed both in full, plus their interaction with the rest of the already-reviewed PR.

Findings

Posted 2 inline — both are the same class of bug: 0cd8263 added a fifth bucket, Later, to close
the "no bucket for in-domain-but-downstream skills" gap Codex flagged, and updated most of the
surrounding prose (README, CHANGELOG, the bucket table, ## The five buckets) — but missed two
other spots that still assert the old count of four:

  • plugins/session-flow/skills/show-options/SKILL.md:2 — the frontmatter description still
    lists only "Now, Next, Skipped upstream (artifact-grounded), and a rotating Spotlight of three,"
    omitting Later entirely.
  • plugins/session-flow/skills/show-options/context/buckets.md:30 — the ## The four header
    (and its line-28 lead-in, "The current four keep the two that earned their place…") now sits over
    five subsections (Now, Next, Skipped upstream, Later, Spotlight), contradicting the file's own
    opening line 3 ("sorts the resolved catalog into five buckets").

Both are genuine, freshly-introduced inconsistencies inside the skill's own docs — not caught by
markdownlint or the repo's structural gates, since none of them check description-vs-body semantic
agreement.

What I checked and found clean

  • The substance of all four Codex fixes in 0cd8263:
    • Later bucket (P1, missing horizon bucket) — well-specified: tier-2-only rendering, bounded by
      the existing irrelevant/unnecessary test, with a stated self-correcting signal if it grows too
      large.
    • Enabled-vs-installed reconciliation (P2) — SKILL.md's new "Reconcile installed against enabled"
      text correctly names the inventory script's actual output keys (installed_plugins /
      enabled_plugins, verified against
      plugins/claude-ops/skills/inventory/scripts/inventory.py),
      and chooses annotate-not-omit, which is the correct resolution given this skill's own never-omit
      rule 1. Minor design note (not filed as a finding — doesn't rise to "a careful reviewer would
      block"): this reconciliation detail lives only in SKILL.md's terse ladder table, not in
      context/candidate-ladder.md,
      the doc SKILL.md itself points to for ladder "detail... and failure behavior" — rung 1's
      "Yields" column there still just says "manual-only included," with no mention of the
      enabled/disabled split.
    • Ledger path/schema (P2) — now fully pinned in buckets.md (path, JSON shape, ordering rule,
      missing/unparsable-ledger behavior). Resolves the ambiguity cleanly.
    • Keep inventory output out of the consumer's repo (P2) — SKILL.md now explicitly directs reading
      stdout and never passing --out. Correct fix for the documented ./claude-inventory.json write.
  • The two new eval cases (ids 8, 9 in evals/evals.json) exercise exactly these two fixes and are
    well-formed against the schema; not filing the id-8/9-before-id-7 ordering as a finding since
    nothing enforces or depends on numeric order (check-evals-quality.sh only checks for duplicate
    ids) — a style nit, not a defect.
  • 93fdde1's change to check-shell-portability.sh is a pure comment rewrap (confirmed via
    git diff 0cd8263..93fdde1) — matches the PR description's own characterization, no behavioral
    change.
  • Everything the prior full review at 308db46 already covered (the ADR follow-up fix, the
    gather.md extraction, the workflow/show-options boundary carve, the shellcheck fix) is
    untouched by these two commits and still holds.

Not independently re-run: the shell/markdown/JSON lint and test commands the PR description lists as
passing locally — author-claimed, not verified by me in this pass.

…bucket

Adding Later left three sites still saying four, and the code review caught two
of them. The frontmatter description — the skill's canonical one-line contract —
still listed four buckets, contradicting the body's own "The five buckets" a
hundred lines below. The spoke's section headers said "Why four" and "The five"
three lines apart from its own intro.

Two historical references to four are deliberately kept: the earlier
four-bucket cut that the measurement section describes and rejects, and the four
neighbouring skills in the boundary section, which really are four.

Also renumbers the eval ids so they ascend with array position; inserting the two
new cases before the last one had left the order 1-6, 8, 9, 7. Cosmetic — the
quality gate only rejects duplicates — but a reader scanning the file should not
have to notice that.

This is the third pass of the same stale-count defect in this PR, which is the
argument for deriving these counts rather than writing them by hand.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Load /review:security-review instructions and gather PR context
  • Run skip-gate check
  • Review diff for security-relevant surface
  • Report findings

No security issues found.

Per the skip gate: only one commit landed since the last reviewed head (93fdde1) — dad3fe0. Reviewed that diff in isolation:

  • plugins/session-flow/skills/show-options/SKILL.md — frontmatter description text edit, adding the Later bucket name to the prose list. No code.
  • plugins/session-flow/skills/show-options/context/buckets.md — two section headers changed from "four" to "five" to match the file's own intro line. No code.
  • plugins/session-flow/skills/show-options/evals/evals.json — eval case id fields renumbered (7/8/9) so they ascend with array position; same content, no new/removed cases.

All three are prose/JSON-metadata edits with no executable content, no new I/O, and no trust-boundary change. This matches every prior review's conclusion for this PR: no security-relevant surface anywhere across the full diff (git diff origin/main...HEAD, 23 files) — prompt/instruction markdown for the new show-options skill and its reference/gather.md extraction, a test-only root-skip guard plus formatter-driven heredoc changes in audit-fleet.test.sh, a portability-scan glob widening and its comment rewrap, and now a bucket-count reconciliation pass over the same markdown/JSON files. Nothing in this incremental diff changes that assessment.

Comment thread plugins/session-flow/skills/show-options/SKILL.md
Comment thread plugins/session-flow/skills/show-options/context/buckets.md
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Code review complete

  • Gather context (PR diff, CLAUDE.md, /review:code-review skill instructions)
  • Review the delta since the last code-review pass against the skip gate and criteria
  • Post inline findings (none — see below)
  • Post summary

Per the skip gate, this pass reviewed the delta since the last full code-review pass (which covered through 0cd8263 and posted two Minor findings): commits 93fdde1 and dad3fe0 (current HEAD).

What changed since the last review

  • 93fdde1 rewraps the over-long comment line in scripts/check-shell-portability.sh that a prior pass had noted as cosmetic-only. Pure whitespace/comment reflow — verified the block now wraps consistently near 80 chars (scripts/check-shell-portability.sh#L160-L172), no logic change.
  • dad3fe0 fixes both Minor findings from the prior code-review pass:
    • SKILL.md's frontmatter description now lists Later alongside the other four buckets, matching ## The five buckets in the body.
    • context/buckets.md's section headers are now ## Why five, and why not the obvious four / ## The five, matching the file's own "sorts... into five buckets" opening line three lines above.
    • It also renumbers the eval case ids (previously 1–6, 8, 9, 7) so they ascend with array position — I confirmed the full evals/evals.json now reads 1 through 9 in order, no duplicates or gaps.

I grepped the whole plugin (SKILL.md, buckets.md, README.md, CHANGELOG.md, docs/CATALOG.md, docs/SKILL-CHEAT-SHEET.md) for remaining stale "four"/bucket-count references — none found. The two intentionally-kept "four" references (the historical four-bucket cut the design doc describes and rejects, and the four neighbouring skills in the Boundaries section) are genuinely four items, not stale counts.

No new findings. Both fixes are correct, complete, and don't introduce any new inconsistency. This matches every prior pass's conclusion for the substantive content of this PR (the show-options skill, the gather.md extraction, the audit-fleet.test.sh root-skip fix, and the portability-scan widening) — all previously reviewed clean, with the ADR contradiction and bucket-count nits from earlier passes now resolved.

Not independently re-run: the shell/markdown/JSON lint and test commands the PR description lists as passing locally — author-claimed, not verified by me in this pass.

…ladder detail

SKILL.md points at context/candidate-ladder.md for the ladder's "detail, formats,
and failure behavior", but the two rung-1 caveats added in 0cd8263 — read stdout
rather than writing ./claude-inventory.json into the consumer's repo, and
reconcile installed_plugins against enabled_plugins — lived only in SKILL.md's
terse table. The spoke's rung-1 row still read "manual-only included" with no
mention of the enabled split, so the designated detail owner was the one place
missing the detail.

Raised by the code reviewer as a design note rather than a filed finding; it is
right, and leaving the two docs disagreeing about the same rung is the drift the
point-dont-copy discipline exists to prevent.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
@claude

claude Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

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


Security review complete

  • Gather context (PR diff, CLAUDE.md, prior review comments)
  • Invoke /review:security-review and follow it exactly
  • Report findings

No security issues found.

Per the skip gate: only one commit landed since the last reviewed head (dad3fe0) — feb54cc. Reviewed that diff in isolation:

  • plugins/session-flow/skills/show-options/context/candidate-ladder.md — moves two rung-1 caveats (read /claude-ops:inventory output from stdout rather than writing ./claude-inventory.json into the consumer's repo, and reconcile installed_plugins against enabled_plugins) out of SKILL.md's terse table and into this spoke, which is the file SKILL.md designates as the ladder's detail owner. Pure prose relocation — no new claims, no code, no executable content, no I/O behavior change (the "stdout, never --out" guidance is unchanged in substance, just relocated).

This matches every prior review's conclusion for this PR: no security-relevant surface anywhere across the full diff (git diff origin/main...HEAD, 23 files) — prompt/instruction markdown for the new show-options skill and its reference/gather.md extraction, a test-only root-skip guard plus formatter-driven heredoc changes in audit-fleet.test.sh, a portability-scan glob widening and its comment rewrap, a bucket-count reconciliation pass, and now this doc-relocation commit. Nothing in this incremental diff changes that assessment.

@kyle-sexton
kyle-sexton merged commit b3e7af7 into main Aug 19, 2026
50 checks passed
@kyle-sexton
kyle-sexton deleted the claude/skill-recommendation-system-opdhxd branch August 19, 2026 02:42
kyle-sexton pushed a commit that referenced this pull request Aug 19, 2026
A plugin's prose counts its own skills by hand — "bundling fourteen
skills", "the other thirteen skills are zero-config" — and nothing
recomputed those numbers when a skill landed. Adding one silently
falsified every sentence that had counted the old set, in files nobody
opens during that change.

The measured record: PR #3011 went stale on six such counts, each caught
by a different human or automated reviewer rather than by anything
mechanical, and session-flow's own CHANGELOG records an "other eleven
skills" line that was already off by one before that PR touched it. A
reviewer catch is not a control.

check-skill-count-claims.sh matches a closed set of composition-claim
grammars across each plugin's README, manifest, and skill bodies, and
compares the claimed number against the skills the plugin actually has.
It found four live drifts on a clean main:

- testing said "Four skills" with five, and its table omitted
  /testing:audit entirely — the count drift marked a wholly undocumented
  skill, now added.
- verification said "Two skills" with three.
- improvement said "One skill" with two, in its first release.
- docs-hygiene said "the other five skills do not use markdownlint-cli2"
  in an eight-skill plugin, and the substance was wrong too:
  extract-ssot does use it. That sentence now names its subjects instead
  of counting them, which is what this plugin's own audit-noise
  `enum-list` shape prescribes.

Scope is stated in the script header rather than left implicit. The
recognized grammars are a closed set, because most sentences pairing a
number with the word "skills" assert nothing about plugin size ("gate one
skill", "nine skills moved into three new plugins"); a gate that fires on
those trains people to route around it. One form was evaluated and
rejected: a definite-article reference reads as a claim about a named
pair as often as about the whole set. CHANGELOG.md is out of scope — a
dated entry describes the release it shipped with, and rewriting it to
match today's tree would falsify history. Document-internal counts ("the
four buckets" in a document defining five) are a real second class this
gate does not hold, and the header says so.

Verification: 16 black-box assertions, the negative ones load-bearing;
six mutations of the gate's logic each turn the suite red. shellcheck at
the repo rcfile, shfmt, actionlint, portability, lane-coverage,
changelog-parity, and all 11 affected suites are green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADsDCToTtjvYut3ZQXJHDM
kyle-sexton added a commit that referenced this pull request Aug 19, 2026
No linked issue

## Summary

A plugin's prose counts its own skills by hand — "bundling fourteen
skills", "the other thirteen skills are zero-config" — and nothing
recomputed those numbers when a skill landed. Adding one silently
falsified every sentence that had counted the old set, in files nobody
opens during that change.

The measured record: PR #3011 went stale on six such counts, each caught
by a different human or automated reviewer rather than by anything
mechanical, and `session-flow`'s own CHANGELOG records an "other eleven
skills" line that was **already off by one before that PR touched it**.
A reviewer catch is not a control.

This adds the gate, and fixes the four live drifts it found on a clean
`main`.

## Fix

**New gate** — `scripts/check-skill-count-claims.sh` + `.test.sh` +
`skill-count-claim-exemptions.txt`, wired into `ci.yml` as
`skill-count-claim-gate` (self-test first, then `--check`) and added to
`ci-status.needs`. It matches a closed set of composition-claim grammars
across each plugin's README, manifest, and skill bodies, and compares
the claimed number — spelled or numeric — against the skills the plugin
actually has.

The spelled form is why nothing found this before: the claims read
"fourteen skills", not "14 skills", so no numeric grep ever hit them.

**Four live drifts found and fixed:**

| Plugin | Claimed | Actual |
|---|---|---|
| `testing` | "Four skills" | **five** — and the README table omitted
`/testing:audit` entirely |
| `verification` | "Two skills" | three |
| `improvement` | "One skill" | two, in its own first release |
| `docs-hygiene` | "the other five skills do not use it" | eight-skill
plugin, **and** `extract-ssot` does use it |

The `testing` case is the payoff: a count drift marked a wholly
undocumented skill, so a reader on that front page had no way to learn
`audit` exists. Both halves are fixed — the count and the missing table
row.

For `docs-hygiene` the sentence was substantively false too, so it now
names its subjects instead of counting them. That is the treatment this
plugin's own `audit-noise` `enum-list` shape prescribes for a hardcoded
consumer count; the two agree on the diagnosis and differ only in
enforcement.

**Scope is stated in the script header, not left implicit:**

- The recognized grammars are a **closed set**, because most sentences
pairing a number with the word "skills" assert nothing about plugin size
("gate one skill", "nine skills moved into three new plugins"). A gate
that fires on those trains people to route around it.
- One grammar was **evaluated and rejected**: a definite-article
reference ("the two skills…") reads as a claim about a named pair as
often as about the whole set — `discipline`, which has 17 skills, says
exactly that.
- **`CHANGELOG.md` is out of scope.** A dated entry describes the
release it shipped with; rewriting it to match today's tree would
falsify history.
- **Document-internal counts** ("the four buckets" in a document
defining five) are a real second class this gate does not hold, and the
header says so rather than letting the coverage read as complete.

**Portability annotations.** `check-shell-portability.sh --all` was
permanently red on 13 hits across three files, which made it unusable as
an audit tool: a mode that always fails teaches you to ignore it, so a
genuine 14th hit would land in the noise. Every one is a false positive
in one of two classes — PCRE tokens as test *data* in
`resolve-convention-pattern.test.sh` (the shorthand is the thing under
test; the assertions prove the resolver rejects it), and Windows path
fixtures (`C:\temp\skills`, `\\server\share\skills`,
`D:\work\acme\keepme`). In each case the construct cannot be spelled
portably because it *is* the input under test, so a per-site
`portability-ok:` with a stated reason is the correct disposition. The
whole-file `portability-scope:` escape was deliberately not used — the
gate reserves it for its own fixture corpus, and these are neighbouring
gates' tests.

## Review findings addressed

**Three Codex P2s, all verified real against the tree before acting**
(`b8df7ec2`):

1. **A plugin whose last skill is removed loses its `skills/`
directory**, and the scan skipped any plugin without one — the
stale-count-by-removal case the gate most needs, reported as success.
Every plugin is now scanned; missing or empty `skills/` is a count of
zero.
2. **Matching worked per physical line**, so a markdown wrap between the
verb and the count hid a claim, and reflowing a paragraph could
permanently disable the gate for a claim it used to hold. Each line is
now tested alone and joined with its successor, stopping at block
boundaries. This found a live wrapped claim in
`plugins/naming/README.md` (correct) and one in
`claude-config/skills/setup/SKILL.md` (a prerequisite-scope subset
count, now exempted with its reason).
3. **The prefilter tested `*[Ss]kill*` on raw text**, which rejects an
all-caps `SKILLS`, dropping a heading before the case fold could reach
it. The fold now runs first.

Fixing 2 surfaced two defects the review did not name: joining
**double-counted** every wrapped-adjacent claim (46 sites reported where
24 exist — the guard must re-test the pattern against the next line, not
check containment of the matched extent, because most forms open with a
boundary group that captures the join's own space); and one sentence can
satisfy **two grammars at once**, so the guard re-tests every form
rather than only the one that matched.

**One Claude review finding** (`3369fbc6`): the `--check` FAIL message
reconstructed the plugin's real count as `expected + 1`, which is wrong
exactly when the zero-clamp fires — it reported "has one" directly above
"should be zero". The count is now carried through the report row and
read from it. Enforcement was never affected; the diagnostic was, in
precisely the last-skill-removed case the zero-skill scan was built for.

## Verification

**22 black-box assertions**, with the *negative* ones load-bearing —
ordinary prose like "gate one skill" and "nine skills moved into three
new plugins" must not flag.

**Mutation-tested.** Reverting any of these turns the suite red:
mismatch-reported-as-ok, `--check`-never-fails,
drop-the-minus-one-basis, drop-the-stale-exemption-guard,
scan-CHANGELOG, exemption-never-hits, drop-the-case-fold,
skip-plugins-without-a-skills-dir, no-line-joining,
narrow-the-dedup-guard-to-one-form, and restore-the-`expected +
1`-reconstruction.

**Gates run locally, all green on the merged tree:**

- `scripts/check-skill-count-claims.sh --check` — 24 claims match the
tree, 2 exempted
- `bash scripts/check-skill-count-claims.test.sh` — all 22 assertions
passed
- `shellcheck --rcfile=.shellcheckrc` and `shfmt -d` on both new scripts
and the three annotated files
- `actionlint` on `ci.yml`
- `scripts/check-shell-portability.sh --all` — `No unexcused GNU-only
constructs in 1325 shell file(s)` (was `exit 1` with 13 hits)
- `scripts/check-changelog-parity.sh` in all four modes — 212 headings
preserved across 6 changed changelogs
- `scripts/check-lane-coverage.sh --check` — all 39 lanes reachable from
`ci-status.needs`
- `scripts/validate-plugins.sh`, catalog sync, cheat-sheet generator
tests, sync-manifest check
- `scripts/affected-tests.sh --run` — all 11 selected suites passed, no
unmapped files

**The gate proved itself on the merge.** `main` landed a `claude-ops`
eleventh skill while this branch was open. Resolving that conflict took
main's manifest wholesale (so the new skill's description survived
rather than being clobbered by a stale ten-skill string), and the gate
then verified the new claim: `plugins/claude-ops/README.md:4` and its
manifest both read "eleven skills" against eleven skill directories.

**Version bumps + CHANGELOG entries:** `testing` 0.6.2, `verification`
0.5.3, `improvement` 0.1.1, `docs-hygiene` 0.17.2, `claude-ops` 0.33.1,
`repo-hygiene` 0.10.7.

*(This section previously listed `claude-ops 0.32.10` and `repo-hygiene
0.10.6` — stale after the merge with `main` renumbered both. A
hand-written count in prose going stale is the exact defect this PR
gates against, and the PR body is not a surface the gate covers. Caught
by review.)*

## Related

- Refs #3011 — the PR whose six stale counts are this gate's motivating
evidence
- Refs #3035 — landed the `claude-ops` eleventh skill mid-review; its
count is what the gate verified on the merge
-
`docs/adr/0016-source-skill-recommendation-from-the-catalog-not-the-listing.md`
— the decision record from #3011
- `plugins/docs-hygiene/skills/audit-noise/SKILL.md` — the `enum-list`
noise shape, this gate's advisory counterpart
- `scripts/check-skill-leaf-names.sh` — the existing gate whose
script/self-test/CI-lane shape this one follows

---------

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants