Let the MCP server say whether it closed or was killed - #439
Merged
Conversation
A session lost all seven commitlore tools mid-conversation, after four of them had returned real data. Afterwards nothing could establish whether the server had ever been running: `claude mcp list` reported it connected while no process existed, the client was not started with `--debug` so the server's stderr went to a pipe, and `ToolSearch` answers "no matching tools" for a withdrawn capability exactly as for one that never existed. The agent in that session fell back to the CLI without recognising a regression had happened. This does not fix that. The registration lives in the client and nothing here can restore it. What it fixes is the other half: the server now records its own start and exit under `.git/commitlore/`, so the question has an answer. The absent line is the signal. A start with no exit beside it, whose pid is not running, is a server that was killed rather than one that closed its session -- and the start line carries the version and the path it was launched from, which is how a stale plugin cache is identified after the fact. `doctor` reports it. Three things this file will not trade away: it swallows every failure, because a server refusing to start over a log would be worse than the bug being investigated; it writes inside `.git/`, which is already ignored; and it truncates itself, oldest half first, so a checkout that starts a server per session cannot accumulate without bound. Limit: the tool registration that was lost belongs to the client, so nothing in this repository can detect the loss from inside a session or restore it Ruled-out: reporting the loss from `doctor` by probing the MCP server | the server starts and answers on demand -- measured, 60 protocol requests and 24 tool calls with no crash -- so a probe reports healthy in exactly the situation being investigated Ruled-out: writing the log outside `.git/` so it survives a clone | it is evidence about one machine's processes, and a path that can reach a commit is a path that will Warn: this makes a silent withdrawal findable afterwards; it is not a fix and must not be described as one Blast: module Undo: easy Certainty: firm Verified: a session closed through stdin writes both lines and `doctor` stays ok; a SIGKILLed session leaves the start alone and `doctor` names its pid and time; a running server is not reported, so the check cannot fire on every healthy session Unverified: whether a killed server is what happened in the reported session -- the ordering of the plugin refresh against the loss was not establishable from any artifact, and this change exists so the next one is Provenance: authored Record-Id: r-mcplife424
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (105)
Ruled out (241)
Truncated: 97 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
MongLong0214
added a commit
that referenced
this pull request
Aug 7, 2026
…der it Two defects, both caught by running the thing rather than reading it. The bundle was built with `--all`, which packs every branch in the source. On this repository that would have handed the measured agent `git show cdeb-pilot:bench/cdeb/pilot/tasks.ts` -- every prompt, every rejected approach, every oracle predicate -- from inside the tree it was being measured in. Confirmed before any run: the pilot branch tip carries those two files and the snapshot the arms are supposed to see carries none of them. The bundle now carries one snapshot and the notes mirror, and a test plants a second branch and proves neither its ref nor its blobs survive the trip. Naming the snapshot took two tries. `git bundle create` stores the ref names among its rev-list arguments, so a bare sha packs objects with nothing for a clone to land on, and `sha:refs/heads/name` is read as a path inside the commit. A temporary ref works, but only under refs/heads/ -- `git clone` builds its checkout from branches, and a bundle whose only ref sat under refs/cdeb/ cloned into a repository that could not read its own tree. It refuses to reuse an existing name and removes the ref in a finally. The second defect was CI's, and it is the same shape as the one I had just posted a hypothesis about on #424. `test/mcp.test.ts` closed its server stub with `child.kill()` and immediately removed the temp directory; kill returns when the signal is delivered, not when the process is gone, and this server writes its exit record on the way out. The window was mine -- src/mcp/lifecycle.ts put a write in the shutdown path in #439 -- and adding a test file was enough to lose the race, as ENOTEMPTY. close() now resolves on the child's exit event, escalates to SIGKILL after five seconds so a server wedged in its own exit path cannot hang the suite, and every caller awaits it. Limit: the snapshot is pinned to one sha in the runner, so a pilot re-run after dev moves measures the old tree until that constant is changed deliberately Ruled-out: bundling --all and trusting the prompt not to mention other branches | the agent has a shell and git, and a seal that depends on the sealed party not looking is not a seal Ruled-out: keeping close() synchronous and retrying the rmSync | it hides a real shutdown-ordering bug behind a loop, in the exact code path #424 now points at Warn: five seconds is the escalation budget before SIGKILL, so a server that legitimately needs longer to flush its exit record will be cut off and the test will not say so Blast: local Undo: easy Certainty: firm Verified: six materializer cases pass including the planted-branch seal, where neither the branch ref nor answers.txt appears in the clone while the notes record still does; mcp suite 44 passed after the shutdown fix, which failed in CI as ENOTEMPTY before it Provenance: authored Record-Id: r-cdebseal
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.
Part of #424. Not a fix — see below.
What #424 could not establish
A session lost all seven commitlore tools mid-conversation, after four of them had returned real data. Afterwards nothing could say whether the server had ever been running:
claude mcp listreported it connected whilepsshowed no process at all--debug, so the server's stderr went to a pipe and nothing reached diskToolSearchanswers "no matching tools" for a withdrawn capability exactly as for one that never existedThe agent in that session fell back to the CLI without recognising a regression had occurred.
What this does
The server records its own start and exit under
.git/commitlore/mcp-lifecycle.log:The absent line is the signal. A start with no exit beside it, whose pid is not running, is a server that was killed rather than one that closed its session. The start line carries the version and the launch path, which is how a stale plugin cache is identified after the fact — the thing #433 showed can differ from what is on disk.
doctorreports it:What it is not
The registration lives in the client and nothing here can restore it, or even detect the loss from inside a running session. This converts a silent withdrawal into something findable afterwards. The commit says so as a
Warn:rather than leaving the PR title to imply more.Also ruled out and recorded: having
doctorprobe the MCP server instead. The server starts and answers on demand — measured at 60 protocol requests and 24 tool calls with no crash — so a probe reports healthy in exactly the situation being investigated.Three properties it will not trade
.git/, already ignored, so nothing can reach a commit.Tests, and the one that caught me
Six cases, driving real servers rather than simulating: a session closed through stdin writes both lines and
doctorstaysok; aSIGKILLed one leaves the start alone anddoctornames its pid and time; a running server is not reported, because a check that fires on every healthy session is a check nobody reads.test/mcp.test.tshas a guard asserting the exact file list ofsrc/mcp/. Addinglifecycle.tsbroke it — correctly: without adding the file toOWNED_SOURCESthe no-network inspection would have silently stopped covering the whole directory. Added, with the reason at the line.My own first version of the clean-exit test was named for a clean exit and sent
SIGKILL, never asserting the exit line at all — the exact distinction the log exists for. Fixed.Verification
Full suite: 94 files, 2185 passed, 1 skipped.
test/dogfood.test.tsre-run after committing: 9 passed.