autonomy pipeline: no dedicated security-review pass — automated code review only

[kyle-sexton]

Gap

The autonomy pipeline runs automated code review on every PR (claude[bot] review + work-lane post-green review pass), but there is no dedicated security review anywhere in the flow. Boris's full-autonomy guidance stresses both: code reviews AND separate dedicated security reviews. We have the first, not the second.

Current state

• review:security-reviewer agent and a security-review skill exist in the toolbox, but nothing wires them into lane flow, the work-lane review pass, or babysit merge criteria.
• No security gate in the feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 autopilot merge tier criteria as currently specified.

Questions for triage

• Scope: every PR, or only PRs touching security-sensitive surfaces (auth, tokens, shell execution, network calls, permission configs)? Path-triggered seems the right default — most plugin PRs are markdown/prose.
• Placement: work-lane post-green pass (before handoff to babysit) vs. a babysit merge criterion vs. a separate scheduled sweep. Placement interacts with feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 — if autopilot merges land, the security pass must be upstream of the merge gate.
• Relation to WP5 (feat(autonomy): guardrail matrix, security binding, setup slice (WP5) #377 guardrail matrix / security binding) — possibly the same contract surface; avoid two systems.

Filed from control-tower checkpoint on operator direction (2026-07-19 late-afternoon).

---

Reference: Boris Cherny, "Steps of AI Adoption" (Google Doc)

[kyle-sexton]

This was generated by AI during triage. · Category: autonomy-pipeline gap (security) · Priority: priority: high (security gap in the autonomy pipeline; Boris guidance stresses dedicated security review). Routed needs-human + wayfind: design.

Why needs-human

Genuine open design + security product-intent, and it interacts with two other in-flight surfaces — a defaultable answer would pre-empt decisions the maintainer must own. The pieces exist (review:security-reviewer agent + security-review skill) but nothing wires them into lane flow; the OPEN questions are scope, placement, and non-duplication:

1. Scope — every PR vs only security-sensitive surfaces (auth, tokens, shell execution, network calls, permission configs). Recommended: path-triggered on security-sensitive diffs (most plugin PRs are markdown/prose); a full-every-PR pass is wasteful.
2. Placement — work-lane post-green pass (before babysit handoff) vs a babysit merge criterion vs a separate scheduled sweep. Interacts with feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476: if the autopilot merge tier lands, the security pass MUST be upstream of that merge gate (a merge criterion at minimum).
3. Non-duplication with feat(autonomy): guardrail matrix, security binding, setup slice (WP5) #377 (WP5 guardrail matrix / security binding) — possibly the same contract surface; the maintainer must decide whether this is one system with feat(autonomy): guardrail matrix, security binding, setup slice (WP5) #377 or a separate lane, to avoid two overlapping security systems.

Recommended framing (for the human to ratify)

Path-triggered review:security-reviewer on security-sensitive diffs, wired as (a) a work-lane post-green review lane AND (b) a #476 merge criterion, unified under the #377 guardrail-matrix contract rather than a parallel system. Then it becomes implementable.

Relations

#476 (autopilot merge tier — security pass must gate it) · #377 (guardrail matrix — candidate shared contract) · #510 (zizmor findings invisible — a security-review pass that also can't see zizmor output still misses that class; wire both) · #513 (security review is one of the role-separated review lanes).

[kyle-sexton]

This was generated by AI (needs-human decision-brief pass).

Decision brief (2-minute read)

Problem. The autonomy pipeline runs automated code review but no dedicated security review pass. This is a wiring gap, not a build gap — the pieces already exist and are invoked on demand; nothing invokes them in the autonomous flow. The open call is scope, placement, and non-duplication.

Today (verified in repo):

• plugins/review/agents/security-reviewer.md exists (opus, effort high) and IS wired into on-demand review: review:fanout, review:quality-gate (which has a dedicated plugins/review/skills/quality-gate/context/security.md), and plugins/source-control/skills/pull-request/reference/prep.md.
• Autonomy flow has no security pass: plugins/work-items/skills/work/SKILL.md contains zero security references; its post-green pass is the generic "explore → plan → implement → test → review → PR" (SKILL.md:142).
• plugins/source-control/skills/babysit-prs/SKILL.md only classifies security findings surfaced by bot reviews ("is this security/P1?", SKILL.md:106,220-222,393) — it never runs security-reviewer. No security gate in the feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 merge-tier criteria.

Options.

• RECOMMENDED — Path-triggered security-reviewer wired at two points: (a) a work-lane post-green lane on security-sensitive diffs, and (b) a feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 merge-tier criterion — unified under feat(autonomy): guardrail matrix, security binding, setup slice (WP5) #377's guardrail-matrix contract, not a parallel system. Path-trigger on auth / tokens / shell-exec / network / permission-config / CI diffs (most plugin PRs are markdown/prose, so full-every-PR is wasteful). Reversibility: MEDIUM — trigger paths and placement are config and easily tuned, but once it's a merge gate blocking autopilot merges, loosening it after a miss is a policy change, not a flag flip. Tradeoff: two wiring points add surface, but feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 requires the gate upstream of merge.
• Single placement — babysit merge criterion only. Tradeoff: one integration point, catches everything pre-merge; but findings surface late (at the gate), so rework is costlier than catching in the work lane. Reversibility: MEDIUM.
• Separate scheduled security sweep. Tradeoff: lowest coupling, but it is not a gate — a PR can merge before the sweep runs, so it fails feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476's "upstream of merge" requirement. Reversibility: HIGH.

Constraint the triage comment misses (#440). The decided default outward_write_mode: report-only suppresses all outward writes. A security pass that posts findings to the PR is an outward write → suppressed under the default. So in report-only mode this must be a local gate that reports/blocks, and "post findings as a review comment" is write-mode-only behavior. This reshapes placement: the gate's verdict must be consumable locally, not dependent on a PR comment landing.

Cross-links.

• feat(source-control): babysit autopilot merge tier — bot-account review + criteria-gated merge #476 (autopilot merge tier — APPROVED staged; the security gate must sit upstream of the merge gate).
• feat(autonomy): guardrail matrix, security binding, setup slice (WP5) #377 (WP5 guardrail matrix — candidate shared contract; fold in rather than build a second security system).
• source-control: no read/consume-only posture — "safe" babysit tier and pull-request monitor write to GitHub autonomously #440 (report-only default — see constraint above).
• ci/agents: zizmor advisory findings are check-run annotations — invisible to every agent surface (green check, unread annotations) #510 (zizmor findings invisible — a security pass that also can't see zizmor output still misses that class; wire both).
• work-items(work): per-issue execution should compose installed skills as a role-separated mini-SDLC (implementer ≠ reviewer ≠ verifier, ephemeral fresh-context agents) #513 (security is one of the role-separated review lanes).

[kyle-sexton]

Operator input (2026-07-19, decision-session batch 2; relayed). DECISION DEFERRED — routed to a design discussion, stays needs-human.

The brief's RECOMMENDED wiring (path-triggered security-reviewer at work-lane post-green + #476 merge criterion) was NOT accepted as-is. Operator's words:

See, my thought was: if every ticket requires a security review before it's PR'd, is there a way to manage that, like labels on the PR? How do we enforce that that's done on every PR, whether it's from a Claude code session that's running in a loop? We have: a loop for PR babysitting, a loop for issue triage, a loop for issue work. It kicks out the PRs. Do we need to have a /loop that does reviews? I don't know. There's probably more to discuss here. I don't really feel comfortable, and there's a bigger design picture here as far as autonomous loops and labels, and having things that run in loops that take action when we make sure of the different lanes of responsibility. Things are not missed, and things are done in a certain way, and then we have conventions documented for all that.

Reading: the open design is not where to invoke the reviewer but how the pipeline enforces and evidences that the pass happened on every PR — enforcement mechanism (labels vs required check vs merge-gate criterion), lane-of-responsibility conventions (does review deserve its own loop lane), and the documentation of those conventions. The brief's wiring proposal becomes one input to that design session, not the decision.

Suggested next step: a /planning:design session scoped to "autonomous-lane responsibility + enforcement conventions (security review as the forcing case)", cross-linking #535 (lane resilience), #530 (quality-net staging), #476 (merge criteria — natural enforcement point), #377 (guardrail matrix), #513 (mini-SDLC roles).