chore: close out the MCP control-surface milestone (docs reconciliation + hygiene)#228
Merged
Conversation
…g replaced The tool description is what an LLM reads when it chooses a tool, and this one told it to read a command's result "with `snapshot`'s scoped terminal text" — polling, which races the very transition the caller is watching for. That was accurate when orchestration shipped and `wait_for_status` did not exist; it has been wrong since. It now routes callers to `wait_for_status` then `read_terminal`, and says plainly not to poll. Two doc-comments still called the wait "a later rung" for the same reason.
… ARCHITECTURE Six slices of #90 landed, and three documents still described the surface as unbuilt. README called workspace orchestration "a planned follow-up"; AGENTS listed `mcp` among the adapters yet to land (and still called `crates/app` a tracing stub); ARCHITECTURE said nothing in the v1 path imports it. README now documents both surfaces and the act -> wait -> observe loop they exist to serve, and AGENTS gains an entry point naming what is settled, what is open, and where each piece lives. The ARCHITECTURE claim is retracted in place rather than edited into correctness: a reader arriving with the old belief needs to recognise it. It also earns a disambiguation, because the collision is real — §15's deferred `mcp` crate is `F-mcp-ide-bridge`, termherd as a *client* of Claude's IDE bridge, which is genuinely unbuilt. The shipped surface runs the other way round and lives in `app`, since it is a bridge into the shell rather than a port `core` calls out through.
…redicate The one lesson from the MCP milestone that generalises past its own feature. The wait rung stated 'has this settled?' at two call sites with hand-written conditions; one counted a session exit as settling, the other only compared against the requested statuses. A wait placed after a crash therefore parked until the caller's timeout — on a session core had already marked terminal. Recorded with its instance rather than as an abstraction, and with the part that makes it a trap rather than an oversight: the doc-comment stating the correct rule sat directly above the code that broke it. A comment is not enforcement.
Two recurring dirt sources, tied off at the root rather than cleaned again next time. The impeccable skill drops a hook.cache.json beside every source directory it reads; four had accumulated under crates/ and none were ignored, so each one surfaced as untracked noise in git status. markdownlint was linting .personal/ and .local/, which are gitignored working notes CI never sees. It reported four errors on a fresh clone's 24 files becoming 37 locally — errors nobody could act on, which teaches you to skim past the summary line. Local and CI now lint the same 24 files.
The pass ran unconfigured and re-derived the canonical locations, the gate list and the commit conventions from the tree. Written down so the next one does not, and so a moved location is corrected in one place. Two entries encode things this session learned the hard way: worktree removal must check the PR state rather than `git log main..branch` (PRs are squash-merged, so a merged branch's commits never appear on main), and .impeccable caches are ignored rather than deleted because they regenerate.
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.
What
Closing pass over the MCP control-surface milestone (#194, #195, #216 — six of
#90's eight slices now shipped). No behaviour changes to the app; this is the
paperwork the three feature PRs could not do individually, because the claims
they falsified only became false once all three had landed.
The one that matters
run_in_session's tool description told an agent to read a command's result"with
snapshot's scoped terminal text" — polling, which races the verytransition the caller is watching for. That was accurate when orchestration
shipped and
wait_for_statusdid not exist. It has been wrong since #195, andit is the text an LLM reads at tool-selection time, so it was actively
steering agents into the pattern the wait rung was built to replace. It now
routes to
wait_for_status→read_terminaland says plainly not to poll.Superseded claims, reconciled
Found by grepping for the claim rather than by re-reading the files already
open — these were all in files none of the three feature PRs touched:
README.mdhandler.rssnapshotbridge.rs,orchestrate.rsAGENTS.mdmcpis an adapter "yet to land (Unsure)";crates/appis "a tracing + single-instance stub"ARCHITECTURE.md§5The ARCHITECTURE claim is retracted in place (struck through, marked
superseded, pointed at what replaced it) rather than edited into correctness —
a reader arriving still holding the old belief needs to recognise it.
It also earns a disambiguation, because the name collision is real and will
otherwise be re-litigated: §15's deferred
mcpcrate isF-mcp-ide-bridge—termherd as a client of Claude's IDE bridge — and it genuinely is unbuilt.
The shipped surface runs the other way round (termherd is the server) and lives
in
apprather than in a crate of its own, because it is a bridge into theshell, not a port
corecalls out through.Entry point
AGENTS.mdgains a Driving termherd over MCP section stating what issettled, what is still open (#215, #196 — both ticketed), and the contradiction
that is not one.
README.mddocuments both surfaces — the automatic livebridge and the manual stdio config server — and the act → wait → observe loop
they exist to serve.
One lesson promoted
AGENTS.mdquality bar: an invariant expressed twice will drift — extractthe predicate. Its instance is the bug found in review on #227: two call
sites decided "has this settled?" with hand-written conditions, one counting a
session exit and the other not, so a wait placed after a crash parked until the
caller's timeout. Recorded with the part that makes it a trap rather than an
oversight — the doc-comment stating the correct rule sat directly above the
code that broke it.
Nothing else generalised past its own feature.
Two dirt sources tied off
.impeccable/— the skill drops a cache beside every source directory itreads; four had accumulated under
crates/and none were ignored..personal/and.local/— gitignored notes CInever sees. It reported 4 errors on 37 local files where CI lints 24 and sees
none: errors nobody could act on, which teaches you to skim the summary line.
Local and CI now lint the same set.
Config
.wrap.mdrecords the locations, gates and conventions this pass had to infer,plus two things learned the hard way today: worktree removal must check the PR
state rather than
git log main..branch(PRs are squash-merged, so a mergedbranch's commits never appear on
main), and.impeccablecaches get ignoredrather than deleted because they regenerate.
Verification
fmt·clippy -D warnings·test --workspace(13 suites) ·markdownlint(25 files) ·
check-deps·check-archall green. No.rslogic changed —the only code edits are doc-comments and one tool description string.