Skip to content

Commit e27a6e1

Browse files
authored
feat(test-classifier): make OBSERVED the default; add --no-run-suite opt-out (#77)
Running the suite when one exists is the high-signal path, so it is now the default for every run (local, CI, Jenkins) rather than CI-only. Flip the AI_RUN_SUITE default 0 -> 1 in the lib and in the dispatcher's prompt branch. The skill still falls back to INFERRED on its own when there is no suite, the toolchain is missing, or it would apply real infra with no teardown. OBSERVED executes branch code, so add an easy opt-out for untrusted diffs: the new --no-run-suite flag (exports AI_RUN_SUITE=0 before the lib loads) and the existing AI_RUN_SUITE=0 env var. Reframe SKILL.md, SETUP.md, and the local docs around 'OBSERVED by default, opt out for untrusted code'. CI/Jenkins keep their explicit AI_RUN_SUITE=1 (now redundant but self-documenting).
1 parent f7d4fb8 commit e27a6e1

6 files changed

Lines changed: 86 additions & 56 deletions

File tree

testing/classifier/.github/workflows/ai-test-classifier.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ jobs:
169169
AI_REVIEW_TOOL: ${{ vars.AI_REVIEW_TOOL }}
170170
CI: 'true'
171171
# Grant the agent execution so it runs the repo's suite for OBSERVED
172-
# verdicts (safe on the ephemeral runner; read-only by default locally).
172+
# verdicts (safe on the ephemeral runner). OBSERVED is the default;
173+
# set explicitly here so CI's behavior is pinned and self-documenting.
173174
AI_RUN_SUITE: '1'
174175
# Pass through whichever API key matches the chosen tool. Each secret
175176
# is set only if the team uses that tool — unused ones stay blank.

testing/classifier/.skills/_lib/ai-classifier-dispatch.sh

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,18 @@ fi
5353
_AI_CLASSIFIER_DISPATCH_LOADED=1
5454

5555
# ── Suite-running mode ──────────────────────────────────────────────────────
56-
# By default the classifier is read-only: it triages whatever failing-test
57-
# signal already exists and never executes the repo's suite. When AI_RUN_SUITE=1
58-
# (the CI workflows set it) the agent is granted execution so it can locate,
59-
# install, and RUN the repo's tests, then classify the OBSERVED failures. This
60-
# gate keeps a local Path-B run safe — a developer's machine won't auto-install
61-
# deps or run tests unless they opt in with AI_RUN_SUITE=1.
62-
AI_RUN_SUITE="${AI_RUN_SUITE:-0}"
56+
# By default the classifier is OBSERVED: the agent locates, installs, and RUNS
57+
# the repo's suite, then classifies the failures it actually observes. Running
58+
# the suite when one exists is the high-signal path, so it is the default for
59+
# every run (local, CI, Jenkins). The skill still falls back to INFERRED on its
60+
# own when there is no suite to run, the toolchain is missing, or it would apply
61+
# real infrastructure with no guaranteed teardown.
62+
#
63+
# OBSERVED executes the change's code on the current machine. That is fine on a
64+
# branch you trust (your own work, CI's checkout). To triage an UNTRUSTED diff
65+
# without executing it, opt out with AI_RUN_SUITE=0 (or --no-run-suite) for a
66+
# read-only, INFERRED-only pass.
67+
AI_RUN_SUITE="${AI_RUN_SUITE:-1}"
6368

6469
# Bounds for the agent loop when it is running the suite, so a runaway
6570
# install/test cycle can't hang the job. The timeout wraps the whole CLI call;

testing/classifier/.skills/test-classifier/SKILL.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,12 @@ out of current scope.
110110
The classifier needs to know *what failed and why it says it failed*. There are
111111
two ways to get that signal, and which one you use determines the result `mode`:
112112

113-
### OBSERVED — run the suite yourself (when `AI_RUN_SUITE=1`)
113+
### OBSERVED — run the suite yourself (the default, unless `AI_RUN_SUITE=0`)
114114

115-
When the environment variable `AI_RUN_SUITE=1` is set (the CI workflows set it,
116-
and you have been granted shell execution), **locate and run the repo's test
117-
suite yourself**, then classify the failures you actually observe:
115+
This is the default mode. Unless `AI_RUN_SUITE=0` is set (the `--no-run-suite`
116+
opt-out, for triaging an untrusted diff without executing it), you have been
117+
granted shell execution: **locate and run the repo's test suite yourself**, then
118+
classify the failures you actually observe:
118119

119120
1. **Locate the test command.** Inspect the checked-out repo: `package.json`
120121
`scripts.test`, a `Makefile` `test:` target, `pytest.ini`/`tox.ini`/
@@ -165,17 +166,18 @@ suite yourself**, then classify the failures you actually observe:
165166
166167
### INFERRED — predict from the diff (fallback)
167168

168-
If `AI_RUN_SUITE` is not set, **or** you cannot locate / install / run the suite
169-
(no test command found, missing toolchain, the suite needs services like a
170-
database, it times out, **or it would apply real infrastructure with no
171-
guaranteed teardown** — see the IaC ladder above), do **not** fabricate a run.
169+
If `AI_RUN_SUITE=0` is set (the explicit `--no-run-suite` opt-out), **or** you
170+
cannot locate / install / run the suite (no test command found, missing
171+
toolchain, the suite needs services like a database, it times out, **or it would
172+
apply real infrastructure with no guaranteed teardown** — see the IaC ladder
173+
above), do **not** fabricate a run.
172174
Instead reason
173175
statically over the diff (Step 2) and PREDICT which tests the change would cause
174176
to fail and why. Then:
175177

176178
- Set the result `mode` to `"INFERRED"`.
177179
- State the reason you fell back in the `summary` (e.g. "no test script found",
178-
"deps failed to install", "suite timed out", "AI_RUN_SUITE not set") so the
180+
"deps failed to install", "suite timed out", "AI_RUN_SUITE=0 set") so the
179181
reviewer sees why this is a prediction, not an observation.
180182
- Prefer lower confidence, and remember `FLAKY_FAILURE`/`ENVIRONMENT_ISSUE` are
181183
generally NOT determinable from a diff alone — only assert them with explicit

testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ SKILL_PATH_CANONICAL="${SKILLS_ROOT}/test-classifier/SKILL.md"
9797
# and skips the prompt + row.
9898
# --gate Exit 1 if the result is CLASSIFIED (CI-blocking mode)
9999
# --json-only Print only the JSON block (machine consumption)
100+
# --no-run-suite Read-only INFERRED pass: do NOT run the repo's suite
101+
# (predict from the diff). The default is OBSERVED (run
102+
# the suite); use this to triage an UNTRUSTED diff
103+
# without executing its code. Same as AI_RUN_SUITE=0.
100104
#
101105
# All other flags (--dry-run, --no-block, --against, --unpushed) fall through to
102106
# the lib. --unpushed classifies the local committed+staged diff with NO PR —
@@ -142,6 +146,12 @@ while [[ $# -gt 0 ]]; do
142146
POST_COMMENT=1
143147
shift
144148
;;
149+
--no-run-suite)
150+
# Opt out of the OBSERVED default: read-only, predict from the diff
151+
# (INFERRED). Exported so the sourced lib and the agent see it.
152+
export AI_RUN_SUITE=0
153+
shift
154+
;;
145155
--gate)
146156
GATE_MODE=1
147157
shift
@@ -370,10 +380,12 @@ ai_review::discover_pr_context() {
370380
# We instruct the AI to emit BOTH a human-readable report AND a fenced JSON
371381
# block. The dispatcher extracts the JSON block to render the PR comment.
372382
#
373-
# Step 1 (the failing-test signal) is mode-dependent: with AI_RUN_SUITE=1 the
374-
# agent has shell execution and must locate + run the suite (OBSERVED); otherwise
375-
# it predicts from the diff (INFERRED). See SKILL.md Step 1 for the full procedure.
376-
if [[ "${AI_RUN_SUITE:-0}" == "1" ]]; then
383+
# Step 1 (the failing-test signal) is mode-dependent: with AI_RUN_SUITE=1 (the
384+
# default) the agent has shell execution and must locate + run the suite
385+
# (OBSERVED); with AI_RUN_SUITE=0 (--no-run-suite) it predicts from the diff
386+
# (INFERRED). See SKILL.md Step 1 for the full procedure. This default must match
387+
# the lib's (ai-classifier-dispatch.sh) — both default to 1 (OBSERVED).
388+
if [[ "${AI_RUN_SUITE:-1}" == "1" ]]; then
377389
read -r -d '' SIGNAL_STEP <<'SIGNAL' || true
378390
1. Collect the failing-test signal in OBSERVED mode (AI_RUN_SUITE=1 — you have
379391
shell execution): LOCATE this repo's test command (package.json scripts,
@@ -497,14 +509,18 @@ test-classifier options (in addition to the shared options above):
497509
"Was this helpful?" and appends one Testing Events row.
498510
--gate Exit 1 when the result is CLASSIFIED (CI-blocking mode).
499511
--json-only Print only the machine-readable JSON block.
512+
--no-run-suite Read-only INFERRED pass: predict from the diff instead of
513+
running the suite (the default). Use it on an UNTRUSTED
514+
diff you don't want to execute. Same as AI_RUN_SUITE=0.
500515
501516
Defaults when no diff-range/PR selector is given:
502517
(no arguments) → --unpushed (committed + staged; local, report-only).
503518
a bare ref → --against <ref>, e.g. test-classifier origin/main
504519
505520
Environment:
506-
AI_RUN_SUITE=1 Run the suite (OBSERVED) instead of inferring from the
507-
diff. Prefix it: AI_RUN_SUITE=1 test-classifier --pr 42
521+
AI_RUN_SUITE Run the suite (OBSERVED, default) vs. infer from the diff.
522+
OBSERVED is the default; set AI_RUN_SUITE=0 (or pass
523+
--no-run-suite) for a read-only INFERRED pass.
508524
METRICSAI_WEBHOOK_URL / METRICSAI_WEBHOOK_KEY
509525
Required only by --submit to append the Testing Events row.
510526
HELP_ADDENDUM

testing/classifier/docs/LOCAL_TEST_CLASSIFIER.md

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,25 @@ and uses the same `--unpushed` scope rule. This is the ergonomics layer over
2727
fast feedback before pushing; it never blocks anything. By default it's
2828
**report-only** (prints to your terminal, posts nothing) — but you can opt in
2929
to posting against a real PR with `--pr N --post-comment` (see Usage).
30-
- **OBSERVED on demand.** By default a local run is INFERRED (it predicts from
31-
the diff — read-only, never touches your machine's toolchain). Opt into
32-
OBSERVED with `AI_RUN_SUITE=1` to have the agent actually locate, install, and
33-
run your suite and triage the real failures — the same behavior CI uses.
30+
- **OBSERVED by default.** When your repo has a suite, a local run is OBSERVED:
31+
the agent locates, installs, and runs your suite and triages the *real*
32+
failures — the same behavior CI uses, and the high-signal path. It falls back
33+
to INFERRED on its own when there's no suite to run. To force a read-only
34+
INFERRED pass (predict from the diff, never touch your toolchain), pass
35+
`--no-run-suite` or set `AI_RUN_SUITE=0`.
3436

3537
> [!WARNING]
36-
> **OBSERVED (`AI_RUN_SUITE=1`) runs the change's code on your machine — there
37-
> is no sandbox.** The agent installs dependencies (running each package's
38+
> **OBSERVED (the default) runs the change's code on your machine — there is no
39+
> sandbox.** The agent installs dependencies (running each package's
3840
> install/postinstall scripts) and executes the test suite **directly in your
3941
> checkout**, with your shell, your environment, and your credentials. It also
4042
> leaves build artifacts behind (`node_modules/`, downloaded browsers, etc.).
4143
>
42-
> Only use `AI_RUN_SUITE=1` on a branch **you trust** — typically your own
43-
> work-in-progress. Do **not** point it at an untrusted or unreviewed PR
44-
> (e.g. `--pr N` for someone else's contribution): installing and running that
45-
> branch's code is arbitrary code execution on your laptop. For untrusted
46-
> changes, use the default **INFERRED** mode (omit `AI_RUN_SUITE`), which only
44+
> This is fine on a branch **you trust** — typically your own work-in-progress.
45+
> Do **not** run it against an untrusted or unreviewed PR (e.g. `--pr N` for
46+
> someone else's contribution): installing and running that branch's code is
47+
> arbitrary code execution on your laptop. For untrusted changes, opt out with
48+
> `--no-run-suite` (or `AI_RUN_SUITE=0`) for the **INFERRED** mode, which only
4749
> reasons over the diff and never executes anything, or let CI run OBSERVED in
4850
> its disposable runner.
4951
@@ -102,24 +104,25 @@ It forwards args verbatim, and the dispatcher applies these defaults:
102104
(`--dry-run`, `--json-only`, …). You can combine a ref with flags too:
103105
`test-classifier origin/main --post-comment`.
104106

105-
Prefix `AI_RUN_SUITE=1` to run the suite locally (OBSERVED) instead of inferring
106-
from the diff: `AI_RUN_SUITE=1 test-classifier`.
107+
By default a local run is OBSERVED — it runs your suite and triages the real
108+
failures. Pass `--no-run-suite` (or `AI_RUN_SUITE=0`) for a read-only INFERRED
109+
pass that only reasons over the diff: `test-classifier --no-run-suite`.
107110

108111
---
109112

110113
## Usage
111114

112115
```bash
113-
test-classifier # everything unpushed: committed + staged (INFERRED)
116+
test-classifier # everything unpushed: runs the suite, triages REAL failures (OBSERVED)
114117
test-classifier origin/main # the committed range origin/main..HEAD
115118
test-classifier HEAD~1 # just the last commit
116119

117-
AI_RUN_SUITE=1 test-classifier # run the suite locally and triage REAL failures (OBSERVED)
120+
test-classifier --no-run-suite # read-only: predict from the diff, don't run the suite (INFERRED)
118121

119122
# Post the result as a PR comment (needs an open PR + gh authed):
120123
test-classifier --post-comment # auto-discovers the current branch's PR
121124
test-classifier --pr 42 --post-comment # explicit PR number
122-
AI_RUN_SUITE=1 test-classifier --pr 42 --post-comment # OBSERVED + post
125+
test-classifier --pr 42 --no-run-suite --post-comment # INFERRED + post
123126

124127
# Streamlined: post AND record the metric in one shot (see --submit below):
125128
test-classifier --pr 42 --submit
@@ -151,8 +154,8 @@ Events** tab — verdict, file context, confidence, and your 👍/👎 (with an
151154
one-line reason on a 👎). No GitHub round-trip, no separate harvest.
152155

153156
```bash
154-
test-classifier --pr 42 --submit # classify (INFERRED) → post → ask → record
155-
AI_RUN_SUITE=1 test-classifier --pr 42 --submit # OBSERVED → post → ask → record
157+
test-classifier --pr 42 --submit # classify (OBSERVED) → post → ask → record
158+
test-classifier --pr 42 --no-run-suite --submit # INFERRED → post → ask → record
156159
test-classifier --submit # auto-discovers the current branch's PR
157160
```
158161

@@ -200,11 +203,11 @@ The row lands in the **Testing Events** tab by default; override with
200203
`→ Fix the CODE`, `→ Re-run / deflake`, `→ Fix the ENV`). That's the punchline:
201204
the classifier is **diagnostic** — it tells you which side to fix, never writes
202205
the patch. Read the summary, then make the change yourself.
203-
- **INFERRED vs OBSERVED.** Without `AI_RUN_SUITE=1` the agent predicts failures
204-
from the diff (INFERRED) and never touches your toolchain. With it, the agent
205-
runs your suite (OBSERVED) — the only mode in which `FLAKY_FAILURE` /
206-
`ENVIRONMENT_ISSUE` are reliably reachable, since you can't see a timeout or
207-
non-determinism from a diff.
206+
- **OBSERVED vs INFERRED.** By default the agent runs your suite (OBSERVED) —
207+
the only mode in which `FLAKY_FAILURE` / `ENVIRONMENT_ISSUE` are reliably
208+
reachable, since you can't see a timeout or non-determinism from a diff. With
209+
`--no-run-suite` (`AI_RUN_SUITE=0`) it predicts failures from the diff
210+
(INFERRED) and never touches your toolchain.
208211
- **Live progress.** On a local interactive run the dispatcher streams the
209212
agent's steps (each `` reasoning line and `` tool call) to your terminal as
210213
they happen, so it isn't a blinking cursor while it works. The final report is
@@ -240,7 +243,7 @@ run (Path B) is still the recorded, metrics-feeding pass; this is your preview.
240243
| `--unpushed: couldn't determine what's been pushed` | No upstream and no remote default branch (e.g. brand-new branch, no remote). Pass an explicit base: `test-classifier origin/main`. |
241244
| `AI_REVIEW_TOOL … not set` | Export `AI_REVIEW_TOOL=claude` (or `codex`/`copilot`); see `README.md`. |
242245
| Classification seems to miss recent edits | `--unpushed` excludes *unstaged* changes. `git add` or commit them first. |
243-
| Everything lands in INFERRED | That's the default. Add `AI_RUN_SUITE=1` to run the suite (OBSERVED). Suites needing services may still fall back to INFERRED, by design. |
246+
| Expected OBSERVED but got INFERRED | OBSERVED is the default, but the agent falls back to INFERRED when it can't run the suite — no test command found, missing toolchain, suite needs services (DB), it times out, or it would apply real infra with no teardown. Check the `summary`, which states the reason. (And confirm you didn't pass `--no-run-suite` / set `AI_RUN_SUITE=0`.) |
244247
| Posted to / recorded the wrong repo (e.g. the upstream of your fork) | The repo is resolved from your `origin` remote. On a fork whose `origin` isn't the repo the PR lives in, export `AI_REVIEW_REPO=owner/name` to set it explicitly. |
245248

246249
---

testing/classifier/docs/SETUP.md

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,16 +123,17 @@ testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh
123123
testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh \
124124
--dry-run
125125

126-
# Opt in to running the suite locally (OBSERVED). OFF by default for the local
127-
# path so a run never auto-installs deps or executes tests on your machine:
128-
AI_RUN_SUITE=1 \
126+
# Read-only INFERRED pass — predict from the diff, don't run the suite. Use on
127+
# an untrusted diff you don't want to install deps for / execute on your machine:
129128
testing/classifier/.skills/test-classifier/scripts/test-classifier-dispatcher.sh \
130-
--post-comment
129+
--no-run-suite --post-comment
131130
```
132131

133-
By default a **local** run is read-only and INFERRED (predicts from the diff) —
134-
it will not install deps or run your suite. Set `AI_RUN_SUITE=1` to let the agent
135-
run the suite locally (the same OBSERVED behavior CI uses). CI sets this for you.
132+
By default a run is OBSERVED (the same behavior CI uses): when the repo has a
133+
suite, the agent installs deps and runs it to triage the real failures. Pass
134+
`--no-run-suite` (or set `AI_RUN_SUITE=0`) for a read-only INFERRED pass that
135+
predicts from the diff and never touches your toolchain — use it on a branch you
136+
don't trust enough to execute.
136137

137138
By default the dispatcher prints the classification report to the terminal
138139
only. Add `--post-comment` to also post the PR comment.
@@ -256,8 +257,10 @@ The full report is also uploaded as a CI artifact, and the run is non-blocking.
256257

257258
#### OBSERVED vs INFERRED — what the classifier actually sees
258259

259-
In CI the workflow sets `AI_RUN_SUITE=1`, which grants the agent shell execution
260-
so it **runs your suite itself**: it locates your test command (package.json,
260+
OBSERVED is the default everywhere; the CI workflow still sets `AI_RUN_SUITE=1`
261+
explicitly (self-documenting, and pins it regardless of the default). That grants
262+
the agent shell execution so it **runs your suite itself**: it locates your test
263+
command (package.json,
261264
Makefile, pytest/tox, go.mod, Cargo.toml, or your CI's test step), installs deps
262265
from your lockfile best-effort, runs the tests, and triages the **real**
263266
failures. That comment is marked **Observed**.

0 commit comments

Comments
 (0)