Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion skills/ce-code-review/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ echo "$RUN_DIR";
When adversarial was selected and scope is `local-aligned` or standalone, read `references/cross-model-review.md` from this skill's directory in full, attest the host, resolve and sanction one fixed route, and make its required egress announcement. Before start, write the reference's compact orchestrator-owned adversarial review brief to the run directory: intent plus the material risk divisions inferred from the current file inventory and diff, without embedding the diff or mechanically copying every path. Then start the detached peer job using the reference's exact invocation and persist its job ID, target, requested model/reasoning, and start epoch in working state.

- If the runner returns a job ID, the peer owns the adversarial lens for this run. Remove `adversarial-reviewer` from the local roster immediately. Do not read its local persona asset or dispatch it later, even if the peer eventually fails.
- If no job starts because of a dispatch-infrastructure failure (a non-zero exit before any job id, an unresolved `$SKILL_DIR`/script path), first attempt the bounded same-route hand recovery from `references/cross-model-review.md` before accepting the fallback: re-run the identical resolved route, holding target/model and read scope fixed, while each failure is a new plausibly recoverable one and the shared peer deadline holds. If recovery returns a job id, treat it as the branch above (the peer owns the lens; remove `adversarial-reviewer`). Only when recovery is exhausted — a failure repeats or the deadline is spent — or the peer was never eligible to start (gate not met, host un-attestable, no different provider, CLI missing/unauthed), keep `adversarial-reviewer` in the local roster as the fallback and record the peer skip reason for Coverage.
- If no job starts because of a dispatch-infrastructure failure (a non-zero exit before any job id, an unresolved `$SKILL_DIR`/script path), first attempt the bounded same-route hand recovery from `references/cross-model-review.md` before accepting the fallback: re-run the identical resolved route, holding target/model and read scope fixed, while each failure is a new plausibly recoverable one and the shared peer deadline holds. If recovery returns a job id, treat it as the branch above (the peer owns the lens; remove `adversarial-reviewer`). Only when recovery is exhausted — a failure repeats or the deadline is spent — or the peer was never eligible to start (gate not met, host un-attestable, no different provider, CLI missing/unauthed, or a known session-quota circuit remains open), keep `adversarial-reviewer` in the local roster as the fallback and record the peer skip reason for Coverage.
- In `pr-remote` / `branch-remote`, do not start the peer; keep the selected in-process adversarial reviewer because it can inspect the reviewed refs.

When a job ID is returned and task tracking is active, add a distinct task that names the independent cross-model adversarial review. Keep it in progress while the detached job runs, then record its terminal outcome when the artifact is collected. Never create this task before a peer starts or leave it behind when the local adversarial fallback runs.
Expand Down
9 changes: 6 additions & 3 deletions skills/ce-code-review/references/cross-model-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Resolve the preference in this order:
3. A preference already in your **project instructions** (the active instructions in your context) — consumed from context, **never** read from a named file.
4. **Default:** first available attested-different target in `codex → claude → grok → composer`; Cursor-default participates only when explicitly preferred.

Before egress, resolve the target to one concrete installed route, verify every recipient against `CROSS_MODEL_PEERS`, announce it, and pass it as `CROSS_MODEL_FIXED_ROUTE`. `CROSS_MODEL_FIXED_ROUTE` accepts exactly these tokens — the worker fail-closes on anything else (including route-shaped guesses like `codex-cli`):
Before egress, resolve the target to one concrete installed route, verify every recipient against `CROSS_MODEL_PEERS`, announce it, and pass it as `CROSS_MODEL_FIXED_ROUTE`. Availability includes the executable, a route-specific authentication preflight where the CLI exposes one, and the private reset-aware route-health circuit. A known-exhausted route is excluded before the reviewed diff is packaged; an explicitly fixed route that becomes unavailable fails clearly and never silently switches recipients. `CROSS_MODEL_FIXED_ROUTE` accepts exactly these tokens — the worker fail-closes on anything else (including route-shaped guesses like `codex-cli`):

