Pre-Commit Review Checklists
September 8, 2026 · View on GitHub
Single source of truth for all automated review checklists (Bible P7: DRY).
Loaded by ouroboros/tools/review.py at review time and injected into the
multi-model review prompt.
When a new reviewable concern appears, add it here — not in prompts or docs.
Advisory Pre-Review Workflow
Default sequence:
1. Finish ALL edits first (`edit_text` / `edit_batch` / `apply_patch` / `write_file`)
2. preflight_review(commit_message="...") ← run AFTER all edits, ONCE
3. commit_reviewed(commit_message="...") ← run IMMEDIATELY after advisory
Rules:
- Successful worktree mutations automatically mark advisory as stale. This includes
write_file,edit_text,edit_batch,apply_patch, and mutatingrun_command/ reviewed-commit paths when they change tracked worktree state. - A stale advisory must be re-run unless Ouroboros explicitly chooses the audited skip below.
- Do NOT interleave edits and advisory calls:
edit → advisory → edit → advisorywastes two expensive advisory cycles. Finish all edits first. - If advisory finds critical issues: strongly recommended to fix them and re-run advisory
before calling commit_reviewed.
Note: commit_reviewed's gate checks snapshot freshness, open obligations, and open
commit-readiness debt — it does not enforce zero advisory FAIL items as a hard
gate. Fixing critical findings and re-running advisory is best practice. Under
OUROBOROS_REVIEW_ENFORCEMENT=advisory, a fresh advisory also downgrades open obligations and commit-readiness debt to a warning by writingadvisory_obligations_acknowledgedtoevents.jsonl; stale advisory still blocks. Underblocking,commit_reviewedcan proceed only when no open obligations or commit-readiness debt remain. - Loud advisory enforcement (BIBLE P3 bound): the owner chooses enforcement;
advisoryis legitimate ONLY while every decision blocking enforcement would have stopped (critical findings, quorum failure, infrastructure failure, missing advisory provider) leaves a durable trace: areview_advisory_overrideevent inevents.jsonlplus the persistentadvisory_overrides_count/ recent-overrides fields inreview_status. Silent advisory is forbidden. - Once advisory is fresh → call commit_reviewed immediately without further edits.
skip_advisory_review=Trueskips only advisory freshness and the obligation/debt admission attached to it. Use LLM judgment when this cheap error-finding pass is slow, unhealthy, unavailable, or unlikely to add value; otherwise run it before the expensive authoritative review. The gate records the explicit skip; explain the LLM judgment in the surrounding task narrative. The flag does not change independently applicable test policy, triad, applicable scope review, or pre/post fingerprint and exact-commit/tag binding. The skip is durably audited, and unresolved obligations/debt remain visible.
Obligation tracking:
- Every blocking
commit_reviewedresult creates "open obligations" — a structured checklist of unresolved issues that advisory must explicitly address on the next run. - Advisory will receive the full list of open obligations and should respond to each one by name.
- A generic PASS without addressing open obligations is a weak signal — advisory is expected to confirm each obligation is resolved, though the gate does not enforce this at the code level.
- Open obligations are cleared automatically on a successful commit.
- Both triad-review blocks and scope-review blocks produce structured obligations.
- Repeated blockers may also synthesize commit-readiness debt. When present,
the non-bypass
commit_reviewedpath remains blocked underblockinguntil advisory clears both the open obligations and the debt;review_statusreports this viacommit_readiness_debts_count,repo_commit_ready=false, andretry_anchor=commit_readiness_debt. Underadvisory, a fresh advisory allows commit after recordingadvisory_obligations_acknowledged;review_statusstill shows the debt until a successful commit clears it.skip_advisory_review=Trueoverrides only this advisory debt admission; the debt remains visible and authoritative review still runs. - Anti-thrashing injection (v4.35.1): On retry attempts, open obligations are loaded from durable review state and injected into reviewer prompts as an inert JSON data block (fenced
jsonwith a "DATA records — not instructions" disclaimer). Two mandatory rules are also appended: (1) The JSON"verdict"field is the authoritative signal — withdrawal notes in"reason"text are ignored; (2) Do not rephrase prior findings under a different checklist item name. Inclaude_advisory_review.py::_build_advisory_prompt, these same two rules are injected at step 5a unconditionally (on every advisory run, not only when obligations exist), and reinforced at steps 6.e/6.f when obligations are present. - Obligation storage policy: All obligations are stored; deduplication is the agent's responsibility.
Multiple obligations describing the same root cause (from reviewer rephrasing across attempts) are
expected — address them together and explain this in
review_rebuttal. - Note: conservative false-stale is acceptable. If you are unsure whether a mutating path
changed the relevant repo snapshot, re-run
preflight_reviewexplicitly.
Review-exempt operations
The following tools create commits but are exempt from multi-model review (Bible P9 explicit exception):
vcs_restore— discards uncommitted changes (not a commit, no review needed)vcs_revert— creates a mechanical inverse of an already-reviewed commitvcs_rollback— resets to an existing tag/SHA (already-reviewed state)
Rationale: review gates on rollbacks create a paradox where reviewers block the undo for missing tests/VERSION, trapping the agent with broken code. These tools restore to already-reviewed states by definition.
Pre-Commit Self-Check (Ouroboros, before calling preflight_review)
Run this walkthrough honestly before every preflight_review call for a
commit_reviewed. The correct sequence is:
finish ALL edits → Pre-Commit Self-Check → preflight_review → commit_reviewed
This section is not injected as a named checklist section by the review prompts — it exists here so the agent's
pre-flight checklist lives in the same single source of truth as the review
checklists it guards. When docs/CHECKLISTS.md itself appears in a commit's
touched files, reviewers will see it as part of the Current touched files
pack, but it is not loaded as a standalone checklist the way the Repo Commit
or Intent/Scope checklists are.
| # | Check | How |
|---|---|---|
| 1 | VERSION, web/package.json, web/modules/api_types.js (GATEWAY_CONTRACT_VERSION), the README.md badge, the named direct-download links (README.md, site/install/index.html, docs/install/index.html), the latest README.md history row, and the docs/ARCHITECTURE.md header — are they all carrying the author-facing spelling (for example 4.50.0-rc.3)? And do pyproject.toml and the uv.lock root version carry the PEP 440 canonical form of that same version (for example 4.50.0rc3)? | read_file each file before editing. Never reconstruct version strings from memory — the in-context copy may be stale. The VERSION vs pyproject.toml divergence is intentional: pyproject.toml must satisfy PEP 440 so pip / build / twine accept it, while VERSION / tags / README / ARCHITECTURE use the author-facing spelling. tests/test_packaging_sync.py::test_version_file_and_pyproject_are_synced enforces the relationship via ouroboros.tools.release_sync._normalize_pep440. |
| 2 | Preparing any commit → is VERSION bumped? | Under BIBLE.md P9, every commit is a release. A VERSION bump is mandatory for every commit, including docs/config/memory changes. Update every carrier together — VERSION, pyproject.toml, uv.lock, web/package.json, web/modules/api_types.js, the README.md badge / history row / download links, both install pages' download links, docs/ARCHITECTURE.md header; ouroboros/tools/release_sync.py::version_carrier_desyncs() checks the file carriers (the git tag follows on commit). Exception: external contribution commits are version-neutral per P9 (proposed) / CONTRIBUTING.md — the maintainer's integration commit performs the bump, so unchanged carriers on a contributor PR are correct. |
| 3 | New or changed logic → does an existing or newly staged test assert on the specific scenario it introduces? | Name the scenario your code handles in plain words. If no test asserts on THAT named scenario, write or update one now. "Tests exist for the module" is not the same as "tests cover this new behavior". |
| 4 | Shared log / memory / replay format changed? | Grep every reader and writer first. JSONL logs (events.jsonl, task_reflections.jsonl, replay indexes), durable state files (advisory_review.json, review_continuations/*.json), and canonical-vs-derived memory pairs (patterns-register journal / patterns.md, improvement-backlog items) must stay coherent across every consumer. |
| 5 | New validation guard, input filter, or edge-case check? | Before the first commit attempt, name three concrete ways it could break: wrong bounds, legitimate inputs it silently blocks, platform-specific edge cases. If you cannot name three, think longer. One honest minute here is cheaper than one reviewer round. |
| 6 | New tool added? | get_tools() exports it, its schema description says WHEN to choose it (each profile receives its visible schema set every round, so the schema is the SSOT of the per-tool contract; prompts/SYSTEM.md is the cross-tool selection policy and mentions a tool only when the change alters that policy, never as a catalog entry; mechanism documentation lives in ARCHITECTURE/DEVELOPMENT), the handler signature matches the declared schema, and (if it mutates repo state) it is routed through the reviewed commit path rather than ad-hoc run_command. Also add an explicit entry in ouroboros/safety.py::TOOL_POLICY (POLICY_SKIP for trusted built-ins, POLICY_CHECK for opaque or outward-facing ones) — the test_tool_policy_covers_all_builtin_tools invariant will fail otherwise, and without an entry the tool falls through to DEFAULT_POLICY = check and pays a light-model LLM call per invocation. |
| 7 | Tests green before first commit_reviewed? | Run pytest -x on the narrowest relevant target(s) you can name before the first preflight_review / commit_reviewed attempt. Size gates no longer block locally: they live in the official-CI-only size_ratchet pytest lane (manifest exactness plus the pairwise base-vs-tip shrink-only transition), and local surfaces (check_worktree_readiness, codebase_health) surface the same validate_size_ratchet findings as "official CI will enforce" warnings. When a size warning appears — or a new .py file lands under ouroboros/ or supervisor/ — run pytest tests/ -m size_ratchet and scripts/regenerate_size_ratchet.py locally to preview and fix what official CI would reject. A red test suite before the first commit attempt has caused repeated $2-5 blocked-review cycles. |
| 8 | Adding a README.md version row? | BIBLE.md P9 hard cap: ≤ 2 major, ≤ 5 minor, ≤ 5 patch visible entries. Categories are mutually exclusive: major = X.0.0 (minor=0, patch=0); minor = X.Y.0 (patch=0, Y≠0); patch = all other X.Y.Z (Z≠0). Count existing rows in the category you are adding to. Easy check: run_command(["python", "-c", "import sys; from ouroboros.tools.release_sync import check_history_limit; warns=check_history_limit(open('README.md').read()); print(warns or 'OK')"]) — if it prints warnings, trim the oldest row in the over-limit category in the same edit before committing. |
| 9 | Changing any of build.sh, build_linux.sh, build_windows.ps1, Dockerfile, or ouroboros/tools/browser.py? | Cross-surface doc sync is mandatory. Check ALL of: README.md Install section (Linux native-lib caveat), README.md Build section (per-platform instructions), docs/ARCHITECTURE.md browser tools paragraph, WebKit/mobile verification notes, and inline comments in the touched build script. Any one of these being stale has blocked review twice. Verify before staging. |
| 10 | Changing ouroboros/tools/commit_gate.py? | Coupled surfaces that MUST be updated atomically in the same commit: (a) claude_advisory_review.py::get_tools() tool description for preflight_review and review_status; (b) claude_advisory_review.py::_next_step_guidance() strings; (c) docs/DEVELOPMENT.md Review & Commit Protocol section; (d) the prompts/SYSTEM.md Self-Modification section IF the commit-gate rule it states changed. Missing any one has blocked review. |
| 11 | Changing VERSION + pyproject.toml? | Ordering matters: (1) write VERSION, pyproject.toml, uv.lock, web/package.json and web/modules/api_types.js first; (2) then write the README.md badge + changelog row + download links, both install pages' download links, and the docs/ARCHITECTURE.md header; (3) then run pytest. Never interleave — updating README before VERSION means test_version_in_readme will catch a stale badge. |
| 12 | Writing or editing any JS file under web/modules/? | New or changed static inline visual properties are blocked: inspect the diff for added/changed style="" markup and .style.<property> assignments, and use CSS classes/tokens plus classList/hidden instead. Unchanged legacy hits are debt, not a blocker. A dynamic measured value may update a narrowly named CSS custom property when that is the actual runtime data flow. |
| 13 | Changing LLM output-token budgets? | Grep the whole repo for max_tokens, max_completion_tokens, _MAX_TOKENS, and max_toks. Keep docs/ARCHITECTURE.md §LLM output token budgets and tests/test_max_tokens_constants.py in sync so main-loop, VLM, summaries, compaction, skill publish, and consciousness floors cannot drift independently. |
| 14 | Changing extension loader/dispatch or isolated deps? | Native-risk extension imports and tool/route/WS handlers must stay out-of-process. Add or run regression tests where a native-risk plugin aborts during import and the host survives, plus tool/route child-dispatch tests. Do not "fix" failures by importing native-risk plugin code in server.py. |
| 15 | Changing supervisor/git_ops.py, launcher.py, server.py, ouroboros/tools/review_helpers.py, ouroboros/tools/git.py, tests, or evolution scheduling/checkpoint code? | Prove two invariants before review spend: (1) pytest/preflight cannot mutate the live repo or live data/ (OUROBOROS_DATA_DIR / OUROBOROS_SETTINGS_PATH must be isolated, and OUROBOROS_MANAGED_BY_LAUNCHER must not leak into test subprocesses); (2) autonomous restart/reset cannot erase active evolution work — it must either land a reviewed local commit or preserve a rescue/transaction recovery pointer and pause/stop the campaign. |
| 16 | Changing devtools/? | Keep operator tools isolated from runtime and package discovery: no runtime, web, or release imports from devtools/; no generated outputs under repo/ or live data/; no secrets printed or committed. A touched devtool is reviewable executable code even when unrelated devtools stay compact in broad review packs. Domain-specific launch, routing, and methodology guidance stays with the relevant devtool. |
| 17 | Diff spawns OS processes (subprocess.Popen / mp.Process without a bounded wait)? | Route it through ouroboros.process_custody.spawn_supervised (or record_process write-through) with an explicit scope (task/session/daemon) so the orphan reaper can find it; tests/test_process_custody.py enforces the allowlist. |
| 18 | New/changed test spawns a real OS process, binds a real port, or mutates a module-level global/registry? | CI and the hermetic commit gate (preflight_runner.py) both run the suite as a parallel -m "not serial" -n auto pass plus a serial pass. Mark such a test @pytest.mark.serial (or add its file to _SERIAL_TEST_FILES in tests/conftest.py) — otherwise it flakes on kill/port-reclaim timing, or crashes a worker, which the gate reports as a PARALLEL_WORKER_CRASH hard block (never retried, never a flake). Static screen before you commit: grep the new test for subprocess.Popen/subprocess.run, socket or fixed-port binding, start_new_session=True, and un-monkeypatched module globals — but syntax cannot prove mocking, so still classify it semantically. The serial marker is the fix for crashes and interference only: the parallel pass also carries a 300s per-test timeout, and the serial pass carries none, so marking a merely-slow test serial moves the hang into the one lane that cannot bound it — make it faster or split it instead. Every other test must stay parallel-safe: tmp_path (not fixed /tmp/... paths), monkeypatch.setenv/setattr (not bare os.environ[...] =), no execution-order assumptions, reset any mutated module global. See docs/DEVELOPMENT.md "The commit gate mirrors the CI split". |
Rule: read before write. Never reconstruct VERSION, pyproject.toml
version, or the README badge from memory — one stale reconstruction creates
a self_consistency FAIL that an entire advisory cycle is then spent on.
After a blocked reviewed commit (commit_reviewed) — mandatory regrouping before the next attempt:
When a reviewed commit returns critical findings, the reflex is to patch the single
flagged finding and retry. That pattern reliably produces 5-10 blocked rounds.
The correct procedure before every retry:
- List all open obligations and commit-readiness debt (
review_statustool or the Review Continuity context section). - Group them by root cause — one underlying problem often generates 2-4 separately-named obligations from reviewer rephrasing.
- Write a short plan in a progress message: one paragraph naming each root-cause group and the single code/doc change that resolves it.
- Only then open any file and edit.
This step takes 2-3 minutes and has saved $20-50 in blocked-review cycles in practice.
The rule is stated where the block message is built (review.py::_build_critical_block_message),
but without it appearing here as a procedural step it stays theoretical rather than reflexive.
Repo Commit Checklist
Used by commit_reviewed for all changes to the Ouroboros repository.
| # | item | what to check | severity when FAIL |
|---|---|---|---|
| 1 | bible_compliance | Does the diff violate any BIBLE.md principle? | critical |
| 2 | development_compliance | Does it follow DEVELOPMENT.md patterns? Check explicitly: (a) naming conventions (snake_case modules/vars, PascalCase classes, UPPER_SNAKE_CASE constants); (b) entity type rules — Gateway classes contain ONLY transport, no business logic; Tool functions are thin wrappers; (c) Python everywhere (including tests//devtools/) and first-party web/**/*.js (including web/tests/) target ~1000 lines; exact repo-relative module debt above the 1600-line hard gate, exact (path, qualname) Python-function debt above 300 lines, the exact-current 1001-1500 band (new/re-entered paths need a nonblank rationale), and exact byte debt above 200,000 canonical UTF-8/LF bytes are checked in to ouroboros/size_ratchet_manifest.py; the enforcing surface for all of these (and for MAX_TOTAL_FUNCTIONS) is the official repository CI's size_ratchet pytest lane — manifest exactness on the tip tree plus the pairwise base-vs-tip shrink-only transition — while local runs surface the same validate_size_ratchet findings as warnings (a stale or growing entry is therefore review debt to flag, not a local commit block); methods above 150 lines are a decomposition signal, runtime-code total Python function/method count stays under ouroboros/review.py::MAX_TOTAL_FUNCTIONS, and more than eight parameters is a decomposition signal, not a hard gate; (d) no gratuitous abstract layers, and any SOLID/minimalism finding names an exact symbol/authority, concrete duplication or coupling, and a smaller contract-preserving alternative rather than citing diff size (P7 Minimalism) — and when the diff ADDS a surface (a new module, state file, ledger, resolver, cache, retry path, tool, endpoint, or background loop), the reviewer consults the docs/ARCHITECTURE.md map and NAMES the existing mechanism that already covers the need when one exists (name it exactly — the reuse-first duty this checklist carries for a CHANGE; the plan-review checklist judges an intention and has no such generative duty); absence of a covering mechanism may be stated in one line; (e) new LLM calls go through the shared LLMClient/llm.py layer, not ad-hoc HTTP clients; (f) cognitive artifacts (identity.md, scratchpad, task reflections, review outputs) must NOT use hardcoded [:N] truncation — explicit omission notes required; (g) new get_tools() exports follow the ToolEntry pattern in registry.py; (h) provider independence — no change may make a core capability (agent loop, multi-model commit review, scope review, or memory/context flows) silently require a second provider or OpenRouter specifically, and every supported single direct provider (local, OpenAI, Anthropic, MiniMax, DeepSeek, Cloud.ru, GigaChat) must keep its model AND review/scope slots self-fillable (see DEVELOPMENT.md "Provider Independence"); (i) a claimed-complete visible UI change includes vision-inspected evidence from at least one relevant real consumer flow. A screenshot file without inspection is insufficient; states/viewports/additional engines are risk-selected, mobile/WebKit are not universal, and an unavailable optional engine alone is not degradation. | critical |
| 3 | secrets_check | Are secrets, API keys, .env files, credentials present in the diff? | critical |
| 4 | code_quality | Careful code review: bugs, logic errors, crashes, regressions, race conditions, resource leaks? | critical |
| 5 | security_issues | Security vulnerabilities: injection, path traversal, secret leakage, unsafe operations? | critical |
| 6 | tests_affected | Did code logic change without corresponding test changes? (PASS if only docs/config/memory changed, or if tests already cover the new behavior.) Critical FAIL requires all three: (a) name a specific behavior, code path, symbol, or failure scenario that THIS diff introduces or changes; (b) explain why existing or newly staged tests do NOT catch that specific scenario; (c) the gap is concrete, not speculative. Adjacent tests in the same module or for the same feature count as coverage. Requiring an additional overlapping selector/unit/e2e test is only justified when a second distinct failure mode is named explicitly. If the only concern is "I'd feel better with one more test," that is advisory, not critical. | critical |
| 7 | architecture_doc | Did the change REPLACE the current description of every node it touched in docs/ARCHITECTURE.md / docs/DEVELOPMENT.md (map rows, flows, invariants)? PASS when a new module, endpoint or data flow gains its one-line row and touched descriptions read as current truth; PASS with "Not applicable" when nothing architectural changed. FAIL when the edit only appended a version stamp, a decision codename, a "used to / previously" narrative, or a second statement of a fact already owned elsewhere in the docs (P6 present-tense map, P7 one home per fact — see DEVELOPMENT.md "Documentation contract"). | critical |
| 8 | version_bump | Does this commit leave VERSION unchanged, or leave release artifacts out of sync? Exception: external contribution commits are version-neutral per P9 (proposed) / CONTRIBUTING.md — the maintainer's integration commit performs the bump; unchanged carriers on a contributor PR are correct, not a FAIL. | critical |
| 9 | changelog_and_badge | VERSION bumped but README.md badge or changelog not updated? (PASS if VERSION not bumped.) | critical |
| 10 | tool_registration | New tool function added but not exported in get_tools() OR missing explicit entry in ouroboros/safety.py::TOOL_POLICY? (PASS if no new tool.) Both surfaces are required: get_tools() makes the tool visible; TOOL_POLICY makes the per-call safety routing explicit and is guarded by the test_tool_policy_covers_all_builtin_tools invariant. | critical |
| 11 | context_building | New data/memory files that should appear in LLM context (context.py) but don't? | advisory |
| 12 | knowledge_index | Knowledge base topics changed but memory/knowledge/index-full.md not updated? | advisory |
| 13 | self_consistency | Does this change affect behavior described in BIBLE.md, prompts/, docs/, or this checklist itself? Check explicitly: (a) version in ARCHITECTURE.md header matches VERSION file; (b) every tool name prompts/SYSTEM.md or prompts/CONSCIOUSNESS.md mentions exists in get_tools() (or the background whitelist) and means the same thing there — completeness is NOT required, the schemas are the catalog; a prompt edit must not restate a tool schema or a structurally enforced gate, and a prompt that gains a sentence loses an equivalent one; a prompt edit is never an incident patch — no keyword, regex or branch added so the model picks a particular tool or route (P5, DEVELOPMENT.md "LLM-first affordances"); and when the diff touches the code that assembles a prompt (context.py, review packet/template builders, advisory, consciousness, plan packet, loop nudges) the injected prompt text and that code still agree; (c) JSONL log/memory file formats described in ARCHITECTURE.md match all readers/writers; (d) any behavioral change reflected in prompts/CONSCIOUSNESS.md if it affects background loop behavior; (e) DEVELOPMENT.md rules still accurate after the change. Severity must follow the shared Critical surface whitelist below — release metadata, tool schema, module map, behavioural documentation, or safety contracts are critical; commentary/prose/stylistic mismatches are advisory. | critical |
| 14 | light_external_artifacts | If tool/runtime policy changed, does light mode still allow external user deliverables via user_files, task-scoped task_drive/artifact_store, and process outputs while blocking Ouroboros repo/control-plane mutation? (The external claude_code_edit cwd lane retired with the tool — D10.) Do review prompts avoid recommending runtime_data/uploads or skill payloads as generic artifact transport? | critical |
| 15 | cross_platform | Does the diff use platform-specific APIs (os.kill, os.setsid, os.killpg, os.getpgid, fcntl, msvcrt, signal.SIGKILL, signal.SIGTERM, subprocess with start_new_session/creationflags, hardcoded / or \\ in filesystem paths) outside of ouroboros/platform_layer.py? Does it import Unix-only or Windows-only modules (fcntl, msvcrt, winreg, resource) at any level without a platform guard (sys.platform/IS_WINDOWS check)? | critical |
| 16 | changelog_accuracy | Do the exact wording, test counts, and minor description details in the README Version History row match what the diff actually does? Wording drift, off-by-one test counts, minor inaccuracies in descriptive prose — these belong here, NOT in self_consistency or changelog_and_badge. This item exists so reviewers have a dedicated advisory bucket for prose-level changelog imprecision that does not affect release metadata, runtime behavior, or safety contracts. | advisory |
| 17 | gateway_parity | If the diff changes any browser-facing endpoint, WebSocket message, or frontend API call, are ouroboros/gateway/contracts.py, ouroboros/gateway/router.py, web/modules/api_client.js, web/modules/api_types.js, and tests/test_gateway_parity.py still aligned? Missing alignment is advisory unless it also breaks a frozen contract, safety guard, release metadata, or runtime behavior. | advisory |
| 18 | subagent_isolation | If the diff changes schedule_subagent, child-task queueing, task constraints, tool discovery/execution, data reads, or memory handoff, does it preserve the accepted live-subagent contract: strict subagent_id + objective + expected_output schema, inferred lineage/workspace/contract/deadline/resource inheritance, local_readonly_subagent schema and execute-time allowlist, subagent-scoped secret/control-file denial for data tools, nested readonly delegation only within configured depth/cap limits (depth bounds how deep delegation NESTS, never actor strength; every new call names subagent_id, the scheduler snapshots the exact normalized ConfiguredSubagent route at task start, and an agent_session snapshot executes on the harness by construction — the host starts that exact leaf before the child's first model round without waiting on it, a definite typed start refusal ends the child unrun and typed at $0, and ambiguous start evidence wakes the model rather than terminaling; the model-visible schema must not expose model_lane/executor, while hidden legacy selectors map deterministically to one migrated row or return subagent_selection_required), no arbitrary local writes/commits/review/runtime/tool-expansion/skills-lifecycle/shell (bounded media projection such as extract_video_frames may write derived outputs only under artifact_store/video_frames through a host-owned command shape), enabled external tools allowed only by owner policy and inherited resources, the subagent browser boundary (external HTTP(S) + file:// scoped to workspace_root + loopback except actual Ouroboros control-service endpoints; concrete private origins require host-established resource_policy.allowed_origins with exact scheme/host/port and inherited/subset authority; unavailable identity for a matching recorded endpoint must not become foreign-service permission; apply the same target checks to direct navigation, actions and intercepted subresources, and validate every available redirect hop before returning page content; native browser redirects may send a request before post-navigation validation, so this is not a pre-request isolation or DNS-rebinding guarantee; metadata/link-local and reserved targets remain refused by the existing URL policy; evaluate JS unavailable to local_readonly_subagent, available to a valid acting_subagent on its current page; vlm_query/analyze_screenshot available), full task-result handoff, new/changed wait/timeout paths for cognitive work using progress-aware/re-decidable waiting rather than a fixed cutoff that discards in-flight work (P5), and tests for both allowed and blocked paths? | critical |
| 19 | evolution_durability | If the diff touches supervisor/git_ops.py, launcher.py, server.py, ouroboros/preflight_runner.py, ouroboros/tools/review_helpers.py, ouroboros/tools/git.py, tests, review gates, or evolution code, does it preserve hermetic preflight, live repo/data mutation fuses, remote-optional local commit success, and transaction/rescue evidence for interrupted self-modification? | critical |
| 20 | context_budget_ssot | If the diff changes context-size budgets/constants (ouroboros/context_budget.py), the context layout/manifest, a section's tier/policy, or the typed ContextFit deficit/reclaim contract: does it keep the low/max context split coherent (single SSOT + both profiles + docs + drift-guard tests in sync), preserve the tier-0 always-full core (BIBLE/SYSTEM/identity/scratchpad/knowledge-index/recent-dialogue) in EVERY mode, use a visible on-demand pointer instead of silent truncation (P1), and leave the blocking scope-reviewer >=1M floor untouched wherever scope review applies? Since v6.80.0 the owner-only OUROBOROS_CONTEXT_MODE ALSO decides scope-review applicability (max: blocking ≥1M gate; low: declaredly not performed with a typed skip row), so any change that widens what low mode implies, or that lets the AGENT reach that setting, is an immune-system change under P3 — not a context-budget tweak. (PASS with "Not applicable" if no context-budget/layout change.) | critical |
| 21 | capability_regression | Does the diff REMOVE or NARROW a previously-supported user-facing behavior or capability — a tool/flag/mode/path that worked before now errors or is gated tighter (e.g. a new is_dir/existence guard that blocks a legitimate create, a tightened allowlist that drops a real path, a removed fallback)? If so, is it INTENTIONAL and disclosed as a breaking/capability change in the commit message + changelog? Accidental capability removal is the failure class this item names. Ask whether a golden "from zero" test would have caught it. Guard-change trigger (executable requirements, not an essay): ADDING or TIGHTENING a guard, filter, allowlist, or deny rule IS a capability change and fires this item. For such a diff the reviewer must verify two things: (a) the diff STAGES A POSITIVE TEST that exercises a legitimate flow THROUGH the new guard and proves it still succeeds — a negative "it blocks X" test alone is insufficient (a gate can pass its own probe while breaking every real run); (b) the diff or its disclosure NAMES THE SURVIVING POSITIVE PATH — the concrete actor and flow that still work after the change. A guard change that stages no surviving-path test is a capability-regression finding, not a safety improvement. Owner acceptance: a narrowing counts as OWNER-ACCEPTED only when a GREEN plan review explicitly names that narrowing; owner acceptance makes the finding advisory (disclosed, non-blocking). Intent wording, a commit-message disclosure, or a changelog row alone is disclosure, NOT acceptance. Severity follows the Critical surface whitelist below — silently removing a documented capability or a safety/release contract is critical; an owner-accepted narrowing or an internal-only refactor is advisory. Standing disclosures for this item live in docs/CHECKLISTS_ARCHIVE.md (owner-accepted removals/narrowings and standing notes); they remain binding on every reviewer — consult that file before raising a removal/narrowing finding on a surface it covers, and do not re-raise anything recorded there. | advisory |
| 22 | cache_friendliness | If the diff builds or reorders LLM prompt/context content (context builders, review prompt assembly, message construction in llm.py callers): does it keep prompt caching intact — stable governance/policy content BEFORE dynamic evidence, no dynamic values (timestamps, hashes, round counters, task ids) injected into a stable cached prefix, and no removal/breakage of existing cache_control markers or session/cache affinity keys? A change that silently fragments an existing cached prefix re-bills the full prompt on every repeat call. (PASS with "Not applicable" if no prompt/context assembly changed.) | advisory |
| 23 | delegated_transport | If the diff touches the delegated execution/review transport (ouroboros/subagents.py dispatch/route-health, ouroboros/tools/delegate.py, ouroboros/delegate_custody.py, ouroboros/delegate_progress.py, ouroboros/review_execution.py session executors, ouroboros/gateways/claudexor.py, ouroboros/claudexor_daemon.py, ouroboros/claudexor_runtime.py), does it preserve the delegation invariants: capability reductions reach all three destinations (durable envelope, child prompt, parent result — D4); a result counts as received only after a hash-bound read to EOF and retries replay the recorded byte-identical body (D7); the exact selected subagent_id snapshot starts its exact session route with custody-durable requested→effective evidence or returns a TYPED refusal, and neither host dispatch nor tool preflight substitutes another session/API/native route — any fallback is a new explicit LLM selection; quota exhaustion needs POSITIVE evidence judged against the route's own model (applies_to_models scoping, absence = unknown = usable); delegated spend settles through custody with unknown-never-rendered-as-zero and root/parent lineage; and no vendor/harness name is ever branched on in core. For direct and configured external work orders, verify complete chosen assignments and host authority across instruction roles, no arbitrary compiler cap or compulsory file/question transport, and no duplicate objective/output copies within instructions. Operative plan normalization and current reviewer inputs must preserve full content and tail-sensitive identity. Real route limits retain original input, cause and execution state. Legacy partial requests retain byte-identical pending recovery, exact renderer/selector/digest/range validation, durable source coverage and apply refusal until complete; reject remains available. Source availability never proves reading or comprehension. (PASS with "Not applicable" if no delegated-transport surface changed.) | critical |
| 24 | perf_lifecycle | If the diff adds or changes an endpoint, poller, subscription, or timer, or reads a growing store (JSONL log, ledger, event table): does any interaction-path read scan an unbounded store per request/message/tick (a full-table read filtered in code is such a scan)? Is a subscription/observer/interval/listener added without a paired disposer? Does O(history) work run on a poll/stream path? Does a GET handler perform new steady-state durable writes outside the two named exceptions? The authoritative definitions are DEVELOPMENT.md "Invariant: Projection over replay (hot readers of growing stores)" and "Invariant: UI resources carry a disposer" — check against those, do not re-derive them here. For an embedded or framed UI surface, also use DEVELOPMENT.md "Invariant: Embedded surfaces declare geometry and refresh semantics" for host geometry/overflow, teardown, retry/error, and real-consumer visual evidence. (PASS with "Not applicable" if the diff touches none of these surfaces.) | advisory |
| 25 | source_completeness | If any changed consumer can authorize PASS, a destructive rewrite, or replacement of a full contract, does its input distinguish complete from partial and carry a source reference the same actor can resolve? Does the consumer materialize every named omitted source before the decision, or abstain with the existing typed incomplete/degraded outcome? A marker or host claim alone is never sufficient. | critical when applicable |
| 26 | actor_readable_projection | If the diff adds a bounded projection, omission marker, summary, or status count, can the actor who must decide read the exact canonical source through an existing path? Verify the ref, root, generation/range or ID, and the reader's ability to resolve it after the real merge, promotion and cleanup; a successful copy or an intermediate packet does not prove the persisted consumer still resolves its citations. Host-unattested or merely hypothetical retrieval does not certify completeness. | critical when applicable |
| 27 | canonical_memory_fork | If the diff touches Project/fork/execution roots, summaries, memory, or GC, does it preserve one canonical identity and distinguish authority/biography from execution-local state? Are referenced canonical artifacts promoted or retained before a child/root is collected, with missing legacy bytes represented as gaps? | critical when applicable |
| 28 | review_artifact_continuity | If the diff changes plan, triad, scope, advisory, or acceptance evidence, are exact artifact bodies, source selectors, candidate SHA, reviewer model/profile/thread/route continuity, and all omissions retained? Continuity, transport, and coverage discrepancies are retained as typed facts beside the exact artifact bodies; a bounded or partial reviewer view must remain DEGRADED/NOT_RUN rather than PASS, and must never be implemented by discarding, blanking, or relabeling the bodies or their original cause. | critical when applicable |
| 29 | display_identity_replay | If the diff changes routing, steering, task cards, or history replay, does it preserve the event-time human Project › Task presentation snapshot in both live and replay paths while keeping opaque IDs as internal/debug facts? | advisory when applicable |
| 30 | web_design_system | If the diff touches web/ (modules, stylesheets, index.html, onboarding assets): does it conform to docs/DESIGN.md (type scale, foreground roles, status pairs, spacing tokens) and to the engineering rules in DEVELOPMENT.md "Design System" (no new inline visual styles, values live in web/style.css tokens, shared components over page-local copies)? When the diff ADDS a UI control, chip, card, dialog, or visual pattern, does it reuse an existing shared frontend primitive (name it — the registry is ARCHITECTURE.md §3 "Navigation and shared UI contracts") or state in one line why none covers the need? The authoritative definitions live in those documents — check against them, do not re-derive them here. (PASS with "Not applicable" if the diff touches nothing under web/.) | advisory |
| 31 | size_cap_paydown | If the diff was shaped by a size limit (module band/hard cap, function-size gate, byte debt): was the limit paid down by SIMPLIFYING the code that lives there — simpler control/data flow, dead code and duplicates removed, an existing SSOT reused, prose made compact and legible — rather than by extracting a helper, a passthrough wrapper or a neighbour module whose only reason to exist is the cap? Extraction is the LAST resort and is acceptable only for a natural boundary with its own reason-to-change, explicit contract and caller (BIBLE P7 «first simplify what exists»; DEVELOPMENT.md "Paying down a size cap"). A cap-driven bucket, a one-caller passthrough, or bytes bought by deleting contract-bearing comments, docstrings, messages or tests is a defect to report, not a paydown. | advisory when applicable |
Timeout-policy pointer for item 18 (2026-08-23): cognitive/review waits must
follow the layered policy in DEVELOPMENT.md and the timeout data-flow in
ARCHITECTURE.md: transport is not a reasoning cutoff, active operations are
typed idle-rail facts, owner deadlines narrow nested waits, and a late physical
result stays in custody rather than enabling a blind retry. The checklist item
does not create another timeout constant or a second scheduler.
Severity rules
- Items 1-5 are always critical.
- Items 6-10, 14-15, 18-20, 23, and 25-28 are conditionally critical: FAIL only when the condition applies. If the condition does not apply, write verdict PASS with a short reason (e.g. "Not applicable — no code logic change").
- Items 11-12, 16-17, 22, 24, 29, and 30 are advisory: FAIL produces a warning but does not
block. Item 22 (
cache_friendliness) passes with "Not applicable" when the diff touches no prompt/context assembly. Item 24 (perf_lifecycle) passes with "Not applicable" when the diff adds/changes no endpoint, poller, subscription, or timer and reads no growing store. Item 30 (web_design_system) passes with "Not applicable" when the diff touches nothing underweb/. - Item 13 (self_consistency) is conditionally critical: FAIL only when the
mismatch falls in the
Critical surface whitelistbelow AND a concrete stale artifact is named (specific file, line, or symbol). If no whitelisted surface is affected, the finding is advisory. If no concrete staleness is found at all, write verdict PASS with a short reason. - Item 16 (
changelog_accuracy) is advisory by design: prose-level wording drift, off-by-one test counts, and minor descriptive inaccuracies in the README changelog row MUST NOT be raised as critical underself_consistencyorchangelog_and_badge. They surface here and do not block. - Item 21 (
capability_regression) is advisory by default but escalates to critical under theCritical surface whitelistbelow: a SILENT removal/narrowing of a documented capability or a safety/release contract is critical; an owner-accepted narrowing (a GREEN plan review that explicitly names it — see item 21) or an internal-only refactor stays advisory. Disclosure alone (commit message, changelog) is not acceptance.
Retry convergence for tests_affected
When the previous blocker was only tests_affected and the new diff changes
only files under tests/ plus release/version touchpoints (VERSION,
pyproject.toml, README.md, docs/ARCHITECTURE.md), reviewers must focus
on verifying whether the newly staged tests address the named gap — not search
for fresh gaps in unchanged code. A new critical finding on this retry round
requires a new concrete artifact, consistent with the Critical threshold rule
below: a reformulation of an earlier concern is not a new finding.
Critical threshold rule (applies to ALL items)
Before marking any item CRITICAL you MUST be able to answer YES to ALL of:
- I can name the exact file, symbol, function, test, or config path in this repository that makes this problem live RIGHT NOW.
- That artifact actually appears in the diff or touched-file context I have been given (not just in a hypothetical future scenario or external environment).
- The fix requires a change to this diff — not a follow-up task or speculative guard.
- If my proposed fix ADDS or TIGHTENS a guard, restriction, or removal: I can name the surviving positive path — the concrete legitimate flow that still works after that fix. (A fix that narrows nothing answers YES automatically.)
If you cannot satisfy all four, use advisory, not critical.
The threshold binds proposed REMEDIES as much as findings: a recommended fix that deletes or narrows a capability is itself a capability change and must meet the same concreteness bar as the finding it answers — exact artifact, live problem, named surviving positive path. A remedy that cannot meet the bar of its own finding is advisory, and repeating it across rounds or reviewers confers no additional authority.
For any finding about narrative, prose, or cross-surface consistency, also apply
the Critical surface whitelist below (same rules for every reviewer — triad,
scope, and advisory). A mismatch outside the whitelist is advisory.
One root cause = one FAIL entry. Do NOT split one underlying problem into multiple FAIL items that all require the same change. Do NOT hold an obligation open by reformulating a fixed concrete issue into a broader future-risk variant — if the named artifact is fixed, mark PASS; raise a new advisory if a broader concern remains. Coverage is semantic, not numerical: zero or one FAIL is valid, and reviewers must never invent findings to reach a count.
Critical surface whitelist (binding for ALL reviewers — triad, scope, advisory)
When marking a cross-surface / self-consistency / narrative / "prose-vs-code" mismatch as critical, the mismatch MUST live in one of these categories:
- Release metadata —
VERSIONvspyproject.tomlvs the root version inuv.lockvsweb/package.jsonvsweb/modules/api_types.jsGATEWAY_CONTRACT_VERSIONvs README badge vs the named direct-download links (README and both install pages) vsdocs/ARCHITECTURE.mdheader — the file carriersrelease_sync.version_carrier_desyncs()checks — and vs the latest git tag once it exists. Also:VERSIONbumped but no README changelog row for the new version. - Tool schema — a tool's
get_tools()schema (name, parameters, description) that disagrees with its handler, or a tool name/argument thatprompts/SYSTEM.mdorprompts/CONSCIOUSNESS.mdnames butget_tools()does not export. The schema is the SSOT of a tool's contract; a prompt that omits a tool is not a mismatch. Applies to user-facing CLI/tool contracts. - Module map —
docs/ARCHITECTURE.mdnaming a module / endpoint / data file / UI page that does not exist (or the reverse: a new one was added and the map was not updated). This is a hard P6 (Architecture mirror) contract. - Behavioural documentation — a docstring, README description, or ARCHITECTURE section explaining what a changed tool/command actually does at runtime, where the description is factually wrong after the change (e.g. "sends files X, Y" when the code sends X, Y, Z). This matters because operators and future reviewers rely on it to use and audit the feature.
- Safety guarding — a documented safety / permission / authorization contract vs. the actual guard in code (e.g. ARCHITECTURE says "panic kills all subprocess trees" but the implementation misses process groups).
- Frozen contracts (v1) — the ABI under
ouroboros/contracts/(ToolContextProtocol,ToolEntryProtocol,SkillManifest,schema_versions) plus the browser gateway contract inouroboros/gateway/contracts.py(canonical HTTP/WS envelope and endpoint index; theouroboros/contracts/api_v1.pycompatibility re-export was removed in ABI 7.0 — an import of the old name fails at load time). Removing a field, renaming a TypedDict key that the runtime already emits, removing an endpoint token that the router still mounts, or breaking theparse_skill_manifest_texttolerance contract is critical, because external skills/extensions and the frontend boundary are expected to pin against this surface. Non-breaking additions are not critical. The regression suites aretests/test_contracts.pyandtests/test_gateway_parity.py.
All OTHER mismatches are advisory, not critical. Including:
- Wording of explanatory comments that is imprecise but does not misstate
runtime behaviour of the feature (e.g. comment says "Claude Opus 4.6"
when the resolved model is
openai/gpt-5.5-pro; the comment is stale but the runtime is fine — advisory). - Stylistic inconsistency between changelog entries, commit-message wording that doesn't literally match the code in every respect, descriptive prose in README intro sections, "N fixes" narrative summaries, formatting of bullet points.
- Documentation that is merely verbose or redundant rather than wrong.
Reviewers MUST apply this whitelist before escalating any prose-level mismatch to critical. If in doubt, advisory.
Loop / state-machine changes
When the diff changes ouroboros/loop.py, task finalization semantics, checkpoint/audit rounds,
or other state-machine behavior, reviewers MUST verify adversarial paths — not only the happy path.
At minimum, check for:
- malformed or empty model output
- false task completion / premature finalization
- replay durability in logs/history
- visible anomaly path when structured output is missing or broken
A state-machine change that only passes the success-path test is incomplete.
Skill Review Checklist
Used by skill_review to vet a single
external skill before it is allowed to execute via skill_exec. This uses the
shared triad reviewer-slot configuration (OUROBOROS_REVIEWER_SLOTS, with the
legacy model/route settings read when absent) and follows every row's configured
delivery, while preserving the existing gate semantics against a skill package in the local
checkout of OUROBOROS_SKILLS_REPO_PATH, not against a staged git diff.
Transport and control skills are first-class (binding for ALL reviewers)
A reviewed, token-bound, enabled, owner-granted, owner-bound transport/control skill — a chat bridge like Telegram today, or any future remote-control skill — is a first-class control surface: a legitimate full replacement for the local UI, not a demo or a toy. Owners may have no screen, no notebook, and no terminal; the control skill is how they run Ouroboros. Such a skill is therefore expected to:
- carry every owner command the local UI accepts, including
/panic,/restart,/evolve,/bg,/review,/status, and free-form owner text; - run a long-lived poller /
supervised_taskto receive inbound traffic; - subscribe to the owner-conversation events it needs to mirror.
Power is not a defect. Reviewers MUST NOT raise a finding because a control skill can issue owner commands, runs a long-lived poller, or observes owner-conversation events — those are its purpose. Reviewers score the skill's actual safety properties instead:
- owner/chat binding (or an equivalent access rule) so only the owner drives it;
- trustworthy source attribution (the host stamps
source=skill:<name>); - bounded polling / backpressure and no unbounded restart loop;
- cleanup on unload/panic so the Emergency Stop invariant always holds;
- host-token confinement (loopback only,
use_in_request(), never logged); - no exfiltration of secrets or owner-conversation content to unrelated parties.
A control skill FAILS only for being unsafe on one of those properties,
never for being powerful. The capability itself is already gated by the
host (token auth, fresh executable review, enablement, content-hash-bound
grants) and by core owner/chat binding (server._process_bridge_updates) — not
by withholding control from the skill. Items 9–12 below are scored against these
safety properties, not against the breadth of control the skill exposes.
Scope of a skill review pack:
- The skill's
SKILL.md/skill.jsonmanifest (parsed byouroboros.contracts.skill_manifest.parse_skill_manifest_text). - The body of the
SKILL.md(human-readable instructions). - Every regular file under
<skill_dir>/that the subprocess couldimport/source/readat runtime (the skill runs withcwd=skill_dirso the reviewed/hashed surface must equal the runtime-reachable surface). This includes top-level helpers likehelper.py, manifest-declared scripts outsidescripts/(e.g.bin/run.sh), and manifest-declared extension entry modules (e.g.plugin.py). Hidden files that are NOT VCS/cache metadata (e.g..hidden_helper.py) are hashed + reviewed for the same reason — a skill could stillimportthem. - The manifest's declared
permissionslist, for comparison against what the code actually does. - Any declared
scheduled_tasksentries, for comparison against the code they trigger and thesupervised_taskpermission they require.
What is deliberately excluded from both the content hash and the review pack:
- VCS / package-manager / editor scratch:
.git,.hg,.svn,.idea,.vscode,.tox,__pycache__,node_modules,.DS_Store(silently excluded — a byte-flip in a cache file does not invalidate a PASS review). - Sensitive file shapes HARD-BLOCK the skill:
.envand its explicitly listed runtime variants in the shared_SENSITIVE_NAMESpolicy,.pem,.key,.p12,.pfx,.jks,.keystore,credentials.json,service-account.json,secrets.yaml,secrets.json,.git-credentials,.netrc,.npmrc,.pypirc. (Allowlist reused fromouroboros.tools.review_helpers._SENSITIVE_EXTENSIONS_SENSITIVE_NAMES.) The loader raisesSkillPayloadUnreadableon first discovery and the skill shows up inlist_skillswith a non-emptyload_error— neither reviewable nor executable until the operator renames or relocates the file outside the skill tree. Rationale: silently excluding the file would leave it runtime-reachable viaopen('.env').read(), so a reviewed skill could still exfiltrate credentials the reviewer never saw..env.exampleis ordinary reviewed payload: its bytes remain in the freshness hash and the existing publication scan.
- Symlinks whose targets resolve outside
skill_dir(confinement guard — otherwise a symlink to/etc/passwdwould leak into the review pack sent to external reviewer models).
Binary payloads are judged by CONTENT, not by filename (#447 X4). A
file whose bytes start with a loader magic (ELF, PE, Mach-O incl. the
fat/byte-swapped variants, the host interpreter's exact .pyc
magic) is a hard review blocker regardless of its name or extension:
executable_magic_kind raises SkillBinaryPayload and
skill_review converts that into status="pending" with an
actionable error. WebAssembly (\x00asm) is no longer a loader-magic
hard blocker: a .wasm file is admitted as a content-hash-bound binary
descriptor (below) because it executes only inside the sandboxed widget
frame, never natively in the host process — the reviewer does not read
the WebAssembly bytes (the review pack never inlines them), so judge the
widget JavaScript that instantiates
it and the module's provenance instead. Any OTHER non-UTF-8 file is
neither blocked nor inlined: the review pack carries a typed
{path,size,mime_from_name,sha256} descriptor instead of raw bytes.
Judge such a descriptor on the merits: mime_from_name is guessed
from the FILENAME and is not a content attestation; size/sha256
are exact. An opaque blob is only dangerous through loader code the
review CAN see (ctypes.CDLL / zipimport / require /
sys.path insertion) — flag any code path that loads or executes a
descriptor-only file, and treat an unexplained opaque blob in an
executable position as a finding (the deliberate exception: a .wasm
module that widget JavaScript instantiates inside the browser-sandboxed
widget frame, which never runs in the host process). skill_exec
independently refuses to execute a declared script that is not valid
UTF-8 text, so a blob renamed into the scripts list cannot run.
Media-carrying skills should still prefer fetching binary assets on
demand from a reviewable HTTPS source over vendoring opaque bytes. This
content-judged posture is the current owner decision, revisitable
explicitly; a future sandbox project (out-of-process / WASM) remains
the prerequisite for trusting opaque bytes as EXECUTABLE payload inside
the skill tree — the WASM there is a host-side WebAssembly runtime, a
separate future topic from the browser-sandboxed .wasm above.
Skills default to disabled and cannot be executed by skill_exec
until review produces a fresh executable verdict. Skill review output is persisted
to ~/Ouroboros/data/state/skills/<name>/review.json with a content
hash so an edit to the skill invalidates the previous verdict.
review.json, enabled.json, grants.json, and marketplace/self-authored
provenance are skill trust/control-plane state: they are mutated only
through the review, toggle, launcher-grant, self-authored finalize,
native launcher-seed trust, and marketplace paths, not through generic
agent/browser file writes.
Native launcher-seed trust (v6.31.0) is a named, hash-pinned, audited
exception to manual first review: when the LAUNCHER itself writes a
bundled native skill payload (bootstrap seed, post-bootstrap new seed,
or version resync — all marked by .seed-origin), it stamps
review.json with status=clean, reviewer_models=["repo_commit_gate"],
and review_profile="native_seed", because those exact payload bytes
already passed the repo triad+scope commit gate. The verdict is bound to
the post-seed content hash (lifecycle control files excluded), so ANY
later edit flips it stale and non-executable exactly like an ordinary
review; removing .seed-origin reclassifies the skill as user-managed.
Zero-grant native seeds (no secret keys, no privileged permissions, only
tool/subprocess surface) also auto-enable — but only when no explicit
owner enable/disable choice exists yet; a version resync never overrides
an owner's disable. The verdict is additionally bound to the marker at
LOAD time: a native_seed review whose .seed-origin is gone reads back
as pending (non-executable). The owner opt-out is
OUROBOROS_TRUST_NATIVE_SEEDED_SKILLS=false; the trust never extends to
clawhub/external/self-authored skills. Packaged installs bind native-seed
trust to the SHA-pinned repo.bundle; source-mode installs copy current
worktree bytes and therefore lack that packaged-byte provenance.
skip_advisory_review changes only advisory coverage: repo triad and
applicable scope review still run.
Self-authored skills carry payload-local .self_authored.json and
owner-state data/state/skills/<skill>/self_authored.json provenance,
but they do not bypass review on the agent's own initiative. skill_review routes them
through the same tri-model skill review as marketplace and user-managed
skills; no deterministic PASS or enablement is written automatically.
EXCEPTION (C1, v6.39; narrowed hub extension in v6.43 — owner attestation): the OWNER
may explicitly skip the EXPENSIVE LLM review for their OWN skill (source=external or
self-authored) or for a hash-verified official OuroborosHub payload (fresh sidecar +
live-catalog hash match, no extra runtime-reachable files) via the owner-only
POST /api/owner/skills/<skill>/attest-review. Native, ClawHub, and unverified
OuroborosHub payloads are not attestable. The DETERMINISTIC preflight floor still runs
(409 on failure); only the LLM phase is skipped. The result is a durable clean verdict
with review_profile=owner_attested, reviewer_models=[owner_attestation], bound to
content_hash (a content edit stales it) and valid only while the owner-issued
owner_attestation.json marker is present. The marker remains owner state, blocked from generic agent file writes and raw
shell/CLI/browser endpoint self-calls. An ordinary task may carry an already
expressed owner instruction through skill_owner_action; the shared host owner
checks the actual caller, resolved member source, selected revision and source
eligibility before invoking this same attestation. The model interprets the
owner's words; a formatted reference alone grants nothing. This remains the
only owner-issued review bypass.
OUROBOROS_AUTO_GRANT_REVIEWED_SKILLS is default-on as of v6.10.0 (the owner
may disable it), in which case a fresh executable review grants only
manifest-declared settings keys and host permissions for that exact content
hash; when disabled, key and permission grants remain explicit.
The Skills UI Repair affordance and automatic repair both start ordinary managed
development tasks. Keep the selected skill, source request and admitted payload
revision; a legacy task_constraint.mode="skill_repair" record remains readable
as a selected-resource binding, never a reduced execution profile. Ordinary file,
shell, browser, delegation and functional skill operations remain available
through their existing guards. Review must not force-unload a skill because the
caller is repairing it. Preserve readonly and acting-child ceilings independently.
Before a selected payload operation, verify the task's known revision. File-tool
effects retain their existing attribution; after opaque process work, persist the
observed payload revision without claiming exclusive authorship. A known foreign
change refuses overwrite. No long shell lock, mandatory private Git copy, new
patch pipeline or automatic rollback is implied. Native seeds with .seed-origin
retain their existing protections. Ordinary user-managed native-directory payloads
without that marker retain the existing logical-external binding. Generic tools
cannot write provenance, review, grants, dependencies or other control state.
UI, CLI and task calls share each existing lifecycle effect owner. An owner-only action names its exact action, selected skill and revision, requested grant items, and source of expressed owner intent. The host validates actual caller identity, source membership/provenance and current manifest/review/dependency restrictions; the acting model interprets the source's meaning. A formatted reference or an automatically generated repair request is not owner permission. Ordinary repair does not grant all permissions, self-attest or authorize deletion. Preserve the configured auto-grant policy, ClawHub no-Skip rule, and explicit owner disable/Stop. Do not require repeated owner clicks for operations already covered by the task.
Keep review, grants, desired enablement, dependency readiness and loaded/tested revision as separate facts. A dependency or load failure does not rewrite a review verdict, and a functional test is not independent review PASS. Reconcile supported owner attestation through the same post-review lifecycle. Project the actual blocking phase and available next action; repeat review only when it addresses that blocker. Verify the real installed script/tool/HTTP/widget/companion flow and preserve an honest partial result if a required permission or runtime is unavailable.
The direct/read-only inspection profiles retain their existing native read/list/ search contract; ordinary Repair does not grant owner-state access or widen unrelated child/Presence authority.
A selected skill_publish task may recover one exact known manifestless
user_repo leaf through the existing selected-candidate binding: omitted
bucket permits inspect operations and creation only of root SKILL.md or
skill.json, after which ordinary discovery and fresh review resume. Reviewers
must verify that grouping roots, unknown/colliding identities, explicit wrong
buckets, nested manifests, and path escapes remain refused.
Output contract
Reviewers return a JSON array covering every item below (16 items total).
Each entry carries item, verdict (PASS/FAIL), severity
(critical/advisory), and reason. If one item has multiple distinct
concrete problems, reviewers may return multiple FAIL entries for that
same item; each distinct root cause must stay visible. If an item has no
problems, return one PASS entry. Do not return duplicate PASS entries,
and do not return PASS for an item that also has a FAIL — the concrete
FAIL is authoritative.
Checklist items
| # | item | what to check | severity when FAIL |
|---|---|---|---|
| 1 | manifest_schema | Does the manifest parse cleanly? Does type match the actual payload (instruction = no scripts/entry; script = at least one entry in scripts; extension = non-empty entry)? Is runtime one of python/python3/node/bash/deno/ruby/go for type: script (empty "" is allowed ONLY for type: instruction since instruction skills never execute; extension entries are Python plugin.py modules)? Is timeout_sec > 0? | critical |
| 2 | permissions_honesty | Do the declared permissions match what the scripts actually do? Missing permission declaration for an effect the code performs is a concrete FAIL. Examples: net must be declared if any script uses httpx/requests/socket/urllib; fs must be declared if a script writes outside the skill state dir; subprocess must be declared if a skill spawns another process. A transport using the presence permission must submit only authenticated provider event facts plus an opaque owner-created binding; supplying prompt text, a profile, tools, roots, destinations, authority hashes, or owner commands as trusted fields is a concrete FAIL. | critical |
| 3 | no_repo_mutation | Does any script attempt to write to the self-modifying Ouroboros repo (~/Ouroboros/repo/)? Import of write_file/commit_reviewed against the system repo, git add/git commit, or any path that starts with OUROBOROS_REPO_DIR / ~/Ouroboros/repo is a concrete FAIL. Skills may only propose patches by returning artifact bundles; commits go through the first-party reviewed path. | critical |
| 4 | path_confinement | Do scripts stay inside the skill directory and the dedicated state dir (~/Ouroboros/data/state/skills/<name>/)? Absolute paths, .. traversal, and writes to arbitrary user home subdirs are concrete FAIL. Reading from outside the skill dir is OK for read-only lookups (e.g. system info), write-path confinement is the strict rule. | critical |
| 5 | env_allowlist | Is env_from_settings a short, justified list of settings keys? Core keys in FORBIDDEN_SKILL_SETTINGS (OPENROUTER_API_KEY, OPENAI_API_KEY, OPENAI_COMPATIBLE_API_KEY, CLOUDRU_FOUNDATION_MODELS_API_KEY, GIGACHAT_CREDENTIALS, GIGACHAT_PASSWORD, ANTHROPIC_API_KEY, MINIMAX_API_KEY, DEEPSEEK_API_KEY, GITHUB_TOKEN, OUROBOROS_NETWORK_PASSWORD) may be declared only when the skill genuinely needs that provider/token for its stated purpose; runtime forwards them only after a fresh executable review and a content-bound desktop-launcher owner grant. v5.2.2 dual-track grants: both type: script skills (forwarded by _scrub_env) and type: extension skills (forwarded by PluginAPIImpl.get_settings) are eligible; type: instruction skills cannot receive core keys. Mark unjustified core-key requests or non-forbidden secrets unrelated to the purpose as FAIL. An empty list is the default and always fine. | critical |
| 6 | timeout_and_output_discipline | Is timeout_sec reasonable for the stated workload (default 60, hard cap 300)? Do scripts print to stdout in chunks that the runtime can cap, rather than streaming unbounded output? Unbounded loops without a break/timeout path are a concrete FAIL. | advisory |
| 7 | extension_namespace_discipline | type: extension only: does the extension register its tool/route/ws-handler/ui-tab under the namespace derived from its name (e.g. provider-safe tool/ws names like ext_<len>_<token>_<surface>, route /api/extensions/<name>/…)? Tool and WS short names must be alphanumeric/underscore and at most 24 characters. Namespace collisions with built-in surfaces are a concrete FAIL. If the extension uses api.send_ws_message, are emitted event names short/provider-safe and paired with reviewed host-owned widget subscription components rather than arbitrary same-origin JavaScript? If the extension declares streaming UI, is it a reviewed extension route consumed by a host-owned stream component? A reviewed module widget may also consume the skill's own routes (including streaming responses) and the skill's namespaced WebSocket events through the host-mediated bridge (OuroborosWidget.fetch / OuroborosWidget.onEvent), which is not arbitrary same-origin JavaScript. If the extension owns background resources (threads, sockets, EventSource clients, subprocesses), does it register cleanup with api.on_unload(callback)? If the extension declares a widget render block, is it one of the host-owned schemas (iframe, module, or declarative v1: forms/actions, markdown/code, JSON/kv/table, tabs/chart, stream/subscription, progress/poll, file/gallery/media, map/calendar/kanban, group/metric/callout), with media sourced from extension routes or safe data URLs and no arbitrary same-origin JavaScript? Nested interactive group/tab children must use stable identity and one host-owned lifecycle, while subscription.render stays transitively passive. For non-extension skills, verdict PASS with reason "Not applicable — type != extension." | severity-driven for applicable extensions |
| 8 | widget_module_safety | v5.7.0+. kind: "module" widgets only. The host fetches reviewed widget.js through GET /api/extensions/<skill>/module/<entry>, embeds the source into a sandboxed opaque-origin <iframe srcdoc sandbox="allow-scripts allow-pointer-lock allow-downloads" allow="autoplay; fullscreen; clipboard-write"> with no allow-same-origin — document.cookie, localStorage, and sessionStorage throw SecurityError there by construction and need no source review — and injects a parent-mediated fetch bridge that rejects paths outside the owning skill route prefix. Reviewers confirm at the source level what the sandbox cannot: (a) no fetch/XMLHttpRequest URL outside /api/extensions/<skill>/ and no bespoke postMessage protocol to window.parent beyond the host bridge; (b) the declared launch policy render.start (SSOT ouroboros/extension_ui_validation.py::WIDGET_START_MODES; see CREATING_SKILLS "Launch policy") fits the widget's weight — auto only for a cheap instrument, manual for a program that should not run all the time, retain only for a program that genuinely must keep running while the owner is elsewhere and stays cheap while hidden; (c) a widget with state worth keeping registers window.__ouroWidgetOnDispose(fn) (never assigns over it) and saves that state through the skill's own routes, because the frame is disposable. Acceptable interactions: fetch('/api/extensions/<skill>/...') (through the host bridge), window.OuroborosWidget.fetch('/api/extensions/<skill>/...'), and host-supplied data attributes. Mark non-module widgets and non-extension skills PASS with reason "Not applicable". | severity-driven when kind=module |
| 9 | inject_chat_minimization | Does any use of the inject_chat permission have a narrow, user-facing transport purpose? The Host Service enforces token auth, skill-source attribution, rate limits, in-flight limits, fresh executable review, enablement, and explicit content-hash-bound grants. Reviewed chat transports may carry the same raw owner text as direct chat, including slash commands such as /panic, /restart, /review, /evolve, /bg, and /status; reviewers must evaluate whether the transport itself is authorized, attributable, bounded, and user-facing rather than treating slash-shaped text as automatically forbidden. A skill that accepts external inbound traffic must still show local defense-in-depth appropriate to its transport: owner/chat binding or an equivalent access rule, bounded polling/backpressure, and no unaudited broadcast to unrelated parties. Missing local defense-in-depth is a concrete FAIL for network transports. Mark PASS with reason "Not applicable" when inject_chat is not declared. | critical |
| 10 | event_subscription_minimization | Are subscribe_event and subscribe_events limited to the minimum host event topics required by the skill? chat.outbound, chat.typing, chat.photo, chat.video, chat.document, and chat.links expose owner/agent conversation data (including delivered file bytes and outbound link actions) and require explicit justification. Wildcards, undeclared topics, or forwarding subscribed chat content to unrelated external services are concrete FAILs. Mark PASS with reason "Not applicable" when subscribe_event is not declared. | critical |
| 11 | companion_process_safety | For companion_process / supervised_task skills: is every command declared as an argument list (not shell string), using an allowlisted runtime, with no writes outside skill_dir / state_dir, no unbounded restart loop, and cleanup on unload/panic? Does the process avoid inheriting secrets except through reviewed env_from_settings grants? Mark PASS with reason "Not applicable" when no long-lived process/task is declared — a transient subprocess.run/subprocess.Popen invocation of a build tool like ffmpeg, ImageMagick, or git inside a normal request handler is NOT a long-lived companion process and does not trigger this item (its safety belongs under items 4 / 6 / 13). | severity-driven when applicable |
| 12 | host_token_handling | If the skill calls the Host Service API, does it use the provided SkillToken.use_in_request() only at request construction sites, avoid logging/serializing tokens, and keep all host-service calls on the loopback endpoint? Printing, persisting, exfiltrating, or embedding the token into user-visible output is a concrete FAIL. Mark PASS with reason "Not applicable" when the skill does not access the Host Service API. | critical |
| 13 | error_handling | Does the skill surface actionable errors instead of swallowing exceptions, returning success on partial failure, or leaving users to inspect raw logs manually? Are retry/backoff paths bounded and purpose-specific? | advisory |
| 14 | integration_preflight | Does the skill include cheap local preflight checks for the APIs/files/runtimes it depends on before spending provider budget or starting long work? Missing preflight for fragile external integrations is an advisory FAIL. | advisory |
| 15 | bug_hunting | Are there obvious runtime bugs in reviewed code: wrong filenames, mismatched manifest script names, missing imports, impossible arguments, JSON/schema mismatches, blocking calls in async handlers, or untested happy-path assumptions? For every FAIL, cite the concrete runtime bug and state how you propose to fix it (file/symbol/change), so the author can apply the correction instead of guessing. Concrete likely runtime breakage should use severity=critical; latent issues, provider fragility, minor cleanup, or dead-code concerns should use severity=advisory. | severity-driven |
| 16 | completion_notification | For long-running or user-visible work, does the skill emit or document a completion/failure notification path (for example a host event, events.jsonl append, or clear stdout marker consumed by Ouroboros)? Mark PASS with reason "Not applicable" for tiny synchronous utilities. | advisory |
Severity rules
- Skill review verdicts are enforcement-independent:
clean— no FAIL findings.warnings— one or more advisory FAIL findings, no blocker findings.blockers— one or more critical/blocker FAIL findings.pending— no reliable completed review verdict.
- Enforcement maps verdicts to execution:
OUROBOROS_REVIEW_ENFORCEMENT=blocking:cleanandwarningsare executable;blockersare not.OUROBOROS_REVIEW_ENFORCEMENT=advisory:clean,warnings, andblockersare executable by operator choice. This changesexecutable_reviewonly; it does not rewrite the verdict, suppress findings, or changeskill_review_statussemantics.pendingand stale reviews are never executable.
- Review state stores findings and computes the verdict at load time. Agents
and UI callers must use
review_gate.executable_review/executable_review, not the raw status string, when deciding whether the skill is runnable. - A deterministic
skill_preflightFAIL is a structural gate failure, not an LLM verdict: it persists and aggregates topending, which is non-executable under EVERY enforcement mode (advisory included) and in every readiness/execution caller — the strongest fail-closed outcome, stronger than an overridable blocker. - Hard trust-boundary items are blocker findings on any FAIL regardless of
reviewer-supplied severity:
manifest_schema,permissions_honesty,no_repo_mutation,path_confinement,env_allowlist,inject_chat_minimization,event_subscription_minimization, andhost_token_handling. - Items 7, 8, 11, and 15 are severity-driven. A
FAILwithseverity=criticalproducesblockers; aFAILwithseverity=advisoryproduceswarnings. Reviewers MUST reserve critical severity for concrete dangerous or runtime-breaking cases that meet the Critical threshold rule below. - Item 8 (
widget_module_safety) applies to module widgets. Reviewers MUST mark it PASS with reason "Not applicable" when the extension does not use a module widget. This runtime rule deliberately does not rely only on manifestui_tabdetection because extensions can register module widgets dynamically fromplugin.pyviaPluginAPI.register_ui_tab. - Items 9, 10, and 12 are critical only when their corresponding capability is declared or used. Reviewers MUST mark them PASS with reason "Not applicable" for skills outside that surface.
Critical threshold rule (applies to ALL items)
Before marking any skill item CRITICAL you MUST be able to answer YES to ALL of:
- I can name the exact file, symbol, function, or manifest field inside the reviewed skill package that makes this problem live RIGHT NOW.
- That artifact actually appears in the file pack or manifest I have been given (not a hypothetical future use the skill might grow into).
- The fix requires a change to the skill payload or manifest — not a follow-up task on the host or a speculative "the author might one day add X" guard.
If you cannot satisfy all three, use advisory, not critical.
One root cause = one FAIL entry. Do NOT split one underlying problem into multiple FAIL items that all require the same change. If the same finding already has a documented accepted rebuttal in the prompt (see "Previously accepted rebuttals"), do NOT re-raise it without new evidence — the rebuttal section is binding guidance, not background reading.
Marketplace-installed skill review (ClawHub provenance)
When a skill's directory carries a .clawhub.json provenance sidecar,
its source is the ClawHub marketplace (v4.50). The review pack will
also contain a SKILL.openclaw.md file — that is the original
publisher-authored manifest, preserved by the marketplace adapter
(ouroboros/marketplace/adapter.py) before it wrote the translated
SKILL.md that the runtime executes. Reviewers MUST cross-check the
two manifests as part of items 2 (permissions_honesty) and 5
(env_allowlist) without adding extra JSON checklist entries:
- Permissions parity — confirm the translated
permissionslist captures every effect the originalmetadata.openclaw.requires.bins/allowed-tools/ scripts imply. A subprocess-spawning publisher that translates to an emptypermissions: []is a concrete FAIL of item 2 (permissions_honesty). - Env key honesty — denylisted/core keys from
metadata.openclaw.requires.envbecome explicit key-grant requirements, not automatic environment access. Ifenv_from_settingsis non-empty and any listed key does not appear in the originalmetadata.openclaw.requires.env, that is a concrete FAIL of item 5 (env_allowlist) — the adapter is fabricating a permission the publisher never asked for. - Install spec policy (v5.7.0+) — the adapter NORMALISES
metadata.openclaw.installspecs into Ouroboros's isolated per-skill dependency lane.pip/pipx/uv/npm/nodespecs land indata/skills/<bucket>/<skill>/.ouroboros_env/; npm defaults to--ignore-scriptsand pip to--only-binary=:all:. A reviewed entry may explicitly opt into source builds/install scripts with a concrete executable check. Exactdownloadentries bind URL, digest, size and relative target; any literal build steps also declare outputs and a check. These declarations must match the fresh reviewed payload at process launch; package-manager exit zero alone is not a functional verdict. Verified caches stay outside replaceable payload/env, and actual resource/package/output facts and diagnostics stay in deps.json. Global-manager specs (brew,apt,cargo,go) remain manual setup warnings. Reviewers should confirm the auto-installed packages match the skill's stated purpose; an unjustifiedpip install <package>for a skill that doesn't import it is a FAIL of item 2 (permissions_honesty). The adapter still rejects Node/TS plugin packages outright at the staging step; seeingopenclaw.plugin.jsonin the file pack means the install pipeline should have aborted, which FAILs item 1 (manifest_schema) because the skill should not have landed. v5.8 generalises the same readiness contract to official and local manifests that declare reviewedinstall/dependenciesmetadata: A fresh executable review installs auto specs into.ouroboros_env, and enable/load/exec paths refuse missing, failed, or stale dependency fingerprints. - Plugin packages —
openclaw.plugin.jsonin the file pack means the publisher shipped a Node/TS plugin. The adapter refuses these, so seeing one in a successfully-installed skill is a contradiction and FAILs item 1 (manifest_schema).
The marketplace pipeline writes the provenance audit trail to
data/state/skills/<name>/clawhub.json (slug, version, sha256,
original_manifest_sha256, translated_manifest_sha256, adapter_warnings).
This file is not part of the review pack (it lives outside the
skill directory) but reviewers may reference its existence as
context — its absence on a data/skills/clawhub/... skill would be a
concrete FAIL of item 1 (the skill claims marketplace provenance
without the audit record).
Skill review vs. repo review
These are separate surfaces with separate models, prompts, and state:
- Repo review (triad + scope + advisory) protects the self-modifying
~/Ouroboros/repo/. Its state lives indata/state/advisory_review.jsonand is keyed by staged diff snapshot. - Skill review protects the external skills repo. Its state lives in
data/state/skills/<name>/review.jsonand is keyed by a content hash of the skill's manifest + payload files.
A blocked skill review must NOT create obligations, commit-readiness debt, or any artefact visible to the repo-review pipeline — the two surfaces are deliberately siloed so a sticky skill finding cannot block repo commits and vice versa.
Plan Review Checklist
Used by plan_task to review an INTENTION before the work starts — the same organ whether the
work is code, research, a deliverable, or an action in the world. Reviewers see the agent's typed
SPEC, the task objective, the evidence the agent declared (attached bounded, with every absence
named), and — for a self-modification plan — BIBLE.md and ARCHITECTURE.md in full (inline for
an api reviewer; a retrieving reviewer reads both in full with its own tools, the pack names them
as mandatory reads); every other plan gets the heading-derived navigation maps of BIBLE.md and
ARCHITECTURE.md and may request more with need_evidence (the host attaches it on the next cycle,
or names its absence as an omission row when the locator cannot be resolved).
Judge only the evidence actually present; nothing missing is ever silent.
One question: is this SPEC sufficient to START the work safely? Not "is everything
specified" — details may be worked out while doing. This is also an important brainstorming
mechanism: challenge the premise and suggest simpler, more general alternatives when useful.
Such advice is an optional note; Ouroboros decides whether to adopt it without mandatory
disposition. A premise challenge, preference or repetition alone does not earn blocking
authority. Independently demonstrated failures still follow the height rule below. No
compulsory competing plan or finding quota: those create endless rewrite/review cycles.
The agent authors the plan (P0); reviewers contribute criticism; the host aggregates and
enforces the actual blocking contract.
The spec you are reviewing
goal · in_scope[] · non_goals[] · acceptance_claims[] (checkable "done" statements)
invariants[] (budget, deadline, safety, irreversibility, external commitments)
decisions[] {choice, rejected[], why} · deferred[] {what, why_safe_to_defer}
affected_resources[] (what the work will CHANGE) · evidence[] (what to look at)
Every element has a host-minted id (goal, claim_N, invariant_N, decision_N, deferred_N).
Those ids are the only valid breaks targets. Ids may shift between cycles when the agent
rewrites the spec — re-target breaks against the CURRENT ids using the Spec delta.
The rubric (five domain-free questions + one for self-modification)
| # | item | what to check |
|---|---|---|
| 1 | success conditions | Are the acceptance claims checkable — could a third party tell whether each one holds? |
| 2 | load-bearing decisions | Are the decisions that are expensive to reverse explicit, with their rejected alternatives and why? |
| 3 | constraints and invariants | Are the real constraints named — budget, deadline, safety, irreversibility, commitments to others? |
| 4 | deferrals | Is anything deferred that will be expensive to change once the work has started? |
| 5 | evidence sufficiency | Is the evidence enough to judge? If not, ask for exactly what is missing (need_evidence with a locator) instead of inventing a gap. |
| 6 | governance (self-modification plans only) | Does the intention contradict BIBLE.md or a frozen contract? Name the principle or contract. |
Height rule — what may block
A finding is blocking only if being wrong about it AFTER the work starts would invalidate work
already done, violate a declared commitment, or make an acceptance claim unverifiable — and it
MUST name the spec id it breaks. Everything else is a note. If missing evidence makes a claim
structurally unverifiable, that is blocking against the claim, not a need_evidence request.
blocking— requiresbreaks: <spec id>. Without a valid id the host demotes it to a note and discloses the demotion.note— optional advice retained in the review; no disposition is required to proceed.need_evidence— a typed request{locator, why}. It never blocks by itself and the same locator is remembered only once per task; repeating a valid request or filling the bounded request memory does not turn it into optional advice. It retains its free disposition, without another remembered locator or paid call. The host attaches a remembered locator on the next cycle (through the same evidence policy), so the agent's next envelope carries it — a new fingerprint, i.e. a paid cycle that actually has the evidence — or, when the locator cannot be resolved, carries a named omission row for it, so the panel judges the absence instead of waiting for a source that will not arrive.
Output
Return ONLY a JSON array of findings (optionally one code fence); for nothing to report return the
empty array followed by NO_FINDINGS. Each element:
{"id": "f1", "class": "blocking|note|need_evidence", "breaks": "<spec id>",
"locator": "<path|url|task:id>", "summary": "...", "recommendation": "..."}
There is no reviewer-authored aggregate line: the HOST computes the aggregate from the findings
across all configured slots using config.adaptive_quorum(N). A reviewer never emits GREEN as
authority, and prose outside the array is not parsed.
Cycles and closure
- GREEN — no findings. Proceed.
- REVIEW_REQUIRED — notes /
need_evidence, or a blocking finding BELOW quorum. A note-only wave closes immediately in either enforcement mode. The agent closes outstandingneed_evidencewith a disposition (accept / reject with rationale / defer) — no new panel, no cost; notes do not need entries. Voluntary dispositions on current closed note-only waves remain available through the same call, without reopening or a paid cycle. A below-quorum blocking finding stays OPEN whatever the disposition says: it closes only through a changed spec (a new fingerprint, the next paid cycle) or a reject the next paid delta cycle judges. - REVISE_PLAN — blocking findings at quorum. A disposition can never close it: the agent either changes the spec (a new fingerprint, the next paid cycle) or rejects a blocking finding with a rationale that rides into that next cycle, where reviewers mark it resolved or still open.
- DEGRADED — no parseable quorum. Not a verdict, but the dispatched panel PAID its cycle:
the wave records OPEN with each slot's typed failure state (code and reset time when known),
the control line reports DEGRADED honestly, and the recorded result replays for free ONLY
under all three conditions — an identical envelope, a NON-EMPTY recorded structural
lane-health epoch that a fresh snapshot still matches, and an unchanged reviewer roster
(slot ids, targets, routes, pinned profiles and EFFORTS). An empty-epoch DEGRADED wave
(slots died at dispatch time, no structural snapshot evidence) re-dispatches a PAID panel
on the identical envelope; so does a healed or newly dead lane or a changed roster. Only a
wave in which no reviewer slot was physically dispatched (typed $0 skip rows only —
pre-fan-out health skips included) stays unpaid.
When the wave's typed rows prove the quorum STRUCTURALLY unreachable, the wave carries
quorum_unreachable+ the earliest reset: under blocking the finalization gate releases for an agent-chosen honestblocked_with_evidenceterminal (review stays open, implementation stays held), waiting via a one-shotschedule_followupand asking the owner stay open too.
Paid cycles per task are bounded by the owner's OUROBOROS_REVIEW_MAX_CYCLES (default 2,
unlimited available). Replaying an identical envelope is free — identical including the
evidence the host attaches for reviewers' need_evidence requests, so a request received in the
last cycle makes the next envelope a new one. On cycle 2+ every reviewer sees
all reviewers' findings from the previous cycle, the agent's dispositions and the spec delta:
a reformulation of an earlier finding is not a new finding, and a new blocking finding must say
why it was invisible before. When the cap is spent under blocking enforcement the host holds
implementation and escalates with the typed review_cycles_exhausted reason; under advisory the
agent may proceed with the wave open under a loud host disclosure.
Rules for reviewers
- Do not name files, functions or modules as a finding unless naming them is what breaks a spec id;
a file-level observation without a
breaksid is a note. - Do not penalise missing tests, version bumps, changelog rows or doc updates — there is no code yet, and the commit gate reviews those at commit time.
- Do not require the plan to specify what can safely be decided while doing the work. "Unspecified" is only a finding when leaving it open is expensive to reverse.
- A plan with no file paths at all (a trip, a deck, a research question) is a first-class subject: the same questions apply, and repository conventions are irrelevant to it.
Intent / Scope Review Checklist
Used by the Atlas-backed scope reviewer, which runs IN PARALLEL with the triad diff review. Unlike triad reviewers who see only the diff, the scope reviewer sees touched files plus a Generated Scope Atlas that accounts for the ENTIRE repository. Its unique advantage is finding cross-module bugs, broken implicit contracts, and hidden regressions that diff-only reviewers cannot see.
Output contract (v4.34.0): the scope reviewer returns a JSON array that covers every
item below (8 items total). PASS entries are mandatory for items with no problems and must
carry 1–2 sentences of justification naming a concrete artifact or code path that was
actually checked — a bare "PASS" or single-word reason is treated as a reviewer failure.
Multiple FAIL entries for the same item are valid when they describe distinct concrete
root causes; do not merge unrelated scope bugs into one summary. Do not emit duplicate
PASS entries, and do not emit PASS for an item that also has a FAIL. See the
Anti pattern-lock guard section of the scope prompt in ouroboros/tools/scope_review.py
for the second-pass requirement when a single FAIL is surfaced. The commit gate still
forwards only verdict == "FAIL" entries; the PASS rows exist so that coverage and the
reviewer's actual reasoning are auditable in scope_raw_result. The scope
pipeline validates this coverage contract before classifying findings: missing
required items, unexpected items, duplicate PASS rows, or PASS+FAIL for the same
item fail closed as reviewer output failures rather than being treated as a
clean response.
| # | item | what to check | severity when FAIL |
|---|---|---|---|
| 1 | intent_alignment | Does the staged change actually fulfill the intended transformation, not merely touch related files? | critical if the incompleteness is concrete and evidenced; otherwise advisory |
| 2 | forgotten_touchpoints | Are there specific coupled files, tests, prompts, docs, configs, or sibling paths that must also change? Name the exact file(s) or symbol(s). | critical if a required touchpoint is concretely omitted; otherwise advisory |
| 3 | cross_surface_consistency | If behavior changed, are adjacent surfaces still consistent: prompts, docs, comments, tool descriptions, automation, or user-visible workflow? Apply the shared Critical surface whitelist — only release metadata, tool schema, module map, behavioural documentation, or safety contracts count as critical; commentary and prose mismatches are advisory. | critical if the mismatch is in a whitelisted surface AND concrete; otherwise advisory |
| 4 | regression_surface | Does wider repository context show a concrete sibling path, migration edge, or parallel flow that remains broken or incomplete after this change? | critical if it leaves a concrete broken/incomplete path; otherwise advisory |
| 5 | prompt_doc_sync | If prompts or docs are relevant to the changed behavior, are they still accurate and mutually consistent? Apply the shared Critical surface whitelist — behavioural documentation describing what a tool/command DOES at runtime is critical; wording/style of comments is advisory. | critical if a whitelisted prompt/doc artifact becomes false; otherwise advisory |
| 6 | architecture_fit | Does the change solve the class of problem, or is it a narrow patch that leaves the underlying pattern unresolved? | advisory |
| 7 | cross_module_bugs | Does this change break something in a different module through implicit coupling, shared state, or assumed call/return patterns? Name the exact module, symbol, or call site. | critical if a concrete cross-module breakage can be cited; otherwise advisory |
| 8 | implicit_contracts | Are there constants, data format assumptions, expected function signatures, or protocol invariants relied upon by OTHER modules that this change violates without updating those callers? Name the exact symbol or file. | critical if a concrete violated contract can be cited; otherwise advisory |
Severity rules
- Any critical FAIL must cite a concrete file, symbol, prompt, doc, test, config, or sibling flow.
- If the reviewer cannot point to an exact touchpoint, the FAIL must be advisory, not critical.
- Scope affects only unchanged code outside the diff. The diff itself remains fully reviewable.
- For narrative / prose / cross-surface findings, apply the shared
Critical surface whitelistdefined in the Repo Commit Checklist section above. Only release metadata, tool schema, module map, behavioural documentation, and safety contracts qualify as critical. Wording of explanatory comments, stylistic mismatches in changelogs, and non-contractual prose are advisory regardless of how concrete the citation is.