Skip to content

fix(server): t3_get_session reads the session on the SQLite reader - #349

Merged
bk-agent-01 merged 2 commits into
stagefrom
fix/bkt3-get-session-reader
Oct 23, 2026
Merged

bk-agent-01 merged 2 commits into
stagefrom
fix/bkt3-get-session-reader

Conversation

@bk-agent-01

@bk-agent-01 bk-agent-01 commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Refs TEC-1502

Problem

The early production check after PR #344 (evidence) found 3 freezes in the first 34 minutes after the 2026-10-23 deploy. Two of them came from the t3_get_session MCP tool:

  • T3ControlToolkit.getSession → ProjectionSnapshotQuery.getThreadDetailSnapshot → sql.transaction on the writer → readCanonicalProjection. The turn-items query took 3.4 s at 16:17:44 UTC and 3.9 s at 16:40:06 UTC. The server logged freezes of 2.8 s and 3.7 s.
  • Before the restart, the same path took 4.1 s at 16:09:52 UTC and caused the 3.6 s freeze at 16:10:16 UTC.
  • After the restart: 21 calls, 7 held the writer for more than 1 s, max 5.2 s.

Agents call this tool often, because the global session protocol tells every session to call t3_get_session before each final reply.

Fix

getThreadDetailSnapshot (fork-only file orchestration-v2/Layers/ProjectionSnapshotQuery.ts) now runs its read transaction under NodeSqliteClient.readOnly, on the SQLite reader worker:

  1. importer.ensureTranscript(threadId) still runs first, on the writer, because the v1 transcript import can write. After the first import, the importer's in-memory cache makes this check free.
  2. The session read (detail(threadId, false) and the snapshot sequence) then runs on the reader. detail(..., false) does not import again, and every query in it is a SELECT.

This is the same boundary as #344. The HTTP thread snapshot route already reads on the reader through ProjectionStore.getThreadSnapshot. Authorization, the result shape and the audit (withMcpToolAnalytics) do not change: the tool handler is unchanged.

Both changed files are fork-only, so no fork markers are necessary.

Not in this PR (Manager decision for Tushar, 2026-10-23): the upstream OrchestratorMcpService.loadCaller also read the caller's thread on the writer (3.0 s at 16:16:19 UTC).

How I tested

This host runs no tests (the dev-server compute policy). CI runs the checks on the head.

New ProjectionSnapshotQuery.expbkt3.test.ts builds the real snapshot query layer on a temporary file database with reader workers. It records the connection of every SQL statement:

  • The first read of a v1 session imports its transcript. Its writes run on the writer, and the read succeeds. On the reader, these writes fail with "attempt to write a readonly database".
  • The second read runs every statement on the reader, including the turn-items read, and returns the session's activity.

After the merge into stage, I check stagebkt3: t3_get_session spans must show db.connection=reader, and the result must stay the same. The bkmain promotion waits for Tushar's go.

Review

A fresh-context review of fbddc9c085 found that the runtime change is correct. Nothing inside the read-only transaction writes. snapshotSequence and the detail come from one reader snapshot. The only caller is t3_get_session, and no caller needs read-your-writes.

  • P0, fixed in 80e17c6f17: the test passed a plain string where activities[].id is a branded EventId (TS2769). This failed Build and Typecheck, validate and Check. The test now uses EventId.make. The write filter also matches the import's WITH … INSERT.
  • P2, kept for the production check: the worker result copy and the decode of every turn item still run on the event loop. I will measure event loop stalled and the getSession spans after the deploy. If the decode stays long, the next step is to read a window (getThreadSnapshotWindow), not the whole session. That change is outside this PR.
  • P2, accepted: four overlapping large reads can hold all 4 reader workers, and short reads then wait. Before this change, those reads waited behind the held writer and the blocked event loop, so this is still a net gain.
  • P2, accepted: a long reader transaction holds back the WAL checkpoint. Shell snapshot reads already do the same, and the bounded admission gate limits the wait for writes to 1 s.

🤖 Generated with Claude Code

getThreadDetailSnapshot, the whole-session read behind the t3_get_session
MCP tool, ran its transaction on the writer. On bkt3 it held the writer, and
so the event loop, for 3.4-5.2 s and caused two of the three freezes in the
first 34 minutes after the 2026-10-23 deploy.

The transcript import still runs first on the writer, because it can write.
The session read then runs under NodeSqliteClient.readOnly on the reader
worker, the same boundary as PR #344.

