Skip to content

Add opt-in free-session budget gate for unauthenticated HTTP traffic - #444

Open
akolotov wants to merge 17 commits into
mainfrom
claude/issue-442-impl-plan-1cf390
Open

Add opt-in free-session budget gate for unauthenticated HTTP traffic#444
akolotov wants to merge 17 commits into
mainfrom
claude/issue-442-impl-plan-1cf390

Conversation

@akolotov

@akolotov akolotov commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

What this changes

Adds an opt-in free-session budget gate for unauthenticated HTTP/REST traffic. When BLOCKSCOUT_SESSION_SECRET is set and the server runs in HTTP mode, an agent must first call __unlock_blockchain_analysis__ / GET /v1/unlock_blockchain_analysis to obtain an opaque session_id, then pass it with every subsequent tool call. Each metered call debits a small per-identifier budget (default 5). Requests carrying a client-supplied Blockscout PRO API key are exempt and unmetered.

The gate is off by default: with no secret set, or in stdio mode, behavior is byte-identical to today — including the absence of the session_id field in the unlock payload.

Closes #442.

Design points worth a reviewer's attention

  • Single terminal budget, not two limits. An expired token and an exhausted budget are deliberately indistinguishable to the agent (one shared SESSION_OVER_MESSAGE) so neither can be probed. This supersedes the two-limit design originally sketched in the issue.
  • Stateless tokens, minimal state. A token is random_part.issued_at.mac, HMAC'd over the random part, the issue time, and a store generation persisted in the database. Verification touches no storage. Replacing the database invalidates every outstanding identifier rather than re-granting budget. Minting writes no row, so an identifier that is never used leaves nothing behind.
  • Fail-closed on store faults, but non-terminal. Any SQLite fault funnels through one chokepoint into SessionStoreUnavailableError503, whose message deliberately omits the "do not retry" markers so agents back off rather than give up.
  • Refund on failure. A debited call that then fails upstream — including on cancellation — refunds the debit; refunds never mask the original exception.
  • Restart survival. Debits and the store generation persist across a clean restart (autocommit + WAL, STRICT tables, RETURNING fully fetched for atomicity under concurrent writers).
  • session_id is redacted at the single log_tool_invocation chokepoint, so it reaches neither logs, analytics, nor telemetry.
  • Parameter description is deliberately minimal ("Opaque session identifier.") — it names no source and no condition, so it creates no pull toward an unlock call for exempt agents or ungated deployments.

Shape of the diff

Three new modules — session_store.py (SQLite store), session_gate.py (tokens, errors, gate predicates, decorators), session_lifecycle.py (gated-startup validation, store init, periodic TTL sweep, composed lifespan) — plus session_id and a gate decorator on 15 tool modules, the REST error mapping and passthrough, and documentation.

mcpb/manifest.json and mcpb/manifest-dev.json are bumped to 0.10.0. Server version files stay at 0.18.0.dev0 — no re-bump within an already-versioned dev cycle.

Post-review hardening: presented tokens are length-capped (512 chars) before any parsing or HMAC work; a misconfigured gated startup exits with a clean one-line error instead of a traceback; and an optional BLOCKSCOUT_SESSION_SWEEP_INTERVAL_SECONDS (default: once per TTL) decouples expired-row sweep cadence from the TTL — cadence never affects correctness, since the deletion cutoff derives from the TTL at call time, so no sweep can remove a live identifier's row.

Testing

  • Unit suite: 1207 passed, 0 skipped.
  • Integration suite (timeout-protected runner): 12 passed, 0 failed, 0 timed out, 80 skipped — all skips are the pre-existing BLOCKSCOUT_PRO_API_KEY not configured environment condition. The suite runs ungated by construction, since the gate requires HTTP mode.
  • ruff check . and ruff format --check . clean.
  • Coverage includes real streamable-HTTP transport tests (not just direct function calls), multi-threaded concurrency against the store, durability across reopen, refund-on-cancellation, and real-lifespan sweep-task fault tolerance.

Not yet performed — the manual cross-process restart E2E from the plan's Phase 9 needs a live BLOCKSCOUT_PRO_API_KEY and was not available in the authoring environment. It should be run before the gate is enabled anywhere: two gated starts against one database, confirming the budget note drops 5 → 4 → 3 across a clean restart, and that deleting the database invalidates the old identifier rather than re-budgeting it.

Production rollout handoff → blockscout/deployment-values

This repository does not deploy itself. Every test here can pass while a rollout still violates the store's topology assumptions or loses state on rollback. The following is release-gating for enabling the gate in production, not for merging this code:

  • Before setting BLOCKSCOUT_SESSION_SECRET: generate the secret once and hold it in the deployment's secret store — every restart and redeploy must reuse the same value, never a per-start command substitution. Mount a persistent volume at the directory of BLOCKSCOUT_SESSION_DB_PATH. Pin the deployment to a single replica with a non-overlapping update strategy (e.g. Recreate) — correctness assumes exactly one writer and a database file private to the running server.
  • Rollback: keep both the volume and the secret when rolling the code back — live identifiers stay valid. Rotating or deleting either invalidates them by design. Emergency disable = unset only the secret (the gate turns off; nothing else changes); re-enabling later with the retained secret and database preserves still-live identifiers.
  • Post-deploy canary: one /v1/unlock_blockchain_analysis call; one metered call with the returned identifier (expect the budget note); one call with a client PRO API key and no identifier (expect no note). Confirm the Session gating: ENABLED startup line. Alert on 503 rates and on "session store failure" ERROR log lines.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added optional session-gated access for HTTP tool endpoints.
    • Unlocking analysis now initializes a session and provides a session ID for subsequent requests.
    • Added configurable call limits, expiration times, persistent session storage, and API-key exemptions.
    • Added remaining-budget notices and secure session-ID redaction in logs and telemetry.
  • Bug Fixes

    • Added clear HTTP responses for missing, invalid, expired, exhausted, or unavailable sessions.
  • Documentation

    • Updated API, deployment, tool, and configuration guidance for session-gated access.

akolotov and others added 11 commits July 30, 2026 16:42
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… limit

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

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 62f3948d-744b-41b8-8fab-e0aea1b97446

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb9f03 and 1c019c9.

📒 Files selected for processing (8)
  • API.md
  • blockscout_mcp_server/session_gate.py
  • blockscout_mcp_server/session_lifecycle.py
  • tests/conftest.py
  • tests/test_server_session_gate_lifespan.py
  • tests/test_session_gate.py
  • tests/test_session_gate_http_transport.py
  • tests/test_tool_descriptions.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • tests/conftest.py
  • tests/test_session_gate.py
  • blockscout_mcp_server/session_lifecycle.py
  • blockscout_mcp_server/session_gate.py
  • tests/test_tool_descriptions.py
  • API.md

Walkthrough

This change adds optional HTTP session gating. The unlock tool issues signed session identifiers. Other tools validate and meter them through SQLite-backed storage. REST wrappers forward identifiers, API errors map to HTTP statuses, and documentation and tests cover the new behavior.

Changes

Session-gated free-tier access

Layer / File(s) Summary
Session contracts, storage, and metering
blockscout_mcp_server/config.py, blockscout_mcp_server/constants.py, blockscout_mcp_server/session_gate.py, blockscout_mcp_server/session_store.py
Adds configurable session secrets, limits, signed tokens, generation binding, atomic metering, refunds, expiry handling, and typed session errors.
Startup and session-store lifecycle
blockscout_mcp_server/server.py, blockscout_mcp_server/session_lifecycle.py
Validates gated startup settings, initializes SQLite storage, runs expiry sweeps, logs status, and wires lifespan cleanup.
Tool enforcement and session propagation
blockscout_mcp_server/tools/*, blockscout_mcp_server/models.py
Adds session parameters and gates to tools, issues identifiers from the unlock tool, adds budget notes, propagates pagination identifiers, and redacts identifiers from telemetry.
REST enforcement and passthrough
blockscout_mcp_server/api/helpers.py, blockscout_mcp_server/api/routes.py, tests/api/*
Forwards session identifiers through REST wrappers, excludes them from downstream direct API queries, and maps session failures to 401, 403, and 503.
Public contracts and deployment documentation
.env.example, Dockerfile, API.md, README.md, SPEC.md, mcpb/*, AGENTS.md
Documents session initialization, configuration, propagation, budgets, storage, errors, deployment, and observability behavior.
Transport and registry validation
tests/test_session_gate_http_transport.py, tests/test_session_gate_tool_coverage.py, tests/tools/*
Validates MCP transport serialization, unlock behavior, tool coverage, decorator ordering, metering, refunds, pagination, and budget notes.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant UnlockTool
  participant SessionGate
  participant SessionStore
  participant ProtectedTool
  Client->>UnlockTool: Initialize analysis
  UnlockTool->>SessionGate: Mint session_id
  SessionGate->>SessionStore: Read store generation
  SessionStore-->>SessionGate: Generation
  SessionGate-->>UnlockTool: Signed session_id
  UnlockTool-->>Client: InstructionsData with session_id
  Client->>ProtectedTool: Call with session_id
  ProtectedTool->>SessionGate: Validate and meter
  SessionGate->>SessionStore: Atomic increment
  SessionStore-->>SessionGate: Remaining budget
  SessionGate-->>ProtectedTool: Permit execution
  ProtectedTool-->>Client: Result with budget note
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR implements session issuance, gating, PRO-key exemption, persistence, lifecycle handling, and REST/MCP coverage, but it lacks separate rolling and lifetime usage limits [#442]. Add distinct rolling-window and lifetime counters and limits, or update #442 acceptance criteria to match the single per-session budget and TTL design.
Docstring Coverage ⚠️ Warning Docstring coverage is 37.60% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: an opt-in free-session budget gate for unauthenticated HTTP traffic.
Out of Scope Changes check ✅ Passed The code, configuration, documentation, manifests, lifecycle changes, and tests all support the session-gated free-tier objectives [#442].
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/issue-442-impl-plan-1cf390

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.

@akolotov akolotov self-assigned this Jul 31, 2026
akolotov and others added 4 commits July 30, 2026 19:12
Two review findings on the session gate:

- SessionStore.initialize() now performs a real write (rewriting the
  current user_version) after the DDL. On a database that already
  carries the schema every prior statement is a no-op read, and SQLite
  silently opens an unwritable file read-only, so an ro-remounted
  volume passed startup as "ENABLED" and then failed every debit at
  runtime with 503s — violating the fail-fast startup contract.

- verify_token() rejects non-ASCII digit forms in issued_at.
  str.isdigit() alone let superscript digits through to int(), which
  crashed with a bare ValueError bypassing the typed error hierarchy
  (generic 400 instead of the 401 refusal contract), and accepted
  Arabic-Indic digits as an alternate spelling of a valid token.

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

Two fixes from the PR #444 review:

- verify_token: an all-ASCII issued_at longer than CPython's str-to-int
  conversion limit (4300 digits, 3.11+) passed the isascii/isdigit guard
  and raised a bare ValueError from int(), bypassing the typed error
  hierarchy (REST answered 400 with a CPython-internal message instead
  of 401 with SESSION_ID_REQUIRED_MESSAGE). Bound the length to 20
  digits before conversion and cover it with a unit test.

- REST error mapping: expired/exhausted sessions now answer 403 rather
  than 429. The refusal is terminal by design, while 429 sits in the
  default retry lists of common HTTP clients and LLM SDKs and would be
  silently retried by middleware before the agent ever reads the
  do-not-retry body. Rationale recorded in SPEC.md; API.md updated.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p-interval knob

- verify_token rejects tokens over 512 chars before any parsing or HMAC work
  (mirrors the _MAX_KEY_LENGTH bound on client PRO API keys) and requires the
  MAC to be exactly 64 hex chars instead of a >=24 minimum.
- Gated-startup failures (SessionStartupError / SessionStoreInitializationError)
  now exit the CLI with a clean stderr message and code 1 instead of a traceback.
- New BLOCKSCOUT_SESSION_SWEEP_INTERVAL_SECONDS (ge=1, blank/unset = once per
  TTL) decouples expired-row sweep cadence from the TTL; deliberately
  unconstrained relative to the TTL since the deletion cutoff derives from the
  TTL at call time and no cadence can remove a live identifier's row. The
  ENABLED startup line now reports the effective cadence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… responses, store connection hygiene

- Add a completeness test that invokes every registered MCP tool (except
  unlock) without a session_id under an enabled gate and requires
  SessionIdMissingError — a tool carrying the parameter but missing the
  gate decorator can no longer ship ungated.
- Stop refunding ResponseTooLargeError: the upstream fetch already
  happened on the server's key, so refunding would let size-capped calls
  consume upstream credits without ever spending session budget.
- Close the SQLite connection on every initialize() error path (including
  generation load, now wrapped in SessionStoreInitializationError), and
  make initialize_store() close a previously registered singleton instead
  of leaking it; a failed re-initialization leaves the old store usable.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@akolotov
akolotov marked this pull request as ready for review July 31, 2026 04:50
@akolotov

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 0eb9f03e1a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@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: 10

🧹 Nitpick comments (6)
blockscout_mcp_server/session_gate.py (1)

1-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Module is exactly at the 500-LOC soft limit.

session_gate.py ends at line 500. The module now owns token mint/verify, the error hierarchy, predicates, store chokepoints, and both decorators. If more gate policy lands here, split the decorators into a separate module (for example session_gate_decorators.py) to stay under the limit.

As per coding guidelines: "Regular Python modules should generally not exceed 500 lines of code (LOC). If a module approaches this limit, consider splitting it into multiple focused modules".

🤖 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 `@blockscout_mcp_server/session_gate.py` around lines 1 - 99, Keep
session_gate.py at or below the 500-LOC guideline by moving the session_gate and
session_gate_unmetered decorator implementations, plus their decorator-specific
helpers, into a focused module such as session_gate_decorators.py. Update
imports and public exports so existing consumers continue accessing these
symbols without changing behavior, while session_gate.py retains token handling,
errors, predicates, and store policy.

Source: Coding guidelines

tests/test_session_store.py (1)

1-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Test module is close to the 500-LOC limit.

tests/test_session_store.py ends at line 493. If more store coverage lands here, split it into focused modules, for example tests/test_session_store_initialization.py and tests/test_session_store_counters.py.

As per coding guidelines: "Unit test files must not exceed 500 LOC; split into multiple focused modules when approaching the limit".

🤖 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 `@tests/test_session_store.py` around lines 1 - 28, Split the oversized test
module into focused files before adding further coverage: move initialization,
singleton, and filesystem setup tests into a session-store initialization
module, and move counter behavior tests into a session-store counters module.
Preserve shared fixtures and imports in each module as needed, keeping each test
file below the 500-line limit.

Source: Coding guidelines

tests/tools/test_session_budget_note.py (1)

45-54: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Split this test into two tests.

test_no_note_when_context_var_unset asserts two independent scenarios: the None notes case and the caller-supplied notes case. Split it so a failure identifies the exact scenario.

♻️ Proposed split
-def test_no_note_when_context_var_unset():
-    """ContextVar unset (default) -> no note; caller-supplied notes pass through
-    unchanged (including None staying None)."""
-    response = build_tool_response(data={"ok": True})
-    assert response.notes is None
-
-    original_notes = ["existing note"]
-    response_with_notes = build_tool_response(data={"ok": True}, notes=original_notes)
-    assert response_with_notes.notes == ["existing note"]
-    assert original_notes == ["existing note"]
+def test_no_note_when_context_var_unset_keeps_notes_none():
+    """ContextVar unset (default) -> notes stays None."""
+    response = build_tool_response(data={"ok": True})
+    assert response.notes is None
+
+
+def test_no_note_when_context_var_unset_passes_caller_notes_through():
+    """ContextVar unset (default) -> caller-supplied notes pass through unchanged."""
+    original_notes = ["existing note"]
+    response = build_tool_response(data={"ok": True}, notes=original_notes)
+    assert response.notes == ["existing note"]
+    assert original_notes == ["existing note"]

As per coding guidelines: "Each unit test must be narrow and specific; split tests that cover multiple scenarios into separate tests".

🤖 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 `@tests/tools/test_session_budget_note.py` around lines 45 - 54, Split
test_no_note_when_context_var_unset into two focused tests: keep the
unset-context assertion that build_tool_response returns notes as None, and move
the caller-supplied notes assertions into a separate test verifying values and
the original list remain unchanged.

Source: Coding guidelines

tests/tools/test_session_gate_decorator.py (1)

45-59: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated store_spy fixture and _minted helper across both session-gate decorator test files. Both files define byte-identical copies of the store_spy fixture and the _minted helper. Move each definition to one shared location, for example tests/conftest.py or a helper module beside tests/pro_api_key_helpers.py.

  • tests/tools/test_session_gate_decorator.py#L45-L59: remove the local store_spy fixture and _minted helper and import or inherit the shared versions.
  • tests/tools/test_session_gate_decorator_composition.py#L42-L56: remove the duplicate store_spy fixture and _minted helper and use the same shared versions.
🤖 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 `@tests/tools/test_session_gate_decorator.py` around lines 45 - 59, Move the
duplicated store_spy fixture and _minted helper into one shared test location,
then remove the local definitions from
tests/tools/test_session_gate_decorator.py lines 45-59 and
tests/tools/test_session_gate_decorator_composition.py lines 42-56. Update both
test modules to use the shared fixture and helper while preserving their
existing behavior.
tests/tools/test_credit_tracking_decorator.py (1)

92-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider also flagging a session-gate decorator applied without @pro_api_key_scope.

Discovery skips any function that lacks pro_api_key_scope (lines 69-70). A tool decorated with @session_gate but without @pro_api_key_scope is therefore never inspected here. Such a tool would gate every caller and never apply the PRO-API-key exemption, which test_stacking_without_pro_api_key_scope_gates_even_with_key_present documents as the behavior. A separate check would catch that combination.

♻️ Proposed additional check
             dec_names = _decorator_names(node.decorator_list)
 
             if "pro_api_key_scope" not in dec_names:
+                orphan_gates = [n for n in ("session_gate", "session_gate_unmetered") if n in dec_names]
+                if orphan_gates:
+                    violations.append(
+                        f"{path.relative_to(TOOLS_ROOT.parent.parent)}:"
+                        f"{node.lineno}: {node.name} has @{orphan_gates[0]} but no `@pro_api_key_scope`, "
+                        f"so the client-key exemption can never apply"
+                    )
                 continue
🤖 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 `@tests/tools/test_credit_tracking_decorator.py` around lines 92 - 104, Extend
the decorator validation discovery to inspect functions that use session_gate or
session_gate_unmetered even when pro_api_key_scope is absent. Add a separate
violation for this combination, while preserving the existing ordering check
between pro_api_key_scope and pro_api_credit_scope for functions that include
both.
tests/tools/test_session_gate_decorator_composition.py (1)

287-346: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Split test_full_production_order into separate tests.

This test asserts four independent scenarios: the exempt call, the gate refusal, the successful gated call, and the failing body. A failure in scenario 1 hides scenarios 2-4. Split it into four tests that share a fixture for the decorated tool and the SpyCreditSink patch.

As per coding guidelines: "Each unit test must be narrow and specific; split tests that cover multiple scenarios into separate tests".

🤖 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 `@tests/tools/test_session_gate_decorator_composition.py` around lines 287 -
346, Split test_full_production_order into four focused tests covering the
exempt call, gate refusal, successful gated call, and failing body refund/reset
behavior. Add a shared fixture that provides the decorated tool, SpyCreditSink
tracking/patching, and any required state setup; keep each test independently
initializing its context and asserting only its scenario.

Source: Coding guidelines

🤖 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 `@AGENTS.md`:
- Around line 28-30: Shorten the three entries in AGENTS.md to concise
module-location hints only. Remove implementation details such as HMAC tokens,
decorators, request-scoped values, SQLite operations, counters, and WEB3_POOL
lifecycle behavior while retaining each module’s high-level purpose.

In `@API.md`:
- Around line 100-101: Update the “Session Ended (403 Forbidden)” documentation
to cover both expired session identifiers and exhausted free budgets as terminal
refusals. Instruct clients not to retry either case and to obtain a Blockscout
PRO API key for future requests.
- Around line 56-58: Update the authentication guidance in API.md (lines 56-58)
to state that a client PRO API key is optional when a valid session_id is
supplied on gated deployments, while requests with a client key are exempt;
update blockscout_mcp_server/llms.txt (line 50) to describe both authentication
paths instead of requiring a client key unconditionally.

In `@blockscout_mcp_server/session_gate.py`:
- Around line 247-253: Update verify_token’s mac validation before
hmac.compare_digest to require ASCII-only canonical lowercase hexadecimal
characters, while preserving the existing _MAC_HEX_LEN check. Ensure any
non-ASCII or non-0-9a-f mac raises SessionIdInvalidError instead of allowing
TypeError or generic failures.

In `@blockscout_mcp_server/session_lifecycle.py`:
- Around line 219-229: Update the cleanup logic surrounding close_store() so a
failure while closing the session store does not prevent await WEB3_POOL.close()
from executing. Isolate the store-close operation with appropriate exception
handling while preserving the existing sweep-task cancellation and pool shutdown
order.

In `@mcpb/manifest.json`:
- Line 48: Update the unlock tool descriptions in mcpb/manifest.json (line 48)
and mcpb/manifest-dev.json (line 55) to document that, when present,
data.session_id is returned and must be forwarded on all subsequent calls; keep
the existing session initialization and call-order guidance intact.

In `@tests/conftest.py`:
- Around line 99-106: Prevent HTTP mode from leaking when initialize_store fails
in the fixture setup: move analytics.set_http_mode(True) after the successful
initialize_store call, or patch the HTTP-mode flag with monkeypatch so
restoration is unconditional. Preserve disabling HTTP mode during normal fixture
teardown.

In `@tests/test_session_gate_http_transport.py`:
- Around line 228-230: Move the inline imports of __unlock_blockchain_analysis__
used by _build_unlock_app and verify_token used in the test body to the module’s
top-level import section. Remove both local import statements, preserve the
existing verify_token(session_id) call location and ordering, and retain the
current functionality.

In `@tests/test_tool_descriptions.py`:
- Around line 286-291: Extend the test loop over gated_tools to verify that
session_id is not listed in the tool schema’s required fields, while preserving
the existing property and description assertions. Read the required list safely
from each tool’s inputSchema so schemas without required remain valid.
- Around line 249-250: Update the forbidden-language assertions in the tool
description test to compare a normalized lowercase version of tool.description,
ensuring “mandatory” and “in every session” are rejected regardless of
capitalization.

---

Nitpick comments:
In `@blockscout_mcp_server/session_gate.py`:
- Around line 1-99: Keep session_gate.py at or below the 500-LOC guideline by
moving the session_gate and session_gate_unmetered decorator implementations,
plus their decorator-specific helpers, into a focused module such as
session_gate_decorators.py. Update imports and public exports so existing
consumers continue accessing these symbols without changing behavior, while
session_gate.py retains token handling, errors, predicates, and store policy.

In `@tests/test_session_store.py`:
- Around line 1-28: Split the oversized test module into focused files before
adding further coverage: move initialization, singleton, and filesystem setup
tests into a session-store initialization module, and move counter behavior
tests into a session-store counters module. Preserve shared fixtures and imports
in each module as needed, keeping each test file below the 500-line limit.

In `@tests/tools/test_credit_tracking_decorator.py`:
- Around line 92-104: Extend the decorator validation discovery to inspect
functions that use session_gate or session_gate_unmetered even when
pro_api_key_scope is absent. Add a separate violation for this combination,
while preserving the existing ordering check between pro_api_key_scope and
pro_api_credit_scope for functions that include both.

In `@tests/tools/test_session_budget_note.py`:
- Around line 45-54: Split test_no_note_when_context_var_unset into two focused
tests: keep the unset-context assertion that build_tool_response returns notes
as None, and move the caller-supplied notes assertions into a separate test
verifying values and the original list remain unchanged.

In `@tests/tools/test_session_gate_decorator_composition.py`:
- Around line 287-346: Split test_full_production_order into four focused tests
covering the exempt call, gate refusal, successful gated call, and failing body
refund/reset behavior. Add a shared fixture that provides the decorated tool,
SpyCreditSink tracking/patching, and any required state setup; keep each test
independently initializing its context and asserting only its scenario.

In `@tests/tools/test_session_gate_decorator.py`:
- Around line 45-59: Move the duplicated store_spy fixture and _minted helper
into one shared test location, then remove the local definitions from
tests/tools/test_session_gate_decorator.py lines 45-59 and
tests/tools/test_session_gate_decorator_composition.py lines 42-56. Update both
test modules to use the shared fixture and helper while preserving their
existing behavior.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5365cc86-13e7-4be5-9c81-9de0e6cf9702

📥 Commits

Reviewing files that changed from the base of the PR and between 4b98ff1 and 0eb9f03.

📒 Files selected for processing (61)
  • .cursor/rules/150-rest-api-implementation.mdc
  • .env.example
  • AGENTS.md
  • API.md
  • Dockerfile
  • README.md
  • SPEC.md
  • blockscout_mcp_server/api/helpers.py
  • blockscout_mcp_server/api/routes.py
  • blockscout_mcp_server/config.py
  • blockscout_mcp_server/constants.py
  • blockscout_mcp_server/llms.txt
  • blockscout_mcp_server/models.py
  • blockscout_mcp_server/server.py
  • blockscout_mcp_server/session_gate.py
  • blockscout_mcp_server/session_lifecycle.py
  • blockscout_mcp_server/session_store.py
  • blockscout_mcp_server/templates/index.html
  • blockscout_mcp_server/tools/address/get_address_info.py
  • blockscout_mcp_server/tools/address/get_tokens_by_address.py
  • blockscout_mcp_server/tools/address/nft_tokens_by_address.py
  • blockscout_mcp_server/tools/block/get_block_info.py
  • blockscout_mcp_server/tools/block/get_block_number.py
  • blockscout_mcp_server/tools/chains/get_chains_list.py
  • blockscout_mcp_server/tools/common.py
  • blockscout_mcp_server/tools/contract/get_contract_abi.py
  • blockscout_mcp_server/tools/contract/inspect_contract_code.py
  • blockscout_mcp_server/tools/contract/read_contract.py
  • blockscout_mcp_server/tools/decorators.py
  • blockscout_mcp_server/tools/direct_api/direct_api_call.py
  • blockscout_mcp_server/tools/ens/get_address_by_ens_name.py
  • blockscout_mcp_server/tools/initialization/unlock_blockchain_analysis.py
  • blockscout_mcp_server/tools/search/lookup_token_by_symbol.py
  • blockscout_mcp_server/tools/transaction/get_token_transfers_by_address.py
  • blockscout_mcp_server/tools/transaction/get_transaction_info.py
  • blockscout_mcp_server/tools/transaction/get_transactions_by_address.py
  • mcpb/manifest-dev.json
  • mcpb/manifest.json
  • tests/api/test_routes.py
  • tests/api/test_routes_session_gate.py
  • tests/api/test_routes_session_gate_e2e.py
  • tests/api/test_routes_session_passthrough.py
  • tests/conftest.py
  • tests/test_config.py
  • tests/test_instructions_data.py
  • tests/test_server_session_gate.py
  • tests/test_server_session_gate_lifespan.py
  • tests/test_session_gate.py
  • tests/test_session_gate_http_transport.py
  • tests/test_session_gate_tool_coverage.py
  • tests/test_session_store.py
  • tests/test_tool_descriptions.py
  • tests/tools/block/test_get_block_number.py
  • tests/tools/chains/test_get_chains_list.py
  • tests/tools/initialization/test___unlock_blockchain_analysis__.py
  • tests/tools/test_credit_tracking_decorator.py
  • tests/tools/test_session_budget_note.py
  • tests/tools/test_session_gate_decorator.py
  • tests/tools/test_session_gate_decorator_composition.py
  • tests/tools/test_session_id_redaction.py
  • tests/tools/transaction/test_get_transactions_by_address.py

Comment thread AGENTS.md
Comment thread API.md
Comment thread API.md Outdated
Comment thread blockscout_mcp_server/session_gate.py Outdated
Comment thread blockscout_mcp_server/session_lifecycle.py
Comment thread mcpb/manifest.json
Comment thread tests/conftest.py
Comment thread tests/test_session_gate_http_transport.py Outdated
Comment thread tests/test_tool_descriptions.py Outdated
Comment thread tests/test_tool_descriptions.py
akolotov and others added 2 commits July 31, 2026 10:16
… 403 docs

- `verify_token` now requires the canonical lowercase-hex alphabet for the
  `mac` part. A 64-character non-ASCII `mac` cleared both length checks and
  made `hmac.compare_digest` raise a bare `TypeError`, which `handle_rest_errors`
  mapped to a generic 500 instead of the typed invalid-token refusal.
- The composed lifespan isolates `close_store()` so a failing
  `sqlite3.Connection.close()` can no longer skip `await WEB3_POOL.close()`.
- API.md documents the terminal 403 as covering both an expired `session_id`
  and an exhausted budget, matching the shared `handle_rest_errors` branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- `enabled_session_gate` now flips HTTP mode on only after `initialize_store`
  succeeds, so a store-initialization failure can no longer leak the flag into
  every later test in the session.
- Hoist the `verify_token` and `__unlock_blockchain_analysis__` imports in the
  HTTP-transport tests to the module top, per implementation rule 7. The
  ordering constraint documented at the `verify_token` call site is about the
  call, not the import, so the call stays where it is.
- Make the forbidden-language assertion on the unlock tool's description
  case-insensitive.
- Assert that `session_id` stays out of every gated tool's schema `required`
  list, not just present in `properties`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@akolotov

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session-gated free tier to make the PRO API key requirement structural

1 participant