Repair-free app.NewClient: per-session MCP clients and read-only CLI stop write-bursting the live store#150
Merged
Conversation
MCP hosts spawn one serve --mcp-stdio process per Claude session, and PR #149's runServeMCPClient opened the store via app.New — which runs the full startup repair-sweep suite (RepairLegacyArtifacts, RepairContentlessRecency, RepairTapbacks, RepairEmptyStubMessages, and the WhatsApp media-placeholder repair) on every open. With dozens of sessions open (25 observed 2026-07-21), the daemon's live messages.db absorbed that many concurrent repair-sweep write bursts at every session start. The adversarial review of #149 flagged this as a follow-up. app.New now delegates to newApp(logger, true) and keeps its behavior byte-for-byte (sweeps then demo seed, same error handling); the sweep block moves verbatim into repairStartupArtifacts. app.NewClient is the repair-free twin: same data-dir resolution, permission hardening, and store open (schema migration included), zero repair writes. Used by: - runServeMCPClient (the per-session MCP client shape) - openCommandReadSource's legacy branch, so the advertised read-only CLI commands (read, status) stop writing to the shared live store Tests: - TestNewClientPerformsNoStoreWrites (internal/app) seeds one canonical trigger row per sweep, snapshots every messages/conversations row plus all other table counts over a read-only connection, and asserts NewClient leaves the store identical; a control leg then proves app.New repairs every seeded row, so the assertion cannot pass vacuously (mutation-tested: NewClient wired to run sweeps fails). - TestRunServeMCPClientDoesNotRepairStore (cmd) runs the real RunServe --mcp-stdio shape to stdin EOF and asserts the trigger row survives, with an app.New contrast leg (called directly — a full daemon RunServe keeps goroutines holding the store briefly past return, racing the verification open). - TestOpenCommandReadSourceLegacyDoesNotRepairStore (cmd) pins the read-only CLI contract. GOWORK=off go test ./cmd ./internal/app ./internal/tools -count=1 green; new tests also green under -race -count=3. Co-Authored-By: Claude Fable 5 <[email protected]>
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
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.
Follow-up to #149 (now merged), addressing its adversarial review's non-blocking nit:
runServeMCPClientopened the store viaapp.New, which runs the full startup repair-sweep suite — and MCP hosts spawn one client process per Claude session, so the daemon's livemessages.dbabsorbed N concurrent repair-sweep write bursts at every session start (25 concurrent clients observed live on 2026-07-21).What changed
app.NewClient(internal/app/app.go): repair-free twin ofapp.New— identical data-dir resolution, permission hardening, and store open (schema migration included), but skipsRepairLegacyArtifacts,RepairContentlessRecency,RepairTapbacks,RepairEmptyStubMessages, and the WhatsApp media-placeholder repair.app.Newdelegates to the samenewAppwith sweeps on; the sweep block moved verbatim intorepairStartupArtifacts. Daemon behavior is unchanged — sweeps still run once per daemon startup.runServeMCPClientnow opens viaNewClient(the per-session MCP client shape).openCommandReadSource's legacy branch now opens viaNewClient, so the advertised read-only CLI commands (read,status) genuinely stop writing to the store they share with the running app.Tests
TestNewClientPerformsNoStoreWrites(internal/app): seeds one canonical trigger row per sweep, snapshots every messages/conversations row (all columns) plus row counts of all other non-FTS tables through a read-only connection, assertsNewClientleaves the store identical at the row level — then a control leg provesapp.Newrepairs every seeded row, so the no-writes assertion cannot pass vacuously. Mutation-tested: wiringNewClientto run sweeps fails the test.TestRunServeMCPClientDoesNotRepairStore(cmd): runs the realRunServe --mcp-stdioshape to stdin EOF and asserts the trigger row survives, with anapp.Newcontrast leg (called directly — a full daemonRunServekeeps background goroutines holding the store briefly past return, which races the verification open).TestOpenCommandReadSourceLegacyDoesNotRepairStore(cmd): pins the read-only CLI contract.GOWORK=off go test ./cmd ./internal/app ./internal/tools -count=1green on the rebase; fullgo test ./...green; the three new tests green under-race -count=3.🤖 Generated with Claude Code