Skip to content

fix(review,implementation): lower effort pins on three binary-criteria checkers to medium - #5538

Merged
kyle-sexton merged 5 commits into
mainfrom
fix/4253-lower-checker-effort-pins
Sep 30, 2026
Merged

kyle-sexton merged 5 commits into
mainfrom
fix/4253-lower-checker-effort-pins

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Refs: #4253

Summary

The owner decided Option B on #4253 (2026-09-29): lower effort: high to effort: medium on the three checkers that verify against binary criteria. security-reviewer and architecture-guardian stay high. medium, not low, because a low-effort executor stops detecting that it is stuck.

Fix

  • effort: medium in phase-verifier, ci-log-auditor, doc-drift-detector.
  • implementation 0.19.15 and review 0.33.9, each with a ### Changed CHANGELOG entry naming the lowered agents and the ones that stay high.
  • docs/plugin-philosophy.md pinned-agents record: eleven high, four medium, the moved agents, the kept-high pair, and a Recheck trigger for a medium checker missing a defect.
  • The plan-reviewer exception text ("not the high that a consequential-verdict lane pins") still reads true and is unchanged.

Verification

Recount on the branch:

git grep -h '^effort: high'   -- 'plugins/*/agents/*.md' | wc -l  -> 11
git grep -h '^effort: medium' -- 'plugins/*/agents/*.md' | wc -l  -> 4
plugins/review/agents/architecture-guardian.md:6:effort: high
plugins/review/agents/security-reviewer.md:6:effort: high

scripts/check-changelog-parity.sh --check and --check-bump origin/main both pass.

Pin probe (headless claude -p, claude 2.1.285, subagent review:code-reviewer pinned effort: high, transcript records read from the sidechain JSONL):

  • --effort low: main session "effort":"low"; subagent "attributionAgent":"review:code-reviewer","effort":"high","perTurnEffort":"high". The pin takes effect over a lower session flag.
  • CLAUDE_CODE_EFFORT_LEVEL=low: subagent "effort":"low","perTurnEffort":"low". The environment variable overrides the pin.
  • Caveat: headless run only. The owner's interactive /tasks check was not done.

The issue stays open. The final call, and the ready flip and merge, stay with the owner.

Related

#4253, #4974, #5268

🤖 Generated with Claude Code

kyle-sexton and others added 4 commits September 29, 2026 23:06
…a checkers to medium

phase-verifier, ci-log-auditor and doc-drift-detector move from effort high to medium.
security-reviewer and architecture-guardian stay high. Refs #4253.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review September 30, 2026 03:11
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-30T03:14:32.987025Z c3fd1da Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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: c3fd1dab8e

ℹ️ 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 docs/plugin-philosophy.md
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review (/review:code-review)

  • Gather context (PR body, diff via gh pr diff, docs/plugin-philosophy.md, CLAUDE.md/AGENTS.md)
  • Review changed files for correctness and maintainability
  • Check CHANGELOG/version bump parity
  • Post findings

No blocking findings. This is a small, well-scoped effort-pin change and it holds together:

  • plugins/implementation/agents/phase-verifier.md, plugins/review/agents/ci-log-auditor.md, plugins/review/agents/doc-drift-detector.md each drop effort: high → effort: medium (single-line frontmatter change, no other body text in those files references the old level, so nothing else went stale).
  • security-reviewer and architecture-guardian are confirmed still effort: high — independently re-verified with git grep -n '^effort: high|^effort: medium' -- 'plugins/*/agents/*.md', which returns exactly 11 high and 4 medium agents, matching the counts the PR claims and the counts docs/plugin-philosophy.md's "Pinned effort: high agents" section now states.
  • The unchanged plan-reviewer exception text at docs/plugin-philosophy.md:1049 ("not the high that a consequential-verdict lane pins") still reads true against the updated section above it — confirmed by reading both passages together.
  • implementation 0.19.14 → 0.19.15 and review 0.33.8 → 0.33.9 each carry a new, non-duplicate ### Changed CHANGELOG entry naming the lowered agents and the ones staying high, consistent with scripts/check-changelog-parity.sh's bump-gate rules (new heading, absent at base, monotonic, real shipped change alongside it).
  • Note: I could not execute scripts/check-changelog-parity.sh --check / --check-bump origin/main myself in this sandbox (the Bash tool declined the invocation, and this unattended lane carries no one to answer an approval prompt), so that specific claim in the PR's Verification section is author-claimed and I did not independently confirm it by running it — the manual trace above is consistent with what the script's documented rules would require, but is not a substitute for the script actually passing in CI.

No correctness, architecture, or maintainability issues found in the diff itself.

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


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

Security review

Reviewed the actual change commit (3945167, matches the PR's 8 changed files) against /review:security-review criteria.

Skip gate applies — no security-relevant surface.

The diff:

No code path, auth/access-control logic, input handling, or secret handling is touched.

Instruction-surface lens (required before skipping, since this edits agent bodies). I read docs/conventions/instruction-exception-register/README.md and checked this change against the six Gate 0 protected classes (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority). An effort pin tunes how much compute a subagent applies to the same verification criteria — it doesn't delete, narrow, or soften any checklist, permission, or behavioral rule the checker enforces, and it doesn't match agent-authority (that class covers approval gates / autonomy limits / spend caps, not verification thoroughness). The PR itself already reasons about the risk this lens exists to catch: it picked medium over low specifically because a low-effort executor "stops detecting that it is stuck," kept security-reviewer and architecture-guardian at high (the two checkers with consequential, non-binary verdicts), and added an explicit docs/plugin-philosophy.md Recheck trigger for "a checker pinned medium misses a defect." That's the deliberate, bounded, documented handling this lens asks for, not an unrecognized guardrail removal — so this doesn't rise to a finding.

Result: no security findings. This is a scoped, well-documented configuration/doc change with no exploitable surface in this diff.

Branch: fix/4253-lower-checker-effort-pins

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review (/review:code-review)

  • Gather context (PR body, diff via gh pr diff, docs/plugin-philosophy.md, AGENTS.md)
  • Review changed files for correctness and maintainability
  • Check CHANGELOG/version-bump parity
  • Re-verify the Codex-flagged contradiction against the latest commit
  • Post findings

No blocking findings. This re-reviews head 5b8099f, which adds one commit (docs: name the three medium-pinned checkers in the effort lane rule) on top of c3fd1da, addressing the one open Codex finding.

  • Codex's finding is resolved. Codex flagged that the lane rule at docs/plugin-philosophy.md (then lines 1211–1223) still required every consequential-verdict agent to pin high, naming only plan-reviewer as an exception, while the frontmatter now ships three more medium pins — a documented contradiction. The new commit fixes exactly this: it extends that rule's carve-out sentence to read `planning:plan-reviewer` pins `medium` by the [recorded exception](#named-agent-bar), and `implementation:phase-verifier`, `review:ci-log-auditor`, and `review:doc-drift-detector` pin `medium` because each checks against binary criteria — docs/plugin-philosophy.md#L1221-L1225. I independently re-checked this against the surrounding rule text and the "Pinned effort: high agents" bullet a few lines down; they now agree.
  • Recounted the frontmatter independently (not just re-stating the PR's claimed numbers): git grep -n '^effort: high' -- 'plugins/*/agents/*.md' returns exactly 11 files, ^effort: medium returns exactly 4 (phase-verifier, plan-reviewer, ci-log-auditor, doc-drift-detector). Both match the "Basis" enumeration at docs/plugin-philosophy.md#L1325-L1332, and security-reviewer/architecture-guardian are confirmed still high.
  • CHANGELOG/version parity holds by inspection: implementation 0.19.14→0.19.15 and review 0.33.8→0.33.9 each add one new, non-duplicate ### Changed entry naming the lowered agents and the ones staying high; both are patch bumps, consistent with this plugin's own precedent for prior ### Changed entries (e.g. review 0.33.6, 0.33.7 in the same file). I could not execute scripts/check-changelog-parity.sh myself in this sandbox (Bash approval was declined and this unattended lane has no one to answer the prompt), so the script's own pass/fail is author-claimed and unverified by me — the manual check above is the closest substitute.
  • Checked for stale references elsewhere in the repo to the old "fourteen high / one medium" framing or the three checkers' previous high pin — found none in docs/ outside the file this PR already updates; the two other doc hits for effort: high (docs/specs/prompt-audit-skills-2026-09.md, docs/upstream/claudedevs-cost-performance.md, docs/cloud-sessions.md) are unrelated to these three agents.
  • Noted a wrapped-prose line at docs/plugin-philosophy.md#L1347 that runs longer than the surrounding paragraph's usual wrap width — not reported as a finding, since .markdownlint-cli2.jsonc explicitly disables MD013 (no line-length rule) and this is a pure style nit excluded by this lane's high-signal bar.

No correctness, architecture, or maintainability issues found in the diff.
· Branch: fix/4253-lower-checker-effort-pins

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review (/review:security-review)

  • Gather context (PR body, diff via gh pr diff, custom instructions)
  • Invoke /review:security-review skill and follow it exactly
  • Post findings

Skip gate does not fully apply, so the diff was read under the instruction-surface lens; result: no security-relevant surface, no findings.

Reviewed the full two-commit diff (3945167..5b8099f, all 8 changed files) against /review:security-review criteria.

  • The functional change is three agent-frontmatter effort: high → effort: medium flips: plugins/implementation/agents/phase-verifier.md#L9, plugins/review/agents/ci-log-auditor.md#L6, plugins/review/agents/doc-drift-detector.md#L6. The rest is two version bumps, two matching CHANGELOG entries, and prose/inventory updates in docs/plugin-philosophy.md. No code path, auth/access-control logic, input handling, secret handling, or GitHub Actions workflow is touched.
  • Instruction-surface lens, applied because this changes an agent's frontmatter pin. I read docs/conventions/instruction-exception-register/README.md and checked the change against the six Gate 0 protected classes (irreversible-action, secret-handling, data-integrity, external-publication, legal-compliance, agent-authority). An effort pin tunes how much compute a subagent spends verifying against the same, unchanged checklist — it doesn't delete, narrow, or soften any behavioral rule, permission, or checklist item the checker enforces, and agent-authority in the register covers approval gates, autonomy limits, and spend caps, not verification thoroughness. docs/plugin-philosophy.md at line ~1310 itself names the risk this lens exists to catch and shows it was reasoned through: medium was picked over low specifically because "a low-effort executor stops detecting that it is stuck," security-reviewer and architecture-guardian (the two checkers with consequential, non-binary verdicts) stay high, and a new Recheck trigger — "a checker pinned medium misses a defect its high pin caught" — was added as a rollback signal. That's bounded, documented handling of the tradeoff, not an unrecognized guardrail removal, so this doesn't rise to a finding under the register.
  • I separately confirmed the doc-consistency gap the earlier Codex review flagged (the exception-clause text near docs/plugin-philosophy.md#L1211-1223 not naming the three checkers) is already resolved by the second commit (5b8099f, "docs: name the three medium-pinned checkers in the effort lane rule"), which adds implementation:phase-verifier, review:ci-log-auditor, and review:doc-drift-detector to that clause. That's a documentation-consistency matter for /review:code-review, not a security finding, and it's moot here since it's already fixed.

Result: no security findings. Scoped configuration/doc change; no exploitable surface in this diff.

Branch: fix/4253-lower-checker-effort-pins

@kyle-sexton
kyle-sexton merged commit c79aa29 into main Sep 30, 2026
19 checks passed
@kyle-sexton
kyle-sexton deleted the fix/4253-lower-checker-effort-pins branch September 30, 2026 03:55
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.

1 participant