Skip to content

fix(contextmd): keep serving reads off the writer connection - #340

Merged
lukemaj merged 1 commit into
mainfrom
fix/contextmd-34-first-input-deadline
Oct 15, 2026
Merged

lukemaj merged 1 commit into
mainfrom
fix/contextmd-34-first-input-deadline

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What: ContextMD serving reads use the existing WAL reader so a writer transaction cannot consume the first-input pointer's deadline.
Why: Authentication and projection reads inherited the main writer connection even though this work only reads the main database.
So what: Obtain independent Sol/Grok reviews and green checks, then the dispatcher may merge; the planner's next build must be retested on rocky before claiming live acceptance.

This repairs the existing serving owner for toolboxmd/contextmd#34 and relates to #239. A deterministic isolated reproduction holds the writer transaction with Deferred latches, without sleeps, polling, or live load. Claude, Codex, and OpenCode each return 204 after the 950 ms work allowance on the baseline; the same first-input requests return 200 using the already configured reader. Both the 1,200 ms moment deadline and the 8,000-byte run budget are preserved.

The change marks only ContextmdServing.moment with the existing NodeSqliteClient.readOnly. Its supplied authorization effect and all projection queries use the reader; the separate synchronous contextmd.sqlite connection still owns claims, accounting, settlement, and logs. No encompassing SQL transaction or snapshot is added. Other service methods keep their existing connection and scope. The new tests start a cold reader, use the real auth-session repository lookup, verify exact pointer charges and same-epoch nonrepeat while the writer remains held, then commit a revocation and verify the next request returns 401.

Verified live facts from build 9f46c833b1: rocky m28142/43, m28145/46, m28152 and m28153 are deadline cuts with no candidates or pointer charges; m28155 succeeds at 791 ms with a 236-byte pointer. Cavallo m585/588/593 completes in 9/8/57 ms. Both hosts have identical tiny registered stores and no session-start files. The queueing mechanism is reproduced, and current source/load supports it, but the exact 23:44 queue stage is unknown because the relevant traces rotated away. This PR does not claim those historical stages were directly timed or the live cells repaired. OpenCode's Zen versus Go selection is a separate diagnosed cause and has no source/config change here.

Validation on f6a8872d1bd13c1caaf169fb6c23c92602fdfaa1:

  • Red: vp test run apps/server/src/contextmd/ContextmdServing.test.ts -t 'serves the first' fails all three cases with expected 200 / actual 204 when the routing marker is absent.
  • Green: the same three cases pass with the routing marker.
  • vp test run apps/server/src/contextmd/ContextmdServing.test.ts apps/server/src/contextmd/ContextmdRoute.test.ts apps/server/src/contextmd/GreatLibrary.test.ts packages/shared/src/nodeSqliteClient.test.ts: 142 pass, 0 fail, 0 skip.
  • tsc-rs --noEmit -p apps/server/tsconfig.json: exit 0.
  • Targeted vp lint, formatting check, and scripts/fork-check.sh --base 8d11fe1261f0dfd74f58a1b1ae8ad7e400905fcb: exit 0.

Source/test logs remain outside the repository. No live database writes, credentials, browser use, installation, service restart, or deployment. No package version change applies to this fork. Website impact: none, because this is an existing backend serving repair. Ships contextmd#34; no Chromeria map part exists for this serving owner.

Upstream first: inspected pingdotgg/t3code main 742c229957b69a0cc5a1379dff6c66b595854057, which has no apps/server/src/contextmd owner, and searched its Issues and PRs for ContextMD and serving timeout fixes. No matching owner/fix to port; broad timeout matches concern other services. #239 has no assignee or open matching PR, with its last prior update on 2026-10-13. No competing writer was found.

Requirements and who asked: The user and #34 dispatcher require causal diagnosis, a failing-first existing-owner fix, unchanged serving/byte limits, and a frozen PR for independent exact-head Sol/Grok review. This restores stored-knowledge discovery supporting Promachos.
Deleted: Limit enlargement, new connections/services/timers, provider edits, Library/persona/preferences edits, live load, installation, and deployment.
Bottleneck: Read-only work waits for the shared writer lock; the existing WAL reader already provides isolation from that lock.
Checked myself: Exact running artifact digests and read-only serving rows on both hosts; identical store sizes and session-start configuration; current serving/auth/SQL routing and all production composition paths; cold-reader red/green tests, revocation/claim/accounting assertions, 142 focused tests, server typecheck, targeted lint/format, and the fork gate.

Detailed serving diagnosis, production composition audit, raw-log identities and exact handoff: toolboxmd/contextmd#34 (comment)

