[amiddavid]

Behavior change: needs a human developer's yes before merge.

• Who: only users who pick the new preset ultrahigh, on Claude models through the Anthropic route. Every other preset is unchanged, xhigh included.
• Old: every tool definition is sent in full on every request.
• New: in ultrahigh, a tool that the user's last 3 sessions declared and never used is sent with defer_loading: true. The proxy adds the API's search tool and a list of the deferred names. The model loads a tool through search when it needs one. No tool is removed.
• Off switches: pick any other preset; toolfilter: {defer: off} for the account; ENABLE_TOOL_SEARCH=false for one Claude Code session.
• Also for every plugin user: the plugin adds one SessionStart hook for startup. It sends nothing unless ENABLE_TOOL_SEARCH turns search off; then it posts the session id and that value to /tool-search.

Data dependence: changes. toolfilter moves from none to metadata (tool names only). A new history table keeps, per user, session and tool: declared, used, project, time. Nothing is shared between users. With the Store off, nothing is deferred. The owner confirmed both rows in docs/data-dependence.md (David Amid, #485, 2026-11-02). The owner also confirmed the behavior change.

Summary

• Adds the preset ultrahigh = xhigh + tool deferral (toolfilter: defer unused tool declarations at session start, by history (was: remove at cold moments) #485). It is opt-in.
• The decision and the rewrite live in a new library package, tooldefer. The proxy only wires it.
• Live check on the plain gateway, Claude Code 2.1.316: 12 built-in tools deferred. The written prefix fell from 30,693 to 12,664 tokens. A search loaded WebFetch and the task passed. A proxy restart kept the set byte for byte. No request returned 400.
• Replaces the approach of feat(toolfilter): auto mode with a session-fixed set and a restore tool (#434) #449. feat(toolfilter): auto mode with a session-fixed set and a restore tool (#434) #449 can close when this merges.

What changed

1. tooldefer (library).
   • Rule: defer a tool when all of these hold. It is not used or loaded in this thread. The user's last 3 sessions, in any project, declared it and used it in none. This project did not use it in its last 10 sessions. It is not on the never-defer list. It weighs at least 200 tokens, weighed on the original request. With fewer than 3 sessions, nothing is deferred.
   • MCP servers are one unit: all of mcp__<server>__* is deferred together when the user's last 3 sessions declared the server and used none of its tools, the project used none of them in its last 10 sessions, and the server's tools weigh at least 200 tokens together. A use or a load of any one of its tools counts as a use of the server. Built-in tools keep the 200-token minimum per tool.
   • Never-defer list: Bash, Read, Edit, Write, Agent, Skill, ToolSearch, AskUserQuestion, context_guru_*, a tool tool_choice forces, and the account keep list (toolfilter.keep).
   • Rewrite: defer_loading: true on each deferred tool, plus tool_search_tool_bm25_20251119 in tools. The names list goes into the role: system message at messages[1] when it exists, else into a new system message at index 1. The bytes are the same on every request.
   • Guards: the last loaded client tool is never deferred. A cache_control mark on a deferred tool moves to the last loaded tool. Server tools and tools the client already deferred are left alone.
   • One decision per thread: at the thread's first request. It is stored under cg:tdf:, a pinned metadata prefix, always on disk: in the session-state table when the session store is durable (store.Durable), else in tool deferral's own file, context-guru-tooldefer.db, next to the dashboard database path. With no durable place, nothing is deferred. Subagents are their own thread.
   • History: its own table, tool_defer_history, newest 64 sessions per user. It is held in memory, loaded and written in the background. The request path never waits on a query. On first use it copies the names from the dashboard's tool_declarations and tool_uses. A load through either search counts as a use.
   • Provider check: behaviour, not status. One loaded tool, one deferred tool with a unique name, and the search tool. It passes only when a tool_search_tool_result loads that tool and a call to it follows. Every check has a nonce. It is kept per upstream, model and route, re-run every 24 hours, and dropped after a 400. The verdict is kept in tool_defer_probe, so a restart keeps it.
   • Net saving: deferred tokens at the tier each request was billed at, minus the names list, the search output, and the loaded definitions read once without the cache. Weights are scaled by tokens.BilledDeltaFactor.
2. Proxy.
   • Runs deferral last, on the final tools, after the proxy's own tools.
   • A 400 or 422 on a changed request never reaches the client. The proxy sends the original once, then turns deferral off for that account and model.
   • The check runs in the background through the host's Dispatcher with the caller's own credential.
   • A new route, POST /tool-search, takes the session id and the raw ENABLE_TOOL_SEARCH value. false, 0, no or off, in any case, turn deferral off for that session. It uses the same checks as /signal, and it works with keep-alive off.
   • /stats gains a tool_defer block: the net ledger and counters.
   • POST /tool-defer/probe has the same checks as /signal and returns 202 at once. The account's next request starts the provider check in the background, with that request's own upstream, model and credential. The proxy holds no credential between requests. Both preset pickers call it when the user picks ultrahigh, and print probe=asked.
3. Config. The ultrahigh preset. New toolfilter fields: defer (off | auto) and keep.
4. Plugin. The startup hook above. ultrahigh is in both preset pickers, settings.py, insights.py, codex_plugin.py and plugin.json. The Codex picker says that on OpenAI models ultrahigh behaves like xhigh (deferral is Anthropic-only in v1, feat(toolfilter): our own tool search for providers without defer_loading (GPT, GLM, Kimi) #825).
5. Docs. docs/reference/presets.md, docs/data-dependence.md, README.md.

How it was tested

All on box 2, CGO_ENABLED=1 go test -race -count=1, at head 48db5295, rebased on c2a06dd5:

Package set	Result
./tooldefer/	ok, 1.3 s
./config/	ok, 76 s
./components/... (includes datadep_test.go)	ok
./store/	ok, 33 s
./metrics/, ./internal/modelinfo/	ok
./context-guru-plugin/	ok, 349 s
./proxy/...	ok, 470 s
./dash/...	ok, 535 s (before the rebase; no dash file changed)

Unit tests cover every rule row, the never-defer list, the 200-token gate on original weights, a set that stays fixed across turns, a simulated restart (rebuild from the request), names-list placement at messages[1], the cache_control move, the last-loaded-tool guard, each off switch, the 400 retry, the check verdicts and nonce, and the net-saving arithmetic. Proxy tests drive the real handler: ultrahigh defers and xhigh does not. A 400 is retried with the original, and the client sees 200. A restart keeps the set. The lifecycle flag works.

Mutation checks: 10 of 10 failed at least one test, and the restored code passes.

#	Mutation	Caught by
1	Never-defer list ignored	TestDecideRuleRows + 3 more
2	200-token gate set to 0	TestDecideWeighsOriginal + 2 more
3	Last-loaded guard removed	TestRewriteKeepsLastLoaded
4	cache_control not moved	TestRewriteMovesCacheControl
5	400 retry off	TestToolDeferRetriesOriginalOn400
6	Set decided again every request	TestApplyDecidesOnceAndKeepsTheSet + 1 more
7	Any ENABLE_TOOL_SEARCH value turns it off	TestApplyOffSwitches
8	Rebuild drops loaded tools	TestApplyRestartRebuildsTheSameSet
9	Names list at the end	TestRewriteShape + 1 more
10	Deferral on in every preset	TestToolDeferOffInXhigh

Live check. The proxy binary was built on the box for darwin/arm64 and run on the laptop with --preset ultrahigh --dashboard. Its upstream was a logging forwarder to the plain gateway ($ANTHROPIC_EXT_BASE_URL), not Context Guru. Claude Code 2.1.316 ran with CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS=1, so its own search was off. Every prompt carried a unique nonce. 16 requests, about $0.30.

Step	What happened
3 seed sessions (Bash only)	The check ran on the first request and passed: the search loaded the probe tool and the model called it. Nothing was deferred, because fewer than 3 sessions existed. Prefix written: 30,693 tokens.
Task 1: weather (MCP tool)	12 built-ins deferred. Prefix written: 12,664 tokens. The next request read 12,664 from cache. The MCP tools stayed loaded, because each weighs under 200 tokens. The task passed.
Task 2: fetch a page	One search with limit: 1 loaded only WebFetch, and the model called it. The task passed. The next request read the 12,669-token prefix. A second check ran for Haiku, which WebFetch's side call uses, and passed.
Proxy restart, then --resume of task 2	tools and messages[1] byte-identical to before the restart. Prefix read 13,363, written 74. The stored decision came back from disk: 0 new decisions, 0 rebuilds. WebFetch was reused without a new search.
Whole run	0 responses other than 200.

Technical notes

• limit is not a field of the tool. The API takes limit in the model's search input. So the names list asks the model to pass limit: 1. In the live run it did, and each search loaded one tool.
• Review round 3 (Osher, B1–B4 and H1–H4) is fixed in c2b7077b:
  • B1: the per-thread record is durable in every setup.
  • B2: the verdict is written after the prober lock is released.
  • B3: an account purge removes the tenant's history, records and opt-outs.
  • B4: the seed and the opt-outs are per tenant.
  • H1: a 400 is proved ours when the original then succeeds.
  • H2: a Fail holds 15 minutes, and the probe has max_tokens 1,000.
  • H3: the opt-out is stored, and it never flips a live thread.
  • H4: only priced models are probed, with caps of 4 in flight and 256 keys.
  • The other review items are in Tool deferral follow-ups from the #861 review (N1–N10, N12) #866.
• Claude Code never echoes our rewritten tools or our names list. So "rebuild from the request" after a lost record means: decide again with the same rule, leaving out the current session's history. Then add every tool our search loaded, which the client does echo. Known limit: a tool the model called without a search, or a history change from another session that ended in between, can move one tool back once. With the session-state database on (the default with --dashboard), the record survives a restart and the rebuild does not run.
• Start-up window: before the background load ends (a few milliseconds), a new thread reads no verdict and no history. It then decides "nothing" for its whole life. That is the safe side.
• A thread that starts before the first check passes defers nothing for its whole life. The owner accepted that the first session per model is not deferred. Picking ultrahigh starts the check at the next request, so later sessions do not wait for one.
• The ledger misses some searches. It reads searches from the 64 KiB head and tail of a stream-through response. A search in the middle of a very long response is not counted, so searches can read low.
• Not done in this PR:
  • the dashboard's own changes: recording deferred built-ins on tool_declarations, a net-saving panel, and save-more tools. The history table already records both of the first two;
  • adding up usage across expand rounds (toolfilter: defer unused tool declarations at session start, by history (was: remove at cold moments) #485, B4). Deferral adds no in-band round, so its own figures do not need it;
  • Claude Code with its own search on. The rule applies only to tools Claude Code sends in full, so its deferred tools are untouched. This path is covered by unit tests only, not by a live run;
  • the 40-task accuracy test from toolfilter: defer unused tool declarations at session start, by history (was: remove at cold moments) #485.
• Left for later, from review round 1: the ledger prices every search at a 30-token output estimate, because the host never sets SearchOutput from usage. A refused 400 round does not show on the dashboard row; /stats retries_400 counts it. After gateTTL (24 h), a stored thread record is applied again with no new probe check.
• Refs toolfilter: defer unused tool declarations at session start, by history (was: remove at cold moments) #485, feat(toolfilter): auto mode with a session-fixed set and a restore tool (#434) #449.

🤖 Generated with Claude Code

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	fresh	1 of 2	1 of 2	48db529521de	blocking

Verdict: blocking

Findings

1. tooldefer/history.go:375 — the history hands out live maps, so a concurrent request can crash the whole proxy

   • What fails: two ultrahigh requests at the same time. Request A starts a new thread and decides it. Request B feeds the history for another session. A reads session maps with no lock while B writes them. Go then throws fatal error: concurrent map read and map write. That error kills the process. No recover() catches it, so the fail-open promise does not hold.
   • Proof: UserSessions returns Session{Declared: s.declared, Used: s.used} — the live maps, not copies (tooldefer/history.go:375). The lock ends when it returns. Decide then reads those maps in unusedIn (tooldefer/tooldefer.go:150) with no lock. Observe writes the same maps under m.mu (s.declared[n] = true at tooldefer/history.go:352, s.used[n] = true at 357). Both run per request on the proxy path: applyToolDefer calls Observe (proxy/tooldefer.go:92), then Apply → Decide → UserSessions. net/http runs each request in its own goroutine, and no mutex in Handler serializes chat. The tests run Observe and Decide in sequence only, so they cannot see it.
   • Fix: in UserSessions, build copies of Declared and Used while holding m.mu, and return those.

2. tooldefer/apply.go:105 — a Fail probe verdict is never re-probed, so one transient failure turns deferral off forever, across restarts

   • What fails: a probe that ends in Fail disables deferral for its upstream+model+route. Nothing ever re-probes it. Verdict returns the stored value with no age check (tooldefer/probe.go:109). NeedProbe is set only when the verdict is Unknown (tooldefer/apply.go:105 and :118). Ensure, the only re-probe path, runs only from startProbe, which runs only when NeedProbe is set (proxy/tooldefer.go:97). SaveVerdict persists a Fail and LoadVerdicts re-seeds it after a restart. So one bad answer — a 401, a 403, a gateway 400, all read as Fail by RunProbe — keeps deferral off for the process life and past it.
   • Proof: the code comments promise the opposite. Verdict says a verdict older than ProbeTTL "is still returned while Ensure re-probes" (tooldefer/probe.go:107). Seed says an older verdict "still seeds: Ensure re-probes it in the background" (tooldefer/probe.go:87). The package comment says the verdict is kept "for ProbeTTL" (tooldefer/probe.go:28). No code path calls Ensure for a stale non-Unknown verdict. Forget is the only removal, and it runs only after a 400 on a changed request — which cannot happen while deferral is off.
   • Fix: set NeedProbe also when the verdict is stale (older than ProbeTTL). Add a Stale(k) method on Prober and check it next to v == Unknown. Ensure already no-ops on a fresh verdict, so the stale-while-re-probing behavior the comments describe then works.

Non-blocking

• tooldefer/gate.go:83 — Break evicts one arbitrary broken entry (first map iteration), not the oldest. searchOff uses pruneOldest. Under pressure the map can drop a fresh turn-off and keep stale ones.
• tooldefer/probe.go (RunProbe) — a 401 or 403 returns Fail, not an error. A credential problem then reads as "no provider support". With finding 2 fixed this self-heals in 24 h; consider treating 401/403 like 429 anyway.
• proxy/tooldefer.go (deferLedger) — Request.SearchOutput is documented as "from usage when known", but the host never sets it. The 30-per-search estimate always applies.
• tooldefer/rewrite.go:184 — ClaudeCodeSearchOn is exported and has no caller. Dead code.
• proxy/tooldefer.go (deferRetry) — it fires on any 400/422 for a changed request. A 400 caused by another component's rewrite still Breaks deferral for tenant+model for 24 h and drops the probe verdict. Conservative, but it disables the feature for a fault it did not cause.
• Tests — SearchesIn, SearchesInSSE and LoadedWeight have no test, and no end-to-end test books a search turn in the ledger. The unit tests cover Price/Add only.
• Process — the new rows in docs/data-dependence.md say "Decided by: David Amid, #485 (to confirm)". CLAUDE.md needs that confirmation before merge.

What I checked

• Read the full diff (35 files) and the code around each change: tooldefer/ (all 7 source files), proxy/tooldefer.go, the proxy.go wiring, clientsignal.go, usage.go, toolfilter.go, config.go, store.go, disk.go, metrics.go, the plugin scripts and hooks, the docs, and every new test.
• Traced the fail-open paths: Apply's panic recover, the outer pre-forward recover in chat, the 400 retry in serve (round 0, di.orig is the pre-deferral body, di cleared so the ledger skips it), and Rewrite's decline paths. All return the input body.
• Traced cache discipline: the record key is pinned (cg:tdf: in DefaultPinPrefixes and metadataPrefixes), the set is fixed per thread, Rebuild re-derives plus search-loaded tools, and the names list lands at messages[1] with stable bytes. Confirmed role:"system" inside messages is a shape Claude Code itself sends (apply/billedpair_capture_test.go:161), so the inserted system message is not an API error.
• Checked the off switches: account off, store_off via Persists(), not_anthropic, ENABLE_TOOL_SEARCH=false (hook → /signal → Gate, session ids scoped the same way on both sides), and the post-400 turn-off.
• Checked the store and history boundaries: tool_defer_history is its own table, bounded at 64 sessions per tenant, seeded from the dashboard tables once, flushed off the request path. ToolDeferPrefix is metadata, so the disk store keeps it at every storage level.
• Checked /stats: the new tool_defer field is additive, and the Prometheus coverage test exemption is registered.
• Verified helpers exist and match use: apply.ProjectFor, internal/deferred, internal/tokens, dispatch, modelinfo.Price, store.Nop.Persists.
• Did not run builds or tests (not approved). Findings above are from reading the code only.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	fresh	2 of 2	1 of 2	48db529521de	blocking

Verdict: blocking

Findings

1. proxy/tooldefer.go:136 — the 400 retry waits on a database write before it answers the client

   • What fails: a ultrahigh request the provider refuses (400/422) stalls on the dashboard database before the retry is sent. deferRetry calls Prober.Forget, which calls Persist = Mem.SaveVerdict. That runs CREATE TABLE IF NOT EXISTS plus a DELETE on the request path, under one 5-second context (tooldefer/history.go:85-107). The dashboard's writer goroutine holds SQLite write locks in batches, so the client can wait seconds on exactly the path that exists to hide the 400.
   • Proof: the trace is serve round 0 (proxy/proxy.go:1965) → h.deferRetry (proxy/tooldefer.go:130) → ds.prober.Forget(di.probe) (proxy/tooldefer.go:136) → p.Persist(k, Unknown, ...) (tooldefer/probe.go:156-163) → m.db.ExecContext twice. The package's own rule forbids this: "the request path must never wait on a query" (tooldefer/history.go:25-26) and SaveVerdict's comment says it "runs on a probe's background goroutine, never on the request path" (tooldefer/history.go:87-88). Forget breaks that contract.
   • Fix: keep Forget's memory clear on the request path and move the persist off it — hand the SaveVerdict call to the prober's Go runner, or let the next Ensure re-probe rewrite the table in the background.

2. proxy/tooldefer.go:170 — /stats and Close read the deferral state without the lock that builds it

   • What fails: toolDefer() writes hist, gate, prober, stop under sync.Once (proxy/tooldefer.go:37-55). deferStats() reads ds.gate, ds.prober, ds.hist and Close() reads h.defer_.stop (proxy/proxy.go:483) without going through the once and without any lock. A /stats call that races the first request that builds the state reads partially initialized fields. It can see gate set and prober still nil, then panic in ds.prober.Runs().
   • Proof: the only synchronization is the once; deferStats takes no lock and does not call toolDefer(). Under the Go memory model the concurrent read is undefined, and go test -race flags it the moment a test covers it.
   • Fix: guard the lazy build and the reads with one mutex, or make deferStats and Close go through a synchronized accessor.

Non-blocking

• tooldefer/rewrite.go:97 — a cache_control mark can be silently dropped. The mark moves to the last loaded tool only when that tool has none. If it already carries one, the deferred tool's mark is deleted and not re-placed. With two marked deferred tools, only the first mark is captured. The request then carries fewer breakpoints, the provider caches a shorter span, and later turns pay fresh input for the rest. Deterministic per thread, so the prefix does not flap.
• A thread-fixed set ignores tool_choice on later turns. A turn that forces a deferred tool can draw a 400. The retry then turns deferral off for the whole tenant and model for 24 hours. Rewrite could leave the forced tool loaded, the same way Decide does for the first request.
• Any 400/422 on a changed request turns deferral off for 24 hours, also when the 400 has another cause (bad model name, malformed content). The client still sees the right error from the retried original, but the tenant loses deferral for a day.
• After gateTTL the stored record re-applies with no probe check. A provider that permanently refuses defer_loading pays one 400 plus retry per live thread per day, because Gate.Broken expires while the cg:tdf: record lives on.
• The refused round is invisible on the dashboard row. cp.noteUpstream runs after the retry, so the row shows one round and status 200. /stats retries_400 is the only trace of the 400.
• tooldefer/gate.go:83-87 — Break evicts an arbitrary entry at maxGate, while NoteToolSearch evicts the oldest (pruneOldest). The two maps age differently.
• Request.SearchOutput is never filled by the host (proxy/tooldefer.go:154-170), so the "usage when known" branch in Price is dead and every search is priced at the 30-token estimate.
• ClaudeCodeSearchOn (tooldefer/rewrite.go:186) is exported and unused.
• Only the literal false turns deferral off. ENABLE_TOOL_SEARCH=0 or FALSE is not a decision, so a user who writes it gets deferral silently left on.
• noteToolSearch builds the full deferral state (dashboard DB open, background history load) on any /signal that carries enable_tool_search, even on a proxy where no tenant uses ultrahigh.
• Process: the two new rows in docs/data-dependence.md say "(to confirm)". Per CLAUDE.md, the person named in "Decided by" must confirm them before merge.

What I checked

• Read CLAUDE.md and docs/design.md, then the full diff of all 35 files, and the code around each change: proxy/proxy.go (chat, serve, streamRec, doUpstream, Close), proxy/clientsignal.go, proxy/usage.go, tooldefer/*.go in full, components/reformat/toolfilter.go, config/config.go, store changes, metrics, the plugin scripts and hooks.
• Traced the wiring: deferral runs last, only in non-observe, non-bypassed mode; the retry path closes the first response and re-sends di.orig; deferLedger runs once per request in serve's defer; the search counts come from the aggregated message, the raw JSON body, or the sniffer window on the three response paths.
• Verified the fail-open paths: Apply recovers panics, Rewrite declines on any shape it does not handle, a 400 never reaches the client, observe mode and bypass are untouched.
• Verified the cache discipline: one decision per thread under the pinned cg:tdf: key, same bytes on every turn, rebuild adds only tools a search loaded, restart keeps the set (tests pin all three).
• Verified the session scoping: /signal and the chat path both key on session.Scoped(tenant, id), so ENABLE_TOOL_SEARCH=false matches; subagent threads get their own record under the same session.
• Verified the library boundary (tooldefer imports no host package), the dashboard table and column names used by seed, and that /stats only gains the additive tool_defer block.
• Read the PR description: it names the behavior change and the data dependence at the top, and lists the known limits I re-derived above (start-up window, first thread after start, ledger search undercount, /signal off without keep-alive).
• Did not run builds or tests (not approved); the PR reports green -race runs on the box for every package set.

[amiddavid]

Reply to reviewer 1, round 1. All findings are fixed at head 448c6319.

Finding 1. History handed out live maps; a fatal concurrent map read and write. Fixed in 5ad98c0b.

• UserSessions now returns copies of Declared and Used, built under m.mu.
• TestHistoryUserSessionsReturnsCopies pins the copy contract. It fails every time with the old code: "0 uses before a later Observe, 3 after".
• TestHistoryConcurrentObserveAndDecide runs Observe and Decide from 16 goroutines under -race. With the old code it hit DATA RACE and fatal error: concurrent map read and map write on some runs, so the deterministic test is the one that guards it.

Finding 2. A Fail verdict was never probed again. Fixed in 0d1dff55.

• New Prober.Stale(k): the verdict is older than ProbeTTL.
• Apply sets NeedProbe on Unknown or a stale verdict, for a Pass and a Fail alike. The stale verdict is used until the new one lands.
• Test: TestStaleFailVerdictIsReprobed. A mutation back to "Unknown only" fails it.

Non-blocking notes:

• Break evicts the oldest entry: taken in 448c6319 (TestGateBreakEvictsOldest).
• A 401 or 403 is now an error with no verdict, like a 429: taken in 0d1dff55 (TestProbeVerdicts).
• SearchOutput is never set from usage: left for later, as the PR says. Every search is priced at the 30-token estimate.
• ClaudeCodeSearchOn deleted: 448c6319.
• Any 400 turned deferral off: taken in 448c6319. Every 400/422 to a changed request is still retried with the original. Deferral is turned off only when the provider's message names a deferral cause. Tests: TestDeferralCause, TestToolDeferOther400DoesNotTurnDeferralOff.
• No test for SearchesIn, SearchesInSSE, LoadedWeight: not taken in this round.
• The "(to confirm)" rows in the data-dependence doc: left for the owner.

Tests at 448c6319 (box 2, -race -count=1): the library set (tooldefer, config, components/..., store, metrics, internal/modelinfo, context-guru-plugin), then ./proxy/... (474 s), then ./dash/... (538 s). All ok. Mutations: each fix has one, and each fails a test.

[amiddavid]

Reply to reviewer 2, round 1. All findings are fixed at head 448c6319.

Finding 1. The 400 retry waited on a database write. Fixed in 0d1dff55.

• Prober.Forget now clears only the memory on the request path. The removal of the kept copy runs on the prober's background runner (Go).
• Test: TestForgetDoesNotPersistOnTheCaller. It uses a Persist that blocks, and checks that Forget returns and Persist did not run on the caller. A mutation back to a direct call fails it after 2 s.

Finding 2. /stats and Close read the deferral state without its lock. Fixed in f251d4ed.

• One mutex now guards the lazy build (toolDefer), the /stats read (deferPeek) and Close (closeToolDefer).
• Test: TestToolDeferStatsRacesFirstRequest runs /stats and /signal against the first ultrahigh requests under -race. With the lock removed from deferPeek, it fails with DATA RACE.

Non-blocking notes:

• A cache_control mark could be dropped: taken in 448c6319. A mark taken off a deferred tool goes on the nearest loaded tool before it that has no mark. Two marked deferred tools keep both marks (TestRewriteKeepsEveryCacheMark).
• tool_choice on later turns: taken in 448c6319. A forced deferred tool stays loaded for that request. The thread's set and the names list keep their bytes (TestRewriteLeavesForcedToolLoaded).
• Any 400 turned deferral off: taken in 448c6319. The original is always retried, but deferral is turned off only for a deferral cause.
• gateTTL re-applies a stored record with no probe check: left for later.
• The refused round does not show on the dashboard row: left for later. /stats retries_400 counts it.
• Break evicts the oldest entry: taken in 448c6319.
• SearchOutput from usage: left for later.
• ClaudeCodeSearchOn deleted: 448c6319.
• Only a literal false counted: taken in 448c6319. false, 0, no and off, in any case, now turn deferral off, in the proxy and in the plugin hook (TestSearchOffValue, TestSignalHookSendsToolSearchFalseOnStartup).
• noteToolSearch built the full deferral state: taken in f251d4ed. The gate is separate now, and /signal writes only the gate (TestToolDeferSignalDoesNotBuildState).
• The "(to confirm)" rows: left for the owner.

Tests at 448c6319 (box 2, -race -count=1): the library set, then ./proxy/... (474 s), then ./dash/... (538 s). All ok.

[amiddavid]

The owner's answers to the open questions are in, at head 2d3f1d1e.

• Data dependence: the owner confirmed it. Both tool deferral rows in docs/data-dependence.md now read "David Amid, toolfilter: defer unused tool declarations at session start, by history (was: remove at cold moments) #485, 2026-11-02". The behavior change for ultrahigh is confirmed too.
• (c) The ENABLE_TOOL_SEARCH flag has its own route: POST /tool-search, with {session, enable_tool_search}.
  • Checks: the same as /signal (JSON only, the same authentication and metering, the same tenant-scoped session id). It does not depend on keep-alive.
  • /signal no longer reads the flag.
  • The plugin hook posts there on a plain session start, and only when the value turns search off.
  • Tests: TestToolDeferToolSearchRouteWithKeepAliveOff checks that /signal returns 404 and the flag still turns deferral off. TestSignalHookSendsToolSearchFalseOnStartup checks the path and the body (3 fields).
  • Mutations: gating the route on keep-alive fails the first test. Posting to /signal fails the second.
• (d) Codex: the Codex preset picker lists ultrahigh. It says in one line that on OpenAI models it behaves like xhigh, because deferral is Anthropic-only in v1 (feat(toolfilter): our own tool search for providers without defer_loading (GPT, GLM, Kimi) #825).
• (a) The 200-token minimum and (b) no deferral before the first check: not changed. The owner wants more detail first.

Tests at 2d3f1d1e (box 2, -race -count=1): the library set, then ./proxy/... (474 s), then ./dash/... (524 s). All ok.

[amiddavid]

The owner decided the open questions (a) and (b). Both are in 8453212d (head).

(a) MCP tools are decided per server.

• All of mcp__<server>__* is deferred together when all of these hold:
  • the user's last 3 sessions declared the server and used none of its tools;
  • the project used none of them in its last 10 sessions;
  • the server's tools weigh at least 200 tokens together, weighed on the original request with the same token count as the per-tool gate.
• A use or a load of any one of its tools counts as a use of the server. A tool of the server on the keep list, forced, or used in this thread keeps the whole server loaded.
• Built-in tools keep the 200-token minimum per tool.
• The history still stores tool names only. The rule groups them by server when it reads them. docs/data-dependence.md says so.
• Tests:
  • TestDecideMCPPerServer: a server of 30 tools of about 120 tokens each is deferred whole. A server under 200 tokens in total is not.
  • TestDecideMCPOneUseKeepsTheServer: one use, the project veto, a server missing from one session, and a use in this thread each keep the server loaded.
  • TestHistoryMCPServerUseCarriesOver: the same through the real history.
• Mutations: going back to per-tool decisions fails 2 tests. Ignoring a use of another tool of the same server fails 2 tests.

(b) The first session per model is still not deferred. Picking ultrahigh now starts the probe.

• New route POST /tool-defer/probe, with the same checks as /signal. It returns 202 at once.
• The account's next Anthropic request starts the probe in the background, with that request's own upstream, model and credential, through the existing prober. The proxy holds no credential between requests. The ask works in any preset, so the request before the restart can start it.
• settings.py preset set --name ultrahigh and codex_plugin.py configure --preset ultrahigh call the route and print probe=asked, or the reason it did not happen (not_routed, unavailable). Other presets send nothing and print nothing new. Both preset-picker skills say so.
• On OpenAI models (Codex) the proxy ignores the ask, because v1 defers on Anthropic only.
• Tests:
  • TestToolDeferProbeAsk: 202 at once, with no upstream call and no probe on the ask itself; the next request starts the probe.
  • TestToolDeferProbeAskInXhigh.
  • TestPresetUltrahighAsksForAProbe (plugin).
  • Two codex unit tests.
• Mutation: ignoring the ask fails TestToolDeferProbeAskInXhigh.

Tests at 8453212d (box 2, -race -count=1): the library set (context-guru-plugin included), then ./proxy/... (476 s). All ok. Codex plugin: python3 -m unittest test_codex_plugin, 37 tests ok.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	1 of 2	2 of 2	8453212ddfa7	blocking

Verdict: blocking. All four round-1 findings are fixed and verified. Two small findings remain, both in the code added after round 1.

Findings

1. proxy/clientsignal.go:211 and :249 — the new routes ignore CONTEXT_GURU_CLIENT_SIGNALS=off, so an operator's kill switch does not stop them

   • What fails: an operator sets CONTEXT_GURU_CLIENT_SIGNALS=off. The /signal route then returns 404 (clientSignalEnabled, clientsignal.go:65-71 and :110). The new /tool-search and /tool-defer/probe routes never call that function. Both stay open. A client hook can still write a session's search-off flag, and the preset picker can still ask for a probe.
   • Proof: the file's own doc comment says the switch "turns the route and the effects off" (clientsignal.go:44). TestSignalsAreInertWhenDisabled pins that promise for /signal only. The owner's decision asked for "the same checks as /signal", minus the keep-alive dependency. The code dropped both halves of clientSignalEnabled: the keep-alive half (intended) and the explicit operator switch (not covered by the decision).
   • Fix: in both handlers, check the CONTEXT_GURU_CLIENT_SIGNALS value alone (off/0/false → 404), without the keep-alive half. One shared helper, one test.

2. context-guru-plugin/scripts/signal.sh:51 — a resumed or forked session never sends ENABLE_TOOL_SEARCH, so the user's opt-out is lost

   • What fails: a user sets ENABLE_TOOL_SEARCH=false and resumes the conversation, or forks it. Claude Code starts a new session id for it. The hook sends the flag only when kind == "startup" (signal.sh:51). The gate keys the turn-off by session id (searchOff, tooldefer/gate.go:16 and 41-48). The new id has no entry, so deferral turns on against the user's explicit opt-out.
   • Proof: round 1 sent the flag on every SessionStart. The new route commit narrowed it to startup only. resume and fork are accepted SessionStart sources (clientsignal.go:104-105), so the hook fires for them but drops the flag. The gate has no tenant-level fallback; SearchOff reads the session key alone.
   • Fix: send the flag on every SessionStart whose source can start a new id (startup, resume, fork, clear), still through /tool-search. Or state in the docs that the opt-out holds only for the first session of an environment.

Non-blocking

• proxy/tooldefer.go (deferRetry) — Retries now counts every 400/422 retry on a changed request, also the ones with another cause. The /stats name retries_400 says less than it now holds. Consider a separate counter or a doc line.
• proxy/tooldefer.go (takeProbeAsk) — the ask is consumed before the !on check. A tenant with deferral off still spends its ask on the next Anthropic request. Harmless: the probe verdict is useful either way, and the test pins the current shape.
• signal.sh:55 — os.environ.get("ENABLE_TOOL_SEARCH", "")[:16] truncates to 16 chars before the proxy's length check. A truncation cannot turn a long value into a clean off value, so this is safe today. A comment would keep it safe.

Earlier findings

• My finding 1 (live history maps, fatal concurrent map read and write): fixed at 5ad98c0b. UserSessions copies Declared and Used under m.mu. TestHistoryUserSessionsReturnsCopies and TestHistoryConcurrentObserveAndDecide pin it.
• My finding 2 (a Fail verdict was never re-probed): fixed at 0d1dff55. Prober.Stale plus NeedProbe on Unknown-or-stale, for a Pass and a Fail alike. TestStaleFailVerdictIsReprobed pins it. 401 and 403 are errors now, like 429.
• Reviewer 2, finding 1 (the 400 retry waited on a database write): fixed at 0d1dff55. Forget clears memory on the request path; the persist runs on the prober's Go runner. TestForgetDoesNotPersistOnTheCaller pins it.
• Reviewer 2, finding 2 (/stats and Close raced the lazy build): fixed at f251d4ed. One mutex in deferHost guards the build, the /stats read and Close. TestToolDeferStatsRacesFirstRequest pins it.
• Author rejections and deferrals, checked and accepted: SearchOutput from usage left for later (documented in the PR); gateTTL re-apply with no probe check left for later; the refused round missing from the dashboard row left for later (counted in retries_400); SearchesIn / SearchesInSSE / LoadedWeight tests not taken this round. The data-dependence rows are confirmed now ("David Amid, #485, 2026-11-02"), so the process item from round 1 is closed.
• Non-blocking items both reviewers raised, taken: cache marks preserved (TestRewriteKeepsEveryCacheMark), forced tool stays loaded (TestRewriteLeavesForcedToolLoaded), a 400 with another cause no longer turns deferral off (TestDeferralCause, TestToolDeferOther400DoesNotTurnDeferralOff), false/0/no/off in any case (TestSearchOffValue), /signal no longer builds the deferral state (TestToolDeferSignalDoesNotBuildState), Break evicts the oldest, ClaudeCodeSearchOn deleted.

What I checked

• Last review: round 1, sha 48db529521de0c439eac8f429d1f4a6223e4440e, posted 2026-11-02T17:17:04Z.
• New commits read: 6 (5ad98c0b, 0d1dff55, f251d4ed, 448c6319, 2d3f1d1e, 8453212d), plus the full diff 48db5295..8453212d.
• Read in full: 2 reviews, 4 comments, 0 inline comments.
• New items since my last review: 4 author comments (two fix replies, the owner's answers (c) and (d), the owner's answers (a) and (b)) and the 6 commits above. I read all of them and checked each fix and each owner decision against the code.
• Verified the fixes above by reading the changed code and the new tests, not by running them.
• Reviewed the new commits as a first review:
  • MCP per server (8453212d): traced Decide end to end. Units key on mcp__<server>__; mcpServer refuses malformed names (a name with no tool part falls back to the built-in per-tool rule). inSet prefix-matching cannot confuse mcp__srv with mcp__srv2, because the stored key is mcp__srv__ with both underscores. The veto list is complete: never-defer, keep (per tool or per server), forced, used in thread, used in the last 3 sessions, project veto, 200 tokens on the original weight. Checked that a client-deferred tool (defer_loading: true) is skipped before grouping, so it neither blocks its server nor joins the names list. Checked sort.Strings keeps the output deterministic, so the record bytes stay stable.
  • The probe ask (8453212d): /tool-defer/probe is authenticated, metered, JSON-only, and bounded (maxProbeAsks 10000). The ask is consumed only by an Anthropic-wire, Anthropic-provider request (applyToolDefer returns before takeProbeAsk on other providers), so an OpenAI request cannot burn it. The proxy holds no credential between requests; the probe uses the next request's own upstream and key.
  • /tool-search (2d3f1d1e): same authentication, metering and tenant-scoped session id as /signal. scopedClientID refuses an id Scoped would rewrite, so one tenant cannot write another's flag. Works with keep-alive off, as the owner decided. This is where finding 1 lives: it also works with client signals off.
  • The 400 retry (448c6319): serve reads up to 16 KiB of the error body and always retries the original on 400/422 for a changed request; deferRetry turns deferral off only when DeferralCause matches. Traced TestToolDeferOther400DoesNotTurnDeferralOff: the retry still happens, the client still sees the provider's 400 from the original round, and the ledger skips the refused round (di cleared). Fail-open holds.
  • Rewrite (448c6319): a forced deferred tool stays loaded for that request and keeps its bytes in the names list; cache marks walk back from the end to the nearest free loaded tool, and two marked deferred tools keep both marks. Traced TestRewriteKeepsEveryCacheMark by hand.
  • The plugin scripts (2d3f1d1e, 8453212d): the shell guard lowercases and matches false/0/no/off; the python payload truncates to 16 chars, which the proxy accepts; the ROUTE switch picks /tool-search only when the body carries the flag; settings.py and codex_plugin.py post the probe ask and print the outcome. This is where finding 2 lives: the payload is built only on kind == "startup".
• Checked the data-dependence doc against the new behavior: the history still stores tool names only; the per-server grouping happens at read time, and the doc says so. The rows are confirmed.
• Did not run builds or tests (not approved). Findings above are from reading the code only.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	2 of 2	2 of 2	8453212d	approve

Verdict: approve

Findings

None. Both my round-1 findings and both of reviewer 1's are fixed. The two new feature commits hold up under a first-review read.

Earlier findings

My finding 1 (the 400 retry waited on a database write): fixed in 0d1dff55. Prober.Forget now clears the memory only (tooldefer/probe.go:168). The kept copy is removed on the prober's background runner (Go). TestForgetDoesNotPersistOnTheCaller pins it with a blocking Persist.

My finding 2 (/stats and Close read the state without its lock): fixed in f251d4ed. One mutex in deferHost now guards the lazy build (toolDefer), the read (deferPeek), Close (closeToolDefer) and the gate (deferGate) (proxy/tooldefer.go:36-120). TestToolDeferStatsRacesFirstRequest races /stats and /tool-search against the first ultrahigh requests.

Agree with reviewer 1, finding 1 (live maps out of the history): fixed in 5ad98c0b. I checked it myself. UserSessions returns copies built under m.mu (tooldefer/history.go:378). ProjectUsed reads through inSet under the same lock. New proof: the per-server rule makes the old race wider, not narrower — unusedIn now scans the returned maps by prefix for every server unit, so a live map would race on every such query, not only per tool.

Agree with reviewer 1, finding 2 (a Fail verdict was never re-probed): fixed in 0d1dff55. I checked it myself. Apply sets NeedProbe on Unknown or a stale verdict (tooldefer/apply.go:101), and Prober.Stale reports the age (tooldefer/probe.go:119). New proof: Ensure counts a stale verdict as not fresh (tooldefer/probe.go:136), so the ask really starts the re-probe; and a 401/403 is now an error with no verdict, like a 429, so a credential problem no longer reads as "no support".

My round-1 non-blocking items. Fixed, each with a test I read: the cache_control mark (448c6319), the forced tool_choice (448c6319), the any-400 turn-off (448c6319), Break eviction (448c6319), the literal "false" (448c6319), the state build on a flag write (f251d4ed and 2d3f1d1e). Left open, with reasons I accept: the gateTTL re-apply (worst case one hidden 400+retry per live thread per day), the dashboard row for the refused round, SearchOutput from usage, and tests for SearchesIn/SearchesInSSE/LoadedWeight. The author rejected nothing.

Process. The data-dependence rows are confirmed: both read "David Amid, #485, 2026-11-02" (docs/data-dependence.md:187 and :218), and the owner's confirmation is in the PR thread.

Non-blocking (new)

• proxy/clientsignal.go:211 — /tool-search does not honor CONTEXT_GURU_CLIENT_SIGNALS=off. /signal answers 404 when that switch is off (proxy/clientsignal.go:65, :110). The new route skips the check, so the hook still reaches the proxy and still turns deferral off per session. The failure direction is safe (it only removes spend), but an operator who turned client signals off still serves the route. /tool-defer/probe skips it too. A check on the client-signals half only, not the keep-alive half, would close it.
• tooldefer/tooldefer.go:186 — two MCP servers whose names differ only after a __ share one unit. mcpServer cuts at the first __, so mcp__a__b__t and mcp__a__t both group under mcp__a__. Such server names are exotic, and the grouping errs toward keeping tools loaded, so I only note it.

What I checked

• Last review: round 1, 48db529521de0c439eac8f429d1f4a6223e4440e, 2026-11-02T17:20:27Z.
• New commits read: 6 — 5ad98c0b fix(tooldefer): hand out copies of the history maps; 0d1dff55 fix(tooldefer): re-probe a stale verdict, and keep Forget off the database; f251d4ed fix(proxy): build and read the deferral state under one lock; 448c6319 fix(tooldefer): review notes from #861 round 1; 2d3f1d1e feat(proxy): give ENABLE_TOOL_SEARCH its own route; confirm data dependence; 8453212d feat(tooldefer): decide MCP tools per server; probe when ultrahigh is picked.
• Read in full: 2 reviews, 4 comments, 0 inline comments (from the three saved files).
• New items since my last review: 4 author comments — the two replies to the round-1 reviews (18:00), the owner's answers (18:28) and the owner's decisions (18:51). I checked each claim against the code, as above. The 6 commits above.
• Read the whole round-2 diff (1,788 lines) and the changed files around each change: proxy/tooldefer.go, proxy/clientsignal.go, proxy/proxy.go (serve retry, Mux, Close), tooldefer/{apply,gate,history,probe,rewrite,tooldefer}.go, the plugin hook and both pickers, the docs.
• Verified the two new routes: the same checks as /signal (JSON only, authenticate, meter, tenant-scoped id); /tool-search not gated on keep-alive (pinned by test); /tool-defer/probe returns 202, holds no credential, and the ask starts the probe at the account's next Anthropic request through the existing prober (takeProbeAsk, bounded at 10,000 asks, dropped on a non-Claude model).
• Verified the retry change in serve: the 400 body is read (16 KB cap) and only a DeferralCause turns deferral off; the client's original is always re-sent; di is nil unless deferral changed the body (proxy/tooldefer.go:183), so an unchanged request is never sent twice.
• Verified the per-server rule: the grouping, the whole-unit veto (never-defer, keep list per tool or per server, forced, used in thread), the 200-token sum weighed on the original request, prefix matching in unusedIn and ProjectUsed, and the tests through the fake history and the real one.
• Verified the rewrite fixes: a moved mark can only land on an unmarked loaded client tool, never on a deferred one; the forced tool stays loaded with the names list byte-stable; Rewrite no longer touches the caller's set.
• Checked one candidate regression and rejected it: the flag moved from /signal to /tool-search, but the whole ENABLE_TOOL_SEARCH path is new in this PR, so no released hook or proxy can skew against it.
• Did not run builds or tests (not approved). The green -race runs are the author's reports; my dispositions come from reading the code.

[OsherElhadad]

Review of #861 at 2d3f1d1e — Osher

Verdict: request changes. Fix 4 items before merge (B1–B4). The rest can be follow-ups.

The design is implemented as agreed on #485. The live check is convincing, and the guards for the 400s are right.

I checked the PR in three ways:

• Reading: tooldefer/*, proxy/tooldefer.go, the proxy.go wiring, store/, /tool-search, the hook and the docs. A second reviewer read the probe, history, gate and ledger independently. I re-checked each of its findings below in the code; "inferred" marks the ones I did not.
• Tests: go vet ./tooldefer/ is clean. go test ./tooldefer/ -race -count=1 is ok.
• A live probe of this PR's exact names-list text and limit: 1 against the plain gateway, with a nonce on every request. I used 8 Claude Code-style deferred tools whose descriptions name each other (CronCreate/CronList/CronDelete, Enter/ExitWorktree, WebSearch/WebFetch, Monitor), and ran each task twice. Result: 8 of 8 used query = exact name, limit = 1, loaded only that tool and called it. Every request after the first read the cache (cr=15408).
• A replay of Decide on the hosted deployment's live DB (read-only snapshot, deleted after). Details in N10.

Blocking

#	Problem	Where	Failure	Fix
B1	A restart without the dashboard DB flips every live thread. Apply gates on Store.Persists(), but *Memory.Persists() is true (store/store.go:669). The disk copy exists only with --dashboard and without --upstreams (cmd/context-guru-proxy/main.go:119-124). The history is also memory-only without a DB (proxy/tooldefer.go:60-66)	tooldefer/apply.go:74, :114	After a restart, each live thread has no record. It takes the Rebuild branch, and Decide sees an empty history, so the set becomes "only what our search loaded". Every deferred tool moves to loaded in one request: a full rewrite of tools and everything after it, for every live conversation, on every restart. The probe showed that a loaded↔deferred move costs cw=18,509 / 25,950 on a 22k prefix. This is the normal setup for the local per-user proxy the owner is moving to	Gate deferral on a durable store: add Durable() bool to store.Store (true only for a Memory with a Disk), and turn ultrahigh's deferral off with a log line when it is false. Test: with a Memory-only store, ultrahigh defers nothing, so a restart cannot change tools; with a Disk, a restart leaves tools and messages[1] byte-identical (your live check, as a test)
B2	A SQLite write holds the lock that the request path takes. Ensure's goroutine calls p.Persist (SaveVerdict: CREATE TABLE + INSERT, 5 s timeout) inside p.mu (defer p.mu.Unlock() at probe.go:147). Verdict and Stale take p.mu on every new thread's first request (apply.go:98-101)	tooldefer/probe.go:146-157	When the dashboard DB is busy, the first request of every new ultrahigh thread waits up to 5 s. That breaks the PR's own claim that "the request path never waits on a query"	Copy k, v, at under the lock, unlock, then call Persist. Test with a blocking Persist, like TestForgetDoesNotPersistOnTheCaller
B3	A tenant purge leaves the history behind. dash/purge.go (PurgeTenant) does not delete tool_defer_history rows (tenant, session, project path, tool names), the cg:tdf: rows in session_state, or Mem.tenants[tenant]. The history has no age bound, only the 64-session count	dash/purge.go, tooldefer/history.go	An account delete keeps that account's project paths and tool names indefinitely. That conflicts with the data-dependence promise	Add DELETE FROM tool_defer_history WHERE tenant_id = ? to the purge, and drop m.tenants[tenant] through a hook. Delete rows with last_ts older than the session-state retention. Test it in the purge test
B4	The data-dependence row says "Nothing is shared between users", and that is not true today: (a) the seed runs only when the whole table is empty, and copies every tenant's sessions, including tenants that never picked ultrahigh (history.go:229-239; the ready map at :49 is unused); (b) the /tool-search opt-out map is one global map capped at 20,000 (gate.go:32, :70-75), so one tenant's posts can push out another tenant's opt-outs	tooldefer/history.go:229, tooldefer/gate.go:70	(a) Data of non-opted-in tenants is copied into a new table. A tenant who enables ultrahigh later gets no seed and waits 3 sessions. (b) Another tenant's ENABLE_TOOL_SEARCH=false stops applying	(a) Seed per tenant, on that tenant's first Apply, only for that tenant. (b) Key and cap the gate per tenant, and store a flag only for a session the proxy has seen. Then the doc row is true. The owner signed the row, so it needs this

High: fix in this PR if you can, else file them before merge

#	Problem	Where	Fix
H1	A 400 that DeferralCause does not recognise makes every later request go out twice. DeferralCause matches 5 keywords. Any other text only counts Retries and never breaks the gate, so each request sends the deferred body, gets the 400, then sends the original	proxy/tooldefer.go:167-180, tooldefer/gate.go:122	Pass the retry's status into deferRetry. If the original succeeds after the deferred one got a 400, that is the proof: Break + Forget. Keep the keyword list only for the log
H2	One flaky probe turns deferral off for 24 h. ProbeVerdict returns Fail for any 200 without a load and a call: the model only wrote text, hit max_tokens: 200 after a preamble, or stopped on pause_turn. Any non-200 other than 401/403/429/5xx is also Fail. A fresh Fail is final for ProbeTTL, and every new thread is decided "nothing"	tooldefer/probe.go:193-256	max_tokens 1,000. pause_turn and 404/408/409/413 give no verdict. Store a Fail only after 2 failures in a row, or keep a Fail for 15 minutes instead of 24 h
H3	The opt-out is lost easily. It is memory-only with a 24 h TTL. The hook posts only on source=startup, so --resume with ENABLE_TOOL_SEARCH=false is not covered. With claude -p, request 1 can reach the proxy before the detached hook (curl, 3 tries): the thread is decided deferred, then the flag lands and apply.go:84 sends the original. That is a mid-thread flip	tooldefer/gate.go:47-75, context-guru-plugin/scripts/signal.sh	Store the flag in the Store under a pinned metadata prefix. Post on resume too. When the flag arrives for a thread that already has a non-empty record, keep the record for that thread; the flag applies to new threads. Never flip a live thread
H4	The probe model name comes from the client. AnthropicModel checks only for the substring claude. Prober.m has no size cap and no concurrency cap. When the caller has no key, incomingModel falls back to the server key (inferred from incomingModel)	proxy/tooldefer.go:129, tooldefer/probe.go	Probe only models the price table knows (modelinfo). Cap in-flight probes (4) and the map (256)

Non-blocking (follow-up issues are fine)

#	Item	Fix
N1	flush sets dirty=false before the transaction and returns on any error (history.go:421-445). Under SQLite contention those sessions are dropped until they change again	Set it back to dirty on failure
N2	Every request moves s.last, which marks the session dirty, so each 5 s flush rewrites every tool row of every active session (history.go:360)	Dirty only on a set change. Write last_ts on a set change or at Close
N3	The project veto reads ≤ 10 project sessions out of the newest 64 per account. A user in 4+ projects, or with scripted claude -p runs, pushes a project's older sessions out (history.go:317, :387)	Keep the newest 10 per (account, project) as well
N4	Rebuild drift is more likely than "ended in between". Observe adds a session at its first request, so any session that merely started in between changes UserSessions. With B1 fixed, this only matters on a store miss (eviction at the pin cap)	When Rebuild gives a set different from what the transcript proves (our search's loads), send the original and do not save. Or accept it, but correct the PR text
N5	sniffer.bytes() is head + tail. For a 64–128 KiB response the windows overlap, and SearchesInSSE counts a search there twice (proxy/usage.go:725)	Scan the tail only past len(head)
N6	The 200-token minimum is in o200k tokens (tokens.Count), about 337 Claude tokens. The ledger scales by BilledDeltaFactor; the rule does not	Compare tokens.Count(t.Raw) * BilledDeltaFactor with 200, or write "200 o200k tokens" in the doc
N7	The net saving leaves out the cache write of a loaded definition on the next request	Add LoadedTokens × write price once per load
N8	The metrics #485 lists are missing: per-account counters, the wrong-guess count (a deferred tool that was then used: Observe already knows both sets), dropped parallel searches, and the keep-alive ledger credit	Add wrong_guesses and dropped_searches to the tool_defer block, per tenant. That is the number that checks the replay's 0.6%
N9	The seeded history misses exactly the tools this feature targets. tool_declarations lacks the built-ins Claude Code defers, and tool_uses lacks search loads. This is the dashboard fix #485 lists	Note it in the PR. Fix it in the dashboard follow-up
N10	Running sessions count as history. UserSessions takes the newest 3 by last activity, without checking that they ended. On the hosted DB (2026-10-28 to 11-02, 452 sessions), at 391 of 415 decisions (94%) at least one of the 3 was still running. Replayed: wrong guesses 24 (as in the PR) against 22 (ended sessions only), with the same volume deferred, because used is read only up to the decision. So this is fine as it is	Write it in a code comment, so nobody "fixes" it
N11	A thread whose first request already holds assistant turns (a fork, or a resume under a new session id) skips the first-request branch and goes to Rebuild (apply.go:105-117), with v == Pass from another thread's probe	Inferred. Add a test, or treat "no record + no search turn of ours" as a first request
N12	Smaller ones: Forget versus a probe in flight can write the verdict back after Forget removed it (probe.go:176). The probe sends no OAuth beta header, so direct Anthropic with a subscription token may 401 for ever (inferred). signal.sh:39 matches "startup" anywhere in stdin. false/0/no/off is wider than the design's literal false	Fine to file as one issue

Not in this PR, as declared

The following are not in this PR, as the PR itself says: save-more tools, the dashboard changes, the 40-task accuracy test, and a live run with Claude Code's own search on. Before ultrahigh is offered in the picker by default, run the 40-task test and the edge-case e2e from #485: a restart with and without --dashboard (B1), /compact, --resume, a subagent, and a parallel search with a tool call.

🤖 Generated with Claude Code

[amiddavid]

Reply to reviewer 1, round 2. Both findings are fixed in 039ddff5 (head).

Finding 1. /tool-search and /tool-defer/probe ignored CONTEXT_GURU_CLIENT_SIGNALS=off.

• A new helper, clientSignalsSwitchOn, reads only the operator's switch (off, 0, false turn it off), without the keep-alive half of clientSignalEnabled.
• clientSignalEnabled now uses it too, so the switch is read in one place.
• Both routes return 404 when the switch is off.
• Test: TestToolDeferRoutesObeyTheClientSignalsSwitch. With the switch off, both routes return 404 and change nothing. With keep-alive off and the switch on, both still work (204 and 202).
• Mutation: a helper that always returns true fails the test.

Finding 2. The opt-out was sent only on SessionStart startup.

• signal.sh now sends the opt-out to /tool-search on startup, resume, fork and clear: every source that can start a new session id.
• resume and clear still send their keep-alive event to /signal as well.
• compact keeps the session id, so it sends only its /signal event.
• The startup hook group now matches startup|fork.
• Without the opt-out, startup and fork still send nothing.
• Test: TestSignalHookSendsToolSearchOffOnEverySessionStart covers each source, the body sent to each route, and no extra request.
• Mutation: sending on startup only fails the test.

Notes taken:

• /stats tool_defer gains turned_off, which counts only the 400s that named a deferral cause. retries_400 still counts every 400/422 retry on a changed request, and a comment says so.
• A comment in signal.sh says why the 16-character cut cannot turn a long value into an off value.

Tests at 039ddff5 (box 2, -race -count=1): the library set (context-guru-plugin included), then ./proxy/... (470 s). All ok.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	2 of 2	3 of 3	039ddff5920c	blocking

Verdict: blocking

The new commit 039ddff5 is good: it fixes both of reviewer 1's round-2 findings, and I found no defect in it. But the owner's review (OsherElhadad, 19:14:43Z, request changes) lists four blocking items, B1–B4. I checked each one myself. All four are real, and none is fixed at head. That is why this round blocks.

Findings

1. tooldefer/apply.go:74 — agree with the owner's review, B1: a restart without the dashboard database flips every live deferred thread. Not fixed at head.

   • What fails: the default local proxy runs with the dashboard off. Its store is memory-only, but (*Memory).Persists() returns true (store/store.go:669), so Apply's gate at apply.go:74 passes and deferral runs. The disk copy attaches only with the dashboard on, a file database, and not in hosted mode (cmd/context-guru-proxy/main.go:618-648). After a restart the memory store is empty and the history is empty too (tooldefer built it from h.rec, which is nil). Every live thread loses its record.
   • Proof: I traced both mid-thread branches. A thread with one of our search turns takes Rebuild (apply.go:114). Decide sees an empty history and returns nothing, so the set is only the tools our search loaded (tooldefer/tooldefer.go:339-357) — every other deferred tool goes back to loaded. A thread with no search turn and no verdict takes the default branch (apply.go:118-123) and stores Record{}: decided nothing, saved, permanent for that thread — even after the probe passes again, the thread never defers. Either way tools changes, and tools hashes first, so the provider rewrites the whole cached prefix. The owner measured that one loaded↔deferred move costs about 18,500–25,950 cache-write tokens on a 22k prefix.
   • Fix: as the owner proposed — gate deferral on a store that is durable across a restart (a Durable() check, true only for a Memory with a Disk attached), and turn deferral off with a log line when it is not.

2. tooldefer/probe.go:146-158 — agree with the owner's review, B2: a probe's database write holds the mutex the request path takes. Not fixed at head.

   • What fails: Ensure's background goroutine locks p.mu and calls p.Persist before it unlocks (defer p.mu.Unlock() at probe.go:147 spans the Persist call at :157). Persist is Mem.SaveVerdict: CREATE TABLE plus INSERT, under a 5-second context (tooldefer/history.go:85-106). Verdict and Stale take the same mutex on the first request of every new thread (apply.go:98-101). A busy SQLite file can then stall that request up to 5 seconds.
   • Proof: this is the same contract my round-1 finding 1 set for Forget: its persist was moved off the caller, but Ensure's stayed under the shared lock. New proof of the frequency: the round-2 stale-verdict change re-probes every key once a day (apply.go:101), and each re-probe's persist holds the lock — the fix for one finding made this one fire more often.
   • Fix: copy k, v, at under the lock, unlock, then call Persist. Test it with a blocking Persist, as TestForgetDoesNotPersistOnTheCaller does.

3. dash/purge.go:126-143 — agree with the owner's review, B3: a tenant purge leaves the tool-deferral data behind. Not fixed at head.

   • What fails: PurgeTenant deletes five tables: archived_sessions, request_content, request_components, requests, tenant_spend. It does not delete tool_defer_history (tenant, session id, project path, tool names), tool_defer_probe (upstream URLs, models, routes), or the cg:tdf: rows in session_state. Mem.tenants also keeps the account in memory until restart.
   • Proof: I read the whole purge. New proof: TenantHasRows (purge.go:163-170), the deletion path's own "no orphans left" check, counts only requests, archived_sessions and tenant_spend — so the check cannot even see the leftover tool-defer rows. The data-dependence doc promises this data stays per user; a deleted user's project paths and tool names stay forever, because the table has no age bound, only the 64-session count.
   • Fix: add DELETE FROM tool_defer_history WHERE tenant_id = ? and the same for tool_defer_probe to the purge, drop Mem.tenants[tenant] through a hook, and count the new tables in TenantHasRows.

4. tooldefer/history.go:229-239 and tooldefer/gate.go:70-75 — agree with the owner's review, B4: the "nothing is shared between users" promise does not hold. Not fixed at head.

   • What fails, part (a): the seed runs only when the whole tool_defer_history table is empty, and then copies every tenant's sessions out of the dashboard tables. Tenants that never picked ultrahigh have their tool names and project paths copied into the new table. A tenant that enables ultrahigh later gets no seed at all, and waits 3 sessions.
   • Proof (a): if len(byKey) == 0 { seed(...) } keys on the global table, not per tenant. New proof: Mem.ready (history.go:49) is declared as "tenants loaded from the database (or known empty)" and never read anywhere — the per-tenant load was designed and not done; the dead field is the trace of it.
   • What fails, part (b): the /tool-search opt-out map is one global map capped at 20,000 entries across all tenants (gate.go:33, :72-75). In hosted mode the keys are tenant-prefixed, but the cap and pruneOldest are global, so one tenant's session starts can evict another tenant's opt-outs.
   • Fix: as the owner proposed — seed per tenant on that tenant's first use, and key and cap the gate per tenant.

Non-blocking

• The new commit 039ddff5 itself: no defect found. I read it as a first review. clientSignalsSwitchOn reads only the operator's switch; clientSignalEnabled now uses it too, so the switch is read in one place. signal.sh now emits one body per line with its route, and the loop parses them correctly (${line%% *} / ${line#* } — a route has no spaces, and json.dumps emits no raw newlines). The opt-out goes out on startup, resume, fork and clear; compact keeps only its /signal event; without an off value, startup and fork send nothing. hooks.json gains fork in the signal group, and start-proxy.sh already runs for every SessionStart source (no matcher). turned_off counts only the deferral-cause 400s, and the comment separates it from retries_400.
• The owner's H1–H4: I checked each and agree they are real. An unrecognized 400 makes every later request go out twice (H1, the DeferralCause keyword list at gate.go:122); one flaky probe reads as Fail for 24 hours (H2, ProbeVerdict fails any 200 without a load and a call, and max_tokens is 200); the opt-out is memory-only with a 24 h TTL and can land after claude -p's first request (H3); the probe model name comes from the client and Prober.m has no caps (H4). The owner's own triage — fix in this PR if you can, else file before merge — is the right one; I do not repeat them as findings.
• My round-2 note on MCP server names that differ only after a __ stands, and is still open. It stays minor: such names are exotic, and the grouping errs toward keeping tools loaded.

Earlier findings

Reviewer 1, round 2, finding 1 (the routes ignored CONTEXT_GURU_CLIENT_SIGNALS=off): fixed in 039ddff5. I verified it myself. clientSignalsSwitchOn (proxy/clientsignal.go:200-207) reads only the operator's switch; both /tool-search and /tool-defer/probe return 404 when it is off, and still work with keep-alive off. TestToolDeferRoutesObeyTheClientSignalsSwitch pins both directions. This was also my round-2 non-blocking note 1; the fix closes it.

Reviewer 1, round 2, finding 2 (the opt-out was sent only on startup): fixed in 039ddff5. I verified it myself. signal.sh now sends the opt-out on every SessionStart source that can start a new id — startup, resume, fork, clear — and TestSignalHookSendsToolSearchOffOnEverySessionStart covers each source, the body on each route, and that nothing extra is sent. A note on my own round 2: I read the kind == "startup" narrowing and accepted it; I missed that a resume or fork starts a new session id and loses the flag. Reviewer 1 caught it; the fix is right.

The owner's review (B1–B4): not fixed at head — that is my findings 1–4 above. The author's reply at 19:20:59Z answers reviewer 1's round-2 findings only; there is no reply to the owner's review yet.

My round-1 and round-2 items: all fixed or accepted in earlier rounds, as recorded in my round-2 review. Nothing reopened.

What I checked

• Last review: round 2, 8453212ddfa755430ea77ff665e65ddc65499d2b, 2026-11-02T19:01:55Z.
• New commits read: 1 — 039ddff5 fix(proxy,plugin): obey the client-signals switch; resend the opt-out (#861 round 2).
• Read in full: 5 reviews, 5 comments, 0 inline comments (from the three saved files).
• New items since my last review: reviewer 1's round-2 review (19:00:51Z, blocking, 2 findings — both now fixed in 039ddff5); the owner OsherElhadad's review (19:14:43Z, request changes, B1–B4 blocking, H1–H4 high, N1–N12 non-blocking); the author's reply to reviewer 1 (19:20:59Z); the commit 039ddff5. I checked each one against the code, as above.
• Read the whole round-3 diff (326 lines) and the changed files around each change: proxy/clientsignal.go, proxy/tooldefer.go, tooldefer/gate.go, context-guru-plugin/scripts/signal.sh, hooks.json, and both test files.
• Verified B1 end to end: (*Memory).Persists() is true (store/store.go:669); the disk store attaches only with the dashboard on, a file database, session-state on, and not hosted (cmd/context-guru-proxy/main.go:618-648); hosted multi-tenant mode is memory-only per tenant by design (the warning at main.go:626). Traced both mid-thread branches of Apply after a record loss.
• Verified B2 in tooldefer/probe.go:143-159 and tooldefer/history.go:85-106; B3 in dash/purge.go and TenantHasRows; B4 in tooldefer/history.go:229-239 (seed), the dead ready field, and tooldefer/gate.go:33, :62-76.
• Spot-checked the owner's H1–H4 premises in gate.go:122, probe.go:193-230, and proxy/tooldefer.go:149-204; each matches the code as I read it in earlier rounds.
• Did not run builds or tests (not approved). The green -race runs are the author's reports; my dispositions come from reading the code.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	1 of 2	3 of 3	039ddff5920c	blocking

Verdict: blocking. My two round-2 findings are fixed, and the new commit is clean under a first-review read. But @OsherElhadad's four blocking items (B1–B4) are real: I verified each one in the code myself, and none is fixed at head 039ddff5. The commit after that review addresses only my round-2 findings.

Findings

I checked each of Osher's blocking items myself. I agree with all four. Each is still open at head. My own proof follows each claim; the numbering is Osher's.

1. Agree with Osher B1, still open — a restart without the dashboard database flips every live thread to fully loaded, and the data-dependence row promises the opposite

   • What fails: the proxy runs without --dashboard (the default) or with --upstreams. The session-state store is then memory-only, but (*Memory).Persists() returns true (store/store.go:669), so the gate at tooldefer/apply.go:74 lets deferral run. A restart empties the memory. Every live thread loses its cg:tdf: record and its history. The next request takes the Rebuild branch (apply.go:114-117).
   • Proof: Decide returns nil when the history holds fewer than 3 sessions (tooldefer/tooldefer.go:123-125). An empty history is exactly that, so Rebuild returns at most the tools a search loaded (tooldefer/tooldefer.go:339-357). len(rec.Deferred) == 0 then returns the body unchanged (apply.go:132-136): every deferred tool goes out loaded, and the names list at messages[1] is gone. That rewrites tools and everything after it, for every live conversation, on every restart. New proof beyond Osher's: the toolfilter row in docs/data-dependence.md:187 says, for storage off, "defer: auto defers nothing: with no Store there is no fixed set per thread, and a set that can move would rewrite the cache." A memory-only store is "no Store" in exactly that sense, and the set does move. The row the owner signed is not true of the code.
   • Fix: Osher's — gate deferral on a durable store (Durable() on store.Store, true only for a Memory with a Disk), with a log line when it is false.

2. Agree with Osher B2, still open — a probe's database write holds the mutex the request path takes

   • What fails: Ensure's background closure locks p.mu and keeps it (a defer unlock) across p.Persist(k, v, e.at) (tooldefer/probe.go:146-157). Persist is SaveVerdict: a CREATE TABLE plus an INSERT under a 5-second timeout. Verdict and Stale take the same p.mu on the request path (tooldefer/probe.go:110-125), on the first request of every new ultrahigh thread with no record. A busy dashboard database then stalls that request for up to 5 seconds.
   • Proof: the trace above, read in the current code. The package already holds this rule for Forget: round 2 moved its persist onto the Go runner for the same reason (commit 0d1dff55). Ensure still breaks it.
   • Fix: copy k, v, e.at under the lock, unlock, then call Persist. Test it with a blocking Persist, like TestForgetDoesNotPersistOnTheCaller.

3. Agree with Osher B3, still open — a tenant purge leaves the deferral history and the records behind

   • What fails: PurgeTenant deletes archived_sessions, request_content, request_components, requests and tenant_spend (dash/purge.go:130-135). It never touches tool_defer_history, and it has no session_state handling at all (zero matches in the file), so the tenant's cg:tdf: rows survive too. The history row holds tenant, session, project path and tool names, with no age bound beyond the 64-session count.
   • Proof: the DELETE list above, read in the current code. An account delete keeps that account's project paths and tool names forever. That conflicts with the data-dependence promise for the row (docs/data-dependence.md:218).
   • Fix: Osher's — add DELETE FROM tool_defer_history WHERE tenant_id = ? to the purge, drop the in-memory tenant through a hook, and delete the cg:tdf: rows with the rest of the session state.

4. Agree with Osher B4, still open — "Nothing is shared between users" is not true of the seed or the gate cap

   • What fails, part (a): Mem.load reads the whole tool_defer_history table with no tenant filter (tooldefer/history.go:197-198). When it is empty, seed copies every tenant's sessions from tool_declarations and tool_uses, again with no tenant filter (tooldefer/history.go:257-296). So the first boot after this PR ships copies the tool names of users who never picked ultrahigh into the feature's table. A user who enables ultrahigh later gets no seed at all: the table is no longer empty.
   • What fails, part (b): searchOff is one global map capped at 20,000 with pruneOldest by time (tooldefer/gate.go:32, :70-77). One tenant's volume can evict another tenant's ENABLE_TOOL_SEARCH=false. That user's opt-out then silently stops applying.
   • Proof: both traces above, read in the current code. The row the owner confirmed says "Nothing is shared between users" (docs/data-dependence.md:218). The code makes tenants share the seed copy and the cap.
   • Fix: Osher's — seed per tenant, on that tenant's first Apply, only for that tenant; key and cap the gate per tenant.

Non-blocking

• Test report for 039ddff5 — the author ran the library set and ./proxy/..., but not ./dash/... (earlier rounds did). The commit touches no dash code, and dash's Go code does not read the tool_defer stats keys, so the risk is low. The run is still not on record for this head.
• Osher's H1–H4 and N1–N12 — I read them all. I agree with the dispositions: fix in a follow-up or before ultrahigh ships by default, not blocking here. Two notes: H3's resume half is now fixed (that was my round-2 finding 2); the claude -p start race and the memory-only 24-hour flag remain open. H1's double-send on an unrecognised 400 is real and stays until the retry's outcome is used as proof.
• Reviewer 2's round-2 note (two MCP servers whose names differ only after a __ share one unit) — I agree, exotic, keep as a follow-up.
• seed's doc comment says "the newest MaxSessions sessions per tenant", but the query has no limit and no newest-first bound (tooldefer/history.go:254-259). Small doc mismatch; the flush path bounds the table later.

Earlier findings

• My round-2 finding 1 (CONTEXT_GURU_CLIENT_SIGNALS=off bypassed): fixed at 039ddff5. clientSignalsSwitchOn reads the operator's switch alone; clientSignalEnabled uses it too, so /signal keeps its old behavior exactly. Both new routes return 404 when the switch is off. TestToolDeferRoutesObeyTheClientSignalsSwitch pins off → 404 with no state change, and on with keep-alive off → working.
• My round-2 finding 2 (the opt-out lost on resume or fork): fixed at 039ddff5. The hook now sends the opt-out on startup, resume, fork and clear — every source that can start a new session id. resume and clear keep their /signal event; compact correctly sends only that. TestSignalHookSendsToolSearchOffOnEverySessionStart covers each source, both bodies, and no extra request.
• My round-2 non-blocking notes: taken. turned_off counts only the 400s that named a deferral cause (Break adds both counters, the other path adds Retries only — the /stats comment now says what each holds). The 16-character cut has its comment in signal.sh.
• Reviewer 2, round 2: approve, no findings. Their two non-blocking notes: the switch bypass (fixed above, same commit) and the MCP unit note (open, agreed above).
• Osher's review (request changes, B1–B4): not fixed. 039ddff5 landed after that review and addresses none of B1–B4. See findings 1–4 above.

What I checked

• Last review: round 2, sha 8453212ddfa755430ea77ff665e65ddc65499d2b, posted 2026-11-02T19:00:51Z.
• New commits read: 1 — 039ddff5 fix(proxy,plugin): obey the client-signals switch; resend the opt-out (#861 round 2). Full diff 8453212d..039ddff5, plus each changed file around the changes.
• Read in full: 5 reviews, 5 comments, 0 inline comments (from the three saved files).
• New items since my last review, and what I did:
  • Reviewer 2's round-2 review (19:01:55Z, approve). Read; their fix dispositions match mine.
  • Osher's review (19:14:43Z, request changes: B1–B4, H1–H4, N1–N12). Read in full. Verified B1–B4 myself in the code (findings 1–4); read H1–H4 and N1–N12 and agree with the dispositions.
  • The author's reply to my round-2 review (19:20:59Z). Read; both fix claims checked against the code.
  • Commit 039ddff5. Reviewed as a first review; no defect found:
    • clientSignalsSwitchOn is the operator's switch alone; clientSignalEnabled = switch AND not keep-alive-off, so /signal behaves exactly as before. Both new routes check the switch first and return 404.
    • signal.sh now emits one "route body" line per event. The parse (route="${line%% *}") is safe: the route token holds no space, and json.dumps always puts one after it. Only a whole off value passes the shell gate, so the 16-character cut cannot fabricate one. startup and fork send only the opt-out (nothing without it); resume and clear send /signal plus the opt-out; compact sends only /signal. The startup|fork matcher change leaves the start-proxy group untouched.
    • The turned_off counter: Break adds Retries and TurnedOff; a 400 without a deferral cause adds Retries only. The /stats comment matches.
• Verified my two round-2 fixes by reading the changed code and the new tests, not by running them.
• Did not run builds or tests (not approved). Findings above are from reading the code only.

[amiddavid]

@OsherElhadad thank you. All 4 blocking items and all 4 high items are fixed in c2b7077b (head). N11 and the signal.sh match are fixed too. The other N items are in #866.

Blocking

• B1, a restart flipping live threads.
  • store.Durable(s) is true only for a Memory with a Disk. It is an optional interface rather than a new Store method, so a host's own Store keeps compiling.
  • When the session store is not durable, ultrahigh keeps each thread's record and the opt-out in its own file, context-guru-tooldefer.db, next to the dashboard database path. The file is opened at the first ultrahigh request that needs it, so other setups create no file. It also holds the history and the probe verdicts when there is no dashboard database.
  • With no durable place at all (a read-only folder, --dashboard-db=:memory:), ultrahigh defers nothing and logs one line saying why.
  • Tests: TestToolDeferSurvivesRestart (memory-only store, restart: tools and messages[1] are byte-identical with no history and no verdict carried over), TestApplyRestartKeepsTheRecordInTheFile, TestApplyNoDurablePlaceDefersNothing, TestToolDeferNoDurablePlaceDefersNothing (also: with a durable session store, the record goes there as today).
• B2, the database write under p.mu. Ensure copies the fields, unlocks, then calls Persist. A Forget during a probe now voids that probe's answer (your N12 race). Tests: TestEnsurePersistsOutsideTheLock, TestForgetVoidsAProbeInFlight.
• B3, the purge.
  • PurgeTenant deletes the tenant's tool_defer_history rows and its cg:tdf: and cg:tso: keys in session_state. The host does the same in tool deferral's file, and drops the history and the pending opt-outs in memory.
  • TenantHasRows counts these rows.
  • The history deletes sessions idle longer than the session-state retention (30 days by default).
  • The probe verdicts stay: they describe an endpoint and a model, not a tenant.
  • Tests: TestPurgeTenantClearsToolDeferral (dash), TestToolDeferPurgeForgetsTheTenant, TestHistoryRetentionPrunes.
• B4, data shared between users.
  • (a) The seed copies one tenant's own sessions, at that tenant's first deferral request. The unused ready map is now seeded.
  • (b) An opt-out for a session the proxy has seen goes into that session's record store. One for an unseen session waits in a per-tenant set (64 each) until the session's first request.
  • Tests: TestHistorySeedsOneTenantFromDashboardTables, TestGatePendingOptOutIsPerTenant, TestToolDeferOptOutForUnseenSessionWaits.

High

• H1: the original succeeding after the deferred body got a 400 or 422 is the proof, so deferral is turned off whatever the text says. When the original fails too, deferral stays on. DeferralCause only labels the log line. Test: TestToolDeferRetryProofIsTheOriginalSucceeding.
• H2:
  • The probe's max_tokens is now 1,000.
  • pause_turn and a 404, 408, 409 or 413 give no verdict.
  • Of your two options, I chose a Fail that holds 15 minutes (FailTTL) instead of 2 failures in a row. A Pass still holds 24 hours.
  • Tests: TestProbeVerdicts, TestProbeFailIsShortLived.
• H3:
  • The opt-out lives in the record store under a pinned metadata prefix, cg:tso:, so a restart keeps it.
  • It blocks new decisions only: a thread that already has a record keeps it, so a flag that lands after request 1 (claude -p) never flips a live thread.
  • Since round 2, the hook sends the flag on startup, resume, fork and clear.
  • Test: TestApplyOptOutAfterFirstRequestKeepsTheThread.
• H4: only a model the price table knows is probed. At most 4 probes run at once, and the prober keeps at most 256 keys. Tests: TestToolDeferUnknownModelIsNotProbed, TestProberCaps.

Non-blocking

• N11: a thread with assistant turns but no record and no search of ours is a first request. A rebuild runs only when our own search turn is in the transcript. Test: TestApplyForkIsAFirstRequest.
• N12, the signal.sh match: the hook now parses the JSON source field (since round 2).
• The rest: N1–N10 and the rest of N12 are in Tool deferral follow-ups from the #861 review (N1–N10, N12) #866. That issue also notes a gap older than this PR: the purge does not remove the tenant's other session-state keys.

Tests at c2b7077b (box 2, -race -count=1): the library set (tooldefer, config, components/..., store, metrics, internal/modelinfo, cmd/..., context-guru-plugin), then ./proxy/... (486 s), then ./dash/... (526 s). All ok.

Mutations: 13, each one fails a test. One per B item and one per H item, plus a second for H2 and H4, and one for N11.

[amiddavid]

Reply to reviewer 2, round 3. Every finding in this review is one of Osher's B1–B4 or H1–H4. All are fixed in c2b7077b (head). The reply to Osher's review lists each item with its fix, its tests and its mutation: #861 (comment)

• B1, the restart flip (the Rebuild branch with an empty history): the records are now always on disk. That is the session-state table when it is durable, else tool deferral's own file. With no durable place, nothing is deferred. A rebuild now runs only when our own search turn is in the transcript.
• B2, Persist under p.mu in Ensure: it now runs after the unlock (TestEnsurePersistsOutsideTheLock; the mutation that moves it back under the lock fails the test).
• B3 and B4: the purge and the per-tenant seed and opt-outs.
• H1–H4: all four are fixed. For H2 I chose a Fail that holds 15 minutes.

Tests at c2b7077b (box 2, -race -count=1): the library set, ./proxy/... (486 s), ./dash/... (526 s). All ok.

[amiddavid]

Reply to reviewer 1, round 3. Every finding in this review is one of Osher's B1–B4 or H1–H4. All are fixed in c2b7077b (head). The reply to Osher's review lists each item with its fix, its tests and its mutation: #861 (comment)

• B1, the restart flip (the Rebuild branch with an empty history): the records are now always on disk. That is the session-state table when it is durable, else tool deferral's own file. With no durable place, nothing is deferred. A rebuild now runs only when our own search turn is in the transcript.
• B2, Persist under p.mu in Ensure: it now runs after the unlock (TestEnsurePersistsOutsideTheLock; the mutation that moves it back under the lock fails the test).
• B3 and B4: the purge and the per-tenant seed and opt-outs.
• H1–H4: all four are fixed. For H2 I chose a Fail that holds 15 minutes.

Tests at c2b7077b (box 2, -race -count=1): the library set, ./proxy/... (486 s), ./dash/... (526 s). All ok.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	1 of 2	4 of 6	c2b7077b9f91	blocking

Verdict: blocking. All four of Osher's blocking items (B1–B4) and all four high items (H1–H4) are fixed; I verified each one in the code. N11 is fixed too. The new commit holds up under a first-review read, with one small defect in the new purge code: a purge can miss tool deferral's own file.

Findings

1. proxy/tooldefer.go (purgeToolDefer) — a purge misses tool deferral's file when the deferral state was never built in this process, and the rows then stay forever
   • What fails: the proxy restarts. No ultrahigh request arrives after the restart, so toolDefer() never runs and h.defer_.file stays nil — the file opens only there (proxy/tooldefer.go, toolDefer). The operator purges an account. purgeTenantData calls h.purgeToolDefer, which reads h.defer_.file, finds nil, and returns 0 (proxy/tooldefer.go, purgeToolDefer). The tenant's cg:tdf: and cg:tso: rows and its tool_defer_history rows stay in context-guru-tooldefer.db on disk. Nothing else removes them: the history's retention delete runs only in a live history's flush, and an unopened file has no live history. Every later purge hits the same nil file, so the rows stay indefinitely while each purge reports success.
   • Proof: h.defer_.file is set in exactly one place, toolDefer() (proxy/tooldefer.go), which runs only from applyToolDefer — and only for a tenant whose pipeline defers (if !on { return }), on an Anthropic request. purgeToolDefer returns 0, nil when the file is nil, without looking at h.opts.ToolDeferPath. The data-dependence row now promises "An account purge deletes the user's history rows, thread records and opt-out flags" (docs/data-dependence.md, Tool deferral history). Reachable states: a deployment whose ultrahigh traffic stopped before the restart (the account being purged, by definition, stopped); --session-state=false with the dashboard on, where the records live only in the file.
   • Fix: in purgeToolDefer, when h.defer_.file is nil but h.opts.ToolDeferPath names a file that exists, open it (or run dash.PurgeToolDefer on a short-lived open) and purge there too. Skip a path that does not exist, so a purge creates no file.

Non-blocking

• In-memory store keys after a purge — with a durable session store, the purge deletes the session_state rows, but the live Memory keeps the tenant's cg:tdf: keys until the state TTL (24 h). The author's #866 already notes the older gap for the tenant's other session-state keys; this is the same class.
• broken is still one global map capped at 20,000 across tenants (tooldefer/gate.go). One tenant's 400s can evict another's turn-offs. B4(b) was about the opt-outs, and those are per tenant now; the turn-off cap is symmetric to before and expires in 24 h, so I only note it.
• startProbe with Prices == nil never probes (proxy/tooldefer.go), so ultrahigh silently defers nothing on a host with no price table. main.go always wires one, and the no_durable_store case got its log line — this case got none. One log line would make it visible.
• TestApplyMidThreadUnknownProbeSendsOriginal (tooldefer/tooldefer_test.go:487) — its comment still says "first seen mid-way … sends the original". Under the N11 change that request is now a first request and the thread is decided nothing, permanently. The test still passes; only the comment is stale.

Earlier findings

• Osher B1 (a restart flips every live thread): fixed. Env.RecordStore() picks a durable session store, else tool deferral's own file, else nil (tooldefer/apply.go). store.Durable is an optional interface, true only for a Memory with a Disk (store/store.go). The new tooldefer.File is a SQLite file with a metadata-only store.Disk, opened only when needed, with a write test at open so a read-only folder fails there. With no durable place, Apply returns no_durable_store and defers nothing, with one log line. I traced the wiring: hosted mode leaves h.store memory-only (main.go, the stateDB switch), so the file opens there; --dashboard-db=:memory: gives no path, so it defers nothing — both match the updated doc rows. Tests: TestApplyRestartKeepsTheRecordInTheFile, TestToolDeferSurvivesRestart (end to end, byte-identical tools and messages[1] after a real restart), TestApplyNoDurablePlaceDefersNothing, TestToolDeferNoDurablePlaceDefersNothing.
• Osher B2 (a probe's database write held p.mu): fixed. Ensure's closure copies e.at and p.Persist, unlocks, then persists (tooldefer/probe.go). TestEnsurePersistsOutsideTheLock blocks Persist and checks Verdict does not wait. Forget now bumps a generation, and a probe that finishes after it is void (TestForgetVoidsAProbeInFlight) — Osher's N12 race, closed too.
• Osher B3 (the purge left the deferral data): fixed, with the gap in finding 1. PurgeTenant now runs PurgeToolDefer: tool_defer_history by tenant, and the cg:tdf: / cg:tso: rows in session_state by byte range. I checked the range arithmetic against the key format (cg:tdf:<tenant>:<session>…): lo = prefix+tenant+":", hi = lo with the last byte +1, and the scoped-id charset ([A-Za-z0-9._:-]) cannot put another tenant's key inside the range — alice and alice2 do not collide. TenantHasRows counts the new tables. The host drops the memory (history, pending opt-outs, turn-offs) and the file. The history prunes sessions idle longer than the retention, in memory and on disk (TestHistoryRetentionPrunes). Probe verdicts stay, as Osher allowed: they describe an endpoint, not a tenant.
• Osher B4 (data shared between users): fixed. (a) The seed is per tenant, at that tenant's first deferral request, WHERE tenant_id = ? in both queries, and the merge keeps only that tenant's rows (SeedTenant; TestHistorySeedsOneTenantFromDashboardTables also checks the other tenant gets nothing). (b) A seen session's opt-out goes to the record store at once; an unseen session's waits in a per-tenant set (64 each) until its first request (NoteToolSearch, TakePending; TestGatePendingOptOutIsPerTenant, TestToolDeferOptOutForUnseenSessionWaits).
• Osher H1 (an unrecognised 400 sent every request twice): fixed. serve now sends the original first, then calls deferRetry with originalOK — the original's status under 400. That is the proof, whatever the error text; DeferralCause only labels the log. I checked the premise: di.orig is the body right before deferral's rewrite (deferral runs last), so the two rounds differ only by deferral. When the original fails too, deferral stays on. TestToolDeferRetryProofIsTheOriginalSucceeding covers both directions.
• Osher H2 (one flaky probe read as Fail for a day): fixed. max_tokens is 1000; pause_turn and a 404/408/409/413 give no verdict; a Fail holds FailTTL (15 minutes) through ttl(v) in Stale, Ensure and the freshness check, while a Pass keeps 24 hours. TestProbeFailIsShortLived, TestProbeVerdicts.
• Osher H3 (the opt-out was lost easily): fixed. The flag lives in the record store under the pinned cg:tso: prefix, so a restart keeps it. It blocks new decisions only: Apply reads it when the thread has no record, and a thread that already has one keeps it (TestApplyOptOutAfterFirstRequestKeepsTheThread). The hook has sent it on startup, resume, fork and clear since round 3.
• Osher H4 (the probe model came from the client, no caps): fixed. startProbe returns unless the price table knows the model; the prober keeps at most 256 keys and 4 probes in flight (TestToolDeferUnknownModelIsNotProbed, TestProberCaps).
• Osher N11 (a fork or resume went to Rebuild): fixed. A thread with assistant turns, no record and no search of ours is a first request; Rebuild runs only when our own search turn is in the transcript (TestApplyForkIsAFirstRequest, TestApplyLostRecordRebuildsTheSameSet).
• Osher N1–N10 and the rest of N12: deferred to #866 by the author. I read the dispositions and accept them; they match Osher's own "follow-up issues are fine" triage.
• Reviewer 2, round 3: their four findings were Osher's B1–B4; all fixed above. Their MCP unit note stays open as a follow-up, as agreed.
• My round-3 findings were Osher's B1–B4; fixed above.

What I checked

• Last review: round 3, sha 039ddff5920c4ca149d95461901fa71db6381f86, posted 2026-11-02T19:27:36Z.
• New commits read: 1 — c2b7077b fix(tooldefer): fix Osher's blocking and high findings on #861 (B1-B4, H1-H4). Full diff 039ddff5..c2b7077b (20 files, +1,266/−188), plus each changed file around the changes.
• Read in full: 7 reviews, 8 comments, 0 inline comments (from the three saved files).
• New items since my last review, and what I did:
  • Reviewer 2's round-3 review (19:26:23Z, blocking). Read; their findings are Osher's B1–B4, and I verified the fixes for all of them.
  • The author's three replies (20:21:16Z to Osher, 20:22:01Z to reviewer 2, 20:22:21Z to me). Read; each fix claim checked against the code, as recorded above.
  • Commit c2b7077b. Reviewed as a first review:
    • store.Durable (optional interface) and Memory.DropPrefix (drops memory and pending disk writes; the rows on disk are the purge's). Read both in full.
    • tooldefer.File: SQLite with WAL, busy_timeout, MaxOpenConns(4), a write test at open, and a metadata-only store.Disk under a store.Memory, so LoadSession loads one session's rows on demand. The two writers to the one file (the Disk and the history flush) share the pool with a busy timeout.
    • Apply's new shape: RecordStore(), the LoadSession call, the opt-out read (SearchOff in the store plus TakePending), the opt-out blocking new decisions only, and the first-request rule !HasAssistantTurn || !hasSearchTurn. Traced each branch against the old one.
    • serve's retry: the original is sent first, originalOK is its status under 400, and deferRetry breaks only on that proof. Checked that di.orig is the pre-deferral body and that the client still sees the original's own error when it fails.
    • The gate's pending opt-outs (per-tenant caps, TTL, taken once), DropTenant (pending, broken), and the probe's generation voiding. Read Ensure, Forget, Seed under the new caps.
    • The purge: dash.PurgeToolDefer, the byte-range deletes (arithmetic checked by hand against the key format and charset), TenantHasRows, and the host hook. Finding 1 lives here.
    • The history: SeedTenant (per tenant, background, k[0] == tenant guard on the merge), the retention prune in flush, DropTenant, Has.
    • The docs: both data-dependence rows and the presets row now describe the file, the per-tenant seed, the purge and the retention, as the code does — except the purge promise, which finding 1 breaks in one state.
  • The new tests, read in full: TestApplyRestartKeepsTheRecordInTheFile, TestApplyNoDurablePlaceDefersNothing, TestApplyLostRecordRebuildsTheSameSet, TestApplyForkIsAFirstRequest, TestApplyOptOutAfterFirstRequestKeepsTheThread, TestGatePendingOptOutIsPerTenant, TestHistorySeedsOneTenantFromDashboardTables, TestHistoryRetentionPrunes, TestProbeFailIsShortLived, TestEnsurePersistsOutsideTheLock, TestProberCaps, TestForgetVoidsAProbeInFlight, TestToolDeferSurvivesRestart, TestToolDeferNoDurablePlaceDefersNothing, TestToolDeferRetryProofIsTheOriginalSucceeding, TestToolDeferPurgeForgetsTheTenant, TestToolDeferOptOutForUnseenSessionWaits, TestToolDeferUnknownModelIsNotProbed, TestPurgeTenantClearsToolDeferral, and the updated TestToolDeferSignalDoesNotBuildState and TestToolDeferRoutesObeyTheClientSignalsSwitch.
• Did not run builds or tests (not approved). Findings above are from reading the code only.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	2 of 2	4 of 6	c2b7077b9f91	approve

Verdict: approve

The new commit c2b7077b fixes all four blocking items (B1–B4) and all four high items (H1–H4) from the owner's review, plus N11. I verified each fix in the code myself. I found no defect in the new code.

Findings

None.

Earlier findings

My finding 1 (= the owner's B1, a restart flipping every live thread): fixed in c2b7077b. I verified it myself.

• store.Durable is an optional interface, true only for a Memory with a disk attached (store/store.go:676-690). Env.RecordStore (tooldefer/apply.go:27) returns the durable session store, else the fallback file, else nil, and Apply defers nothing with Why: "no_durable_store" (apply.go:91). The proxy logs that once per process.
• The fallback is tool deferral's own file (tooldefer/file.go): one SQLite file, a metadata-only store.Disk, opened at the first ultrahigh request that needs it.
• New proof beyond the author's reply: the default local proxy keeps the file too. --dashboard defaults to false, but --dashboard-db defaults to ./context-guru-dashboard.db (cmd/context-guru-proxy/main.go:136), so ToolDeferPath is set even with the dashboard off (main.go:656-658). The owner's "normal setup for the local per-user proxy" is covered, not only the dashboard-on one.
• I traced the record path end to end: Save/Load key on cg:tdf: + the scoped session, and Memory.LoadSession (store/disk.go:425) restores a session's rows from the disk copy, with boundaryOK accepting the record's key shape. So a restarted proxy reads the thread's record back before Load.

My finding 2 (= B2, a probe's database write under p.mu): fixed in c2b7077b. Ensure copies the verdict and the time under the lock, unlocks, and calls Persist after it (tooldefer/probe.go:186-193). Verdict and Stale take only p.mu. Forget now also voids a probe that is in flight when it runs, through a generation counter and a p.m[k] != e check (probe.go:168, :178) — that closes the owner's N12 race as well.

My finding 3 (= B3, the purge leaving tool-deferral data behind): fixed in c2b7077b. dash.PurgeToolDefer (dash/purge.go:179) deletes the tenant's tool_defer_history rows and its cg:tdf: and cg:tso: key ranges in session_state; PurgeTenant calls it, and TenantHasRows counts the same rows, so the "no orphans left" check can see them now. The proxy's purgeTenantData also purges tool deferral's own file and the in-memory state, and does so even with no dashboard (proxy/control.go:2076). The history deletes sessions idle longer than the session-state retention, in memory and on disk.

My finding 4 (= B4, data shared between users): fixed in c2b7077b. Part (a): SeedTenant copies one tenant's own sessions, at that tenant's first deferral request, in the background; the seed queries filter tenant_id, and the dead ready field is now the seeded map it was meant to be. Part (b): an opt-out for a session the proxy has seen goes into that session's record store at once; one for an unseen session waits in a per-tenant set capped at 64 (10,000 tenants), so one tenant cannot push out another's.

The owner's H1–H4: fixed, each verified in the code.

• H1: serve now sends the client's original first and passes the outcome to deferRetry (proxy/proxy.go:1973-1980). The original succeeding is the proof: Break and Forget run whatever the provider's text says. When the original fails too, deferral stays on. DeferralCause only labels the log line (proxy/tooldefer.go:296-315).
• H2: the probe's max_tokens is 1,000; a 404, 408, 409, 413 or pause_turn gives no verdict; a Fail holds 15 minutes (FailTTL), a Pass 24 hours (tooldefer/probe.go).
• H3: the opt-out lives in the record store under the pinned cg:tso: prefix, so a restart keeps it. It blocks new decisions only: a thread that already has a record keeps it (tooldefer/apply.go:112-121), so a flag that lands after claude -p's first request never flips a live thread.
• H4: only a model the price table knows is probed (proxy/tooldefer.go:276-281); at most 4 probes run at once and the prober keeps at most 256 keys (tooldefer/probe.go:39-48).

N11: fixed. A thread with assistant turns but no record and no search of ours is a first request; a rebuild now needs our own search turn in the transcript (tooldefer/apply.go:132-149).

Reviewer 1's round-3 findings were the same B1–B4 items. All fixed, as above.

The follow-up issue is real: #866 is open, filed by the author at 20:20:19Z, and holds N1–N10 and the rest of N12.

Non-blocking

• tooldefer/history.go (flush) — the retention delete runs only in a flush that has dirty rows. The function returns early when no session is dirty, before the disk-side DELETE ... WHERE last_ts < cutoff. On a fully idle proxy, rows older than the retention stay until any session changes. Bounded by the purge and by any later activity.
• The dashboard purge leaves the live session-store memory as it is. It deletes the tenant's cg:tdf:/cg:tso: rows on disk, but the session store's memory keeps its pinned keys until the process ends. They are unreachable after the account delete (nothing can authenticate), and #866 already notes the older gap for the other session-state keys.
• Two small wording slips. toolDefer()'s comment names Options.ToolDeferFile; the field is ToolDeferPath (proxy/tooldefer.go:91). The no-durable log line says the file "could not be opened", also when no path was configured at all (--dashboard-db="").
• My round-2 note on MCP server names that differ only after a __ stands, as a follow-up.

What I checked

• Last review: round 3, 039ddff5920c4ca149d95461901fa71db6381f86, 2026-11-02T19:26:23Z.
• New commits read: 1 — c2b7077b fix(tooldefer): fix Osher's blocking and high findings on #861 (B1-B4, H1-H4).
• Read in full: 7 reviews, 7 comments, 0 inline comments (from the three saved files).
• New items since my last review, each read and checked: the commit c2b7077b; the author's reply to the owner (20:21:16Z), to me (20:22:01Z) and to reviewer 1 (20:22:21Z); issue #866 (20:20:19Z, verified open and covering N1–N10 and N12). No new comment from the owner.
• Read the whole round-4 diff (2,314 lines) and the changed files around each change: the new tooldefer/file.go, store/store.go and store/disk.go (Durable, DropPrefix, LoadSession, getGlobal), tooldefer/{apply,gate,history,probe,tooldefer}.go, proxy/tooldefer.go, proxy/control.go, proxy/proxy.go (the retry), dash/purge.go, cmd/context-guru-proxy/main.go, both docs, and the new tests.
• Verified B1 end to end: the Durable gate, RecordStore, the file's open condition in toolDefer() against the three real setups (dashboard on, dashboard off, hosted), the --dashboard-db default that gives the default local proxy a file path, and the record key shape against LoadSession/boundaryOK.
• Verified the purge on both stores: the key ranges in toolDeferPurge against session.Scoped's <tenant>: shape, the TenantHasRows count built from the same queries, DropPrefix (memory and pending disk writes), and that purgeTenantData is reachable only with a real tenant id (destructiveTarget resolves the account from the registry).
• Verified the retry: round 0 only, the deferred response's body read and closed before the resend, di cleared so the ledger skips the refused round, and the client sees the original's own 400 when the original fails too.
• Checked the pending opt-out flow both ways: a seen session stores at once through noteToolSearch (which never builds the state — deferPeek first, deferEnv only when the state exists); an unseen session's flag waits and is applied at its first request by TakePending inside Apply, before the record load.
• Checked candidate regressions and rejected them: an unpriced model now never defers (Unknown verdict, Record{} per thread) — that is the owner's H4 rule, not a defect; a mid-thread record loss with no search turn is decided as a first request — with a Pass verdict the old code rebuilt a fresh set too, so the exposure is not new, and the owner asked for the fork behavior in N11.
• Did not run builds or tests (not approved). The green -race runs are the author's reports; my dispositions come from reading the code.

[amiddavid]

Reply to reviewer 1, round 4. The finding is fixed in 46172250 (head).

Finding 1. A purge missed tool deferral's file when this process had not opened it.

• purgeToolDefer now handles a file that exists at ToolDeferPath but is not open: it opens the file on its own connection (tooldefer.OpenForPurge), deletes the tenant's rows, and closes it.
• OpenForPurge uses mode=rw, so a purge never creates the file. A path with no file is skipped.
• Test: TestToolDeferPurgeReachesAnUnopenedFile. It writes a tenant's record, opt-out and history rows into the file, then starts a handler with no ultrahigh request and purges the tenant. The rows are gone, another tenant's record stays, and the call reports at least 3 rows. A purge with no file returns 0 and creates no file.
• Mutations: putting the early return back fails the test. Letting the open create the file also fails it.

Non-blocking notes:

• In-memory keys after a purge: taken. The purge drops the tenant's cg:tdf: and cg:tso: keys from the process session store in memory too.
• startProbe with no price table: taken. One log line now says that ultrahigh defers nothing then.
• The stale comment on TestApplyMidThreadUnknownProbeSendsOriginal: fixed.
• The global broken map: not taken. Per-tenant caps there are more than a one-line change, and an entry expires in 24 h. I added it to Tool deferral follow-ups from the #861 review (N1–N10, N12) #866.

Tests at 46172250 (box 2, -race -count=1): ./tooldefer/, ./store/, ./config/, ./components/all/, then ./proxy/... (479 s), then ./dash/... (518 s). All ok.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 1 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	1 of 2	5 of 6	4617225001f9	approve

Verdict: approve. My round-4 finding is fixed, and I verified the fix in the code. The commit also takes three of my four non-blocking notes. I found no defect in the new code.

Findings

None.

Non-blocking

• The purge can now fail where it always succeeded before (proxy/control.go, purgeTenantData). When a file exists at ToolDeferPath but cannot be opened (a permission error, corruption), purgeToolDefer returns the error and the account purge reports failure. I accept this: the doc promises deletion, and a purge that reports success while rows stay is the worse failure. The dashboard rows are deleted before the error surfaces, so a retry completes it.
• The two wording slips reviewer 2 raised in round 4 are still open: the comment names Options.ToolDeferFile where the field is ToolDeferPath (proxy/tooldefer.go:92), and the no-durable log line says the file "could not be opened" also when no path was configured. Still agree with reviewer 2: comment-level only.
• Reviewer 2's round-4 flush note still stands: the history's retention delete runs only in a flush that has dirty rows, so a fully idle proxy keeps rows past the retention until any session changes. Bounded by the purge and by later activity. Accepted in round 4; unchanged.

Earlier findings

• My round-4 finding 1 (a purge missed tool deferral's file when this process had not opened it): fixed in 46172250. I verified it myself:
  • purgeToolDefer now falls through to Options.ToolDeferPath when the file is not open (proxy/tooldefer.go:182-197). It stats the path, skips a path with no file, opens the file with tooldefer.OpenForPurge, and runs dash.PurgeToolDefer on it.
  • OpenForPurge opens with mode=rw, so a purge never creates the file (tooldefer/file.go:77-87). db.Ping makes the open real before any query.
  • dash.PurgeToolDefer guards each table with tableExists, so a file created but never flushed (no tool_defer_history table yet) is not an error.
  • I checked the coverage this adds beyond my finding: a deployment that once ran without a dashboard (records in the file) and now runs with one (records in the session state) gets BOTH purged — the session-state rows by the dashboard purge, the old file by the new path.
  • Test: TestToolDeferPurgeReachesAnUnopenedFile. It writes a tenant's record, opt-out and history rows into the file, restarts the handler with no ultrahigh request, purges, and checks: at least 3 rows reported, the tenant's rows gone, another tenant's record still there, the file still not open as the deferral state, and a purge with no file creates none.
• My round-4 non-blocking notes:
  • In-memory store keys after a purge: taken. The purge now drops the tenant's cg:tdf: and cg:tso: keys from the process session store (proxy/tooldefer.go:170-173). Memory.DropPrefix locks m.mu itself (store/disk.go:540-565), so it is safe against a concurrent request on the same store, and it also clears the pending disk writes. This closes most of reviewer 2's round-4 note on the live session-store memory, for the deferral prefixes.
  • startProbe with no price table: taken. One log line, once per process (noPricesOnce, proxy/tooldefer.go:299-303).
  • The stale test comment: fixed (tooldefer/tooldefer_test.go:486-487).
  • The global broken cap: not taken, moved to #866. I verified the item is there: issue #866 comment at 20:56:30Z names Gate.broken, the 20,000 global cap, and the per-tenant fix. Accepted — the entries expire in 24 h and the change is more than a one-liner.
• Reviewer 2, round 4 (approve): I read their review. Their fix dispositions match mine (B1–B4, H1–H4, N11 all fixed in c2b7077b). Their non-blocking notes are accounted for above.
• Osher's B1–B4, H1–H4 and N11: fixed in c2b7077b, verified in my round-4 review. 46172250 touches only the purge, the log line and test comments, so those dispositions stand.
• Osher N1–N10 and the rest of N12: in #866 (open, verified). Accepted.

What I checked

• Last review: round 4, sha c2b7077b9f91ecf5b0fb911ecbaff96a75c497f0, posted 2026-11-02T20:32:57Z.
• New commits read: 1 — 46172250 fix(proxy): purge tool deferral's file even when it is not open (#861 round 4). Full diff c2b7077b..46172250 (4 files, +115/−6), plus each changed file around the changes.
• Read in full: 9 reviews, 9 comments, 0 inline comments (from the three saved files).
• New items since my last review, and what I did:
  • Reviewer 2's round-4 review (20:34:37Z, approve). Read; their dispositions match my round-4 verification, and I re-checked their non-blocking notes against the new head (above).
  • The author's reply to my round-4 review (20:56:28Z). Read; each claim checked against the code, as recorded above.
  • Issue #866's new comment (20:56:30Z, the broken map item). Read; verified the issue is open and the item is in it.
  • Commit 46172250. Reviewed as a first review:
    • purgeToolDefer: traced every branch — state built (memory drops), process session store (DropPrefix, lock and pending-write behavior checked in store/disk.go:540-565), file open (store drops plus dash.PurgeToolDefer on f.DB), file not open (stat, OpenForPurge, purge, close). Checked the byte-range and prefix arithmetic against the key format again: cg:tdf:<tenant>:<session> cannot cross tenants.
    • OpenForPurge: mode=rw never creates; busy_timeout matches the file's own connection, so a purge against a live-opened file (the race where the state builds while the purge runs) waits rather than errors.
    • The error path: purgeTenantData with no dashboard returns the purge error; with a dashboard it runs the dashboard purge first, then surfaces the tool-defer error. Noted under Non-blocking.
    • The test: read in full, including the fixture (a real file with the tenant's record, flag and history, a restarted handler with the file not open) and both assertions directions.
    • The noPricesOnce log and the fixed test comment.
• Did not run builds or tests (not approved). Findings above are from reading the code only.

[amiddavid]

Reviewer	Kind	Started by	Context	Review	Round	Head	Verdict
Reviewer 2 (glm, user of @amiddavid)	GLM rits/zai-org/glm-5-3 via Claude Code 2.1.316	offload-review skill, @amiddavid's session	resumed	2 of 2	5 of 6	4617225001f9	approve

Verdict: approve

The new commit 46172250 fixes the one finding from round 4 (reviewer 1's) and takes three of its four non-blocking notes. I verified the fix in the code myself. I found no defect in the new code.

Findings

None.

Earlier findings

Agree with reviewer 1, round 4, finding 1 (a purge missed tool deferral's file when this process had not opened it): the finding was real, and I missed it in round 4. Fixed in 46172250. I verified both halves myself.

• The defect at c2b7077b: h.defer_.file is set in exactly one place, toolDefer(), which runs only from applyToolDefer — only for a tenant whose pipeline defers, on an Anthropic request. purgeToolDefer read h.defer_.file, found nil after a restart with no ultrahigh request, and returned 0. The tenant's rows stayed in the file on disk, and every later purge reported success. I approved that head without seeing this; reviewer 1 posted the finding two minutes before my round-4 review, and my conversation fetch for that round had already run. The mechanism is exactly as they traced it, and I checked each step in the round-4 code.
• The fix at 46172250 (proxy/tooldefer.go, purgeToolDefer): when the file is not open but exists at Options.ToolDeferPath, the purge opens it on its own connection, runs dash.PurgeToolDefer, and closes it. A path with no file returns 0 and creates none.
• New proof beyond the author's reply: the fix's database details hold up. OpenForPurge opens with mode=rw (tooldefer/file.go), a standard SQLite URI flag the driver honors, so the open fails rather than creates. The file's WAL journal mode is a persistent property of the file, so the plain connection can delete rows the WAL writer wrote. The test closes the file first (f.Close() flushes the pending disk writes), so the rows it checks are really on disk before the purge.
• The test covers the gap itself: TestToolDeferPurgeReachesAnUnopenedFile starts a handler that never opened the file, asserts the purge still removes the tenant's record, flag and history rows, keeps another tenant's record, leaves h.defer_.file nil, and that a purge with no file at the path creates none.

Reviewer 1's round-4 non-blocking notes: three taken, one filed.

• In-memory keys after a purge: taken. purgeToolDefer now drops the tenant's cg:tdf: and cg:tso: keys from the process session store in memory (h.store.(*store.Memory) → DropPrefix), which also removes their pending disk writes. This closes my round-4 note on the live session-store memory.
• startProbe with no price table: taken. One log line per process (noPricesOnce) says ultrahigh defers nothing then.
• The stale test comment: fixed. TestApplyMidThreadUnknownProbeSendsOriginal now describes the N11 behavior.
• The global broken map: filed, not taken. The author added it to #866 at 20:56:30Z; I verified the item is there. The reason (a per-tenant cap is more than a one-line change, entries expire in 24 h) is right.

My round-4 non-blocking items: the in-memory-keys note is closed by this commit, as above. The rest stand, listed below.

All blocking and high items (B1–B4, H1–H4) and N11 remain fixed, as verified in my round-4 review; this commit touches only the purge, the log line and a test comment, and I re-read the changed code around each.

Non-blocking

• The two wording slips from round 4 stand: toolDefer()'s comment still names Options.ToolDeferFile (the field is ToolDeferPath), and the no-durable log line says the file "could not be opened" also when no path was configured (--dashboard-db="").
• The history's retention delete still runs only in a flush that has dirty rows (my round-4 note; on a fully idle proxy, rows older than the retention stay until any session changes).
• My round-2 note on MCP server names that differ only after a __ stands, as a follow-up.

What I checked

• Last review: round 4, c2b7077b9f91ecf5b0fb911ecbaff96a75c497f0, 2026-11-02T20:34:37Z.
• New commits read: 1 — 46172250 fix(proxy): purge tool deferral's file even when it is not open (#861 round 4).
• Read in full: 9 reviews, 9 comments, 0 inline comments (from the three saved files).
• New items since my last review, each read and checked:
  • Reviewer 1's round-4 review (20:32:57Z, blocking, one finding). It was posted between my round-4 conversation fetch and my round-4 post, so I had not read it. I verified its finding myself in the round-4 code and the fix in 46172250 — see "Earlier findings". My round-4 approve was wrong on that point; this round records it.
  • The author's reply to reviewer 1 (20:56:28Z). Each fix claim checked against the code.
  • Commit 46172250. Reviewed as a first review: the whole diff (198 lines) and each changed file around the changes — proxy/tooldefer.go (purgeToolDefer, noPricesOnce), tooldefer/file.go (OpenForPurge), and both test files.
  • Issue #866: the broken-map item added at 20:56:30Z (verified present) and the N12b answer at 20:23:55Z (the owner's decision, relayed by the author; the wide off-value set has been in place since round 1 and Osher's review triaged it as a follow-up).
  • No new comment from the owner.
• Verified the fix's reach: purgeTenantData calls purgeToolDefer in both branches (with and without a dashboard), so the unopened-file path covers the no-dashboard deployment too; ToolDeferPath == "" still returns 0, and a Stat error other than "not exists" fails the purge loudly rather than silently.
• Checked that the new h.store DropPrefix cannot drop another tenant's keys: the purge is reachable only with a real tenant id (destructiveTarget resolves the account from the registry), and the prefixes carry that id. Hosted tenants keep no cg:tdf:/cg:tso: keys in their per-tenant stores (those stores are memory-only, so RecordStore picks the file), so nothing is missed there.
• Checked the test's fixture logic: f.Close() flushes the pending disk writes before the purge, so the counted rows were on disk; the handler never serves a request, so the file stays unopened, as the fixture asserts.
• Did not run builds or tests (not approved). The green -race runs are the author's reports; my dispositions come from reading the code.