Skip to content

feat(b1): Railway ingestion service owns chunk persistence + terminal document status (forwarded user JWT, no service-role key)#70

Open
tornidomaroc-web wants to merge 1 commit into
mainfrom
feat/b1-railway-owns-chunk-persistence
Open

feat(b1): Railway ingestion service owns chunk persistence + terminal document status (forwarded user JWT, no service-role key)#70
tornidomaroc-web wants to merge 1 commit into
mainfrom
feat/b1-railway-owns-chunk-persistence

Conversation

@tornidomaroc-web

Copy link
Copy Markdown
Owner

Implements enabler (b1) from register #50 and logs it as register #51.

The Railway ingestion service now runs convert → chunk → embed → persist chunks → write the terminal documents status, and returns only a small ack {document_id, chunk_count, status}. src/app/api/ingest/route.ts still creates the row, forwards, and awaits — but no longer receives or inserts chunks, and no longer materializes the 9-13× return blob register #50 measured (every chunk's 1024-float embedding plus the full markdown, via await pyResponse.json()).

This is the B6 enabler, not B6. Ingestion stays synchronous — Next still awaits Railway. It is also not B5b progress: the B5b §1 row stays ⬜.

Auth: Option B (forwarded user JWT), not a service-role key

Railway writes as the uploading user. Next lifts the already-verified session's access_token and forwards it in X-Supabase-Token (Authorization still carries the service-to-service INGESTION_TOKEN, so the two credentials stay separate). The service builds a per-request Supabase client from the public anon key + that bearer, so auth.uid() resolves to the uploader and the existing RLS policies apply unchanged (001_initial_schema.sql:53, 20260501_rag_pgvector.sql:31-41).

Why not a service-role key — register #45 blast radius. #45 proved this exact service was publicly duplicable with a shared bearer token. An RLS-bypassing credential living in that process would have upgraded #45 from "a duplicated converter" to a full-database read/wipe breach — precisely the failure backfill.py's own docstring forbids. No SUPABASE_SERVICE_ROLE_KEY was added to the service, its Dockerfile, its env, or the route.

The client is built per request, never at module level: the client object holds the bearer header, so a shared one would leak one user's token into another user's writes.

Mechanism verified offline on the pinned supabase==2.29.0 before shipping — AsyncClientOptions(headers={'Authorization': ...}) reaches PostgREST as apiKey: <anon> + Authorization: Bearer <user JWT>, and supabase-py's create() skips its own session lookup when that header is supplied, so the forwarded token is the client's only identity.

Rename, and why deploy order matters

/convert/ingest. Both the request shape (now needs document_id, kb_id, the user token) and the response shape are incompatible with the old contract, so a version-skewed deploy gets a loud 404 instead of an old Next silently overwriting markdown_content with null on a document the service already finished correctly. Either deploy order is loud and recoverable; uploads inside the window fail to status='error'.

Hardening that rode along

  • Target row is read through RLS before any expensive work — an invisible row is a 404, so a foreign document_id never costs a Voyage batch.
  • kb_id is taken from the database, and a caller-supplied mismatch is rejected — a holder of INGESTION_TOKEN cannot file chunks under one KB while pointing them at a document in another.
  • The ready-update checks it matched a row: an RLS-filtered UPDATE returns 200 with zero rows, not an error, and would otherwise ack ready for a document whose status never moved.
  • Persistence reuses backfill.py's idempotent shape (delete().eq(document_id) then batch-insert at 50), so a retry of a half-written document converges.
  • /health reports supabase_configured as a boolean only — never the URL or key.

Error ownership moved with the work

Railway writes its own status='error' + error_message. Next keeps unconditional error writes only pre-forward; every post-forward failure — including a thrown or timed-out fetch, which previously fell to the outer catch and stranded the row at processing forever — now writes through a .eq('status','processing') guard.

That closes the orphan class without opening its mirror image: if Railway succeeded and only the ack was lost, the row is already ready and the guarded write is a no-op rather than stomping a correct document into error.

Honest residual: in that lost-ack case the browser is told the upload failed while the document is in fact ready. Data is correct; the message is pessimistic; router.refresh() shows the real row. Fixing the message is B6's territory.

Not in this PR

  • B6 itself — fire-and-forget. Next still awaits.
  • backfill.py's deploy posture — untouched, still a local-only script, still not copied into the image.
  • PIVOT_PLAN.md — untouched. PROGRESS.md:86 (B5b ⬜) — untouched.

Checks

tsc --noEmit clean. main.py compiles. No migration and no DB change, so db-types cannot drift by construction. Docs: register row #51, one new §7 block (20 → 21, additions-only, all 20 landed entries byte-identical — verified by sha256 of §7-minus-the-new-block against main).


Pre-merge verification (Abo Jad) — EIGHT items, all required

Verification must go through the authenticated preview UI or a read-only SQL query. The preview is SSO-gated — curl cannot reach it anonymously, and an unauthenticated request produces the SSO page, not a result. Uploads burn a rate-limit credit and the #22 free tier: use a throwaway subject and delete the rows after.

Deploy Railway and Vercel both from this branch before testing — the endpoint rename means a mixed deploy 404s (loudly, by design).

  1. Image builds without an import crash. Railway build succeeds and the container starts — this PR adds supabase==2.29.0 to requirements.txt and imports acreate_client/AsyncClient/AsyncClientOptions at module scope, so a bad resolve is a boot-time crash, not a runtime one. Confirm /health returns 200.
  2. Railway env present. GET /health returns "supabase_configured": true. If it is false, SUPABASE_URL and/or SUPABASE_ANON_KEY are missing and every upload will fail closed at 503. Set SUPABASE_ANON_KEY to the anon/publishable key — never the service-role key.
  3. Authenticated end-to-end upload reaches status='ready'. Upload through the preview UI; then confirm the documents row has status='ready', embedding_status='ready', a chunk_count matching the actual chunks row count, and non-null markdown_content.
  4. chunks rows present with a non-null vector(1024). Confirm the chunk rows exist for that document_id and that embedding is non-null with 1024 dimensions (vector_dims(embedding)) — this is the check that the embeddings actually crossed into Postgres as vectors rather than as null/text.
  5. The Ask/agent retrieval path returns results for the new doc. Ask a question in that subject whose answer is only in the new document; confirm match_chunks retrieves from it. This is the real proof that persistence-by-Railway produced chunks the existing retrieval path can use.
  6. A forced failure leaves status='error' with no partial chunk set and no stuck processing. Force a failure (e.g. temporarily break VOYAGE_API_KEY on Railway, or upload a file that fails conversion) and confirm the row lands at status='error' with an error_message, zero chunk rows for that document, and nothing left at processing. Restore the env afterwards.
  7. study_events kind='material_uploaded' fires exactly once for one successful upload — not zero (the emit is still gated on the ack) and not twice.
  8. BLOCKING SECURITY CHECK — no service-role key reached Railway. Inspect the Railway service's environment variables directly and confirm no SUPABASE_SERVICE_ROLE_KEY and no other RLS-bypassing credential is set, and that SUPABASE_ANON_KEY holds the anon key (decode it: the JWT's role claim must be anon, not service_role). If a service-role key is present, do not merge — remove and rotate it first. Repo-side proof is already in this PR (grep shows zero service-role env reads in main.py, the Dockerfile, or the route; the only hits are prose forbidding it and the local-only backfill.py, which the Dockerfile does not copy into the image) — item 8 verifies the live environment, which the repo cannot prove.

Do not merge until all eight pass.

🤖 Generated with Claude Code

… status

Enabler (b1) from register #50. The ingestion service now runs
convert -> chunk -> embed -> persist chunks -> write the terminal
`documents` status, and returns only a small ack
{document_id, chunk_count, status}.

/convert is renamed /ingest: both the request shape (it now needs
document_id, kb_id and the user's token) and the response shape are
incompatible with the old contract, so a version-skewed deploy gets a
loud 404 instead of an old Next silently overwriting markdown_content
with null on a document the service already finished correctly.

Auth is Option B - a forwarded user JWT, NOT a service-role key. Next
lifts the already-verified session's access_token and forwards it in
X-Supabase-Token (Authorization still carries INGESTION_TOKEN); the
service builds a per-request Supabase client from the public anon key
plus that bearer, so auth.uid() is the uploader and the existing RLS
policies apply unchanged. Register #45 proved this exact service was
publicly duplicable with a shared bearer token, so an RLS-bypassing
credential in that process would have been a full-database breach -
the failure backfill.py's docstring forbids. The client is built per
request, never module-level, because it holds the bearer header.

Hardening that rode along: the target row is read through RLS before
any expensive work (invisible row -> 404, so a foreign document_id
never costs a Voyage batch); kb_id is taken from the database and a
caller-supplied mismatch is rejected; the ready-update checks it
matched a row, since an RLS-filtered UPDATE returns 200/0-rows rather
than an error; persistence reuses backfill.py's idempotent shape
(delete().eq(document_id) then batch-insert at 50).

Error ownership moves with the work. Railway writes its own
status='error' + error_message. Next keeps unconditional error writes
only pre-forward; every post-forward failure - including a thrown or
timed-out fetch, which previously fell to the outer catch and stranded
the row at 'processing' forever - now writes through a
.eq('status','processing') guard. That closes the orphan class without
opening its mirror image, where Next would stomp 'error' over a 'ready'
row the service had already written before the ack was lost.

Next no longer receives or inserts chunks, so the 9-13x return blob
(every chunk's 1024-float embedding plus the full markdown, materialized
by await pyResponse.json()) is gone from the Next function's memory.

Not in this PR: B6 itself - ingestion stays synchronous, Next still
awaits. backfill.py's deploy posture is untouched (still local-only).
B5b (§1) stays unstarted; (b1) is the B6 enabler, not B5b progress.

Docs: register row #51, one new §7 block (20 -> 21, additions-only,
all landed entries byte-identical), header lines 8-10 refreshed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
knowflow Ready Ready Preview, Comment Jul 25, 2026 7:00pm

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.

1 participant