From bcf509a440b12a73ba0dfd0ea1f337bd58591155 Mon Sep 17 00:00:00 2001 From: Stefano Maffeis Date: Sat, 4 Jul 2026 06:09:34 +0000 Subject: [PATCH] Drop dead last_ws_id/last_tool_call_id columns from mcp_pending_consent Migration 054 added these columns but they were never populated (the sole writer hardcodes None) and never read by any query, dashboard, or API. Drop them so the schema matches reality. Closes #769 Co-Authored-By: Paperclip --- tests/test_mcp_oauth_handlers.py | 4 --- tests/test_mcp_pending_consent_endpoints.py | 2 -- tests/test_mcp_pending_consent_storage.py | 22 ------------- turnstone/core/mcp_client.py | 11 ++----- turnstone/core/storage/_postgresql.py | 8 ----- turnstone/core/storage/_protocol.py | 12 +++---- turnstone/core/storage/_schema.py | 2 -- turnstone/core/storage/_sqlite.py | 8 ----- .../064_drop_mcp_pending_consent_dead_cols.py | 31 +++++++++++++++++++ 9 files changed, 37 insertions(+), 63 deletions(-) create mode 100644 turnstone/core/storage/migrations/versions/064_drop_mcp_pending_consent_dead_cols.py diff --git a/tests/test_mcp_oauth_handlers.py b/tests/test_mcp_oauth_handlers.py index a1680a21..2b474fab 100644 --- a/tests/test_mcp_oauth_handlers.py +++ b/tests/test_mcp_oauth_handlers.py @@ -630,8 +630,6 @@ class TestCallback: server_name="srv-oauth", error_code="mcp_consent_required", scopes_required=None, - last_ws_id="ws-1", - last_tool_call_id="tool-1", now_iso="2026-05-11T12:00:00", ) storage.upsert_mcp_pending_consent( @@ -639,8 +637,6 @@ class TestCallback: server_name="srv-oauth", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso="2026-05-11T12:00:00", ) token_store = _make_token_store(storage) diff --git a/tests/test_mcp_pending_consent_endpoints.py b/tests/test_mcp_pending_consent_endpoints.py index 3c002558..77587d7e 100644 --- a/tests/test_mcp_pending_consent_endpoints.py +++ b/tests/test_mcp_pending_consent_endpoints.py @@ -108,8 +108,6 @@ def _seed_pending( server_name=server_name, error_code=error_code, scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso=now_iso, ) diff --git a/tests/test_mcp_pending_consent_storage.py b/tests/test_mcp_pending_consent_storage.py index f225674c..172024b9 100644 --- a/tests/test_mcp_pending_consent_storage.py +++ b/tests/test_mcp_pending_consent_storage.py @@ -24,8 +24,6 @@ class TestUpsertAndList: server_name="srv-x", error_code="mcp_consent_required", scopes_required="read write", - last_ws_id="ws-1", - last_tool_call_id="tool-1", now_iso=_iso(), ) rows = backend.list_mcp_pending_consent_by_user("user-a") @@ -35,8 +33,6 @@ class TestUpsertAndList: assert r["server_name"] == "srv-x" assert r["error_code"] == "mcp_consent_required" assert r["scopes_required"] == "read write" - assert r["last_ws_id"] == "ws-1" - assert r["last_tool_call_id"] == "tool-1" assert r["occurrence_count"] == 1 assert r["first_seen_at"] == r["last_seen_at"] @@ -46,8 +42,6 @@ class TestUpsertAndList: server_name="srv-x", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso="2026-05-11T12:00:00", ) backend.upsert_mcp_pending_consent( @@ -55,8 +49,6 @@ class TestUpsertAndList: server_name="srv-x", error_code="mcp_insufficient_scope", scopes_required="read", - last_ws_id="ws-2", - last_tool_call_id="tool-2", now_iso="2026-05-11T13:00:00", ) rows = backend.list_mcp_pending_consent_by_user("user-a") @@ -66,8 +58,6 @@ class TestUpsertAndList: assert r["occurrence_count"] == 2 assert r["error_code"] == "mcp_insufficient_scope" assert r["scopes_required"] == "read" - assert r["last_ws_id"] == "ws-2" - assert r["last_tool_call_id"] == "tool-2" assert r["last_seen_at"] == "2026-05-11T13:00:00" # first_seen_at preserved — that's the load-bearing audit value. assert r["first_seen_at"] == "2026-05-11T12:00:00" @@ -78,8 +68,6 @@ class TestUpsertAndList: server_name="srv-old", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso="2026-05-11T10:00:00", ) backend.upsert_mcp_pending_consent( @@ -87,8 +75,6 @@ class TestUpsertAndList: server_name="srv-new", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso="2026-05-11T11:00:00", ) rows = backend.list_mcp_pending_consent_by_user("user-a") @@ -100,8 +86,6 @@ class TestUpsertAndList: server_name="srv", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso=_iso(), ) assert backend.list_mcp_pending_consent_by_user("user-b") == [] @@ -114,8 +98,6 @@ class TestDelete: server_name="srv-x", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso=_iso(), ) assert backend.delete_mcp_pending_consent("user-a", "srv-x") is True @@ -133,8 +115,6 @@ class TestDelete: server_name=name, error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso=_iso(), ) # Cross-user row that must NOT be touched. @@ -143,8 +123,6 @@ class TestDelete: server_name="srv-z", error_code="mcp_consent_required", scopes_required=None, - last_ws_id=None, - last_tool_call_id=None, now_iso=_iso(), ) assert backend.delete_all_mcp_pending_consent_by_user("user-a") == 3 diff --git a/turnstone/core/mcp_client.py b/turnstone/core/mcp_client.py index cbb00522..27d8fa6c 100644 --- a/turnstone/core/mcp_client.py +++ b/turnstone/core/mcp_client.py @@ -2133,13 +2133,8 @@ class MCPClientManager: Both best-effort wrappers — the sync dispatch-path :meth:`_record_pending_consent_best_effort` and the loop-path :meth:`_persist_pending_consent_best_effort` — route through here so the - 7-kwarg ``upsert_mcp_pending_consent`` call and the timestamp format live - in ONE place. ``last_ws_id`` / ``last_tool_call_id`` are always ``None``: - neither path captures a triggering ws / tool-call id (the mcp dispatch - layer never receives one, and the proactive sweep has no dispatch), and - no caller reads those columns today — they are unpopulated schema - affordances from migration 054. Raises on storage error — callers wrap - best-effort. + 5-kwarg ``upsert_mcp_pending_consent`` call and the timestamp format live + in ONE place. Raises on storage error — callers wrap best-effort. """ if self._storage is None: return @@ -2148,8 +2143,6 @@ class MCPClientManager: server_name=server_name, error_code=error_code, scopes_required=scopes_required, - last_ws_id=None, - last_tool_call_id=None, now_iso=datetime.now(UTC).strftime("%Y-%m-%dT%H:%M:%S"), ) diff --git a/turnstone/core/storage/_postgresql.py b/turnstone/core/storage/_postgresql.py index 06b912b7..7e300bd9 100644 --- a/turnstone/core/storage/_postgresql.py +++ b/turnstone/core/storage/_postgresql.py @@ -4892,8 +4892,6 @@ class PostgreSQLBackend: server_name: str, error_code: str, scopes_required: str | None, - last_ws_id: str | None, - last_tool_call_id: str | None, now_iso: str, ) -> None: from sqlalchemy.dialects import postgresql @@ -4903,8 +4901,6 @@ class PostgreSQLBackend: server_name=server_name, error_code=error_code, scopes_required=scopes_required, - last_ws_id=last_ws_id, - last_tool_call_id=last_tool_call_id, first_seen_at=now_iso, last_seen_at=now_iso, occurrence_count=1, @@ -4914,8 +4910,6 @@ class PostgreSQLBackend: set_={ "error_code": stmt.excluded.error_code, "scopes_required": stmt.excluded.scopes_required, - "last_ws_id": stmt.excluded.last_ws_id, - "last_tool_call_id": stmt.excluded.last_tool_call_id, "last_seen_at": stmt.excluded.last_seen_at, "occurrence_count": mcp_pending_consent.c.occurrence_count + 1, }, @@ -4940,8 +4934,6 @@ class PostgreSQLBackend: server_name=m["server_name"], error_code=m["error_code"], scopes_required=m["scopes_required"], - last_ws_id=m["last_ws_id"], - last_tool_call_id=m["last_tool_call_id"], first_seen_at=m["first_seen_at"], last_seen_at=m["last_seen_at"], occurrence_count=m["occurrence_count"], diff --git a/turnstone/core/storage/_protocol.py b/turnstone/core/storage/_protocol.py index 26db64a3..3bf681cd 100644 --- a/turnstone/core/storage/_protocol.py +++ b/turnstone/core/storage/_protocol.py @@ -108,8 +108,6 @@ class MCPPendingConsentRow(TypedDict): server_name: str error_code: str scopes_required: str | None - last_ws_id: str | None - last_tool_call_id: str | None first_seen_at: str last_seen_at: str occurrence_count: int @@ -2233,8 +2231,6 @@ class StorageBackend(Protocol): server_name: str, error_code: str, scopes_required: str | None, - last_ws_id: str | None, - last_tool_call_id: str | None, now_iso: str, ) -> None: """Insert or refresh a deferred-consent record for ``(user, server)``. @@ -2242,10 +2238,10 @@ class StorageBackend(Protocol): On insert: ``first_seen_at = last_seen_at = now_iso``, ``occurrence_count = 1``. On conflict (existing row for the same composite PK): rewrites ``error_code``, ``scopes_required``, - ``last_ws_id``, ``last_tool_call_id``, ``last_seen_at`` to the - current values; bumps ``occurrence_count`` by 1. Preserves - ``first_seen_at`` so the dashboard can show how long the - deferred-consent need has been pending. + ``last_seen_at`` to the current values; bumps + ``occurrence_count`` by 1. Preserves ``first_seen_at`` so the + dashboard can show how long the deferred-consent need has been + pending. """ ... diff --git a/turnstone/core/storage/_schema.py b/turnstone/core/storage/_schema.py index b331f4b1..e6215346 100644 --- a/turnstone/core/storage/_schema.py +++ b/turnstone/core/storage/_schema.py @@ -999,8 +999,6 @@ mcp_pending_consent = sa.Table( sa.Column("server_name", sa.Text, nullable=False), sa.Column("error_code", sa.Text, nullable=False), sa.Column("scopes_required", sa.Text, nullable=True), - sa.Column("last_ws_id", sa.Text, nullable=True), - sa.Column("last_tool_call_id", sa.Text, nullable=True), sa.Column("first_seen_at", sa.Text, nullable=False), sa.Column("last_seen_at", sa.Text, nullable=False), sa.Column("occurrence_count", sa.Integer, nullable=False, server_default="1"), diff --git a/turnstone/core/storage/_sqlite.py b/turnstone/core/storage/_sqlite.py index c53118bc..8d3a89a2 100644 --- a/turnstone/core/storage/_sqlite.py +++ b/turnstone/core/storage/_sqlite.py @@ -5066,8 +5066,6 @@ class SQLiteBackend: server_name: str, error_code: str, scopes_required: str | None, - last_ws_id: str | None, - last_tool_call_id: str | None, now_iso: str, ) -> None: from sqlalchemy.dialects import sqlite as sa_sqlite @@ -5077,8 +5075,6 @@ class SQLiteBackend: server_name=server_name, error_code=error_code, scopes_required=scopes_required, - last_ws_id=last_ws_id, - last_tool_call_id=last_tool_call_id, first_seen_at=now_iso, last_seen_at=now_iso, occurrence_count=1, @@ -5088,8 +5084,6 @@ class SQLiteBackend: set_={ "error_code": stmt.excluded.error_code, "scopes_required": stmt.excluded.scopes_required, - "last_ws_id": stmt.excluded.last_ws_id, - "last_tool_call_id": stmt.excluded.last_tool_call_id, "last_seen_at": stmt.excluded.last_seen_at, "occurrence_count": mcp_pending_consent.c.occurrence_count + 1, }, @@ -5114,8 +5108,6 @@ class SQLiteBackend: server_name=m["server_name"], error_code=m["error_code"], scopes_required=m["scopes_required"], - last_ws_id=m["last_ws_id"], - last_tool_call_id=m["last_tool_call_id"], first_seen_at=m["first_seen_at"], last_seen_at=m["last_seen_at"], occurrence_count=m["occurrence_count"], diff --git a/turnstone/core/storage/migrations/versions/064_drop_mcp_pending_consent_dead_cols.py b/turnstone/core/storage/migrations/versions/064_drop_mcp_pending_consent_dead_cols.py new file mode 100644 index 00000000..6cf9930b --- /dev/null +++ b/turnstone/core/storage/migrations/versions/064_drop_mcp_pending_consent_dead_cols.py @@ -0,0 +1,31 @@ +"""Drop dead columns from mcp_pending_consent. + +``last_ws_id`` and ``last_tool_call_id`` were added in migration 054 but +were never populated (the sole writer hardcodes None) and never read by +any query, dashboard, or API. Drop them so the schema matches reality. + +Revision ID: 064 +Revises: 063 +Create Date: 2026-07-04 +""" + +from alembic import op + +revision = "064" +down_revision = "063" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + with op.batch_alter_table("mcp_pending_consent") as batch_op: + batch_op.drop_column("last_ws_id") + batch_op.drop_column("last_tool_call_id") + + +def downgrade() -> None: + import sqlalchemy as sa + + with op.batch_alter_table("mcp_pending_consent") as batch_op: + batch_op.add_column(sa.Column("last_ws_id", sa.Text, nullable=True)) + batch_op.add_column(sa.Column("last_tool_call_id", sa.Text, nullable=True))