From 8407fdf401cfd86e18a4f236828256c74861d360 Mon Sep 17 00:00:00 2001 From: Jiri Puc Date: Mon, 31 Aug 2026 23:48:49 +0200 Subject: [PATCH] feat(e0): refuse an unroutable MIME at the E0 gate instead of dead-lettering it (D104) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The conversion router was consulted for the first time inside the convert worker. An upload whose MIME had no route was admitted, hashed, written to immutable raw storage, returned as a normal accepted-not-ready receipt, and only then dead-lettered through UnroutableMimeError -> mark_version_failed -> NonRetryableHandlerError. The caller discovered a terminal state by polling, having been told the upload was accepted. The waste was knowable at admission: the route table was already in memory. And the caller cannot undo it — identical bytes are a no-op (D55) and no API lets them request reprocessing, so recovery is operator work only (`remember ops replay` reopens the dead-lettered work row, after which a now-routable version converts normally). Each wrong guess costs a durable raw object and an operator ticket to discover what a set lookup already knew. The gate is in E0, not on a surface, and that placement is the decision. Three ingresses reach E0 without sharing a handler: HTTP POST /ingest, the local MCP ingest tool, and the connector sync worker, the latter two calling the composed port directly. A check on the HTTP handler would have left two of three paths still admitting bytes the convert stage can only dead-letter, while looking fixed. UploadIngestor is the one object all three write through — the library boundary already requires that ingestion always writes through E0 — so the check sits in _guard_ingest beside the D74 guard, and surfaces only render it. Two details make that placement hold rather than merely sound right. The route table is a REQUIRED argument: a default of "no check" would make the invariant as strong as every composer remembering to pass it, and every deployment has a table (the settings default is the stock text one), so omission expresses only a mistake. And routability is decided BEFORE the D74 admission query — both orders are safe since neither writes bytes, but deciding it first avoids an admission query for a request that cannot be accepted and stops a forget-state error from masking a plain "we do not convert that". This is not a media defect. The mechanism keys on absence from the table, so it fired identically for audio, video, images, office documents and archives. The table is the only authority and the gate cannot be looser than the worker: build_conversion_routes refuses composition on an unknown adapter, so a process's router keys are exactly its configuration's keys, and the gate does the same exact lookup on the same string. That guarantee is per-configuration, not global — gate and worker are separately composed, so a route-table change leaves a window where one has restarted and the other has not, which is why UnroutableMimeError stays in the worker and stays non-retryable. Surfaces: HTTP renders 415 with the accepted set. map_backend_error maps the typed error for local MCP and the 415 for remote MCP, so an agent gets unsupported_media_type with an actionable next step on both, instead of internal_error locally and a flattened engine_client_error remotely. Not in scope: a routing verdict, not a content one. An MP3 labelled text/plain still passes the gate and fails in the converter, correctly. Tests assert store.writes == 0 on refusal — the exception alone would not prove it, since the defect was that bytes became durable. Both E0 entry points are covered; one test proves D74 is not consulted for an unroutable input; one locks the gate/router key-set equivalence. Two structural audits guard the shape itself: that record_upload has exactly one caller, so a future ingress cannot quietly become a second door, and that routable_mimes keeps no default. Docs: the API reference documents the 415 and notes MCP and connector sync refuse the same types; configuration and troubleshooting no longer claim an unrouted MIME dead-letters on convert, and troubleshooting says how to recover versions dead-lettered before this existed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KfBUpQuuehJKf2kbNa2D2D --- .github/ci/check_test_inventory.py | 4 +- .github/ci/unit-paths.txt | 9 +- decisions.md | 118 ++++++++ plan/analysis/unroutable_mime_preflight.md | 279 ++++++++++++++++++ plan/designs/e0_files_design.md | 45 +++ src/rememberstack/model/conversion.py | 21 +- src/rememberstack/profiles/selfhost.py | 5 + src/rememberstack/surfaces/http_api.py | 57 +++- .../surfaces/mcp_memory_tools.py | 41 +++ src/rememberstack/surfaces/sdk.py | 58 +++- src/rememberstack/workers/e0.py | 44 ++- src/tests/surfaces/test_client_sdk.py | 38 +++ .../surfaces/test_ingest_routability_http.py | 135 +++++++++ src/tests/surfaces/test_mcp_memory_tools.py | 38 +++ src/tests/surfaces/test_retrieval_api.py | 1 + src/tests/workers/test_e0_chain.py | 45 ++- src/tests/workers/test_e1_chain.py | 1 + src/tests/workers/test_e2_chain.py | 1 + src/tests/workers/test_e3_chain.py | 1 + src/tests/workers/test_ingest_admission.py | 2 + .../test_ingest_gate_is_the_only_door.py | 109 +++++++ src/tests/workers/test_ingest_routability.py | 194 ++++++++++++ .../workers/test_lifecycle_reconciliation.py | 1 + src/tests/workers/test_reuse_lifecycle.py | 5 +- src/tests/workers/test_watch_loop.py | 1 + website/src/app/docs/configuration/page.mdx | 4 +- .../src/app/docs/ingestion/pipeline/page.mdx | 2 +- website/src/app/docs/project-status/page.mdx | 25 ++ website/src/app/docs/reference/api/page.mdx | 25 ++ website/src/app/docs/reference/mcp/page.mdx | 5 + website/src/app/docs/troubleshooting/page.mdx | 4 +- 31 files changed, 1277 insertions(+), 41 deletions(-) create mode 100644 plan/analysis/unroutable_mime_preflight.md create mode 100644 src/tests/surfaces/test_ingest_routability_http.py create mode 100644 src/tests/workers/test_ingest_gate_is_the_only_door.py create mode 100644 src/tests/workers/test_ingest_routability.py diff --git a/.github/ci/check_test_inventory.py b/.github/ci/check_test_inventory.py index 7c2626706..c6d3f0aa6 100755 --- a/.github/ci/check_test_inventory.py +++ b/.github/ci/check_test_inventory.py @@ -56,9 +56,7 @@ def main() -> int: errors.append("test files not in any inventory:\n " + "\n ".join(orphans)) unreachable = [ - p - for p in integ - if not any(p.startswith(prefix) for prefix in SOFT_PREFIXES) + p for p in integ if not any(p.startswith(prefix) for prefix in SOFT_PREFIXES) ] if unreachable: errors.append( diff --git a/.github/ci/unit-paths.txt b/.github/ci/unit-paths.txt index a63108a68..be3169c60 100644 --- a/.github/ci/unit-paths.txt +++ b/.github/ci/unit-paths.txt @@ -2,10 +2,10 @@ src/tests/adapters/test_bounded_postgres_read.py src/tests/adapters/test_codex_writer.py src/tests/adapters/test_control_plane_spend_lease.py src/tests/adapters/test_hashed_bearer_auth.py -src/tests/adapters/test_postgres_p1.py src/tests/adapters/test_minio_store.py src/tests/adapters/test_mistral_ocr.py src/tests/adapters/test_openrouter.py +src/tests/adapters/test_postgres_p1.py src/tests/adapters/test_selfhost_forget.py src/tests/adapters/test_selfhost_git.py src/tests/adapters/test_selfhost_purge.py @@ -55,15 +55,16 @@ src/tests/surfaces/test_client_sdk.py src/tests/surfaces/test_cost_export_contract.py src/tests/surfaces/test_forget_admission.py src/tests/surfaces/test_graph_http_api.py +src/tests/surfaces/test_ingest_routability_http.py src/tests/surfaces/test_login.py src/tests/surfaces/test_mcp_memory_tools.py src/tests/surfaces/test_postgres_graph_sql.py src/tests/surfaces/test_query_sandbox_graph_d98.py +src/tests/test_d98_removal_audit.py src/tests/test_forget_port_contract.py src/tests/test_port_inventory_and_conformance.py src/tests/test_port_model_values.py src/tests/test_queue_port_contract.py -src/tests/test_d98_removal_audit.py src/tests/test_smoke.py src/tests/workers/test_chunk_level_extract.py src/tests/workers/test_claim_valid_time.py @@ -71,10 +72,12 @@ src/tests/workers/test_claimify_loss_ledger.py src/tests/workers/test_d79_structure_route.py src/tests/workers/test_d79_summaries.py src/tests/workers/test_d79_summary_consumption.py +src/tests/workers/test_e3_bare_head_noun.py src/tests/workers/test_e3_claim_normalize_fanout.py src/tests/workers/test_e3_entity_obs_flush_fanout.py -src/tests/workers/test_e3_bare_head_noun.py src/tests/workers/test_extraction_temperature.py src/tests/workers/test_hard_forget_handler.py src/tests/workers/test_hard_forget_service.py src/tests/workers/test_ingest_admission.py +src/tests/workers/test_ingest_gate_is_the_only_door.py +src/tests/workers/test_ingest_routability.py diff --git a/decisions.md b/decisions.md index 80d5bdf14..c3309ba18 100644 --- a/decisions.md +++ b/decisions.md @@ -4793,3 +4793,121 @@ unread; fixing only the sort order while keeping the flag. rest of D21 (connected-components-to-gather, HAC distance-cut, nDR incremental re-decision, `merge_events`, `merged_into`, `resolution_exclusions`) is unchanged. Does not change D95, D99, or D100. + +## D104. Ingest refuses a MIME type the deployment cannot convert + +**Decision (2026-08-31).** Routability is an **admission** property, not a +conversion-time discovery. **E0 itself** compares the declared `mime` against +the deployment's configured conversion route table and raises +`UnroutableMimeError` for an absent type, carrying the refused MIME and the +accepted set. Nothing is written, no version is created, no lineage is touched. +Surfaces render it in their own idiom; HTTP `POST /ingest` returns **415 +`unsupported_media_type`**. + +**The gate is in E0, not on a surface, and that placement is the decision.** +Three ingresses reach E0 without sharing a handler: HTTP `POST /ingest`, the +local MCP `ingest` tool, and the connector sync worker, the latter two calling +the composed ingest port directly. A check on the HTTP handler would leave two +of three paths still admitting bytes the convert stage can only dead-letter, +while appearing fixed. `UploadIngestor` is the one object all three write +through — which is what the library boundary already requires: ingestion always +writes through E0, and no extension point may bypass an invariant. The check +therefore sits in `_guard_ingest`, beside the D74 admission guard that is there +for the same reason. + +The table is the only authority. `conversion_routes` is deployment policy +(D61), and `build_conversion_routes` refuses composition when a route names an +unknown adapter — so a process's router key set is exactly the key set of the +configuration it was composed with, and a membership test against that +configuration is a membership test against that router. The gate performs the +same **exact** dictionary lookup the router performs, on the same string: any +MIME normalization belongs in the router where both callers inherit it, never +at the gate alone, because a gate more permissive than the worker would +recreate the dead letters this decision removes. + +The guarantee is per-configuration, not global. The gate and the convert worker +are separately composed processes, so a route-table change has a window in +which one has restarted and the other has not — the same race the worker's +`UnroutableMimeError` still covers. + +**The table is a required argument, not an option.** Every deployment has a +route table — the settings default is the stock text table — so omission +expresses nothing except a composition that forgot, and a default would let any +consumer silently opt out. An invariant with an opt-out is not an invariant, so +`UploadIngestor` refuses to construct without one. + +**Routability is decided before the D74 per-source admission query.** It is +the cheaper and more fundamental question: an input no route accepts is refused +whatever that source's forget state. Deciding it first avoids an admission +query for a request that cannot be accepted, and stops a forget-state error +from masking a plain "we do not convert that". D74 still runs before any byte +is written. This ordering is internal to the gate: a deployment-wide +availability barrier — an in-progress forget that makes the whole deployment +503 — is composed above the surfaces and still answers first, because it says +the deployment cannot serve the request at all. + +**Why.** The router was previously consulted for the first time inside the +convert worker, on every ingress. An unroutable upload was accepted, hashed, written to immutable +raw storage, returned as a normal accepted-not-ready receipt, and only then +dead-lettered through `UnroutableMimeError` → `mark_version_failed` → +`NonRetryableHandlerError`. The caller discovered a terminal state by polling. +The waste was knowable at submission: the route table was already in memory. + +The decisive property is who can recover. Under D55 identical bytes return the +existing version, so a caller cannot repair a dead-lettered upload by sending +the file again, and no API lets them request reprocessing. Recovery exists only +as operator work: the convert failure dead-letters a work-ledger row, and +`remember ops replay` (`WorkLedger.replay_dead_letter`) reopens one such row +with a fresh attempt allowance, after which a now-routable version converts +normally. Recovery is therefore out-of-band, per-document, and unavailable to +the person who caused it — each wrong guess costs a durable raw object, a +committed reservation in the managed product, and one operator ticket, to +discover something a set lookup already knew. + +This is not a media decision. The mechanism keys on absence from the table, so +it fired identically for audio, video, images, office documents and archives — +every type outside a deployment's configured routes. + +**Rejected.** Place the check on the HTTP ingest handler (leaves the MCP tool +and connector sync bypassing it — two of three ingresses still dead-lettering +while the defect looks fixed); default the route table to "no check" (makes the +invariant as strong as every composer remembering it); keep discovering +unroutable input in the worker; hard-code a known-media deny list at ingest (drifts from the table immediately, and is +wrong for any deployment that configures a route it does not know); build the +`ConversionRouter` in the API process to query it (constructing adapters would +force provider credentials into a process that performs no conversion); +content-sniff at admission and route on the detected type (a decode-and-inspect +step with its own cost and sandboxing needs — it answers "is this really what +you said it is", a different and more expensive question, and must not gate the +cheap one; the two compose, cheap check first). + +**Scope of the guarantee.** *No upload is admitted under a MIME this +deployment cannot convert* — not *no unconvertible version is ever created*. +The gate reads the declared MIME; the convert stage reads +`content_objects.mime`, which is first-write-wins per content hash. They +diverge only when the same bytes are ingested again under a different MIME and +the first-seen one has since become unrouted, and closing that would cost a +content-object lookup on every ingest. That trade is recorded as an open +question in the analysis rather than paid or dismissed here, and the worker's +`UnroutableMimeError` still covers the case. + +**Consequences.** `POST /ingest` gains one published refusal, 415 +`unsupported_media_type`. The MCP ingest tool and the connector sync worker +raise `UnroutableMimeError` where they previously returned an accepted receipt; +a connector pointed at a directory of unconvertible files now fails those items +at admission instead of dead-lettering them one stage later. Callers that +previously received an accepted receipt for an unconvertible type now receive a +synchronous refusal, which is a behavior change in their favour but a change +nonetheless. `UnroutableMimeError` gains optional `mime` and `supported_mimes` +attributes so a surface can render it; both are None when the raiser did not +know the table. It remains in the worker and remains non-retryable, covering +the window where a deployment's route table changes between admission and +conversion. + +**Design.** `plan/designs/e0_files_design.md` §3. + +**Analysis.** `plan/analysis/unroutable_mime_preflight.md`. + +**Amends.** Adds an admission check to the D38 conversion router's contract. +Preserves D55 lineage and identical-byte semantics, D61 deployment policy, D65 +converter envelope, and the worker's D12 non-retryable handling unchanged. diff --git a/plan/analysis/unroutable_mime_preflight.md b/plan/analysis/unroutable_mime_preflight.md new file mode 100644 index 000000000..5ca2f09a0 --- /dev/null +++ b/plan/analysis/unroutable_mime_preflight.md @@ -0,0 +1,279 @@ +# Unroutable MIME types: why ingest must refuse them at the gate + +**Status:** analysis (non-binding). Supports **D104** and the `e0_files_design.md` +§3 amendment that makes routability an admission check. +**Date:** 2026-08-31. Engine evidence read at `origin/main` `9ea2953b` +(post-`v0.9.0`) and verifiable in this tree. Statements about the managed +fleet's live route table and its control-plane reservation behaviour come from +a **different repository** (`ultimate-memory-cloud` at `2db8c2c`, +`infra/prod.yaml` and `spend_safety/dp_proxy.py`) and cannot be checked here; +they are marked where they appear and none of the engine reasoning depends on +them. + +--- + +## 1. What happened before D104 + +This section describes the **pre-D104 baseline** — the behaviour at parent +commit `9ea2953b`, which this analysis argued should change. It is written in +the present tense of that commit and is no longer true of `main`. + +The conversion router was consulted for the first time **inside the convert +worker**, long after the upload had been accepted. + +`ConversionRouter.converter_for` (`core/conversion.py`) resolves a MIME by +**exact dictionary lookup** and raises `UnroutableMimeError` when the key is +absent. There is no non-raising way to ask the router whether a type is +routable — `converter_for` is its only query. + +The ingest surface never asks. `_mount_ingest` in `surfaces/http_api.py` +validates the body size, the `source_kind`/`source_ref` pairing, the principal +headers, and the timezone of `source_modified_at`. It does not look at whether +`mime` can be converted. The upload is admitted, hashed, written to immutable +raw storage, and returned as a normal accepted-not-ready receipt with a durable +`version_id`. + +The failure surfaces one stage later, in `workers/e0.py::ConvertHandler.handle`: + +```python +try: + converter = self._router.converter_for(mime=source.mime) +except UnroutableMimeError as err: + self._catalog.mark_version_failed(version_id=source.version_id, error=str(err)) + raise NonRetryableHandlerError(str(err)) from err +``` + +So the sequence a caller actually experiences is: **accepted → stored → +(in the managed product, a spend reservation committed) → silently dead-lettered +→ discovered only by polling readiness.** + +## 2. Why this is worse than an ordinary error + +Three properties compound, and the third is the one that makes it worth fixing +before anything else. + +**The receipt is a lie by omission.** `accepted` means E0 stored the bytes, and +that is literally true. But the caller reads it as "this will become memory", +because for every routable type it does. Nothing in the response distinguishes +"accepted and will convert" from "accepted and cannot possibly convert". + +**The cost is charged before the impossibility is discovered.** In the managed +product the control-plane proxy commits its reservation on engine acceptance. +The engine has by then written a durable raw object it will never derive +anything from. Both the money and the storage are spent on work that was +knowably impossible at the moment of submission — the route table was already +in memory. + +**The caller cannot fix it themselves.** D55 makes identical bytes a no-op: a +resubmission returns the existing version rather than creating a new one, so +uploading the file again after the deployment gains a route changes nothing. +The caller has no API by which to ask for the version to be reprocessed. + +Recovery exists, but only as **operator** work. The convert stage's failure +dead-letters a work-ledger row, and `WorkLedger.replay_dead_letter` — reached +through `remember ops replay` — reopens exactly one such row with a fresh +attempt allowance. Once a route exists, an operator can replay the row and the +convert handler will route the version normally; `mark_version_failed` does not +bar a later promotion. + +So the honest statement is not "the damage is permanent" — it is that recovery +is **out-of-band, per-document, and unavailable to the person who caused it**. +Each mistaken upload leaves a durable raw object, a committed reservation in the +managed product, a caller with no self-service path, and one more row an +operator must be asked to replay. That burden scales with the number of wrong +guesses, and every one of them was refusable at admission for free. + +That asymmetry is the argument. The check costs a set lookup; the alternative +costs storage, money, a confused caller, and an operator ticket. + +## 3. This is not about any one format + +The defect was found while analysing audio, but audio is only the instance that +happened to be looked at. The mechanism keys on *absence from the route table*, +so it fires identically for every type the deployment has not configured. + +The stock table (`STOCK_CONVERSION_ROUTE_NAMES`) is two entries, +`text/markdown` and `text/plain`. The managed fleet's live table, set as +deployment policy, is those two plus `application/pdf` routed to `mistral_ocr`. +Everything else a caller might plausibly send — audio, video, images, office +documents, archives, JSON, CSV — takes the accepted-then-dead-letter path today, +in exactly the same way, for exactly the same reason. + +A per-format fix would therefore be four fixes that each solve a quarter of one +problem. The check belongs on the route table, once. + +## 4. Where the check must live + +Two placement questions, and getting the first wrong makes the second moot. + +### 4.0 In E0, not on a surface + +The obvious place to put an admission check is the HTTP ingest handler, and it +is the wrong one. Three ingresses reach E0, and they do not share a handler: + +- HTTP `POST /ingest` (`surfaces/http_api.py`), which the SDK and CLI use; +- the local MCP `ingest` tool (`surfaces/mcp.py`), which calls the composed + ingest port directly; +- the connector sync worker (`workers/sync.py`), which calls + `ingest_observed` directly on the same port. + +A check on the HTTP handler would leave the MCP tool and every connector still +admitting bytes the convert stage can only dead-letter — the defect would +survive on two of three paths while appearing fixed. + +`UploadIngestor` is the single object all three write through, which is what +CLAUDE.md's library-boundary rule already asserts: *ingestion always writes +through E0*, and no extension point may bypass an invariant. So the check +belongs in `UploadIngestor._guard_ingest`, beside the D74 admission guard that +is there for exactly the same reason. Surfaces then only *render* the refusal +in their own idiom — HTTP as 415, a tool call as a typed error. + +Two details make that placement actually hold rather than merely sound right. + +The route table is a **required** constructor argument. A first draft defaulted +it to "no check", which would have made the gate exactly as strong as every +composer remembering to pass it; the shipped self-host profile would have been +fine and any other composition silently would not. Every deployment has a route +table — the settings default is the stock text table — so omission expresses +nothing but a mistake, and refusing to construct is the honest response. + +And routability is decided **before** the D74 per-source admission query +rather than after. Both orders are safe, since neither writes bytes, but +deciding routability first avoids an admission query for a request that cannot +be accepted, and stops a forget-state error from masking a plain "we do not +convert that" — a caller sending an unsupported type should be told that, not +handed an unrelated failure whose cause they cannot act on. + +The claim is about the gate's own two checks, not about everything that can +refuse a request. A deployment-wide availability barrier — an in-progress +forget that makes the whole deployment answer 503 — is composed above the +surfaces and still runs first. That is correct: it says the deployment cannot +serve anything right now, which is a different statement from "we do not +convert that type". + +### 4.1 Against the deployment's own route table + +The route table is **deployment policy** (D61): a deployment declares +`conversion_routes` as a `MIME → adapter-name` map, and +`build_conversion_routes` materializes it. Any admission check must read that +same policy rather than embed a guess about what "should" be convertible. + +| Option | Verdict | +| --- | --- | +| Keep discovering it in the worker (status quo) | Rejected. The information is available at admission; spending storage and money to rediscover it is pure waste, and the caller cannot recover from it without an operator (§2). | +| Hard-code a "known media types" deny list at ingest | Rejected. It would drift from the route table immediately and would be wrong for any deployment that configures a route the list does not know about. The table is the only authority. | +| Build the `ConversionRouter` in the API process and ask it | Rejected. Building routes constructs the adapters, and a provider-backed adapter refuses composition without its API key. This would force provider credentials into a process that performs no conversion — a strictly worse secret posture for a membership test. | +| Sniff the bytes at admission and route on detected type | Rejected **as the mechanism for this check**, though valuable on its own. Sniffing is a decode-and-inspect step with its own cost and sandboxing requirements. It answers a different question ("is this really what you said it is?") and must not gate the cheap question ("is what you said routable at all?"). The two compose: the cheap check runs first and always. | +| **Compare the declared MIME against the deployment's configured route-name table (chosen)** | The table is already the authority; membership is a set lookup requiring no credentials, no decode, and no new configuration. | + +### 4.2 Why the two tables agree + +An obvious objection to checking the *settings* map rather than the *built* +router is that the two could drift, and then ingest and the worker would +disagree about what is routable — the worst possible outcome, because it would +reintroduce dead letters while claiming to have removed them. + +They cannot drift *within one configuration*. `build_conversion_routes` raises +`UnknownConverterError` when a route names an adapter it does not know, so +composition fails at startup rather than producing a router with fewer keys than +the configuration. A process running a given configuration therefore has a +router whose key set is exactly that configuration's key set, and checking +membership in the configuration is checking membership in the router. + +The precise scope matters. The guarantee is per-configuration, not global: the +gate and the convert worker are separately composed processes, so a deployment +that changes its route table has a window in which one has restarted and the +other has not. That window is the same race §6 leaves `UnroutableMimeError` in +the worker to cover, and it is bounded by a deliberate operator action rather +than being a property of ordinary traffic. The claim to make is "the gate is +never looser than the worker it was composed with", not "the two can never +differ". + +### 4.3 Matching is exact, deliberately + +`converter_for` does an exact dictionary lookup, so the admission check performs +the same exact lookup on the same string. Normalizing at the gate — lowercasing, +stripping `; charset=utf-8` — would be an improvement to *routing*, but applying +it only at the gate would create precisely the divergence §4.1 exists to +prevent: a type the gate accepts and the worker then rejects. + +If MIME normalization is wanted, it belongs in the router, where both callers +inherit it. Until then the gate is exactly as strict as the worker, which is the +property that matters. + +## 5. What the caller gets instead + +A refusal at admission, with the HTTP status that means this and no other thing: +**415 `unsupported_media_type`**, naming the types this deployment does convert. + +Three consequences follow, and all three are improvements: + +- no raw object is written, so nothing durable is created for input that can + never produce a representation; +- no reservation is committed, because the engine never reports acceptance; +- the caller learns at submission time, synchronously, instead of by polling a + readiness endpoint until it reports a terminal state. + +It also makes the deployment's capability *discoverable by trying* — the refusal +names the supported set, so a client that guesses wrong is told what it may send. + +## 5.1 One divergence the gate does not close + +The gate checks the MIME the caller declared. The convert stage does not read +that value: `_SELECT_CONVERT_SOURCE` joins `content_objects` and takes `c.mime`, +and that row is written `ON CONFLICT (deployment_id, content_hash) DO NOTHING` +— so for any given bytes, the **first MIME ever seen wins permanently**. + +Those two values are normally identical, and diverge only when the same bytes +are ingested a second time under a different MIME. If the first-seen MIME is +unrouted while the second is routed, the gate admits and the worker +dead-letters — the exact outcome D104 removes, reachable in that narrow case. +Getting there needs the first MIME to have become unrouted since it was stored +(a route-table change) or the row to predate this gate, because otherwise the +gate would have refused the first ingest too. + +**Closing it costs a query on every ingest.** The gate would have to look up +the existing content object by hash to learn the effective MIME, since nothing +in the request reveals it. That is one indexed read added to a hot path, to +cover a case that requires byte reuse across MIMEs plus a route change. The +trade is real in both directions and is left as an explicit open question +rather than silently paid or silently skipped: + +- if it is paid, the refusal must name the *effective* MIME, not the declared + one, or the caller is told their own request is unsupported when it is the + stored reading that cannot be routed; +- until it is, the worker's `UnroutableMimeError` still catches it, and the + outcome for those specific ingests is exactly the pre-D104 behaviour — not a + regression, an improvement that does not reach them. + +The honest statement of the guarantee is therefore: *no upload is admitted +under a MIME this deployment cannot convert*, not *no unconvertible version is +ever created*. + +## 6. What this deliberately does not do + +- It does not make any format supported. Registering an adapter is a separate + act; this only stops pretending that unregistered formats might work. +- It does not validate that the bytes match the declared type. A caller that + labels an MP3 as `text/plain` still gets past the gate and fails in the + converter — correctly, since that is a content error, not a routing one. +- It does not change what the worker does. `UnroutableMimeError` remains, and + remains non-retryable: it is still reachable when a deployment's route table + changes between admission and conversion, which is exactly the narrow race the + worker's handling exists for, and in the content-object divergence of §5.1. + +## 7. Sources + +- `src/rememberstack/core/conversion.py` — `ConversionRouter.converter_for`, + `STOCK_CONVERSION_ROUTE_NAMES`. +- `src/rememberstack/adapters/converters/__init__.py` — + `build_conversion_routes`, `UnknownConverterError` on unknown adapter names. +- `src/rememberstack/surfaces/http_api.py` — `_mount_ingest`, the admission + checks that exist today. +- `src/rememberstack/workers/e0.py` — `ConvertHandler.handle`, the + `UnroutableMimeError` → `mark_version_failed` → `NonRetryableHandlerError` + path. +- `src/rememberstack/spine/document_catalog.py` — `mark_version_failed`; no + counterpart re-enqueues a failed version. +- `decisions.md` D55 (identical bytes are a no-op), D61 (deployment policy vs + engine defaults), D38/D65 (the conversion router and its envelope). diff --git a/plan/designs/e0_files_design.md b/plan/designs/e0_files_design.md index d1dac35bc..e6e691383 100644 --- a/plan/designs/e0_files_design.md +++ b/plan/designs/e0_files_design.md @@ -174,6 +174,51 @@ gates everything downstream: - **Versioned** (`converter_version`): a converter or routing change re-converts the affected docs (a batch keyed by version), which rebuilds everything downstream — the D7 rebuildability discipline applied to the foundation. +- **Routability is admission, not discovery — D104.** The routing table is consulted in **E0 + itself**, before any byte is stored. A declared `mime` absent from the deployment's configured + routes raises `UnroutableMimeError` carrying the refused type and the accepted set; no raw + object is written, no version row is created, and no lineage is touched. Surfaces render it in + their own idiom — HTTP `POST /ingest` returns **415 `unsupported_media_type`** naming the types + this deployment converts. + + **The placement is the point.** Three ingresses reach E0 without sharing a handler: HTTP + `POST /ingest`, the local MCP `ingest` tool, and the connector sync worker — the latter two + calling the composed ingest port directly. A check on the HTTP handler would leave two of three + paths still admitting bytes the convert stage can only dead-letter, while looking fixed. The + gate therefore sits in the E0 ingestor's guard, beside the D74 admission check that is there for + the same reason: ingestion always writes through E0, so E0 is where an ingestion invariant + belongs. The alternative — admitting the upload and discovering the missing route one stage + later in the convert worker — writes a durable raw object for input that can never yield a + representation, and the caller cannot undo it: identical bytes are a no-op (D55) and no API lets + them ask for reprocessing. Recovery is operator work only — `remember ops replay` + (`WorkLedger.replay_dead_letter`) reopens the dead-lettered work row, after which a now-routable + version converts normally. So each wrong guess costs durable storage and an operator ticket to + discover what the route table, already in memory at admission, could have answered for free. + + The **route table is the only authority**. `conversion_routes` is deployment policy (D61); + `build_conversion_routes` refuses composition when a route names an unknown adapter, so a + process's router key set is exactly the key set of the configuration it was composed with, and + a membership test against that configuration is a membership test against that router. The + guarantee is per-configuration, not global — gate and convert worker are separately composed, + so a route-table change leaves a window in which one has restarted and the other has not. The gate performs the **same exact lookup** the router + performs, on the same string — MIME normalization, if ever wanted, belongs in the router where + both callers inherit it, never at the gate alone, because a gate more permissive than the + worker would recreate the dead letters this rule removes. The route table is a **required** argument to the E0 + ingestor: every deployment has one (the settings default is the stock text table), so omitting + it expresses nothing but a composition that forgot, and a default would let any consumer + silently opt out — an invariant with an opt-out is not one. + + The guarantee is scoped: *no upload is admitted under a MIME this deployment cannot convert*, + not *no unconvertible version is ever created*. The gate reads the declared MIME while convert + reads `content_objects.mime`, which is first-write-wins per content hash; the two diverge only + when the same bytes return under a different MIME whose first-seen reading has since become + unrouted. Closing that needs a content-object lookup on every ingest — an open question in the + analysis, not a silent cost. + + This is a *routing* verdict, not a *content* one. It does not check that the bytes match the + declared type; an MP3 labelled `text/plain` passes the gate and fails in the converter, which + is correct — that is a content error. `UnroutableMimeError` therefore remains in the worker and + remains non-retryable, covering the route-change window above. Output Markdown → artifacts bucket; the source map + manifest + converter metadata → `conversion.json`; the blockizer's `blocks.json` beside them; Postgres gets only the URIs + `converter_version` + diff --git a/src/rememberstack/model/conversion.py b/src/rememberstack/model/conversion.py index a62aa5a0a..aa6f56f9e 100644 --- a/src/rememberstack/model/conversion.py +++ b/src/rememberstack/model/conversion.py @@ -353,7 +353,26 @@ class ConversionError(Exception): class UnroutableMimeError(Exception): - """No configured conversion route accepts the input's MIME type (D38).""" + """No configured conversion route accepts the input's MIME type (D38). + + Raised at the E0 ingest gate (D104), where refusing costs nothing, and + still raised in the convert stage for the narrow case where a deployment's + route table changes between admission and conversion. `supported_mimes` + is None when the raiser did not know the table — the convert stage knows + only that its own lookup missed. + """ + + def __init__( + self, + message: str, + *, + mime: str | None = None, + supported_mimes: tuple[str, ...] | None = None, + ) -> None: + """Carry the refused type and, when known, what would be accepted.""" + super().__init__(message) + self.mime = mime + self.supported_mimes = supported_mimes class UnknownConverterError(Exception): diff --git a/src/rememberstack/profiles/selfhost.py b/src/rememberstack/profiles/selfhost.py index 01aee0c75..837ce6c69 100644 --- a/src/rememberstack/profiles/selfhost.py +++ b/src/rememberstack/profiles/selfhost.py @@ -807,6 +807,11 @@ def api(self) -> FastAPI: catalog=DocumentCatalog(engine=self._engine), raw_store=self._raw_store, admission=ForgetCatalog(engine=self._engine), + # D104: the same table that builds the router. + # build_conversion_routes refuses composition on an unknown + # adapter name, so a running deployment's router keys are + # exactly these keys. + routable_mimes=frozenset(self._settings.conversion_routes), ), pipeline_readiness=PipelineReadinessCatalog( engine=self._engine, diff --git a/src/rememberstack/surfaces/http_api.py b/src/rememberstack/surfaces/http_api.py index 1e07b4315..119f654a7 100644 --- a/src/rememberstack/surfaces/http_api.py +++ b/src/rememberstack/surfaces/http_api.py @@ -55,6 +55,7 @@ from rememberstack.model import SpendLeaseRefused from rememberstack.model import SpendLeaseUnavailable from rememberstack.model import ToolDescriptor +from rememberstack.model import UnroutableMimeError from rememberstack.ports.auth import AuthPerimeterPort from rememberstack.surfaces.graph_queries import GraphBusyError from rememberstack.surfaces.graph_queries import GraphHydrationError @@ -326,6 +327,11 @@ def build_api( are buffered (413 over the cap; 411 when no Content-Length is declared). None — the self-host default — imposes no limit: caps are deployment policy, never an engine default (D61). + + A `POST /ingest` whose MIME the deployment has no conversion route for is + refused with 415 (D104). That verdict is reached in the E0 gate, not here, + so every ingress inherits it; this surface only renders the resulting + `UnroutableMimeError` as HTTP. """ if surface is not None and surface.deployment_id != deployment_id: raise ValueError( @@ -991,20 +997,45 @@ def ingest_document( " source_kind/source_ref" ), ) - return ingest.ingest( - deployment_id=deployment_id, upload=upload, **attribution + try: + return ingest.ingest( + deployment_id=deployment_id, upload=upload, **attribution + ) + except UnroutableMimeError as error: + raise _unsupported_media_type(error) from error + try: + return ingest.ingest_observed( + deployment_id=deployment_id, + source_kind=source_kind, + source_ref=source_ref, + upload=upload, + versioning_mode=versioning_mode, + source_modified_at=source_modified_at, + source_version_ref=source_version_ref, + sync_cycle_id=None, + **attribution, ) - return ingest.ingest_observed( - deployment_id=deployment_id, - source_kind=source_kind, - source_ref=source_ref, - upload=upload, - versioning_mode=versioning_mode, - source_modified_at=source_modified_at, - source_version_ref=source_version_ref, - sync_cycle_id=None, - **attribution, - ) + except UnroutableMimeError as error: + raise _unsupported_media_type(error) from error + + +def _unsupported_media_type(error: UnroutableMimeError) -> HTTPException: + """Render the E0 gate's D104 refusal as HTTP 415. + + The verdict is the gate's, not this surface's: every ingress writes + through E0, so rendering here keeps HTTP from being the only path that + refuses. `supported_mimes` is present whenever the raiser knew the + deployment's table, which the gate always does. + """ + detail: dict[str, object] = { + "code": "unsupported_media_type", + "message": str(error), + } + if error.mime is not None: + detail["mime"] = error.mime + if error.supported_mimes is not None: + detail["supported_mimes"] = list(error.supported_mimes) + return HTTPException(status_code=415, detail=detail) def _mount_connectors( diff --git a/src/rememberstack/surfaces/mcp_memory_tools.py b/src/rememberstack/surfaces/mcp_memory_tools.py index 62e333594..a5a9a3404 100644 --- a/src/rememberstack/surfaces/mcp_memory_tools.py +++ b/src/rememberstack/surfaces/mcp_memory_tools.py @@ -49,6 +49,7 @@ from rememberstack.model.client import PipelineReadinessReport from rememberstack.model.client import ReadinessRequirements +from rememberstack.model.conversion import UnroutableMimeError from rememberstack.model.documents import IngestedVersion logger = logging.getLogger(__name__) @@ -484,6 +485,27 @@ def map_backend_error(error: BaseException) -> ToolError: return _map_http_style_error( status_code=status_code, detail=str(detail), explicit_code=explicit_code ) + if isinstance(error, UnroutableMimeError): + # D104 refuses at the E0 gate, so a LOCAL composition raises this typed + # error with no HTTP status to key off. Mapping it here keeps the local + # MCP answer identical to the remote one instead of degrading to + # internal_error, which would tell an agent to report a defect for a + # perfectly ordinary "we do not convert that". + supported = error.supported_mimes + return ToolError( + code="unsupported_media_type", + message=( + str(error) + if supported is None + else f"{error}; this deployment converts: {', '.join(supported)}" + ), + http_status=415, + retryable=False, + agent_action=( + "Convert the file to a supported type, or ingest its text; do" + " not retry the same bytes under the same MIME type." + ), + ) if isinstance(error, ValidationError): return ToolError( code="local_backend_error", @@ -1240,6 +1262,25 @@ def _map_http_style_error( if reason_code and code != "spend_safety" else (reason_code or _reason_from_spend_detail(detail=detail)), ) + if status_code == 415: + # Bound to the status, not the code alone: a 500 carrying this detail + # must not be relabelled as a typed routing refusal, which would undo + # the SDK's status-and-path trust rule one layer up. + return ToolError( + code="unsupported_media_type", + message=( + detail + if code == "unsupported_media_type" + else "This deployment has no conversion route for that MIME type." + ), + http_status=415, + retryable=False, + agent_action=( + "Convert the file to a supported type, or ingest its text; do" + " not retry the same bytes under the same MIME type." + ), + reason_code=reason_code, + ) if code == "body_too_large" or status_code == 413: return ToolError( code="body_too_large", diff --git a/src/rememberstack/surfaces/sdk.py b/src/rememberstack/surfaces/sdk.py index 581246576..fd99e8e63 100644 --- a/src/rememberstack/surfaces/sdk.py +++ b/src/rememberstack/surfaces/sdk.py @@ -741,7 +741,11 @@ def _json( elif path.startswith("/query/"): detail = "deployment API returned a malformed structured error" else: - detail = str(public_detail) + code, detail = _structured_refusal( + detail=public_detail, + status_code=response.status_code, + path=path, + ) else: detail = str(public_detail) elif isinstance(body, dict) and "detail" in body: @@ -807,6 +811,58 @@ def _structured_query_error( return code, message +_REFUSAL_CONTRACT: Final[dict[str, tuple[int, str]]] = { + "unsupported_media_type": (415, "/ingest") +} +"""Non-query structured refusals this client trusts: code -> (status, path). + +A code is honoured only at the status *and* on the endpoint the deployment API +binds it to, the same discipline `_structured_query_error` applies. Trusting a +code on status alone would let any endpoint returning 415 claim to be an ingest +routing refusal; trusting it unconditionally would let a server relabel any +failure. Widening this table is a contract change, not a convenience. +""" + + +def _structured_refusal( + *, detail: dict[object, object], status_code: int, path: str +) -> tuple[str | None, str]: + """Preserve a recognised `{"code", "message", ...}` refusal, else stringify. + + The deployment API renders structured refusals as a detail object with a + stable `code`, a human `message`, and sometimes extra actionable fields + (D104's `mime` and `supported_mimes`). Flattening that with `str()` yielded + a Python dict repr and dropped the code, so a consumer had to parse a dict + literal to learn what happened — and the MCP mapper could only report a + generic engine error where a local composition reports a typed one. + + Only codes in `_REFUSAL_CONTRACT`, at their bound status and on their bound + endpoint, are honoured. Everything else falls back to the previous + stringification, so an unrecognised envelope still surfaces rather than + being silently trusted. + """ + code = detail.get("code") + message = detail.get("message") + if not isinstance(code, str) or not isinstance(message, str) or not message: + return None, str(detail) + bound = _REFUSAL_CONTRACT.get(code) + if bound is None or bound != (status_code, path): + return None, str(detail) + extras = [ + f"{key}={_join_scalar(value)}" + for key, value in sorted(detail.items(), key=lambda item: str(item[0])) + if isinstance(key, str) and key not in {"code", "message"} + ] + return code, "; ".join([message, *extras]) if extras else message + + +def _join_scalar(value: object) -> str: + """Render one extra detail field as compact readable text.""" + if isinstance(value, (list, tuple)): + return ", ".join(str(item) for item in value) + return str(value) + + def _validated_list( model: type[_ModelT], payload: object, *, endpoint: str ) -> list[dict[str, object]]: diff --git a/src/rememberstack/workers/e0.py b/src/rememberstack/workers/e0.py index 30a6e7ba4..8d2cb98b1 100644 --- a/src/rememberstack/workers/e0.py +++ b/src/rememberstack/workers/e0.py @@ -12,6 +12,7 @@ still never fails structuring. """ +from collections.abc import Collection from collections.abc import Iterable from datetime import datetime import hashlib @@ -157,11 +158,25 @@ def __init__( catalog: DocumentCatalog, raw_store: ObjectStorePort, admission: IngestAdmission, + routable_mimes: Collection[str], ) -> None: - """Bind the connector to the catalog and the deployment's raw bucket.""" + """Bind the connector to the catalog and the deployment's raw bucket. + + ``routable_mimes`` is the deployment's conversion route table (D104), + and it is **required**: every deployment has a route table (the + settings default is the stock text table), so there is no real state + this could be omitted to express — only a composition that forgot. + A default would let any consumer silently opt out of the gate, and an + invariant with an opt-out is not an invariant. + + The gate lives here, at E0, rather than on any one surface: HTTP, the + local MCP tool and connector sync all write through this object, so + this is the only placement that cannot be bypassed. + """ self._catalog = catalog self._raw_store = raw_store self._admission = admission + self._routable = frozenset(routable_mimes) def ingest( self, @@ -183,6 +198,7 @@ def ingest( source_kind=UPLOAD_SOURCE_KIND, source_ref=content_hash, content_hash=content_hash, + mime=upload.mime, ) doc_id = uuid5( NAMESPACE_URL, f"rememberstack:upload:{deployment_id}:{content_hash}" @@ -244,6 +260,7 @@ def ingest_observed( source_kind=source_kind, source_ref=source_ref, content_hash=content_hash, + mime=upload.mime, ) doc_id = uuid5( NAMESPACE_URL, f"rememberstack:{source_kind}:{deployment_id}:{source_ref}" @@ -288,8 +305,31 @@ def _guard_ingest( source_kind: str, source_ref: str, content_hash: str, + mime: str, ) -> None: - """Check D74 before writing forgotten bytes back into the raw store.""" + """Refuse what this deployment cannot convert, and what D74 forgot. + + Routability (D104) is checked first. It is the cheaper question and the + more fundamental one — an input no configured route accepts is refused + whatever its forget state — so answering it first avoids an admission + query for a request that cannot be accepted, and avoids letting a + forget-state error mask a plain "we do not convert that". The lookup is + exactly the router's own, an exact match on the same string, so this + gate can never admit what the convert stage would dead-letter. + + D74's per-source check follows, still before any byte is written, so + forgotten content is never recreated. This ordering is internal to the + gate: a deployment-wide availability barrier (an in-progress forget + making the whole deployment 503) is composed above the surfaces and + still answers first, as it must — it says the deployment cannot serve + the request at all, not that these bytes are unwelcome. + """ + if mime not in self._routable: + raise UnroutableMimeError( + f"no conversion route accepts mime {mime!r}", + mime=mime, + supported_mimes=tuple(sorted(self._routable)), + ) self._admission.guard_ingest( deployment_id=deployment_id, source_kind=source_kind, diff --git a/src/tests/surfaces/test_client_sdk.py b/src/tests/surfaces/test_client_sdk.py index 6a3efaa6e..1cffde129 100644 --- a/src/tests/surfaces/test_client_sdk.py +++ b/src/tests/surfaces/test_client_sdk.py @@ -968,3 +968,41 @@ def test_body_cap_holds_when_the_app_is_mounted_under_a_prefix() -> None: content=b"small note", ) assert accepted.status_code == 200 + + +def test_structured_415_refusal_keeps_its_code_and_supported_types() -> None: + """D104: the ingest refusal survives the SDK instead of becoming a dict repr. + + Only codes bound to their status are trusted, so this proves both halves: + the refusal at 415 is honoured, and the same body at another status is not. + """ + refusal = { + "code": "unsupported_media_type", + "message": "no conversion route accepts mime 'audio/mpeg'", + "mime": "audio/mpeg", + "supported_mimes": ["text/markdown", "text/plain"], + } + honoured = httpx.Client( + base_url="http://memory.test", + transport=httpx.MockTransport( + lambda _request: httpx.Response(415, json={"detail": refusal}) + ), + ) + with pytest.raises(MemoryApiError) as refused: + MemoryClient(client=honoured).ingest( + b"x", filename="meeting.mp3", mime="audio/mpeg" + ) + assert refused.value.code == "unsupported_media_type" + assert "text/markdown" in refused.value.detail + + mismatched = httpx.Client( + base_url="http://memory.test", + transport=httpx.MockTransport( + lambda _request: httpx.Response(500, json={"detail": refusal}) + ), + ) + with pytest.raises(MemoryApiError) as untrusted: + MemoryClient(client=mismatched).ingest( + b"x", filename="meeting.mp3", mime="audio/mpeg" + ) + assert untrusted.value.code is None diff --git a/src/tests/surfaces/test_ingest_routability_http.py b/src/tests/surfaces/test_ingest_routability_http.py new file mode 100644 index 000000000..f18731e68 --- /dev/null +++ b/src/tests/surfaces/test_ingest_routability_http.py @@ -0,0 +1,135 @@ +"""D104 at the HTTP surface: the E0 gate's refusal is rendered as 415. + +The routing verdict itself belongs to `UploadIngestor` and is proved in +`tests/workers/test_ingest_routability.py`. This module asserts only what the +HTTP surface adds: that an `UnroutableMimeError` raised by the composed ingest +port becomes an HTTP 415 whose body tells the caller what may be sent instead. +""" + +from datetime import datetime +from uuid import UUID +from uuid import uuid4 + +from fastapi.testclient import TestClient +from httpx import Response +import pytest + +from rememberstack.model import DocumentUpload +from rememberstack.model import IngestedVersion +from rememberstack.model import IngestPrincipal +from rememberstack.model import UnroutableMimeError +from rememberstack.surfaces.http_api import build_api + +_DEPLOYMENT_ID = UUID("103b0000-0000-0000-0000-000000000001") +_SUPPORTED = ("text/markdown", "text/plain") + + +class _RefusingIngest: + """An `IngestPort` standing in for a gate that refuses this MIME.""" + + def _refuse(self, upload: DocumentUpload) -> IngestedVersion: + """Raise exactly what the E0 gate raises for an unrouted type.""" + if upload.mime in _SUPPORTED: + return IngestedVersion( + deployment_id=_DEPLOYMENT_ID, + doc_id=uuid4(), + version_id=uuid4(), + content_hash="0" * 64, + created=True, + ) + raise UnroutableMimeError( + f"no conversion route accepts mime {upload.mime!r}", + mime=upload.mime, + supported_mimes=_SUPPORTED, + ) + + def ingest( + self, + *, + deployment_id: UUID, + upload: DocumentUpload, + ingested_by: IngestPrincipal | None = None, + ) -> IngestedVersion: + """Refuse or accept the one-shot upload path.""" + _ = deployment_id, ingested_by + return self._refuse(upload) + + def ingest_observed( + self, + *, + deployment_id: UUID, + source_kind: str, + source_ref: str, + upload: DocumentUpload, + versioning_mode: str, + source_modified_at: datetime | None, + source_version_ref: str | None, + sync_cycle_id: UUID | None, + ingested_by: IngestPrincipal | None = None, + ) -> IngestedVersion: + """Refuse or accept the lineage path.""" + _ = ( + deployment_id, + source_kind, + source_ref, + versioning_mode, + source_modified_at, + source_version_ref, + sync_cycle_id, + ingested_by, + ) + return self._refuse(upload) + + +class _OpenBoundary: + """Admission/readiness that never refuses, so proofs stay about ingest.""" + + def assert_available(self, *, deployment_id: UUID) -> None: + """Never close the D74 admission barrier during these proofs.""" + _ = deployment_id + + def ensure_ready(self, *, deployment_id: UUID) -> tuple[UUID, ...]: + """Report no outstanding forget manifests to replay.""" + _ = deployment_id + return () + + +def _client() -> TestClient: + """Build the API over the refusing ingest port.""" + return TestClient( + build_api( + engine=None, # type: ignore[arg-type] + deployment_id=_DEPLOYMENT_ID, + admission=_OpenBoundary(), + readiness=_OpenBoundary(), + ingest=_RefusingIngest(), + ) + ) + + +def _post(mime: str, *, lineage: bool = False) -> Response: + """POST one byte payload declaring `mime`, optionally as a lineage push.""" + params: dict[str, str] = {"filename": "input.bin", "mime": mime} + if lineage: + params |= {"source_kind": "drive", "source_ref": "file-1"} + return _client().post("/ingest", params=params, content=b"hello") + + +def test_a_routed_mime_still_succeeds() -> None: + """The control: the surface does not refuse what the gate accepted.""" + assert _post("text/plain").status_code == 200 + + +@pytest.mark.parametrize("lineage", (False, True)) +def test_the_gates_refusal_becomes_415(lineage: bool) -> None: + """Both HTTP ingest shapes render the refusal, not just the one-shot path.""" + assert _post("audio/mpeg", lineage=lineage).status_code == 415 + + +def test_the_415_body_names_what_the_deployment_converts() -> None: + """A caller that guessed wrong is told what it may send instead.""" + detail = _post("application/zip").json()["detail"] + # `code` (not `error`) is this surface's key for a structured refusal. + assert detail["code"] == "unsupported_media_type" + assert detail["mime"] == "application/zip" + assert detail["supported_mimes"] == ["text/markdown", "text/plain"] diff --git a/src/tests/surfaces/test_mcp_memory_tools.py b/src/tests/surfaces/test_mcp_memory_tools.py index 91414fe01..b6c84e176 100644 --- a/src/tests/surfaces/test_mcp_memory_tools.py +++ b/src/tests/surfaces/test_mcp_memory_tools.py @@ -21,6 +21,7 @@ from rememberstack.model.client import PipelineReadinessReport from rememberstack.model.client import ReadinessRequirements from rememberstack.model.client import VersionPipelineReadiness +from rememberstack.model.conversion import UnroutableMimeError from rememberstack.model.documents import DocumentUpload from rememberstack.model.documents import IngestedVersion from rememberstack.surfaces.mcp import OperationMcpServer @@ -982,3 +983,40 @@ def test_remote_and_local_descriptors_match() -> None: ] assert remote_names == local_names == ["ingest", "pipeline_readiness"] _ = uuid4() + + +def test_local_unroutable_mime_maps_to_unsupported_media_type() -> None: + """D104: a LOCAL composition raises the typed error with no HTTP status. + + Without this branch the agent was told `internal_error` — "report a + composition defect" — for the perfectly ordinary case of sending a file + type the deployment does not convert. + """ + error = map_backend_error( + UnroutableMimeError( + "no conversion route accepts mime 'audio/mpeg'", + mime="audio/mpeg", + supported_mimes=("text/markdown", "text/plain"), + ) + ) + assert error.code == "unsupported_media_type" + assert error.http_status == 415 + assert error.retryable is False + assert "text/markdown" in error.message + assert error.agent_action + + +def test_remote_unsupported_media_type_keeps_its_structured_code() -> None: + """The remote path must not degrade the refusal to engine_client_error.""" + error = map_backend_error( + MemoryApiError( + status_code=415, + detail="no conversion route accepts mime 'audio/mpeg'; " + "supported_mimes=text/markdown, text/plain", + code="unsupported_media_type", + ) + ) + assert error.code == "unsupported_media_type" + assert error.http_status == 415 + assert error.retryable is False + assert "text/markdown" in error.message diff --git a/src/tests/surfaces/test_retrieval_api.py b/src/tests/surfaces/test_retrieval_api.py index bfa3846cb..f7cd93a46 100644 --- a/src/tests/surfaces/test_retrieval_api.py +++ b/src/tests/surfaces/test_retrieval_api.py @@ -232,6 +232,7 @@ def __init__(self, *, engine: Engine, root: Path) -> None: catalog=document_catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) generation = chunker_version(params=_PARAMS) registry = HandlerRegistry() diff --git a/src/tests/workers/test_e0_chain.py b/src/tests/workers/test_e0_chain.py index 40c0a6f28..5e509efd4 100644 --- a/src/tests/workers/test_e0_chain.py +++ b/src/tests/workers/test_e0_chain.py @@ -27,6 +27,7 @@ from rememberstack.adapters.testing import NoopCostMeter from rememberstack.core import blockize from rememberstack.core import ConversionRouter +from rememberstack.core import Converter from rememberstack.core import MarkdownPassthroughConverter from rememberstack.model import ClaimedWork from rememberstack.model import ConversionCoverage @@ -269,22 +270,25 @@ def __init__(self, *, engine: Engine, root: Path) -> None: retry_backoff_base_s=0.0, retry_backoff_max_s=0.0 ), ) + # One table feeds both the D104 admission gate and the router, the + # way a real deployment's `conversion_routes` does. Duplicating it + # would let the harness prove a divergence production cannot have. + routes: dict[str, Converter] = { + "text/markdown": MarkdownPassthroughConverter(), + "text/plain": MarkdownPassthroughConverter(), + "text/html": MarkitdownConverter(), + "application/x-fake-scan": _FakeScanConverter(), + "application/x-unlabeled": _UnlabeledConverter(), + "application/x-invalid-envelope": _InvalidEnvelopeConverter(), + "application/x-transient": _TransientlyFailingConverter(), + } self.ingestor = UploadIngestor( catalog=self.catalog, raw_store=self.raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset(routes), ) - router = ConversionRouter( - routes={ - "text/markdown": MarkdownPassthroughConverter(), - "text/plain": MarkdownPassthroughConverter(), - "text/html": MarkitdownConverter(), - "application/x-fake-scan": _FakeScanConverter(), - "application/x-unlabeled": _UnlabeledConverter(), - "application/x-invalid-envelope": _InvalidEnvelopeConverter(), - "application/x-transient": _TransientlyFailingConverter(), - } - ) + router = ConversionRouter(routes=routes) registry = HandlerRegistry() registry.register( stage=PipelineStage.CONVERT, @@ -598,8 +602,23 @@ def test_exhausted_provider_retries_finalize_the_version(rig: _E0Rig) -> None: def test_unroutable_mime_dead_letters_without_retries(rig: _E0Rig) -> None: - """No route for the MIME type is deterministic — one attempt, dead-lettered.""" - ingested = rig.ingestor.ingest( + """No route for the MIME type is deterministic — one attempt, dead-lettered. + + D104 normally makes this unreachable: the gate refuses an unrouted MIME at + admission, so the worker never sees one. The worker's handling survives for + the window where the two disagree — gate and convert worker are separately + composed processes, so a route-table change leaves one restarted and the + other not. This constructs exactly that window by admitting through a gate + whose table is wider than the router the worker runs, and proves the + convert stage still fails closed rather than retrying forever. + """ + admitting_gate = UploadIngestor( + catalog=rig.catalog, + raw_store=rig.raw_store, + admission=ForgetCatalog(engine=rig.engine), + routable_mimes=frozenset({"application/x-unknown"}), + ) + ingested = admitting_gate.ingest( deployment_id=_DEPLOYMENT_ID, upload=DocumentUpload( filename="blob.bin", mime="application/x-unknown", content=b"\x00\x01\x02" diff --git a/src/tests/workers/test_e1_chain.py b/src/tests/workers/test_e1_chain.py index 2e43229df..fb3e405b5 100644 --- a/src/tests/workers/test_e1_chain.py +++ b/src/tests/workers/test_e1_chain.py @@ -122,6 +122,7 @@ def __init__(self, *, engine: Engine, root: Path) -> None: catalog=document_catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) registry = HandlerRegistry() registry.register( diff --git a/src/tests/workers/test_e2_chain.py b/src/tests/workers/test_e2_chain.py index b5c234496..9ad0f7488 100644 --- a/src/tests/workers/test_e2_chain.py +++ b/src/tests/workers/test_e2_chain.py @@ -203,6 +203,7 @@ def __init__(self, *, engine: Engine, root: Path) -> None: catalog=document_catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) registry = HandlerRegistry() registry.register( diff --git a/src/tests/workers/test_e3_chain.py b/src/tests/workers/test_e3_chain.py index b0c335f49..6ecf84925 100644 --- a/src/tests/workers/test_e3_chain.py +++ b/src/tests/workers/test_e3_chain.py @@ -220,6 +220,7 @@ def route(prompt: str, type_name: str) -> dict[str, object]: catalog=document_catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) self.p1 = PostgresP1Index( engine=engine, embedding_model=P1Settings().embedding_model diff --git a/src/tests/workers/test_ingest_admission.py b/src/tests/workers/test_ingest_admission.py index ac00ae989..79b7c826d 100644 --- a/src/tests/workers/test_ingest_admission.py +++ b/src/tests/workers/test_ingest_admission.py @@ -70,6 +70,7 @@ def test_guard_runs_before_upload_and_observed_raw_writes(observed: bool) -> Non catalog=cast(DocumentCatalog, object()), raw_store=store, admission=DenyingAdmission(), + routable_mimes=frozenset({"text/markdown"}), ) upload = DocumentUpload( filename="forgotten.md", mime="text/markdown", content=b"forgotten" @@ -109,6 +110,7 @@ def test_observed_ingest_rejects_non_utc_time_before_raw_write( catalog=cast(DocumentCatalog, object()), raw_store=store, admission=AllowingAdmission(), + routable_mimes=frozenset({"text/markdown"}), ) with pytest.raises(ValidationError, match="timezone-aware UTC"): diff --git a/src/tests/workers/test_ingest_gate_is_the_only_door.py b/src/tests/workers/test_ingest_gate_is_the_only_door.py new file mode 100644 index 000000000..c950fcb91 --- /dev/null +++ b/src/tests/workers/test_ingest_gate_is_the_only_door.py @@ -0,0 +1,109 @@ +"""D104 holds only while E0 is the single door into a document version. + +The routability gate lives in `UploadIngestor._guard_ingest` precisely because +every ingress writes through that object. Three things could quietly undo that, +and none would fail any behavioural test: + +- a new caller reaching `DocumentCatalog.record_upload` directly, creating a + version without passing the gate; +- a new module writing `document_versions` itself, going around the catalog; +- `routable_mimes` regaining a default, letting a composition opt out by + omission. + +These are structural audits, in the spirit of the repo's other inventory +proofs. They fail loudly when the shape changes, which is the point — a +behavioural test cannot notice a door that did not exist when it was written. +""" + +import ast +from collections import Counter +import inspect +from pathlib import Path +import re + +from rememberstack.workers.e0 import UploadIngestor + +_PACKAGE = Path(inspect.getfile(UploadIngestor)).parents[1] +_CATALOG = _PACKAGE / "spine" / "document_catalog.py" +_GATED_METHODS = {"ingest", "ingest_observed"} + + +def _enclosing_scope(tree: ast.Module, target: ast.AST) -> tuple[str, ...]: + """Return the nested def/class names containing `target`, outermost first.""" + for node in ast.walk(tree): + if not isinstance(node, ast.ClassDef | ast.FunctionDef | ast.AsyncFunctionDef): + continue + for child in ast.walk(node): + if child is target: + inner = _enclosing_scope( + ast.Module(body=list(node.body), type_ignores=[]), target + ) + return (node.name, *inner) + return () + + +def _record_upload_callers() -> Counter[tuple[str, tuple[str, ...]]]: + """Count every `.record_upload(` call by (module stem, enclosing scope). + + A Counter, not a set: collapsing duplicates would hide a *second* call + added inside an already-allowed method — including one placed before the + guard runs, which is exactly the regression this audit exists to catch. + """ + callers: Counter[tuple[str, tuple[str, ...]]] = Counter() + for path in _PACKAGE.rglob("*.py"): + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if ( + isinstance(node, ast.Call) + and isinstance(node.func, ast.Attribute) + and node.func.attr == "record_upload" + ): + callers[(path.stem, _enclosing_scope(tree, node))] += 1 + return callers + + +def test_only_the_gated_e0_methods_create_document_versions() -> None: + """`record_upload` is called only from the two guarded `UploadIngestor` methods. + + Checking the enclosing scope, not just the file, is the point: a second + call inside `e0.py` but outside `ingest`/`ingest_observed` would skip + `_guard_ingest` and restore the accepted-then-dead-letter path D104 + removed. If this fails, route the new caller through the guard — do not + widen this assertion. + """ + assert _record_upload_callers() == Counter( + {("e0", ("UploadIngestor", method)): 1 for method in _GATED_METHODS} + ) + + +def test_only_the_catalog_writes_document_versions() -> None: + """No runtime module goes around `DocumentCatalog` to insert a version row. + + `record_upload` being the single gated entry point means nothing if another + module can write `document_versions` itself. Migrations are excluded: they + define the table rather than ingest through it, and they run under an + operator, not a caller. + + This is a text scan, normalised for case and whitespace, so it cannot see + SQL assembled at runtime. It is deliberately biased toward false positives + — a comment mentioning the statement fails this test, and someone looks — + because the failure mode worth avoiding is the silent one. + """ + pattern = re.compile(r"insert\s+into\s+document_versions", re.IGNORECASE) + writers = { + path.relative_to(_PACKAGE) + for path in _PACKAGE.rglob("*.py") + if "migrations" not in path.parts and pattern.search(path.read_text("utf-8")) + } + assert writers == {_CATALOG.relative_to(_PACKAGE)} + + +def test_the_route_table_cannot_be_omitted_by_a_composition() -> None: + """`routable_mimes` has no default, so no composition can skip the gate. + + An earlier draft defaulted it to None. That made D104 as strong as every + composer remembering to pass it, which is not what an invariant means. + """ + parameter = inspect.signature(UploadIngestor.__init__).parameters["routable_mimes"] + assert parameter.default is inspect.Parameter.empty + assert parameter.kind is inspect.Parameter.KEYWORD_ONLY diff --git a/src/tests/workers/test_ingest_routability.py b/src/tests/workers/test_ingest_routability.py new file mode 100644 index 000000000..9fd9382cd --- /dev/null +++ b/src/tests/workers/test_ingest_routability.py @@ -0,0 +1,194 @@ +"""D104: E0 refuses a MIME the deployment has no conversion route for. + +The gate lives in `UploadIngestor`, not on any one surface, because every +ingress writes through it — HTTP `POST /ingest`, the local MCP `ingest` tool, +and the connector sync worker all call the same object. A check placed on the +HTTP handler would leave the other two admitting bytes the convert stage can +only dead-letter. + +The decisive assertion in these proofs is `store.writes == 0`: a refusal that +still wrote the raw object would have left exactly the durable garbage D104 +exists to prevent. +""" + +from typing import cast +from uuid import UUID + +import pytest + +from rememberstack.adapters.converters import build_conversion_routes +from rememberstack.model import DocumentUpload +from rememberstack.model import ObjectKey +from rememberstack.model import UnroutableMimeError +from rememberstack.spine.document_catalog import DocumentCatalog +from rememberstack.workers.e0 import UploadIngestor + +_DEPLOYMENT_ID = UUID("103a0000-0000-0000-0000-000000000001") +_ROUTES = {"text/markdown": "passthrough", "text/plain": "passthrough"} + + +class AllowingAdmission: + """Accept every input so these proofs stay about routing, not D74.""" + + def guard_ingest( + self, + *, + deployment_id: UUID, + source_kind: str, + source_ref: str, + content_hash: str, + ) -> None: + """Permit the attempted observation.""" + + +class RecordingStore: + """Object-store fake proving refusal happens before the first write.""" + + def __init__(self) -> None: + self.writes = 0 + + def read_bytes(self, *, key: ObjectKey) -> bytes: + raise AssertionError(f"unexpected read of {key.root}") + + def write_bytes( + self, *, key: ObjectKey, content: bytes, storage_class: str | None = None + ) -> None: + self.writes += 1 + + +def _ingestor(store: RecordingStore, *, routable: frozenset[str]) -> UploadIngestor: + """Build the E0 gate over a recording store and an open admission.""" + return UploadIngestor( + catalog=cast(DocumentCatalog, object()), + raw_store=store, + admission=AllowingAdmission(), + routable_mimes=routable, + ) + + +def _run(ingestor: UploadIngestor, *, mime: str, observed: bool) -> None: + """Drive whichever E0 entry point the case is proving.""" + upload = DocumentUpload(filename="input.bin", mime=mime, content=b"hello") + if observed: + ingestor.ingest_observed( + deployment_id=_DEPLOYMENT_ID, + source_kind="drive", + source_ref="file-1", + upload=upload, + versioning_mode="living", + source_modified_at=None, + source_version_ref=None, + sync_cycle_id=None, + ) + else: + ingestor.ingest(deployment_id=_DEPLOYMENT_ID, upload=upload) + + +@pytest.mark.parametrize("observed", (False, True)) +def test_unrouted_mime_is_refused_before_any_raw_write(observed: bool) -> None: + """Both E0 entry points refuse, so no surface can bypass the gate. + + `ingest` is the one-shot upload path the HTTP surface and the MCP tool + use; `ingest_observed` is the lineage path the connector sync worker uses. + Proving both is what makes the gate universal rather than HTTP-only. + """ + store = RecordingStore() + with pytest.raises(UnroutableMimeError): + _run( + _ingestor(store, routable=frozenset(_ROUTES)), + mime="audio/mpeg", + observed=observed, + ) + assert store.writes == 0 + + +def test_the_refusal_carries_what_the_deployment_does_convert() -> None: + """The error is renderable: a surface can tell the caller what to send.""" + with pytest.raises(UnroutableMimeError) as caught: + _run( + _ingestor(RecordingStore(), routable=frozenset(_ROUTES)), + mime="application/zip", + observed=False, + ) + assert caught.value.mime == "application/zip" + assert caught.value.supported_mimes == ("text/markdown", "text/plain") + + +def test_a_composition_cannot_omit_the_route_table() -> None: + """There is no opt-out: `routable_mimes` is a required argument. + + An earlier draft defaulted it to None, meaning "no check". That made the + gate as universal as every composer remembering to pass it — and an + invariant with a silent opt-out is not an invariant. Every deployment has + a route table (the settings default is the stock text table), so omission + expresses nothing except a mistake, and this proves it is refused loudly. + """ + with pytest.raises(TypeError, match="routable_mimes"): + UploadIngestor( # type: ignore[call-arg] + catalog=cast(DocumentCatalog, object()), + raw_store=RecordingStore(), + admission=AllowingAdmission(), + ) + + +def test_matching_is_exact_so_the_gate_is_never_looser_than_the_worker() -> None: + """A parameterised MIME is refused, because the router would refuse it too. + + `ConversionRouter.converter_for` is an exact dict lookup. If the gate + normalised `text/plain; charset=utf-8` down to `text/plain` and the worker + did not, the upload would be admitted and then dead-lettered — the exact + outcome D104 exists to prevent. Normalisation belongs in the router, where + both callers inherit it. + """ + store = RecordingStore() + with pytest.raises(UnroutableMimeError): + _run( + _ingestor(store, routable=frozenset(_ROUTES)), + mime="text/plain; charset=utf-8", + observed=False, + ) + assert store.writes == 0 + + +def test_routability_is_decided_before_the_d74_admission_query() -> None: + """An unroutable input is refused whatever its forget state. + + Ordering is a decision, not an accident: routability is the cheaper and + more fundamental question, so answering it first avoids an admission query + for a request that cannot be accepted, and stops a forget-state error from + masking a plain "we do not convert that". This admission fake fails the + test if it is ever consulted for an unroutable type. + """ + + class ExplodingAdmission: + def guard_ingest( + self, + *, + deployment_id: UUID, + source_kind: str, + source_ref: str, + content_hash: str, + ) -> None: + raise AssertionError("D74 consulted for an unroutable input") + + ingestor = UploadIngestor( + catalog=cast(DocumentCatalog, object()), + raw_store=RecordingStore(), + admission=ExplodingAdmission(), + routable_mimes=frozenset(_ROUTES), + ) + with pytest.raises(UnroutableMimeError): + _run(ingestor, mime="audio/mpeg", observed=False) + + +def test_the_gate_and_the_router_read_the_same_key_set() -> None: + """The equivalence D104 rests on: configured keys are the router's keys. + + The gate tests membership in the configured route-name table while the + worker tests membership in the built router. That is only safe because + `build_conversion_routes` materialises exactly the configured keys — it + refuses composition on an unknown adapter rather than silently dropping a + route. If this stopped holding, the gate could admit what the worker + dead-letters. + """ + assert frozenset(build_conversion_routes(route_names=_ROUTES)) == frozenset(_ROUTES) diff --git a/src/tests/workers/test_lifecycle_reconciliation.py b/src/tests/workers/test_lifecycle_reconciliation.py index 5a2595694..8c7b1a450 100644 --- a/src/tests/workers/test_lifecycle_reconciliation.py +++ b/src/tests/workers/test_lifecycle_reconciliation.py @@ -247,6 +247,7 @@ def __init__(self, *, engine: Engine, root: Path) -> None: catalog=document_catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) self.p1 = PostgresP1Index( engine=engine, embedding_model=P1Settings().embedding_model diff --git a/src/tests/workers/test_reuse_lifecycle.py b/src/tests/workers/test_reuse_lifecycle.py index 5453bb225..68454ae10 100644 --- a/src/tests/workers/test_reuse_lifecycle.py +++ b/src/tests/workers/test_reuse_lifecycle.py @@ -175,7 +175,10 @@ def __init__(self, *, engine: Engine, root: Path) -> None: self.document_catalog = catalog self.artifact_store = artifact_store self.ingestor = UploadIngestor( - catalog=catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine) + catalog=catalog, + raw_store=raw_store, + admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ) registry = HandlerRegistry() registry.register( diff --git a/src/tests/workers/test_watch_loop.py b/src/tests/workers/test_watch_loop.py index 89417417a..e33cb3ef6 100644 --- a/src/tests/workers/test_watch_loop.py +++ b/src/tests/workers/test_watch_loop.py @@ -104,6 +104,7 @@ def __init__(self, *, engine: Engine, root: Path) -> None: catalog=catalog, raw_store=raw_store, admission=ForgetCatalog(engine=engine), + routable_mimes=frozenset({"text/markdown"}), ), settings=SyncSettings(debounce_quiet_seconds=0.0), ) diff --git a/website/src/app/docs/configuration/page.mdx b/website/src/app/docs/configuration/page.mdx index 1e205c084..27740396b 100644 --- a/website/src/app/docs/configuration/page.mdx +++ b/website/src/app/docs/configuration/page.mdx @@ -73,7 +73,7 @@ Replace every local-only secret before any non-isolated use. | Variable | Purpose | | --- | --- | -| `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` | JSON object mapping input MIME type to a converter adapter name, e.g. `{"text/markdown": "passthrough", "text/plain": "passthrough", "text/html": "markitdown", "application/pdf": "mistral_ocr"}`. Shipped adapter names: `passthrough`, `markitdown`, `mistral_ocr`. Setting the variable replaces the whole table (defaults are the stock text table); a MIME type without a route dead-letters on convert, and an unknown adapter name refuses startup | +| `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` | JSON object mapping input MIME type to a converter adapter name, e.g. `{"text/markdown": "passthrough", "text/plain": "passthrough", "text/html": "markitdown", "application/pdf": "mistral_ocr"}`. Shipped adapter names: `passthrough`, `markitdown`, `mistral_ocr`. Setting the variable replaces the whole table (defaults are the stock text table); a MIME type without a route is refused at ingest on every path — HTTP, MCP, and connector sync — naming the accepted set (HTTP renders it as 415 `unsupported_media_type`), and an unknown adapter name refuses startup | The `mistral_ocr` route is BYO-key (provider-backed; off unless routed): @@ -188,7 +188,7 @@ What is **not** sent: request bodies, local vars, breadcrumbs, PII, prompts/comp | Piece | Config posture | | --- | --- | | Markdown smoke | Default Compose conversion route | -| HTML/PDF/media | Bind converter routes per MIME type via `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` (the smoke profile does not auto-route beyond text) | +| HTML/PDF/media | Bind converter routes per MIME type via `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` (stock routes are `text/markdown` and `text/plain`; anything else is refused at ingest until you register a converter) | | Watched directory | Connector extra + connector management API/CLI; credentials stay deployment-side | | Lineage identity | `source_kind` + `source_ref` together on ingest; `versioning_mode=snapshot|living` | diff --git a/website/src/app/docs/ingestion/pipeline/page.mdx b/website/src/app/docs/ingestion/pipeline/page.mdx index fedb21274..e505de96e 100644 --- a/website/src/app/docs/ingestion/pipeline/page.mdx +++ b/website/src/app/docs/ingestion/pipeline/page.mdx @@ -46,7 +46,7 @@ All stages are **idempotent** with respect to their versioned inputs. Workers us **Job:** produce an immutable **representation**: clean Markdown + source map (+ media sidecars when composed). -**Shipped smoke profile:** registers `text/markdown` → Markdown passthrough. Unregistered MIME types fail as non-retryable unroutable input. +**Shipped smoke profile:** registers `text/markdown` and `text/plain` → Markdown passthrough. Unregistered MIME types never reach this stage: ingest refuses them, before raw storage and before a version exists (see [Troubleshooting](/docs/troubleshooting)). **Designed / composable routes** (register on the deployment; do not assume the Compose smoke stack has them): diff --git a/website/src/app/docs/project-status/page.mdx b/website/src/app/docs/project-status/page.mdx index 7c415b991..0b88737ea 100644 --- a/website/src/app/docs/project-status/page.mdx +++ b/website/src/app/docs/project-status/page.mdx @@ -108,6 +108,31 @@ Current public release: [`v0.11.0`](https://github.com/writeitai/remember-stack/ - Graph planner settings are deployment-local and transaction-contained, with migrations and readiness checks for the PostgreSQL 19 execution plans. +## Unreleased on `main` + +- **Ingest refuses a MIME type this deployment cannot convert.** A type absent + from the conversion route table is rejected before raw storage and before a + version exists: HTTP `POST /ingest` returns `415 unsupported_media_type` + naming the accepted set, the MCP `ingest` tool returns the same code, and + connector sync counts the item as failed for that cycle. Previously such an + upload was accepted, stored, and dead-lettered one stage later in convert — + and a caller could not undo it, because identical bytes are the D55 no-op + and only an operator can reopen the work row with `remember ops replay`. +- The check sits in E0, not on a surface. All three ingresses write through + the same ingestor, so a surface-level check would have left two of them + bypassing it. +- **Breaking for library consumers**: `rememberstack.workers.UploadIngestor` + now takes a required `routable_mimes` argument — the deployment's conversion + route table, the same mapping you pass to `ConversionRouter`. Pass + `frozenset(conversion_routes)` where you build it. It is required rather + than defaulted because a default would let a composition silently opt out of + the gate, and every deployment has a route table (the settings default is + the stock text table). Deployments configured through + `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` need no change. +- Versions dead-lettered by an unroutable MIME before this change remain + `failed`; adding the route does not convert them retroactively. Reopen them + with `remember ops replay` once the route exists. + ## What landed after v0.9.0 (in v0.10.0) - **A shared name no longer costs a candidate anything.** The diff --git a/website/src/app/docs/reference/api/page.mdx b/website/src/app/docs/reference/api/page.mdx index 9dce0e629..26d090623 100644 --- a/website/src/app/docs/reference/api/page.mdx +++ b/website/src/app/docs/reference/api/page.mdx @@ -287,6 +287,31 @@ connector cursor, but the existing version's `source_modified_at` never changes or clears because it already fed extraction. A supplied source timestamp must be timezone-aware UTC; omitting it preserves an unknown source time. +The declared `mime` must be one this deployment has a conversion route for. +An unrouted type is refused with **415** and a body naming what is accepted: + +```json +{ + "detail": { + "code": "unsupported_media_type", + "message": "no conversion route accepts mime 'audio/mpeg'", + "mime": "audio/mpeg", + "supported_mimes": ["text/markdown", "text/plain"] + } +} +``` + +The refusal happens before anything is stored, so no document version is +created and there is nothing to clean up. Matching is exact, so a type carrying +parameters (`text/plain; charset=utf-8`) is not the same key as `text/plain`. +The accepted set is the deployment's own `REMEMBERSTACK_SELFHOST_CONVERSION_ROUTES` +table — see [Configuration](/docs/configuration) — so adding a converter widens +it automatically. + +The check lives in the ingest layer, not this endpoint, so the MCP `ingest` +tool and connector sync refuse the same types; they surface it as an error +rather than an HTTP status. + ### `POST /readiness` When composed, accepts document-version UUIDs plus an exhaustive `require` diff --git a/website/src/app/docs/reference/mcp/page.mdx b/website/src/app/docs/reference/mcp/page.mdx index 6117420a8..2b5449b26 100644 --- a/website/src/app/docs/reference/mcp/page.mdx +++ b/website/src/app/docs/reference/mcp/page.mdx @@ -124,6 +124,11 @@ the tool does **not** block until ready. When `created=false` the content-hash no-op applied — call `pipeline_readiness` once rather than assuming work is in flight. +A file whose MIME type the deployment has no conversion route for is refused +before anything is stored; the tool returns `unsupported_media_type` naming the +types that deployment does convert. Convert the file, or ingest its text — +retrying the same bytes under the same MIME type cannot succeed. + Body size limits are enforced by the **deployment**. Client preflight applies only when a served capability document supplies a max body size; otherwise the server rejects and the tool maps `body_too_large` / `empty_body`. Path reads diff --git a/website/src/app/docs/troubleshooting/page.mdx b/website/src/app/docs/troubleshooting/page.mdx index e0a5feb27..c528c9d4e 100644 --- a/website/src/app/docs/troubleshooting/page.mdx +++ b/website/src/app/docs/troubleshooting/page.mdx @@ -50,10 +50,12 @@ docker compose exec postgres psql -U rememberstack -d rememberstack -c \ ### 1. Unroutable MIME -Smoke profile routes **Markdown**. PDF/HTML/audio without a registered converter → non-retryable failure for that version. +Smoke profile routes **Markdown** and **plain text**. A type without a registered converter is **refused at ingest**, before any document version exists. HTTP returns `415 unsupported_media_type` naming the accepted set, and the MCP `ingest` tool returns the same code and set. Connector sync also refuses, but reports it only as an incremented `failed` count for that cycle — check the deployment's route table when a sync reports failures it cannot explain. Nothing is stored either way, so there is nothing to clean up. **Fix:** ingest `text/markdown`, or compose additional conversion routes. +Versions dead-lettered by an unroutable MIME *before* this refusal existed are still `failed`; adding the route does not retroactively convert them. Reopen those with `remember ops replay` once the route is in place. + ### 2. Missing / bad provider key Extraction and embeddings need a live provider. Bad credentials surface as stage failures / DLQ after retries.