Skip to content

fix(triage): gate the remediation planner on the triage verdict (ADR-0051 §6) #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
galanko merged 3 commits into from
Jun 15, 2026
Merged

fix(triage): gate the remediation planner on the triage verdict (ADR-0051 §6) #276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2be0d4e
fix(triage): gate the remediation planner on the triage verdict (ADR-…
galanko Jun 14, 2026
7f639b8
fix(triage): address review — lint, concrete-verdict gate, verdict-poll
galanko Jun 15, 2026
d331d38
fix(triage): self-review — honor human confirm-real; fix re-run regre…
galanko Jun 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions backend/cliff/api/routes/agent_execution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,48 @@ async def run_all_pipeline(
executor = request.app.state.agent_executor
context_builder = request.app.state.context_builder

# The Plan gate (ADR-0051 §6 / ADR-0054): a finding triaged as *not real*
# never gets a remediation plan — clearing noise with reasoning is the
# product's value, and an alarmist plan on a non-reachable finding is a
# false-positive-shaped result. The CLI (`cliffsec fix`) gates on the
# verdict before it ever calls run-all; this is the server-side backstop so
# NO caller (web, API) can drive the planner on noise. Defence-in-depth, so
# it is intentionally narrow: it engages only when triage has run AND
# returned a non-real verdict AND there is no human-approved plan overriding
# it. A finding that was never triaged behaves exactly as before.
sidebar = await get_sidebar(db, workspace_id)
triage = sidebar.triage if sidebar and isinstance(sidebar.triage, dict) else None
# Gate only on a CONCRETE non-real verdict. A missing/partial triage write
# ({} or no ``verdict``) leaves ``verdict`` empty and falls through to the
# normal pipeline — only a real dismissal/needs_review verdict blocks the
# planner, so an in-flight or never-triaged finding behaves as before.
verdict = (triage.get("verdict") or "").lower() if triage else ""
Comment on lines +347 to +352

@baz-reviewer baz-reviewer Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triage.get("verdict").lower() is called unconditionally, but sidebar.triage can be arbitrary JSON from PUT /workspaces/{workspace_id}/sidebar, so a non-string or missing verdict raises AttributeError and 500s POST /pipeline/run-all — should we guard with isinstance(verdict, str) and fall back to '' when not?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents 
In `backend/cliff/api/routes/agent_execution.py` around lines 347-352 inside
`run_all_pipeline`, add a type guard before calling `.lower()` on
`triage.get("verdict")`. If the value is missing or not a `str`, treat it as unapproved
(e.g., normalize to `''` or `None`) so malformed or legacy sidebar JSON can't raise
`AttributeError`. Ensure the endpoint continues down the normal gated response path
instead of failing the request.

Heads up!

Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription

if verdict and verdict != "real":
# ...but a HUMAN can override a non-real verdict. The web "Looks real —
# remediate" action confirms the finding by advancing its status to
# ``triaged`` (ADR-0051 §6 — ``triaged`` means "confirmed real") BEFORE
# firing run-all; an already-approved plan is the same signal. Only an
# unconfirmed finding still sitting at ``new`` with a non-real verdict is
# blocked, so the gate stops auto-planning on noise without breaking the
# human confirm-real path. (The CLI `real` path passes the verdict check
# above and never reaches here.)
finding = await get_finding(db, workspace.finding_id)
human_confirmed = finding is not None and finding.status != "new"
plan_approved = bool((sidebar.plan or {}).get("approved")) if sidebar else False
if not human_confirmed and not plan_approved:
Comment on lines +362 to +365

@baz-reviewer baz-reviewer Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

human_confirmed treats any status other than "new" as confirmed-real, so mark_started_on_workspace_create() + run_triage_endpoint() can leave a non-real triage eligible for /pipeline/run-all; should we narrow this to finding.status == "triaged" plus the approved-plan override instead of status != "new"?

Severity

Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents 
Before applying, verify this suggestion against the current code. In
backend/cliff/api/routes/agent_execution.py around lines 318-367 inside
`run_all_pipeline`, the gate computes `human_confirmed = finding is not None and
finding.status != "new"`, which incorrectly treats any non-new Finding.status as a
confirmed-real verdict and can bypass the auto-planning/noise block. Refactor this logic
so the bypass only happens when the finding is actually in the confirmed-real state
(e.g., `finding.status == "triaged"`) and still allow the approved-plan override to
bypass as intended. Ensure `finding` is fetched before the gate, and update the log
message/variable naming to reflect the narrower condition so non-real triage updates
can’t slip through.

Heads up!

Your free trial ends tomorrow.
To keep getting your PRs reviewed by Baz, update your team's subscription

logger.info(
"run-all gated for workspace %s: triaged %r, finding still new — no plan",
workspace_id,
verdict,
)
return RunAllResponse(
status="gated",
message=(
f"Triaged as {verdict} — no remediation plan needed. "
f"Confirm it's real to plan a fix."
),
)

try:
await executor.check_not_busy(db, workspace_id)
except AgentBusyError as exc:
Expand Down
267 changes: 267 additions & 0 deletions backend/tests/test_routes_agent_execution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import asyncio
from datetime import UTC, datetime
from types import SimpleNamespace
from typing import TYPE_CHECKING, Any
from unittest.mock import AsyncMock, patch

Expand Down Expand Up @@ -212,6 +213,11 @@ def _capturing_create_task(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
# No prior triage → the Plan gate is dormant; behaviour unchanged.
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=None),
),
patch(
"cliff.api.routes.agent_execution._resolve_repo_env_vars",
AsyncMock(return_value={}),
Expand Down Expand Up @@ -278,6 +284,11 @@ def _capturing_create_task(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
# No prior triage → the Plan gate is dormant; behaviour unchanged.
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=None),
),
patch(
"cliff.api.routes.agent_execution._resolve_repo_env_vars",
AsyncMock(return_value={}),
Expand All @@ -296,6 +307,262 @@ def _capturing_create_task(

assert executor.execute.await_count == 1

@pytest.mark.asyncio
async def test_run_all_gated_when_triaged_not_real(
self, app: FastAPI, client: AsyncClient
) -> None:
"""The Plan gate (ADR-0051 §6 / ADR-0054): a finding triaged as *not
real* never drives the planner pipeline. run-all returns ``gated`` and
executes no agent — the report-tour bright line, enforced server-side so
no caller (web, API) can emit an alarmist plan on noise."""
executor = app.state.agent_executor
executor.check_not_busy = AsyncMock()
executor.execute = AsyncMock(return_value=_make_execution_result())

# Capture create_task so we can assert the gated branch schedules NO
# background pipeline at all (a regression could create the task and
# fail before execute() — checking execute alone would miss it).
captured_tasks: list[asyncio.Task[Any]] = []
real_create_task = asyncio.create_task

def _capturing_create_task(
coro: Coroutine[Any, Any, Any],
) -> asyncio.Task[Any]:
task = real_create_task(coro)
captured_tasks.append(task)
return task

sidebar = SimpleNamespace(
triage={"verdict": "unexploitable", "confidence": 0.88},
plan=None,
)
with (
patch(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=sidebar),
),
# Finding still `new` (un-confirmed) → the non-real verdict gates.
patch(
"cliff.api.routes.agent_execution.get_finding",
AsyncMock(return_value=SimpleNamespace(status="new")),
),
patch(
"cliff.api.routes.agent_execution.asyncio.create_task",
_capturing_create_task,
),
):
resp = await client.post("/api/workspaces/ws-1/pipeline/run-all")

assert resp.status_code == 202
assert resp.json()["status"] == "gated"
assert not captured_tasks, "the gated branch must schedule no background pipeline"
assert executor.execute.await_count == 0, (
"the planner pipeline must never run for a non-real triage verdict"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

@pytest.mark.asyncio
async def test_run_all_not_gated_when_human_confirmed_real(
self, app: FastAPI, client: AsyncClient
) -> None:
"""A human can override a non-real verdict: the web "Looks real —
remediate" action advances the finding to ``triaged`` (ADR-0051 §6)
before firing run-all. The gate must let that through even though
``sidebar.triage.verdict`` is still non-real — otherwise the confirm-real
path is silently dead-ended (no plan, no error)."""
executor = app.state.agent_executor
executor.check_not_busy = AsyncMock()
executor.execute = AsyncMock(return_value=_make_execution_result())
executor.push_permission_event = lambda ws_id, evt: None

# All sections present so the loop breaks immediately (no execute call) —
# we only need to prove the gate let the run through (status running).
app.state.context_builder.get_context_snapshot = AsyncMock(
return_value={
"finding": {"id": "f-1"},
"enrichment": {"normalized_title": "x"},
"ownership": None,
"exposure": {"recommended_urgency": "high"},
"evidence": {"affected_files": [], "fix_safety": "safe_bump"},
"plan": {"plan_steps": ["s"]},
"remediation": {"status": "pr_created"},
"agent_run_history": [],
}
)

# Non-real verdict, NO approved plan — but the human confirmed it real
# (finding advanced to ``triaged``).
sidebar = SimpleNamespace(
triage={"verdict": "needs_review", "confidence": 0.55}, plan=None
)
captured_tasks: list[asyncio.Task[Any]] = []
real_create_task = asyncio.create_task

def _capturing_create_task(
coro: Coroutine[Any, Any, Any],
) -> asyncio.Task[Any]:
task = real_create_task(coro)
captured_tasks.append(task)
return task

with (
patch(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=sidebar),
),
patch(
"cliff.api.routes.agent_execution.get_finding",
AsyncMock(return_value=SimpleNamespace(status="triaged")),
),
patch(
"cliff.api.routes.agent_execution._resolve_repo_env_vars",
AsyncMock(return_value={}),
),
patch(
"cliff.api.routes.agent_execution.asyncio.create_task",
_capturing_create_task,
),
):
resp = await client.post("/api/workspaces/ws-1/pipeline/run-all")
assert resp.status_code == 202
assert resp.json()["status"] == "running", (
"a human-confirmed (triaged) finding must NOT be gated"
)
assert captured_tasks, "the confirm-real path must reach the pipeline"
await captured_tasks[0]

@pytest.mark.asyncio
async def test_run_all_not_gated_on_empty_triage_verdict(
self, app: FastAPI, client: AsyncClient
) -> None:
"""A partial/empty triage write ({} or no verdict) must NOT gate — only
a concrete non-real verdict blocks the planner. An in-flight or
never-triaged finding behaves exactly as before (proceeds)."""
executor = app.state.agent_executor
executor.check_not_busy = AsyncMock()
executor.execute = AsyncMock(return_value=_make_execution_result())
executor.push_permission_event = lambda ws_id, evt: None

app.state.context_builder.get_context_snapshot = AsyncMock(
return_value={
"finding": {"id": "f-1"},
"enrichment": {"normalized_title": "x"},
"ownership": None,
"exposure": {"recommended_urgency": "high"},
"evidence": {"affected_files": [], "fix_safety": "safe_bump"},
"plan": {"plan_steps": ["s"], "approved": True},
"remediation": {"status": "pr_created"},
"agent_run_history": [],
}
)

sidebar = SimpleNamespace(triage={}, plan=None) # partial write, no verdict
captured_tasks: list[asyncio.Task[Any]] = []
real_create_task = asyncio.create_task

def _capturing_create_task(
coro: Coroutine[Any, Any, Any],
) -> asyncio.Task[Any]:
task = real_create_task(coro)
captured_tasks.append(task)
return task

with (
patch(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=sidebar),
),
patch(
"cliff.api.routes.agent_execution._resolve_repo_env_vars",
AsyncMock(return_value={}),
),
patch(
"cliff.api.routes.agent_execution.asyncio.create_task",
_capturing_create_task,
),
):
resp = await client.post("/api/workspaces/ws-1/pipeline/run-all")
assert resp.status_code == 202
assert resp.json()["status"] == "running"
assert captured_tasks, "an empty triage verdict must not gate the pipeline"
await captured_tasks[0]

@pytest.mark.asyncio
async def test_run_all_proceeds_when_triaged_real(
self, app: FastAPI, client: AsyncClient
) -> None:
"""A ``real`` verdict releases the Plan gate — run-all proceeds normally
(status ``running``, background pipeline created)."""
executor = app.state.agent_executor
executor.check_not_busy = AsyncMock()
executor.execute = AsyncMock(return_value=_make_execution_result())
executor.push_permission_event = lambda ws_id, evt: None

# All sections present + plan approved → the loop breaks immediately
# (no execute call), but the task is still created — proving the gate
# let the run through rather than short-circuiting to ``gated``.
app.state.context_builder.get_context_snapshot = AsyncMock(
return_value={
"finding": {"id": "f-1"},
"enrichment": {"normalized_title": "x"},
"ownership": None,
"exposure": {"recommended_urgency": "high"},
"evidence": {"affected_files": [], "fix_safety": "safe_bump"},
"plan": {"plan_steps": ["s"], "approved": True},
"remediation": {"status": "pr_created"},
"agent_run_history": [],
}
)

sidebar = SimpleNamespace(
triage={"verdict": "real", "confidence": 0.82},
plan={"plan_steps": ["s"], "approved": True},
)
captured_tasks: list[asyncio.Task[Any]] = []
real_create_task = asyncio.create_task

def _capturing_create_task(
coro: Coroutine[Any, Any, Any],
) -> asyncio.Task[Any]:
task = real_create_task(coro)
captured_tasks.append(task)
return task

with (
patch(
"cliff.api.routes.agent_execution.get_workspace",
return_value=_make_workspace(),
),
patch(
"cliff.api.routes.agent_execution.get_sidebar",
AsyncMock(return_value=sidebar),
),
patch(
"cliff.api.routes.agent_execution._resolve_repo_env_vars",
AsyncMock(return_value={}),
),
patch(
"cliff.api.routes.agent_execution.asyncio.create_task",
_capturing_create_task,
),
):
resp = await client.post("/api/workspaces/ws-1/pipeline/run-all")
assert resp.status_code == 202
assert resp.json()["status"] == "running"
assert captured_tasks, "the gate must let a real verdict proceed"
await captured_tasks[0]


# ---------------------------------------------------------------------------
# Suggest-next endpoint
Expand Down
Loading
Loading