Skip to content

agent: file_lock module, unreachable deadline-free readers, LISTEN_PID operator note - #141

Merged
jaredLunde merged 2 commits into
mainfrom
jared/mcp-harness-polish
Oct 7, 2026
Merged

jaredLunde merged 2 commits into
mainfrom
jared/mcp-harness-polish

Conversation

@jaredLunde

@jaredLunde jaredLunde commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

What

This PR fixes the four low-severity items left by the audit of #139. Every testable fix has a test that fails without it: I reverted each fix in a scripted mutation run and all 5 reverts were caught.

# Item Fix Proving test (fails without the fix)
1 Document the LISTEN_PID change for operators ARCHITECTURE.md's transport section has an operator note. Launchers that pass sockets without naming the process they are for now get a startup error: systemfd --no-pid, or wrappers that start the daemon as a child (a script running serve without exec, a supervisor that forks after exporting the variables). The note gives the exact error text and the fix: drop --no-pid; exec the daemon so it keeps the pid the launcher named; or, where a wrapper must fork, set LISTEN_PID=$$ in the child right before it runs serve. It also says systemd, systemd-socket-activate and plain systemfd are unaffected. Docs only
2 The frame-deadline lint matched BufReader::new( and stdout on one line A reader without the deadline is now unreachable, not just discouraged. Every child-stdout read in the serve_* and mcp_* suites and tests/common goes through one helper, common::child_frames(&mut child), which takes the pipe and returns the deadline-bound Frames. That covers 272 converted sites, including common/mcp_events_fixture.rs (its bare-BufReader fixture banner read and its own Frames type, now built on the shared reader) and the mcp_* suites. The lint drops comments and all whitespace, then fails on any ChildStdout, or any .stdout.take()/as_mut/as_ref/unwrap/expect/=, outside that one helper. Line breaks and formatting cannot hide a use. It immediately found one more raw read (mcp_tasks_conformance), now converted. serve_harness_deadlines::only_child_frames_touches_a_childs_stdout. It catches a bare reader split across lines in an mcp_* suite (which the old same-line lint missed), and the events fixture going back to a bare reader.
3 update_store_off_runtime discarded a JoinError A panic inside a manifest write is logged at warn (with the manifest path and the error). The cache is left as it was, because the rename never happens. mcp_manifest::tests::a_write_that_panics_is_logged_and_leaves_the_cache_as_it_was: with the error discarded, the capture is empty. It also checks that the file is unchanged and the lock was released.
4 The manifest lock reached into session_store's process-wide registry The kernel lock (flock via File::try_lock), the same-process path registry and the inode check are now their own small module, crate::file_lock (try_lock, FileLock). The session lock (SessionLock = file_lock::FileLock) and the manifest lock both use it; session_store only names the session's lock file. file_lock::tests (one holder at a time and free again on drop; independent paths; the moved inode check; and new: a path held in this process is refused a second time without opening the file, even if the file was replaced underneath). That last one kills dropping the registry, which the local-flock behaviour alone would hide. Also mcp_manifest::tests::the_manifest_lock_is_its_own_file_lock.

Audit gaps closed (1eccec5)

# Gap Fix Proving test (fails without the fix)
G1 The lint was bypassable: std::mem::take(&mut child.stdout) (mutant T2b) survived, as would Option::take or destructuring a Child The pipe is no longer on the Child. ChildGuard takes it at spawn and holds it, so child.stdout is None however it is reached. child_frames takes it from the guard. ChildGuard::raw_stdout() exists only for tests about the pipe itself: run_stdout_robustness uses it for its EPIPE tests, which need to close the read end. The lint is now a function over source text and flags every route: the ChildStdout type, Option methods on the field, assignment, take/replace/swap(&mut ….stdout), Child { .., stdout, .. } patterns, unguarded .spawn(), and raw_stdout. It leaves an Output's captured bytes and harness fields named stdout alone. The suites outside its scope (smoke, settings_store, run_cli_flags) now read through child_frames too, and the shared NATS server spawns guarded. serve_harness_deadlines::the_lint_catches_every_way_to_the_pipe (15 bypass forms, single- and multi-line), the_lint_leaves_what_is_not_the_pipe_alone, and a_guarded_childs_stdout_field_is_empty_however_it_is_reached (at run time, .take(), mem::take, Option::take and as_mut all find nothing).
G2 same_file was not tested through try_lock (mutant T4) A test-only seam in file_lock replaces the lock file between the open and the lock. try_lock must go round again and return a lock on the file now at the path. file_lock::tests::a_lock_file_replaced_between_open_and_lock_is_not_the_one_held
G3 The lint skipped directory modules It recurses into serve_*/mcp_* directories (tests/mcp_tasks_env/mod.rs) and all of tests/common, and asserts that it found them. only_child_frames_touches_a_childs_stdout (a bypass planted in mcp_tasks_env/mod.rs is caught)

Rebased onto main 473af49 (which includes #140, #142 and #143). #140 added no raw stdout readers.

Proof (local, 1eccec5)

  • Every beyond-ai-agent test except exec_endpoint_live (lib, all serve_*, all mcp_*): 2549/2549 passed.
  • Lib suite: 1561/1561, 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.
  • Mutation runs: 5/5 killed for the original four items, 6/6 for the audit gaps (T2b, Option::take, a destructured raw Child, a bypass in a directory module, the guard leaving the pipe on the Child, T4).

CI: green on the first attempt: all 21 jobs passed (run 37582507963).

🤖 Generated with Claude Code

https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

@jaredLunde
jaredLunde force-pushed the jared/mcp-harness-polish branch from 82f6e18 to 502bce7 Compare October 7, 2026 06:14
jaredLunde and others added 2 commits October 6, 2026 23:24
…D operator note

Audit of #139:
- Operators: ARCHITECTURE.md's transport section says what changed for
  launchers that pass sockets without LISTEN_PID (`systemfd --no-pid`,
  forking wrappers), the error they now see, and the fix (`exec` the
  daemon, or set LISTEN_PID in the child just before it runs).
- The frame-deadline lint matched `BufReader::new(` and `stdout` on one
  line. Now every child-stdout read in the serve_* and mcp_* suites and
  tests/common goes through `common::child_frames`, the events fixture
  included, and the lint fails on any other `ChildStdout` or
  `.stdout.take()`/`as_mut`/`as_ref` (comments dropped, whitespace
  ignored, so formatting cannot hide one). It found one more raw read.
- `update_store_off_runtime` logs a JoinError (a panicking manifest
  write) at warn instead of discarding it.
- The kernel lock and same-process path registry are their own module,
  `file_lock`; the session lock and the manifest lock both use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
… test the inode recheck

Audit of 502bce7:
- The lint was bypassable (`std::mem::take(&mut child.stdout)` survived,
  as would `Option::take`, or destructuring a raw `Child`). `ChildGuard`
  now takes the pipe off the `Child` at spawn, so `child.stdout` is
  `None` however it is reached; `child_frames` takes it from the guard,
  and `raw_stdout` exists only for tests about the pipe itself. The lint
  is a function over source text with tests per bypass form: the
  `ChildStdout` type, `Option` methods on the field, assignment,
  `take`/`replace`/`swap(&mut ….stdout)`, `Child { .., stdout, .. }`
  patterns, unguarded `.spawn()`, and `raw_stdout`; it leaves an
  `Output`'s bytes and harness fields named `stdout` alone. The suites
  outside its scope (smoke, settings_store, run_cli_flags) read through
  `child_frames` too; run_stdout_robustness uses `raw_stdout` for its
  EPIPE tests.
- `file_lock`'s inode recheck is tested through `try_lock`: a test seam
  replaces the file between open and lock, and `try_lock` must go round
  again and return the lock on the file now at the path.
- The lint scans directory modules (`tests/mcp_tasks_env/mod.rs`).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
@jaredLunde
jaredLunde force-pushed the jared/mcp-harness-polish branch from 502bce7 to 1eccec5 Compare October 7, 2026 06:37
@jaredLunde
jaredLunde merged commit 57c8eda into main Oct 7, 2026
21 checks passed
@jaredLunde
jaredLunde deleted the jared/mcp-harness-polish branch October 7, 2026 06:49
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…d Frames

#141's ChildGuard takes the stdout pipe at spawn and its lint refuses any other route to it; the
follow-up tests built Frames from child.stdout.take(). They now pass the guard, like every suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde added a commit that referenced this pull request Oct 7, 2026
… refusals, runtime restore (#136)

* agent: MCP Events follow-ups — panic exit, delivery by tag, permanent refusals, runtime restore

- A panic out of `run`/`serve` no longer hangs in runtime drop on the parked stdin reader: `main`
  catches the unwind, abandons the runtime's threads, sweeps stdio servers and exits 101.
- Steered MCP Events batches are delivered by tag: `SteeringMessage::tag` comes back in
  `AgentEvent::Steered` (emitted after the checkpoint on both paths), and the batch is recorded
  as delivered right then — a later compaction can no longer make it look undelivered.
- Permanent refusals (event not offered, extension unsupported, invalid/denied, no mode) are
  classified, reported as `refused` (frame + `mcp_events_list.unestablished`), retried hourly,
  and no longer keep the events session alive.
- Runtime subscriptions are persisted and restored after a restart; a daemon boots the sessions
  that hold them, and their webhook tokens are held like configured ones.
- Tests for nested elicitation during `events/poll` and `events/stream` (stdio and daemon).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: MCP Events follow-ups — audit fixes (F1–F6), session panic containment, events-session restart

- Unknown/removed servers are permanent refusals; a restore refused for good forgets the runtime
  subscription; runtime subscriptions expire after their session's last client command
  (RUNTIME_TTL_MS, 7 d); boot restoration is bounded (MAX_RESTORED_SESSIONS, 32).
- Steered-batch delivery relies only on the Steered tag (the redundant received list is gone); the
  compaction test now asserts the durable done records and an empty pending queue.
- agent-core reports Steered before ending a terminating batch.
- Direct-HTTP events requests refuse server->client requests with an error instead of ignoring
  them (unary SSE and events/stream).
- -32012 is retried with fresh credentials on a short backoff, refused only after 5 in a row.
- The unused blob fixture tool is gone.
- A panicking daemon session is contained (error frame, session ends, daemon carries on); the MCP
  Events session is restarted after a panic, with backoff.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: one MCP message cap on every path — the events HTTP wire and every HttpClient POST

Extends #134's single per-message cap (mcp_stdio::max_message_bytes,
BEYOND_AI_AGENT_MCP_MAX_MESSAGE_BYTES) to the paths it did not cover: the MCP Events direct-HTTP
wire (unary JSON, unary SSE, events/stream — its own 4 MiB / 1 MiB caps are gone) and every
mcp_wire::HttpClient POST (not only skills/*; a request's over-cap answer becomes its JSON-RPC
error), plus the GET stream through rmcp's SSE-event limit. The skills-only 32 MiB constant is gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: an over-cap MCP Events push event is skipped and reported, not reconnected into forever

The over-cap event's bounded head is read structurally (mcp_stdio::oversized_stand_in — the same
member walk as scan_head) for its routing, cursor and id; a small $oversized stand-in replaces it on
both transports (the stdio pump, and the events SSE reader, which skips the rest of the event
unread). The subscription keeps the cursor (else the next heartbeat's), records an 'oversized' gap
(frame + model notice) and carries on — no reconnect into the same giant event.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: follow-up tests on #135's race-free daemon ports

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: MCP Events re-audit fixes (N1–N7) — runtime TTL from creation, refusal Accept, inline refusals, boot ranking, purge, restart, one POST path

- N1: creating a runtime subscription records client activity (set_runtime), and a state with
  runtime subscriptions but no record is no longer trusted forever: one made by a single command
  expires one TTL after creation. (a_runtime_subscription_made_by_one_command_expires_with_no_client)
- N2: a direct-HTTP server->client request's refusal POST carries the same Accept as every events
  POST; the fixture now 406s an answer without it, as the Python SDK would. (over_direct_http_*)
- N3: refusals on an events/stream are answered inline, one at a time — a flooding server cannot
  make the reader spawn without limit.
  (a_flood_of_server_requests_on_an_events_stream_is_answered_one_at_a_time)
- N4: the boot cap keeps the sessions a client used most recently, not the first alphabetically.
  (the_boot_cap_keeps_the_most_recently_used_sessions)
- N5: a forgotten runtime subscription (TTL expiry, permanent refusal on restore, server ended)
  is forgotten whole: its webhook token and secret leave the snapshot.
  (a_runtime_subscription_to_a_removed_server_is_forgotten_not_resurrected)
- N6: a panicked session holding runtime subscriptions is restarted with per-session backoff,
  like the events session. (a_session_with_runtime_subscriptions_is_restarted_after_a_panic)
- N7: ordinary_limit reads the request id directly instead of serializing the whole message;
  every streamable-HTTP POST goes through post_bounded (rmcp's post_message is unused) — pinned
  against rmcp's own client on request headers and response shapes
  (an_ordinary_request_is_sent_as_rmcps_own_client_would / ..._answered_as_...).
- ARCHITECTURE.md: the above, and the over-cap push-event skip from the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: a refused server request is answered with the current OAuth token (after #138)

#138 made every direct events/* POST carry the server's current shared token and refresh-and-resend
once on a 401; the refusal POST for a server->client request still sent the headers built at dial,
so after a refresh it spent a 401 and the server kept waiting. It now goes through the same
Conn::send (Accept included; no Mcp-Method, since an answer has no method).
(a_refused_server_request_is_answered_with_the_current_token_and_refreshed_on_401)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: reconcile the one POST path with #140 — drop HttpClient's oauth flag, pin the 401 divergence

#140 routed an OAuth server's POSTs through post_bounded (HttpClient { oauth }) so a 401 stays
visible; this branch already routes every server's POSTs there, so the flag selected nothing: it is
removed, with OAUTH_MAX_MESSAGE_BYTES (the one per-message cap, mcp_stdio::max_message_bytes,
applies). The rmcp-parity test now pins the one deliberate divergence: a 401 with no challenge and
a JSON-RPC error body is AuthRequired here, an error response from rmcp's own client.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: widen the steered-compaction test's window so full-suite load cannot close it

Its second event must be polled, coalesced and steered within one stalled turn; 2.5 s was missed
once under full-suite load on the host. 6 s per turn, and a 90 s bound on the run's response.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent: reconcile the one POST path with #142 — every server, two pinned divergences from rmcp

#142's routing docs and tests described post_bounded as the OAuth servers' path, chosen by the
removed oauth flag; it answers every server's POSTs. ARCHITECTURE.md now names both deliberate
differences from rmcp's client (any 401 is AuthRequired; a non-JSON-RPC success answering a request
is an error), each with its pinning test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

* agent tests: read the MCP Events suites' stdout through #141's guarded Frames

#141's ChildGuard takes the stdout pipe at spawn and its lint refuses any other route to it; the
follow-up tests built Frames from child.stdout.take(). They now pass the guard, like every suite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…w tests read through child_frames

- mcp_apps_session::view_html_is_cached…: rmcp handles list_changed on a
  task of its own, so the response to change_view can arrive first; the
  test slept 200 ms and hoped. The handler now logs (debug) when it has
  run, and the test waits for that line in the daemon's stderr within a
  deadline. Under 8 concurrent copies plus a full lib suite × 6 rounds:
  48/48.
- The OAuth routing tests read the serve child's stdout through
  common::child_frames, as #141's harness rule requires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…w tests read through child_frames

- mcp_apps_session::view_html_is_cached…: rmcp handles list_changed on a
  task of its own, so the response to change_view can arrive first; the
  test slept 200 ms and hoped. The handler now logs (debug) when it has
  run, and the test waits for that line in the daemon's stderr within a
  deadline. Under 8 concurrent copies plus a full lib suite × 6 rounds:
  48/48.
- The OAuth routing tests read the serve child's stdout through
  common::child_frames, as #141's harness rule requires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde added a commit that referenced this pull request Oct 7, 2026
…w tests read through child_frames

- mcp_apps_session::view_html_is_cached…: rmcp handles list_changed on a
  task of its own, so the response to change_view can arrive first; the
  test slept 200 ms and hoped. The handler now logs (debug) when it has
  run, and the test waits for that line in the daemon's stderr within a
  deadline. Under 8 concurrent copies plus a full lib suite × 6 rounds:
  48/48.
- The OAuth routing tests read the serve child's stdout through
  common::child_frames, as #141's harness rule requires.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
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