review: proactive triggers on security-reviewer and ecosystem-specialist, and the instruction-surface lens in the agent #5281

Activity

1. kyle-sexton

   added

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

   on Sep 29, 2026
2. github-actions

   added

   needs-triageNot yet classified. Floor until a type and one priority tier are set.Not yet classified. Floor until a type and one priority tier are set.

   on Sep 29, 2026
3. kyle-sexton commented on Sep 29, 2026

   

   kyle-sexton

   on Sep 29, 2026

   ContributorAuthor

   More actions

   Owner decision (2026-09-29): Q1: drop the proactive trigger from ecosystem-specialist only; keep security-reviewer's scoped trigger. Q2: decide with evidence; the owner accepted the recommendation, which delegates gathering how often guardrail-only deletions reach the local review path (/review:quality-gate security mode, /review:fanout), measured against the lens at plugins/review/skills/security-review/SKILL.md:112.

   Next: an agent edits the ecosystem-specialist description line (Q1), then gathers the Q2 evidence and posts it here. Because Q2 is a security-posture change, it stops after posting the evidence and the owner makes the final call; needs-human stays.
4. kyle-sexton

   added

   agent-readyFully specified and briefed; eligible for autonomous pickup from the frontier.Fully specified and briefed; eligible for autonomous pickup from the frontier.

   and removed

   status: needs-decisionAwaiting a human or maintainer judgment call.Awaiting a human or maintainer judgment call.

   on Sep 29, 2026
