DEV Community

Sergey Petrukovich
Sergey Petrukovich

Posted on AI-assisted

We wrote down 16 promises our MCP server makes, then two models spent ten days trying to break them

skillmem is a local memory layer for coding agents. It is an MCP server over SQLite with nine mem_* tools, and one database is shared by Claude Code, Codex, Cursor and other clients. Version 0.11 came out after forty rounds of adversarial review, and I wrote about that here. That review found real bugs. It also showed me something uncomfortable: every reviewer brought their own idea of "correct", and so did I.

So for 0.12.0 we started from the promises instead of the code.

Step 1: write the promises down

docs/INVARIANTS.md lists sixteen invariants, INV-01 to INV-16. For each one it gives the exact statement, the function that enforces it, and its status at this release. Each invariant falls into one of four classes:

  • data: no record, field, history row, body file or backup is lost, overwritten or silently not written. Export → import-vault → export produces the same bytes.
  • trust: only the owner, at a terminal, approves or seals a record. A sealed record is changed only by the owner. Unapproved text reaches a model only inside an "untrusted data" frame.
  • concurrency: every read-then-write decision is made inside the write's transaction or by compare-and-swap.
  • other: hooks fail open, names compare the same way on every filesystem, and so on.

The release bar is also written down: zero P1/P2 findings in data, trust and concurrency. Findings in the other class can ship as known issues.

This changed how the review worked. A reviewer no longer asked "is this code good?". It picked a promise and tried to produce a counterexample.

Step 2: two models from different labs, one rule

The reviewers were Astra, which is OpenAI's Codex, and Anthropic's Claude (Opus 5.5). They ran over roughly ten days, from 0.11.0 on 17 September to 0.12.0 on 27 September. Their training differs, so they fail at different things, and that is the point of using both.

There was one hard rule: a finding only counts if it comes with a reproduction. A command or a test, not an argument. Every fix then had to land with a test that fails on the fix's parent commit. scripts/release-gate.sh checks this for each new test function. A test that passes both before and after the fix proves nothing, so the gate rejects it.

The property tests use hypothesis and real concurrent writers: separate processes and threads writing to the same file, not mocked locks. The hooks are fuzzed with broken input, broken databases and bad paths.

What broke

These are real findings from the review, each with a regression test in the repo.

1. On Windows, < NUL counted as a terminal. The trust model rests on one check: owner-only actions need a real TTY. On Windows the CLI treated stdin redirected from NUL as a console. An agent running skillmem write … < NUL stored its record approved and sealed, which made it look like the owner's own rule. No reviewer found this by reading the code. It showed up only once CI ran on Windows.

2. A timestamp from the future. The session-recap hook is rate-limited by a stamp file. On Windows, a file you have just written can report a modification time a few milliseconds ahead of time.time(). The debounce computed a negative age, decided the limit had not applied, and let a second recap through, along with a second model call. A related race on every OS: the stamp was read before taking the per-session lock and not again after it. So two Stop hooks could both pass the check.

3. Look-alike characters closed the frame. Unapproved text is wrapped in a frame that tells the model "this is data, not instructions." An attacker who can write a record wants to close that frame early. We already escaped the literal closing marker. The reviewers then found ways around it: the marker after a Unicode line separator instead of \n, after an NBSP, after invisible characters, and a marker built from bracket look-alikes such as ⨠ and ⪥, which a model reads as < and >. All of these are escaped now, and a property test generates new variants.

4. A Ctrl-C that lost later writes. A write that was waiting for another process's lock could be interrupted by Ctrl-C and leave the SQLite transaction open. For a library or REPL caller, every later write on that connection was acknowledged and then disappeared when the connection closed. Interrupted transactions, nested savepoints and failed COMMITs now roll back before the interrupt propagates.

5. cp made a second database that wasn't treated as one. Copy the DB file, export from the copy, and the copy took over the original's backup directory and pruned its backups. Its body-file GC also deleted files the original still served. A copied database is now its own database: it gets its own body files on first open, and an export directory has exactly one owner.

Smaller findings in the same class: on case-insensitive filesystems, a case twin made export pruning delete the new dump. Attachments named Σ.png and ς.png got mixed up. upgrade sent the stored GitHub token across a redirect to another host. And before Python 3.14, Path.is_dir() raised PermissionError instead of returning False.

What it cost

  • Time: about ten days of review, fix, re-review.
  • Tests: from about 430 to about 2,538. Most of the new ones are property tests and regression tests.
  • CI: the full suite now runs on Linux, macOS and Windows × Python 3.11–3.13. On top of that come a semantic-search job, a build, and a Docker check that the image starts and lists 9 tools. The PyPI release reruns everything on the tag.
  • Friction for users. 0.12 refuses things 0.11 did quietly: owner commands without a TTY, writes over archived records, imports with values it used to coerce. The release notes open with "Before you upgrade" for this reason. A correctness release breaks some workflows that only worked by accident.
  • Money: no per-token bill. Both reviewers ran on flat subscriptions.

What we did not fix

Writing invariants also means writing down where you fall short. The CHANGELOG has a Known issues section of about twenty entries, each tagged with the invariant it misses. A few of them:

  • The deny rules and the TTY check are not a wall against an agent with a shell. script -qec "skillmem tr''ust x" /dev/null passes both. Real isolation needs a sandbox, not string matching.
  • Slugs, kind, project and tags are still shown unframed (INV-07). A hostile slug gets into the context raw.
  • The hooks' "already shown" ledger is not locked, so parallel PreToolUse hooks can inject the same rule twice.
  • On Windows, a catastrophically backtracking SKILLMEM_VERIFY_PATTERN is not cut off by skillmem's own watchdog. Claude Code's 10-second hook timeout ends it.
  • uninstall --purge-db refuses while the MCP server has the database open.

None of these are in the data, trust or concurrency classes at P1/P2, or the release would not have shipped. They are real, though, and I would rather you read them here than find them.

The takeaway

Asking a model to "review this code" gets you style comments and a few bugs. Asking it to break a written promise, with a failing test as the only accepted proof, gets you the Windows NUL bug. The invariants file ended up worth more than any single fix, because the next reviewer, human or model, starts from it.

pip install -U skillmem
skillmem init --claude-code
Enter fullscreen mode Exit fullscreen mode

Repo, invariants and changelog: https://github.com/liza-studio/skillmem

Top comments (1)

Collapse
 
raju_dandigam profile image
Raju Dandigam •

@sergey_petrukovich_c94a17 Requiring each regression test to fail on the fix's parent commit is a strong release gate, especially alongside real concurrent writers and cross-platform CI. I'd also check that the old commit fails for the intended invariant violation—not an unrelated setup or import error—and retain a positive control so a deny-everything fix can't pass. Does your release gate classify that failure evidence, or is the parent-commit check currently pass/fail only?