epic-095 iter1: mp-rewrite — KurrentDB rewrite tool (+ MicroPlumberd.Migration, Scripting)

[rafalmaciag]

Summary

Epic 095 iteration 1 — mp-rewrite, a CLI that repairs a KurrentDB store by rewriting its history into a fresh store and swapping data directories, never editing the old one. Docs: docs/epics/epic-095-kurrentdb-rewrite-tool (requirements, design, ADR, iterations/iteration-1/test-scenarios.md).

This PR also carries the MicroPlumberd.Migration library (previously only on feature/mp-index-backed-merge, merged with master, 58/59 green with the recorded-negative SPIKE-2a skipped) per the owner's base-branch rule.

What is new

• MicroPlumberd.Migration: IMigrationBuilder.Transform(Func<RawEvent, RawEvent?>), RawEvent.EventId/Created, optional MigrationApplied.Descriptors, Migration.ChecksumOverride.
• MicroPlumberd.Migration.Scripting: Jint 4.16.1 sandbox (no CLR, recursion 64, 2 s per event) hosting the Kurrent Replicator transform(original) contract plus dropStream/dropEvent/update/updateById/renameType/renameStream/log.
• MicroPlumberd.Rewrite (mp-rewrite, dotnet tool + single-file linux-x64/arm64): inspect → guards (sibling container, connected client via /metrics kurrentdb_current_incoming_grpc_calls, named volume) → scratch store from the same image → MigrationRunner copy → Verifier → swap by rename → restart → report; --dry-run, --rollback, --status, exit codes 0..5.
• MicroPlumberd.Rewrite.Tests: UT-01..08 and E2E-01..12 against real KurrentDB containers with a bind-mounted data dir (50 tests, sequential, ~7 min). CI: rewrite-e2e.yml.
• publish-nuget.yml: packs the three new packages and attaches both single-file binaries to the tag's release.

Measured, not assumed (dev-log)

• /stats proc.tcp.connections counts legacy TCP only (0 with a live gRPC $all subscription); the guard reads /metrics.
• The store writes data.new.<ts> as uid 1001 with 0755 subdirs; a non-root operator cannot delete it, so purge/ownership go through a root helper container from the same image (path double-guarded).
• KURRENTDB_MEM_DB inherited from an in-memory source would swap an empty store in; forced off.
• A Faulted linkTo projection reproduces on 26.1; E2E-01 asserts Faulted → Running with a healthy projection as control.

Open questions for the owner (progress.md)

1. int64 above 2^53 in a payload a rule touches is rounded (JS number type; Replicator has the same limit).
2. The client guard counts open gRPC calls, not connected clients; an idle client reads 0.

Ruled in iteration 1

--force-volume-copy is refused (exit 2): the named-volume swap is not implemented in this version.

🤖 Generated with Claude Code

https://claude.ai/code/session_01G7cLv6W5QG6qcWcwCHGHP1

[rafalmaciag]

[MUST] / [SHOULD] / [COULD] / [WOULD] — Moderator tagging of this summary comment.

