fix: force code review lanes to be additive only, not opt-in, and bump the verifier run cap
This commit is contained in:
7 files changed
+108
-29
No files matched your search
@@ -174,7 +174,7 @@ nodes:
|
|||||||
- fs_read
|
- fs_read
|
||||||
- fs_cat
|
- fs_cat
|
||||||
- fs_grep
|
- fs_grep
|
||||||
max_iterations: 14
|
max_iterations: 28
|
||||||
max_attempts: 2
|
max_attempts: 2
|
||||||
state_updates:
|
state_updates:
|
||||||
verdict: '{{output}}'
|
verdict: '{{output}}'
|
||||||
|
|||||||
@@ -120,7 +120,15 @@ instructions: |
|
|||||||
update — never delete or silence it).
|
update — never delete or silence it).
|
||||||
4. **Clean, local, isolated.** Fresh ephemeral state, mocked externals, no dependence on
|
4. **Clean, local, isolated.** Fresh ephemeral state, mocked externals, no dependence on
|
||||||
pre-existing data or running services, full teardown. Bounded retries for startup only —
|
pre-existing data or running services, full teardown. Bounded retries for startup only —
|
||||||
never to mask a flaky assertion.
|
never to mask a flaky assertion. Teardown MUST NOT depend on a shell trap firing (a
|
||||||
|
killed or timed-out lane leaks containers — it has happened): pick one unique run
|
||||||
|
prefix up front (e.g. `probe-$$`), give EVERY container you create `--name <prefix>-…`
|
||||||
|
and `--label coyote-probe=<prefix>` (compose: `-p <prefix>`), prefer `docker run --rm`,
|
||||||
|
and make the LAST action before emitting your verdict an explicit sweep:
|
||||||
|
`docker ps -aq --filter label=coyote-probe=<prefix> | xargs -r docker rm -f` (compose:
|
||||||
|
`docker compose -p <prefix> down -v`). The label filter keeps the sweep scoped to YOUR
|
||||||
|
resources — rule 0 still forbids touching anyone else's. A leftover you cannot remove
|
||||||
|
is a finding to REPORT in the verdict, never to silence.
|
||||||
5. **Classify every failure** as BUG / EXPECTED-CHANGE / ENV per the skill table. The verdict
|
5. **Classify every failure** as BUG / EXPECTED-CHANGE / ENV per the skill table. The verdict
|
||||||
depends on the classification being honest.
|
depends on the classification being honest.
|
||||||
6. **Committed tests are the deliverable** alongside the verdict: write them where the repo's
|
6. **Committed tests are the deliverable** alongside the verdict: write them where the repo's
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ flowchart TD
|
|||||||
parse -. "fallback" .-> pfault["parse_fault<br/>(script: PIPELINE-FAULT marker)"]
|
parse -. "fallback" .-> pfault["parse_fault<br/>(script: PIPELINE-FAULT marker)"]
|
||||||
pfault --> gate
|
pfault --> gate
|
||||||
signals --> select["select_lanes<br/>(llm: ADDITIVE-only judgment)"]
|
signals --> select["select_lanes<br/>(llm: ADDITIVE-only judgment)"]
|
||||||
select --> build["build_items<br/>(script: rules floor ∪ additions;<br/>forced lanes override exactly)"]
|
select --> build["build_items<br/>(script: rules floor ∪ additions;<br/>forced lanes add)"]
|
||||||
select -. "fallback" .-> dlanes["default_lanes<br/>(script: deterministic degraded<br/>selection — wider, never narrower)"]
|
select -. "fallback" .-> dlanes["default_lanes<br/>(script: deterministic degraded<br/>selection — wider, never narrower)"]
|
||||||
dlanes --> build
|
dlanes --> build
|
||||||
build --> mcr["map: code-review"] & madv["map: adversary"] & msec["map: security"] & mpb["map: probe"]
|
build --> mcr["map: code-review"] & madv["map: adversary"] & msec["map: security"] & mpb["map: probe"]
|
||||||
@@ -155,7 +155,7 @@ Plan / acceptance criteria:
|
|||||||
Local-run recipe / usage suites:
|
Local-run recipe / usage suites:
|
||||||
<how to boot the service locally + where existing suites live — enables the probe lane>
|
<how to boot the service locally + where existing suites live — enables the probe lane>
|
||||||
|
|
||||||
Forced lanes: (optional — e.g. 'adversary, probe' to run exactly those)" \
|
Forced lanes: (optional, ADDITIVE — e.g. 'adversary, probe' run on top of the computed selection; forcing never drops the always-on floor)" \
|
||||||
--variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"}
|
--variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"}
|
||||||
```
|
```
|
||||||
|
|
||||||
@@ -210,9 +210,9 @@ verbatim.
|
|||||||
(`'[]'` ⇒ "none declared"); a declaration that is not a JSON array of
|
(`'[]'` ⇒ "none declared"); a declaration that is not a JSON array of
|
||||||
strings executes nothing and records a `PIPELINE-FAULT`, which the
|
strings executes nothing and records a `PIPELINE-FAULT`, which the
|
||||||
adversary lane reports as DIVERGES and the gate as BLOCKED.
|
adversary lane reports as DIVERGES and the gate as BLOCKED.
|
||||||
- Signals failures degrade gracefully: if git can't compute the diff, lane
|
- Signals failures degrade WIDER: if git can't compute the diff, the surface
|
||||||
selection falls back to caller context + forced lanes, and the report notes
|
is unknown — the security lane is added (and probe, when a local-run recipe
|
||||||
it. The auth-path regex deliberately over-fires (`auth(?!or)` matches
|
exists) on top of caller context + forced lanes, and the report notes it. The auth-path regex deliberately over-fires (`auth(?!or)` matches
|
||||||
auth/authn/authz/oauth but not "author") — the safe direction, since signals
|
auth/authn/authz/oauth but not "author") — the safe direction, since signals
|
||||||
only ever ADD the security lane.
|
only ever ADD the security lane.
|
||||||
- Requires the `code-reviewer`, `adversary`, `security-reviewer`, and `probe`
|
- Requires the `code-reviewer`, `adversary`, `security-reviewer`, and `probe`
|
||||||
|
|||||||
@@ -107,10 +107,14 @@ nodes:
|
|||||||
You extract the facts of a review request into structured fields.
|
You extract the facts of a review request into structured fields.
|
||||||
EXTRACTION ONLY — do not decide which review lanes should run.
|
EXTRACTION ONLY — do not decide which review lanes should run.
|
||||||
|
|
||||||
- `diff_spec`: the git diff specification the caller named — a ref
|
- `diff_spec`: the git diff specification the caller named, as a BARE
|
||||||
range ("main...HEAD", "HEAD~3..HEAD"), a single ref, or the literal
|
revision token usable verbatim in `git diff <spec>` — a ref range
|
||||||
"worktree" for uncommitted changes against HEAD. When the caller
|
("main...HEAD", "HEAD~3..HEAD"), a single ref, or the literal
|
||||||
did not name one, use "worktree".
|
"worktree" for uncommitted changes against HEAD. NEVER prose, a
|
||||||
|
command, or a path: from "run get_diff --base origin/main" extract
|
||||||
|
"origin/main...HEAD"; from "diff origin/main...HEAD in ~/code/x"
|
||||||
|
extract "origin/main...HEAD". When the caller did not name one, use
|
||||||
|
"worktree".
|
||||||
- `project_dir_in`: the repository directory the caller named as holding
|
- `project_dir_in`: the repository directory the caller named as holding
|
||||||
the change (absolute path preferred); empty string when the caller
|
the change (absolute path preferred); empty string when the caller
|
||||||
named none.
|
named none.
|
||||||
@@ -127,7 +131,8 @@ nodes:
|
|||||||
pointers for black-box probing (how to boot the service, where the
|
pointers for black-box probing (how to boot the service, where the
|
||||||
suites live). Empty string when the caller supplied none.
|
suites live). Empty string when the caller supplied none.
|
||||||
- `forced_lanes`: lanes the caller explicitly demanded (verbatim
|
- `forced_lanes`: lanes the caller explicitly demanded (verbatim
|
||||||
names). Empty array = automatic selection.
|
names). Empty array = automatic selection. Forcing is ADDITIVE
|
||||||
|
downstream: it widens the computed selection, never replaces it.
|
||||||
- `extra_context`: any remaining caller context reviewers should see,
|
- `extra_context`: any remaining caller context reviewers should see,
|
||||||
condensed.
|
condensed.
|
||||||
prompt: |
|
prompt: |
|
||||||
@@ -261,7 +266,7 @@ nodes:
|
|||||||
build_items:
|
build_items:
|
||||||
id: build_items
|
id: build_items
|
||||||
type: script
|
type: script
|
||||||
description: Deterministic lane builder — hard rules are the floor, LLM flags may only add, forced lanes override exactly. Emits one 0-or-1-item list per lane; an empty list skips that lane's map. On a re-run pass (retry_lanes non-empty) it skips selection and emits items only for those lanes, each carrying its attempt number.
|
description: Deterministic lane builder — hard rules are the floor, LLM flags may only add, forced lanes are ADDITIVE (selection only ever widens; unavailable signals widen it too). Emits one 0-or-1-item list per lane; an empty list skips that lane's map. On a re-run pass (retry_lanes non-empty) it skips selection and emits items only for those lanes, each carrying its attempt number.
|
||||||
script: scripts/build_items.py
|
script: scripts/build_items.py
|
||||||
timeout: 30
|
timeout: 30
|
||||||
next: [map_code_review, map_adversary, map_security, map_probe]
|
next: [map_code_review, map_adversary, map_security, map_probe]
|
||||||
|
|||||||
@@ -1,14 +1,20 @@
|
|||||||
#!/usr/bin/env python3
|
#!/usr/bin/env python3
|
||||||
"""Deterministic lane builder for the review gauntlet.
|
"""Deterministic lane builder for the review gauntlet.
|
||||||
|
|
||||||
Selection = hard rules (the floor) ∪ LLM additions; forced lanes override
|
Selection = hard rules (the floor) ∪ LLM additions ∪ caller-forced lanes.
|
||||||
everything exactly. Emits one 0-or-1-item list per lane: an empty list makes
|
Forcing is ADDITIVE: it can only widen the computed selection — honoring a
|
||||||
that lane's map node run zero branches (map-over-nothing), which IS the
|
forced list as the exact set once dropped the always-on code-review lane in
|
||||||
skip mechanism — no dynamic routing anywhere.
|
a live run. Emits one 0-or-1-item list per lane: an empty list makes that
|
||||||
|
lane's map node run zero branches (map-over-nothing), which IS the skip
|
||||||
|
mechanism — no dynamic routing anywhere.
|
||||||
|
|
||||||
The LLM's add_* flags can only ADD lanes. A flag that is false never
|
The LLM's add_* flags can only ADD lanes. A flag that is false never
|
||||||
removes a lane the hard rules selected.
|
removes a lane the hard rules selected.
|
||||||
|
|
||||||
|
Unavailable diff signals (malformed spec, git failure) mean the surface is
|
||||||
|
UNKNOWN — selection degrades WIDER (security on; probe when a recipe
|
||||||
|
exists), never narrower.
|
||||||
|
|
||||||
A re-run pass (retry_gate emitted `retry_lanes`) skips selection: only the
|
A re-run pass (retry_gate emitted `retry_lanes`) skips selection: only the
|
||||||
named lanes get an item, stamped with their attempt number, and every other
|
named lanes get an item, stamped with their attempt number, and every other
|
||||||
lane maps over nothing — retry_gate restores their earlier reports.
|
lane maps over nothing — retry_gate restores their earlier reports.
|
||||||
@@ -101,10 +107,6 @@ def main():
|
|||||||
(forced.append(canon) if canon else unknown.append(str(lane)))
|
(forced.append(canon) if canon else unknown.append(str(lane)))
|
||||||
|
|
||||||
reasons = []
|
reasons = []
|
||||||
if forced:
|
|
||||||
lanes = set(forced)
|
|
||||||
reasons.append(f"forced lanes honored exactly: {sorted(lanes)}")
|
|
||||||
else:
|
|
||||||
lanes = {"code-review"}
|
lanes = {"code-review"}
|
||||||
reasons.append(
|
reasons.append(
|
||||||
"code-review: always on — the gauntlet is only spawned for non-trivial work"
|
"code-review: always on — the gauntlet is only spawned for non-trivial work"
|
||||||
@@ -128,6 +130,22 @@ def main():
|
|||||||
"probe: consumer-facing surface changed and a local-run recipe/suite pointer exists"
|
"probe: consumer-facing surface changed and a local-run recipe/suite pointer exists"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Unavailable signals (malformed diff spec, git failure) = surface UNKNOWN.
|
||||||
|
# Degrade WIDER, never narrower — a run without signals once silently
|
||||||
|
# dropped the security and probe lanes.
|
||||||
|
if text("signals_summary") == "signals unavailable":
|
||||||
|
if "security" not in lanes:
|
||||||
|
lanes.add("security")
|
||||||
|
reasons.append(
|
||||||
|
"security: diff signals unavailable — surface unknown, degrading wider"
|
||||||
|
)
|
||||||
|
if "probe" not in lanes and text("probe_context"):
|
||||||
|
lanes.add("probe")
|
||||||
|
reasons.append(
|
||||||
|
"probe: diff signals unavailable and a local-run recipe/suite pointer "
|
||||||
|
"exists — degrading wider"
|
||||||
|
)
|
||||||
|
|
||||||
# LLM judgment is additive-only.
|
# LLM judgment is additive-only.
|
||||||
llm_why = text("lane_reasons")[:300]
|
llm_why = text("lane_reasons")[:300]
|
||||||
for flag, lane in (
|
for flag, lane in (
|
||||||
@@ -152,6 +170,28 @@ def main():
|
|||||||
lanes.add(lane)
|
lanes.add(lane)
|
||||||
reasons.append(f"{lane}: added by lane-selection judgment — {llm_why}")
|
reasons.append(f"{lane}: added by lane-selection judgment — {llm_why}")
|
||||||
|
|
||||||
|
# Caller-forced lanes are ADDITIVE: they widen the computed selection,
|
||||||
|
# never replace it. (Honoring them as the exact set dropped code-review —
|
||||||
|
# documented as always-on — and adversary in a live run.) Forcing also
|
||||||
|
# bypasses the context guards above: the caller demanded the lane, so it
|
||||||
|
# runs, with the expectation named in the summary.
|
||||||
|
if forced:
|
||||||
|
newly = sorted(set(forced) - lanes)
|
||||||
|
lanes |= set(forced)
|
||||||
|
reasons.append(
|
||||||
|
f"forced lanes (additive): {sorted(set(forced))}"
|
||||||
|
+ (f" — newly added: {newly}" if newly else " — all already selected")
|
||||||
|
)
|
||||||
|
if "probe" in forced and not text("probe_context"):
|
||||||
|
reasons.append(
|
||||||
|
"probe: forced without a local-run recipe/suite pointer — expect INCONCLUSIVE"
|
||||||
|
)
|
||||||
|
if "adversary" in forced and not text("plan_context"):
|
||||||
|
reasons.append(
|
||||||
|
"adversary: forced without spec/plan context — it fails closed "
|
||||||
|
"(DIVERGES) without extractable criteria"
|
||||||
|
)
|
||||||
|
|
||||||
if unknown:
|
if unknown:
|
||||||
reasons.append(f"ignored unknown forced lane name(s): {unknown}")
|
reasons.append(f"ignored unknown forced lane name(s): {unknown}")
|
||||||
|
|
||||||
|
|||||||
@@ -8,8 +8,10 @@ AND a local-run recipe exists (build_items gates probe the same way — a
|
|||||||
probe without a recipe is INCONCLUSIVE by construction),
|
probe without a recipe is INCONCLUSIVE by construction),
|
||||||
plus security when the attack-surface signals (auth/deps/exec) or a hardened
|
plus security when the attack-surface signals (auth/deps/exec) or a hardened
|
||||||
posture demand it, unioned with any lanes the caller forced (canonicalized
|
posture demand it, unioned with any lanes the caller forced (canonicalized
|
||||||
with the same aliases build_items uses) — which build_items honors exactly,
|
with the same aliases build_items uses) — which build_items honors
|
||||||
so selection degrades WIDER, never narrower. The degradation note goes into
|
ADDITIVELY, so selection degrades WIDER, never narrower. Unavailable diff
|
||||||
|
signals also widen: unknown surface adds security (and probe when a
|
||||||
|
local-run recipe exists). The degradation note goes into
|
||||||
`lanes_degraded`; build_items folds it into lanes_summary so the final
|
`lanes_degraded`; build_items folds it into lanes_summary so the final
|
||||||
report names the degradation.
|
report names the degradation.
|
||||||
|
|
||||||
@@ -72,6 +74,11 @@ def main():
|
|||||||
or state.get("security_posture") == "hardened"
|
or state.get("security_posture") == "hardened"
|
||||||
):
|
):
|
||||||
lanes.add("security")
|
lanes.add("security")
|
||||||
|
# Unavailable signals = surface UNKNOWN → widen (mirrors build_items).
|
||||||
|
if (state.get("signals_summary") or "") == "signals unavailable":
|
||||||
|
lanes.add("security")
|
||||||
|
if isinstance(probe_context, str) and probe_context.strip():
|
||||||
|
lanes.add("probe")
|
||||||
# Caller-forced lanes survive degradation — union, never overwrite.
|
# Caller-forced lanes survive degradation — union, never overwrite.
|
||||||
for lane in state.get("forced_lanes") or []:
|
for lane in state.get("forced_lanes") or []:
|
||||||
if canon := ALIASES.get(str(lane).strip().lower()):
|
if canon := ALIASES.get(str(lane).strip().lower()):
|
||||||
|
|||||||
@@ -3,9 +3,10 @@
|
|||||||
|
|
||||||
Pure git — no LLM judgment. Every signal is computed from the changed-file
|
Pure git — no LLM judgment. Every signal is computed from the changed-file
|
||||||
list and the added lines of the diff. Failures never sink the gauntlet:
|
list and the added lines of the diff. Failures never sink the gauntlet:
|
||||||
on any error the script reports `signals_error` and leaves the defaults
|
on any error the script reports `signals_error` and marks the signals
|
||||||
(false/empty) in place, which biases lane selection toward the caller's
|
unavailable — downstream (build_items / default_lanes) treats that as
|
||||||
context and forced lanes.
|
UNKNOWN surface and degrades WIDER (security on; probe when a recipe
|
||||||
|
exists), on top of caller context and forced lanes.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import json
|
import json
|
||||||
@@ -26,6 +27,19 @@ proj = os.path.expanduser(
|
|||||||
(state.get("project_dir_in") or "").strip() or state.get("project_dir") or "."
|
(state.get("project_dir_in") or "").strip() or state.get("project_dir") or "."
|
||||||
)
|
)
|
||||||
spec = (state.get("diff_spec") or "worktree").strip()
|
spec = (state.get("diff_spec") or "worktree").strip()
|
||||||
|
spec_raw = spec
|
||||||
|
# The parse stage should emit a bare revision token, but callers write prose
|
||||||
|
# and extraction drifts ("run get_diff --base origin/main" once reached
|
||||||
|
# `git diff` verbatim, killing the signals — and with them the security and
|
||||||
|
# probe lanes). Deterministically recover a usable spec before giving up.
|
||||||
|
_TOKEN = r"[\w./~^@{}-]+"
|
||||||
|
if spec not in ("", "worktree") and not re.fullmatch(
|
||||||
|
rf"{_TOKEN}(\.\.\.?{_TOKEN})?", spec
|
||||||
|
):
|
||||||
|
if m := re.search(rf"({_TOKEN}\.\.\.?{_TOKEN})", spec):
|
||||||
|
spec = m.group(1) # a revision range buried in prose
|
||||||
|
elif m := re.search(rf"--base[= ]({_TOKEN})", spec):
|
||||||
|
spec = f"{m.group(1)}...HEAD" # the get_diff idiom
|
||||||
|
|
||||||
|
|
||||||
def git(*args):
|
def git(*args):
|
||||||
@@ -112,13 +126,18 @@ try:
|
|||||||
),
|
),
|
||||||
"docs_only": bool(files)
|
"docs_only": bool(files)
|
||||||
and all(re.search(r"\.(md|rst|txt|adoc)$", f, re.I) for f in files),
|
and all(re.search(r"\.(md|rst|txt|adoc)$", f, re.I) for f in files),
|
||||||
"signals_summary": f"{len(files)} file(s) changed (diff: {spec})",
|
"signals_summary": f"{len(files)} file(s) changed (diff: {spec})"
|
||||||
|
+ (f" — spec normalized from {spec_raw!r}" if spec != spec_raw else ""),
|
||||||
}
|
}
|
||||||
)
|
)
|
||||||
except Exception as e: # noqa: BLE001 — degraded signals must not sink the gauntlet
|
except Exception as e: # noqa: BLE001 — degraded signals must not sink the gauntlet
|
||||||
|
what = f"diff spec {spec!r}" + (
|
||||||
|
f", normalized from {spec_raw!r}" if spec != spec_raw else ""
|
||||||
|
)
|
||||||
out["signals_error"] = (
|
out["signals_error"] = (
|
||||||
f"NOTE: diff signals could not be computed ({e}); "
|
f"NOTE: diff signals could not be computed ({what}: {e}); surface UNKNOWN — "
|
||||||
"lane selection falls back to caller context and forced lanes."
|
"lane selection degrades WIDER (security lane added; probe eligible when a "
|
||||||
|
"local-run recipe exists), plus caller context and forced lanes."
|
||||||
)
|
)
|
||||||
out["signals_summary"] = "signals unavailable"
|
out["signals_summary"] = "signals unavailable"
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user