Skip to content

Covenant rebuild: MCP contract linter (layers 0-2) + review-ready polish - #1

Merged
Mhemd139 merged 8 commits into
mainfrom
feat/review-ready
Jul 2, 2026
Merged

Covenant rebuild: MCP contract linter (layers 0-2) + review-ready polish#1
Mhemd139 merged 8 commits into
mainfrom
feat/review-ready

Conversation

@Mhemd139

@Mhemd139 Mhemd139 commented Jul 2, 2026

Copy link
Copy Markdown
Owner

What this PR is

Two things in one reviewable diff:

  1. The Covenant rebuild (3 commits, previously unpushed): the repo rebuilt from the demo-first "Warden" proxy into covenant-mcp — a contract linter for MCP servers, in three dependency-ordered layers:

    • Layer 0 — contract core: introspect a server (stdio/HTTP), commit a deterministic baseline (covenant.lock.json), diff live vs baseline, classify every change BREAKING / DEGRADED / COMPATIBLE, exit non-zero in CI. Pure classifier (covenant/diff.py), no I/O.
    • Layer 1 — transparent proxy + quarantine: byte-for-byte JSON-RPC forwarding (SSE passthrough); a tools/call to a quarantined tool is short-circuited with a clean MCP isError. Detection is proxy-owned (POST /covenant/refresh), never dependent on client tools/list timing.
    • Layer 2 — optional Postgres store: durable quarantine, call log, drift events. Best-effort writes — telemetry can never drop traffic.
  2. Review-ready polish (3 new commits):

    • README rewritten — every command/output block was executed and verified before being documented
    • MIT LICENSE (pyproject already declared MIT)
    • CI: ruff + strict mypy + pytest on 3.11/3.12/3.13 with a real Postgres service, plus a dogfood job — Covenant lints its own example server, then injects a real breaking change (COVENANT_DRIFT=1) and asserts exit 1
    • Project CLAUDE.md (invariants + commands) and a covenant-severity skill for the classification model, built test-first (baseline agent: 1/4 correct without it, 4/4 with it)

The severity model (the interesting design decision)

The consumer of an MCP tool is an LLM agent that re-reads the schema every run — not a pinned client. So input-side changes fail loud → DEGRADED (warn), output-side changes fail silent → BREAKING (fail CI / quarantine). Full rationale with named edge cases in docs/superpowers/specs/.

Verification

  • pytest: 86 passed, 3 skipped locally (Postgres tests run in CI via service container)
  • ruff check . + mypy covenant (strict): clean
  • Demo verified end-to-end: clean check exits 0; drifted check flags balance_usd removed as BREAKING, exits 1

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a new MCP contract-linting CLI with snapshot, check, and proxy modes.
    • Introduced drift classification with breaking, degraded, and compatible results.
    • Added an optional proxy quarantine flow and persistent call/drift history storage.
    • Included runnable example servers and demos showing drift detection and restart recovery.
  • Documentation

    • Rewrote project guidance, setup, and usage docs for the new workflow.
    • Added design notes, build logs, and contributor instructions.
  • Tests

    • Added end-to-end and unit coverage for config, contracts, drift, proxy, reporting, and storage behavior.

Mhemd139 and others added 6 commits July 1, 2026 18:03
Replace the Warden proxy prototype with Covenant: a focused, pip-installable
CLI that catches breaking MCP tool-contract changes before they ship.

Layer 0 (contract core):
- introspect an MCP server (stdio or HTTP) and snapshot every tool's
  input/output JSON Schema to a committed baseline (covenant.lock.json)
- diff a live server against the baseline and classify each change as
  breaking / degraded / compatible, exiting non-zero for CI
- classifier models the consumer as an LLM agent that re-reads the schema
  each run: input-side changes fail loud (degraded), output-side changes
  fail silent (breaking); the design quarantines only silent failures
- handles nested objects (dotted paths), arrays, the type-union matrix, and
  a labeled composition punt ($ref/allOf/anyOf/oneOf)