Refs TEC-1502

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS labels Oct 23, 2026
…INSERT writes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Merging into stage only, pinned to head 80e17c6f1700529bb7fcb01183de976f06ad3cc1.

  • Go: Manager judgement for Tushar (Manager rule 2026-10-21 and rule 2026-10-23), relayed in the T3 session Debug BKT3 Connection Drops: "Merge PR fix(server): t3_get_session reads the session on the SQLite reader #349 into stage only, pinned to 80e17c6, after your immediate fresh preflight." The Manager skill allows green bkt3 stage merges by judgement. This go does not cover a bkmain promotion. Tushar keeps that go.
  • Preflight on 80e17c6f1700529bb7fcb01183de976f06ad3cc1 (just now): MERGEABLE/CLEAN. 0 unresolved review threads. Every check passes or skips: CI (Build and Typecheck, Lint, Check, Test, Test Server 1-8), markers, validate. The newest Mobile Fingerprint Check run (39996551366) succeeded. The cancelled run 39996532506 is an older run that the newer one replaced.
  • Review: Round 2 of the fresh-context review found nothing new on this head (see "## Review" in the PR body).

Next: validate the stagebkt3 deploy and the t3_get_session reader spans, then record the proof here and on TEC-1502.

🤖 Generated with Claude Code

@bk-agent-01
bk-agent-01 merged commit 1d8cb7a into stage Oct 23, 2026
26 of 27 checks passed
@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Stage validation (stagebkt3, merge 1d8cb7a571)

  • Deploy: "Validate and deploy stage T3" run 40000764534 succeeded. ~/.t3/stage-dev/deployed-sha is 1d8cb7a5714f022d6acf18dafff4eef4970ec2a9. t3-stage.service is active since 18:25:09 UTC, and the health endpoint gives HTTP 200.
  • Real tool call: I started one Codex session (gpt-6.1-sol, low effort) in a new scratch project on stage. Its prompt allowed only the t3_get_session tool. The session made two calls:
    1. Its own session: OK. The title was right and snapshotSequence was 2994847.
    2. Session 302ae3a8 (20,469 turn items): denied by the access rule ("An in-session agent may only control its own T3 session"). The denial comes before any read. This is correct, but this check therefore did not read a large session.
  • Trace of call 1 (T3ControlToolkit.getSession, 18:26:28 UTC, 18.0 ms, Success):
    • The transcript check SELECT … FROM orchestration_v2_legacy_imports ran with db.connection=reader (2.6 ms). This is a native session with no import row.
    • sql.transaction ran with db.connection=reader (13.7 ms). It contains ProjectionStore.readProjection and readCanonicalProjection.
    • Before this PR, this transaction ran on the writer.
  • Trace of call 2: 8.1 ms, Failure, no transaction.
  • Stage freezes since the restart: 0.
  • Cleanup: the test session is archived (18:27:27 UTC), and the scratch project is soft-deleted. The 1-hour stage token a04a384b is revoked: the server logged "Session token revoked" at 18:27:29, and auth session list no longer shows it. No process remains in the scratch repository.

Not done here: no bkmain promotion, and no read of a large session in production. Tushar keeps the promotion go. The large-session effect shows only on bkt3 after a promotion.

🤖 Generated with Claude Code

@bk-agent-01

Copy link
Copy Markdown
Collaborator Author

Production proof (bkt3, 2026-10-24)

This PR has been live on bkt3 since the #350 deploy at 2026-10-23 19:38:42 UTC. bkt3 now runs cdf2769a76, which contains it.

t3_get_session in the production trace (10:50 to 16:23 UTC):

  • 28 calls ran 34 read transactions. All 34 ran with db.connection=reader, and 0 ran on the writer.
  • Before this PR, 7 of 21 calls held the writer for more than 1 s, and the longest held it for 5.2 s.
  • Large sessions still take long, now off the event loop: p50 2.2 s, p90 7.8 s, longest 12.5 s (15:16:44 UTC).
  • None of the 4 long reads (12.5, 9.8, 7.8 and 5.9 s) lines up with a logged freeze.
  • The review's P2 note still holds. A 12.5 s answer is slow for the agent and holds one of the 4 readers. Reading a window instead of the whole session would fix both, as a later change.

Freeze rate: 0.44 per hour over 20.4 h with #344 and this PR, against 1.43 per hour in the 24 h before #344. The remaining freezes come from upstream paths. The full table and causes are in the PR #344 measurement.

🤖 Generated with Claude Code

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