Skip to content

A1: live-MCP upgrade smoke-check (post-restart gate) - #11

Merged
jpwinans merged 2 commits into
mainfrom
feat/v340-smoke-and-qdrant-migration
Jun 13, 2026
Merged

A1: live-MCP upgrade smoke-check (post-restart gate)#11
jpwinans merged 2 commits into
mainfrom
feat/v340-smoke-and-qdrant-migration

Conversation

@jpwinans

Copy link
Copy Markdown
Owner

A1 — Live-MCP upgrade smoke-check

Part of the MemPalace v3.4.0 Adoption + Backend Migration envelope (/collaborate-to-build, N=2). Codifies this session's #1 lesson: diary_write existed in code but a stale running server failed to advertise it to the client — a silent capability gap.

What it does

python -m mempalace.smoke_upgrade — a runnable post-restart gate that spawns the configured MCP server over the real stdio JSON-RPC wire (not a module import) and asserts:

  1. serverInfo.version >= 3.4.0
  2. tools/list advertises every expected tool — load-bearing: mempalace_diary_write (the staleness catch)
  3. diary_write -> diary_read round-trips
  4. search returns without raising

Emits PASS/FAIL + exit code (0/1) so A4's runbook can use it as the mandatory post-upgrade gate.

Guardrails honored (envelope §2)

  • version + advertise + search run read-only against the configured palace.
  • the diary round-trip writes only to a disposable sandbox palace (MEMPALACE_PALACE_PATH=tmpdir), auto-removed — so the real palace gains no drawer and no col.delete is ever needed.
  • Verified live: real palace drawer count 67073 unchanged after a full gate run.

TDD

Honest red→green history: 0e1c918 (test, failing pre-impl) → 146b5ee (implementation, green). 19/19 tests pass; 287 existing mcp_server+config tests still green; ruff clean.

Finding surfaced

The literal __healthcheck__ agent/wing name in the spec's §2 guardrail is rejected by mempalace's own sanitize_name (_SAFE_NAME_RE disallows leading/trailing underscores). Sandbox-palace isolation makes the agent name cosmetic; uses smoke-healthcheck. Flagged to architect — affects §2 phrasing + B4 docs.

Scope boundary

A1 = post-restart gate (verifies on-disk code current + configured launch advertises/runs tools). It deliberately does NOT introspect an already-bound client stdio subprocess (client-owned, unreachable from a standalone process). The in-session "is diary_write in my live tool list?" check is an agent action → lives in A4's runbook.

jpwinans added 2 commits June 13, 2026 13:46
Assertions-first for the post-restart upgrade gate: version>=3.4.0 floor,
tools/list advertises every expected tool (load-bearing: mempalace_diary_write
— the one that silently vanished against a stale server), diary_write->read
round-trip on a disposable sandbox palace, search no-raise. Includes the
no-stray-real-palace-drawer guard. Fails pre-implementation (module absent).
Runnable post-restart gate (python -m mempalace.smoke_upgrade). Spawns the
configured server over the real stdio JSON-RPC wire (not a module import) and
asserts version>=3.4.0, all expected tools advertised, diary round-trip, and
search no-raise. version/advertise/search run read-only against the configured
palace; the round-trip writes only to a disposable sandbox palace
(MEMPALACE_PALACE_PATH=tmpdir), so the real palace gains no drawer and no
col.delete is ever needed. Emits PASS/FAIL + exit code for use as A4's gate.
@jpwinans

Copy link
Copy Markdown
Owner Author

Cross-coder review under /collaborate-to-build protocol

Both coders authenticate as the same GitHub account; GitHub blocks gh pr review --approve on own PRs. This comment mirrors the vestige-hearing cross-coder review (ves-coder-2) for GitHub-level audit trail under /collaborate-to-build (MemPalace v3.4.0 Adoption envelope, /goal-sealed).

Verdict: APPROVE

Verification performed (not just read)

  • Checked out the PR branch in a throwaway worktree and ran the suite: 19/19 pass in ~10s, including the 7 integration tests that spawn the real server subprocess over stdio.
  • Confirmed TDD red→green in history: 0e1c918 adds the test file only (import fails → RED) → 146b5ee adds smoke_upgrade.py (GREEN).

Focal items — all confirmed

  1. Real stdio, not import. _default_server_cmd() spawns python -c "from mempalace.mcp_server import main; main()" as a subprocess and drives it over newline-delimited JSON-RPC. The integration tests genuinely exercise this (verified by running them) — it tests the actual advertise path a stale server gets wrong, not an in-process import.
  2. Round-trip isolation / real palace untouched. Session 1 (version/advertise/search) is read-only against the configured palace; Session 2 (diary write→read) runs with MEMPALACE_PALACE_PATH=<mkdtemp sandbox>, cleaned up with shutil.rmtree on the temp dir — no col.delete anywhere. test_diary_roundtrip_in_sandbox_leaves_configured_palace_untouched asserts before == after drawer count via the read-only drawer_count(create=False) path (never opens a write path, never triggers repair). Guardrail §2 satisfied. Automated suite uses temp palaces (conftest.palace_path = mkdtemp) + autouse _isolate_home, so the real palace is never in scope.
  3. diary_write in the advertised set. EXPECTED_TOOLS pins the full frozenset (catches ANY expected tool disappearing, not just the one that bit us) with mempalace_diary_write load-bearing; test_expected_tools_includes_diary_write guards the guard.

Good catch noted: __healthcheck__ is invalid under sanitize_name (leading/trailing underscores rejected), correctly swapped to the valid throwaway smoke-healthcheck; the disposable sandbox is the real isolation mechanism regardless.

Minor notes (non-blocking — coder's discretion)

  1. Session 2 omits the initialize handshake that Session 1 performs. Works because the server doesn't gate tools/call on initialize, but it's latent coupling: if MCP-handshake enforcement is added server-side later, the round-trip breaks. Consider adding initialize to Session 2 for wire-protocol consistency.
  2. _parse_version is lenient on non-digits: "3.4.0rc1"[3,4,1] (rc1 reads as patch 1). Harmless for a >= floor gate; flag only.
  3. _mcp_session subprocess.TimeoutExpired propagates uncaught out of run_smoke (crash vs. a clean FAIL report). Non-zero exit either way for the gate; optional hardening would be a try/except → CheckResult FAIL.
  4. from .version import __version__ is imported-unused (noqa'd "for parity"); drop or keep, no-op.

None block. Well-designed, guardrail-compliant, TDD-disciplined, verified green including the live path.

@jpwinans
jpwinans merged commit 97fe17b into main Jun 13, 2026
3 of 7 checks passed
jpwinans added a commit that referenced this pull request Jun 14, 2026
…nused import (#14)

* test(smoke): A1-polish assertions — prerelease version parse + hung-server FAIL (TDD red)

Adds: _parse_version must read leading-digits-per-component so '3.4.0rc1'->[3,4,0]
not [3,4,1]; _mcp_session must catch subprocess.TimeoutExpired and return {} so a
hung server yields a clean FAIL report instead of crashing the gate; run_smoke
reports ok=False (not a crash) when the server hangs. Fail pre-fix.

* fix(smoke): A1-polish — leading-digit version parse, hung-server clean FAIL, drop unused import (TDD green)

Addresses coder-2's #11 cross-review notes (1-3):
- remove the unused 'from .version import __version__' import.
- _parse_version takes leading digits per component (pre-release suffix no longer
  misread as a higher patch).
- _mcp_session catches subprocess.TimeoutExpired -> returns {} so a hung server
  produces a FAIL report, not a crash. (Note 4, initialize-handshake-skip, kept
  as intentional: mempalace doesn't gate tools/list on the initialized notification.)
22/22 tests pass; live gate still PASS on the qdrant backend; ruff clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant