Honor sessions_enabled on every surface, and make the MCP schema budget deterministic#559
Merged
Conversation
sessions_enabled was read only by the TUI. With it off the TUI hid the drawer and stopped saving, but the seven session MCP tools stayed on the wire and still wrote session files, the HTTP routes still created and appended, and lilbee sessions still worked, so "turning sessions off writes nothing to disk" held on one surface out of four. Gate the MCP tools with _tool_if(sessions_enabled()) and refuse inside each body, matching memory exactly: the decorator keeps them off the wire when sessions are off at import, and the body check covers the flag being toggled at runtime. On HTTP and CLI the check lives in the _store() helper every handler and command already routes through, so the invariant is structural rather than eight or four call sites that can drift, which is how this was missed the first time. The schema budget test measured whatever the environment happened to register. wiki_ depends on cfg.wiki, memory_ on cfg.memory_enabled, and crawl on whether the crawl4ai extra is installed, so the same commit read 9672 bytes in the docs-site job and 10143 on a developer box. That is why a schema regression passed every release job and took down the website deploy instead. Exclude the ambient families, pin the default surface by name so an ungated tool fails by name rather than as a byte overflow, and measure only that. Pinned, the default surface including all seven session tools is 8799 bytes, under the original 8800 cap, so sessions never actually breached the budget. The ceiling moves to 9500 to replace one byte of headroom with room for a tool or two.
Blast-radius review found the same runtime-toggle hole this PR closed on MCP, HTTP, and CLI still open in the TUI. _persist_user_turn returns early when sessions are off, but by then the session id is already set, so switching sessions off mid-chat left _persist_assistant_turn and the compaction summary write appending to a conversation the user had turned off. Re-check the flag at both sites. The compaction test initially asserted nothing: it never enabled chat_compaction, so _compact_history took the free default path and returned before any fold. It now asserts the fold actually ran and landed in memory, and only then that nothing reached the disk. Convert the new MCP and HTTP refusal tests to parametrize, matching the convention in the rest of the suite and giving one result per surface rather than stopping at the first failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
sessions_enabledwas only honored by the TUI. With sessions off, the MCP tools stayed on the wire and kept writing, the HTTP routes still created and appended, andlilbee sessionsstill worked.Separately, the MCP schema budget test measured whatever tools the environment happened to register (
wiki_oncfg.wiki,memory_oncfg.memory_enabled, crawl on the installed extra). The same commit read 9672 bytes in the docs-site job and 10143 locally, which is why it failed only in Pages and blocked the index deploy.Solution
Gate the seven MCP tools with
_tool_if(sessions_enabled())plus a body check, matching memory. On HTTP and CLI the check lives in the_store()helper every caller already routes through. Same for the two TUI writes that outlive the toggle (assistant turn, compaction summary).The budget test now excludes the ambient families and pins the default tool set by name, so an ungated tool fails by name instead of as a byte overflow. Pinned, the default surface including all seven session tools is 8799 bytes, under the original 8800 cap. Ceiling set to 9500 for headroom.
Tests cover the refusal on each surface and assert nothing is written.