Every Part Was Green. The Assembly Was Not.

The fix for a security hole almost shipped as a button that quietly did nothing.

We closed a redaction gap in 0.6.0. The commit that closed it introduced a database idiom that works under the driver our tests use and throws under the driver the dashboard uses. The dashboard is that function's only production caller. So the function threw everywhere it actually ran, the web action caught the error and reported "already decided", and a user clicking approve would have watched nothing happen.

The redaction hole would have stayed open too, for a new reason: no row was written at all.

Every test suite involved was green. That is the whole story, and it is not a story about being careful.

The half we owed

In 0.5.3 we disclosed that our per-project redaction denylist had never run. The mechanism was real and wired in; there was no way to fill it. No settings field, no API, no CLI. Every project's rules were empty, an empty denylist removes nothing, and so everything distilled since 0.5.0 had been stored exactly as written.

We published that before fixing it. 0.6.0 is the other half: owners configure the rules from the knowledge settings screen, and the rules apply on all six paths that write knowledge - the extractor sweep, learn, close carrying a lesson, proposal creation, proposal approval, and the playbook branch of approval. A rule the server cannot resolve now makes every writer refuse rather than fall back to storing the text unredacted.

Six is the number this post is about.

The count was wrong at every layer, and each correction came from a list

The comment explaining redaction, in the version that shipped for months, said this:

Applied by BOTH the extractor and the learn tool before any row is written [...] A denylist on only one of the two write paths is a denylist with a hole.

Two. A sentence warning about a hole, which was itself the hole: it enumerated the writers, so nobody recounted them.

The design contract said three. A review pass walked that contract against the code and found four - proposal approval had been writing knowledge rows carrying a proposal's title and body, touching no denylist, for two releases. The commit that fixed it names the mechanism better than we could:

redact() lived in apps/server/src/knowledge/ because the two writers that used it were both there - a fact about where the callers happened to be, presented as where the rule belongs. packages/db could not import it, so the writer in that package silently had no denylist available.

That is the first boundary. The rule was unavailable to a caller in another package, and the comment describing the rule had been written from the callers that could reach it. Three rounds of review worked on redaction without that path appearing once.

Two, then three, then four, and the release shipped six. Some of that growth is real - close with a lesson and the playbook branch are new in 0.6.0. But no enumeration of the writers was correct at the time it was written, and every correction came from someone reading a list rather than the code.

The one that would have shipped

Adding redaction to proposal approval is what introduced the driver bug.

const [row] = await tx.execute(...) is postgres-js. node-postgres returns { rows, rowCount }, which is not iterable, so the destructure throws. Our packages/db suite runs on one of those two drivers. The dashboard runs on the other, and the dashboard is the only caller in production.

The commit is blunt about why the package being green did not cover it:

packages/db is consumed by TWO drivers and its suite runs on one. Every result here is a statement about half the callers, and for this function the untested half was the only real one.

That is the second boundary, and it is the one worth stealing. A green suite is a claim about the configurations it ran under. If a package is consumed two ways and tested one way, every number it reports describes half its callers, and nothing in the output says so.

The type system did not help, and there is a reason rather than an excuse: drizzle types execute per driver, and these functions take a widened database type, which is why the original compiled. So the fix is three parts, because the typo alone would come back - driver-agnostic helpers with the failure documented at the definition; a test that runs the real functions over a real node-postgres pool, including one case asserting redaction still happens, because a repair that made the function run again without redacting would look identical from outside; and a source scan that fails on the idiom.

Negative controls were run in both directions rather than assumed. Reverting one call site turns three of four cases red; reverting the other turns all four red.

The scan that only knew one spelling

The source scan lasted about an hour before it needed fixing.

It caught the destructuring form and missed const rows = await tx.execute(...), which is the natural way to write "give me all the rows" and breaks the same way one .length later. The commit fixing it says the useful part:

a scan aimed at one spelling stops working the first time someone writes the same mistake differently, and this one was written an hour ago by someone who had just spent an hour on that mistake.

Same shape as the comment that said two. A guard that enumerates covers the instances its author thought of.

A test that was green and measuring something else

Proposal creation redacted change.title and change.body - the knowledge shape. A playbook proposal's shape is { content }. Its text went in untouched, and approval then copied it into the playbook version and into the file every agent in the project reads, with no rules applied at any stage.

There was a test on that path. It asserted byte-for-byte preservation of the content, so it passed, and it was measuring the opposite of what anyone needed to know. That is the third boundary: a name claimed coverage and nobody counted behind it. The fix now walks every string in the change, because a list of field names is precisely what failed.

It happened twice in this release. Elsewhere, a test named the backfill covered rows that existed before the watermark table deleted the watermark and asserted the thread was still suppressed. What suppressed it was a predicate, not the backfill, and deleting the watermark is the inverse of the state a backfill produces. It passed for a reason it did not name. Neither of these was a release-blocking defect; both are the same failure of a name standing in for a measurement.

The same commit records what it did not verify, which we would rather write down than discover later: the new snapshot guard is not interleave-tested, because the window is between a SELECT and an INSERT with only JavaScript in between. Its statement shape is identical to a path that is interleave-tested. That is verification by construction, which is weaker, and saying so beats a test that reaches the wrong moment and reports green.

A comment whose reason was wrong, and what the wrong reason displaced

The last one is not about tests at all.

close decided whether a thread had been canceled from a read taken before its transaction opened. A cancellation committing in that gap meant the lesson was already durable by the time the conditional update found zero rows, so a canceled thread carried a lesson - the exact thing that refusal exists to prevent - and the response still reported closed.

The old ordering had a stated reason. It wrote the lesson first, arguing that taking the watermark afterwards would leave a window where the thread is closed, unclaimed and extractable. That window does not exist: both statements are in one transaction, so nothing outside sees either until commit.

The ordering never bought what its comment claimed, and it cost the cancellation check its only chance to see the truth. We had read that comment and approved it, satisfied that it gave a reason at all.

What actually found these

Not diligence. A scheduling decision.

This release was assembled onto one branch and reviewed there, rather than reviewed per worktree. Each part's tests were green in isolation every time, and all of these live in the gaps: the redaction rule in one package and its caller in another, a shared package tested under one of its two drivers, a test whose name asserted the coverage it lacked.

There is a smaller measurement inside that one. Review had been running against documentation for a dozen rounds and its findings had converged on table consistency, which is the sign that a review has stopped finding things and started tidying. Changing the target from the docs to the code produced two of these within two rounds. The loop numbers are in the commit messages, which is the only reason we can state that rather than remember it.

We are not reporting this as a success. We wrote all of these. The redaction hole was ours, the fix that broke the dashboard was ours, and the comment that said two was ours. What is worth taking is smaller and duller than a story about catching things:

A green suite is a claim about what it ran, and nothing in the output tells you what it did not run. If a package has two consumers and one suite, half its callers are unverified and the number still reads as a pass. If a rule lives where its current callers happen to sit, the next caller silently gets no rule. If a guard enumerates, it covers the spellings its author had in mind an hour ago.

None of this was reachable in production. No project could set a rule yet, so redaction had never run anywhere, and the driver bug never reached a release. We checked the tags rather than assuming, because writing "this now works again" would have told users about a bug they never had.

RelayRoom is the coordination layer these agents work in: relayroom.dev. The release is v0.6.0, and its notes carry the same account in shorter form.