5. kyle-sexton commented on Sep 29, 2026

   

   kyle-sexton

   on Sep 29, 2026

   ContributorAuthor

   More actions

   Evidence for Q2 (not a decision). How often a guardrail-only deletion reaches the local review path (/review:quality-gate security mode, /review:fanout, both via agents/security-reviewer.md, which lacks the lens) versus the CI lane, where the "Instruction-surface deletions" lens at plugins/review/skills/security-review/SKILL.md:112 fires.

   Bottom line

   • Deletion-only changes to instruction surfaces are rare: 7 of 2,833 commits in 90 days. Reading their removed lines, none is a clear register-class guardrail deletion. feat!: reset instruction surfaces to the unhobble bare baseline #2188 and chore: blank the retired standards-synced AGENTS.md #2991 are the closest, and each has a caveat (below).
   • The CI lens has been in the tree for about one day (feat(review): security-review lens for instruction-surface deletions (#3566) #4773, merged 2026-09-28 19:45 -04:00). A historical CI-versus-local comparison of the lens cannot be measured.
   • No record shows the local path running on any of these changes. That is absence of telemetry, not evidence it never ran.
   • The numbers bound the frequency. They do not settle A versus B, because local-path exposure cannot be measured with the data this repo keeps.

   Method

   • Source: origin/main at 96436062d, window 2026-07-01 to 2026-09-29 (90 days; the first commit in the repo is 2026-06-22). Merges are squashes; 2,817 of the 2,833 non-merge commits carry a PR number.
   • Script: git log origin/main --since=2026-07-01 --no-merges -M --numstat, parsed with a Python script (scratch, not committed). Per commit, added and deleted lines are summed over instruction-surface files: CLAUDE.md/AGENTS.md, .claude/rules/**, plugins/*/skills/**/SKILL.md, plugins/*/agents/*.md, plugins/*/hooks/block-*.sh.
   • The hook row is a proxy for "hook block/allow lists": those scripts hold code as well as lists, so that row overcounts.
   • "Deletion-only" = adds 0, deletes > 0. "Deletion-dominant" = 0 < adds < deletes. "Rewrite" = adds >= deletes.
   • Guardrail heuristic: a removed line matching never | must not | do not | don't | forbid* | deny | block* | refus* | prohibit* | only | always | stop (case-insensitive), compared with the same count on added lines. It is a keyword screen, not a judgment about whether a rule was a guardrail.

   Counts (2026-07-01 to 2026-09-29)

   Measure	Commits
   All non-merge commits	2,833
   Touched an instruction surface	1,475
   ...and deleted at least one line	1,234
   ...deletion-only	7
   ...deletion-dominant	79
   ...rewrite	1,148
   Removed more guardrail-keyword lines than it added	96
   ...of those, also net-negative overall (5 deletion-only + 33 deletion-dominant)	38
   Removed guardrail-keyword lines and added none	18

   By surface (files with deletions / deleting more than adding / adding nothing): SKILL.md 1,110 / 75 / 1; agents 66 / 4 / 0; CLAUDE.md/AGENTS.md 23 / 10 / 6; hook block scripts 103 / 6 / 0; .claude/rules 6 / 2 / 0.

   Most of the 1,148 rewrites and 79 deletion-dominant commits are editorial (de-slop, spoke extraction, compression). The keyword screen does not separate an editorial cut from a guardrail cut, so read every count above as an upper bound.

   The 7 deletion-only commits, by what I read of their removed lines:

   • feat!: reset instruction surfaces to the unhobble bare baseline #2188 (unhobble baseline reset): removed 72 lines of CLAUDE.md (a fresh-docs mandate, plugin design rules) and 28 of AGENTS.md, plus the hooks and enabledPlugins blocks in .claude/settings.json. An intentional baseline reset. By my reading the CLAUDE.md content is process and design rules that map loosely, if at all, to the register's six classes (judgment).
   • chore: blank the retired standards-synced AGENTS.md #2991 (blanks the retired AGENTS.md): removed "never patch the materialized copy here" and a cloud-sessions note. managed-files-guard.yml enforces the first mechanically, and the lens does not count a mechanically enforced rule as a finding (judgment: the lens would probably not have flagged it).
   • chore: sync standards components #2587, chore: sync standards components #2062, chore: sync standards components #2107 (standards syncs): removed AGENTS.md sections of procedural guidance (run affected suites, the exec-bit CI fix, Cursor cloud setup). Advisory, not register-class by my reading.
   • docs(work-items): drop the ceremonial ## Purpose from the work skill #3185 (drops a ## Purpose heading) and revert: remove Cursor dual-target marketplace manifests (#1835) #1839 (reverts manifests): classified by title only, not read.

   CI lane on the 96 commits that lost guardrail-keyword lines

   Classified from the ### Security review issue comment by the claude lane on each PR:

   Lane outcome	PRs
   Reviewed with findings or "no issues"	22
   Skip gate	18
   Infrastructure failure	5
   No security comment found	51

   So 56 of 96 had no completed lane review. The 51 were not investigated individually (early PRs, draft PRs, skipped runs, or removed comments are all possible).

   Pre-lens examples:

   • chore: blank the retired standards-synced AGENTS.md #2991: the lane returned "Skip gate: no security-relevant surface ... documentation/provenance-pointer change only". No finding (and, per above, probably correctly).
   • feat!: reset instruction surfaces to the unhobble bare baseline #2188: the lane raised one IMPORTANT finding on the disabled guardrails plugin (a settings removal) and called the CLAUDE.md/AGENTS.md blanking "a soft/advisory-instruction loss, not a technical control". So pre-lens it caught the settings change and dismissed the instruction deletion.
   • docs: run all eight docs-hygiene audits repo-wide, and fix the four detector defects that blocked them #3362 (deleted plugins/ai-briefing/skills/generate/context/execution-flow.md): the lane checked that the outbound-collection safety rules in it survived elsewhere, and found they did. Pre-lens, the lane sometimes checks a deletion without the lens.

   After the lens landed (2026-09-28 19:45 -04:00 onward): 91 commits with surface deletions, 90 with a security-lane comment, 73 of those comments name the instruction-surface lens or the register. None raised a finding about a deleted guardrail. Three raised findings on other subjects, which I read:

   • feat(architecture): land the map-* skills below the landscape #5158: symlink handling in collector scripts. Not lens-class.
   • fix(source-control): repair audit findings in linkage, worktree, config-root and polling #5317: the do-not-merge label gate was enforced only in one tier, after a SKILL.md reword admitted the gap. I called it a code-gate gap, not a softening by deletion.
   • feat(code-tidying): Native step + tidy route for bundled simplify (#4051) #4825: the new unattended flag invokes a mutating native step without asking, a departure from the agent-authority rail stated in docs/conventions/native-references/README.md. This is lens-adjacent (register class, standing instruction overridden) but an addition, not a deletion.

   #5287 shows the lens working as written: the lane refused the prose-only skip, checked each removed passage against the register's six classes, and reported no finding. This window is about one day, so 73 applications with 0 deletion findings is not a rate.

   Local path (/review:quality-gate security mode, /review:fanout)

   • Persisted findings: both write to .work/reviews/<branch-slug>/, gitignored (plugins/review/reference/topic-docs.md). Found 0 findings files in the main checkout's .work and in the worktrees under ~/worktrees.
   • Session transcripts on this machine: 3,381 files, last-write dates 2026-09-17 to 2026-09-29, so about 13 days of history. review:quality-gate Skill calls: 0. Typed <command-name> invocations of review:quality-gate or review:fanout: 0. review:fanout Skill call: 1 session. security-reviewer subagent dispatch: 9 sessions (change under review not determined).
   • PR bodies and comments for the 96 PRs: none records a local run. The four text matches I read describe features, file paths, or the skill as a backstop, not a run.

   Gaps in the data

   • No local-run telemetry: nothing in the repo records skill invocations, and transcript history is short and covers one machine. Other hosts, cloud sessions, and every consumer repo that installs the plugin without this repo's CI lane are not covered. A consumer without the CI lane has only the local agent as its reviewer.
   • The lens is one day old, and CI lane comments were matched by text (the lens or register named), which shows it was invoked, not that it was applied correctly.
   • Deletion counts come from a keyword screen. Only the 7 deletion-only commits had their removed lines read: feat!: reset instruction surfaces to the unhobble bare baseline #2188 and chore: blank the retired standards-synced AGENTS.md #2991 in full, chore: sync standards components #2587, chore: sync standards components #2062 and chore: sync standards components #2107 in part (first lines only), docs(work-items): drop the ceremonial ## Purpose from the work skill #3185 and revert: remove Cursor dual-target marketplace manifests (#1835) #1839 by title.
   • I did not confirm which plugin version the hosted lane loads (claude-security-review-hosted.yml calls ci-workflows v0.30.1); the post-lens comments show the lens active by 2026-09-29.

   Other enforcement that already catches instruction deletions

   • claude-config:audit-instructions and claude-config:unhobble hold register-class deletions, only inside those two trimming skills (register README, consumer table). A hand edit or any other route relies on the lens at PR review.
   • Pinned-gate tests fail CI when a specific gate's text is removed, for example plugins/implementation/skills/implement-dispatch/reattach-slice.test.sh (6 assert_gate checks). They cover only enumerated gates.
   • .github/workflows/silent-revert-canary.yml: post-merge and advisory, detects a squash that drops content another recent commit added. It is not a check on guardrail deletion in general.
   • .github/workflows/managed-files-guard.yml: advisory during soak; fails a PR that hand-edits a standards-managed file. It covers managed files only.
   • No hook, validator, or CODEOWNERS entry reads the instruction exception register; its only code-side reference is an unhobble eval fixture.

   What this implies for the options

   • Option A (mirror the lens into the agent). Closes the local/CI gap for reviewers without the CI lane. Cost: the lens was applied on 73 of the 90 PRs in the post-lens window that deleted a line in an instruction surface, so mirroring adds a register read to most local runs on this repo's own changes. In that one day it produced 0 deletion findings and 1 lens-adjacent finding (feat(code-tidying): Native step + tidy route for bundled simplify (#4051) #4825, an addition, not a deletion), so precision on real deletions is unmeasured.
   • Option B (keep it CI-lane only). On this repo's numbers the exposure is small: no deletion-only change in the window is a clear guardrail-only deletion, 40 of 96 guardrail-keyword-loss PRs had a completed lane review and 90 of 91 post-lens PRs had a lane comment, and no record shows the local path used on these changes. Draft PRs skip the lanes, so a draft-stage change is not covered by CI. The data cannot speak to consumer repos, where the local agent may be the only review.
   • Neither option is favored here. The unmeasured factor that would decide it is how often consumers run the local path with no CI lane, and no data in this repo bears on that.

   The final call on Q2 stays with the owner; needs-human stays.
6. kyle-sexton

   added a commit that references this issue on Sep 29, 2026

   fix(review): drop the proactive trigger from ecosystem-specialist (#5396

   ...

   2d8bc28
7. kyle-sexton commented on Sep 30, 2026

   

   kyle-sexton

   on Sep 30, 2026

   ContributorAuthor

   More actions

   Owner decision (2026-09-29): approved consensus

   Question. Q2 (the only open question; Q1 was decided as B): should the #3566 instruction-surface-deletion lens also live in plugins/review/agents/security-reviewer.md? A. Mirror the lens into the agent. B. Keep it CI-lane only and keep the stated gap.

   Decision. Q2: A, mirrored as a short pointer paragraph (trigger condition, consequence-based recognition, cite docs/conventions/instruction-exception-register/README.md), not a copy of SKILL.md:112-125. Also add the agent as a row in the register's "Who consumes this" table.

   Rationale. The register says "A register nothing consumes changes no behavior" and that a deletion "by hand or by any other route" must meet the lens. The local agent is such a route and currently has no consumer entry (register README lines 40-63). The cost objection is weak: the lens only fires on diffs that delete, narrow or soften a standing instruction (7 of 2,833 deletion-only commits in 90 days), and the "73 of 90" figure counts CI comments naming the lens, not local runs. Local-path exposure cannot be measured, and consumers without the CI lane rely on the agent alone. Confidence is moderate (0.6): precision is unmeasured, so the choice rests on convention.

   Next: needs-human and status: needs-decision are removed and agent-ready is set. An agent will add the pointer paragraph to security-reviewer.md, add the register consumer-table row, and apply the Q1=B description edit to ecosystem-specialist.md, in a draft PR.
8. kyle-sexton

   removed

   needs-humanHuman-in-the-loop required; autonomous sessions must not resolve items carrying this.Human-in-the-loop required; autonomous sessions must not resolve items carrying this.

   on Sep 30, 2026
9. kyle-sexton

   closed this as completedin f1f47dbon Sep 30, 2026