Skip to content

feat: register placements with engine and persist publisher_id - #23

Closed
trial-gravity wants to merge 5 commits into
mainfrom
feat/placement-registration
Closed

feat: register placements with engine and persist publisher_id#23
trial-gravity wants to merge 5 commits into
mainfrom
feat/placement-registration

Conversation

@trial-gravity

@trial-gravity trial-gravity commented Mar 13, 2026

Copy link
Copy Markdown
Collaborator

Depends on: https://github.com/Try-Gravity/gravity/pull/88 (merge that first)

Summary

  • When generate_code is called with a valid API key, the MCP server now POSTs to the engine's POST /api/v1/placements endpoint to register the placement, then stores the returned publisher_id in the local PostgreSQL alongside the existing placement data.
  • Adds a publisher_id column to the local placements table with an ALTER TABLE migration for existing databases.
  • Changes the INSERT from ON CONFLICT DO NOTHING to ON CONFLICT DO UPDATE so repeat calls backfill publisher_id and refresh placement type.

Details

mcp-server/data/db.py: Schema change -- adds publisher_id TEXT DEFAULT '' to CREATE TABLE, migration SQL, and INSERT. Upsert now updates publisher_id and placement on conflict.

mcp-server/tools/generate_code.py: Adds _register_placement() helper that calls the engine and extracts publisher_id from the response. Gracefully returns "" on any failure (timeout, network error, non-200). Wired into generate_code() before write_placement() so the publisher_id flows into the local DB row.

Test plan

  • All 61 existing unit tests pass
  • Verified engine endpoint returns publisher_id via direct curl
  • Verified full MCP -> engine -> local DB flow with patched write_placement captures correct publisher_id
  • Verified graceful fallback: when engine is unreachable, publisher_id is "" and no error surfaces to the caller

Note

Medium Risk
Adds an outbound HTTP call during generate_code and changes persistence semantics to an upsert with schema migration, which could affect runtime behavior on failures and existing DB rows.

Overview
When generate_code is called with an api_key, it now POSTs to the Gravity engine (/api/v1/placements) to register the placement and captures the returned publisher_id (falling back to "" on any error/timeout).

Local persistence is extended to store publisher_id: the placements table gains a new column with an ALTER TABLE migration, and the write path switches from insert-only to an upsert that backfills publisher_id (only overwriting when non-empty) and refreshes placement on conflicts. Unit tests are updated to mock the new outbound registration call.

Written by Cursor Bugbot for commit 5556182. This will update automatically on new commits. Configure here.

Add publisher_id to the CREATE TABLE, migration SQL, and INSERT
statement. Switch ON CONFLICT from DO NOTHING to DO UPDATE so
repeat calls backfill publisher_id and refresh placement type.

Made-with: Cursor
Call POST /api/v1/placements on the engine before writing to the
local DB so the returned publisher_id is captured. Falls back to
empty string if the engine is unreachable — backfilled on next call
via the ON CONFLICT DO UPDATE in the prior commit.

Made-with: Cursor

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Upsert overwrites valid publisher_id with empty string
    • The upsert now preserves the existing placements.publisher_id unless the incoming EXCLUDED.publisher_id is non-empty, preventing valid IDs from being overwritten by empty strings.

Create PR

Or push these changes by commenting:

@cursor push a53a332a4a
Preview (a53a332a4a)
diff --git a/mcp-server/data/db.py b/mcp-server/data/db.py
--- a/mcp-server/data/db.py
+++ b/mcp-server/data/db.py
@@ -35,7 +35,10 @@
 INSERT INTO placements (placement_id, placement, style, type, framework, platform, performance, publisher_key_hash, publisher_id, created_at)
 VALUES (%(placement_id)s, %(placement)s, %(style)s, %(type)s, %(framework)s, %(platform)s, %(performance)s, %(publisher_key_hash)s, %(publisher_id)s, %(created_at)s)
 ON CONFLICT (placement_id) DO UPDATE
-SET publisher_id = EXCLUDED.publisher_id,
+SET publisher_id = CASE
+        WHEN EXCLUDED.publisher_id <> '' THEN EXCLUDED.publisher_id
+        ELSE placements.publisher_id
+    END,
     placement = EXCLUDED.placement
 """

Comment thread mcp-server/data/db.py
When the engine is unreachable or no api_key is provided,
publisher_id resolves to "". The unconditional SET overwrote
previously stored values. Use a CASE expression to preserve the
existing publisher_id when the incoming value is empty.

Made-with: Cursor

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Tests make unintended real HTTP calls to production
    • I updated the autouse test fixture to patch tools.generate_code._register_placement so tests with an API key no longer perform real outbound HTTP requests.
  • ✅ Fixed: Strict 200 check silently drops 201 Created responses
    • I changed _register_placement to use resp.is_success so successful 2xx responses such as 201 now preserve and return publisher_id.

Create PR

Or push these changes by commenting:

@cursor push 3e13a537b1
Preview (3e13a537b1)
diff --git a/mcp-server/tests/test_tools.py b/mcp-server/tests/test_tools.py
--- a/mcp-server/tests/test_tools.py
+++ b/mcp-server/tests/test_tools.py
@@ -16,8 +16,10 @@
 
 @pytest.fixture(autouse=True)
 def _mock_db():
-    """Prevent real DB writes during unit tests."""
-    with patch("tools.generate_code.write_placement"):
+    """Prevent real DB writes and network registration during unit tests."""
+    with patch("tools.generate_code.write_placement"), patch(
+        "tools.generate_code._register_placement", return_value=""
+    ):
         yield
 
 

diff --git a/mcp-server/tools/generate_code.py b/mcp-server/tools/generate_code.py
--- a/mcp-server/tools/generate_code.py
+++ b/mcp-server/tools/generate_code.py
@@ -27,7 +27,7 @@
             json={"placement_id": placement_id, "placement": placement},
             timeout=5.0,
         )
-        if resp.status_code == 200:
+        if resp.is_success:
             return resp.json().get("publisher_id", "")
     except Exception:
         logger.debug("Engine placement registration failed", exc_info=True)

Comment thread mcp-server/tools/generate_code.py
timeout=5.0,
)
if resp.status_code == 200:
return resp.json().get("publisher_id", "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strict 200 check silently drops 201 Created responses

Low Severity

_register_placement only extracts publisher_id when resp.status_code == 200, but this is a POST endpoint that registers a new resource. The HTTP-standard response for resource creation is 201 Created. If the engine returns 201, the publisher_id is silently discarded and stored as "". This contrasts with validate_api_key in data/auth.py, which checks for known failure codes instead, making it resilient to any successful status code.

Fix in Cursor Fix in Web

psycopg3's execute() only runs the first statement when given
multi-statement SQL. Split _MIGRATE_SQL into individual ALTER TABLE
calls so the publisher_id column migration actually runs.

Made-with: Cursor
The autouse _mock_db fixture only patched write_placement, so any test
passing a truthy api_key triggered a real httpx.post to the engine with
a 5-second timeout — making the suite slow and flaky in CI/offline.

Made-with: Cursor
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.

2 participants