Full ruling: docs/epics/epic-095-kurrentdb-rewrite-tool/iterations/iteration-1/moderation-log.md.
Every finding was re-derived from source at 0d02ef6; every requirements.md/design.md/adr.md
quotation in this review was checked against the document and is accurate. 20 of 22 findings hold as
filed, one holds with its remedy corrected (#2), one is downgraded (#19), and two severities are
raised (#18, plus a moderator finding M1 below). Nothing was dismissed. No Developer↔Reviewer collusion
detected — the one place both sides agreed too easily is M1.

Item	Tag	Ruling
BLOCKER 1 · pure copy writes no history	[MUST]	Holds. Lead decision 4 is literal: a pure copy is a run.
BLOCKER 2 · verification is self-consistent, exit 4 unreachable	[MUST]	Holds — remedy corrected: SourceCount == Kept + Dropped is a tautology in CopyEngine.cs:288-312 and would be a guard that can never fail. Build instead: rule-attribution for fully-dropped streams, plus M1's source anchor.
BLOCKER 3 · transform result missing Stream/EventType	[MUST]	Holds. Absent property ⇒ throw; explicitly empty string still drops (ADR 3).
MAJOR 4–8, 11, 22	[MUST]	All hold; see the line comments for the scoped remedy on #2, #5 and #7.
MAJOR 9 (--status debris)	[SHOULD]	Reporting is the iteration-1 ask; SIGINT handling is a follow-up.
MAJOR 10 (credentials)	[SHOULD]	The message half matters more than the default.
MAJOR 12 (release gating)	[SHOULD]	Add the master trigger and a concurrency: group here. Gating publish on the suite, and moving the nuget.org push after the binaries, are follow-ups — a failed arm64 publish is recoverable. But T8 must not publish until #18 lands.
MINOR #13 exit 2 for a named-volume refusal	[SHOULD]	Holds. Decision 7's binding content is that it refuses, which stands; the exit code was never reasoned about. GuardRefusal (1) is truer to the table.
MINOR #14 plan omits dry-run counts	[SHOULD]	Unclear spec, not a code defect. The code is right; design.md § Confirmation loses the clause.
MINOR #15 --no-projection-copy untested	[SHOULD]	Holds.
MINOR #16 no second-run history test	[SHOULD]	Holds — and give ScriptIdPrefix sub-second precision to eliminate the same-second id collision rather than document it.
MINOR #17 Debug preview binary	[COULD]	Holds; repo-wide preview convention, binary is correct, only unoptimised.
MINOR #18 no packaging metadata	[MUST]	Severity raised MINOR → MAJOR: T8 publishes these to nuget.org immediately after this PR and package metadata is permanent.
MINOR #19 floating KurrentDB.Client 1.*	[WOULD]	Downgraded: accurate, but not introduced by this PR — MicroPlumberd.csproj:33 and MicroPlumberd.Testing.csproj:35, both long published, carry the identical float. Repo-wide convention, owner's call.
MINOR #20 Transform XML doc omits Data == null	[COULD]	Holds.
Owner question 1 · int64 rounding	[WOULD]	The lead has reserved this. A canonicalisation-drift refusal changes the script contract, which ADR 3 fixes verbatim — owner's call, not an iteration-1 fix. Recorded, with agreement it ranks above named-volume support.
Owner question 2 · the guard counts gRPC calls	[WOULD]	Also the owner's, and the measurement behind it is sound.

Moderator finding M1 — [MUST] — the guards are evaluated once, minutes before an irreversible swap

Raised because this review files it only as a footnote under owner question 2, where it reads as part
of a question the lead has reserved. It is not: which metric the guard reads is the owner's question;
when the guard is evaluated is this iteration's bug. EvaluateGuardsAsync runs once at
RewriteCommand.cs:91 — before the unbounded confirmation prompt at :102 and before the whole copy —
while the old store stays running and writable by contract. Any event appended after the copy's $all
read passes its position is destroyed by the swap, silently, with RESULT: OK. It is invisible to
Finding #2's gate for exactly the reason #2 gives.

Required, both halves from data already in hand: (1) re-run EvaluateGuardsAsync immediately before
StopAsync(c.Id) — nothing is swapped yet, so a refusal there is free; (2) record the source's last
$all commit position before the copy (CopyEngine.ReadLastCopyablePositionAsync already computes it)
and re-read it before the swap — an advance aborts with exit 4. Half (2) is the only verification check
in reach that is independent of the copy engine's own bookkeeping, so it is the honest answer to
Finding #2, and it gives ExitCode.EngineFailure its first reachable, testable trigger.

---

.NET review — epic-095 iteration 1 (mp-rewrite)

Head 0d02ef6. Full report: docs/epics/epic-095-kurrentdb-rewrite-tool/iterations/iteration-1/review-findings.md.

3 BLOCKER · 9 MAJOR · 9 MINOR. Each is a line comment above; this is the summary plus the MINORs and what I verified.

First, the part that is not a finding: this is high-quality work. D1–D19 in dev-log.md are decisions with reasons rather than assertions; the /stats question was answered by measurement (and the measurement is right — proc.tcp.connections really is legacy-TCP-only); the fixture proves its own preconditions instead of assuming them; and three defects — D3's canonicalisation bug, the duplicated mutual-exclusion check, the README synopsis drift — were found and reported by the engineer rather than left for review. The findings below are what survived that.

Two of the three BLOCKERs are places where the suite cannot go red, which is why they were not caught. All three were confirmed by execution in a throwaway worktree, not by reading.

BLOCKERs, one line each

1. A pure copy writes no MigrationApplied record. RewriteCommand.cs:150 passes [] when there is no script and MigrationHistory.cs:67 returns early on an empty list — so the tool's headline invocation (acceptance criterion Welcome to micro-plumberd Discussions! #1) rewrites, swaps and restarts a store while leaving no record that it happened, against requirements.md § Safety ("Every run appends...") and lead decision 4. Confirmed: E2E-01 with one added assertion fails while the tool reports exit 0.
2. The verification gate cannot fail for the reason requirements.md gives it. AllOk compares the destination against copy.SourceStreams[].Kept — the copy engine's own bookkeeping, as MigrationRunner.cs:250 says outright — so "a mismatch not explained by the applied ops" is undetectable by construction, and a stream emptied by a bug is printed as an intended drop. ExitCode.EngineFailure (4) is exercised by no test; dev-log's own exit-code list skips it.
3. A transform returning an object missing Stream/EventType silently drops every event, at exit 0. JsRuleHost.cs:365-368 folds undefined into "", and "" means drop. Confirmed: return { Data: o.Data, Metadata: o.Metadata } and return [] both delete the event. Chained with Make implementing uniqueness easy #2 that is total data loss printed as RESULT: OK.

MINOR

• Add support for Snapshots #13 RewriteCommand.cs:43 — --force-volume-copy returns exit 2 (ScriptError, documented as "the script does not parse") while the same store without the flag is refused with exit 1. The lead ruled the code, not the semantics; an operator's script branching on 2 reads "your store is on a named volume" as "my script has a typo". Would GuardRefusal (1) be truer to the table?
• Feature/mp userdefined index #14 design.md § Confirmation specifies the plan prints "container, image, data path, script checksum, rule descriptors, dry-run counts". ConfirmAsync (RewriteCommand.cs:393-408) prints everything but the counts. Deliberate (they would cost a full scratch copy before the prompt) or missed? If deliberate, design.md should lose the clause.
• MicroPlumberd.Services.Identity: declarative identity seeding on readiness, never fatal (epic-083 feature-001, patch) #15 --no-projection-copy is implemented (RewriteOptions.cs:118, RewriteCommand.cs:136) and referenced by no test. E2E-01 proves the default; nothing would notice if the flag stopped being read.
• fix(projections): guard linkTo against unresolvable source events #16 No test that a second run appends a second history record — E2E-12 asserts ContainSingle. That is lead decision 4's whole purpose and reviewer-brief point 7 asks for it. The mechanism reads correctly, but note ScriptIdPrefix is second-resolution: two runs of the same script inside one second share an id and the second is skipped by the history guard. Worth a README line if not a test.
• epic-095 iter1: mp-rewrite — KurrentDB rewrite tool (+ MicroPlumberd.Migration, Scripting) #17 publish-nuget.yml:55-58 sets CONFIGURATION=Debug for -preview/-alpha/-beta tags and rewrite-binaries reuses it (:260). With DebugType=embedded in Debug (MicroPlumberd.Rewrite.csproj:10), the binary an operator scps onto a neuron from a preview release is an unoptimised Debug build. Intended?
• #18 The three new packages ship with no packaging metadata: the generated nuspec has <authors>mp-rewrite</authors> (defaulted from AssemblyName) and no <license>, <icon>, <readme> or <projectUrl> — unlike MicroPlumberd.csproj:10-15. They would appear on nuget.org authored by "mp-rewrite" and unlicensed, beside 20 packages that are not.
• #19 MicroPlumberd.Migration.csproj:14 floats KurrentDB.Client Version="1.*". Harmless while the project was repo-internal; publish-nuget.yml:193 now packs it, so the float becomes the range consumers resolve and they can land on a client version CI never built against. Worth pinning before the first publish.
• #20 IMigrationBuilder.Transform's XML doc (IMigrationBuilder.cs:60) is thorough about reference-equality and ignored fields but never says RawEvent.Data can be null — which it is for an unparseable-verbatim payload. JsRuleHost.Apply handles it (dev-log D6); a .NET caller of the new public API would meet it as an NRE mid-rewrite.
• #21 One E2E-01 run failed with RpcException Status(Internal, "...HTTP/2 error code 'HTTP_1_1_REQUIRED'...") from KurrentDBClient rather than on an assertion; the runs immediately before and after were clean. Recording it so it is not later waved off as "a known flake" without a cause.

Verified OK — guards I made fail, and what caught them

What I broke	Went red	Verdict
ScratchContainersAsync filter → a never-matching prefix	nothing — E2E-06 and E2E-08 both passed	→ Finding #4
Added "pure copy writes history" to E2E-01	E2E-01, with the tool at exit 0	→ Finding #1
transform returning {Data,Metadata} / []	probe test — both silently dropped	→ Finding #3

Verified correct as claimed, no finding:

• Preconditions are established, not assumed. AssertDanglingLinkExistsAsync and WaitUntilDanglingAsync fail loudly; E2E-01 asserts >Faulty really is Faulted and >Test really is not before claiming a repair; total.Should().BeGreaterThan(0, "zero links would satisfy 'none dangling' vacuously") anchors the negative; WaitUntilOpenCallsAsync throws rather than proceeding, so E2E-09 cannot pass against a store with nothing connected. This is the right shape and it is most of the suite.
• WaitForFaultedAsync/WaitForRunningAsync return the last status on timeout and the caller asserts on it, so a timeout is a red test naming the last state — the correct pattern (contrast the WaitUntil note below).
• Every assertion reads through the ORIGINAL container after its restart, never the scratch store — checked call by call across all 16 scenarios.
• Reference-equality change detection plus the entry snapshot taken as JS text rather than .NET text (JsRuleHost.cs:243-245) — dev-log D3's fix, and it is the right one: 1.0 survives untouched and a touched event is re-serialised from exactly what the script returned. UT-01 pins both directions.
• The per-event budget really is shared across every helper and transform (PerEventBudgetConstraint.Reset() a deliberate no-op); UT-05's three-handler case is what distinguishes it from Jint's per-entry TimeoutInterval. D4's reasoning is correct.
• No CLR reachable from the script: no AllowClr(), no modules, helpers registered as ClrFunctions.
• ReadMetric returns −1 for an absent metric and the caller refuses — with unit tests for the absent case and for a longer metric name that merely starts the same. Right call: a renamed counter fails loudly instead of silently disabling the guard.
• BuildScratchEnv replaces rather than appends the forced keys, MEM_DB=false included.
• The swap never leaves a half-swapped state: Swap reverses its first rename in a catch before rethrowing, Restore is idempotent about which rename already happened, and E2E-10 proves the post-swap restore end to end including a write through the restarted container.
• --force-volume-copy is refused before any docker call, with a real control — Without_the_flag_the_same_command_line_gets_as_far_as_docker is exactly the assertion that makes "refused before docker" falsifiable.
• Standards: no Moq, no lock, no config["key"], no Console.WriteLine as logging (the Console uses are the CLI's own report and prompt, overridable via RewriteOptions.Output), no sync-over-async, System.Text.Json throughout, sealed record DTOs, file-scoped namespaces, ArgumentNullException.ThrowIfNull, xUnit + FluentAssertions, and MigrationApplied.Descriptors correctly optional per RUNBOOK m10. Zero CS1591 in the new projects.
• Packaging verified by execution, not by reading YAML: dotnet pack --no-build on the PackAsTool project really does produce a self-contained tool package (28 DLLs under tools/net10.0/any, no <dependencies> node), both single-file RIDs build to the paths the mv and the release glob expect so fail_on_unmatched_files: true will not trip, no typo'd job output, and nothing non-MicroPlumberd.* reaches the nuget.org push.

The Spike1 claim (progress.md § Items for the reviewer) — verified, holds

Asked to check rather than accept it, and it checks out on both halves:

• ProjectionBehaviorSpikes.cs (it lives at src/MicroPlumberd.Migration.Tests/, not Integration/) has using declarations for KurrentDB.Client and MicroPlumberd.Testing only — no Migration type appears anywhere in the file, so no line of the MigrationPlan/CopyEngine payload-parse change is reachable from it.
• The proposed root cause is visible at ProjectionBehaviorSpikes.cs:91-95: WaitUntil returns silently on timeout. Spike1 uses it (:108) to wait for $et-SpikeA/$et-SpikeB to fill before creating the join projection, so under load the projection is created over partially populated $et streams and observes the interleaved-ish order instead of the type-clustered one.

Pre-existing defect in the spike harness, not in this diff — a starved fixture reports a wrong-looking finding about KurrentDB instead of "the precondition never held". Recommend a follow-up to make WaitUntil throw. Not iteration-1 scope.

Restating the two owner questions

1. int64 rounding. The engineer recommends a canonicalisation-drift refusal before this tool is pointed at a production store with int64 ids. I agree, and would rank it above named-volume support in the follow-up queue — Finding Make command bus (EventStoreDB) #3 is the same failure class (a silent, plausible-looking wrong result) and this iteration already contains two of them.
2. The connected-client guard is narrower than requirements.md's wording. The measurement is sound and the reasoning for not gating on kurrentdb_kestrel_connections is right; accept as narrowed. But note the guard is evaluated once at RewriteCommand.cs:91 and never re-checked — a client can connect during the confirmation prompt and the entire copy. A cheap re-check immediately before StopAsync(c.Id) would close the window that actually matters.

— .NET Reviewer

Tagged by Moderator

[rafalmaciag]

[MUST] — Moderator: the promotion of #21 to MAJOR #22 is correct and I uphold it. It is observed rather than theorised, the leaked directory is undeletable by a uid-1000 operator (dev-log D11 measured exactly that), and saturn's root filesystem sat at 92 % throughout the iteration — so calling it a transport flake would have hidden a real host-filling defect. Fix RewriteFixture.StartAsync in this PR. The #23 renumbering is accepted, and the Spike1 verification it carries was independently re-checked and holds (ProjectionBehaviorSpikes.cs references no Migration type; WaitUntil returns silently on timeout at :91-95) — pre-existing harness defect, [WOULD], follow-up.

---

Addendum — counts revised to 3 BLOCKER · 10 MAJOR · 8 MINOR.

While cleaning up after my own mutation runs I found the cause of the transport flake I had listed as MINOR #21, and it is a real defect rather than a flake, so it has been promoted and re-filed as MAJOR #22 (line comment on RewriteFixture.cs:150): StartAsync has no cleanup between CreateContainerAsync and return fixture, so any failure in WaitLiveAsync or SeedAsync leaks the container and its bind-mounted temp directory permanently — and the leaked directory cannot be deleted by a uid-1000 operator, exactly as dev-log D11 measured. I reproduced it and had to clear it with the tool's own root-helper incantation.

The Spike1 verification section is renumbered #23 accordingly. review-findings.md is updated.

Housekeeping: my scratch worktree is removed, the main checkout is untouched at 0d02ef6 with a clean working tree, and no mp-rewrite-* container or temp directory of mine remains on saturn.

— .NET Reviewer

Tagged by Moderator

[rafalmaciag]

Re-review — epic-095 iteration 1, head c140508

Verdict: approve to merge. No remaining [MUST]. 3 BLOCKER + 10 MAJOR + 8 MINOR all addressed; 4 new MINOR and 1 new SHOULD, none merge-blocking.

I verified the three BLOCKERs and M1 by mutation, not by reading — four mutations in a throwaway worktree, each expected to break exactly one thing. All four discriminated. Worktree removed, main checkout clean at c140508, docker and tmpfs clean.

The mutation table

Mutation	Expected red	Result
anchorAfter != anchorBefore → false && … (RewriteCommand.cs:253)	the exit-4 test	A_write_to_the_source_during_the_run_aborts_before_the_swap_with_exit_4 — "Expected report.Code to be ExitCode.EngineFailure {value: 4}, but found ExitCode.Ok {value: 0}"
RewriteVerification.Check returns [] unconditionally	the gate's unit tests	3 failed, 8 passed — and the shape is right: the three "…_is_caught" tests went red while all four "…_is_accepted / raises_nothing" controls stayed green. A mutation that killed the controls too would have meant the tests only assert "returns something"
RequiredString reverted to the old undefined → "" folding	the new UT-02 cases	4 failed — all three missing-field theory rows plus the non-string case
RewriteFixture.ScratchContainersAsync filter → never-matching prefix	the new positive anchor	The_scratch_container_query_sees_the_scratch_container_while_it_is_up — "…if this query cannot see a scratch container that certainly existed, every 'no scratch container' assertion in this file is unfalsifiable"

The last one matters most to me: at 0d02ef6 that same mutation left every scenario green. The anchor is real now.

The three BLOCKERs

#1 — fixed at the root. RewriteCommand.cs:80-90 no longer models "no script" as "no migration"; a script-less run is a run of the empty script, so it has an id, a checksum (sha256("")) and an empty-but-present descriptor list. That is the right shape — it keeps a pure copy comparable with a scripted one instead of special-casing it out of the history. E2E-01 now asserts the record, its rewrite_ id, the checksum, and — nicely — Descriptors.Should().NotBeNull().And.BeEmpty("'no rules were recorded' and 'no rules ran' must not look the same"), which is exactly the distinction that made MigrationApplied.Descriptors optional in the first place.

#2 — fixed, and my proposed remedy was correctly refused. The moderator was right that SourceCount == Kept + Dropped is a tautology in CopyEngine.cs:288-312; I filed a check that could not fail, which is the class I spent the review objecting to. RewriteVerification.cs:150-153 now carries that reasoning as a comment where the check would have gone, which is better than silently omitting it. Confirming what you asked: nothing tautological remains in RewriteVerification. Its two checks both compare independently obtained facts — RecountSourceAsync re-reads the source and deliberately re-states the engine's classification rather than sharing it (:90-96), which is the right call: sharing it would inherit the bug it exists to catch.

#3 — fixed exactly as prescribed, both directions pinned. RequiredString (JsRuleHost.cs:342-364) rejects absent and non-string; an explicit "" still drops, so ADR 3's Replicator compatibility is untouched. The theory covers {Data, Metadata}, {Stream, Data} and return [].

Your four questions

2. Is the anchor sound, and can a write slip between the re-read and the stop?

The anchor is sound in principle — ReadSourceHeadAsync reads $all backwards and skips $-streams and $-typed events, which is what stops idle $stats traffic and projection $> links from advancing it on their own and refusing every run. That reasoning is correct and it is the only version of this check that can work.

Yes, a write can still slip, and the window is larger than it needs to be. Order today (RewriteCommand.cs:230-245) is: read anchorAfter → dispose the tool's clients → WaitForOwnCallsToDrainAsync (up to 15 s) → re-run the guards → compare anchors → StopAsync. So the unprotected span is the drain plus the guard evaluation, not a round trip. It is partly covered: a client still holding a call at guard time is refused. What is missed is a client that writes and fully disconnects inside that window. Reading anchorAfter after the guard re-check instead would shrink the window to one round trip; the cost is opening one more of the tool's own gRPC calls after the guard has already passed, which is harmless because the guard has already passed. Filed as a MINOR, not a blocker — this is now a seconds-wide race where it used to be minutes-wide, and the remaining sliver inside StopContainerAsync itself (the store accepts writes until it shuts down) cannot be closed without a read-only mode KurrentDB does not have.

One more anchor caveat, filed MINOR: a user projection using emit() rather than linkTo() writes real events into a user stream, so it would advance the anchor and abort the run — correctly (those events really are not in the new store) but unhelpfully, and the operator has no override. The fleet is linkTo throughout (ADR 6) so this is not a today problem, and it is untested.

3. The purge mount — re-proved. DockerStoreLifecycle.cs:236-244: Binds = [$"{full}:/purge"] and rm -rf /purge/..?* /purge/.[!.]* /purge/*. The blast radius is now the named directory by construction: nothing outside it is visible inside the container, every glob is rooted at /purge/, ..?* requires at least three characters so it cannot match .., and no glob ever yields . or ... P — live store and every backup — is never mounted into a root container at all. The depth heuristic is gone and only "not a filesystem root" remains, which is the honest guard once the mount is the interlock. This also explains why my original remedy was wrong: RewriteFixture purges data and data.bak.* for its own cleanup, so a name check inside the method would have broken the suite — the moderator caught that and the prescribed fix is better than mine.

4. Decisions 8 and 9 — no hole in either.

Decision 8 is defensible and, more importantly, pinned as a pair: A_store_on_a_named_volume_is_refused_with_exit_1 with a positive control, alongside the existing exit-2 theory for the flag. "Unsupported argument" (2, like --script with --eval) versus "the store's state" (1) is a real distinction and it can no longer drift.

Decision 9 is not merely acceptable, it corrects me. My #5 lumped KURRENTDB_LOG in with _DB/_INDEX; the engineer pushed back with the decisive fact — KurrentDB's own default log path (/var/log/kurrentdb) is outside the data directory, so refusing on it would refuse essentially every container. Reported-not-refused is right, and _DB/_INDEX plus both EVENTSTORE_* aliases are refused with the setting named. Containment_is_by_path_SEGMENT_not_by_string_prefix pins the /var/lib/kurrentdb-index trap, which is the bug this check would otherwise have shipped with.

5. The abort — hypothesis, ~20 minutes spent.

The strongest candidate is not in the code, it is the host: /tmp on saturn is a tmpfs, so every store the suite creates lives in RAM.

tmpfs on /tmp type tmpfs (rw,nosuid,nodev,...)        TMPDIR unset → .NET's GetTempPath() is /tmp
Mem: 121 total, 64 used, 4 free   (available 56, of which 62 is buff/cache)

RewriteFixture.StartAsync bind-mounts Path.Combine(Path.GetTempPath(), ...), and each scenario holds up to three stores at once (data, data.new.<ts>, data.bak.<ts>) across ~16 container scenarios. That fits the symptom precisely: a testhost that vanishes rather than throwing (vstest then reports the completed subset as Passed!), non-deterministic at roughly 1 in 3, specific to saturn, and invisible on GitHub runners where /tmp is disk-backed and the VM is fresh. It also composes with finding #22's class — RewriteFixture.DisposeAsync wraps its purges in Safe(...), which swallows, so a failed cleanup silently leaves a whole store resident in RAM for the rest of the run.

I ruled out the alternatives you listed, cheaply: ThrowUnobservedTaskExceptions is not set anywhere, so an unobserved task exception is not fatal; both Task.Run blocks (RewriteE2ETests.cs:337, :641) are awaited, so their faults surface as test failures; there are no finalizers in either project; and the fixture does dispose its DockerClient (RewriteFixture.cs:446), including on the new #22 failure path. I could not check for an OOM kill directly — dmesg is Operation not permitted for this user and the journal is not readable — which is the one piece of evidence that would settle it.

One line falsifies it: run the suite with TMPDIR=/var/tmp (disk-backed) and see whether the abort rate changes. If it does, either keep that in the runbook or have the fixture take its root from an env var defaulting to a disk path. sudo dmesg | grep -i oom right after an abort would confirm it outright. I would not treat the abort as understood until one of those two is done — and I would not let the #11 floor be the reason it stops being investigated, since the floor makes CI honest about it without explaining it.

New findings — 1 SHOULD, 4 MINOR, none blocking

[SHOULD] RR-1 — --status's two new lines are untested; the production scratch query has zero coverage.
RewriteCommand.cs:359-367 adds interrupted : (from swap.NewStoreDirs()) and scratch : (from store.FindScratchContainersAsync). E2E11_status_… (:666-689) still asserts only backups, (none) and safe :. Delete both new lines and no test goes red. Worse, the new anchor test exercises RewriteFixture.ScratchContainersAsync — the fixture's own parallel copy — not DockerStore.FindScratchContainersAsync, so the production query and NewStoreDirs() are both unguarded. This is the same class as #4, one level over: the fix for a vacuous negative was itself added without an anchor. Two assertions in E2E-11, ideally against the production query.

[MINOR] RR-2 — the anchor is re-read before the drain and the guard re-check, widening the window it exists to close. See question 2 above. Moving anchorAfter to immediately before StopAsync shrinks it to one round trip.

[MINOR] RR-3 — an emit()-based user projection on the source would advance the anchor and abort the run. Correct but unhelpable, with no override. Not a fleet problem (ADR 6 is linkTo), untested, worth a README line.

[MINOR] RR-4 — nothing pins the purge mount. The interlock is structural now, so a test is defence-in-depth rather than the guard itself — but the one line that makes it safe (Binds = [$"{full}:/purge"]) can be changed back to parent and nothing anywhere notices. A unit assertion on the CreateContainerParameters would cost little.

[MINOR] RR-5 — the state-path check reads env vars only. CheckStatePathsInsideMount and ResolveDataLocation both consult Config.Env; a container configured with --index in its Cmd or via a mounted kurrentdb.conf bypasses both silently. The fleet uses compose env so this is a documentation gap more than a defect, but the refusal's promise ("a state path outside the mount is refused") is narrower than it reads.

Nit — rewrite-e2e.yml's MIN_TESTS=80 equals the exact current test count, so adding a Skip turns CI red. It fails safe and the comment says to raise it deliberately, so it is fine; the prose beside it still says "71 of 76 tests" and is stale.

What I checked and am satisfied with

Every other item: #5 (refusal naming the setting, segment-wise containment), #6 (--rollback now runs the guards, and <dir> is constrained to swap.Backups() by full-path match), #8 (deny-path test asserting RootEntries() and StartedAtAsync() unchanged, plus a confirm-proceeds control), #10 (401/403 separated from unreachable; --user/--password and MP_REWRITE_USER/_PASSWORD threaded to both the esdb URI and the HTTP client, with no hard-coded credential left outside the tool's own scratch container), #11 (floor distinguishes missing-TRX from executed=0, runs if: always(), and exits 1), #12, #13, #15 (with its positive control in E2E-01), #16 (millisecond prefix plus a two-record test), #17, #18 (all three packages pack cleanly — verified by running dotnet pack, with icon and readme present inside each nupkg), #20, #22 (StartAsync cleans up on failure).

Special mention for the two E2E tests behind M1: driving the race through a TextReader that writes to the store the moment the tool asks for confirmation makes the window deterministic with no fault-injection hook, so what runs is the real trigger rather than a simulation of one — and Acted.Should().BeTrue() stops the scenario passing vacuously if the write never happened. That is a better test than the one I would have asked for.

— .NET Reviewer

[rafalmaciag]

Re the re-review summary (#17 (comment)) — both items are in.

RR-1 is answered on its own thread.

Anchor window (MINOR): anchorAfter now reads last, after the drain wait and the pre-swap guard re-check, so the gap between the final look at the source and StopAsync is one round trip instead of up to ~15 s plus a guard evaluation. A client that wrote and fully disconnected inside that span used to be missed by both checks. It uses a fresh short-lived client, which is safe precisely because the guard has already run and so cannot count it against us.

No timing change to the existing exit-4 test: its write lands at the confirmation prompt, long before either position, so it is caught the same way. Re-ran it and the sibling-started-during-the-run test to confirm.

The three follow-up MINORs are untouched and stay open.

— .NET Engineer

[rafalmaciag]

Readiness defect found on the pre-merge run

The pre-merge suite run failed one scenario — E2E05_a_script_file_containing_only_the_Replicator_transform_runs_unchanged — with

Grpc.Core.RpcException Internal: "Error starting gRPC call. HttpRequestException:
The HTTP/2 server closed the connection. HTTP/2 error code 'HTTP_1_1_REQUIRED' (0xd)."

It had passed on every earlier run (50, 63, 78, 80 tests). It is not a flake, and the defect is in the tool, not only in the test fixture.

Mechanism

From the TRX, the failing call is RewriteFixture.SeedAsync's first AppendToStreamAsync, inside the gRPC channel handshake (GrpcServerCapabilitiesClient.GetAsync). The line printed immediately before it is fixture: …/health/live is live.

/health/live is served over HTTP/1.1 and says nothing about whether the gRPC endpoint is servable. Measured on a freshly started container (saturn, 2026-09-07):

signal	ready at
/health/live (HTTP/1.1)	2433 ms
first successful gRPC call	2962 ms
window	528 ms

Inside that window the server answers the HTTP/2 preface with GOAWAY HTTP_1_1_REQUIRED. Every client this tool opens is gRPC, so the health check was simply the wrong readiness signal. The client's internal retry hides it most of the time, which is why it took ~80 scenarios on a loaded host to surface.

Where the tool was exposed — measured, and not where it looks

Pointing a KurrentDBProjectionManagementClient at an HTTP/1.1-only listener fails, so GetStatusAsync is gRPC, not HTTP. That splits the tool's two waits:

• Scratch store (RewriteCommand.cs:157 → :163): WaitLiveAsync is followed by WaitProjectionRunningAsync, which polls GetStatusAsync inside a catch-and-retry loop. It was acting as an accidental gRPC readiness wait — which is why the copy never hit this.
• After the swap (:299 → :303): WaitLiveAsync is followed by FaultedProjectionsAsync, a bare ListAllAsync with no retry. Genuinely exposed — and the consequence is the most expensive one in the tool: an RpcException there is caught by the post-swap handler, which restores the backup. A perfectly good rewrite would be rolled back and reported as a post-swap failure (exit 5), because the store took half a second longer to serve gRPC than to answer a health check.

Fix

StoreHealth.WaitLiveAsync — the single place every client in the tool and the suite waits — now requires /health/live and then a real gRPC round trip (a read of a stream that cannot exist), retried against the same deadline, timing out with a message that names gRPC. A fresh client per attempt, because the client caches channel discovery and a channel poisoned inside the window would otherwise be reused for the rest of the run. Both tool paths and the fixture are covered; neither needs its own retry.

Reproduction

Deterministic, not timing-dependent. StoreHealthTests stands up an HttpListener — HTTP/1.1-only by construction — that answers /health/live with 204:

• the old wait called that ready (shown failing first: "Expected a TimeoutException … but no exception was thrown");
• the new wait times out naming gRPC;
• a control asserts an unreachable store still fails on the health check itself, so not every failure is now reported as a gRPC problem;
• mutation (skip the gRPC wait) reddens the first and leaves the control green.

CI test-count floor raised to 83.

— .NET Engineer