Rectify: open_kitchen Registry Loss — Client-Cache Coherence - #4407
Merged
Conversation
After `close_kitchen()` → `open_kitchen()`, the server-side state was correct (global `mcp.enable()` calls work), but the connected MCP client kept serving a stale post-close tool cache because no `ToolListChangedNotification` was sent after the global re-enables. The `_skip_notify` flag correctly prevents notification at boot (no client connected yet) but is incorrectly reused mid-session (client IS connected and synced to the disabled state). `close_kitchen` unconditionally sends the notification via `ctx.reset_visibility()`, creating an asymmetry: tools vanish but never return. Fix: send an explicit `ctx.send_notification(ToolListChangedNotification())` after the global re-enables in `open_kitchen`'s `_use_global_enable` branch (formerly `_skip_notify`). The variable rename clarifies that the branch selects an enable mechanism (global vs session-scoped), not notification suppression. A scope-placement invariant comment is added at the `if not _skip_handler:` guard to prevent future maintainers from breaking the gate_infrastructure_ready transition. Arch guard: `_SKIP_GUARD_NAMES` extended with `_use_global_enable` so the rename doesn't break `test_enable_components_calls_have_notification_fallback`. A companion check (`test_guard_branches_send_notification`) verifies that every guarded `if`-branch body contains a `send_notification` call, scoped to the branch body (not the whole function) so an unrelated notification elsewhere doesn't vacuously satisfy the check. Line-limit exemption for tools_kitchen.py bumped 1800 → 1830 with rationale citing #4399. Tests: - Test A: `test_close_open_roundtrip_restores_tools_via_session` drives the real `close_kitchen`/`open_kitchen` tools through an in-process `fastmcp.Client` session with `supports_tool_list_changed=False`, asserting the post-open `client.list_tools()` returns kitchen tools. - Test B: `test_open_kitchen_exempt_surface_renders_real_content` passes a well-formed payload through `_format_response` and asserts the `--- STEP FLOW ---` marker (proving substantive content rendering, not a degenerate stub). Co-Authored-By: Claude <noreply@anthropic.com>
…te allowlist (#4399) Two follow-ups to the open_kitchen client-cache coherence fix: 1. tests/arch/test_cascade_map_guard.py's file-level drift guard flagged server/test_tools_kitchen_envelope.py as missing from LAYER_CASCADE_CONSERVATIVE['hooks']: the new test_open_kitchen_exempt_surface_renders_real_content test imports autoskillit.hooks.formatters.pretty_output_hook directly to drive the _fmt_open_kitchen contract. Add it as a file-level entry with a comment referencing #4399 criterion 4. 2. tests/infra/test_schema_version_convention.py's allowlist ratchet flagged stale line numbers: the same fix shifted json.dumps+atomic_write sites in tools_kitchen.py from 290/309/343/1575 to 291/310/344/1602 (one-line import added, followed by the net-new try/except notification block).
… (dead cruft, rename already complete)
…cement invariant comment
Trecek
commented
Jul 28, 2026
Trecek
left a comment
Trecek
left a comment
Collaborator
Author
There was a problem hiding this comment.
AutoSkillit review passed. No blocking issues found.
(Verdict: approved. Self-authored PR — GitHub does not permit a self-APPROVE review, so this is posted as a COMMENT review instead.)
5 tasks
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
1 participant
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.
Summary
After
close_kitchen()→open_kitchen(), all 41 kitchen-tagged MCP tools vanish from the client's tool list. The server-side state is correct — the globalmcp.enable()calls added in PR #4401 work and the server reports tools as enabled — but the client is never notified to re-querytools/list, so it keeps serving a stale cache from the close transition.The architectural weakness is a notification asymmetry:
close_kitchenunconditionally sendsToolListChangedNotification(viactx.reset_visibility()), butopen_kitchendeliberately suppresses notification when_skip_notify=True(all production backends). The_skip_notifyflag correctly prevents notification at boot (no client connected yet) but is incorrectly reused mid-session (client IS connected and synced to the disabled state).The fix is to send an explicit
ToolListChangedNotificationafter the global re-enables inopen_kitchen, and to structurally prevent this class of bug from recurring via an architectural guard and a session-level round-trip regression test.Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/remediation-20260728-095428-498581/.autoskillit/temp/rectify/rectify_open_kitchen_registry_loss_residual_2026-07-28_101500.mdCloses #4399
🤖 Generated with Claude Code via AutoSkillit