Implemented with gpt-6.1-sol in the Codex harness.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Sol review found no blocking findings on f6a8872d1bd13c1caaf169fb6c23c92602fdfaa1.
Why: The moment's reachable main-database work is read-only, and the existing WAL reader removes its reproduced dependency on the writer while preserving authorization and the separate serving-state owner.
So what: The parent can use this exact-head Sol verdict for admission; merge still requires the separate Grok verdict and green CI, and installed/live acceptance remains pending.

Verdict and identity

NO BLOCKING FINDINGS. No supported actionable defect or required source correction in the inspected scope. Reviewed independently by codex / gpt-6.1-sol, high reasoning effort; I did not author this PR. This is a code review, not a repeated design panel or merge authorization.

  • Base: 9ab53882e87a9e0a6413f74d8b313ce516721d40.
  • Head: f6a8872d1bd13c1caaf169fb6c23c92602fdfaa1.
  • Read the complete two-file Git diff and forge diff, current PR body, complete contextmd#34 body/comments, serving handoff 6181320605, and separate OpenCode diagnosis 6181265773.
  • Confirmed the frozen author worktree is clean at that head and the forge reports the same base/head. No source edits, checkout changes, tests in the author workspace, merge, installation, deployment, restart, credential changes, browser use or live DB access by this reviewer.

Inspected scope and cleared concerns

  1. Dynamic routing and write ownership: ContextmdServing.moment at lines 908-1097 includes authorization, bounded body read, identify/config/Library selection, run/thread joins, pending deny/read checks, Cue transcript reads, and the final decision. Its main SqlClient statements are SELECTs. Select, Store and Cue transport introduce no SqlClient writes. Claims, budgets, logs, stale release, deny-check results, Cue availability and settlement use the independently opened synchronous contextmd.sqlite connection. Native-fork recording, Grok instruction installation/attachment, startup indexes and background sweeps are outside the new marker.
  2. Real authorization: traced ContextmdRoute to EnvironmentAuth.authenticateHttpRequest, SessionStore.verify, and AuthSessionRepository.getById. Signature/expiry/scope/revocation validation remains at its existing owner. Both ordinary and reusable-dev credentials read the real repository. DPoP replay recording, when applicable, writes through the filesystem-backed secret store, not SqlClient, so the marker does not redirect it. There is no added enclosing SQL transaction or frozen authentication snapshot; subsequent requests read committed revocations. As before, a revocation concurrent with an already completed authorization check is not a new atomic check-at-delivery guarantee.
  3. Pinned Effect semantics: inspected installed effect 4.0.4 SqlClient, Statement and context implementation against the lockfile and patch configuration, plus the complete existing NodeSqliteClient routing/worker implementation. Acquisition evaluates the current fiber context; an existing transaction connection takes precedence over the marker. provideService restores the previous context on exit, including failure/interruption. This neither changes the ambient client nor marks another service call. The moment adds no transaction or fork that exports the marker to persistent SQL work.
  4. Production composition and cold/failure paths: CLI/development/desktop backend entry points lead to the shared server composition, whose auth/runtime/HTTP layers receive persistence configured through layerConfig and layerFromPath with readerWorker: true and WAL setup. In-memory fixtures deliberately use the writer. The existing worker starts lazily and opens read-only; unavailable/non-WAL/startup failure fallback remains the writer. Startup waiting is interruptible. An unavailable or overloaded reader can still exhaust the unchanged moment allowance; this patch does not promise delivery under every load or fallback condition.
  5. Other harness/events and exceptions: the marker changes acquisition, not parsing, selection, epoch rules or response formatting. Reviewed reachable Grok/Cursor/Antigravity and non-first-input paths, Cue/window and deny/read behavior, replay/off/oversize/bad-event/config paths, timeout logging and outer failure recovery. Grok native instruction methods keep their existing scope. No new wire contract, byte limit, timer, service or connection is introduced.

Regression oracle and inspected proof

The new three cases use a real disk/WAL persistence layer, a cold reader, the real serving method and real auth-session repository. The writer latch is signalled only inside withTransaction, after acquisition and BEGIN, and held through first delivery, exact UTF-8 charge assertions, settlement and same-epoch suppression. This would deadlock or return 204 on the writer path. The test then releases/joins the writer, commits a repository revocation and requires 401. forkScoped plus SQL transaction finalizers cancel/roll back and release resources when an assertion fails; there is no sleep/polling oracle. The fixture bypasses token issuance/signature verification, which I checked in source separately; it is not a full HTTP/native-client acceptance test.

Read the raw red and green logs: all three baseline cases fail expected 200 / actual 204, while all three fixed cases pass. Their SHA-256 values match dispatch exactly:

  • Red: ec0494dce65f0204f3dbc90b30d4150e7eeee83238ab12e1148464dcc9b3ff40.
  • Green: 198e3a821a9705b927eec2ea31db660db8fdc3c16576fb670aa7803b2626ee7c.

Inspected and hash-verified the final focused log: 142 passed, no failures/skips, covering ContextmdServing.test.ts, ContextmdRoute.test.ts, GreatLibrary.test.ts, and nodeSqliteClient.test.ts. Also inspected the lint/fork logs and classified all 230 typecheck lines as suggestions, with no error lines. Fork log ends fork-features: OK (38 features) / fork-check: OK. Existing SQLite tests cover read/write transaction precedence, snapshot freshness after commit, reader write rejection, worker failure/fallback, startup cancellation and late-ready retirement. I reused these author-run records after inspection; I did not independently rerun tests or claim a complete local gate. Formatting exit success is recorded in the handoff, without a separate raw format log inspected here.

Evidence identity clarification, cleared: the dispatch manifest digest 6b6b1764b02d57634ad546c790dcb810179a122676ff9acb268e8068ac8ac58c matches retained MANIFEST-before-pr-evidence-link.json. Current MANIFEST.json is cd20b645b53310fa73bacc2e415ec955cc1a4f9c3914a467197d9413b82fc9c3; all 13 current artifact hashes match. The pre-link manifest's only current mismatch is PR.md, whose evidence link changed; all proof log hashes remain identical. This is not a source or proof blocker.

Proof limits

The archived sanitized live rows are consistent with deadline cuts before completed selection and a later successful pointer. They do not identify the historic 23:44 queue stage or lock holder. Current load is not retroactive timing proof. The independent Zen/Go provider cause remains outside this patch. No deployed repair, fresh native consumption, natural compaction, twelve-cell fleet acceptance, or resolution of chromeria#239 is claimed. At the last forge inspection, CI still had Test/Test Server 6 running; the parent owns fresh final admission.

Requirements and who asked: User and contextmd#34 dispatcher require an independent exact-head Sol review of the existing first-input repair, with auth/scope/cold-reader/test-oracle scrutiny and an ordered findings comment then review status; reliable knowledge discovery supports Promachos.
Deleted: Source changes, merge, duplicate design review, unnecessary test reruns, limit changes, new infrastructure, historical timing overclaims and installed/live acceptance claims.
Bottleneck: Read-only moments unnecessarily acquire the main writer; the existing reader removes that reproduced dependency. Separate Grok review and CI remain admission owners.
Checked myself: Full exact diff and reachable consumers, production composition, pinned SQL/fiber semantics, actual test oracle/finalizers, raw red/green/focused/type/lint/fork records and artifact hashes, archived sanitized rows, exact clean author head and live PR identity.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 4.9 KiB 5.0 KiB +23 B (+0.5%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB +23 B (+1.9%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.8 KiB 20.9 KiB +41 B (+0.2%) 29.3 KiB ✅
Codex Live turn messages 1 2 +1 (+100.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB −24 B (−0.5%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB −24 B (−2.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB −41 B (−0.2%) 29.3 KiB ✅
Claude Live turn messages 2 1 −1 (−50.0%) 8 ✅

Baseline: 9ab5388 · PR result: f6a8872 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

What: Independent Grok review of f6a8872d1bd13c1caaf169fb6c23c92602fdfaa1 finds no blocking defect. ContextmdServing.moment reads the main database through the existing WAL reader, and claims, budgets, and logs stay on contextmd.sqlite.
Why: Those reads were waiting on the shared writer lock and spending the first-input deadline. The marker removes that wait without a new connection, a longer budget, or a snapshot that hides a later revocation.
So what: This review changes no source. The parent owns merge. Live cells and the historical 23:44 stage stay unproven.

Verdict

No blocking findings. Context review/independent-grok, head f6a8872d1bd13c1caaf169fb6c23c92602fdfaa1, base 9ab53882e87a9e0a6413f74d8b313ce516721d40. Author worktree /home/luke/dev/toolboxmd/chromeria-worktrees/34-first-input-deadline was clean at that HEAD. This review did not edit, check out, or merge.

Inspected scope

  • Diff: apps/server/src/contextmd/ContextmdServing.ts and ContextmdServing.test.ts only. The production change is NodeSqliteClient.readOnly around moment, with catchCause left outside it.
  • Pinned client: packages/shared/src/nodeSqliteClient.ts at this HEAD, and installed effect@4.0.4 SqlClient.ts / Statement.ts.
  • Call path: ContextmdRoute builds authorized as EnvironmentAuth.authenticateHttpRequest and runs it inside moment. SessionStore.verify and AuthSessionRepository.getById are selects. Signature, expiry, scope, and revocation stay in that effect. DPoP replay uses the file secret store.
  • Main-database statements reachable from moment: resolveRun, threadDirectory, checkDenies, checkReads, and Cue transcript. All are selects. Deny, read, claim, budget, settlement, and log writes use ServingState on a separate DatabaseSync for contextmd.sqlite.
  • Production composition: server.ts provides SqlitePersistence.layerConfig, and layerFromPath sets readerWorker: true. ContextmdRoute.layer is on that server. layerSetup sets WAL before listen. In-memory layers do not start a reader, so readOnly stays on the writer there.
  • Other readOnly callers (ShellStream, ThreadPullRequestService) are unchanged. moment does not call withTransaction and does not fork.

Cleared

  • Routing. readOnly is Effect.provideService of the ReadOnly reference for that effect only. With no transaction service in context, each statement uses acquirer, and route picks the reader when the reference is set and a worker exists. An open write transaction keeps the writer connection (SqlClient.getConnection). The moment route does not open one. A write on the reader fails with "readonly database"; nothing in this path issues one.
  • Marker end. The provision ends when moment ends. settle and the test's later revoke run outside it. The green run commits a revocation and the next request returns 401, so the reader did not keep the old snapshot.
  • Freshness. moment does not begin a read transaction. Each select is its own autocommit on the reader, so a commit made after the previous request is visible. Auth failure in the route becomes false through orElseSucceed, then 401.
  • Cold reader and fallback. The new test seeds the run and holds the writer before the first moment, so the worker starts cold. Existing client tests cover a failed open, non-WAL mode, startup timeout, and interrupt during startup: the reader turns off and later reads use the writer. That fallback can bring the deadline failure back for the life of the process. It is the existing client behavior.
  • Other events. Claude, Codex, and OpenCode first input are executed under the lock. Grok and other hook kinds use the same moment. Their main-database work is the same selects. Cue, deny, and read updates stay on contextmd.sqlite.
  • Oracle. Without the marker, all three harnesses returned 204 where 200 was required, at 1074 ms, 1018 ms, and 1032 ms (red.log, SHA-256 ec0494dce65f0204f3dbc90b30d4150e7eeee83238ab12e1148464dcc9b3ff40). With the marker, the same three passed in a 1.85 s run (green.log, SHA-256 198e3a821a9705b927eec2ea31db660db8fdc3c16576fb670aa7803b2626ee7c), too short to have spent three 950 ms allowances. The committed file's assertions, including bytes, settlement, same-epoch 204, and revocation 401, are in the later 142-pass run.
  • Separate OpenCode failure. The Zen versus Go diagnosis is a different cause. This diff does not change provider or model selection.
  • contextmd serving log: over_budget moments carry no kind, session or tool-call id #252 memory test. ContextmdRoute.test.ts still expects 204 while a writer sleeps, on layerMemory, which has no reader. It still covers the timeout log. The disk test is the production-shaped lock proof.

Proof limits

  • This review read the diff, the pinned client, the auth and serving paths, and the author logs. It did not re-run tests, typecheck, or lint.
  • Final author run on this HEAD: 142 passed, 0 failed, 0 skipped, 19.26 s, started 02:09:18 after the 02:08:59 commit (focused-final.log). Server typecheck log has suggestions and no error TS line. Lint log has no findings. fork-check reports OK against 8d11fe1261f0dfd74f58a1b1ae8ad7e400905fcb. The red stack points at line 334; the committed assert is line 337. The 142-pass run is the committed file.
  • Cited manifest 6b6b1764b02d57634ad546c790dcb810179a122676ff9acb268e8068ac8ac58c is MANIFEST-before-pr-evidence-link.json. Current MANIFEST.json is cd20b645b53310fa73bacc2e415ec955cc1a4f9c3914a467197d9413b82fc9c3 and records the same red and green hashes. Those files match.
  • CI run 38658490581 showed pass for build, lint, typecheck, test, server shards, web, Rust, and release smoke at comment time, and the fork stack model was pass. EAS Preview and Mobile Native Static Analysis were skipping. This review did not use that rollup as its own execution.
  • The exact 23:44 queue stage and lock holder remain unknown. Current load and live rows are not a timing of those calls. No deployed repair is claimed. OpenCode's model failure is not fixed here.
  • Sol's status was already success. This verdict does not use it.

Elon record

  • Requirements and who asked: The user and the feat(server): link GitHub Issues to threads (#28) #34 review dispatch require an independent exact-head Grok review of this SHA, a findings comment, then review/independent-grok. The repair must keep the first native input pointer inside its deadline by using the existing reader, with the same budgets and no new connection, service, or timer.
  • Deleted: A source change, a re-run of the suite, live database access, install, deploy, and a second design round.
  • Bottleneck: Showing that readOnly moves moment's main-database reads off the writer lock while serving writes and revocation checks stay where they are.
  • Checked myself: The full diff, Effect SQL 4.0.4 acquire and transaction routing, the reader implementation and its tests, the moment select and ServingState paths, SessionStore.verify, server.ts layerConfig composition, and the red, green, and final logs above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant