mirror of
https://github.com/turnstonelabs/turnstone.git
synced 2026-08-12 23:12:23 -06:00
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 <noreply@paperclip.ing>
This commit is contained in:
committed by
Patrick Buckley
parent
10f726f83d
commit
bcf509a440
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
)
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"),
|
||||
)
|
||||
|
||||
|
||||
@@ -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"],
|
||||
|
||||
@@ -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.
|
||||
"""
|
||||
...
|
||||
|
||||
|
||||
@@ -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"),
|
||||
|
||||
@@ -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"],
|
||||
|
||||
@@ -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))
|
||||
Reference in New Issue
Block a user