Repository navigation
fix(byod): generate the bootstrap from migrations and fix BYOD's limits - #690
Conversation
The hand-written BYOD schema had drifted: it defined 6 of the 48 RPCs the edge calls, and its memory_write still took p_ttl_days while both write paths send p_ttl_seconds, p_kind, p_host and p_origin_*, so PostgREST cannot resolve the call (PGRST202). Generate the file from the migrations instead: one transaction, a guard against re-running, and the migration history recorded so the database upgrades with supabase db push. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…without supabase-js Move the generator to supabase/byod/ and add a supabase:test target, so the check job's nx affected -t test runs the drift spec on any change under supabase/ without a workflow edit. doctor and bootstrap imported @supabase/supabase-js, which the published zero-dependency CLI does not ship, so the BYOD check failed for every npm install. Replace it with one fetch-based PostgREST probe. bootstrap no longer prints the service-role key as a Postgres password (it is not one), and its help no longer claims it creates the schema. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
byod.md: BYOD is read from the mcp function's own secrets, so it needs a self-deployed function; an env block in .mcp.json never reaches the hosted one. Document the generated bootstrap.sql, upgrades via supabase db push, and the limits found while fixing it: only the mcp function has a BYOD mode, and a token's user must exist in the storage project's auth.users. tasks/local-sqlite-mode.md: a phased proposal for running the dashboard and usage views on SQLite without Supabase, by implementing the REST contract locally rather than porting the 48 RPCs. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…npm installs - byod.md: the service key is needed for reads (the read policies need auth.uid(), which an API key never carries); say what JWT callers see; add the path for databases set up from the old hand-written file; enable pg_cron first. - bootstrap: probe first and print the download URL when supabase/ is not in the install, instead of exiting 1 for every npm user. - generator: Supabase CLI filename grammar, CRLF/BOM normalisation, fail on duplicate versions, terminate each migration's last statement. - cli.md: document bootstrap and doctor's byod storage check. - local-sqlite-mode.md: onboarding and plan usage also read memories directly; they need REST routes before local mode. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…FKs in storage Three BYOD limits documented in the previous commits, fixed: - An anon-key storage client wrote but read back nothing (the read policies need auth.uid()). resolveStorageAdapter now requires LOREKIT_STORAGE_SERVICE_KEY and says why. - With the service key, a Supabase-session (JWT) caller reached the tools with no user id, so it read every row in the storage project and wrote ownerless rows. storageUserId scopes it by its resolved id, the path an lk_* token already takes, and refuses one with no id. - Every lk_* write failed on memories_user_id_fkey: the storage project's auth.users never holds the function project's users. storage-overlay.sql drops every public FK into auth.users (by query, idempotent); bootstrap.sql now ends with it and the db push route runs it after each push. doctor accepts either storage key. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
Teams are a requirement, and team features (orgs, invites, tokens, RLS) live in ~50 Postgres RPCs, so SQLite would mean a second implementation. Propose one compose bundle of Supabase's open-source services plus the dashboard, scaffolded by a small lorekit selfhost init, with the CLI size measurements and the work items a proof of concept must settle. Co-authored-by: dash0-dev[bot] <257284812+dash0-dev[bot]@users.noreply.github.com>
…re 500
resolveStorageAdapter() throws when LOREKIT_STORAGE_URL is set without
LOREKIT_STORAGE_SERVICE_KEY. Called bare, the throw escaped Deno.serve, so
every MCP call (initialize and tools/list included) got a bodyless
"500 Internal Server Error" and the reason reached only the function log,
although docs/byod.md promises an error naming the missing variable.
Catch it and answer with jsonrpcError(-32603), the in-band shape the auth
failure above already uses; mark the root span ERROR (error.type
storage_config). Only authenticated callers reach it; the message names
env vars, never values. A source-scan guard in mcp-guards/ pins it.
local-verify F-1 (AC-6): AC6-nosvc-* probes returned HTTP 500
"Internal Server Error"; after the fix the same probes return
{"code":-32603,"message":"LOREKIT_STORAGE_URL is set but
LOREKIT_STORAGE_SERVICE_KEY is missing. …"}, unauthenticated calls still
get -32001, and BYOD reads/writes with the service key are unchanged.
The 'How BYOD is wired' summary still listed LOREKIT_STORAGE_SERVICE_KEY as optional while §3 (and resolveStorageAdapter) require it; the anon key is the optional one. local-verify F-2: docs/byod.md:15-16 contradicted docs/byod.md:94 and the AC-6 probe, which refuses a storage URL without the service key.
A hosted MCP call authenticated by a Supabase session JWT ran with
toolUserId = null on the assumption that RLS scopes it. True for reads,
not for writes: memory_write reads p_user_id = null as an ownerless row,
so the caller's own memory.list never returned it, and recordAudit then
failed RLS on audit_log. The same null made memory.purge, policy.*,
groom.* and memory.protect refuse session callers ("requires a user_id").
storageUserId now returns the resolved id for a JWT caller in both
modes, the value the REST create handler already passes (auth.userId).
RPCs that take p_user_id still pin non-service callers to auth.uid(),
so the id cannot widen access. A hosted session with no resolved id
still falls back to RLS; BYOD still refuses it.
local-verify F-3 (EC-13): before, EC13-hosted-jwt-write-B stored
user_id NULL, the caller's list was empty, and the log showed
"[recordAudit] insert failed … row-level security policy for table
audit_log". After: the row is owned by the caller, the audit row is
written, read/list return it, user A cannot read it, policy.list
answers, and every probe window is free of errors.
A transport failure's message carries the request URL, and a PostgREST
URL carries every filter value. With the BYOD storage project
unreachable, the -32603 sent to the client read "error sending request
for url (http://<storage-host>/rest/v1/memories?…&user_id=eq.<uuid>)".
Spans already redact it via Span.error → redactUrls; the response now
uses the same helper and keeps the reason ("Connection refused").
local-verify F-4 (EC-7): F4-reverify-write/-list now answer
"error sending request for url (<url>): … Connection refused". The
7 s (write) / 14 s (list) latency is postgrest-js's default network
retry (3 retries, exponential backoff, per query), shared with hosted
mode, and is left as is: disabling it trades transient-error resilience
for faster failure on a misconfiguration `lorekit doctor` already
diagnoses in milliseconds.
CLAUDE.md and llms.txt (template.md) said a read-only token on a write, or a write-only token on a read, gets -32001. The edge answers those with JSONRPC_FORBIDDEN (-32003, mcp-handler.ts) since the in-band authz split; -32001 is only for a missing, invalid or rotated token. docs/mcp-tools.md already had it right. llms.txt regenerated with nx generate:llms. local-verify F-5 (EC-3/EC-4): lk_ro write and lk_wo read both returned -32003.
|
The dashboard preview for this PR — redeployed on each push that changes the web app.
Preview always points at this PR's latest commit; Deployment is the immutable build of |
✅ No issues foundNo code changes since the last review at 086ee01; the edited description now matches the diff, and both prior threads are resolved. Checked: 0 of 10 dependent files traced Progress: open review threads 2 → 0 → 0 → 0 across the last 4 reviews What this change reaches — 13 changed exports · 10 dependent files · 0 checked · 10 not checked · 1 changed file imported by 1 fileflowchart LR
subgraph pr["Changed in this PR"]
s1["createBYODAdapter<br/>body changed"]:::changed
s2["bootstrap<br/>body changed"]:::changed
s3["handleMcp<br/>body changed"]:::changed
s4["resolveStorageAdapter<br/>body changed"]:::changed
s5["toolPurge<br/>body changed"]:::changed
s6["probeByodStorage<br/>added"]:::changed
smore["+7 more changed exports<br/>listed below"]:::changed
m1["packages/cli/src/commands/doctor.mjs<br/>file changed"]:::changed
end
s1c1["packages/mcp-core/src/index.ts<br/>? not checked"]:::unknown
s1c2["packages/mcp-core/src/db/storage-adapter.spec.ts<br/>? not checked"]:::unknown
s2c1["packages/cli/src/commands.mjs<br/>? not checked"]:::unknown
s3c1["supabase/functions/mcp/index.ts<br/>? not checked"]:::unknown
s4c1["supabase/functions/mcp/index.ts<br/>? not checked"]:::unknown
s5c1["supabase/functions/mcp/tool-dispatch.generated.ts<br/>? not checked"]:::unknown
s6c1["packages/cli/src/commands/ · 2 files<br/>? not checked"]:::unknown
s6c2["packages/cli/test/byod.test.mjs<br/>? not checked"]:::unknown
m1i1["packages/cli/src/commands.mjs<br/>imports this file"]:::imports
s1 -.-> s1c1
s1 -.-> s1c2
s2 -.-> s2c1
s3 -.-> s3c1
s4 -.-> s4c1
s5 -.-> s5c1
s6 -.-> s6c1
s6 -.-> s6c2
m1 -.-> m1i1
classDef changed fill:#eef2ff,stroke:#6366f1,color:#1e1b4b
classDef ok fill:#e7f6ec,stroke:#16a34a,color:#14532d
classDef partial fill:#fef9c3,stroke:#ca8a04,color:#713f12
classDef unknown fill:#f3f4f6,stroke:#9ca3af,stroke-dasharray:4 3,color:#374151
classDef bad fill:#fde8e8,stroke:#dc2626,color:#7f1d1d
classDef warn fill:#fff4e5,stroke:#d97706,color:#78350f
classDef imports fill:#f8fafc,stroke:#64748b,stroke-dasharray:4 3,color:#1e293b
Review details
Found Quality — produced 0 → posted inline 0 · cleared 0 · carried forward 0 · deferred 0 · below-bar 0 Run incremental · 0 lines in delta · tier deep · depth checkout Nothing to report — standards (skipped), optimality (skipped), measurability (skipped), severity, 0 files skipped.
|
… caller's id, not null Addresses @mthines's comment: #690 (comment) Refs: #690
…the matrix and troubleshooting too Addresses @mthines's comment: #690 (comment) Refs: #690
…onfiguration, which a retry won't clear Addresses @mthines's comment: pr-reviewer report #690 (comment) Refs: #690
…only the anon key Addresses @mthines's comment: pr-reviewer report #690 (comment) Refs: #690
…lit into BYOD misconfig, server fault and isError scope rows Addresses pr-reviewer report finding (docs/install.md:271): #690 (comment) Refs: #690
…failure
The health function answers 200 {"status":"ok","db":"ok",...} and
503 {"status":"degraded","db":"error",...}. install.md's Step 6 and
smoke-test blocks, the self-host page, and the skill all promised
"db":true / "db":false, so a self-hoster comparing output literally
would read a healthy deploy as wrong. Found by local-verify.
The Troubleshooting row is left for after #690 lands: that table sits
inside #690's install.md hunk.
🎬 BYOD walkthrough: local verificationbyod-walkthrough.mp4This 83-second walkthrough runs on two local Supabase stacks: a function project and a separate storage project.
|
) * feat(self-host): add /lorekit-self-host skill, a public self-host docs page, and self-host-correct install.md Today's install.md points a self-hoster at the hosted project in nine places, a lorekit.io-minted token can't authenticate their own project, and the default deploy only ships mcp + health (missing memories, which hooks, remote CLI commands, and the dashboard REST API depend on). Adds a guided /lorekit-self-host runbook, a public /docs/self-host comparison page, and fixes install.md to use <your-project-ref> throughout with an all-functions deploy step. * test(install): cover the self-host endpoint path and lock the hosted default Extends install.test.mjs — the file that already owns --endpoint/.mcp.json coverage — rather than inventing a new harness: a self-hosted --endpoint is written verbatim and its REST base derives correctly via mcpToRestBase, omitting --endpoint still falls back to the hosted URL, and the new lorekit-self-host skill is never scaffolded by a fresh install (D2). * address review comment: use port 4200 for the locally-run dashboard Addresses @mthines's comment: #694 (comment) Refs: #694 * address review comment: run BYOD bootstrap/doctor with storage vars inline Addresses @mthines's comment: #694 (comment) Refs: #694 * address review comment: add --create-dirs to the self-host skill curl one-liner Addresses @mthines's comment: #694 (comment) Refs: #694 * address review comment: correct what reads a committed mcp.endpoint Addresses @mthines's comment: #694 (comment) Refs: #694 * address review comment: carve out self-host instructions from the no-placeholder endpoint rule Addresses @mthines's comment: pr-reviewer report #694 (comment) Refs: #694 * address review comment: word the BYOD "Never" rule version-neutrally Addresses @mthines's comment: pr-reviewer report #694 (comment) Refs: #694 * address review comment: stop presenting the hosted-URL constant as an env var Addresses @mthines's comment: pr-reviewer report #694 (comment) Refs: #694 * docs(self-host): hooks check ~/.claude.json before mcp.endpoint * docs(decisions): record the self-host placeholder exception * docs(self-host): health returns "db":"ok", and a 503 "db":"error" on failure The health function answers 200 {"status":"ok","db":"ok",...} and 503 {"status":"degraded","db":"error",...}. install.md's Step 6 and smoke-test blocks, the self-host page, and the skill all promised "db":true / "db":false, so a self-hoster comparing output literally would read a healthy deploy as wrong. Found by local-verify. The Troubleshooting row is left for after #690 lands: that table sits inside #690's install.md hunk. * docs(self-host): put the hosted URL in a code block so the page fits a phone At 390px the inline 57-character URL in the opening callout could not wrap, so the whole page scrolled sideways. A fenced block scrolls on its own, as the other docs pages already do for this URL. Found by local-verify. * docs(self-host): say exactly when install falls back to the hosted URL A plain re-run on an already-installed project returns early and leaves .mcp.json alone. The hosted fallback applies when install WRITES the entry without --endpoint: a --force reinstall, a new project, a global install, or a missing entry. The advice (always pass --endpoint or export LOREKIT_MCP_URL) is unchanged. Found by local-verify. * docs(self-host): doctor can't catch an endpoint missing /mcp, so check it lorekit doctor checks connectivity and auth over REST, and the REST base drops the /mcp suffix, so --endpoint .../functions/v1 passes every doctor check while mcp-remote gets a 404. Step 8's gate now also checks the suffix and points at Step 9 as the real MCP check. Found by local-verify. * docs(self-host): show the delete call Step 9 asks for Step 9 said to write, read, then delete the test lesson but showed no delete call. Add memory.delete and note that it archives, so a later read returns null. Found by local-verify. * docs(self-host): --hooks, --mcp-json and skill upgrades also rewrite the endpoint The early return in install only fires for a plain re-run on a fully installed project; --hooks, --mcp-json, --completions, --force, or a missing skill all rewrite the MCP entry with the hosted URL when no --endpoint is given. Say that instead of listing a subset. * docs(self-host): list --completions among the install paths that rewrite .mcp.json Also align self-host.mdx with install.md (--mcp-json, the plain re-run exception) and make the skill description admit Step 9's throwaway write/read/delete round-trip. * docs(self-host): say the GitHub App needs its own registration when self-hosting
Why
BYOD did not work: its hand-written
bootstrap.sqlhad drifted (6 of 48 edge RPCs; amemory_writethe edge's calls cannot resolve), everylk_*write hitmemories_user_id_fkeybecause the storage project'sauth.usersnever holds the function project's users, an anon-key client read back nothing, and Supabase-session callers read every row in the storage project.What changed
bootstrap.sqlis generated fromsupabase/migrations/(supabase/byod/generate.mjs), ends withstorage-overlay.sql(drops every public FK intoauth.users), records migration history forsupabase db pushupgrades;supabase:testfails on drift.mcpfunction requiresLOREKIT_STORAGE_SERVICE_KEYin BYOD mode — a missing one is answered in-band as-32603naming the variable, not a bodyless 500 — and stamps session (JWT) callers with their own id in hosted and BYOD mode alike; hosted session writes were previously ownerless.-32603messages are URL-redacted, and the docs andllms.txtnow say a token-tier denial is-32003(not-32001) and that a BYOD misconfiguration's-32603does not clear on retry.doctor/bootstrapprobe storage with plainfetchand stop printing the service key;docs/byod.md/docs/cli.mdmatch the code, anddocs/tasks/self-host-without-supabase-cloud.mdproposes a Docker self-host bundle.How to verify
node --test supabase/byod/generate.test.mjs packages/cli/test/byod.test.mjsplus themcp-corestorage-adapter andmcp-guardsspecs. Also verified end to end on two local Supabase stacks: dashboard-minted token and session writes land in the storage project, hosted mode unchanged.Notes for reviewers
The drift gate rides
nx affected -t testbecause the app that pushed this cannot edit workflows. In BYOD mode the dashboard and REST API still read only the function project — documented indocs/byod.md→ Limitations.