diff --git a/CLAUDE.md b/CLAUDE.md index d81d0ee..0426688 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -41,9 +41,12 @@ modes, `FAKE_COUNT_ALL` to count invocations). ## Changing a schema -Edit `schemas/*.v1.json`, then add/adjust a **positive** example in `schemas/examples/` **and** a -**negative** fixture in `schemas/examples/invalid/` that proves the new invariant fails. -`validate_schemas.py` discovers both by filename suffix. +Edit `schemas/*.v1.json` in place for an additive or invariant-preserving change, then add/adjust +a **positive** example in `schemas/examples/` **and** a **negative** fixture in +`schemas/examples/invalid/` that proves the new invariant fails. `validate_schemas.py` discovers +both by filename suffix. Bump to a new version file (`*.v2.json`) only for a **breaking** change — +one that would invalidate existing stored records, or make new output that current validators reject +— never for an in-place additive edit. ## Layout @@ -51,8 +54,10 @@ Edit `schemas/*.v1.json`, then add/adjust a **positive** example in `schemas/exa `impasse_consent` (consent store), `impasse_run` (process supervisor + `review()`), `impasse_report`. - `schemas/` — `reviewer-response` + `reconciliation-result` + `examples/` (+ `invalid/`). - `docs/` — `protocol`, `security-model`, `environments`, `backends/{codex,claude}`, proposals. -- Independence ladder: cross-provider (Codex) > same-provider (`claude -p`) > self-review - (sandbox/Cowork only, refused for code). Model choice: `--model` / `IMPASSE_{CODEX,CLAUDE}_MODEL`. + Project vocabulary: `docs/glossary.md`. +- Independence backends, strongest first: cross-provider (Codex) > same-provider (`claude -p`) > + self-review (sandbox/Cowork only, refused for code). (The full tier enum also has `undetermined` + for an unattributable host — see `docs/glossary.md`.) Model choice: `--model` / `IMPASSE_{CODEX,CLAUDE}_MODEL`. ## Never commit diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 388a706..52c7293 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -7,24 +7,31 @@ an issue to discuss before a large change. - **Keep the shipped helpers stdlib-only.** No runtime pip dependencies in `scripts/`. `jsonschema` is a dev/CI dependency (used by `tests/validate_schemas.py`) — that's the line. -- **Schemas are a contract.** Changes go through a new version file (`*.v2.json`), not an - edit that breaks existing validators. Add example fixtures under `schemas/examples/` and - keep them valid. +- **Schemas are a contract.** Edit `schemas/*.v1.json` in place for an additive or + invariant-preserving change, pairing it with a new **positive** example under + `schemas/examples/` and a **negative** fixture under `schemas/examples/invalid/` that proves + the new invariant fails. Bump to a new version file (`*.v2.json`) only for a **breaking** + change — one that would invalidate existing stored records, or make new output that current + validators reject. See CLAUDE.md's "Changing a schema". - **The review path stays read-only.** Anything that edits an artifact belongs in delegate mode ([`docs/delegate-mode.md`](docs/delegate-mode.md)), which is experimental and isolated. - **Honesty over polish.** Don't claim platform support, provider neutrality, or safety the code doesn't actually provide — document limitations instead. -## Before a PR +## Before a PR — the three gates ```bash -pip install jsonschema # dev only -python tests/validate_schemas.py -python tests/test_helpers.py +python3 tests/test_helpers.py # stdlib, no pytest: supervisor, consent, backends, env policy, records +.venv/bin/python3 tests/validate_schemas.py # jsonschema lives in the repo-root .venv, not on PATH +.venv/bin/ruff check scripts/ tests/ # ruff too ``` -Both must pass (CI runs them). Please describe what you changed and why, and note any schema -or security-model implications. +All three must pass (CI runs them). `jsonschema` and `ruff` live in the repo-root `.venv`, not +on your PATH — invoke them through `.venv/bin/…` as shown, and run the stdlib helper test with +`python3`. Please describe what you changed and why, and note any schema or security-model +implications. + +Project vocabulary: [`docs/glossary.md`](docs/glossary.md). ## Contributor terms diff --git a/README.md b/README.md index 0236f0b..f6aec4d 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@ to the work, so a sloppy assumption or a missed edge case can sail straight thro from a different provider** is less likely to share your AI's blind spots. Think of it as an **independent advocate for one thing: the quality of the result you ship.** The second AI challenges the work with evidence; then Impasse checks each objection against your actual work, fixes the -problems that hold up, and brings you the few calls that are genuinely yours to make. +problems that hold up, and brings you the few calls that are yours to make. The reviewer **never edits your work** — the critic and the editor stay separate: fixes are applied by the host you're already working in (Claude Code or Codex, which drives Impasse), or by you, never by @@ -45,12 +45,15 @@ for one round, then to you if neither side can win. **Status: pre-release.** The open implementation of the pattern — named in [the CLAR essay](https://www.movingavg.com/essays/cross-lab-adversarial-review.html) and told as a field story in [*AI's Second Opinion: When Rival Models Disagree*](https://www.movingavg.com/essays/ai-second-opinion-rival-model.html). -The Codex path, consent gate, and schemas are implemented and tested; verify → reconcile → -escalate is **directed by the host skill, not enforced in code** — a review is only as good as the -host's adherence to the protocol (see [How it works](#how-it-works)). Dogfooding it on its own source caught a real -shipping bug before release. It drives the Codex CLI it finds on your machine (see Install for how it's located). That CLI is a -fast-moving alpha, so behavior is best-effort and version-sensitive — the `docs/backends/codex.md` -observations may go stale. Expect rough edges. + +**What's enforced in code:** the Codex review path, the consent gate, and the schemas (tested). +**What isn't:** verify → reconcile → escalate is directed by the host skill, so a review is only as +good as the host's adherence to the protocol (see [How it works](#how-it-works)). + +Dogfooding it on its own source caught a real shipping bug before release. It drives the Codex CLI +it finds on your machine (see Install for how it's located). That CLI is a fast-moving alpha, so +behavior is best-effort and version-sensitive — the `docs/backends/codex.md` observations may go +stale. Expect rough edges. ## Example @@ -62,18 +65,19 @@ It runs a cross-provider reviewer, verifies each finding against your artifact, report — the problems worth acting on, the ones the host threw out, and the calls that are yours: ```text -📊 Findings: 4 raised → 🤝 2 resolved · ❌ 1 refuted · ⚖️ 1 escalated to you +📊 Decisions: 4 finding(s) raised → ✅ 2 resolved · 🤝 0 accepted · ❌ 1 rejected · ⚖️ 1 escalated to you ────────────────────────────────────────────────────────────── -F001, F003 🟢 🤝 resolved — host confirmed and fixed both (details elided) -F002 🟠 high ❌ refuted +F001, F003 🟢 ✅ resolved — host confirmed and fixed both (details elided) +F002 🟠 high ❌ rejected 🔎 Reviewer: the go-to-market is undifferentiated. ◀ Host: the memo already concedes the product itself is a commodity and stakes its case on distribution — a rediscovered premise, not a gap. Refuted, with the quote. F004 🟠 high ⚖️ ESCALATED — needs your decision ❓ Enter Europe to diversify beyond a single market, or protect the nine-month runway? ────────────────────────────────────────────────────────────── +⚖️ 1 decision(s) need you; the rest the models settled between themselves. 📈 Your Impasse record — 9 reviews reconciled - 31 findings reviewed · 22 resolved · 4 accepted · 3 refuted with evidence · 2 escalated to you + 31 findings reviewed · 4 accepted · 3 refuted with evidence · 22 resolved · 2 awaiting you ``` *Example output. The reviewer never edits your work; the host applies the fixes it verifies, and @@ -118,8 +122,9 @@ submitted; Impasse's reviewer never holds the pen. The study's reviewer **cannot produced the eighteen-point gain in the other direction, but it denies that reviewer any execution feedback, where Impasse's host verifies each finding against the artifact with whatever evidence and tools apply. And the study has **no evidence requirement and no verification step** — a claimed -defect goes directly into the submitted program, where Impasse requires anchored evidence, checks -each finding, and escalates deadlocks instead of applying them. The authors flag two of these +defect goes directly into the submitted program, where Impasse requires [anchored +evidence](docs/glossary.md), checks each finding, and escalates [deadlocks](docs/glossary.md) +instead of applying them. The authors flag two of these themselves: static review understates what tool-using agents with sandboxes could achieve, and reviewers always emit code with no separate non-intervention action, which may inflate harmful rewrites. @@ -183,7 +188,7 @@ It is **domain-general** — the same protocol reviews: **See a second decision reviewed end to end** — a *different* memo (build-vs-buy on payments infrastructure), not code — from rival finding to the call that needs a human: [`docs/walkthrough-decision.md`](docs/walkthrough-decision.md). -Full protocol: [`docs/protocol.md`](docs/protocol.md). +Full protocol: [`docs/protocol.md`](docs/protocol.md). Terms: see the [glossary](docs/glossary.md). ## What the reviewer checks for @@ -242,7 +247,8 @@ of every event in every conversation: **Escalation counts are deliberately not reported yet.** An important operational metric is how often findings need a human ruling — no reliable historical rate exists. The counting rule only -recently became channel-independent (an operator ruling that decides a disposition now counts as +recently became channel-independent (a ruling by the operator — you, the human who owns the +decision — that decides a disposition now counts as an escalation whether it arrived through a formal deadlock or through conversation), and the operator attests that more judgment calls reached him than the pre-rule records captured. Historical events whose exact wording is no longer recoverable can't be amended in (the rule @@ -495,7 +501,7 @@ runs with decisions you haven't answered yet; `prune --older-than N` cleans up o they're kept `0600` and never committed. Every `show` closes with a **running recap across your reconciled runs** — findings reviewed, -resolved, accepted, refuted with evidence, and escalated to you — a plain reminder of what independent +accepted, refuted with evidence, resolved, and awaiting you — a plain reminder of what independent review has surfaced. Deeper longitudinal reporting (trends over time, per-artifact history) is still roadmap; each run is fully inspectable on its own. diff --git a/SKILL.md b/SKILL.md index 08bce10..50b7af0 100644 --- a/SKILL.md +++ b/SKILL.md @@ -225,7 +225,7 @@ backend is the cross-provider reviewer). The host is auto-detected (`IMPASSE_HOS **Speed (Fast mode).** A separate **codex-only** service-tier knob, **independent of effort**. Precedence: `--speed ` (this run) > `IMPASSE_CODEX_SPEED` env > persisted default (`impasse_run.py set-speed `, clear with `--clear`) > **`standard`** (Fast mode - **off**, the default). `fast` turns Codex **Fast mode** on — roughly ~1.5× faster serving at a + **off**, the default). `fast` turns Codex **Fast mode** on — faster serving at a **higher credit cost** — via `-c service_tier="fast" -c features.fast_mode=true`; `standard`/unset add nothing. Values are allowlisted at every entry; a bad `IMPASSE_CODEX_SPEED` is a structured `backend_error` naming the var, not a traceback. The claude backend has no speed knob — nothing diff --git a/docs/backends/claude.md b/docs/backends/claude.md index 93f8ef2..a36c8db 100644 --- a/docs/backends/claude.md +++ b/docs/backends/claude.md @@ -60,11 +60,14 @@ observations, not a durable API): (`stdout_truncated`) rather than trying to parse a cut-off object. - **Read-only is fail-closed, and it is NOT a process sandbox.** Unlike codex's `--sandbox read-only` (a real OS-level sandbox), the Claude reviewer runs in your normal Claude - Code process. Its read-only posture is instead: an **empty allowlist** (`--allowed-tools ""`) so - *no* tool is permitted (the artifact is on stdin — the reviewer needs none); `--strict-mcp-config` - so no MCP servers load; and a pinned `--permission-mode default` so it can't inherit a permissive - ambient mode (`acceptEdits`/`bypassPermissions`) or a `settings.json` that pre-allows network - tools. An allowlist fails *closed* as Claude Code adds tools; the `--disallowed-tools` list is + Code process. Its read-only posture is three independent controls instead: + - an **empty allowlist** (`--allowed-tools ""`) so *no* tool is permitted (the artifact is on + stdin — the reviewer needs none); + - `--strict-mcp-config` so no MCP servers load; + - a pinned `--permission-mode default` so it can't inherit a permissive ambient mode + (`acceptEdits`/`bypassPermissions`) or a `settings.json` that pre-allows network tools. + + An allowlist fails *closed* as Claude Code adds tools; the `--disallowed-tools` list is defense-in-depth (it also names the exfiltration vectors `WebFetch`/`WebSearch` and the spawn tool `Task`). Verified on 2.1.197: under this config the reviewer's attempts to `Read` a local file and to `WebFetch` are both blocked, yet it still answers from stdin. **Caveat:** do not run diff --git a/docs/backends/codex.md b/docs/backends/codex.md index adee557..39e8ea5 100644 --- a/docs/backends/codex.md +++ b/docs/backends/codex.md @@ -4,7 +4,8 @@ The reference reviewer backend. Impasse's protocol is backend-neutral; Codex is implementation, alongside the `claude` fallback backend (`docs/backends/claude.md`). `scripts/impasse_lib.py` resolves it and `scripts/impasse_run.py` supervises it. -Its independence tier is **host-relative**: to a Claude host (the usual case) it's the +Its independence tier is **host-relative**: to a Claude [host](../glossary.md) (the agent driving +Impasse — usually Claude Code) it's the `cross_provider` reviewer; to a Codex host (`IMPASSE_HOST=codex`) it's `same_provider` and the runner says so — use `--backend claude` there instead. See `docs/environments.md`. @@ -34,7 +35,8 @@ codex exec --json --output-last-message \ "" # artifact piped on stdin, then EOF ``` -Verified behaviors (on `codex-cli 0.144.0-alpha.4` — re-check with `codex exec --help`, these +Verified behaviors (on `codex-cli` 0.144.0-alpha.4; the ChatGPT.app rebrand noted above was seen on +0.145.0-alpha.18 — re-check with `codex exec --help`, these are version observations, not a durable API): - **stdin must reach EOF.** `codex exec` blocks indefinitely if stdin is an open, unwritten @@ -106,20 +108,20 @@ The argv builder re-checks the allowlist before interpolating into `-c` (defense ## Fast mode / service tier -A separate codex-only knob, **independent of reasoning effort**: `--speed fast` selects Codex's -**Fast mode** service tier — roughly ~1.5× faster serving at a **higher credit cost** — by adding -`-c service_tier="fast" -c features.fast_mode=true`. `standard` (or unset) is the default and adds -no flags (Fast mode off). Precedence mirrors effort: `--speed` (per run) > `IMPASSE_CODEX_SPEED` -env > a persisted default (`impasse_run.py set-speed `, stored in `settings.json` -beside the model/effort defaults) > `standard`. The two-value allowlist (`standard|fast`) is -enforced at every entry point — the CLI flag and `set-speed` by argparse choices, `set_default_speed` -on write, `get_default_speed` again on read (a hand-edited `settings.json` can't smuggle a bad -value), and the env var at resolution: an invalid `IMPASSE_CODEX_SPEED` fails as a structured -`backend_error` naming the variable, never a traceback. The argv builder re-checks the allowlist -before interpolating into `-c` (defense in depth, so a future caller can't inject config syntax). -The review result reports the resolved value in `speed` (`standard` or `fast`); the claude backend -has no speed knob — an `IMPASSE_CLAUDE_SPEED` is ignored, never an error, and it reports `speed: -null`. Speed and effort compose freely (e.g. high effort with fast mode). +`--speed fast` turns on Codex's **Fast mode** — a higher service tier: meaningfully faster serving +at a **higher credit cost** — by adding `-c service_tier="fast" -c features.fast_mode=true`. +`standard` (or unset) adds no flags (Fast mode off). It is **independent of reasoning effort** — +the two compose freely (e.g. high effort with fast serving). Precedence mirrors effort: `--speed` +(per run) > `IMPASSE_CODEX_SPEED` env > a persisted default (`impasse_run.py set-speed +`, stored in `settings.json` beside the model/effort defaults) > `standard`. The +review result reports the resolved value in `speed` (`standard` or `fast`); the claude backend has +no speed knob — an `IMPASSE_CLAUDE_SPEED` is ignored, never an error, and it reports `speed: null`. + +Validation: the two-value allowlist (`standard|fast`) is enforced at every entry point (CLI flag, +`set-speed`, the persisted-value read, and the env var at resolution) and re-checked by the argv +builder before interpolation into `-c` — exactly the defense-in-depth the effort section describes. +An invalid `IMPASSE_CODEX_SPEED` fails as a structured `backend_error` naming the variable, never a +traceback. ## Failure handling (limits & outages) diff --git a/docs/environments.md b/docs/environments.md index c44d811..911c2c3 100644 --- a/docs/environments.md +++ b/docs/environments.md @@ -15,25 +15,29 @@ cross-provider choice**. The runner computes every tier relative to the detected Host identity (`detect_host()` / `host_detection()`): `IMPASSE_HOST` is authoritative (`claude | codex | gemini | cursor | other`), and the four common hosts are **auto-detected** from -genuine, **strict-value** env markers — deliberately not from `detect_environment()`, whose +**strict-value** env markers — deliberately not from `detect_environment()`, whose `IMPASSE_ENV` override is a surface-policy knob and must not be able to manufacture a host identity: | Host | Marker (strict value) | Confidence | Provider | |---|---|---|---| -| `claude` | `CLAUDECODE=1` (or a genuine Cowork/chat-sandbox surface marker) | strong | Anthropic | +| `claude` | `CLAUDECODE=1` (or a Cowork/chat-sandbox surface marker) | strong | Anthropic | | `gemini` | `GEMINI_CLI=1` | strong | Google | | `cursor` | `CURSOR_AGENT=1` | — | none (operator-chosen model) | | `codex` | `CODEX_SANDBOX=seatbelt` or `CODEX_SANDBOX_NETWORK_DISABLED=1` | **heuristic** | OpenAI | -Detection is **fail-safe**: markers are matched by exact value (an inherited `GEMINI_CLI=0` doesn't -count); ≥2 attributable markers, or one attributable marker plus `CURSOR_AGENT`, resolve to -`unknown` (an unordered inherited env set carries no nesting depth, so the driver is genuinely -ambiguous); and `IMPASSE_HOST` is **validated and conflict-checked** — a nonempty unrecognized value, -or a value that disagrees with an observed marker, yields `unknown` rather than silently letting a -weaker marker win. An undeclared/ambiguous host is `unknown` → `undetermined`, **never a positive -cross-provider claim**. `cursor`/`other` run an operator-selected model, so they too are -`undetermined` in either direction, as is a backend routed through an unattributable endpoint (a -custom gateway). +Detection is **fail-safe** — three rules, each resolving ambiguity to `unknown` rather than a guess: + +- **Strict-value exact match** — a marker counts only at its exact value (an inherited `GEMINI_CLI=0` + doesn't count). +- **≥2 attributable markers, or one plus `CURSOR_AGENT` → `unknown`** — an unordered inherited env set + carries no nesting depth, so the driver is ambiguous. +- **`IMPASSE_HOST` validated and conflict-checked → `unknown`** — a nonempty unrecognized value, or one + that disagrees with an observed marker, yields `unknown` rather than silently letting a weaker marker win. + +An undeclared/ambiguous host is `unknown` → [`undetermined`](glossary.md) (provider correlation +unestablished), **never a positive cross-provider claim**. `cursor`/`other` run an operator-selected +model, so they too are `undetermined` in either direction, as is a backend routed through an +unattributable endpoint (a custom gateway). Provenance rides on the result as `host_detection: {method, confidence}`. **Codex is a heuristic:** its sandbox-state vars are absent under `--dangerously-bypass-approvals-and-sandbox`, so a diff --git a/docs/glossary.md b/docs/glossary.md new file mode 100644 index 0000000..ba9987b --- /dev/null +++ b/docs/glossary.md @@ -0,0 +1,105 @@ +# Glossary + +The recurring vocabulary Impasse's docs and code reuse. Each entry gives the **mechanism** (what +the thing is) and its **role** (how it fits the whole). Terms marked *(coined)* are Impasse's own; +the rest are ordinary terms of art used in Impasse's specific sense. Link here on a term's first use +in a doc rather than re-defining it inline. + +## Roles + +- **host** — the AI agent that loads the skill and drives the protocol (Claude Code or the OpenAI + Codex CLI). It runs the reviewer subprocess, verifies each finding, applies the fixes, and puts the + deadlocks to the operator. *Independence is measured relative to the host* (see **independence tier**). +- **reviewer** *(coined, project sense)* — the second AI, run as a subprocess (`codex exec` or + `claude -p`), that inspects the artifact **read-only** and returns findings. It argues; it never + edits the artifact and never applies fixes (the critic never holds the pen). +- **backend** — the resolved reviewer CLI plus the data destination its choice implies. The `codex` + backend defaults to `https://api.openai.com` (or wherever `OPENAI_BASE_URL` points — Azure, a + gateway, localhost); the `claude` backend to Anthropic. The concrete tool that realizes the + abstract "reviewer," and what **consent** is keyed to — at the *resolved* endpoint, not the default. +- **operator** — the human who owns the decision. They receive only the escalated **deadlocks**, as + crisp questions — not the raw finding list. +- **artifact** — what's under review: a decision memo, an essay, a research claim, a dataset, or code. + Its `kind` is chosen explicitly, never auto-detected. + +## The protocol + +- **CLAR — Cross-Lab Adversarial Review** *(coined)* — the practice of running a model from a + *different lab* as an adversarial reviewer, so its blind spots are less likely to match the work's. + Impasse is its reference implementation. +- **finding** — one issue the reviewer raises about the artifact, carrying a claim, a severity, and + its **anchored evidence**. The unit the host verifies and dispositions. +- **anchored evidence** *(coined)* — an evidence item that pairs a specific **locator** in the + artifact (a `file:line` range, a quoted span, a section, a JSON pointer) **with** an observation of + what's wrong there. Every finding must carry at least one; a bare location is not evidence, and the + schema enforces the pairing. It's what lets the host check a claim against the real artifact instead + of debating tone. (`docs/protocol.md` also calls these *anchored observations* — same thing.) +- **reconciliation** *(project sense)* — the host's per-finding pass after the review: for each + finding, verify it against the artifact, then assign a **disposition**. +- **disposition** *(coined)* — the state the host assigns a finding: **accepted** (host agrees, notes + it), **rejected** (host refuted it with contradicting evidence), **resolved** (addressed/fixed, or + an escalation the operator has since ruled on), **deadlocked** (neither side can settle it), or + **withdrawn** (the reviewer retracted it). Drives the tally and what escalates. +- **deadlock / deadlocked** *(coined)* — a finding neither side can settle: an evidence conflict, a + value/priority call that's the operator's to make, or a host objection it couldn't back with + evidence (`dispute_kind: unverified_refutation`). The disposition normally queued for the operator + via **escalation**; once they rule on it, the item becomes **resolved** (their ruling recorded as + the resolution). +- **escalation** *(project sense)* — surfacing a deadlock to the operator as a question, carrying its + `dispute_kind`, `stop_reason`, and `operator_question`. An operator ruling counts as an escalation + regardless of channel. +- **converged / deadlocked / incomplete / failed** — run-level outcomes (distinct from a single + finding's disposition): **converged** = every finding reached a terminal, non-deadlocked state; + **deadlocked** = at least one escalated; **incomplete** = a round/budget cap stopped it; + **failed** = a backend/timeout/consent/invalid-response error (never reported as converged). + +## Independence + +- **independence tier / ladder** *(coined)* — the reviewer's independence *relative to the host's + provider*, ranked: **cross_provider** (different provider — the point) > **undetermined** > ** + same_provider** (shares the host's provider and blind spots — breadth, not independence) > + **self_review** (the host model reviewing in its own context — last resort). Computed per run. +- **cross-provider / cross-lab** — a reviewer from a different provider (the tool's proxy) or lab + (the intent) than the host. Where the blind-spot decorrelation comes from. +- **undetermined** *(coined)* — the tier when provider correlation can't be established (a + mixed-model host like Cursor, or an unattributable endpoint). Never a positive cross-provider claim. +- **self-review** *(project sense)* — the host model reviewing the artifact in its own context. + Near-zero independence; permitted only where no reviewer subprocess can run (chat sandbox / Cowork), + and refused for `kind=code`. +- **host-detection provenance** *(coined)* — the recorded basis for a host label, carried on every + result as `host_detection: {method, confidence}` (e.g. `auto`/`override`; `strong`/`heuristic`/ + `none`) so a heuristic guess is never presented as certainty. +- **fail-safe / fail-open / fail-closed** — a control's default when it's uncertain. Host detection + is *fail-safe*: any ambiguity resolves to `unknown` (→ tier `undetermined`), never a guessed + positive. An **allowlist** *fails closed* (nothing is permitted unless named); a **denylist** can + only *fail open* (a new item is allowed until banned), so it's defense-in-depth only. + +## Runtime & safety + +- **consent gate / data boundary** *(project sense)* — reviewing sends the artifact to a third-party + provider, so Impasse **blocks by default** until the operator approves the destination. Consent is + keyed to the normalized endpoint (a gateway/proxy needs its own grant) and stored `0600`. +- **hermetic** — the codex reviewer runs isolated from the host's own config and repo rules + (`--ignore-user-config --ignore-rules`), so neither `~/.codex/config.toml` nor a repo `AGENTS.md` + can reroute the data or inject instructions into the read-only reviewer. +- **run record** *(coined)* — the persisted reviewer-response (and, once saved, the reconciliation) + for one review, under `config_dir()/runs//`. The audit trail; holds artifact content, so + it's kept `0600` and never committed. +- **supervisor** *(project sense)* — the process manager (`supervise()`) that runs the reviewer + subprocess under a hard wall-clock cap **and** an idle (no-output) cap, bounds its output, and on an + abnormal exit tears down the subprocess's **process group** (best-effort: a descendant that calls + `setpgid`/`setsid` escapes the group, and one can briefly outlive a clean exit). The reviewer is + untrusted and can hang or flood output, so it can't run unbounded. + +## Reviewer controls (codex backend) + +- **model** — which reviewer model runs (`--model` / `set-model` / `IMPASSE_CODEX_MODEL`). +- **effort** — Codex reasoning-effort level, `none`…`xhigh` (`--effort` / `set-effort` / + `IMPASSE_CODEX_EFFORT`). How hard the model thinks. The `claude` backend has no such knob. +- **Fast mode / execution speed** — Codex's higher *service tier* (`--speed fast`, adding + `service_tier="fast"` + `features.fast_mode=true`): faster serving at higher credit cost. + Codex-only, and independent of **effort** (you can combine high effort with fast serving). +- **raw mode** *(coined)* — `--raw`: return the reviewer's **unverified** findings and skip the + verify → reconcile → escalate protocol, recording nothing. A throwaway self-check on your own work, + never a review you hand to the operator. (Distinct from **Fast mode** — `--raw` skips verification; + Fast mode only changes serving speed.) diff --git a/docs/panel-mode.md b/docs/panel-mode.md index 18f3104..c31ff1d 100644 --- a/docs/panel-mode.md +++ b/docs/panel-mode.md @@ -1,7 +1,7 @@ # Panel mode (proposal — NOT built) **Status: design proposal.** Nothing here is implemented. This captures the design and, more -importantly, the constraints that keep panel mode from quietly breaking Impasse's thesis. Run +importantly, the constraints that keep panel mode from breaking Impasse's thesis. Run this proposal through Impasse (`kind=decision`) before building any of it. ## Motivation @@ -49,9 +49,9 @@ Notes: ## Orchestration — this is a Workflow, not the core protocol -Panel mode is a fan-out/synthesize pipeline, which is what the Workflow layer is for. It must feed -back into the **same** reconciliation machinery so the human still gets verified findings + routed -deadlocks: +Panel mode is a fan-out/synthesize pipeline — the job of the **Workflow layer**, a stage layered +*above* the core one-reviewer protocol. It must feed back into the **same** reconciliation machinery +so the human still gets verified findings + routed deadlocks: ``` select stakeholders → fan out: [stakeholder×N, skeptic(Codex), creative, meta] @@ -87,7 +87,8 @@ gets swamped and the "only deadlocks reach the human" promise breaks. ## Relationship to the Claude fallback backend Panel mode and the [Claude fallback backend](backends/) are independent but complementary: the -fallback lets Impasse run with no second vendor install (breadth/independence ladder — see the -Guardrails independence caveat), and panel mode is where a same-provider Claude fleet earns its +fallback lets Impasse run with no second vendor install (breadth/independence ladder — a +same-provider fallback reviewer shares the host's blind spots, so it buys breadth, not +independence), and panel mode is where a same-provider Claude fleet earns its keep as *breadth* while a cross-provider skeptic supplies *independence*. Build the fallback backend first (smaller, on-thesis); panel mode only if decisions are the proven use. diff --git a/docs/protocol.md b/docs/protocol.md index 6efc790..159c5dc 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -12,10 +12,14 @@ prepared → finding_triage host classifies each finding → evidence_verification host checks the evidence against the real artifact/facts → host_response accepted | rejected (with evidence) | contested | resolved + (persisted states: accepted / rejected / resolved / deadlocked / withdrawn) → reviewer_rebuttal one round, on contested findings only → resolved | deadlocked ``` +The reviewer returns [anchored observations](glossary.md) — each tied to a specific locator in the +artifact (file:line, a quoted span, a section); an unanchored claim can't serve as evidence. + **A rejection needs evidence, or it isn't a rejection.** `rejected` requires at least one verification that *contradicts* the finding — the schema enforces this. A refutation resting only on host judgment ("I don't think this matters," "that tradeoff is fine") is a disagreement the diff --git a/docs/security-model.md b/docs/security-model.md index b12f6bb..2cbfeb0 100644 --- a/docs/security-model.md +++ b/docs/security-model.md @@ -8,7 +8,7 @@ Both are handled deliberately. Report vulnerabilities per `SECURITY.md`. Reviewing an artifact means its content **leaves this machine** for the reviewer's provider, under that provider's terms and retention. -- **Consent is block-by-default** and keyed to the *normalized endpoint* (`scheme://host:port`), +- **Consent is block-by-default** and keyed to the *normalized endpoint* (`scheme://host[:port]`), not just a provider label — so pointing `OPENAI_BASE_URL` at Azure, a proxy, or localhost requires a fresh grant. URLs with embedded credentials are rejected. - **Codex-backend routing caveat.** The endpoint Impasse keys consent to is derived from @@ -43,8 +43,11 @@ under that provider's terms and retention. paths, or the instruction. - Reasoning-effort values are allowlisted before being passed to the backend. - The reviewer runs read-only by default. Wall + idle timeouts and POSIX process-group - termination bound it; transient run artifacts (`*.txt`/`*.jsonl`/`*.err`) may contain artifact - content and are `.gitignore`d — treat them as sensitive and clean them up. + termination bound it; the runner's scratch dir (holding the reviewer's `last-*.txt` output) is + removed automatically via `shutil.rmtree` in a `finally` on normal cleanup, so you don't clean it + up by hand — though an abrupt kill (`SIGKILL`) or a host crash can leave one behind. + The `*.jsonl`/`*.err` run artifacts may contain artifact content and are `.gitignore`d; the + persistent sensitive artifacts are the run records (below). - **Run records (the audit trail) contain artifact content.** Each run's reviewer-response and reconciliation-result are persisted under `config_dir()/runs//` (`0600` files, `0700` dir), never committed. This is deliberate — a governance tool should keep receipts — but it @@ -59,6 +62,7 @@ it. Impasse is a second opinion, not an adjudication oracle. Agreement is eviden ## Delegate mode raises the risk -Letting the reviewer edit the artifact is a different trust level — it runs in an isolated -temporary worktree, never the operator's checkout, and is experimental and opt-in. See +Letting the reviewer edit the artifact is a different trust level: a separate, experimental, opt-in +capability that is **not implemented** in the read-only review path in `scripts/`. As designed it +*would* run in an isolated temporary worktree, never the operator's checkout. See `docs/delegate-mode.md`. diff --git a/docs/walkthrough-decision.md b/docs/walkthrough-decision.md index b5147c2..60e8c41 100644 --- a/docs/walkthrough-decision.md +++ b/docs/walkthrough-decision.md @@ -37,7 +37,7 @@ record before accepting it: - **F001** — confirmed the assumption and the missing risk by reading the record, then **re-ran the record's own cost model** under a one-quarter dual-run. Build's margin over Buy narrows from 18% to 4% — real, but it doesn't flip. -- **F002** — confirmed the contradiction: the weights genuinely invert the stated strategic +- **F002** — confirmed the contradiction: the weights invert the stated strategic priority. ## 3. Reconcile diff --git a/scripts/impasse_lib.py b/scripts/impasse_lib.py index 9954975..520b056 100644 --- a/scripts/impasse_lib.py +++ b/scripts/impasse_lib.py @@ -1,7 +1,20 @@ -"""Shared, stdlib-only helpers for Impasse: config dir, backend resolution, hashing. - -No third-party dependencies — this ships with the skill. (Schema validation, which -needs `jsonschema`, is a dev/CI concern under tests/, not a runtime dependency.) +"""WHAT IT'S FOR: the stdlib-only core Impasse shares across its CLIs — resolves which reviewer +backend to run and where its data goes (consent keying), decides how independent that reviewer is +from the host, and persists run records + settings. No third-party deps; ships with the skill. + +The real subsystems living here: + - config dir + backend resolution — where local state lives, and turning a backend name into a + runnable command + its normalized data destination (get_backend / resolve_codex_command / + resolve_claude_command / normalize_destination); + - host detection + host-relative independence policy — identify the agent DRIVING the protocol + and grade a reviewer's independence RELATIVE to it (host_detection / independence_tier / + review_mode / the independence_notice disclosure); + - the run-record audit trail + a small persisted settings store (reserve_run_id / save_run_doc / + list_runs / load_run / forget_run; load_settings + the set/get_default_* accessors); + - content hashing for evidence digests and manifests. + +(Schema validation, which needs `jsonschema`, is a dev/CI concern under tests/, not a runtime +dependency.) POSIX (macOS/Linux) is the supported runtime; Windows is a documented roadmap. """ diff --git a/scripts/impasse_run.py b/scripts/impasse_run.py index 98105bd..d7ccc99 100644 --- a/scripts/impasse_run.py +++ b/scripts/impasse_run.py @@ -6,8 +6,9 @@ is what makes `codex exec` hang; and writing on a thread means a backend that stops reading stdin can't dodge the timeouts below; - a hard WALL timeout AND an IDLE (no-output) timeout; - - reliable process-TREE termination (own process group -> SIGTERM -> grace -> - SIGKILL, polling the GROUP, not just the leader), then a BOUNDED reap; + - process-GROUP termination on an abnormal exit (own process group -> SIGTERM -> grace -> + SIGKILL, polling the GROUP, not just the leader) — best-effort, not full-tree containment: a + descendant that calls setpgid/setsid escapes the group (F006 limitation) — then a BOUNDED reap; - size-capped stdout/stderr capture (avoids pipe-buffer backpressure deadlock); - a machine-readable termination reason. @@ -89,7 +90,8 @@ def _kill_tree(proc: subprocess.Popen, pgid: int | None = None, grace: float = 5 `pgid` should be the group id CAPTURED right after Popen (== proc.pid under start_new_session). Pass it explicitly: once proc.wait() has reaped the leader, os.getpgid(proc.pid) fails with ESRCH, - so a clean-exit teardown that relied on the lookup couldn't reach surviving descendants (F002).""" + so a clean-exit teardown that relied on the lookup couldn't reach surviving descendants + (crash-safe pgid capture — F002 in the security audit).""" if _POSIX: if pgid is None: try: @@ -129,6 +131,18 @@ def _kill_tree(proc: subprocess.Popen, pgid: int | None = None, grace: float = 5 def supervise(argv, input_bytes: bytes | None = None, *, wall_timeout: float = 180.0, idle_timeout: float = 60.0, max_output_bytes: int = 8_000_000, cwd: str | None = None, env: dict | None = None) -> RunResult: + """Run ONE reviewer subprocess to completion under a hard wall-clock cap AND an idle + (no-output) cap, and return a RunResult describing how it ended. + + Contract: captures size-limited stdout/stderr (each bounded by max_output_bytes, with a + truncation flag, so a chatty backend can't deadlock on pipe-buffer backpressure), feeds + input_bytes on a SEPARATE stdin thread then closes it (EOF), and on any abnormal exit tears + down the subprocess's process GROUP (own process group -> SIGTERM -> grace -> SIGKILL on + POSIX; process-level fallback elsewhere) — best-effort, not guaranteed whole-tree containment: a + descendant that calls setpgid/setsid escapes the group (F006). It NEVER raises for backend misbehavior — a crash, + wall timeout, idle stall, or spawn failure all come back as a RunResult.termination the + CALLER classifies (only invalid arguments raise ValueError here). + """ for label, val in (("wall_timeout", wall_timeout), ("idle_timeout", idle_timeout)): if not (isinstance(val, (int, float)) and math.isfinite(val) and val > 0): raise ValueError(f"{label} must be a positive finite number") @@ -148,7 +162,7 @@ def supervise(argv, input_bytes: bytes | None = None, *, wall_timeout: float = 1 return RunResult("spawn_error", None, b"", str(e).encode(), False, False, False, 0.0) # Capture the process-group id NOW, while the leader is alive: start_new_session makes the child a # group leader, so its PGID == proc.pid. Saved here, teardown works even after proc.wait() reaps the - # leader (when os.getpgid would fail with ESRCH) — F002. + # leader (when os.getpgid would fail with ESRCH) — this is the crash-safe pgid capture, F002. _pgid = proc.pid if _POSIX else None out = bytearray() @@ -224,11 +238,13 @@ def stdin_writer(): # the leader was still ALIVE (poll() returned None that iteration), so the captured pgid (proc.pid) # is valid and killpg targets the real group. On a CLEAN exit we must NOT signal: proc.poll() in the # loop above already REAPED the leader (waitpid/WNOHANG), freeing its pid — signaling the stale pgid - # then would risk hitting a recycled group (F005). ACCEPTED TRADEOFF: a rare descendant that outlives - # a cleanly-exited leader is therefore NOT terminated here; the bounded reader joins below only stop - # it from HANGING the supervisor (they time out and set reader_err) — they do not end the descendant's - # life (F009). Group-scoped only either way: a descendant that calls setpgid/setsid escapes this - # teardown (F006 — a known limitation, not full-tree containment). See F002 for the up-front pgid. + # then would risk hitting a recycled group — the reaped leader's pid/group can be reused (no + # signal-after-reap — F005). ACCEPTED TRADEOFF: a rare descendant that outlives a cleanly-exited + # leader is therefore NOT terminated here; the bounded reader joins below only stop it from HANGING + # the supervisor (they time out and set reader_err) — they bound a hang, they do not end the + # descendant's life (F009). Group-scoped only either way: a descendant that calls setpgid/setsid + # escapes this teardown (F006 — a known limitation, not full-tree containment). See the up-front + # pgid capture above (F002) for why the captured group id is what killpg targets on the timeout path. if termination != "completed": try: _kill_tree(proc, _pgid) @@ -328,8 +344,8 @@ def build_claude_argv(backend_command, *, instruction: str, model: str | None = the instruction is the prompt. The final message is read from STDOUT — `claude -p` has no `--output-last-message` file. (Reasoning effort has no Claude analog, so there is no effort knob here.) The variadic tool flags come after the fixed flags; `--disallowed-tools` comes - last. Read-only is - enforced fail-closed — see the note on `_CLAUDE_DENIED_TOOLS` and docs/backends/claude.md. + last. Read-only is enforced fail-closed by the empty allowlist + strict-mcp-config; the denylist + is defense-in-depth — see the note on `_CLAUDE_DENIED_TOOLS` and docs/backends/claude.md. """ argv = list(backend_command) + [ "-p", instruction, @@ -540,7 +556,8 @@ def review(*, kind: str, instruction: str, artifact_bytes: bytes, backend: str = if not (isinstance(_val, (int, float)) and math.isfinite(_val) and _val > 0): _m = f"--{_label} must be a positive finite number (got {_val!r})" return {**_fail("backend_error", _m, kind, _m, manifest), "host": host, "host_detection": hdblock} - # F002: 'auto' selects the most host-independent AVAILABLE backend, mirroring the `mode` + # Host-relative 'auto' backend selection (F002): 'auto' picks the most host-independent AVAILABLE + # backend, mirroring the `mode` # pre-flight (review_mode) — so a bare review on a Codex host picks the cross-provider `claude` # backend instead of the same-provider `codex` default. review_mode already accounts for # availability, endpoint attribution, and the Bedrock/Vertex refusal; we pass the host snapshot @@ -711,7 +728,8 @@ def _schema_fail(detail): # every schema defect is a structured backend_error, # re-spend on blindly and often signal a systematic cause (artifact echoed back, a # degenerate loop) — but they stay retryable: true, like rate_limited: the hint # means "recovery is plausible, offer it", not "the supervisor will re-spend"; the - # message carries the remedy (operator ruling on finding F002, 2026-07-16). + # message carries the remedy (operator ruling on the size-bound-retry finding, F002, + # 2026-07-16). final_bytes = None if out_last is not None: # codex writes the final message to a file try: @@ -756,7 +774,7 @@ def _schema_fail(detail): # every schema defect is a structured backend_error, run_id = parsed.get("review_id") recorded = False record_path = None - # raw mode is a fast throwaway (findings only, no verify/reconcile/escalate) — don't record. + # raw mode is a throwaway self-check (findings only, no verify/reconcile/escalate) — don't record. skip_record = no_record or raw # Persistence is a data boundary too: surface where the reviewed content lands locally. record_notice = (("Not recorded (raw mode)." if raw else "Not recorded (--no-record).") @@ -765,10 +783,12 @@ def _schema_fail(detail): # every schema defect is a structured backend_error, reserved = None # set only AFTER reserve_run_id returns, so cleanup can't touch a pre-existing run try: # reserve a UNIQUE dir first so an untrusted/duplicate review_id can't overwrite - # another run's record (F004); the reserved id is what we report and reconcile against. + # another run's record (unique-run-dir reservation — F004); the reserved id is what we + # report and reconcile against. reserved = lib.reserve_run_id(run_id) run_id = reserved - # Propagate the reserved id INTO the stored document (F002): reconciliation keys its + # Propagate the reserved id INTO the stored document (reserved-id propagation — F002): + # reconciliation keys its # save off the document's review_id, so the record's review_id must equal the dir name, # or a later reconciliation would land in the wrong directory. parsed["review_id"] = run_id @@ -782,7 +802,8 @@ def _schema_fail(detail): # every schema defect is a structured backend_error, ) except OSError: # A failure AFTER a successful reservation: remove only the dir WE reserved (never the - # reviewer-supplied original id, which could be a pre-existing run — F007) and report + # reviewer-supplied original id, which could be a pre-existing run; clean up only our + # own reservation — F007) and report # that the content wasn't persisted rather than silently claiming success. if reserved is not None: try: @@ -843,8 +864,8 @@ def _main(argv=None) -> int: "gap is not a hang; keep this ≈ --wall (it can't distinguish a hang from a long API wait).") rv.add_argument("--no-record", action="store_true", help="don't persist the run record") rv.add_argument("--raw", action="store_true", - help="fast mode: return the reviewer's UNVERIFIED findings and skip the " - "verify/reconcile/escalate protocol (implies --no-record)") + help="return the reviewer's UNVERIFIED findings and skip the " + "verify/reconcile/escalate protocol (records nothing; implies --no-record)") md = sub.add_parser("mode", help="report the strongest honest review mode for this environment") md.add_argument("--kind", required=True, choices=["code", "document", "decision", "research", "data", "other"]) md.add_argument("--environment", default=None, help="override auto-detection (else IMPASSE_ENV / auto)")