Ships an example FastMCP server with a drift lever for an end-to-end demo,
the Layer 0 design spec, and 61 tests. ruff + mypy(strict) clean.

Roadmap (Project.md): proxy+quarantine, Postgres store, probe agent + RAG,
observability, and a K8s operator arrive in later layers.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Turn the linter into a live firewall. A FastAPI async reverse-proxy sits in
front of an MCP server, forwards every JSON-RPC exchange byte-for-byte, and
short-circuits a tools/call to a drifted tool with a clean isError result — so
the agent fails safe instead of reading a silently-wrong response.

- covenant/proxy/quarantine.py: in-memory quarantine store (Layer 2 adds durability)
- covenant/proxy/detect.py: reuses Layer 0 diff.py; only breaking changes
  quarantine (degraded/compatible pass, visible via /covenant/status)
- covenant/proxy/server.py: transparent proxy; Covenant-owned detection on
  POST /covenant/refresh (client-independent) + best-effort in-band tools/list
- `covenant proxy` CLI command; deps behind an optional [proxy] extra so the
  core linter stays mcp/typer/rich only
- example server gains an HTTP mode; examples/demo_layer1.py runs the full live
  beat (agent through proxy fails safe vs silent wrong shape direct)

17 new tests (quarantine/detect/proxy via mocked upstream); the real
client->proxy->server path is verified by the demo script. 78 tests total,
ruff + mypy(strict) clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Give the proxy durable memory without making a database mandatory. A small async
Store interface has two backends: InMemoryStore (default, no deps) and
PostgresStore (asyncpg + JSONB, optional [store] extra). The proxy runs fully
in-memory unless given --database-url / DATABASE_URL.

- persistent quarantine: a broken tool stays blocked across proxy restarts,
  loaded from Postgres on startup (no refresh needed)
- call log: every proxied tools/call recorded (tool, latency, error, blocked);
  GET /covenant/calls exposes it (feeds Layer 4)
- drift events historized as a JSONB audit trail
- recording is best-effort: a store error is logged, never raised into the
  request path (a firewall must not drop traffic on a telemetry hiccup)

