agent: MCP skills follow-ups, and the serve suite back in CI

[jaredLunde]

What

This PR fixes the five follow-ups left after the MCP PRs (#130–#133) merged. Fixing item 4 turned up a bigger problem: 48 serve tests were failing or hanging on main, and nobody saw it because CI hasn't run them since #39. That is fixed here too, and the serve tests run in CI again.

A second commit (d72c375) fixes what the audit of the first (237d9a5) found. The biggest change: approvals are no longer persisted (F1, below), because a persisted approval could be forged. It also removes the free_port() race from the agent test suite, now that the serve tests run in parallel in CI.

Every fix has a test that fails without it. I checked this by reverting each fix in scripted mutation runs: 8/8 killed for the first commit, 8/8 killed for the audit fixes. For item 5, the proof is the lib suite run with TMPDIR inside the worktree: 7 failures before the fix, 0 after.

The five items

#	Gap	Fix	Proving test
1	A skill-load approval question always said origin: "main"	A subagent's registry now binds the skill tools in its own name (filter_by_enabled_as, Consent::Model(origin)). The question carries {agent, spawn_id}, the same as a code-execution question.	mcp_skills_approval::a_subagents_skill_load_is_asked_about_in_its_own_name
2	A refreshed listing only reached the on-disk cache at the next live connect	ServerSkills::refresh_if_due writes it back right away (mcp_manifest::store_skills, which amends only the skills fields). If the listing is now cacheScope: private, the server is forgotten instead.	mcp_skills_listing::a_refreshed_listing_is_written_back_to_the_manifest_cache (a restart served from the cache offers the newly published skill), a_listing_that_turns_private_on_refresh_is_forgotten_by_the_cache
3	Approvals only lasted for the session	Deliberately not done. The first commit persisted approvals. The audit showed they could be forged (F1, below), so approvals stay session-lifetime, in memory, and ARCHITECTURE.md explains why.	mcp_skills_resume::an_approval_lasts_the_session_and_cannot_be_planted_on_disk_or_by_a_client
4	serve_get_tree_since_works_from_the_busy_loop_mid_prompt hung	Not a race. It hangs on main every time, along with 47 other serve tests (see below).	serve_session_title::a_title_call_does_not_consume_the_conversation_script, the_scripted_servers_recognise_the_real_title_request; the original test now passes
5	7 lib tests failed with TMPDIR inside a git checkout or under a CLAUDE.md (6 reported; a 7th, serve::tests::git_branch_reports_none_outside_a_git_repository, failed the same way)	The tests use test_support::isolated_tempdir(), a temp root with no .git, CLAUDE.md, AGENTS.md or .agents above it. Production code is unchanged.	The full lib suite passes with TMPDIR in the worktree (1518/1518) and with the system TMPDIR

Item 4 in detail: the serve suite

• Root cause. After a session's first successful run, serve makes a session-title request to the model. The in-order mock model treated that request as the next scripted conversation turn and handed it a reply. Every later turn then got the reply meant for the turn before it. A test waiting for a tool_start hung, and others failed on the wrong reply or on a "transport error" once the script ran out.
• How many. Running every binary(~serve) test on main: 42 failed and 6 timed out, out of 348.
• Why nobody saw it. CI has excluded binary(~serve) since feat(gateway): model routing with connect- and status-level failover #39, after those harnesses froze the runner.
• Fix, in the harness (crates/test-support). The in-order servers answer title requests themselves, recognised by SESSION_TITLE_MARKER. Title requests no longer consume the script and are not recorded. A test that is actually about titles routes them explicitly, which serve_session_title now does. A test pins the marker to the real SESSION_TITLE_SYSTEM, so rewording the prompt fails loudly instead of quietly breaking the harness again.
• Two real bugs the broken suite was hiding:
  • Unstable session order. Session listings tied on updated_at, which has one-second resolution, had no stable order, so offset paging could serve one session twice and skip another. Fixed with an id tie-break (session_store::by_recency, used by every listing sort), descending since d72c375 so that same-second sessions list newest first (F3). Proved by serve_list_paging::a_listing_is_one_page_and_reports_the_total and session_store::tests::same_second_sessions_list_newest_first_in_one_stable_order.
  • A regression from agent: MCP Skills extension (SEP-2640) client support #131. In a session without --approve, an approve that matched no question was acknowledged. It is refused again, with a pointer to --approve. A matching MCP-skill answer still succeeds. Proved by serve_approval::a_malformed_approve_is_rejected_and_a_session_without_a_gate_says_so.
• CI change. A new agent-serve shard runs the serve tests again. The comment explaining why they had been excluded is updated.

Audit fixes (d72c375)

#	Finding	Fix	Proving test (fails without the fix)
F1	Persisted approvals could be forged. The session file belongs to the user, so a model with write/edit can append an mcp_skill_approval entry to it, and it knows the fingerprint because that is in the loaded skill's tag. A client could also append_custom one. Either way, after a restart an execute approval would run code without asking.	Approvals are no longer persisted or restored; they last the session, in memory. A client's append_custom of a kind the agent writes itself (session_store::HOST_CUSTOM_KINDS: mcp_task, mcp_task_result, mcp_skill_approval) is refused. ARCHITECTURE.md says why approvals are never persisted.	mcp_skills_resume::an_approval_lasts_the_session_and_cannot_be_planted_on_disk_or_by_a_client (the audit's forgery probe, adopted). It checks four things: an approval holds for the rest of the session; a restart asks again; an approval written into the session file is ignored; a client-appended one is refused and grants nothing. Also serve_session_tree::serve_append_custom_refuses_the_kinds_the_agent_writes_itself
F2	Approvals from an abandoned branch could survive switch_branch	Moot, because nothing is restored from any branch. Decisions are set only by the user's answers in this process, and restore_from_transcript rebuilds only the acting window, never approvals.	the F1 test
F3	The paging tie-break had no test of its own (mutant P7 survived)	Ties now break by id, descending. A generated id starts with its creation time in fixed-width hex nanoseconds, so within a second the newest session still comes first.	session_store::tests::same_second_sessions_list_newest_first_in_one_stable_order (kills both an ascending tie-break and no tie-break)
F4	A typed /skill: became a remembered approval, so it pre-approved every later model-initiated load of that skill	User consent now covers only that one invocation.	mcp_skills_approval::a_typed_skill_invocation_does_not_approve_the_models_later_loads
F5	store_skills did an unlocked read-modify-write of the manifest, through a fixed json.tmp name	Every manifest write (store, forget, store_skills) runs under the repo's lockfile pattern (mcp_manifest::FileLock) and renames a temp file of its own into place.	mcp_manifest::tests::concurrent_stores_and_skills_refreshes_lose_no_update (24 concurrent connects, then 24 concurrent refreshes; no update lost, no lock or temp file left behind)
F6	serve_state_reporting hung when title recognition broke	Its stdout is read with a 20s deadline (common::frames_with_deadline), so a stall panics and says why. The scripted servers now count title calls (title_calls), and a test pins the count at one per session.	With title recognition broken, the suite fails at the deadline instead of hanging. serve_session_title::a_title_call_does_not_consume_the_conversation_script asserts exactly 1 title call over 3 turns and fails when a title is requested every turn.

Test ports: the free_port() race

free_port() picked a port, released it, and hoped the child would bind it before anything else did. Under parallel suites, another process sometimes took it in between. This caused the one flake in the first proof run: serve_service_grant got HTTP 200 from another test's server where it expected a WebSocket upgrade. Each serve child now gets its port in one of three ways, none of which leaves a gap:

• spawn_listening (most tests): passes --listen 127.0.0.1:0 and reads the bound port back from the line serve already prints (serve: websocket listening on …). The child's stderr is forwarded into the test's captured output, so a crash shows up in a failing test instead of being discarded.
• HeldPort (the MCP Events daemons): their callback URL names the port before serve starts. The test binds the listener and hands it to serve by socket activation (LISTEN_FDS, which serve already supports for systemd), passed as stdin because the workspace forbids unsafe. A daemon restarted on the same port gets the same socket. down() keeps the port reserved between the two daemons while refusing connections, as a stopped daemon would.
• DeadPort (a port where nothing listens): a socket that is bound but never listening. Connections are refused, and no other process can take the port.

free_port() remains only for the gateway and nats-server, which can neither report a bound port nor adopt one. The mcp_apps_resilience retry loop that worked around the race is gone.

Load proof: three concurrent nextest runs, 8 threads each, of every affected binary (26 binaries, 155 tests), two rounds, while a separate process churned ephemeral ports (1,595,136 binds). Result: 930/930 passed.

Proof (local, d72c375)

• Every beyond-ai-agent test except exec_endpoint_live (lib, all mcp_*, all serve_*): 2484/2484 passed.
• Lib suite: 1520/1520, with TMPDIR inside the worktree and with the system TMPDIR.
• clippy (-D warnings, agent + test-support + fleet-sim, all targets, code-mode), cargo fmt --check and dprint check: clean.
• SEP-2640 conformance: 4/4.

CI: green on the first attempt: all 21 jobs passed (run 37567072020), including agent-serve, agent-rest, agent-lib, agent-code-mode and the changed-lines mutants.

Proof (local, 237d9a5)

• Every beyond-ai-agent test except exec_endpoint_live (lib, all mcp_*, all serve_*) under nextest with 8 threads: 2479/2480 passed. The one failure, serve_service_grant::a_daemon_without_service_mode_ignores_the_grant_header, passed 3/3 on rerun. It is a race in the free_port() helper: another parallel test can grab the port between the helper picking it and serve binding it, and here that test answered HTTP 200 where a WebSocket upgrade was expected. That helper race predates this PR; Fixed in d72c375 (see "Test ports" above).
• Lib suite: 1518/1518, with TMPDIR inside the worktree and with the system TMPDIR.
• clippy (-D warnings, agent + test-support + fleet-sim, all targets, code-mode), cargo fmt --check and dprint check: clean.
• SEP-2640 conformance suite: 4/4 passed.

CI: green on the first attempt: all 21 jobs passed (run 37563481230). That includes the new agent-serve shard (7m26s), the first CI run of the serve tests since #39, alongside agent-lib, agent-rest and agent-code-mode.

Notes

• Approvals are session-lifetime only. run never asks: it denies unless --approve-mcp-skills is set.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk