Drop the old connector tables - #127
Merged
Merged
Conversation
`connector_instances`, `connector_cursors`, `webhook_subscriptions` and `sync_runs` were the bookkeeping for the worker that filled the document index dropped in 0010. #97 removed the last code that touched them along with `server/src/connectors.ts`, so all four, and the two enums that existed only for their columns, are now declared and read by nothing. What goes is operational history rather than content: cursors, run outcomes and each source's metadata. No credential goes with it. The pointer into the vault was ON DELETE SET NULL and lived on the connector row, not the other way round, so the `credentials` row and its ciphertext are untouched. `credential_kind` keeps its `connector` value: that is a kind of secret, and removing a value from an enum in use is a different question from removing a table nothing reads. A webhook the old connector registered at the vendor is not withdrawn by this. Nothing has listened to those deliveries since the connector went, and the subscription lapses on its own, but the migration says so rather than implying the vendor side is clean. "Connector" still means the per-person plugin connectors, which live in the plugins schema and are untouched.
davidmckayv
requested review from
MikeRyanDev,
guidovizoso and
tylerslaton
as code owners
August 22, 2026 00:27
davidmckayv
added a commit
that referenced
this pull request
Aug 22, 2026
#126 and #127 dropped the document index and the old connector tables, so the changelog line that said the index was "read by nothing" now understates it: the tables are gone. Say dropped, and add the one Upgrading note that matters, which is that those migrations destroy that data and cannot be rolled back. architecture.md still listed connector state among what the database holds; #127 removed it, so the line goes too. The README's database line never named those tables, so it needs nothing. #123 is CI and build hardening, not a deployment behavior, so it earns no changelog line.
davidmckayv
added a commit
that referenced
this pull request
Aug 22, 2026
* Catch the changelog and docs up to what shipped The Unreleased notes already tracked most of the recent work, since each change carried its own line in. This fills the gaps and fixes what went stale. Two merged changes had no line. The address guard's alternate-encoding refusal: it turned away the metadata and private addresses as usually written but not the same ones spelled as an IPv6-mapped or NAT64 form, an integer, or with a trailing dot, and it now canonicalises before it checks and refuses the container credential endpoints even with the private-host opt-in on. And the supervisor refusing to adopt a container it did not create, so a shared Docker host cannot hand it a stranger's container with the computer token. Docs that drifted: the README and the Cloud Run note still said one replica, which the deployment doc's own Replicas section now contradicts, so both point at the real remaining constraint instead, which is the shared browser. And AUDIT_RETENTION_DAYS and COMPUTER_SANDBOX were configurable and documented in the changelog and the README but missing from the configuration table. The knowledge back-out left one more orphan the removal missed: agents/invocation.ts routed a built-in agent to the knowledge agent that is gone, and nothing live constructs it. Deleted with its test. The changelog line that said the local index's connector "is going away" is now "has been removed", because it has been. README stays a build doc; none of this adds history to it. * Reconcile with the table drops that landed after #126 and #127 dropped the document index and the old connector tables, so the changelog line that said the index was "read by nothing" now understates it: the tables are gone. Say dropped, and add the one Upgrading note that matters, which is that those migrations destroy that data and cannot be rolled back. architecture.md still listed connector state among what the database holds; #127 removed it, so the line goes too. The README's database line never named those tables, so it needs nothing. #123 is CI and build hardening, not a deployment behavior, so it earns no changelog line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Clears the last of what #124 flagged and left.
connector_instances,connector_cursors,webhook_subscriptionsandsync_runsgo, with theconnector_typeandsync_statusenums that existed only for their columns.These four were the bookkeeping for the worker that filled the document index dropped in #126: which sources were configured, where each sync had reached, which webhooks were registered with the vendor, and how each run went. #97 removed
server/src/connectors.tsand the worker's sync runner, which were the last code that touched them.Worth correcting one thing in #124's notes, because it was true when written and is not now. It listed
connector_instancesas live, onserver/src/connectors.tsselecting, updating and inserting against it. That file no longer exists. Nothing outsidecore.tsandschema.test.tsreferences any of the four, so this is the whole subsystem rather than the three leftovers it looked like an hour ago.knowledge-agent.ts, the fourth thing #124 flagged, is already gone. #97 took it.What is lost, and what is not
Operational history rather than anybody's content: cursors, run outcomes, and each source's
source_metadata. Still worth the warning at the top of the migration, since it cannot be rolled back.No credential goes with it, and I checked rather than assuming. The vault pointer was
credential_idon the connector row withON DELETE SET NULL, pointing atcredentials, not the other way round. Seeded aconnectorcredential, attached it to an instance, ran the migration:A webhook registered at the vendor is not withdrawn by this, and the migration says so instead of implying the vendor side is clean. Nothing has listened to those deliveries since the connector was removed and the subscription lapses on its own, but a deployment that wants it gone now has to revoke it at the vendor.
credential_kindkeeps itsconnectorvalue. That is a kind of secret, not a row in these tables. Removing a value from an enum that is in use is a different question from removing a table nothing reads, and it is not this PR's."Connector" still means something
The per-person plugin connectors from #97 live in the
pluginsschema, are untouched here, and are what/admin/pluginsconfigures. The docs that talk about connectors are describing those, so no doc change:docs/architecture.md:22says this database holds connector state, which stays true ofmcp_servers,plugin_grantsandmcp_user_credentials.Where it runs
No new state outliving a request, nothing different on a second replica, nothing serialised, nothing fanned out to a browser, no new listener, port or schedule. One migration and the declarations it follows from.
Boundary and audit
N/A. No gateway, policy or audit path touched.
Proof
Driven against a real Postgres.
Upgrade path with rows in all four, migrated to
main(0010) first, then a credential, an instance pointing at it, a cursor, a webhook subscription and a completed run:Fresh database,
0000→0011: applies clean.Migration chain:
Suite, against a database migrated to
0011:bun run testbun run typecheckbun run format:checkbunx biome lint .mainbun run buildSame two choices as #126
Named in dependency order and dropped without CASCADE, so a fork that hung something off these gets a failed migration that changes nothing rather than a silent removal. The whole file is one transaction, so it is all or nothing.
No changelog entry. The
Unreleasedentry is being written elsewhere and I did not want to collide with it.