feat: memlawb as a service, phase 2 (packaging, container, docs) - #9
Draft
beardthelion wants to merge 35 commits into
Draft
feat: memlawb as a service, phase 2 (packaging, container, docs)#9beardthelion wants to merge 35 commits into
beardthelion wants to merge 35 commits into
Conversation
getStore() memoizes for the life of the process, which is correct for the server and leaves tests no way to install a fault-injecting store or a second driver. setStore/resetStore open that door for tests only. The seam is production code, so the guard is that production never reaches it. A grep for callers would be an absence claim proved by grep; the test walks the real import graph from src/index.ts and src/mcp/server.ts instead, and carries a positive control that the walk reached the modules it claims to cover. Verified load-bearing: planting a reference in src/handler.ts turns that named control red and nothing else. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Entry blobs now live at a path named by their own ciphertext hash, so an overwrite never mutates a blob the visible manifest still points at. Commit order is blobs, then the manifest that publishes them, then reclaim what the new manifest no longer references. A crash before the manifest write leaves orphans no reader can see; a crash after it leaves stale extras no reader can see. Reads fall back to the old key-derived path, so entries written before this need no migration. Two consequences the sweep forced out. A corrupt manifest used to start clean on the reasoning that the blobs survived and a re-push would rebuild it; reclaiming blobs makes that destructive, so an unreadable manifest now refuses the write. And deterministic ciphertext means two entries can share one blob, so reclaim checks the live hash set before removing anything. The sweep injects a fault at every mutating call, evidences the plant landed at that index, and asserts the visible state is either the previous one or the new one and never torn. Verified red on the pre-change code at three tests, green after. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
A push may now carry, per entry key, the ciphertext hash it believes that key holds (or null for "should not exist"). The comparison runs inside the namespace lock against the manifest the write would actually mutate, before any projection, and collects every disagreeing key so one round trip tells the caller everything that moved under it. Disagreement is a 409 carrying those conflicts. The window this guards is the caller's own turn, which is why the base is per entry rather than a namespace version: a single-key write should not be refused because an unrelated key changed. A request with no base is accepted unconditionally, so existing clients are untouched, and the hashes view advertises the capability: without that a client cannot tell a server that enforces this from one that ignores an unknown field, and would report a guarantee it is not getting. DELETE reached upsert outside the catch that maps typed errors, so a conflict there would have surfaced as a 500. It now takes a base on the query, shape- checked separately since the body parser never sees a DELETE. Verified: removing the comparison turns exactly the five refusal tests red, and every pre-existing test passes unchanged. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Whether a delete removes the bytes is a property of the store, and a client cannot see which driver a deployment runs. So BlobStore declares it and the server reports it where a client already looks: the hashes view and every write response. fs and s3 erase. A store that keeps history does not, and a client that knows can refuse a scan mode that would let a secret land somewhere it can never be removed from. The interface change is pinned by a @ts-expect-error on a store missing the attribute: if the requirement is ever relaxed the directive goes unused and the type check fails. Verified by relaxing it and watching that happen. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The health route is unauthenticated, so everything it says is public. It echoed the store's description, which on a driver whose label carries a URL or an owner would hand that to anyone who asks. It now returns liveness and the service name and nothing else, pinned by an exact-equality assertion. Reachability is still worth knowing, so it moved to startup: one write, read back, compare and remove under a reserved prefix, before the socket binds. A store we cannot reach now stops the process instead of answering 200 over it. The failure detail is the error's class, never its message, because a store error commonly carries an endpoint, a bucket and an object path, and that path carries a namespace slug. The disjointness control asserts the probe prefix against the paths the builders actually produce. Asserting that the namespace validators reject it would prove the wrong property and could not fail: tenants supply namespaces, never paths. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
An operator could not tell which account was refused or why: the only output was a startup line and a catch-all that dumped the raw error. Every refusal now emits one JSON line carrying timestamp, owner, code, status and route class. The field set is an allowlist, not a denylist. On a crypto-blind server the space of things that must never reach a log is open-ended, so a denylist only catches what someone thought to forbid; five fields cannot carry any of it because there is nowhere for it to go. The type has no index signature, so the compiler enforces it too. A namespace slug is deliberately absent: it reads as opaque but is a hash of a low-entropy namespace, so it is a stable per-tenant identifier anyone can reverse by dictionary. Logging happens once, where the response leaves handleRequest, so the code recorded is the code the caller received and a future refusal branch cannot be added without being covered. The catch-all now logs the error's class rather than its message, because a store error carries an endpoint, a bucket and an object path, and that path carries a slug. resetRateLimit exists because bun shares one process across test files and a test that exhausts a bucket would otherwise refuse requests in every later suite. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Three reviewers over the source diff. What they found: resetRateLimit duplicated _reset, which already existed and was already used by the rate-limit suite. Removed mine and moved the better explanation onto the one that was there. The blobstore module header still described one blob per entry key, which is the layout this branch replaced, and entryPath's docstring read as the current way to store an entry rather than the legacy one. A reader landing on either would have written new entries the old way. A code comment cited R12, a requirement id in docs/plans, which is in .git/info/exclude. That reference resolves to nothing for anyone reading the repo, so it now states the property instead. The QuotaError and StaleBaseError mapping was duplicated across the PUT and DELETE branches. Both reviewers verified folding it into the outer catch would be behavior-preserving today; it goes in a helper instead, so a future read path that threw QuotaError cannot silently answer 413 rather than 500. Also: prev now reuses checksumsFrom rather than rebuilding it, the unreachable half of the mutated disjunct is gone, a double branch on the same condition is one guard, and the readManifest comment no longer claims the refusal is write-only when reads take the same path. One tradeoff taken deliberately. The legacy blob sweep now runs only for keys the pre-write manifest knew, since only those can have a legacy blob. On s3 the unguarded version was a network round trip per touched key on every write, forever, while holding the namespace and owner locks. What is given up is sweeping a crash-orphaned legacy blob for a key absent from the manifest, which today is only swept in the narrow case where that key happens to be written again. Verified the sweep is still covered: removing it turns both legacy tests red. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Nine reviewers went at the previous commits. Three findings were real bugs and two were my tests failing the bar I set for them. Reclaim ran inside the commit closure, so a transient store delete after the manifest was published turned a durable write into a 500 and skipped the owner usage write, leaving quota under-counting. It now runs after the write is durable and never throws: collecting garbage that is already invisible to every reader must not fail, or roll back, a write that landed. Reclaim was also driven from the keys a request touched, which cannot see the orphans that matter. A write that died before publishing left blobs no later request can name, and a delete whose collection failed removed the key from the manifest so nothing could name its hash again. Those bytes stayed forever, uncounted by quota, while the server advertised erasure: 'erases'. BlobStore gains list() and reclaim now sweeps the namespace's blob directory against the live hash set, which finds both. Manifest hashes form storage paths now, and a manifest is parsed JSON rather than validated input, so contentPath proves the digest is bare hex before building a path. A malformed percent escape threw out of handleRequest entirely, past the envelope, the security headers and the log line, falsifying the comment saying no refusal branch escapes coverage. On the tests: assertConsistent compared two maps getData fills on consecutive lines behind one guard, so its missing-blob half could not fail; it now compares against the manifest. The corrupt-manifest control read the legacy path seed() never writes, so it asserted null equals null. The sweep reused one namespace, so later indices never reached a mutating call and it covered half the sequence; it re-seeds per index and pins the count. The reclaim guard, the s3 erasure declaration, the log's owner and route defaults, and the traversal check had no coverage at all. Verified by mutation rather than by claim. Seven mutations that previously left the suite green now fail it: reclaim's live check dropped, reclaim removed, s3 erasure flipped to retains, the handler guard removed, route hardcoded, owner default changed, and the digest check removed. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Round 2 of review, against the previous commit. Six of its seven fixes held under mutation; two things did not. Making contentPath throw was right for the write path and wrong for the read one: getData called it unguarded, three lines above its own comment about skipping rather than 500ing, so a single non-digest hash took the entire namespace's read with it where before it skipped one entry. It now skips that entry, with a control proving the healthy ones still read. The store-seam walk change was inert. Following import() adds no reach today because bin/memlawb.ts's only dynamic targets are roots the walk already had, and the floor of 20 sat below the real count of 25, so neither half could fail. The count is now exact and the comment no longer claims something untrue: a dropped root turns it red. Also from that round: reclaim resolves the store inside its try so "never throws" is structural rather than nearly-true, its failure line names the namespace so an operator can act on it, and its doc records what it costs and that it is single-instance for the same reason the lock is. s3's list had no coverage at all while the same commit asserted s3's erasure, which is backwards for the driver the hosted service runs. It now has a fake client proving pagination follows the continuation token and stops. Not covered, stated rather than implied: the nsSlug field on the reclaim failure line. That path writes to stderr directly rather than through the injectable sink. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
One predicate now decides whether a base hash is well formed, and both write verbs use it. Before, only DELETE checked the shape, so the same malformed value answered 400 there and 409 on PUT: the garbage reached the manifest comparison, never matched, and reported a conflict. A caller reading "someone wrote under you" would re-read and retry the same bad value forever. An unreadable manifest gets its own error and a 503 with code manifest_unreadable, on reads as well as writes. Refusing is right, but a generic 500 tells a caller to retry what no retry can fix and leaves an operator unable to tell it from any other fault. The rate limiter now runs before the refusal branches, keyed on the caller when there is one and a shared anonymous bucket when there is not. The unknown-route and unauthorized branches sat ahead of it, and every refusal writes a log line, so an unauthenticated caller could turn a trivially cheap request into unbounded log volume on the machine holding every tenant's ciphertext. Keying everything on the shared bucket would have let that abuse throttle real accounts, which is why the key is a named rule with its own test rather than an inline expression. The startup probe carries a deadline. Neither adapter sets a socket timeout, so a hung connect left startup pending forever and the failure line this module exists to produce was never printed. Smaller: the full view carries erasure like the other two surfaces, so a client that only pulls can still see it; a key deleted and re-added in one request is reported only as accepted, since naming it in both arrays tells a client mirroring deleted to drop a file the same response stored; the probe's byte-comparison branch, s3 list pagination, and the context defaults now have tests; and both files that install a store override reset it in afterEach, because bun shares one process and a failure before the inline reset leaked a stub into every later suite. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The README's API table still advertised a `/health` field this branch removed and a PUT body without `base`, and RELEASE-0.1 still listed the store round trip as pending work on `/health` when it moved to a startup probe. The README is the published contract, so a reader building a monitor against it would key on a field the server no longer sends. Also documents what the write path gained: the optional `base` precondition and its 409, the `supports` and `erasure` fields, and the 503 a namespace answers when its index cannot be parsed. Test headers cited plan identifiers that resolve only inside docs/plans, which is in .git/info/exclude. For anyone reading this repository those pointed at nothing, so each header now states the property in plain English instead. BREAKING CHANGE: `GET /health` no longer returns `store`, and a namespace whose manifest cannot be parsed now answers 503 `manifest_unreadable` where it previously served an empty view. Without this footer release-please would cut a patch for a changed contract, because `bump-patch-for-minor-pre-major` is set. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Every other test drives one layer. This drives the stack the way a deployment does: AES-GCM in the client, HTTP on a real port, the content-addressed store on disk. It covers what layer-local tests structurally cannot, a change that is correct in memory.ts and wrong once ciphertext, the wire format and the storage layout have to agree on the same bytes. Twelve cases: the push/pull/modify/delete lifecycle, client-side delta, no plaintext or passphrase on disk under the new layout, a wrong passphrase failing to read, the shipped client (which sends no base and reads neither supports nor erasure) still round-tripping against a server that enforces preconditions, the 409 refusal over the wire with the competing write surviving decryptably, delete actually removing bytes, liveness-only health, a corrupt index answering 503 rather than looking empty, and a budgeted caller getting a retry hint and recovering. Gaps closed alongside it. Reclaim failures go through an injectable sink like refusals do, so the namespace they name is asserted rather than assumed; that field is the only thing making the line actionable. The filesystem listing's absent-directory and temp-file branches are covered, both of which reclaim hits on ordinary writes. The server's delta short-circuit had no test at all: removing it survived the whole suite, and it now fails against a store that counts writes. A rate-limited request to a memory route was logged as route 'other', because the throttle ran before the route was classified. Verified by mutation rather than by claim: a client that stops encrypting fails six e2e cases including the ciphertext-at-rest walk, and disabling the precondition, reclaim, the corrupt-index refusal, or leaking the store label through health each fail it too. Also smoke-tested against a separately spawned server process: two entries pushed and pulled through real encryption, one deleted, zero plaintext on disk. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The server refuses a write whose base disagrees with the manifest, but that was worth nothing while no client sent one. The client now tracks, per namespace, the ciphertext hash it last saw each entry hold, and sends those as the base for the keys a write touches. Which read fills that map is the whole design. `push` performs its own hashes call immediately before the PUT, so a base taken from there would be milliseconds old and would guard a window that barely exists, while the window that matters stayed open: the caller's own turn between reading an entry and writing it back. Only reads the caller asked for fill the map, and push's pre-flight read is routed around it. A namespace this client never read sends no base at all, so a first write is unconditional. Refusals are now a typed error carrying the server's status, code and details. Flattening every failure into one message string left a caller unable to tell a stale write from a bad key from a quota breach, which is exactly what an agent needs in order to recover rather than report failure. A 404 no longer always means an empty namespace. Only the server's own `empty` code does; a wrong URL or something in front of the server used to reach the caller as a successful read of nothing, which is a denial rendered as success. Both the full read and the hashes view decide this separately, so both are covered: a fix to one is not a fix to the other. The client can also ask whether a server enforces the precondition at all, since one that ignores an unknown body field accepts a stale write silently. Verified by mutation: removing the base from push or delete, letting push's pre-flight read fill the map, dropping either 404 guard, or losing the error code each fail the suite. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
An agent running memlawb usually has somewhere else to put a fact too: the host's own local session log, and a repo-shared team memory. All three fire on the same trigger, and memlawb's guide already asks for "a stable fact (a preference, a project decision, a convention, or feedback on how to work)", which is the same sentence the host's own prompting acts on. With no rule, the same fact lands in whichever system the model happened to pick that turn, and a later recall looks in the other one. The rule is on memlawb's side because it is the only side we can change: it routes by what the fact is, not by which system asked first. Durable facts that must survive across machines go here, the session log stays local, repo-shared facts stay in the repo. The guide also now asks for one namespace per codebase beneath the owner's authorized subtree. One namespace for everything puts unrelated projects in the same recall corpus, which pushes the entries a query actually wants down the ranking, and the reader is a developer running agents against several repositories. The rule is deliberately in both the guide file and the inline fallback that is served when the file cannot be read, which is exactly what makes a test for it untrustworthy: every assertion passes whether or not the file was read, so a broken path resolution would ship unnoticed. The control asserts on markers that exist only in the file, as a pair (present in the guide, absent from the fallback), so a marker leaking into the fallback fails too. Verified by pointing the resolution at a path that does not exist: the control goes red while the clause assertions stay green, which is the whole reason it is there. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Getting a first user configured means handing them a namespace, a URL, a key and a passphrase in a shape their agent accepts. The passphrase is the part that must never reach us, so the module that renders the block takes no passphrase parameter at all: the console will call this same code in the browser, and a parameter it could fill is the one way the secret ends up on the wire. A deliberate expect-error line pins that absence, proven load-bearing by adding the parameter and watching the directive itself report as unused. The namespace is pinned to the owner's subtree, which is what makes a first save succeed. The server grants an owner user:<owner> and its children and refuses everything else, so the built-in user:me default is unauthorized for every hosted user. The card also documents the per-codebase form beneath that subtree, matching the guide. URLs must be https unless the host is loopback, checked against the parsed hostname rather than a substring, so localhost.attacker.com is refused. Both directions are asserted: a validator that refused everything would pass a one-sided test. The passphrase is 26 characters over a 32-symbol alphabet, 130 bits. The alphabet size divides 256, so the byte reduction is unbiased with no rejection loop, and the test computes the bits from the exported constants rather than pinning a number a reader cannot check. No request-capture test: this module makes no network call, so an assertion that the passphrase appears in no captured request is true by construction and proves nothing about the console, which is the component that actually transmits. That half is a console-side obligation. What is provable here is structural, so that is what is asserted: the module imports nothing, checked fail-closed against any surviving import token with its own positive and negative controls. The import-graph count in the store-seam test moves 25 to 26 for this module, which is the exact-count pin doing its job. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Every server refusal reached the agent as one string wrapping the raw JSON body, so a stale write, a rejected key, a quota breach and a rate limit all read the same. A model given that can report a failure but cannot recover from one, and two enabled consumers sharing a namespace make 409 ordinary traffic rather than an edge case. Each status now renders its own text with its own next move. 401 says the key cannot work and retrying will not help. 409 names the keys that changed, what they hold now, what this write was computed against, and to re-read before saving again. 413 says to free space. 429 says not to retry, and not in a loop, which matters because the retry it would otherwise invite lands on a single machine. Anything unrecognized keeps the old generic message rather than being dressed up as something understood. The 403 text names the subtree the key can reach and deliberately does not echo the namespace that was refused: a denial is the one moment the caller is provably reaching outside its own subtree, so repeating the target would feed another owner's namespace back into the model's context. The prefix is derived as the owner root rather than the configured namespace, because the guide and the setup card both ask for one namespace per codebase, so the configured value is routinely a child and naming it would understate what the key reaches. The base a write was computed against is the one thing a refusal payload cannot carry, since the server reports only what each key holds now. The client attaches what it actually sent, and a base recorded as absent renders as nothing rather than as a base that was sent. The tools now take a structural client type so a test can drive a specific refusal without a live server: auth mode, quota caps and the rate limiter are all frozen at config import, so a single process cannot produce all five. Each denial has its own marker rather than a shared distinctness check: five generic strings wrapping five different bodies are already distinct, so a set count passes against the code this replaces. Every branch was removed once and the named test observed red, including the two controls that survived their first mutation. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The manifest is cleartext, so a wrong or unexpanded passphrase still lists keys
and still saves. That first save leaves a namespace written under two keys,
after which every later pull fails GCM authentication for the correct passphrase
as well. The damage is done by the first tool call, so the check has to happen
before any tool is served and the process has to exit rather than degrade.
Six configurations are now refused, each with its own diagnostic naming the one
thing to change: an unexpanded variable reference, a missing passphrase, a
server that never answered, a rejected service key, a namespace the key does not
own, and a passphrase that cannot decrypt what is already stored. A transport
failure and a refusal are separated deliberately, because they are debugged in
completely different places.
The misexpansion check matches any ${...} in a secret-bearing value rather than
one canonical spelling. openclaude substitutes an unset reference with its own
literal text and registers the server anyway, so memlawb receives a template as
a non-empty passphrase; matching only ${MEMLAWB_PASSPHRASE} would miss every
config that named the variable something else. That check runs before anything
is sent anywhere.
An empty namespace with a wrong passphrase starts, and that is correct: nothing
exists to authenticate against, so it is indistinguishable from a first run, and
the first save is what fixes the key. The undecryptable check therefore only
runs once the read reports entries.
Startup moved off module top level so it can be driven without spawning a
process, which meant the CLI's side-effecting import had to become a call.
Left as an import, `memlawb mcp` would exit zero having served nothing.
No diagnostic echoes the passphrase or the service key. These go to stderr,
which is what a launcher captures into a log, and a message quoting the value
that failed is the natural way to write one. That is asserted across all six
refusals with a control that all six were actually exercised, since an absence
claim holds just as well over an empty list.
The refusal alone is not the property worth having: the test asserts the
namespace is still fully readable by the correct passphrase afterwards, proven
by a mutation that refuses and corrupts, which passes the first half.
The setup card's env-key guard now reads both modules, because the keys it
checks moved here while the transport stayed behind. The import-graph count
moves 26 to 27 for this module.
Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
push reads the namespace to work out what changed, and that read already carries the version. When nothing had changed it threw the read away and asked again, so re-saving a fact that had not changed, the commonest write an agent makes, cost two round trips. The second read was also the last place in the client that treated any 404 as an empty namespace. Both other read paths were changed to accept only the server's own `empty` code, precisely because a wrong URL or something in front of the server otherwise arrives as success; this one still turned it into a completed no-op write at version 0. Reusing the first read removes the duplicate request and the second copy of the rule together. Also here, three things a reader trips over rather than defects: the block explaining why the 403 text withholds the refused namespace had drifted above a second doc comment and bound to neither function, so the security rationale read as if it described the helper below it; the startup preflight built the same unmapped-status sentence verbatim in two places, which is two chances to drift; and pull walked the entries object twice, once to decrypt and once to hash the same bodies. Every guard on this code was re-mutated afterwards and each still turns its own named test red. A refactor that keeps the suite green can still leave a proof vacuous, and the suite alone would not have said so. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Review found the guard did not guard. Three separate defects, one root: the map of what this client has seen conflated "never read this namespace", "read it and the key was present", and "read it and the key was absent". Presence of the map meant read, and a missing key meant absent, so the code could not tell ignorance from knowledge. A client that only ever wrote never armed the precondition at all. Recording a write returned early when no map existed, and only a read created one, so the second and every later write went unconditional. Client A wrote into a namespace it had never read, B overwrote the key, and A's next write silently clobbered B. That is the lost update this whole feature exists to prevent. A key whose blob had gone missing was locked out of every future write. A full read drops an entry whose body cannot be served, and drops its checksum with it, so the client saw the key as absent and asserted it must not exist. The server disagreed and refused, permanently, and re-reading could not clear it. The distinction that settles all of it: a hashes read enumerates the namespace authoritatively, a full read does not. So a hashes read may assert a key is absent, a full read may only vouch for what it actually decrypted, and a write now seeds the map so a client's own writes arm the next one. The exception is the server's own empty answer, which means no manifest exists, so nothing can be hidden and a create after it can still assert absence. Deleting a key the client knows is absent now asserts that absence rather than going unconditional. The query form has no spelling for a null base, so it routes through the body, which already carries one. Three smaller things found in the same pass. A push reported every key it sent as uploaded, including ones the server refused for an invalid key, bad base64 or size, and folded their hashes into the map, so a refusal read as success and poisoned the next write. The error path read the response body twice, so a non-JSON refusal always rendered empty. And an error message embedded the whole response body unbounded, which an MCP tool then puts into a model's context, so it is now stripped of control characters and truncated. A decrypt failure is now its own error type. A caller could not tell a wrong passphrase from a truncated response, and the MCP preflight was telling operators their passphrase was wrong whenever the network hiccuped. Every guard here was removed once and the named test observed red, including both halves of the write fold, which review found could be deleted with the whole suite still green. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The preflight treated "the read did not throw" as "the passphrase works". It can also mean there was nothing to decrypt: a full read drops any entry whose stored body is missing, so a namespace whose manifest lists entries the server can no longer serve returned nothing, raised nothing, and started with a passphrase that was wrong. The gate this file exists to be was passing on the one input it was built to catch. It now counts what actually came back. Listed entries and none served is refused with its own diagnostic, because that is server-side drift rather than a passphrase problem and telling an operator to change their passphrase is the worst possible advice there. A partial return starts and warns instead: one entry decrypting proves the key, and taking memory away over entries the key is innocent of would punish the wrong thing. Every failure that was not an HTTP refusal was reported as a wrong passphrase, so a truncated body or a dropped socket sent operators to change the one value that must not change. Following that advice after a transient failure is exactly how a namespace ends up written under two keys, which is the corruption this file was written to prevent. Only a real decrypt failure says so now. An unrecognized MEMLAWB_SCAN was cast rather than checked, so a typo left the secret scanner in no mode at all and a live credential could be encrypted and stored without a word. It is now refused, naming the three real modes. The undecryptable diagnostic names the entry that failed, which is server-chosen text landing in a launcher's log, so it is stripped of control characters first: a key carrying a newline and an escape sequence could otherwise forge a ready line in that log. Two additions. The server now advertises whether it enforces the write precondition, and a deployment that does not gets a warning rather than a refusal, since an older server is supported and memory still works against it. Nothing had ever called that check. And the success path is tested for the first time: it came up, wrote nothing to stdout before the transport connected, and the ready line reached stderr. The leak matrix now drives the two paths that interpolate server-controlled text, which were the ones it did not cover. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The server can accept a request and still refuse an entry inside it, for an invalid key, bad base64, or size. It says so in the response. The tool ignored that and reported the keys it had sent, so a refused save came back as saved "x" in ns and the model went on believing its memory had landed. That is a denial rendered as success, which is the fourth time this branch has shipped that shape and the reason the test double now has to be able to express a refusal at all: it could only ever report everything as stored, which is the hole this went through. The three tools a model calls most had no typed denial at all. recall, search and list still dumped the raw response body into the model's context, so the 403 that deliberately withholds another owner's namespace and the 429 that tells the model not to retry were missing exactly where a retry loop starts. Two texts told the model to do things it cannot. The 401 said to fix the API key in the server configuration and start it again, which is not available to a model and, unlike the rate-limit text, never fell back to telling the user. And a 403 while configured for a namespace outside the user: grammar promised a subtree the server can never grant, sending the model to retry somewhere no key can reach. Both now name a move the model actually has. The per-codebase namespace convention resolved to user:<owner>/<repo>, so the comment and fixture still describing the old form are corrected. Every branch was removed once and the named test observed red. One control was decoration until the double got honest: it only reported a refusal when the refused key was in the same push, so a mutation that took the wrong key from the list still passed. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Two reviewers found the same thing independently. The guide told the model to use user:<owner>/<repo> for a codebase while the setup card handed the operator user:<owner>/repo/<repo>. Both surfaces shipped in the same change, each tested against itself, so nothing caught it. A user pasting the card and an agent following the guide would put the same repository's memory in two different subtrees, and recall would find nothing in whichever was not used, with no error anywhere. The card now renders the form the guide documents, pinned by a test that reads the form out of the guide rather than restating it, and that refuses to guess if the guide ever documents more than one. The routing rule had the same shape of gap. Its actual tie-breaker, the question of who needs the fact, lived only in the full guide, which a model reaches only if it chooses to call the prompt. The instructions that are always in context carried the three categories and stopped there, which leaves the overlapping case exactly as ambiguous as it was before the rule existed. The tie-breaker is now in both. The CLI subcommand had no test at all, so it now has one, including that the generated passphrase is printed once and never appears in the block itself. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The only read that returned ciphertext returned all of it, so proving a single entry decrypts meant downloading everything. A namespace caps at 2000 entries and 10 MB, and the MCP server paid that on every launch to answer a question about one entry. GET ?view=entry&key=<entryKey> answers with that entry's base64 ciphertext and its checksum, byte-identical to what the full read puts under the same key, so a client decrypts it with the code it already has. It sits inside the existing authorization check like every other branch, and the key goes through the same validation as every other attacker-controlled name before it reaches a path. Two answers are deliberately distinct where it would have been easier to collapse them. A key that does not exist in a namespace that does is its own 404, separate from the code that means the namespace itself is absent, because clients now treat only that second code as empty and anything else as an error; a pairwise test asserts the same key yields different codes depending only on whether the namespace exists. And an entry the manifest names whose stored body is gone is a 503 rather than a miss. The full read skips such an entry, which is right when the caller still gets everything else, but with one entry in play a skip would say the key was never written. That is the denial rendered as success this codebase has now shipped four times. Entries written before content addressing are read through the legacy path first, so old data keeps working. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
No request this client makes had a timeout. A server that accepted the connection and then said nothing left the caller waiting forever, which for the MCP server meant a subprocess that never started and never exited, a worse outcome than any refusal it was built to produce. Every call now gives up after a bounded wait and raises its own error type, so a hang is distinguishable from a refusal and from a server that is not there. The default is sized for a full 10 MB transfer on a slow link, and the knob is there because the wait is total elapsed time rather than idle time, so a genuinely slow large transfer is cut while still making progress. Two per-namespace caches grew forever. Every memory tool takes the namespace as an argument the model supplies, so a model naming many namespaces grew both for the life of the session, and one of them holds derived key material. Both are now bounded. Eviction costs nothing but a guarantee: the next write into an evicted namespace is unconditional, which is the same path a namespace this client has never read already takes, and the write after it is armed again. A new method reads a single entry through the bounded server view. What it records is the part worth reading: exactly one key's hash, merged into what this client already knew, leaving the enumerated flag alone. Reading one entry is positive knowledge about that key and nothing about any other, so it may arm the precondition for that key and must never let a later write assert some other key's absence. Getting that wrong is what cost a drifted key every future write before this branch fixed it. A 200 whose body is not that view's shape is refused rather than handed to the decrypter, because a decrypt failure is the one error a caller is entitled to blame on the passphrase and a misrouted server must not be able to trigger it. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Startup proved the configured passphrase could decrypt stored memory by pulling every entry and counting what came back. Every agent session paid a whole namespace, up to 10 MB, before its first tool call, to learn something one entry settles. The bounded read makes a probe possible, and the reason it was not done before was the choice of which key. Take the first and a single drifted entry condemns a namespace whose others are fine; take one at random and the same configuration passes on one launch and fails on the next. So: sorted order, stopping at the first entry that decrypts, and at most five of them. Sorted for determinism, several because one drifted entry proves nothing about the key, capped because the cost has to stay bounded and a namespace whose first five entries are all unreadable is broken enough to stop for. What that gives up, stated because it is a real loss: drift after the first readable entry is never looked at. Drift the probe walks past on the way is still reported, so the warning names what was seen and says plainly that anything beyond it was not checked. A hung server now has its own diagnostic. It had been reported as unreachable, which sends an operator to check DNS and firewalls for a server that accepted their connection and simply never answered. An unexpanded service key no longer borrows the passphrase's warning about writing memory under a key nobody can reproduce. Template text in a service key gets a 401 and stores nothing, and pointing an operator at the wrong value while their real problem is one line away is its own kind of failure. The cap needed its own control. The first test only proved the probe stops early, which holds whether or not the cap exists, because a readable first entry ends it either way. Removing the cap survived until a test drove twenty unreadable entries and asserted it read five. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The card is the first thing a new user gets, so a value it accepts and the server later rejects costs them a debugging session over a field they cannot see. Two inputs could do that. A service URL carrying a username or password passed validation and landed verbatim in the pasted block, putting a credential in a file the user copies around when the block already carries the service key in its own variable. The owner and repository strings were interpolated into a namespace unchecked. A slash moves the segment the authorization rule matches on, and traversal or whitespace produces something the server refuses later with a message about the namespace rather than about the owner field that was actually wrong. Both are now checked against an allowlist mirroring the server's own grammar, with the rule each clause mirrors named in a comment beside it, since this module cannot import the real one and silent drift between the two would be worse than the duplication. Both refuse rather than sanitize. A quietly rewritten owner is a namespace the user did not ask for. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The fifth instance of the shape this branch keeps producing, and the one a model acts on hardest. A namespace whose manifest names entries whose stored bodies are gone reads back with nothing in it, exactly like a namespace nobody has written to, and recall answered both with "no memory stored yet". A model told its memory does not exist does not go looking for it, it starts again and saves over what is still there. The two are distinguishable and were not being distinguished: a namespace that was never written is at version zero, one that has lost its bodies is not. The read tools now say so, and say plainly not to treat it as a fresh start. Listing is deliberately left alone, because it reads the manifest and still names the missing keys, which is the true answer there. An unrecognized failure could also put an unbounded response body into a model's context, since the fallback rendered the error message and an HTTP error's message carries the body. It is stripped of control characters and truncated. Adds an end-to-end suite for the surfaces this phase introduced. The existing one drives the storage round trip; this drives what a deployment exposes and what only meets in a running process: a generated card parsed exactly as an agent would parse it and used unedited to save and recall, the preflight refusing a wrong passphrase and the memory surviving it, the bounded proof counted on the wire, a stale save refused as tool text and recovered from, an entry the server refuses not reported as saved, a namespace with its bodies deleted from disk, and the whole flow captured through a proxy to assert no passphrase or plaintext ever crosses it. Six mutations were run against it and each turned a named case red. A seventh survived twice and both survivals were defects in the test rather than the code: the first because the per-repo namespace was never exercised, the second because the assertion compared the card against the function the card itself calls, which passes however both move. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The contract gained a bounded single-entry read, the CLI gained the subcommand that generates a user's configuration, and the MCP server gained a startup refusal and a timeout knob. None of it was written down, so the README described a service that no longer matches the code. The entry view's three answers are spelled out because they are the part a client has to get right: no such namespace, no such key, and the manifest naming a key whose body the store cannot produce are different conditions, and collapsing them is how a caller ends up treating lost data as an empty namespace. The write precondition is documented alongside them, including that a write without one stays unconditional so an older client keeps working. Verified against a running server rather than from the source: each documented status and code is what the route actually returns. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
A background security pass on the pushed branch found the asymmetry, and it was mine: the previous commit bounded the unrecognized-failure path and left the typed ones alone. Those are the paths a server can actually steer, because it picks the status that selects them. A stale-write refusal quotes the entry keys and hashes the server named, and a quota refusal quotes the server's own error code. All of it is read off a response body and rendered into an AI agent's context. Newlines and escape sequences are the sharp part rather than length: text carrying them can forge a turn or an instruction in the conversation the tool result lands in, and a refusal is a message the model is meant to act on. Everything server-chosen now passes through the same gate, and a refusal may name at most a few keys and says how many more there were. Without that, a server answering with thousands of conflicts fills the context with one tool result while every individual key stays short. Each of the five guards was removed once and its named test observed red, including the control that the key is still named, so this bounds the text rather than dropping the detail a model needs to recover. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The package could not be used by a Node consumer at all. `exports` pointed at
TypeScript source, and Node refuses to strip types inside node_modules:
ERR_UNSUPPORTED_NODE_MODULES_TYPE_STRIPPING
Stripping types is currently unsupported for files under node_modules
That is deliberate policy with no flag to turn off, so the only fix is to emit
JavaScript. The bin was worse: `#!/usr/bin/env bun` on a machine without Bun
fails with `/usr/bin/env: 'bun': No such file or directory`, exit 127, and npm's
shim runs whatever shebang the target carries.
There is now a build producing Node output for the three client entries and the
bin, with declarations from a second config, and an exports map whose key order
matters more than it looks: types first or the TypeScript resolver hands back
JavaScript, then a `bun` condition pointing at source because Bun otherwise
prefers `.js` inside node_modules and the Bun path would silently run the build
instead of the source it is meant to run. No `require` key, since no CommonJS is
emitted and a missing key gives a clearer error than a broken one.
The guide needed inlining, and this is the part that would have shipped broken
in silence. `loadMemoryGuide` walks up from its own module path to find SKILL.md
and falls back to a short inline copy when the read fails, so a bundle at a
different depth serves the fallback and reports nothing. Proven by driving the
installed build's MCP server over stdio under Node and fetching the guide: 5671
bytes carrying markers that exist only in the file, against 1161 bytes of
fallback from a build with the inlining removed. The build also refuses if the
slot it substitutes into ever drifts.
`memlawb serve` needs Bun and now says so, instead of dying on an undefined
global. The client commands all run on Node.
Also declares zod, which shipped code has imported directly while only the MCP
SDK was declared. It resolved by hoisting, which is not a guarantee: a stricter
installer does not hoist, and the day the SDK drops it the import breaks for
consumers while every gate here stays green, because this repo's own install
still has it. A test now walks the shipped source and asserts every bare import
is declared, with a control proving the walk found the imports it checks.
Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
The suite was green while a Node consumer could not import this package at all. It has to be: everything in-repo runs from source under Bun, so no test here can see what the tarball contains, what the exports map resolves to on Node, or what shebang the bin carries. That blind spot is the whole reason the packaging was broken without anyone noticing. This packs, installs into a scratch directory, and drives the result on a PATH that deliberately has no Bun. It imports the package and both subpaths, runs the bin, checks that `serve` refuses on Node while naming Bun, and speaks MCP over stdio to the installed binary to complete a real save and a real recall against a running server. It also parses the block `memlawb setup` prints and uses it unedited, because every other reason a first save fails, an unreachable URL, a rejected key, a quota, a scan mode, is invisible to a check on the namespace string alone. Three checks exist to catch drift rather than breakage: the tarball must contain what the exports map names, derived from the manifest so it cannot fall behind it; the bin name and the environment variables the card emits must be ones the server still reads, since the consumer repos hardcode both and would otherwise fail at spawn time in another repository; and the served guide must be the real file, since a bundle at the wrong depth silently serves the short inline copy. The script is only worth what it has been shown to catch, so four failures were induced and each was caught: a tarball built without the build step, a renamed bin, and a build that inlines the wrong guide text. The first attempt at the renamed bin crashed the run instead of reporting it, which is why the name is checked before anything invokes it and a missing bin now fails cleanly. CI gains a Node job on 20 and 22. 20 is the declared engines floor and is otherwise never exercised anywhere. The release job runs the same check before publishing: `prepack` does build, verified with a dry run after deleting the output, but shipping a package no consumer can install is the one failure every in-repo gate is structurally blind to, and it cannot be taken back once a version is on the registry. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
…hat meets the floor The image pinned bun 1.1 while the package declared a floor of 1.2. Nothing failed, because an engines range is a string nobody executes; it surfaces later as a runtime feature missing on a deployment. A test now compares the two and goes red in both directions, whether the image slips behind or the floor moves ahead. Standalone binaries let a machine with neither runtime use this, and building them turned up the guide trap for the third time. `src/mcp/guide.ts` locates SKILL.md by walking up from its own module path, so every packaged form breaks it: the Node bundle was fixed last commit, and a compiled binary has no such path at all. Measured: the binary served 1153 bytes of inline fallback instead of 5633 of guide, while save and recall worked perfectly and nothing anywhere reported a problem. The compile now reuses the same inlining, verified by driving the binary over MCP stdio with neither bun nor node on PATH. The inliner moved into its own module because the binary script importing the bundle script ran the bundle as a side effect, which is its own small lesson about build scripts with top-level work. Bun cross-compiles every target from one runner, so the release builds all seven in a single job rather than a matrix, and writes checksums beside them: a downloaded binary is the one artifact a user cannot inspect before running it. Not verified here: the image build itself. The Docker daemon is not reachable from this environment, so the base bump rests on the version test and on CI rather than on a build I watched succeed. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
PLAN.md called the sync API an openclaude drop-in that works "by changing a base URL". There is no memlawb integration in openclaude, and the one that is planned goes through the MCP tools rather than that route, so the claim named a capability nobody could use. Corrected in all three places it appeared rather than softened in one. The README had no install section at all, which stopped being acceptable the moment the package started serving Node consumers and shipping binaries. It now says which of the three ways in a reader wants, and that `serve` needs Bun while every client command does not. The write precondition needed its scope stated. It reads like a property of the product; it is a property of a client that has done a read, which means the MCP server across a session. `memlawb push` builds a fresh client per invocation and has read nothing, so it sends no base and its writes are unconditional. That is by design and it is exactly the sort of thing a reader assumes the other way. The environment example was missing every auth and S3 variable the server actually reads, so anyone configuring multi-tenant or object storage from it would have found the file silently incomplete. Also notes that S3 needs Bun, since that adapter uses Bun's client and has no Node path. The setup block is labelled a public interface, because the consumer repos point back at it and the packaging test asserts the variables it emits are ones the server reads, so a rename fails here rather than at spawn time elsewhere. Verified by running every command the README documents on a clean path: the server starts and answers health, push and pull round-trip byte-identically, setup renders, and the three development commands pass. The two health-route descriptions the plan flagged were already corrected in an earlier phase. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
Claimed they need nothing installed. True for the glibc builds, verified on a stock debian:12-slim with neither Bun nor Node present. False for the musl ones: they link against libstdc++ and libgcc, so a bare Alpine refuses to load them with a relocation error until `apk add libstdc++`. Found by running one, which is the only way this was ever going to surface, since it builds and checksums identically either way. Also verified in a container rather than argued: the image now builds, reports bun 1.2.23 against the declared floor of 1.2.0, serves health, round-trips an encrypted push and pull byte-identically, and holds no plaintext on its disk (with a control proving the grep that says so can find something). The published package installs and works under Node 20, which is the engines floor this machine could not otherwise exercise. And the binary drives a real save and recall over MCP stdio from a container with no runtime, serving the real guide rather than the fallback. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
CI caught what local runs could not. The packed-tarball job failed on both Node majors with `SyntaxError: Duplicate export of 'ciphertextHash'`, while the same Bun version built a clean artifact here and in a container. The runner is x64 and this machine is arm64, and that is as far as chasing it is worth going. Code splitting was the condition. It saved one copy of the crypto module across `.` and `./crypto`, but `client/index.ts` re-exports crypto while `client/crypto.ts` is also its own entry, and that overlap let the shared chunk export the same name twice. Splitting is now off for the client entries: each carries what it needs, which costs a few KB and removes a class of failure whose appearance depended on which machine ran the build. The more useful half is that the build now refuses to emit an artifact Node cannot load. It imports every entry it produced and fails if any does not. A bundler can emit a file that is valid to it and a syntax error to Node, and shipping that is worse than failing: the tarball is produced, the checksums match, and every gate in the repository stays green while nobody can install the package. Verified by appending a duplicate export to the output and watching the build stop with that exact message. Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
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
1 participant
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.
Phase 2 of memlawb as a service: packaging, the container floor, and a documentation truth pass.
Stacked on #8, which is stacked on #7. Neither parent branch is in this repo, so this PR cannot target one and is opened against
main. It therefore shows all three phases. The 5 commits fromfeat(build): publish something Node can actually runonward are the ones to review here. Draft until the parents land, after which this rebases and comes out of draft.Why this phase existed
The package could not be used by a Node consumer at all, and nothing said so.
exportspointed at TypeScript source, and Node refuses to strip types insidenode_modules:That is deliberate policy with no flag, so the only fix is to emit JavaScript. The bin was worse:
#!/usr/bin/env bunon a machine without Bun fails with/usr/bin/env: 'bun': No such file or directory, exit 127. The whole suite was green throughout, because everything in-repo runs from source under Bun.What landed
A build producing Node output plus declarations, an exports map with a
buncondition so Bun still resolves source, standalone binaries for seven targets with checksums, and a container base that meets the engine floor it declares.A packed-tarball job is now the packaging test: it packs, installs into a scratch directory, and drives the result on a PATH with no Bun, including a real save and recall over MCP stdio through the installed binary and the block
memlawb setupprints used unedited.One bug in three costumes
src/mcp/guide.tslocates the memory protocol by walking up from its own module path and silently falls back to a short inline copy. Every packaged form breaks that path, and the symptom is always nothing: save and recall keep working.skills/at all, which does not matter today because it only runs the server.The inliner now lives in its own module and both build paths use it. It refuses rather than degrades if the slot it substitutes into ever drifts.
Verified by running, not reasoning
Running the binaries also disproved something this PR had already written down. The glibc builds do run on a stock
debian:12-slimwith nothing installed; the-muslbuilds link againstlibstdc++andlibgccand fail on bare Alpine untilapk add libstdc++. Corrected rather than quietly dropped, because it builds and checksums identically either way.Also fixed
src/mcp/server.tsimportedzodwhile only the MCP SDK was declared. It resolved by hoisting, so it worked here and would break for a consumer on a stricter installer, or the day the SDK drops it, with every gate here still green. A test now walks shipped source and asserts every bare import is declared.PLAN.mddescribed the sync API as an openclaude drop-in that works by changing a base URL. There is no such integration, so the claim named a capability nobody could use.Not verified here
The darwin and windows targets cross-compile but cannot be executed on this machine, and there is no qemu binfmt for the x64 Linux targets, so those rest on CI. The
>=20Node floor is now proven; the upper end is whatever CI runs.Post-deploy monitoring and validation
No additional runtime monitoring required: this phase changes how the package is published, not what the server does. The signals that matter are release-time rather than deploy-time, and both are now gated in CI: the packed-tarball job must pass before publish, and the release attaches binaries with a
SHA256SUMSfile. After the first release using this, the checks worth doing once are thatnpm install @gitlawb/memlawbresolves under Node on a clean machine, and that a downloaded binary's checksum matches the published file.