docker-compose.yml provides a local postgres:16. 11 new tests: InMemoryStore
(unit), PostgresStore (integration, skipped unless COVENANT_TEST_DB is set),
proxy+store wiring. examples/demo_layer2.py proves quarantine survives a real
proxy restart. 89 tests total, ruff + mypy(strict) clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The old README described the deleted Warden build (warden/, demo/*.ps1,
dashboard) — none of it exists in the tree. Every command and output block
in the new README was run and verified. LICENSE file added to match the
MIT declaration already in pyproject.toml.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gfood job

The test job runs the full suite against a real Postgres service so the
store tests stop being skipped. The contract-check job is Covenant linting
its own example server: a clean check must pass, then COVENANT_DRIFT=1
injects a real breaking change and the job asserts exit 1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… build log

CLAUDE.md records commands, layer boundaries, and the invariants future
sessions must not break. The covenant-severity skill was built RED-GREEN:
a fresh agent scored 1/4 classifying tricky schema changes without it
(REST intuition inverts Covenant's direction principle), 4/4 with it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Mhemd139, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 37209c86-bd3b-452f-a6bf-4f23a5aa547e

📥 Commits

Reviewing files that changed from the base of the PR and between 471e177 and 6042d99.

📒 Files selected for processing (8)
  • README.md
  • covenant.lock.json
  • covenant/cli.py
  • covenant/diff.py
  • covenant/proxy/server.py
  • covenant/store/memory.py
  • docs/BUILD_LOG.md
  • tests/test_diff.py
📝 Walkthrough

Walkthrough

This PR replaces the prior "Warden" implementation with a new "Covenant" package: a contract-drift linter and CLI (snapshot/check/proxy) with deterministic baseline generation, a pure schema-diff severity classifier, an optional FastAPI reverse-proxy quarantine layer, and optional in-memory/Postgres persistence. It also removes legacy Warden modules, demo scripts, and infrastructure, and adds new docs, specs, examples, and CI.

Changes

Covenant Package Rebuild

Layer / File(s) Summary
Core types, errors, config
covenant/_types.py, covenant/errors.py, covenant/__init__.py, covenant/config.py, tests/test_config.py
Adds the JsonDict alias, the CovenantError exception hierarchy, and covenant.toml-based load_config with server-override handling.
Contract model & baseline files
covenant/contract.py, tests/test_contract.py, covenant.lock.json, covenant.toml
Defines ToolContract, deterministic schema_hash, and baseline read/write helpers, plus the committed lock and config files.
Diff classifier & severity rules
covenant/diff.py, tests/test_diff.py, .claude/skills/covenant-severity/SKILL.md
Implements diff_tools/Change classification for field, type, enum, and composition changes across BREAKING/DEGRADED/COMPATIBLE tiers.
Introspection & example server
covenant/introspect.py, tests/test_introspect.py, examples/mcp_server.py
Adds stdio/HTTP MCP introspection and an example server with a COVENANT_DRIFT-controlled breaking schema change.
CLI commands
covenant/cli.py, tests/test_cli.py
Implements snapshot, check, and proxy Typer subcommands with typed error handling and exit codes.
Proxy quarantine & drift detection
covenant/proxy/quarantine.py, covenant/proxy/detect.py, covenant/proxy/__init__.py, tests/test_quarantine.py, tests/test_detect.py
Adds an in-memory Quarantine store and detect() mapping breaking diffs to quarantine reasons.
Proxy server & reporting
covenant/proxy/server.py, covenant/report.py, tests/test_proxy.py, tests/test_report.py
Implements the JSON-RPC reverse proxy enforcing quarantine, status/calls/refresh endpoints, and CLI/JSON reporting.
Store layer
covenant/store/*.py, covenant/store/schema.sql, tests/test_memory_store.py, tests/test_store_postgres.py, tests/test_proxy_store.py
Defines the Store protocol with InMemoryStore and PostgresStore implementations plus schema and tests.
Demo scripts
examples/demo_layer1.py, examples/demo_layer2.py
Adds runnable demos for quarantine-on-drift and quarantine persistence across restarts.
Documentation & specs
README.md, Project.md, CLAUDE.md, LICENSE, docs/BUILD_LOG.md, docs/superpowers/specs/*
Rewrites project docs and adds layer 0/1/2 design specifications.
CI & build config
.github/workflows/ci.yml, pyproject.toml, docker-compose.yml
Adds CI test/drift-check jobs, package metadata, and a simplified Postgres-only compose file.

Estimated code review effort: 4 (Complex) | ~75 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant CLI as covenant.cli.check
  participant Introspect as covenant.introspect
  participant Diff as covenant.diff
  participant Report as covenant.report

  User->>CLI: covenant check
  CLI->>Introspect: introspect(config)
  Introspect-->>CLI: current tools
  CLI->>Diff: diff_tools(baseline, current)
  Diff-->>CLI: changes
  CLI->>Report: exit_code(changes, strict)
  Report-->>User: exit code + rendered report
Loading
sequenceDiagram
  participant Client
  participant Proxy as covenant.proxy.server
  participant Quarantine
  participant Upstream as MCP Upstream

  Client->>Proxy: POST /mcp tools/call
  Proxy->>Quarantine: is_quarantined(tool)
  alt quarantined
    Proxy-->>Client: JSON-RPC isError response
  else not quarantined
    Proxy->>Upstream: forward request
    Upstream-->>Proxy: response
    Proxy-->>Client: relayed response
  end
  Client->>Proxy: POST /covenant/refresh
  Proxy->>Upstream: tools/list
  Proxy->>Quarantine: sync(detect(baseline, live))
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: rebuilding the repo into Covenant’s layered MCP contract linter plus polish.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/review-ready

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

🧹 Nitpick comments (6)
.github/workflows/ci.yml (1)

1-8: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Harden GitHub Actions token/credential handling.

Add an explicit least-privilege permissions: block and disable credential persistence on checkout, per static analysis findings.

🔒 Proposed hardening
 name: CI
 
+permissions:
+  contents: read
+
 on:
   push:
     branches: [main]
   pull_request:
 
 jobs:
   test:
     ...
     steps:
-      - uses: actions/checkout@v4
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
       - uses: actions/setup-python@v5

Apply the same persist-credentials: false to the contract-check job's checkout step (line 44).

Also applies to: 28-28, 41-44

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 1 - 8, The CI workflow currently
relies on default GitHub token permissions and leaves checkout credentials
persisted, so harden the jobs by adding an explicit least-privilege permissions
block and disabling credential persistence in every checkout. Update the
workflow-level permissions for the jobs in the CI config, and apply
persist-credentials: false to the checkout steps in both the main flow and the
contract-check job, using the existing checkout step definitions as the anchors.

Source: Linters/SAST tools

examples/demo_layer2.py (1)

50-50: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Fixed sleep is a flakiness risk for store startup.

A hardcoded 1s sleep to wait for store.connect() + load_quarantine() can race under slower Postgres connections, making the demo flaky. Poll STATUS until it responds instead of a fixed delay.

♻️ Proposed fix: poll for readiness instead of fixed sleep
-    p = subprocess.Popen(
-        [PY, "-m", "covenant.cli", "proxy", "-u", UPSTREAM, "-p", str(PROXY_PORT)], env=env
-    )
-    wait_port(PROXY_PORT)
-    time.sleep(1.0)  # let startup (store.connect + load_quarantine) finish
-    return p
+    p = subprocess.Popen(
+        [PY, "-m", "covenant.cli", "proxy", "-u", UPSTREAM, "-p", str(PROXY_PORT)], env=env
+    )
+    wait_port(PROXY_PORT)
+    _wait_ready(STATUS)
+    return p
def _wait_ready(url: str, timeout: float = 10.0) -> None:
    end = time.time() + timeout
    while time.time() < end:
        try:
            if httpx.get(url, timeout=1).status_code == 200:
                return
        except httpx.HTTPError:
            pass
        time.sleep(0.2)
    raise RuntimeError("proxy store never became ready")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/demo_layer2.py` at line 50, The demo startup wait in demo_layer2.py
uses a fixed sleep, which can race with slower store.connect() and
load_quarantine startup. Replace the hardcoded delay with a readiness poll that
repeatedly checks STATUS until it responds successfully, then continues; add a
small timeout/backoff loop similar to a _wait_ready helper so the demo only
proceeds once the proxy store is actually ready.
examples/demo_layer1.py (1)

29-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate helpers across demo scripts.

wait_port and stop are copied verbatim into examples/demo_layer2.py. Consider extracting a small shared examples/_demo_util.py to avoid drift between the two copies.

Also applies to: 74-81

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@examples/demo_layer1.py` around lines 29 - 37, Both demo scripts contain
duplicated helper logic for wait_port and stop, which should be centralized to
avoid divergence. Extract these helpers into a shared utility module such as
examples/_demo_util.py, then update the existing callers in demo_layer1.py and
demo_layer2.py to import and use the shared functions instead of maintaining
separate copies.
covenant/config.py (1)

42-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Malformed [server]/[baseline] TOML sections could raise an uncaught AttributeError.

If server or baseline in the TOML file is not a table (e.g. server = "oops"), .get() on line 43/44/55 will raise AttributeError rather than a clean ConfigError. Per the covenant/cli.py guideline, CLI errors must be typed CovenantError instances that print one clean line and exit 2 — an AttributeError here would leak as an unhandled exception/stack trace instead.

🛡️ Proposed defensive fix
-    server = data.get("server", {})
+    server = data.get("server") or {}
+    if not isinstance(server, dict):
+        raise ConfigError(f"invalid config {p}: [server] must be a table")
     command = server.get("command")
     url = server.get("url")

Also applies to: 55-55

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@covenant/config.py` around lines 42 - 44, The TOML parsing for the server and
baseline sections can crash with an uncaught AttributeError when those keys are
not tables, so make the config loading path in covenant/config.py defensively
validate the values before calling .get(). Update the code that reads server and
baseline to check for dict-like/table types and raise ConfigError with a clear
message when the section is malformed, ensuring the CLI only emits typed
CovenantError-style failures instead of a stack trace.

Source: Path instructions

covenant/errors.py (1)

14-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Custom ConnectionError shadows the builtin.

Naming it ConnectionError shadows Python's builtin exception of the same name for any module that does from .errors import * or imports both. Since usage elsewhere is explicit (covenant.errors.ConnectionError), this is low risk, but consider a more distinct name (e.g. TransportError) to avoid confusion when catching exceptions broadly.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@covenant/errors.py` around lines 14 - 15, The custom ConnectionError in
errors.py shadows Python’s builtin exception and can cause confusion in broad
exception handling or wildcard imports. Rename the Covenant-specific exception
to a more distinct name such as TransportError, and update any references that
currently use covenant.errors.ConnectionError so the new class name is used
consistently throughout the codebase.
covenant/diff.py (1)

52-107: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Repeated tier = X if out else Y pattern could be a small helper.

The direction-dependent tier selection is repeated ~8 times across _diff_type/_diff_enum. A tiny helper (e.g. _dir(out, out_tier, in_tier)) would reduce duplication without changing behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@covenant/diff.py` around lines 52 - 107, The direction-dependent tier
selection in _diff_type and _diff_enum is duplicated in several branches, so add
a small helper to centralize it and keep behavior identical. Introduce a
local/private helper such as one that takes the out flag plus the output/input
tier values, then use it in the Change construction paths in _diff_type and
_diff_enum to replace the repeated inline ternaries. Keep the existing tier
outcomes and messages unchanged while reducing duplication around the out check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@covenant.lock.json`:
- Around line 1-4: The committed server snapshot in covenant.lock.json is
inconsistent with the configured command and hardcodes a Windows-specific
interpreter path. Update the lock generation for the server entry by using the
exact command defined in covenant.toml (the server command), and ensure the
snapshot records the configured command string rather than the invoking Python
executable path. Re-snapshot the lock via the server-related generation path so
the covenant_version/server/tools baseline stays byte-identical across
platforms.

In `@covenant/cli.py`:
- Around line 91-106: The CLI setup in the command body handles dependency
failures by catching ImportError inline, which bypasses the existing
CovenantError-based error flow. Change the uvicorn/create_app import block and
the PostgresStore import block to raise a typed CovenantError instead, then let
the surrounding except CovenantError as e handler print the single clean message
and exit(2) consistently. Use the same symbols already present in this command
path, including create_app, PostgresStore, and the outer CovenantError handler,
to keep the CLI error contract uniform.
- Around line 25-27: The CLI option definitions in covenant/cli.py use None
defaults for values that are typed as str, which conflicts with strict mypy.
Update the relevant parameter annotations on the option helpers and the command
arguments that consume them, including _server_opt and the database_url-related
definitions, to use str | None or Optional[str] so the signatures match the
default values while preserving the existing Typer behavior.

In `@covenant/diff.py`:
- Around line 52-86: The _diff_type function is classifying mixed
scalar-to-structural unions through the generic union_widened/union_narrowed
branches, which downgrades output-breaking changes. Move the structural-type
check in _diff_type ahead of the added/removed-only union branches so cases like
number to [number, object] are reported as type_changed_structural with breaking
tier for output. Update the affected logic in _diff_type and add a test in
tests/test_diff.py covering the mixed-union scalar↔object/array case.

In `@covenant/proxy/server.py`:
- Around line 64-69: The _safe() helper in server.py only swallows exceptions,
so a slow or hung awaitable can still block the request path. Update _safe(coro)
to enforce a bounded timeout around the awaited store write, keep logging and
swallowing failures, and ensure the quarantine-blocked fast path still returns
promptly. Use the existing _safe symbol (and its callers) as the place to
centralize this best-effort latency guard.
- Around line 72-93: The _list_upstream flow currently waits indefinitely on the
MCP listing call and lets upstream exceptions escape as a raw 500. Update the
_list_upstream function in server.py to wrap the ClientSession/list_tools path
with a timeout, catch upstream failures, and translate them into a 502 response
for /covenant/refresh while preserving the existing lister short-circuit when
app.state.lister is present.

In `@covenant/store/memory.py`:
- Around line 1-5: The `sync_quarantine` behavior in `Store`/`memory.py` is
wiping all statuses instead of matching `PostgresStore` semantics. Update the
`sync_quarantine` implementation so it only removes entries currently marked
quarantined and then adds the `breaking` entries, while preserving any other
values previously set via `set_status`; keep the logic aligned with
`PostgresStore.sync_quarantine` and the in-memory `_status` map.

In `@docs/superpowers/specs/2026-07-01-covenant-layer0-contract-core-design.md`:
- Around line 176-179: The CLI module map is missing the public proxy surface
exposed by covenant/cli.py, so update the layout entry for cli.py to include
proxy alongside snapshot and check. Use the cli.py symbol mapping in the spec to
keep the documented public API accurate for future edits.

In `@README.md`:
- Around line 98-109: The Optional persistence section overstates the guarantee
by saying “every call and drift event is persisted”; update the README wording
to make persistence conditional on store availability and best-effort behavior
instead. Keep the existing examples, but rephrase the description around
Postgres and the store failures note so it clearly says persistence occurs when
the store is available and does not imply absolute durability.

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1-8: The CI workflow currently relies on default GitHub token
permissions and leaves checkout credentials persisted, so harden the jobs by
adding an explicit least-privilege permissions block and disabling credential
persistence in every checkout. Update the workflow-level permissions for the
jobs in the CI config, and apply persist-credentials: false to the checkout
steps in both the main flow and the contract-check job, using the existing
checkout step definitions as the anchors.

In `@covenant/config.py`:
- Around line 42-44: The TOML parsing for the server and baseline sections can
crash with an uncaught AttributeError when those keys are not tables, so make
the config loading path in covenant/config.py defensively validate the values
before calling .get(). Update the code that reads server and baseline to check
for dict-like/table types and raise ConfigError with a clear message when the
section is malformed, ensuring the CLI only emits typed CovenantError-style
failures instead of a stack trace.

In `@covenant/diff.py`:
- Around line 52-107: The direction-dependent tier selection in _diff_type and
_diff_enum is duplicated in several branches, so add a small helper to
centralize it and keep behavior identical. Introduce a local/private helper such
as one that takes the out flag plus the output/input tier values, then use it in
the Change construction paths in _diff_type and _diff_enum to replace the
repeated inline ternaries. Keep the existing tier outcomes and messages
unchanged while reducing duplication around the out check.

In `@covenant/errors.py`:
- Around line 14-15: The custom ConnectionError in errors.py shadows Python’s
builtin exception and can cause confusion in broad exception handling or
wildcard imports. Rename the Covenant-specific exception to a more distinct name
such as TransportError, and update any references that currently use
covenant.errors.ConnectionError so the new class name is used consistently
throughout the codebase.

In `@examples/demo_layer1.py`:
- Around line 29-37: Both demo scripts contain duplicated helper logic for
wait_port and stop, which should be centralized to avoid divergence. Extract
these helpers into a shared utility module such as examples/_demo_util.py, then
update the existing callers in demo_layer1.py and demo_layer2.py to import and
use the shared functions instead of maintaining separate copies.

In `@examples/demo_layer2.py`:
- Line 50: The demo startup wait in demo_layer2.py uses a fixed sleep, which can
race with slower store.connect() and load_quarantine startup. Replace the
hardcoded delay with a readiness poll that repeatedly checks STATUS until it
responds successfully, then continues; add a small timeout/backoff loop similar
to a _wait_ready helper so the demo only proceeds once the proxy store is
actually ready.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c3a50d25-4abe-4aad-a48f-43bf7727aeaa

📥 Commits

Reviewing files that changed from the base of the PR and between d68fb3c and 471e177.

⛔ Files ignored due to path filters (1)
  • docs/demo.png is excluded by !**/*.png
📒 Files selected for processing (75)
  • .claude/skills/covenant-severity/SKILL.md
  • .env.example
  • .github/workflows/ci.yml
  • CLAUDE.md
  • LICENSE
  • Nvidia_Job.md
  • On_Boarding.md
  • Project.md
  • README.md
  • covenant.lock.json
  • covenant.toml
  • covenant/__init__.py
  • covenant/_types.py
  • covenant/cli.py
  • covenant/config.py
  • covenant/contract.py
  • covenant/diff.py
  • covenant/errors.py
  • covenant/introspect.py
  • covenant/proxy/__init__.py
  • covenant/proxy/detect.py
  • covenant/proxy/quarantine.py
  • covenant/proxy/server.py
  • covenant/report.py
  • covenant/store/__init__.py
  • covenant/store/base.py
  • covenant/store/memory.py
  • covenant/store/postgres.py
  • covenant/store/schema.sql
  • demo/_shot.py
  • demo/a2a.ps1
  • demo/a2a_task.py
  • demo/agent.ps1
  • demo/agent_task.py
  • demo/drift.ps1
  • demo/drift_patch.md
  • demo/mcp_probe.py
  • demo/reset.ps1
  • demo/run_demo.md
  • demo/watch.ps1
  • docker-compose.yml
  • docs/BUILD_LOG.md
  • docs/superpowers/specs/2026-07-01-covenant-layer0-contract-core-design.md
  • docs/superpowers/specs/2026-07-01-covenant-layer1-proxy-quarantine-design.md
  • docs/superpowers/specs/2026-07-01-covenant-layer2-contract-store-design.md
  • examples/demo_layer1.py
  • examples/demo_layer2.py
  • examples/mcp_server.py
  • pyproject.toml
  • requirements.txt
  • test_mcp_server/Dockerfile
  • test_mcp_server/__init__.py
  • test_mcp_server/server.py
  • tests/test_cli.py
  • tests/test_config.py
  • tests/test_contract.py
  • tests/test_detect.py
  • tests/test_diff.py
  • tests/test_introspect.py
  • tests/test_memory_store.py
  • tests/test_proxy.py
  • tests/test_proxy_store.py
  • tests/test_quarantine.py
  • tests/test_report.py
  • tests/test_store_postgres.py
  • warden/Dockerfile
  • warden/__init__.py
  • warden/capture.py
  • warden/cli.py
  • warden/dashboard.py
  • warden/db.py
  • warden/drift.py
  • warden/main.py
  • warden/proxy.py
  • warden/schema.sql
💤 Files with no reviewable changes (26)
  • Nvidia_Job.md
  • demo/watch.ps1
  • demo/drift.ps1
  • warden/Dockerfile
  • .env.example
  • demo/reset.ps1
  • warden/drift.py
  • requirements.txt
  • warden/schema.sql
  • demo/agent.ps1
  • demo/mcp_probe.py
  • demo/a2a.ps1
  • warden/capture.py
  • demo/drift_patch.md
  • On_Boarding.md
  • test_mcp_server/server.py
  • demo/run_demo.md
  • warden/main.py
  • warden/proxy.py
  • demo/agent_task.py
  • warden/db.py
  • warden/cli.py
  • demo/a2a_task.py
  • warden/dashboard.py
  • test_mcp_server/Dockerfile
  • demo/_shot.py

Comment thread covenant.lock.json
Comment thread covenant/cli.py
Comment thread covenant/cli.py Outdated
Comment thread covenant/diff.py
Comment thread covenant/proxy/server.py
Comment thread covenant/proxy/server.py
Comment thread covenant/store/memory.py
Comment on lines +176 to +179
diff.py PURE classifier: (baseline_tools, current_tools) → list[Change]
report.py render Changes via rich; --json machine output; exit-code summary
cli.py typer app: `snapshot`, `check`
examples/

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Include proxy in the CLI module map.

covenant/cli.py also exposes proxy, so this layout currently understates the public CLI surface and will mislead future edits.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/superpowers/specs/2026-07-01-covenant-layer0-contract-core-design.md`
around lines 176 - 179, The CLI module map is missing the public proxy surface
exposed by covenant/cli.py, so update the layout entry for cli.py to include
proxy alongside snapshot and check. Use the cli.py symbol mapping in the spec to
keep the documented public API accurate for future edits.

Comment thread README.md
Mhemd139 and others added 2 commits July 3, 2026 00:46
…y guards

Verified findings from the PR #1 review, applied after checking each against
the code:

- diff.py: classify a union that gains/loses a structural type (object/array)
  as type_changed_structural BEFORE the widen/narrow branches. A scalar->[scalar,
  object] output change was under-classified as a degraded union_widened when it
  is a breaking silent read-model break. Adds a regression test.
- store/memory.py: sync_quarantine now clears only quarantined entries and
  preserves other set_status values, matching PostgresStore semantics (the
  docstring promises the same interface).
- proxy/server.py: bound store writes (_safe) and the /covenant/refresh upstream
  re-list with timeouts; refresh returns 502 on upstream failure instead of a
  raw 500. A firewall must not stall traffic on slow telemetry or a hung upstream.
- cli.py: proxy's dependency-missing paths raise typed CovenantError through the
  one error handler (one clean line, exit 2); server/database_url typed str|None
  to match their None defaults.
- covenant.lock.json: re-snapshotted via the covenant.toml command, replacing a
  hardcoded Windows .venv path so re-snapshots are byte-identical cross-platform.
- README: soften the persistence claim to best-effort/when-reachable.

Skipped: the Layer 0 spec's module map 'missing' proxy — proxy is Layer 1 scope,
documented in the Layer 1 spec.

pytest 87 passed / 3 skipped, ruff + mypy(strict) clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Mhemd139
Mhemd139 merged commit 258d3ea into main Jul 2, 2026
5 checks passed
@Mhemd139
Mhemd139 deleted the feat/review-ready branch July 4, 2026 14:58
Mhemd139 added a commit that referenced this pull request Jul 4, 2026
…y guards

Verified findings from the PR #1 review, applied after checking each against
the code:

- diff.py: classify a union that gains/loses a structural type (object/array)
  as type_changed_structural BEFORE the widen/narrow branches. A scalar->[scalar,
  object] output change was under-classified as a degraded union_widened when it
  is a breaking silent read-model break. Adds a regression test.
- store/memory.py: sync_quarantine now clears only quarantined entries and
  preserves other set_status values, matching PostgresStore semantics (the
  docstring promises the same interface).
- proxy/server.py: bound store writes (_safe) and the /covenant/refresh upstream
  re-list with timeouts; refresh returns 502 on upstream failure instead of a
  raw 500. A firewall must not stall traffic on slow telemetry or a hung upstream.
- cli.py: proxy's dependency-missing paths raise typed CovenantError through the
  one error handler (one clean line, exit 2); server/database_url typed str|None
  to match their None defaults.
- covenant.lock.json: re-snapshotted via the covenant.toml command, replacing a
  hardcoded Windows .venv path so re-snapshots are byte-identical cross-platform.
- README: soften the persistence claim to best-effort/when-reachable.

Skipped: the Layer 0 spec's module map 'missing' proxy — proxy is Layer 1 scope,
documented in the Layer 1 spec.

pytest 87 passed / 3 skipped, ruff + mypy(strict) clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mhemd139 added a commit that referenced this pull request Jul 4, 2026
Covenant rebuild: MCP contract linter (layers 0-2) + review-ready polish
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant