Add MCP client tools with authority, taint, and published-server lifecycle - #178
Merged
Conversation
The pgvector path ranked a user's files by `ORDER BY embedding <-> query`
and nothing else. That is the single-vector paradigm at its most exposed,
and it contradicted this project's own rule: SPEC §2.5 said the hybrid
ranking applied "to rag, notes, and conversation recall alike", and notes
and compaction obeyed it while the primary RAG path did not.
Worse, RAGService was never passed `is_semantic`. With the default hash
encoder the production path therefore ranked documents by hash-vector
distance — the SPEC's own definition of noise — with no keyword channel to
fall back to.
Weller et al. (ICLR 2026) is the argument for why one channel is not
enough: the number of top-k document sets a d-dimensional embedding can
return is capped by d, so some relevant combinations are unreachable by any
query, whatever the encoder was trained on. Their LIMIT probe makes it
concrete — 46 documents, one-clause queries, best embedder at 54.3
recall@2 against BM25's 97.8. On a synonym rewrite of the same corpus BM25
collapses to 10.6 and the embedders hold. Neither channel is safe alone,
and they fail on disjoint inputs.
So retrieval now runs both and fuses them:
- a lexical channel over Postgres FTS, OR'd terms, backed by a GIN
expression index on to_tsvector('simple', content). Measured on 50k
chunks: 28.7 ms/query with the index, 239.7 ms without.
- weighted reciprocal rank fusion, semantic 0.55 / lexical 0.45. Rank,
not score: cosine is bounded and BM25 is not, so any weighted sum needs
a normalizer and every normalizer moves with the pool. Fusion by
position also lets a chunk both channels rank well beat one that only a
single channel loves.
- a channel ranks only what it matched. Zero is silence, not a weak
opinion, since an arbitrary order over non-matches would otherwise carry
the channel's full weight.
- without a real encoder the dense channel does not speak at all, and a
query with no lexical match returns empty rather than the nearest hash
vectors. Arbitrary chunks read to the model as evidence.
An optional rerank stage lands behind `rag_rerank`, off by default. The
serving model reads query and shortlist in one pass; it is the only stage
bound by neither ceiling and the only one that can say "none of these". It
is bounded, it treats the chunks as untrusted input to a decision inside an
explicit envelope, and it fails open — losing the model must never mean
losing the user's grounding.
SPEC §2.5 is amended rather than quietly broken: "bm25 is the tie-breaker,
never the peer" is not expressible as a rank fusion, and the evidence above
is against it. Lexical is now a weighted peer that can win.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
A closer read of Weller et al. changed three things. Fusion is now the only ranking mechanism, not one of three. Notes search weighted lexical 0.6 against semantic 0.4 — the opposite of the precedence rule it claimed to follow — and conversation recall had to score a turn it never had the budget to embed as a literal zero, which its weighted sum then held against that turn. Rank fusion has no such problem: a channel that did not rank something simply did not rank it. Both now call service/ranking.py. recall keeps semantic_weight; it is the semantic channel's fusion weight now rather than a blend coefficient. Reranking becomes auto | on | off, defaulting to auto. The paper's reranker result is a frontier long-context model solving all 1000 queries where the best embedder stayed under 60 — evidence that does not transfer to a 7B running locally, which is this project's premise. So auto asks whether there is positive evidence for the serving model: a curated family list plus the parameter count an open-weight name declares. Unknown is a no, because this stage can drop a user's grounding. A mixture-of-experts name reads as its per-expert size and lands off, which is the safe direction. The resolution is logged so an operator can see the guess instead of inferring it from latency, and on/off overrule it in either direction. Retrievers now return a shortlist and the stages above cut it, so both retrieval paths get the rerank stage and it sees more than the answer it is meant to improve. The candidate pool widens to the reranker's declared appetite: a reranker handed exactly the chunks that were going to be returned anyway can reorder them but never reach the one that placed just outside the cut. A bare NONE from the reranker is now honoured. It is the one thing this stage can say that no ranking can, and grounding an answer in chunks just judged irrelevant is how a model ends up citing text that does not support it. Anything with more to say than the word itself stays a hedge, and every other unreadable reply still fails open. SPEC §2.5 is rewritten rather than patched: the theory with its actual bounds, the LIMIT numbers for both splits, the synonym result that stops anyone concluding "just use BM25", the qrel-density metric that says which parts of this system sit at the hard end of the scale, and the finding that LIMIT does not correlate with BEIR — so a good benchmark position is not a mitigation. Multi-vector retrieval is recorded as deliberate future work with the reason it is not here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
This is the one entry in the paper that attacks the dimension bound rather than working around it. The bound describes a score that is one inner product between one query vector and one document vector; MaxSim is not that, and the authors say so in their limitations. It is also the only architecture they measure that beats single-vector on both LIMIT splits — 83.5 against 54.3 recall@2 on the 46-document set, 23.1 against 3.0 on the 50k one. A chunk is now optionally stored as several vectors in knowledge_chunk_vector, one per sentence-sized segment, and scored by MaxSim: for each part of the query, its best-matching part of the chunk, summed. Retrieval is the usual two stages — each query part gathers candidates by nearest segment, so a chunk qualifies on its best part rather than its average, and candidates are then scored exactly against all of their segments. Approximate search decides who is considered; it never decides the order. The failure this fixes is asserted, not assumed. A pooled embedding has to answer for a whole chunk at once, so a chunk covering two subjects lands between them and loses to one that is merely nearby. The tests build exactly that corpus and check both halves: pooled-only similarity returns the near miss first, and the same corpus with segments kept separate returns the right chunk. Neither test is true by construction of the other. It is not ColBERT and the code says so in as many words. Segments are sentence-sized and embedded by the same encoder as everything else, because that encoder is an OpenAI-compatible /embeddings endpoint and such an endpoint returns one vector per input — per-token vectors are not reachable through it. What carries over is the mechanism, not the granularity, at roughly an order of magnitude less storage. The seam is deliberate: a real late-interaction model replaces segment_text and the embed call without touching the storage, the candidate generation, or the scoring. Fusion gains the channel at 0.55 and the pooled vector steps back to 0.25 rather than out — the same signal read less precisely should not vote twice at full strength, but a whole-chunk vector still says something no single best part does. Off by default, and honestly so: it costs an embedding call per segment at ingestion and a row per segment in the index, it needs a real encoder, and coverage is not retroactive. A chunk without segments is unranked by the channel rather than penalised by it, which keeps a partly-migrated corpus working; that it tilts toward the covered part is the cost of enabling it before a backfill exists, and SPEC says so rather than leaving it to be found. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
A review of the three retrieval commits on this branch found eight real defects in them. Fixed here. Two were silent no-ops. The four new retrieval settings are read once, when RAGService is built, but were absent from both lists of model-affecting keys — so an admin could switch reranking on, read the new value back from the console, and have nothing happen until the process restarted. And late_segments defaulted to 0, clamped to max(1, ...), which made segment_text return a single part that _index_segments then skipped: late interaction reported itself enabled, did the extra query work, and kept an empty index. The floor is now 2, because one segment is the pooled vector under another name. One lost data. Widening the local_hybrid pool without removing the caller's truncation meant retrieve() cut an unsorted concatenation, so the first context filled every slot and the second was unreachable however well it matched. Those lists are ranked within a context and carry no score to rank across them, so they are interleaved now. One could fail an ingest that had already succeeded: _index_segments guarded the embed call but not the write, so a missing table or a dimension mismatch escaped to the caller as a 500 after the chunk rows were committed, and the retry duplicated them. The rest are smaller. Late candidates are now capped across all query parts rather than per part, with a set for the dedup — nine parts times a hundred each, all scored by pure-Python MaxSim, is not something to put on a synchronous path. A trailing fragment no longer earns its own vector: the merge only ever looked backwards at a segment that was still short, so a short last sentence survived alone, and MaxSim takes the best segment. Notes search scales its fused score to the fusion ceiling, because the search route publishes it as "score" and the witness report as "similarity", and a raw value tops out near 0.016. And conversation recall stops normalizing scores that rank fusion discards, and takes its weight from the shared constant instead of its own copy. Both regressions have tests that fail without the fix. Three findings are left alone as out of scope, all pre-existing: JSON Patch remove creates the parents it is removing from, taint withdraws run_python but not web_fetch, and "testserver" sits in the production tenant trust list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Three findings from the review of this branch, all pre-existing.
Tenancy had a hole where its own docstring promised strictness. localhost,
127.0.0.1, ::1 and the test client's testserver resolved to the default
tenant even with tenant_domains configured, on the theory that a probe
arrives by address rather than by site name. But Host is chosen by whoever
can reach the port, so anyone reaching the service directly named the
default tenant — and with allow_signup on, registered an account there. No
probe ever needed it: probes do not authenticate and never resolve a tenant.
The exemption is gone; an operator who wants a bare hostname served lists it
like any other.
The two halves are now one named rule rather than an inline comparison
repeated three times. The site comes from the host the server was addressed
as; the account comes from the authenticated session, never the request.
Neither is sufficient alone, which is the point: the host is attacker-chosen
on the unproxied path, and a session is a bearer credential that stays valid
against whatever site it is replayed at. A blank on either side is now a
mismatch rather than a skipped check — the caller with nothing to compare is
the one that resolved no site, which is the case least safe to wave through.
taint.py withdrew run_python and nothing else, which withdrew the wrong
thing. run_python's own schema promises no network, so it was never how a
secret leaves; web_fetch takes a model-supplied URL, which is exactly what an
injected page asks for. web_search too — the provider is fixed but the query
is not, and a query is as good a channel as a path for anything short. Local
search stays available, because a tainted turn must still be able to tell the
user what the page attempted. The refusal message says which of the two it
is.
JSON Patch resolved remove through the creating walker, so removing a missing
path built the containers it was removing from: remove /a/b on {} returned
{"a": {}}. Model-authored config ops and artifact PATCH bodies drop optional
keys routinely, so this was quietly writing empty objects into runtime config
and artifact schemas. remove now walks without creating and does nothing when
the parent is not there.
Left alone deliberately: apply_op still skips an op missing its verb or path
rather than rejecting it. The review read that as the same silent-success
problem, but test_config_ops.py asserts the skip by name, so it is a decision
this codebase already made rather than an oversight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The last commit tightened the tenant rule in three places, deleted the host exemption, and wrote SPEC prose asserting the invariant as though that were all of it. A second review found the four places it was not. complete_oauth never compared tenants at all. app_user.email is globally unique, so resolving an account by provider id or email finds it whatever site the flow began at — signing in with Google at globex minted acme's tokens, while the password path had always refused exactly that. login kept the old truthy comparison, so a blank site tenant short-circuited it to False and login would admit any user in any tenant while refresh and authenticate rejected the same request. Both now go through AuthService._site_matches, along with the two that were already converted. One method rather than four copies of a six-line rule: the copy that gets missed on the next edit is an authorization hole, and this commit exists because two were missed. Making a blank hint fail also made a blank default_tenant_id an unrecoverable lockout — every user's tenant is blank too, so every request 401s, including the admin call that would put the value back. The field now refuses to be empty. And deleting the infra-host exemption exposed a normalizer mismatch: settings split hosts at the first colon while the request path kept IPv6 brackets, so "::1" stored nothing, "[::1]" stored the key "[", and a bracketed Host matched neither. IPv6 loopback was unmappable and, without the exemption, unreachable. There is one normalizer now, and a bare literal canonicalizes to the bracketed spelling the wire uses. Retrieval had two of its own. The candidate cap added last commit was first-come across all query vectors, and the first vector is the whole query — so it took the entire pool and no query part was ever consulted, collapsing late interaction back to single-vector recall. Each part gets a share now. The local_hybrid interleave fixed one context starving the other but guaranteed an irrelevant context half the answer; the union is scored again so relevance decides, with the interleave surviving only as the tie-break. Smaller: fusion_ceiling counted channels that ranked nothing, so a note placing first in the only channel with an opinion published as half a bar. The model-affecting settings list is one tuple instead of two that had to be edited together — the drift that caused the bug it was added to fix. Dead _normalized_scores removed, compaction's redundant deferred import hoisted, and web.py's comment no longer claims a false positive costs only a redaction now that it also withdraws the turn's web access. ruff caught an undefined name in the semantic local_hybrid branch that no test reached. It has a test now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Fourteen findings, all real, none in tenancy — that area traced clean this pass. Most of these are mine from the last two commits. The worst was a NONE verdict returning the unread tail. The reranker reads the top rag_rerank_candidates and, told none of them help, was handing back everything below them — chunks that rank *worse* than the ones just judged unhelpful. "Nothing here answers this" became "here are the weaker ones", in a stage whose stated purpose is to stop a model citing text that does not support it. It returns nothing now, and the test that asserted the old behaviour asserted a bug. auto was judging the wrong model. Everywhere else in the kernel the serving model is `adapter_server_model or base_model`; reading base_model alone meant an operator pointing at a self-hosted 7B while leaving model_path at its default got reranking enabled on the strength of "gpt-4o-mini". Which was itself wrong twice over: a prefix match cannot tell gpt-4o from gpt-4o-mini, and gpt-4o-mini is the shipped default — so out of the box, auto turned the stage on for the smallest model in the family. Small variants are now matched as whole name parts, and a size the name declares beats family membership. The first attempt at that used a substring check and rejected every Gemini model there is, because "mini" lives inside "gemini". parse_order harvested digits from anywhere in the reply, including the visible reasoning blocks that o1, o3 and deepseek-r emit — so "passage 3 mentions 2024 revenue" parsed as a ranking, counted as success, and silently reordered the user's context. And a chunk could forge a candidate: the numbered list was newline-joined without collapsing the snippet, so a passage containing its own "[1] ..." line added an entry the model could pick. Late interaction had the mirror of the guard it already had: the write path degraded on a missing table, the read path did not, so enabling the setting on a database that had not had sql/schema.sql re-applied broke every retrieval instead of losing one channel. Indexing now also stops after the first failure rather than buying 4000 embeddings to throw away. The notes score was worse for being normalized. Rank fusion packs its scores together by construction, so scaling to the ceiling moved everything into 0.90-1.00 — and it went out under the same "similarity" name that vault_sweep uses for a real cosine, from the same module. Search publishes a rank now, and the witness publishes an actual cosine. And config.py held a second MODEL_AFFECTING_SETTINGS that the last commit did not update, so the console never labelled the four retrieval settings as rebuilding the model stack — while its own test compared the schema against that same stale list and agreed with itself. This is verbatim the drift the last commit claimed to have eliminated. There is one list now. Smaller: segment inserts batch instead of one round trip each, the local path stops embedding a query the store will ignore, a redundant index on (chunk_id) is dropped in favour of the composite that already covers it, and LegacyOnlyStore returns ids like the real store so a future late-interaction test cannot pass by skipping the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Thirteen findings. The pattern the reviewer named is the useful part: the three worst all sit in rerank.py, and each is a place where the comment or docstring states an invariant the code does not implement — and in two of them the test passed because it exercised a stub more capable than the real object, or a reply shape a real model would not emit. The adapter-server fix from the last pass did not work. LLMService keeps adapter_server_model on its backend, not on itself, so reading it off the service found nothing and fell back to the configured base — the exact bug the comment claimed to have fixed. It is a serving_model property now, on LLMService, because the tokenizer and the context window resolve the same pair and it had already been copied twice. The test builds a real LLMService; the SimpleNamespace that hid this is gone. The NONE branch refused to return the unread tail because it ranks below what the model rejected. The partial-rejection path — far more common — appended it anyway, so fusion ranks 21+ took grounding slots from head chunks the model had just read and dismissed. Only what the reranker kept comes back now. The reasoning-block strip only matched a closed <think>...</think>, so a reply truncated mid-thought was scanned for digits and the narration parsed as a ranking. And "last line with a digit" read an ordered list backwards: "2. Passage 1" parsed as the answer 2. The answer is picked by shape now. Two more that mattered. The lexical channel was filtered by embedding_model_id, so flipping that managed setting made every stored chunk invisible to keyword search as well as to vector search — retrieval returning nothing at all, for an exact filename as much as for a paraphrase, with no backfill job to recover. The filter belongs to the vector channels; keyword search compares no vectors. And late interaction's query side never ran on a real question: segment_text's 12-word floor is tuned for chunks, so every query folded back to one segment and the channel became the single-vector recall it exists to avoid. The query side has its own floor. The rest: min_length=1 accepted " ", so the lockout that constraint was added to prevent still happened; the ingest stop was scoped to one file, so a directory walk still paid segments x chunks embeddings per file; content_tsv is a stored generated column so ts_rank stops re-tokenizing every matching row; chunks.index() compared full embedding vectors to find a position enumerate gives for free; rerank borrows web.py's envelope markers instead of defining a second vocabulary; and add_chunk_vectors' docstring no longer claims a re-ingestion idempotence the ingest path does not provide. Not fixed, deliberately: rag_rerank and rag_rerank_candidates still rebuild the whole model service stack, because they are read at construction. The review is right that a prompt-sizing integer should not cost a teardown of the LLM, embeddings, training and workflow services, but the fix is to make RAGService read them per call, and that is a change worth making on its own rather than at the end of a review pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Both of its settings only ever shape one prompt: whether to run, and how many snippets go into it. Capturing them when RAGService was built made them structural anyway — they had to sit in MODEL_AFFECTING_SETTINGS, so nudging a candidate budget from 20 to 25 tore down and rebuilt the LLM backend, the embeddings service, RAG, training, the clusterer and the workflow engine, took the reload lock, and interrupted in-flight work. For a number that bounds a prompt. The closure-with-attributes is now an object that reads a settings provider per call. The runtime hands it `lambda: self.settings`, which refresh_settings already keeps current for every managed setting, so `auto`/`on`/`off` and the candidate budget take effect on the next turn with nothing rebuilt. RAGService needs no new knowledge for this. It already asked the reranker for its budget to size the candidate pool, and a disabled reranker now answers zero — so the pool does not widen for work that will not happen, and the stage hands back what it was given without a model call. The two late interaction settings stay in the rebuild list, because they change what ingestion writes rather than how one prompt is shaped. One thing that had to change with it: `auto` is re-decided every retrieval, so logging the resolution each time would be noise and logging none would leave an operator inferring the guess from latency. It logs on change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Eleven findings. The worst was an upgrade hazard I created last pass: content_tsv is a new column on an old table, and the startup verifier walks a table list, so an install that pulled this branch without re-running migrations booted clean and then answered every grounded chat turn with a 500. Startup now names the column and the fix, the lexical channel is guarded like the late one so a channel failing costs a channel rather than the turn, and knowledge_chunk_vector joins the required tables. Three defects in the reply parser, all in code whose comments claim the opposite. A single numbered line was not treated as a list, so "1. Passage 3" harvested both digits and promoted a chunk the model had not chosen. The NONE verdict was matched against the raw reply while numbers came from the reasoning-stripped one, so a bare NONE from any reasoning family — which is every family the allowlist targets — never registered. And the module docstring still said reranking was off by default after the default became auto. Two settings bugs of the same shape as ones already fixed. The encoder gate the pgvector path shed was still live on the memory path, so flipping embedding_model_id there still answered nothing at all until a full re-ingest. And _require_non_blank rejected " " but returned " acme " unstripped, which matches no account — the same lockout by a quieter route, and tenant_domains was already stripping its side. The witness report ranked and published hash-fallback cosine, because _pair_similarity never checked is_semantic. That is the one thing SPEC §2.5 says must never happen, in a module where search_notes had just been changed to obey it. Performance, both measured. MaxSim normalizes once per vector and compares by dot product instead of calling a general cosine per pair: 0.44 s to 0.069 s for a 20-chunk pool, same arithmetic. Segment ingestion embeds one batch per chunk rather than one call per segment, so a 500-chunk file is 500 round trips instead of 4000. Also: the reranker's two properties collapse into one, because `enabled` both mutated state and logged while being read twice per retrieval; and a dead branch in rank_turns whose comment described a decision the code did not make. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The previous commit shipped the code without these: the script that edits SPEC failed an assertion partway through, so it wrote nothing and the commit went out with the prose describing the old behaviour. Records what changed: the startup check for content_tsv and the guard behind it, the two parser rules (a single numbered line is still a list; NONE is tested against the same stripped text the numbers come from), and the two measured performance changes in late interaction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Five review passes over this branch's retrieval work found roughly sixty defects, and they fell into a small number of shapes that repeat. The three rules that would have caught most of them go in CLAUDE.md so the next session starts with them. Execute before claiming: "mini" is a substring of "gemini", and no amount of re-reading the line said so — running it did, immediately. Build test doubles from the real object: a SimpleNamespace carrying an attribute the real service does not have made a broken fix pass its own test twice. Grep the class when you fix the instance: the encoder gate was removed from one retrieval path and left on the other; the settings list was deduplicated in one file and not the second; the closed form of a tag was handled and the unclosed form was not. And the discipline underneath them: a comment is not evidence. Four times a comment or a SPEC line stated an invariant the code beside it did not implement, because the prose and the code came from the same intent and neither checked the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Standing authorization from the owner for multi-agent orchestration in code review specifically: fan out finders across distinct angles, then put every finding through two independent verifiers before reporting it — one that has to reproduce the failure by running code, one that argues the finding is wrong and rejects when uncertain. Both must agree for it to survive. Recorded here because it is an authorization, and the next session starts without it otherwise. It covers review; orchestrating other work still needs asking. The justification is measured. Five single-pass reviews of this branch each missed defects the next pass found, and several of those were bugs introduced by the previous pass's own fix. The finding rate never fell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The first review pass to run adversarial verification rather than one reader. Six finders across distinct angles, then every finding through a reproducer and a skeptic independently; a finding survived only if both agreed. The gate earned its cost in both directions. Three findings were rejected with substance — one reporter had measured segment_text on a string ingest_text never stores, one described a candidate-starvation bound that does not occur on any corpus the real ingester produces. Reading alone would have "fixed" all three. The worst confirmed defect loses the answer entirely under the kernel's own default configuration. Postgres indexes "user_id" as 'user' + 'id'; the BM25 tokenizer's \w+ keeps it whole. So the store's full-text query matched a chunk on "user id" and the re-score in _fuse gave it 0.0, ranked_positive deleted it as silence, and with the hash encoder — where lexical is the only live channel — retrieval returned nothing at all for a question the corpus answers. On a folder of ordinary source files, 6 of 9 queries came back with no grounding while the store had found the chunk. The class is wider than that tokenizer pair: any pre-filtered pool re-scored by a different scorer can be emptied. So the fix is not to match the two tokenizers but to stop discarding — BM25 orders the pgvector lexical pool and may no longer empty it, because membership in that pool IS the match signal. The local path keeps the silence rule, since its pool is a top-N by another score and a zero there really is a non-match. That distinction is a parameter, not a comment. The rest: The pooled vector was demoted for every chunk whenever ANY chunk in the pool had segments, so a chunk late interaction had nothing to say about was buried by its neighbours. It now steps back only for chunks the late channel ranked. segment_text packed into ceil(len/max) buckets, which yields fewer than the cap — nine pieces at a cap of eight produced five, missing the operator's budget by up to half. Two more in the reply parser. A newline-separated ranking collapsed to the model's LAST pick and discarded the rest, logging a successful rerank. And prose was read as a ranking: "NONE of the 5 passages help" grounded the answer in passage 5. Nothing ranking-shaped now means no opinion, which fails open. And ':' was in neither model-name separator class, so an Ollama tag (deepseek-r1:1.5b) or an OpenRouter suffix (openai/gpt-4o-mini:online) hid the part that decides and auto turned reranking on for a 1.5B. 25 of 34 findings went unverified — the run verified 9 by its own cap and said so rather than reading as full coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
…is total
Reliability and injection hardening for the rerank stage, closing the three
unverified findings from the adversarial pass and the transport problem
underneath all of them.
The structural change: when the serving backend speaks tool calls, the
verdict arrives as one (submit_ranking, ranking array, empty list = none
help) and is read from the tool_calls wire field — a channel beside content
that document text physically cannot write to. A passage, or an entire
reply, that spells out a perfect ranking call is still just characters in
the content channel. The parser problem and the forgery problem die
together, which is why the transport is the fix rather than another round
of parser patching. One model call either way: a tool-capable model that
answers in text falls through to the prose parser on the same response.
Both transports land in one validator — booleans rejected before int admits
them, since true would read as passage 1; picks deduped, range-checked,
bounded.
The prose parser remains for backends without tools, and it is now total.
Reply bounded before any regex touches it. Digit runs longer than any valid
index are prose, skipped before int() — CPython refuses str->int past ~4300
digits, and that ValueError sat outside the fail-open guard, so an
unreadable reply crashed the very turn fail-open exists to save. Ascending
spans ("1-3") expand so the middle passage is ranked instead of deleted; a
descending pair is ambiguous and fails open rather than guessing. A bare
</think> with no opener — what an R1-style template leaves when the opener
lives in the prompt — marks everything before it as reasoning, and the NONE
check runs on the same stripped text. An explicit trailing answer
("Final: 3, 1") outranks numbered narration, whose prose digits otherwise
pollute the order.
The query seam is closed. It sits outside the untrusted envelope because
the model must read it as the question — and on the agent path it is
model-authored, which after a tainted fetch means attacker-influenced. It
gets the passage treatment anyway: collapsed to one line so it cannot mint
a numbered entry or an instruction block, markers neutralized so it cannot
touch the envelope, bounded so it cannot bury the instructions after it.
SPEC states out-of-band verdicts as a rule, not a reranker feature: any
verdict that gates data prefers a structured channel; surviving prose
parsers must be total and fail open; the witness and digest parsers are
named as the next candidates.
The end-to-end test the adversarial pass flagged as missing exists now — a
reranker inside a real RAGService against a real store, off/on/NONE/garbage
— plus the tool transport verified against the exact dict shape
model_backend returns, and every parser case above as a regression test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The tool transport made prose the degraded mode, and a degraded mode nobody is told about is a wrong answer waiting to be diagnosed from the outside. The component now says it, in both places an operator looks. The reranker owns a transport property — None, "tool", or "prose" — and an active reranker resolving to prose logs rag_rerank_prose_transport once per transition, not per retrieval: a warning on every turn teaches the operator to ignore the log, which is worse than no warning. The admin console gets the same fact as a runtime warning the schema route attaches to the rag_rerank entry, rendered beside the control, present while the condition holds and gone when it clears. Which backends this names, verified from the code rather than asserted: local_lora / local_gpu_lora have no tool method at all — the project's own self-hosted premise is exactly the deployment that cannot signal out-of-band today — and an API backend without a configured client is the other. Wire support is not model behaviour, though: a small model behind a tool-capable endpoint can still answer in text every time, which the latched warning cannot see. So every prose-path verdict also logs transport="text", tool-capable wire or not; the warning sees the wire, the per-event field shows the habit. Building the end-to-end test exposed a gap the per-call settings change left behind. Removing rag_rerank from MODEL_AFFECTING_SETTINGS also removed the only synchronous refresh the settings PUT gave it — every other setting leans on the polling watcher, which exists for the OTHER workers, so "the change is live" was true only after an interval nobody promised the admin. The PUT now refreshes its own worker's settings before responding; reads-per-retrieval of a stale object were still stale. The console flow is tested through the real endpoint with a real admin: warning absent while off, present after PUT rag_rerank=on against the tool-less test backend, gone again after PUT auto. The latch is tested for once-per-transition including re-warning after off-and-on, and a tool-capable double never draws it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The premise deployment was the one stuck on the degraded transport:
local_lora / local_gpu_lora had no generate_with_tools at all, so the
self-hosted JAX path — the deployment this project exists for — could not
signal out-of-band, and an active reranker on it drew the admin warning
with no way to clear it.
A raw checkpoint has no second wire, so the channel is what vllm and
llama.cpp do serverside: a contract the backend enforces. Tools are
advertised in a system block (JSON Schema plus the emission format), the
model emits <tool_call>{json}</tool_call> — the de-facto local standard,
the tag Qwen and Hermes chat templates already emit — and the backend
parses that block out of MODEL OUTPUT ONLY, returning the same dict shape
as the API backend so nothing downstream can tell the transports apart.
The property that makes a provider's tool channel unforgeable by documents
survives in one line: input text is never parsed. A chunk, a fetched page,
a pasted document can spell the tag — it lands in input, and only the
model writes to the output stream. The test states that as an executable
fact: a perfect call block in the input never becomes a call.
The extractor refuses to guess. A malformed block stays visible text
rather than becoming an inferred call — digit-harvesting wearing a new tag
— and shapes that are not calls (empty name, non-dict arguments, a JSON
array) are rejected. Call count and block size are bounded before
json.loads sees anything.
Two honest limits, stated where they bind. Whether a given checkpoint
emits the contract is model behaviour, not a capability the flag can
promise — it shows per event as transport="text", the same wrinkle
tool-capable API wires have. And a parrot-prone small model is one echo
away from carrying a document's block into its own output, so
neutralize_markers now defangs the tag in untrusted input alongside the
envelope markers — every consumer of that function gains the defense at
once, the rerank prompt included.
supports_tools is a side-effect-free property (reading a capability flag
must not load a tokenizer or touch JAX), and its going true also opens the
agent tool loop for local deployments — same channel, same contract,
deliberate. The admin prose-transport warning now names only the backend
that earns it: an API backend without a configured client.
Verified by execution end to end before the tests were written: a real
LLMService over the real local backend, only the forward pass canned,
reranks over the tool channel with transport == "tool", and an empty
ranking is the NONE verdict on the same wire.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
POST /v1/responses speaks OpenAI's Responses dialect over the kernel's own
chat turn, so any agent framework gets personas, adapters, hybrid RAG,
notes and memory behind a base-model-shaped endpoint. Success bodies are
the bare Responses object and errors are OpenAI's {"error": {...}} — SDKs
parse by shape, so the Envelope never leaks here. previous_response_id
resolves through the assistant message to its conversation; context_id (a
liminallm extension) grounds the thread on the first turn. v1 rejects
stream, caller tools, instructions and store=false by name.
Auth for headless agents: sk-liminal- API keys, SHA-256 at rest, plaintext
shown once. Only the /v1/responses dependency reads keys — a leaked key
can chat and nothing else; it cannot list conversations or mint or revoke
keys. Mint/list/revoke at /v1/auth/api-keys (session auth, 20 active max,
tombstone revocation) or from the new API Keys section on the Settings
tab. Agent-created conversations carry meta.source="responses" and the
sidebar tags them "api".
The kernel's internal tool loop keeps its transport on this surface —
including local_lora/local_gpu_lora via the advertised tool channel — so
agents get the same grounded answers on every backend.
SPEC: §2.1 user_api_key, §13 envelope exception, §13.1 served surface,
§13.2 key endpoints, §17 UI expectations. README: agents section +
endpoints. Tests: 19 wire-shape/continuity + 7 key-lifecycle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
…input cap
A review probe reproduced three leaks on POST /v1/responses. A JSON-array
or malformed body drew FastAPI's 422 {"detail": ...} — the exact shape the
route's docstring claimed to prevent; the body is now read raw and
validated by hand, so both cases 400 in OpenAI's vocabulary. A ServiceError
mid-turn (provider failure) escaped to the app-wide handler and answered in
the kernel Envelope; the route now reshapes service errors (status kept),
storage conflicts (409), and crashes (generic 500 — internals never reach
the wire). And input had no length bound while ChatMessage.content caps at
100k chars to prevent DoS: the same cap now applies, enforced as item text
accumulates rather than measured after the join.
Six regression tests pin the shapes; SPEC's wire-shape and scope bullets
describe the hardened behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Three gaps between this kernel and the frontier, closed in one pass: - The agent loop runs a round of read-only tool calls (file_search, history_search, note_search) concurrently, results and snippets always in call order. Rounds containing a taint-class tool stay strictly sequential — a web_fetch that records an injection finding must be able to withdraw run_python later in the same round, and that ordering only exists one call at a time. The egress guard is thread-local, so the round runner re-applies it inside every worker; without that, the socket allowlist would silently permit. - POST /v1/responses speaks stream:true as SSE response.* events (created -> deltas -> completed, response.failed on error), with the reply's id minted before the first event and the assistant message persisted under it, so created and completed carry the same id. Everything refusable refuses before the stream starts; admission slots release however the stream ends. - POST /v1/mcp is a minimal MCP server (Streamable HTTP, 2025-06-18): initialize, ping, tools/list, tools/call; notifications 202; batching rejected by name; GET 405. Two read-only tools backed by the kernel's own retrieval: note_search and knowledge_search. Read-only is the security posture — no egress for an injected document to abuse. Same API keys as /v1/responses; the SPEC carries the MCP roadmap (resources, prompts, OAuth, and an MCP client under the taint discipline). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The route walker caught GET /v1/mcp enforcing no rate limit. It is a constant 405 — this server offers no server-initiated stream — which is the version_info class of endpoint: the limit would cost more than the handler. The POST half, where all the work happens, authenticates and limits. Recorded in EXEMPT with the reason, the way the sweep asks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The compat layer carried reasoning_tokens and cached_tokens through the agent loop so a consumer could see them, and the served usage mapper then threw them away in the last ten feet. Now usage serves the details objects (always present, zeros when unknown — typed SDKs require the fields) and the total falls back to input+output for backends that report parts without a sum. The local JAX path also stamps total_tokens at the source, so its own tokenizer's counts arrive whole on every surface. Server-side tool runs stop being invisible: file_search and web_search appear in output as the dialect's own items (file_search_call with queries, web_search_call), streamed as output_item added/done pairs that close before the message item opens. Only dialect-native types are used — typed SDK parsers never meet an unknown discriminator — so note_search, history_search, run_python and web_fetch stay out of output and ride the full trace instead. Grounding snippets, that full trace, and the active adapters ride under one namespaced top-level key (liminallm), which the OpenAI SDKs preserve and strict readers never see. Citations are deliberately NOT faked into annotations: an annotation needs a character anchor and a file identity this surface cannot honestly provide. At ingestion, refusal parts now count as text — a reply that was entirely a refusal used to flatten to "" and the turn then fabricated "No response generated." over the model's actual words. Skipped on purpose: an "incomplete" status for round-capped agent turns (the loop withholds tools on the last round to force a real answer — that is completion by design, not truncation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
The Responses ingestion path and the Gemini shim both carried cached_tokens and reasoning_tokens through; the chat.completions sites read only the three totals — silencing exactly the servers the self-hosted lane runs, since vLLM-style prefix caching reports its cache hits as prompt_tokens_details.cached_tokens on that transport. One _chat_usage helper now serves all three sites (blocking, tool-calling, streaming), with the same flat-int convention the other compat layers use, so the loop aggregates the details and the served usage fills in with no consumer changes. On local_lora/local_gpu_lora cached stays 0 truthfully: the local forward pass keeps no KV state to reuse, so there is nothing cached to report — the SPEC now says so rather than leaving the zero to be read as a gap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
…euse Serving used to run LoRA over a synthetic embedding table — sin(arange(...)) with no attention and no layers. That made the local lane shape-correct and semantically empty: it could not answer anything, and it held no KV state, which is why local cached_tokens had nothing to report. service/transformer.py is the base model that stand-in was standing in for: plain JAX (no flax, no torch), RMSNorm + RoPE + grouped-query attention with a KV cache + SwiGLU, loading config.json and *.safetensors straight from the model directory. A missing tensor raises rather than defaulting; a half-loaded model answers confidently and wrongly. Four invariants are pinned by tests that run a real checkpoint, because each is the kind that reads correct and is not: incremental decode reproduces a full recompute, attention is causal, a LoRA adapter at B=0 changes not one logit, and a warm prefix cache produces byte-identical output to a cold one. The prefix cache exploits the one property chat guarantees — turn N's prompt is a strict prefix of turn N+1's. Entries are content-addressed and adapter-keyed, matched on strict token prefix only, bounded by a token budget, and cleared outright whenever adapter weights actually reload. The reused length is reported as cached_tokens, so it surfaces as input_tokens_details.cached_tokens on the served Responses API with no consumer change. Two defects the execution found, both from this project's recurring classes: - Clamping out-of-vocab ids collapsed distinct prompts into identical model input. The fix is upstream — a loaded checkpoint's vocabulary is authoritative, including for the tokenizer fallback. - test_adapter_checksum built its backend with __new__ plus hand-set attributes, so it broke the moment the class gained state. It now constructs the real object, which cannot drift from the real interface. Training still fits LoRA against the old synthetic table, so adapters it produces do not match the serving matrices. They are refused whole and logged rather than half-applied, and SPEC §5.1 carries that gap as a named next step instead of implying the ladder works locally. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
SPEC §5.4.4 defines the loss over model_apply(params_base, lora_params, inputs). The loop instead minimised a loss over a synthetic sine embedding table with no attention and no positions: a context-free bigram task over a geometry where token ids are neighbours because their integers are. The optimizer worked, the gate ran, versions were written, and the weights fitted no model that exists. Training now loads the same frozen checkpoint serving loads, applies the LoRA matrices inside its attention projections, and differentiates only those matrices — the base parameters are closed over, so "only on adapters, never on the base model" is structural rather than a promise, and a test asserts they come out bit-identical. The L2 term SPEC asks for is in the objective; the holdout number deliberately is not, because B starts at zero and can only grow, so charging the regularizer to the eval would count learning as a penalty against promotion. Matrices are now sized from the checkpoint per §5.2 — A is [r, d_in], B is [d_out, r], and grouped-query attention makes k/v narrower than q, which one guessed hidden width could never express. B initializes to zero so a fresh adapter is exactly the identity, which is what lets §5.5 hold an adapter on the prompt rung without perturbing the model before data has earned any weights. What cannot be trained is now skipped rather than faked: no base checkpoint, no LoRA matrices, or matrices that match no projection. A skipped run never promotes (§5.4.6), so the adapter stays on the prompt rung. A dataset too small to hold anything out no longer promotes either — "promoted only when holdout loss improves" refuses what it cannot measure, and unevaluated weights now change the model. Verified end to end on a real checkpoint: after training toward a target, that target's rank in the serving model improves from 8 to 2, and serving's blended weights are bit-identical to the trained file. Two test doubles built with __new__ and hand-set attributes are now real objects; they encode what the test believed the class held, and broke as soon as it held anything else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Review of 595ae38 found the real-forward trainer had made three pre-existing paths load-bearing, and each was wrong. Composition did not implement the §5.2 equation its own docstring quoted. It gate-weighted A and B separately, summed across adapters and divided by the total weight: for one adapter that computes (gA)/g = A, so the router's gate cancelled itself and 0.2 behaved exactly like 1.0; for two it forms the product of averages, whose expansion contains B1A2 and B2A1 — one adapter's up-projection against another's down-projection, terms the SPEC sum does not have. Composition is now by rank concatenation, A* = [A1;A2] and B* = [g1a1B1, g2a2B2], so B*A* = sum_j g_j a_j B_j A_j exactly. Ranks may differ; a closed gate contributes nothing rather than being normalized back into existence. The declared `.scale` key is now recognized rather than reported as an unmatched name, so the serialization contract round-trips. Tokenizer fidelity was the larger hole. Training and serving each had a hash fallback, and they are different hashes — poly-31 against FNV-1a — so a real checkpoint whose tokenizer failed to load could train an adapter on ids serving never emits, pass a holdout tokenized the same wrong way, and promote. "Train against the model that will serve it" includes its tokenizer: serving now refuses such a checkpoint, training skips, and an id outside the checkpoint's vocabulary is refused rather than clipped into a token nobody wrote. SFT sequences encoded prompt and target independently, splicing a second BOS into the middle for any tokenizer that adds one, and truncated by slicing the head of prompt+target — which can drop the entire supervised span and leave an all-zero loss mask, an example that reports zero loss and reads as perfectly learned. The target is now reserved first and the oldest prompt context trimmed, and an example with no supervised token is dropped rather than emitted. The tests could not have caught any of this: every training test began from hand-written token ids, and the fixture had no tokenizer at all. The fixture now ships a real one beside the weights, and the new end-to-end test runs text through the production tokenizer into training and out through serving with no hand-written ids anywhere. Two tests asserting the averaging behaviour now assert the composition equation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Review of a39d742 found the composition equation was exactly right about a number it never received. RouterEngine computes a gate per adapter, but _select_adapters rebuilt the activated list from the candidate lookup and returned the gates separately for tracing, so the weight never reached the backend, which defaulted to 1.0. A policy asking for 0.18 was served at 1.0. The gate now travels on the adapter dict it gates; the backend still reads one place. That made a second defect reachable: the KV prefix cache keyed on adapter id and version, so the same adapter at 0.2 and 0.8 shared a cache identity even though every cached tensor was computed under one of them. The signature now includes the gate. SPEC said both things at once - gates are per-request in 5.3, the cache key excluded them - and is corrected. Four more, each a contract stated and not implemented: alpha was documented as a constant but entered the optimizer tree, where the L2 term alone gives it a gradient; evaluation used the original value while the file written afterwards carried the moved one, so a gate could pass under one model and serve another. The trainable tree is now the LoRA matrices only and constants reattach at serialization. Prompt truncation asked the tokenizer to truncate first. Tokenizers truncate from the right, keeping the OLDEST tokens, so the deliberate "keep the newest context" slice ran on tokens that had already lost it. Truncation is now the caller's decision, and the sentinel test uses a tokenizer that refuses to truncate so the code must do it. An empty target became [0] rather than being dropped: supervision teaching the model to emit token 0, carrying positive mask weight so no zero-mask check could catch it. Serving still clipped an out-of-vocabulary id after training learned to refuse one - the same fix stopping at the first instance. It refuses now, and checks negative ids too. Composition itself partially applied: an A without its B, or adapters disagreeing on a projection's dimensions, logged and continued with parts[0], serving a stack the router never chose. Both refuse the whole stack. The adapter RAM cache is keyed (adapter_id, version) as 5.3 declares. A test places a decoy v0002 that must lose to the promoted v0001. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
Review of 5f123ca found the mechanics correct and two states still conflated. _ensure_model treated _model_error as terminal, so a refused request was followed by one served from the synthetic stand-in: generate() only asked whether _model_state existed, and "no real model" meant both "dev box, no checkpoint" and "checkpoint present but unusable". The missing-tokenizer case never raised at all. There are now three states. absent allows the stand-in and says so; valid serves the real model; broken raises on every request, because a production configuration failure is not a development lane. Un-promoted weights were servable through a second door. current_version of 0 pinned nothing and fell through to a directory scan, and a training job writes vNNNN/params.json before its eval gate runs - so a prompt-rung adapter could serve weights the gate never saw, permanently if a crash landed between writing and quarantine. Version 0 now resolves to no weights, and prompt mode contributes none regardless of what is on disk. Two locks, because one is a race. Training and serving also did not serialize the same conversation. The role labels are tokens to a raw decoder, so USER: and user: are different inputs; serving still let the tokenizer truncate, which keeps the oldest tokens, while training had been fixed to keep the newest. Both now use one serializer for labels, the context marker, and truncation. SFT prompts included messages from after the feedback target: the target row was skipped but later turns were not, so an event trained after the conversation continued taught its answer conditioned on the future. The bound is now the target's sequence number. Three smaller contracts: batch shape reported the source slice rather than the rows emitted after dropping targetless examples; an orphan B was invisible because the composition loop is A-driven, leaving one silent case in "malformed weights fail visibly"; and the gate cache key rounded to six decimals, where two distinct gates could share KV state. The end-to-end test is new and was written wrong first: an if/else on promotion meant every run took the refusal branch, because the fixture improves its holdout by less than the 1% bar - it would have passed with promotion entirely broken. It now relaxes only the threshold and asserts both that the improvement was real and that promotion happened, then follows one value from preference events through training, the gate, promotion, a routed 0.4 gate and LLMService into local generation, with a decoy version on disk that must lose. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
…ing them Review of 822f817 found five rules stated in SPEC and true only in the paths already under test. Partial application survived one level above the fix. The pair and dimension checks live in composition, but assembly then skipped names it did not recognize and applied the rest, so an adapter carrying a valid Q pair beside a foreign one still changed the model while the foreign half went to a log line. One validate_lora_weights checks name, target, layer index, pairing, rank agreement and the projection's real dimensions, and raises on the first violation. Serving calls it before assembly, training before indexing - training skips, and a skipped run cannot promote. Hybrid adapters were served twice. Prompt injection keyed off the adapter's own backend field and never asked what the active backend does, so a graduated skill on local JAX received its trained weights and the instructions they were distilled from: an input the eval gate never scored. The local backend now advertises that it applies weights, and a promoted hybrid adapter skips injection there while keeping it on API backends. An unpromoted hybrid keeps its prompt everywhere, since no weights will load for it. The SFT prompt boundary needed the store, not a window. list_messages returns the newest 200, so a target older than that was simply absent, target_seq was None, and the guard did nothing - every later turn became training context for exactly the events most likely to have them. The target is now fetched by id and history queried as seq < target_seq; an unresolvable target drops the example. Fetching the target directly also keeps its content available as the label, which bounding alone would have silently removed. Context reached the local model in a format training never wrote: an appended "Context: a | b" inside the user turn, rather than the context: turn the adapter was fitted against. The local path now uses the shared serializer; API backends keep the provider representation. And a path pointing straight at params.json answered before the version check, so a direct file served at current_version 0. The end-to-end test is now what its docstring claimed: the skill is born on the prompt rung through SemanticClusterer, trained, graduated to hybrid with its prompt retained, routed at 0.4 by a real policy through RouterEngine and WorkflowEngine, and served with a context snippet - asserting both that the context uses the shared format and that no fallback prompt accompanies the weights. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
…lacement Review of cfca075 found four rules enforced one step too late or read from the wrong field. Serving validated the composed pair rather than each adapter. Composition carries only A/B pairs forward, so a foreign key never reached a validator running afterwards - and concatenation adds ranks up, so two adapters that each disagree with themselves (A rank 2 with B rank 1, and A rank 1 with B rank 2) compose into a pair whose totals agree at 3 and 3 while every row pairs with the wrong column. Validation now runs on each adapter's raw matrices as they load, with the composed pair checked afterwards as defense; without a loaded model, self-consistency is still enforced, which is the check that catches that case. A selected adapter could also leave the stack silently: a promoted local/hybrid adapter whose weights would not load was skipped and the rest served. Weightless is legitimate only where the ladder says so - prompt rung, nothing promoted, closed gate - and otherwise refuses. Version authority had two escape hatches. A `latest` pointer was trusted without checking where it resolved, so version 1 could serve v0002 whenever v0001 was missing; and a path pointing straight at params.json was accepted for any positive version, serving a file on the artifact's say-so. A bare file cannot demonstrate which version it is, so it can only satisfy an artifact that was never versioned. The earlier test asserted the opposite and is inverted. The background worker defaulted a missing gate decision to promoted, while the run summary it read had dropped eval_gate entirely - so gate-rejected runs were marked succeeded and credited the adapter's router state for a rollout that never happened. The summary carries the decision; absent means unknown, and unknown is not approval. Prompt injection branched on the legacy `backend` field while SPEC calls `mode` authoritative, so mode/backend disagreement gave an adapter both its weights and the prompt they were distilled from, or neither. And training and serving agreed on the context marker while disagreeing on where it goes: appended after every message on one side, inserted before the final user turn on the other. For a raw decoder that is a different input. Placement is now one function, and the test compares the exact serialized string from both sides. SPEC 6.1 described hybrid as a controller/executor plan, contradicting 5.0.1; 5.0.1 controls and 6.1 is corrected to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q1aXnifvwPBfqtnaCYt14n
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…test-pg CI's lint job failed on the first push, and every other job is `needs: lint`, so the 3.10/3.11/3.12 matrix and the browser job never ran. Seven errors, none of them on main — all introduced somewhere in this branch and invisible to every local run. The reason they were invisible is the interesting half. `make lint` passed `--ignore E402`, and ruff's `--ignore` does not add to the configured ignore list, it replaces it. `[tool.ruff.lint]` in pyproject.toml already says `select = [E, F, W, I]` and `ignore = [E501]`, so the flag suppressed E402 locally and re-enabled E501, while CI — whose explicit `--select`/`--ignore` only restate the same config — had it the other way round. Five E402s and two unsorted import blocks sat on the branch through every `ruff check` I ran. So the flags come off: `ruff check liminallm/ --fix` uses pyproject, which is what CI uses. The tests line keeps its relaxation with `--extend-ignore`, which adds rather than replaces. The seven errors are fixed at the cause rather than suppressed. The E402s were not deliberate late imports — `_password_hasher` had been inserted above `auth.py`'s import block, so the block is moved back above it. The two I001s are ruff's own sort. Separately, from Bugbot on the PR: `test-pg` was the one lane still running `pytest tests/` with no marker filter, missed when the browser marker was introduced. Measured rather than assumed — with Playwright installed by the dev extra and no Chromium binary, a browser test *errors* rather than skipping, so `make test-pg` failed after an ordinary `make install`. Not fixed here, and pre-existing on main: `make lint` also fails on `tests/` (22 errors there, 25 on this branch) — unsorted imports, `l` as a variable name, and six repeated dict keys whose values are identical, so nothing is dropped. Unrelated to this PR, and CI does not lint `tests/`. Full parallel lane 2816 passed, 26 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
`main`'s security job has failed on every push since 2025-11-30 — about thirty consecutive runs, last green at 911e7df. The step is byte-identical between main and this branch, so nothing here caused it; the gate has simply not been read in nine months. Fifteen bandit findings at `-ll --skip B101`, twelve of them already on main. `git blame` against origin/main identified the three this branch added: all B608, all in postgres.py, all the same shape as seven that were already there. All fifteen were examined rather than suppressed on sight. Ten B608 are dynamic SQL where every interpolated fragment is a source literal — "title = %s", "visibility = 'private'" — selected by an `is not None` check. No caller value reaches the f-string and every value is bound. Suppressed per line with that reason. One B613, the only HIGH, is fixed rather than suppressed. `web.py` held raw bidi and zero-width characters inside `_INVISIBLE_RE`, the class it uses to strip exactly those characters from fetched pages. It is data, not a Trojan Source attack — but a character class nobody can read in an editor or a diff is not reviewable, and a file carrying raw bidi controls has the attack's shape whatever the intent. Now written as `\u` escapes with a comment per range, and proven equivalent by comparing old against new across all 1,114,112 codepoints: zero differences, 155 characters matched by both. B314 (ElementTree in the extractor, no external entities, size-guarded, and confined to the extraction child) and B102 (exec in the code interpreter, which is its purpose) are suppressed with their reasons. Two B615 are the only findings that are not false positives — revision pinning for `from_pretrained` is real supply-chain hardening — and are suppressed with a comment saying the pinning question is a product decision rather than something to settle in a lint pass. One defect made and caught while doing this: the first pass appended `# nosec` by line number, and one of those lines opened a triple-quoted f-string, so the comment became part of the SQL. Bandit was satisfied and the statement still parsed. Found by asserting the real property rather than the proxy — walking every module's AST for a string literal containing "nosec", of which there may be none. That statement is now concatenated so the suppression has a line to sit on. `make security` and CI still differ (`-q` against `--skip B101`), left alone because CI is the more permissive of the two: the local command cannot pass while CI fails, which is the safe direction for a mismatch. Full parallel lane 2816 passed, 26 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
With lint passing, the matrix ran for the first time and every job that
loads the suite died before a single test:
tests/conftest.py:20: from tests.harness import run_id, worker_id
E ModuleNotFoundError: No module named 'tests'
Not a 3.12 problem, though that is the job that reported it — reproduced on
3.11 locally in one command. `python -m pytest` puts the working directory
on sys.path; bare `pytest` does not, and CI runs bare `pytest`. Every local
run on this branch used the first form. So a conftest importing
`tests.harness`, which this branch introduced along with the worker
isolation, was never exercised the way CI would exercise it. The browser
job failed identically, same line, same cause.
`pythonpath = ["."]` in [tool.pytest.ini_options] makes the two the same
invocation, which is the property that was missing rather than the path
itself. Verified by running both lanes with bare pytest, as CI does: 2816
passed and 26 skipped on the non-browser lane, 11 passed on the browser
lane.
Side effect worth knowing: CI installs the project non-editably, so
`import liminallm` resolved to site-packages. With the repository root on
the path it resolves to the checked-out tree — the copy the run is meant
to be testing.
That is the third gate on this branch whose local command differed from its
blocking command, after the ruff flags and the unread bandit findings. In
all three the local form was the more permissive one, so local green meant
nothing and nothing showed that it meant nothing. Recorded in
docs/ISSUES.md as one lesson rather than three incidents.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
CI got as far as importing the application and died there on every Python
version:
liminallm/service/auth.py:17: import httpx
E ModuleNotFoundError: No module named 'httpx'
`httpx` is imported at module scope by five files — auth, web, sandbox,
voice, gemini_backend — and appears in no dependency list. It has only ever
arrived because `openai` depended on it.
Not a resolution accident: resolving the base set as CI does gives
`openai==3.3.1`, and openai 3.x moved from `httpx` to `httpx2`. Locally the
dev extra pins `openai>=2.8.1` and the lockfile holds 2.8.1, which still
uses `httpx` — so every local environment had it and no CI environment did.
Measured with `uv pip compile` on the exact base set, before and after:
absent, then `httpx==0.28.1` alongside `httpx2==2.12.0`.
A direct import satisfied by somebody else's requirement holds only until
their requirement changes, and when this one broke the application did not
degrade — it failed to import, so every test job died in the conftest.
A sweep of every third-party import in `liminallm/` found two more
undeclared, neither a defect: `numpy` is function-local beside
`safetensors.numpy` in the checkpoint loader, now named in the train extra
because the code imports it directly; `tiktoken` sits in a `try:` with a
heuristic fallback, which is what optional should look like.
`tests/test_declared_dependencies.py` enforces the rule going forward, on
position rather than identity: a module-scope third-party import must be a
declared base dependency, a function-local one need not be. It carries its
own can-see-something check so a broken walk cannot report a clean list, and
pins numpy and tiktoken as deliberately lazy so moving either becomes a
decision. Removing the httpx line fails it.
Still unqualified and recorded rather than fixed: CI installs unpinned and
therefore tests against an openai the suite has never been run against.
3.3.1 does still export every Responses type those tests import, so they
will collect; whether the shapes validate is what the run will say.
Full parallel lane 2820 passed, 26 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Declaring httpx got CI past the conftest for the first time. The 3.10 job reached 2701 collected items, where every earlier run had died before collecting one. It then reported two more instances of the shape the guard was written to catch, both introduced by the guard itself. tomllib entered the standard library in 3.11. This project's floor is 3.10, where it is the tomli backport under another name. So the test that catches undeclared dependencies was itself an undeclared dependency, on the one interpreter that had to be checked and was not. tomli is declared in the dev extra rather than taken from pytest, which does supply it on 3.10 -- measured, pytest 9.1.1 requires it there. That is the same arrangement httpx had: a direct import satisfied by somebody else's requirement, which holds until their requirement changes. packaging.requirements came out for the same reason, in favour of a regex over the distribution name. Two entries in the name map could never match, because the regex strips the extra before the lookup and both fell through to a default that happened to agree; a test now rejects that shape. numpy is in the train extra, which no CI lane installs. The test job gets it because its install line names jax, so it looks available everywhere. The browser job installs base plus dev only. Two test modules new on this branch imported numpy at module scope beside their own importorskip guards for jax and safetensors, and took that lane down at collection -- not a failing test but an aborted run, deselecting 2694 tests that never executed. The guard now walks tests/ as well, against the narrowest lane rather than against [project] dependencies: a test module may import at module scope only what every lane installs, and reaches anything else through importorskip. Verified rather than reasoned about. The tomllib fix runs on a real 3.10 interpreter, and removing it reproduces the collection error. The numpy fix was checked by blocking numpy on sys.meta_path to recreate the browser lane's install set: the mutant gives "Interrupted: 1 error during collection", the fix collects cleanly. Full parallel lane: 2823 passed, 26 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
The tests/ guard added in the previous commit asks whether a module-scope import is in base plus dev, which is what the browser lane installs. It read requirement strings with a regex that takes the distribution name and stops, so an environment marker was invisible to it. tomli>=2.0; python_version < '3.11' is declared in the dev extra, so the set named installed_everywhere contained tomli -- a package installed on 3.10 and on nothing else. The browser lane runs 3.11. A module-scope import tomli in tests/ would have passed the guard and aborted that lane's collection anyway, which is the one failure the guard exists to prevent. Reported by Cursor Bugbot against 1030758. Measured before fixing: 'tomli' in the trusted set was True, and find_spec("tomli") on the 3.11 interpreter this suite runs on returned None. The set was named for a property it did not have. Any marker now disqualifies a name, including one that would hold everywhere. The parse cannot evaluate markers and should not pretend to, and the two ways of being wrong are not symmetric: too strict costs one unnecessary importorskip, too lax costs an aborted lane. Witnessed behaviourally rather than by inspecting the set. A module-scope import tomli dropped into tests/ is flagged with the fix in place; with the marker exclusion reverted the same file passes. That is the reported hole, reproduced and closed. 8 passed on 3.10 and on 3.11; fast parallel lane 2713 passed, 25 skipped. Three findings in this file now, all the same sentence with a different subject: what is declared, what is imported and what is installed are three different sets, and every defect here came from treating two of them as one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
CI's 3.10 job failed after fourteen minutes, with 3.11 and 3.12 cancelled by matrix fail-fast rather than failed -- so CI could not say whether the defect was version-specific, and the local 3.11 lane had passed 2823 tests half an hour earlier. Reproduced by building a 3.10 environment with CI's own two install lines, which resolves openai 3.3.1 where every local environment held 2.8.1. openai 3.x made input_tokens_details.cache_write_tokens a required field. In 2.8.1 that object required only cached_tokens, and the served usage block emitted only that -- so as of 3.x this server had stopped conforming to the dialect it claims to speak. _responses_usage's own docstring already states the rule it broke: the details objects are always present, zeros when unknown, because typed SDKs require the fields. The principle was written down and the field was not added when the SDK added it. Read from the turn's usage like its sibling rather than hard-coded to zero, so a backend that starts reporting cache writes needs no change here. Checked for siblings rather than assumed: required-field sets diffed across every model under openai.types.responses in both SDKs. The first diff walked only top-level exports and could not see InputTokensDetails, which lives in a submodule -- walking submodules raised it from 218 models to 390 and found six models that gained a required field. This server emits one of them. The four *Item variants belong to a stored-items endpoint it does not serve, and the computer and shell tool outputs are capabilities it does not implement. The same run failed three more tests on a missing Pillow. It is in the ocr extra, which no CI lane installs, and the three are not OCR tests -- they gate on nothing because they exercise the refusal paths, an unreadable image naming the remedy and a decompression bomb refused before it allocates. So the three that most deserve to run in CI were the three that could not. Pillow is declared in the dev extra now so they run rather than skip; importorskip would have made the lane green by never testing a bomb refusal on any machine but a developer's. openai stays uncapped, and the comment says why. The unpinned range is what surfaced a wire this server had genuinely stopped conforming to; a cap would have preserved a green suite over a payload no current SDK accepts. The older note claiming a lockfile qualified the snapshot was never true of CI. Mutation: removing cache_write_tokens reproduces exactly those five failures, restoring it gives 42 passed. Verified on both SDKs -- 42 on 3.3.1 under 3.10 and 42 on 2.8.1 under 3.11 -- so following the newer type did not break the older one. Full suite on the CI-matching 3.10 environment: 2671 passed, 35 skipped. ISSUES.md records one finding this commit deliberately does not fix: five more tests import a package no lane declares, starlette among them by way of fastapi, which is the same shape as httpx. None fails today. That is a tranche, not a carry-over, and mixing it into a commit about the red would obscure both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
`.coverage` was ignored and `coverage.xml` was not. CI's test step passes `--cov-report=xml`, so reproducing a CI failure locally runs the same command and leaves a generated file sitting in the working tree looking like work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
CI's 3.10 and 3.11 jobs both fail, and the cause is not the interpreter or the
dependency set: a CI-matching environment passes 2671 tests here in parallel,
serially, and serially with coverage against a schema built by migrate.sh.
Reading the job log finally gave the answer -- 51 failures, 31 of them one
line:
PermissionError: [Errno 13] Permission denied: '/proc/self/setgroups'
That is the sandbox working. There is no unconfined fallback by design, so a
kernel that refuses the namespace means model-written code does not run. What
the message does not say is which policy refused, and the coverage data from
the runner narrows it usefully: lines 115-117 of confine.py are unexecuted
while 118 is not, so _linux_available() returned True there -- both sysctls
present and permitting -- and unshare itself succeeded. The refusal is one
line later, inside a namespace the kernel had just granted.
So two things.
The three /proc writes that establish the identity mapping now raise with the
operation and errno, the way unshare, mount, pivot_root and umount2 already
did. They were the only calls in the sequence that surfaced as a bare
PermissionError naming a file and not an operation, and the difference between
"this host has user namespaces switched off" and "this host allowed the
namespace and then refused the mapping inside it" is the difference between
two different fixes. This is an operator-facing improvement independent of CI.
And a temporary diagnostic job reports the runner's kernel, distribution,
userns sysctls, apparmor status, whether unshare(1) works, each call of the
confinement sequence with its own errno, and what _linux_available() and
backend_name() conclude. Its own job rather than a step in `test`, because
`test` prints 2700 verbose lines and then the entire Postgres service-container
log, which makes anything before the tests unreadable in practice -- that is
why this took a full log download and a grep to diagnose at all.
continue-on-error, because a report that fails the build is a report nobody
reads. Delete it once it has answered.
Deciding from the answer rather than now: if this is a runner knob, enable it
explicitly and keep the 31 tests exercising real confinement. If GitHub-hosted
runners prohibit the primitive outright, the boundary moves to a required
confinement-capable lane rather than becoming 31 permanent skips. The sandbox
does not get weakened so that a hostile host policy stops complaining.
Not addressed here, and separate from confinement: ripgrep is absent on the
runner so two settings tests error, two more fail starting a second Postgres,
and seven workflow retry tests report zero retries. Three of those four files
predate this branch, so CI has never run them -- it has never reached them
before this week.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
The diagnostic answered. GitHub's hosted runner is Ubuntu 24.04 with
kernel.apparmor_restrict_unprivileged_userns=1, which restricts unprivileged
user namespaces: the namespace is still created, the process holds no
capabilities inside it, and the identity mapping is refused. That is exactly
what the sandbox saw -- unshare succeeding and the next line returning EACCES
-- and it fails 31 tests here because there is no unconfined fallback by
design.
kernel.unprivileged_userns_clone = 1 the probe read this
user.max_user_namespaces = 63838 and this
kernel.apparmor_restrict_unprivileged_userns = 1 it did not read this
Two defects, and only the second is about CI.
_linux_available() was wrong on a mainstream distribution. Every knob it
consulted said yes while the one that decides said no, so on a stock Noble host
backend_name() advertised an interpreter that fails on every call. It reads the
AppArmor knob now. Pessimistic on purpose: an AppArmor profile carrying
`userns create` lifts the restriction for the programs it covers, and a wrong
False withholds a working interpreter while a wrong True offers a broken one.
confine() stays the authoritative answer either way.
The runner enables the primitive explicitly. Not `|| true`: if that stops
working the lane fails at that step, next to the comment explaining it, rather
than quietly downgrading to skipping the confinement suite.
Which is the part worth stating plainly. Fixing the probe alone would have made
CI green and meant nothing -- requires_backend skips this file when
backend_name() is None, so a correct probe on a restricted runner turns 31
failing confinement tests into 31 passing skips and reports success with the
security boundary completely untested. So the lane declares
LIMINALLM_REQUIRE_CONFINEMENT and a test fails loudly when no backend exists.
It runs code inside the sandbox rather than reading a sysctl, because what
needs proving is that the boundary engages, not that a knob looks encouraging.
Mutation against the runner's real setting: with the knob reading 1,
_linux_available() returns False and backend_name() returns None; the armed
probe then fails naming kernel.apparmor_restrict_unprivileged_userns, and
unarmed the same suite skips 18 quietly, which is correct on a laptop.
The temporary diagnostic job is deleted, having answered. Bugbot was right
about it: `unshare ...; echo rc=$?` under bash -e aborted the step before the
echo, so the two steps that mattered most never ran -- skipped by the very
failure mode the job existed to distinguish. The answer arrived from unshare's
stderr instead, which is luck rather than design. A probe whose failure is the
datum must not be written so that failing suppresses the report. That is also
why the replacement is a test and not a shell step.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Forty-six of the fifty-one failures are the confinement cause fixed in the previous commit. That is measured, not assumed: pointing the availability probe at a knob reading 1 -- the runner's setting -- reproduces the CI run file by file, 7 lease failures against 7, 7 retry against 7, and single failures in child_wire, injection_taint, path_races, tool_authority, web and workflow_rag_scope, each matching. Of the five that remain, three are fixed here and one is deliberately left alone. There was never a retry bug. The seven test_workflow_retry_timeout failures read `assert 0 == 3`, but the log shows three attempts, backoff of 10ms then 40ms, and workflow_node_retries_exhausted -- SPEC 18.3 behaving exactly as written. The counter lives in a closure in the parent and _run_builtin_body never ran there, because every attempt returned 'error': 'worker_unconfined' from the same /proc/self/setgroups refusal. The assertion reported a true fact about a cause three layers below its subject. Breaking confinement locally reproduces all four assertions in CI's order. Nothing in the retry path changed; fixing it would have meant editing correct code to satisfy a symptom. ripgrep is a binary no lane installs, so two settings sweeps raised FileNotFoundError on the runner and passed on every machine that happened to have it -- the httpx shape again, this time not a Python package at all, which is why no dependency guard could catch it. Replaced with a pathlib walk and re, not with grep, which would only move the problem: its regex dialect is not the one these patterns are written in and it is still an external process. The walk's output is byte-identical to ripgrep's on both patterns. One of them legitimately matches nothing, which is the shape that goes vacuous unnoticed, so both are mutation-tested against a planted violation. A scratch Postgres reached outside its scratch directory. Debian and Ubuntu compile unix_socket_directories as /var/run/postgresql, owned by postgres -- writable when the harness runs as root and su's to that user, and not writable by an ordinary CI user, which is why the cluster would not start there. The socket now goes in the data directory. The second half matters more: _run sent both streams to DEVNULL and pg_ctl only prints "could not start server. Examine the log output", so the reason existed the whole time, in a file, and the harness discarded it. It now raises with the command, exit status, both streams and the tail of the server log. Third instrumentation gap of this shape in two days. Measured as an unprivileged user: with the socket fix reverted the cluster still fails, and the new message states the permission error outright. One failure is left and is not reproduced. test_generation_lifecycle::test_a_source_rooted_above_the_file_still_serializes is a real two-thread race, and it does not fail here normally, with confinement broken, or pinned to one or two CPUs. Its time.sleep(1.0) is the obvious thing to harden and is not the proximate cause -- CI failed the later assertion, so that gate held. Editing a race test's synchronisation without being able to reproduce its failure is how a test starts passing vacuously. The next run has 46 fewer failures and workers that start, which changes its timing; a second failure would be worth acting on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Fixing confinement took CI from 51 failures to 6 with no mention of setgroups
or worker_unconfined anywhere in the log. Four of the six were the previous
commit. The fifth changed its story, and the sixth is still not reproduced.
test_injection_findings_reach_the_workflow_trace used to fail as
worker_unconfined and now failed with the model answering without reading the
page, because "Egress address '127.0.0.1' is not allowlisted for tools".
Two guards stand between a tool and a local address. web_fetch_allow_private
is the SSRF check on the URL, and the test opts out of it explicitly. The tool
network allowlist is a separate socket-level guard, built once from settings in
the engine's constructor, so patching settings afterwards never reaches it. The
test never opted out of that one.
It passed anyway, everywhere, for a reason worth recording:
connection_allowlist() returns the proxy's host when a proxy is configured, and
this development environment sets HTTPS_PROXY to a loopback address. So the
allowlist was literally ['127.0.0.1'] and the loopback server the test stands
up was permitted by coincidence of the developer's proxy settings. CI has no
proxy, so the real target list applied. Reproduced by unsetting HTTPS_PROXY:
the test fails locally with CI's exact message and passes with it set. The rig
opts out of both guards now, and dropping the allowlist entry makes it fail
again, so the opt-out is not covering a test that would pass regardless.
Fourth environment-coincidence defect in two days and the least comfortable:
httpx, numpy and ripgrep were things present here and absent there, but this
was a security control satisfied by an unrelated environment variable. A guard
whose test passes only because of the tester's proxy configuration was not
being tested.
test_a_source_rooted_above_the_file_still_serializes has failed twice on CI and
reproduces on no local configuration tried -- ordinary, with confinement
broken, without a proxy, pinned to one CPU, to two, and to one under three
competing hogs at twice the wall clock. So it is not fixed. Its assertions
could only report that the answer was wrong when the question is which commit
landed last, so the gate records each commit and the failure message carries
the sequence.
Two details of that, because the first version was useless and the second
nearly was. Labelling by thread name gave asyncio_0 for both actors, since the
test client runs each request on an executor thread rather than the one that
started it -- evidence that distinguishes nothing. The label reads the
committed chunks instead and says "neither" rather than guessing. Verified by
forcing the assertion: a passing run reads [('neither (1 chunks)', 1.1999),
('upload', 1.4146)], upload last, which is the correct outcome. Forcing it also
caught the check landing on the wrong function, since this file holds two tests
with an identical block.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
The browser lane failed once with psycopg.errors.DeadlockDetected at fixture setup, failing a test that had not started. Not the commit it appeared on: that one touched ScratchPostgres, which this lane never constructs because it sets TEST_DATABASE_URL, and a settings test the lane deselects. First sighting on a lane CI has only just become able to run. _truncate_all's docstring already named the assumption it breaks -- "this statement assumes nothing else is looking at it" -- which holds in every lane but this one. The browser lane runs a real uvicorn server in a thread against the same database with a pool of its own, so a request still in flight holds ACCESS SHARE on some tables while the wipe wants ACCESS EXCLUSIVE on all of them. Two sessions taking locks across many tables in different orders deadlock, and Postgres kills one of them. Reproduced rather than reasoned about: a reader holding one table and reaching for a second, against a TRUNCATE holding the second and reaching for the first, deadlocks every time. The probe's reader lost where CI's fixture lost, so either side can be chosen and the fixture has to survive being it. The first fix did not work, and the measurement caught it before it was committed. A plain retry against six continuously looping readers changed nothing: 51 of 60 truncates failed with and without it, identical numbers. Identical numbers are what prompted checking whether the except branch was reached at all -- it was, and DeadlockDetected was the right class. Retries land in the same steady state and stop being independent, so under saturated contention a retry is not a fix. Against the contention this lane actually produces -- one request finishing, overlapping the wipe -- it is decisive: 40 of 40 failed without the retry and 0 of 40 with it. Both numbers are in the docstring, because the boundary is the useful half. A lane that keeps a database busy while wiping it needs the server quiesced rather than a higher attempt count, and exhausting these attempts is how it will say so. Table order is deterministic now as well. That narrows the window and cannot close it, since the other session picks its own order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Fixing the socket directory moved test_worker_isolation from failing at pg_ctl
to failing at psql: exit status 3, which is ON_ERROR_STOP firing. Which
statement failed is in stderr, and apply_schema sent stdout to DEVNULL and
never captured stderr, so the answer was discarded one line before it was
needed.
That is the fifth instrumentation gap of this shape in two days, after
confine.py's /proc writes, pg_ctl's own log, the sandbox's worker_unconfined,
and the deadlock retry's counters. Consistent enough to state as a rule:
anything that runs a subprocess and checks its status has to keep what the
subprocess said, because the status is a number and the reason is text.
apply_schema now raises with the database name, the exit code and the tail of
psql's output. Measured against a database that does not exist:
applying sql/schema.sql to 'does_not_exist_db' failed (psql exit 2):
psql: error: ... FATAL: database "does_not_exist_db" does not exist
The likely cause of the exit 3 is schema.sql line 236, CREATE EXTENSION
vector. CI reaches pgvector through a service container, and a scratch cluster
is built from the host's binaries, which are stock PostgreSQL. This
development box happens to have postgresql-16-pgvector installed, so the
control file is there and the schema applies -- the fifth environment
coincidence in the same ledger.
So ScratchPostgres.available now asks whether the installation can supply the
extensions the schema creates, reading the control files beside the binaries
rather than starting a cluster to find out. A host that cannot gets a skip
naming the missing extension and saying that a pgvector service container does
not help, because it is a different server. The three call sites report that
reason rather than "needs initdb", which was true of none of them.
This is a skip and the earlier argument against skips still stands, so the
difference is worth stating. The confinement tests would have skipped a
security boundary on the lane meant to prove it. These cover the harness's own
worker isolation on a scratch cluster, the property is exercised anyway by
every xdist run that provisions per-worker databases, and a host that cannot
host the schema cannot run them at all. Saying so beats an opaque exit code.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Two things, both about making a failure say what happened.
Bugbot found a fourth availability call site. Three were updated to report
`unavailable_reason`; `_external_or_skip` still said "needs initdb and
redis-server", so a host with initdb and without pgvector -- the case the
check had just been extended to catch -- was told the one explanation that
could not apply to it. It sat behind `_External.available`, which composes two
availability checks and had a skip message of its own, and one indirection was
enough to hide it from a grep for `.available`. This repository's own rule
covers it: grep the class when you fix the instance. The class is swept now,
and the four remaining "needs redis-server" skips were checked and left --
each is gated on ScratchRedis alone, so the message is accurate.
The generation-lifecycle instrumentation now records the source path and chunk
count as well as the generation, because "neither" alone was ambiguous on its
first CI failure. That one line was already worth the change:
CI [('walk', 1.2324), ('neither', 1.4468)]
local [('neither', 1.1999), ('upload', 1.4146)]
The upload's marked commit was missing entirely from the failing run, which is
a different fact from "the walk committed last" and was not visible before.
With the path recorded, a passing run reads
[('neither', '.checksums.json', 1, 0.19), ('upload', 'report.md', 1, 1.65)]
and the unnamed commit turns out to be an unrelated file the directory walk
also covers. That explains the ordering difference: the gate arms on the walk's
*first* commit, whichever file the filesystem hands it first. Here that is
.checksums.json, so the gate holds an uncontested file and the race never
happens. On the CI runner it is report.md, so it does.
Reproduced by arming the gate on report.md instead, which is CI's observed
order: three runs, three failures, reading
[('neither', '.checksums.json', 1, 0.19), ('walk', 'report.md', 1, 1.38)]
The walk's stale generation lands last and the upload never commits at all,
while the upload returns 200 and the new bytes are on disk -- the assertion
above this one checks that and passes. So the file is updated and the index
keeps the previous generation.
That is a product finding rather than a CI one, and it is left for a decision
rather than fixed here: the subsystem is untouched by this branch, and the
test's gate needs to become deterministic in the same change, or it will go on
passing locally for the wrong reason. docs/ISSUES.md has the full evidence.
Full parallel lane: 2824 passed, 27 skipped.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
The previous commit message said docs/ISSUES.md held the full evidence for the race, and it did not yet — the fourth-call-site note had landed and the product finding had not. It does now: the gate arming on whichever file the filesystem hands the walk first, `.checksums.json` here and `report.md` on the runner, and the three-run reproduction showing the walk's stale generation landing last while the upload's commit never happens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
…pted The previous entry said the index keeps the previous generation and would answer out of bytes that are gone. Measured, it does not: the rows show one chunk for .checksums.json and none for report.md, so the walk's stale commit landed and the upload's invalidation removed it. The error came from reading the test's assertion message instead of the index. That assertion is only "UPLOAD WROTE in indexed", which fails both on stale text and on no text, and its message names the first case; the assertion that would have told them apart never runs. The defect stated correctly: replacing a file invalidates every covering context's chunks for that path, nothing re-indexes the new generation because an ordinary upload names no context, and the manifest's context association is reset -- so a context silently stops covering a file, with no error and no record. Less severe than stale answers, still a defect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
A context covers a path; the bytes at that path are replaced through the
ordinary upload endpoint, which names no context because that is what
replacing a file looks like. Afterwards the old text must not be retrievable,
the new text must be, and the file must still belong to the context.
Three tests, one invariant from three sides: coverage acquired by a directory
source, coverage acquired by naming the context on the first upload, and an
upload that names one context while another already covered the path. All
three fail here.
Sequential and API-only in its actions -- no threads, no gate, no sleep, no
reach into the engine. Concurrency was never needed to expose this. An earlier
attempt at the same invariant on another branch used two threads and a gated
commit, and it passed or failed according to the order a directory listing came
back in: it held an uncontested file on one machine and the contested one on
another, so it passed locally for a reason unrelated to its subject. This
version fails on every machine where the invariant is broken, which is the only
property that makes it worth having.
Reads go to the store because the served surface has no chunk listing. That is
observation; the actions are the API's, so the same file runs unchanged against
histories whose implementations differ.
Measured against both, and they fail differently:
main chunks frozen at the first generation, so the context answers out
of bytes the file no longer holds
#178 chunks correctly invalidated and never replaced, so the file leaves
the context entirely
Same invariant, two manifestations. The branch fixed the invalidation half and
left the re-index half unwritten, turning stale into missing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
This branch already emptied every context covering a replaced path, and was right to: a chunk claiming to be the file's contents is false the moment new bytes exist. What it could not do under the publication lock was re-read and re-embed for a set of contexts the request never chose, so it left those contexts covering a file they no longer described. The finding in ISSUES named that; this closes it. `context_source` is the single authority for what a context covers. Never `knowledge_chunk`, which is the materialisation of it — a stray row would promote itself into a relationship nobody created, and coverage would evaporate whenever a cleanup removed the index. Never the upload manifest, which records only the contexts an upload named, which is exactly how a directory source stayed invisible. The upload keeps the bounded half and records the rest: an `ingest_job` per covering context, drained after the response and on every worker poll. Between empty and refill the path is absent from those contexts, which is recoverable and, unlike a stale answer, honest. The queue takes this branch's own `service.fs.path_lock`, on the same key an upload takes, and re-reads the generation inside it — waiting for a lock is when a replacement is most likely to have happened. A worker that cannot get it stands aside without spending an attempt. Jobs are generation-checked, collapse onto one pending slot holding the newest, retry on a schedule rather than immediately, and carry a lease so a process killed mid-job returns its work instead of stranding it. Conversations' implicit indexes stay outside all of it, on both sides: §19.5 scopes an attachment to the chat that received it, so the coverage query excludes them exactly as the invalidation already did. Two tests here asserted the old end state — the path absent from the covering context — which described the code accurately and the intent inaccurately. They now assert the path is still described, and that what it says is the current generation. Fast lane: 2734 passed, 26 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
`context_id` stayed in `run_job` after the body moved into `_reindex_under_lock`, which the CI linter rejects (F841). CI runs `ruff check liminallm/ --select=E,F,W,I --ignore=E501`, which is a different selection from `make lint` — notably it also checks import order, so it is the command to run before claiming lint is clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Replacing a file's bytes changes its generation, not its coverage Closes the silent coverage loss this branch documented: emptying every covering context on replacement is correct but only half a correction, because a context that keeps its context_source row while its chunks are gone has lost the file rather than been told about it. context_source is the single authority for coverage. The upload keeps the bounded half under the publication lock and records an ingest_job per covering context for the rest; the queue takes that same lock, on the same key, and re-reads the generation inside it. Fixes test_a_source_rooted_above_the_file_still_serializes, this branch's only remaining CI failure.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c8d085b. Configure here.
The named context is left out of the enqueue loop because it is about to be ingested in the request. When that ingest fails, what is left behind is a `context_source` row saying the context covers the path and no chunks describing it — a context covering a file it says nothing about, which is the coverage loss this queue exists to prevent, arriving through the one branch that does not use it. Reintroduced by this tranche, not pre-existing: before the source row was written there was no durable claim to strand, so the manifest omission was the whole story. The failure path now records the re-read alongside emptying the stale chunks. The worker's poll is what refills it, since the request aborts and nothing it scheduled will run. Attachments are excluded exactly as they are where the source row is written — none is written for one, so nothing claims coverage. Witnessed, and the witness was checked against the unfixed code: without the enqueue it fails on the pending-job count, which is the assertion that names the defect. Fast lane: 2735 passed, 26 skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A turn can use tools that live on a remote MCP server. The headline work is
six commits; the branch carries 127 more from earlier qualification
tranches — see "Also in this branch" below, which is the part to read before
deciding what merging this means.
What MCP does now
tool category, no separate loop: a discovered tool joins the same offered
list as
file_searchandweb_fetch, in the same dialect, and is dispatchedby the same round runner.
mcp>=2,<3is a runtimedependency and the wire arbiter. Nothing here names a protocol version or a
transport frame;
Client(url)negotiated2026-07-28against the SDK's ownserver with no version logic in this codebase. Streamable HTTP only — stdio
is out of scope because "connect to a server" would become "spawn the
executable this row names".
admin-owned
mcpartifact of kindmcp.server; ownership is read from theartifact row, never from a field inside
schema. The map from model-visiblename to remote tool lives on
InvocationContextand never crosses the pipeto the worker — a worker sends a name, and the parent decides what that name
means. An entry carries a URL and a taint class, and a worker that could send
either could name a host of its own and call it
local_read.dispatch run inside the same
tool_network_guardas every other tool,redirects included. A result is bounded, scanned and wrapped in the same
envelope fetched web content gets, with the rule stated in the system block
whenever such a tool is offered. A tainted turn loses every
egressserveralongside
web_fetch; alocal_readserver survives for the reasonfile_searchdoes.POST /v1/artifactsaccepts avisibility, defaulting toprivate, withshared/globalgated on theadmin role read from the authenticated token. The admin console has a form
for it and points at the config-ops flow for retirement.
turn needs tools — attachments, web, or a configured server — from
persisted state, with no network probe in the decision.
descriptionandinputSchemareach the model in the tool contract, before any call and sobefore any result has been scanned. They are bounded in size, depth and
count and scanned for injection patterns and envelope markers; a tool whose
metadata fails is dropped rather than rewritten, because neutralizing a
schema changes enum values and property names and offers the model a
contract the server does not implement. Rejection logs and does not taint.
Deleting an account removes its private artifacts and detaches the published
ones. A detached MCP server keeps its row, versions and patch history, and
goes inert — the admin attestation was what made it a capability — until an
admin publishes it again.
artifact.owner_user_idisON DELETE RESTRICT. A key cannot seevisibility, and both answers it could give on its own destroy something:
CASCADEremoves published configuration,SET NULLleaves a privateartifact and its payload behind an erased account.
delete_userdeletes,detaches, then removes the account, so nothing references it by then and the
restriction never blocks the supported path.
Review findings
Five of the six MCP commits fixed something a layer above where the previous
commit's tests stood. The witnesses were not wrong; they were at the wrong
altitude, and each one looked complete from where it sat.
kind must start with the type prefixcheck made the chosen type/kind pair impossible tocreate.
_build_agent_contextdirectly could not see that no ordinarychat ever selected the tool-agent path.
callby hand could not see that theSDK exposes
input_schema, not the wire'sinputSchema— so every remotetool had been offered to the model with an empty parameter list.
Two instrument corrections belong to the same lesson: a heartbeat tick count
over a whole turn measured nothing, because the count reached any threshold
from the parts that were never blocked, and a length assertion that read the
module's own constant moved with the mutation. Both passed against the defect
before being rewritten.
Also in this branch
127 commits of earlier qualification work, grouped:
extraction limits
closed
filesystem namespace, and account erasure as one boundary
root — and the xdist lanes built on it
test-subsumption passes arbitrated by mutation
browser auth with the first Playwright lane
If the intent is to merge only the MCP work, this branch is the wrong unit and
should be split; the title describes the last six commits, not the diff.
Evidence
Local: 2,816 non-browser tests passed, 26 skipped (
make test-xdist), and11 browser tests passed (
make test-browser).No GitHub checks have run on this branch to date, which is expected rather than
a fault: the workflow triggers on
pushtomain/developand onpull_requesttargeting them, and this branch had neither event. Opening thisPR should start the 3.10/3.11/3.12 matrix plus the browser job. If no checks
appear, that is a real CI plumbing problem rather than a branch-configuration
one.
🤖 Generated with Claude Code
https://claude.ai/code/session_01DQtPsg9YSUXaStGXyUjozA
Generated by Claude Code
Note
Overview
Replaces ASD-STE100 with a repository writing-style skill (Google developer docs–aligned) and points
CLAUDE.mdat it, plus rules for which test lane to run and how to verify claims.Local and CI test harness. Makefile adds
test-fast/test-xdist/test-browser, excludes the Playwright lane from other targets, and makesqause the parallel lane. Ruff now followspyproject.tomlinstead of replacing ignore lists. GitHub Actions applies schema viascripts/migrate.sh, requires real user-namespace confinement (LIMINALLM_REQUIRE_CONFINEMENT, sysctl on the runner), and adds a dedicated Chromium/Playwright job.Operator and product docs.
SHARED_FS_ROOTis documented as an environment-only path (needed before DB settings exist). README/INSTALL clarify embedding-width immutability, adapter promotion vs disk files, the OpenAI-compatible Responses API, MCP search tools, API keys, and the local JAX decoder/KV-cache path.Reviewed by Cursor Bugbot for commit df82b56. Bugbot is set up for automated code reviews on this repo. Configure here.