| Target | Route token(s) |
|--------|----------------|
Expand All @@ -43,7 +43,7 @@ Before egress, resolve the target to one concrete installed route, verify every
| `cursor` | `cursor` |
| `composer` | `composer` |

A failed route returns no artifact and never changes provider or intermediary internally. A retry is a new disclosed and sanctioned dispatch. For backward compatibility, either `cursor` or `composer` in `CROSS_MODEL_PEERS` sanctions Cursor as an intermediary, but selecting Cursor-default requires target `cursor`; `grok` alone never sanctions Grok-via-Cursor.
A failed route returns no artifact and never changes provider or intermediary internally. A retry is a new disclosed and sanctioned dispatch. Claude exposes authentication state but no zero-cost quota-remaining status, so the first newly exhausted session can be learned only from an API-bearing request. When that request returns session-quota evidence, the worker persists only the route, failure class, observation epoch, and reset epoch under the private peer-state root. Later invocations exclude that route before prompt packaging, re-check the circuit immediately before egress, and retry only after the recorded reset. A plain 429 remains a transient rate limit; execution-context authentication and network-transport failures are classified separately and do not open the session-quota circuit. For backward compatibility, either `cursor` or `composer` in `CROSS_MODEL_PEERS` sanctions Cursor as an intermediary, but selecting Cursor-default requires target `cursor`; `grok` alone never sanctions Grok-via-Cursor.

`CROSS_MODEL_PEERS` is an optional restriction: when unset, it leaves the resolved route unfiltered and this skill invocation plus the concrete pre-egress disclosure sanctions that route; when set, the selected target/intermediary must appear. Use this contract directly. Do not inspect the worker source to rediscover its allowlist behavior.

Expand Down Expand Up @@ -94,10 +94,13 @@ PY="$(for c in python3 python py; do command -v "$c" >/dev/null 2>&1 && "$c" -c
```bash
SKILL_DIR="<absolute path of the directory containing the ce-code-review SKILL.md you read>";
PY="$(for c in python3 python py; do command -v "$c" >/dev/null 2>&1 && "$c" -c '' >/dev/null 2>&1 && { echo "$c"; break; }; done)"; [ -n "$PY" ] || { echo "no working Python 3 interpreter on PATH" >&2; exit 1; };
"$PY" "$SKILL_DIR/scripts/peer-route-health.py" preflight --route "<fixed-route>" >/dev/null || { echo "peer-route-unavailable=<fixed-route>" >&2; exit 3; };
echo "peer-deadline-secs=$(( ${CROSS_MODEL_HARD_SECS:-1200} + 10 ))";
CE_PEER_HARD_SECS= CROSS_MODEL_HOST_HARNESS="<host-harness>" CROSS_MODEL_FIXED_ROUTE="<fixed-route>" "$PY" "$SKILL_DIR/scripts/peer-job-runner.py" start --skill ce-code-review --run-id "<run-id>" --label adversarial -- env CROSS_MODEL_HOST_HARNESS="<host-harness>" CROSS_MODEL_FIXED_ROUTE="<fixed-route>" bash "$SKILL_DIR/scripts/cross-model-adversarial-review.sh" "<host-serving-family>" "<target>" "<base-ref>" "<run-dir>"
```

The preflight and runner start stay in the same shell call. A failed preflight returns before any job ID, review-prompt packaging, or egress, so Stage 3d keeps the in-process `adversarial-reviewer` fallback. The worker repeats the preflight immediately before egress to close the race where another review learns a quota reset after this outer check.

The nested windows are one budget with one knob, `CROSS_MODEL_HARD_SECS`. The runner derives its supervisor hard window from that ambient knob automatically (`max(1230, knob + 30)`). Clear `CE_PEER_HARD_SECS` on the start prefix (`CE_PEER_HARD_SECS=`) so a stale ambient value from an earlier session or harness export cannot undercut that derivation — an explicit numeric `CE_PEER_HARD_SECS` still wins when a skill deliberately sets one (ce-work / elevation), which this path must not do. Print the orchestrator deadline as `knob + 10` in the same shell as `start` (as above) and use that printed `peer-deadline-secs=<n>` below; never hardcode it, because a literal survives a knob change and then reaps a healthy peer.

**Do not forward `CROSS_MODEL_HARD_SECS` to the worker.** The runner already passes the ambient environment through, so a knob the user actually set reaches the worker on its own. Re-exporting the orchestrator's *resolved* value would convert a fallback into an explicit override and destroy the one distinction the worker still needs: idle-guarded routes (codex + streaming claude/cursor-family) use the raised `HARD_SECS` default, while `grok-cli` keeps the lower `UNGUARDED_HARD_SECS` bound because its `--json-schema` path cannot stream. Forcing one value would silently restore the doubled hang on that hard-only route.
Expand Down Expand Up @@ -149,7 +152,7 @@ Repeat that call until the job is terminal or the derived deadline is spent; do
- **Ran but produced no usable output** — the job reached `done` (or any terminal state) yet no `adversarial-<provider>.json` exists (the peer ran and egressed but returned nothing schema-shaped — unparseable output, empty findings the script dropped). Distinct from not-run: note "cross-model pass: peer ran, no usable output" in human-facing markdown Coverage. Never fail the review.
- **Started but not `done`** — the final status read reports `failed`, `timeout`, or `died-without-result` (a job reaped at the shared deadline records `timeout`, with the reap noted in its reason) → still non-blocking, but never silent: name the peer and its terminal state in Coverage (e.g. "cross-model adversarial peer: timeout"). Silent absence stays correct only for passes that never started or were skipped.
- Empty `findings` → note "cross-model pass: no additional issues" in Coverage.
- **Classify the skip reason before deleting.** Read `out.log` before cleanup, including bounded lines prefixed `peer skip evidence:`, and name observed quota, authentication, or capability failure. An authentication-shaped peer failure (`not logged in`, `please log in`, 401, or CLI text prompting login) describes only the peer's execution context: a sandboxed host — e.g. a restricted Codex task denying spawned commands network or keychain access — produces the identical signal to a genuine account logout, so classify it as a cross-model execution-context authentication failure and never report it as the user's account being logged out or prompt the user to run a login command on that basis. The cross-model pass is additive and the local review still completed; obtaining it requires a context where the peer CLI can reach the network (for example, outside the restricted sandbox). After the same quota or usage-limit evidence appears more than once in this session, do not retry that route automatically. A retry uses a newly resolved, disclosed, and sanctioned fixed route; never silently continue to another recipient.
- **Classify the skip reason before deleting.** Read `out.log` before cleanup, including bounded lines prefixed `peer skip evidence:`, and name observed quota, authentication, or capability failure. An authentication-shaped peer failure (`not logged in`, `please log in`, 401, or CLI text prompting login) describes only the peer's execution context: a sandboxed host — e.g. a restricted Codex task denying spawned commands network or keychain access — produces the identical signal to a genuine account logout, so classify it as a cross-model execution-context authentication failure and never report it as the user's account being logged out or prompt the user to run a login command on that basis. The cross-model pass is additive and the local review still completed; obtaining it requires a context where the peer CLI can reach the network (for example, outside the restricted sandbox). Session-quota evidence opens the private reset-aware circuit on its first observation; do not retry that route before its recorded reset. A retry through another route requires a newly resolved, disclosed, and sanctioned fixed recipient; never silently continue to another provider.
- After fold-in (or after deadline reaping), delete the consumed job directory (`<run-dir>/jobs/<job-id>/`) — its log and result are review content and must not outlive their use.
- A finding sharing a fingerprint with in-process `adversarial` promotes only when the artifact records `independence_verified: true`. Cursor-default artifacts default false; an unattested host skips automatic dispatch.

Expand Down
41 changes: 40 additions & 1 deletion skills/ce-code-review/scripts/cross-model-adversarial-review.sh
Original file line number Diff line number Diff line change
Expand Up @@ -311,11 +311,46 @@ esac
SKILL_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" || skip "cannot resolve skill root; skipping"
PERSONA="$SKILL_ROOT/references/personas/adversarial-reviewer.md"
SCHEMA="$SKILL_ROOT/references/findings-schema.json"
ROUTE_HEALTH="$SKILL_ROOT/scripts/peer-route-health.py"
[ -f "$PERSONA" ] || skip "persona brief not found at $PERSONA; skipping"
[ -f "$SCHEMA" ] || skip "findings schema not found at $SCHEMA; skipping"
[ -f "$ROUTE_HEALTH" ] || skip "route-health helper not found at $ROUTE_HEALTH; skipping"
SCHEMA_CONTENT="$(cat "$SCHEMA")" || skip "cannot read findings schema; skipping"
SCHEMA_REF="$SCHEMA_CONTENT"

PY="$(for c in python3 python py; do command -v "$c" >/dev/null 2>&1 && "$c" -c '' >/dev/null 2>&1 && { echo "$c"; break; }; done)"
[ -n "$PY" ] || skip "no working Python 3 interpreter on PATH; skipping"

route_preflight_available() {
local route="$1" status
$PY "$ROUTE_HEALTH" preflight --route "$route" >/dev/null
status=$?
if [ "$status" -eq 0 ]; then
return 0
fi
if [ "$status" -eq 3 ]; then
log "route '$route' is unavailable until its recorded session-quota reset; skipping before review payload packaging"
else
log "route '$route' health or authentication state could not be verified; skipping before review payload packaging"
fi
return 1
}

record_route_failure() {
local route="$1" receipt class opened
shift
[ "$route" = "claude" ] || return 0
receipt="$($PY "$ROUTE_HEALTH" record --route "$route" \
--evidence-file "$1" --evidence-file "$2" 2>/dev/null)" || {
log "route '$route' failure class could not be persisted"
return 0
}
class="$(printf '%s' "$receipt" | jq -r '.failure_class // "other"' 2>/dev/null)"
opened="$(printf '%s' "$receipt" | jq -r '.circuit_opened // false' 2>/dev/null)"
log "route '$route' failure class: $class"
[ "$opened" = true ] && log "route '$route' session-quota circuit opened until its recorded reset"
}

# --- derive repo root (read-only in-tree review) ---------------------------
REPO_ROOT="$(git rev-parse --show-toplevel 2>/dev/null)" || skip "not inside a git repository; skipping"
PEER_WORKDIR="$REPO_ROOT"
Expand All @@ -341,7 +376,7 @@ cursor_egress_ok() { [ -z "$ALLOW" ] || in_csv cursor "$ALLOW" || in_csv compose
provider_available() {
case "$1" in
codex) command -v codex >/dev/null 2>&1 ;;
claude) command -v claude >/dev/null 2>&1 ;;
claude) command -v claude >/dev/null 2>&1 && route_preflight_available claude ;;
grok) command -v grok >/dev/null 2>&1 || { cursor_egress_ok && command -v cursor-agent >/dev/null 2>&1; } ;;
cursor) command -v cursor-agent >/dev/null 2>&1 ;;
composer) command -v cursor-agent >/dev/null 2>&1 ;;
Expand Down Expand Up @@ -855,6 +890,9 @@ run_provider() {
fi
primary="$fixed"
validate_model_override "$primary" || { log "model override '${CROSS_MODEL_MODEL_OVERRIDE:-}' not compatible with route '$primary'; skipping"; rm -f "$OUT"; return 0; }
# A concurrent review may have learned that this route is exhausted after
# discovery. Re-check immediately before prompt composition / provider egress.
route_preflight_available "$primary" || { rm -f "$OUT"; return 0; }
ACTUAL_ROUTE="$primary"
attempt_route "$provider" "$primary"

Expand Down Expand Up @@ -908,6 +946,7 @@ run_provider() {
log "wrote $n finding(s) to $OUT (reviewer adversarial-$provider)"
else
log "provider $provider produced no usable schema-shaped output; skipping fold-in"
record_route_failure "$ACTUAL_ROUTE" "$PEERLOG" "$PEERERR"
# Surface bounded peer output so the orchestrator can
# reason about WHY it was skipped (quota/usage-limit exhaustion vs an ordinary
# empty review) and, in a repeated-pass session, deprioritize an exhausted
Expand Down
Loading