Repository navigation
agent: MCP tasks follow-ups — run --continue resumes, unreachable stays pending, sealed journal - #137
Merged
Merged
Conversation
jaredLunde
force-pushed
the
jared/mcp-tasks-followups
branch
2 times, most recently
from
October 7, 2026 05:02
6d52dff to
a896d67
Compare
jaredLunde
force-pushed
the
jared/mcp-tasks-followups
branch
3 times, most recently
from
October 7, 2026 07:50
2706467 to
639c321
Compare
…ys pending, per-session journal auth - run journals the MCP tasks it starts and `run --continue` resumes a session's pending tasks before its turn (serve already did). - A server that cannot be reached is not an answer: the resume redials with backoff; still unreachable (or lost for good mid-poll), the call gets a "[MCP task result pending]" placeholder, no result is journaled, and a later prompt or start retries; the real result replaces the placeholder. A configured server that failed to dial at startup is kept dormant for it. - A placeholder is marked structurally (an `mcp_task_placeholder` journal entry, a reserved host kind), never recognised by its text: a real result that begins like one is an answer. - The task journal is authenticated per session, never per machine (`mcp_resume::JournalAuth`). Service mode trusts its tenant-sealed segments and adds no MAC, so another replica on the same store resumes. Locally each entry carries an HMAC under a key in a 0600 sidecar beside the session (`<session>.jsonl.mcp-task-journal.json`, a suffix so `work.1`/`work.2` never share one; the old with_extension name is carried over), made by its first journal write under an exclusive lock on the sidecar, so concurrent first writers (threads, processes) agree on one key. It moves, trashes and restores with the session, so a session copied to another machine or a fresh $HOME still resumes. A prompt-time resumer restart counts the previous resumer's results as answers, so a finished task is never polled again. Replay ignores unsealed entries, so lines planted in the session .jsonl (write/edit) never cause a poll or reach the model as a result. A session without a key (older, or not yet journaled) is read as before; its next journal write makes the key with the entries already there accepted. Records must also match the call they name and a configured server. Residual local risk documented in ARCHITECTURE. - A session file is tightened to 0600 by the next append if it was looser; task journal entries are confirmed sealed in service storage. - The sessionId ownership rule gets an e2e test that fails without it. - Test fixtures write the flag/pid/state files tests read atomically (temp + rename), and the sticky-task abort test waits for complete content: the cancel flag could be read created-but-empty. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk
jaredLunde
force-pushed
the
jared/mcp-tasks-followups
branch
from
October 7, 2026 09:19
639c321 to
915fec2
Compare
jaredLunde
added a commit
that referenced
this pull request
Oct 7, 2026
…ession locks One commit for #144, rebased onto #137/#148/#150 (0602cac). - OAuth fixture: full streamable-HTTP server (sessions + 404 expiry, SSE streamed event by event with an in-POST elicitation, GET stream, DELETE). Routing tests: DELETE with bearer + session on reap and on exit, in-POST question routed and answered mid-stream. - HTTP MCP sessions get a bounded, parallel, authenticated DELETE on every graceful exit (mcp_http_exit); sessions held weakly, orphan DELETEs dropped once their attempt is over; the registry is a value, tests use their own. - file_lock: open file description locks (F_OFD_SETLK via nix), released by an explicit unlock — no other descriptor releases them, a forked child's copy cannot keep them, two descriptions conflict in one process. Off Linux, flock with explicit LOCK_UN. Legacy flock half for mixed rollout. Target::Itself for #137's journal key: locked itself, written through the locked descriptor, no legacy flock (it would self-conflict on NFS). Verified on loopback NFS v4.2/v4.0/v3. - A failed start's session dir: lock released and closed before its files are unlinked (NFS silly-renames open files and rmdir then fails). - write_atomic never renames over a lock file, record or legacy (only a legacy one beside a record one, so Cargo.lock is untouched). - apps cache test waits for list_changed deterministically; output.rs sweep test uses a unique prefix. 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
…ession locks (#144) One commit for #144, rebased onto #137/#148/#150 (0602cac). - OAuth fixture: full streamable-HTTP server (sessions + 404 expiry, SSE streamed event by event with an in-POST elicitation, GET stream, DELETE). Routing tests: DELETE with bearer + session on reap and on exit, in-POST question routed and answered mid-stream. - HTTP MCP sessions get a bounded, parallel, authenticated DELETE on every graceful exit (mcp_http_exit); sessions held weakly, orphan DELETEs dropped once their attempt is over; the registry is a value, tests use their own. - file_lock: open file description locks (F_OFD_SETLK via nix), released by an explicit unlock — no other descriptor releases them, a forked child's copy cannot keep them, two descriptions conflict in one process. Off Linux, flock with explicit LOCK_UN. Legacy flock half for mixed rollout. Target::Itself for #137's journal key: locked itself, written through the locked descriptor, no legacy flock (it would self-conflict on NFS). Verified on loopback NFS v4.2/v4.0/v3. - A failed start's session dir: lock released and closed before its files are unlinked (NFS silly-renames open files and rmdir then fails). - write_atomic never renames over a lock file, record or legacy (only a legacy one beside a record one, so Cargo.lock is untouched). - apps cache test waits for list_changed deterministically; output.rs sweep test uses a unique prefix. 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
Tests that failed only under heavy parallel load, each made independent of host timing: - mcp_events_nested (a real race): serve started the MCP Events hub before installing the session's elicitation and sampling gates, so a server asking a question during the very first events/poll was declined as "no client". The hub now starts after the gates. A debug-only seam (BEYOND_AI_AGENT_TEST_SLOW_GATE_INSTALL_MS) holds the window open; the new test fails on the old order. - mcp_events_receiver a_retry_that_beats_the_resubscribe (a real race): a restarted daemon answered 410 (stop) to a delivery for a persisted callback that arrived before its events session had read its state and reserved the token. Unknown tokens are now 503 from daemon start until that session has reserved its tokens (bounded). A debug-only seam (BEYOND_AI_AGENT_TEST_SLOW_EVENTS_RESTORE_MS) holds the window open, and the test no longer sleeps 800 ms. - serve_uds uds_stale_socket_file_is_reclaimed: the "stale" node was a listener bound and dropped in the test process, which a child forked by another test in that instant keeps alive until its exec; a connect then succeeded against it. The node is now a datagram socket, which no stream connect can reach. - mcp_stdio a_panic_unwinding_past_the_exit_guard_still_sweeps: sweep_before_exit's wait is bounded, and a loaded host could stall a sweep thread past it. The exiting thread now kills every server still running itself after the wait. The test reads "killed" from the kernel (gone, a zombie, or SIGKILL pending), not from scheduling. - mcp_events wire many_events_in_one_chunk_drain_in_linear_time: counts the bytes scanned and moved (must stay within 2x the input) instead of timing the drain. - serve_lifecycle a_slow_lifecycle_consumer_does_not_delay_the_prompt_response: the collector holds every POST unanswered until released, and the prompt response must arrive first: proved by order, not by a 1.5 s bound. - serve_health: /readyz callers wait out the one transient answer, "shard probe timed out" (the probe's real answer lands in the memo); any other answer, a 503 for another reason included, is returned at once. - mcp_skills_integrity a_listing_that_never_ends_is_cut_off_and_said_so: the listing stays fresh for the run, so the turn does not page through all 64 again under the refresh's 30 s bound; asserts exactly 64 skills/list. - tool_reactor_stall (#148's probe) failed on CI: a 4 MB write took under a millisecond on a CI runner, and the tool's fixed per-call overhead (~0.26 ms in a debug build) alone was a third of that baseline. The edit and write subjects are now ~36 MB, so the baseline is milliseconds everywhere and the bar (25%) is unchanged. #137 follow-ups: - The journal key's maker re-reads it through the locked descriptor (FileLock::file); with #144's OFD locks another descriptor's close no longer releases the lock, NFS included. - An old-name key (not yet carried over) trashes, restores and hard-deletes with its session. - Why a prompt needs no drain of queued journal writes is documented where the prompt reads the journal. 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
Tests that failed only under heavy parallel load, each made independent of host timing: - mcp_events_nested (a real race): serve started the MCP Events hub before installing the session's elicitation and sampling gates, so a server asking a question during the very first events/poll was declined as "no client". The hub now starts after the gates. A debug-only seam (BEYOND_AI_AGENT_TEST_SLOW_GATE_INSTALL_MS) holds the window open; the new test fails on the old order. - mcp_events_receiver a_retry_that_beats_the_resubscribe (a real race): a restarted daemon answered 410 (stop) to a delivery for a persisted callback that arrived before its events session had read its state and reserved the token. Unknown tokens are now 503 from daemon start until that session has reserved its tokens (bounded). A debug-only seam (BEYOND_AI_AGENT_TEST_SLOW_EVENTS_RESTORE_MS) holds the window open, and the test no longer sleeps 800 ms. - serve_uds uds_stale_socket_file_is_reclaimed: the "stale" node was a listener bound and dropped in the test process, which a child forked by another test in that instant keeps alive until its exec; a connect then succeeded against it. The node is now a datagram socket, which no stream connect can reach. - mcp_stdio a_panic_unwinding_past_the_exit_guard_still_sweeps: sweep_before_exit's wait is bounded, and a loaded host could stall a sweep thread past it. The exiting thread now kills every server still running itself after the wait. The test reads "killed" from the kernel (gone, a zombie, or SIGKILL pending), not from scheduling. - mcp_events wire many_events_in_one_chunk_drain_in_linear_time: counts the bytes scanned and moved (must stay within 2x the input) instead of timing the drain. - serve_lifecycle a_slow_lifecycle_consumer_does_not_delay_the_prompt_response: the collector holds every POST unanswered until released, and the prompt response must arrive first: proved by order, not by a 1.5 s bound. - serve_health: /readyz callers wait out the one transient answer, "shard probe timed out" (the probe's real answer lands in the memo); any other answer, a 503 for another reason included, is returned at once. - mcp_skills_integrity a_listing_that_never_ends_is_cut_off_and_said_so: the listing stays fresh for the run, so the turn does not page through all 64 again under the refresh's 30 s bound; asserts exactly 64 skills/list. - tool_reactor_stall (#148's probe) failed on CI: a 4 MB write took under a millisecond on a CI runner, and the tool's fixed per-call overhead (~0.26 ms in a debug build) alone was a third of that baseline. The edit and write subjects are now ~36 MB, so the baseline is milliseconds everywhere and the bar (25%) is unchanged. #137 follow-ups: - The journal key's maker re-reads it through the locked descriptor (FileLock::file); with #144's OFD locks another descriptor's close no longer releases the lock, NFS included. - An old-name key (not yet carried over) trashes, restores and hard-deletes with its session. - Why a prompt needs no drain of queued journal writes is documented where the prompt reads the journal. Claude-Session: https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
This PR follows up on #130 (MCP Tasks, SEP-2663) and fixes every gap found since #130 merged. It is rebased onto #146 (
771d91b) and is a single commit.For each fix I reverted it with the tests unchanged and checked that the named test fails. Item 2's sealing test is the one exception: it confirms behaviour that already worked, so there was no fix to revert.
What changed since
a896d67a896d67the key lived in~/.claude, so a session opened under another key (another replica, a fresh$HOME, another machine, an upgrade) ignored its whole journal. In service mode$HOMEdoesn't exist, so ata896d67the journal was never trusted at all. That breakage came froma896d67's own per-HOME key, not from main: on main (3139d81), before journals were sealed, service-mode resume worked. The auditor ranmcp_tasks_servicethere and it passes.639c321(items 11–16 below).mcp_tasks*readers use agent: file_lock module, unreachable deadline-free readers, LISTEN_PID operator note #141'scommon::child_frames.Relaylistener for its whole life, and it relays to whichever fixture is up. The fixture's fixed-port option is gone.What #135 already covers (dropped from this PR)
append_customcheck: agent: MCP skills follow-ups, and the serve suite back in CI #135'ssession_store::is_host_custom_kindalready covers the journal kinds; the newmcp_task_placeholderkind is added to it. My task test now relies on it.serve_approvalexpectation: agent: MCP skills follow-ups, and the serve suite back in CI #135 restored the original behaviour, so my change to that test is dropped too.Items
sessionIdownership rule.switch_branchkeeps the same id. So the e2e test uses the real case of a session file copied (restored or migrated) under a new id. The copy carries the unanswered call and the task record, and must not resume the original's task.mcp_tasks_resume::a_journal_carried_under_another_session_id_is_not_resumed0600before writing;createalready used0600. Service mode: journal entries go through the tenant-keyed sealed segments, which needed no change.session_store::tests::an_append_tightens_a_loosened_session_file_to_private;session_segments_sealing::mcp_task_journal_entries_are_sealed_and_read_back(confirmation only)run --continuedid not resume tasks.runnow saves the tasks it starts (RunJournal), andrun --continueresumes the session's pending tasks before its turn. N3 (the resumer restarting immediately on a session switch) is already proven byswitching_sessions_moves_the_resumer_with_it.mcp_tasks_resume::run_continue_resumes_a_task_a_killed_run_left_in_flightmcp_taskrecord could trigger a poll..jsonlwithwrite/editnever causes a poll and never reaches the model orget_messagesas a result. A record must also match the call it names and name a configured server.a_forged_task_record_from_a_client_is_refused_and_never_polled,journal_lines_planted_in_the_session_file_are_ignored_on_resume,mcp_resume::tests::{a_keyed_journal_refuses_unsealed_tampered_and_foreign_entries, pending_ignores_a_record_that_does_not_match_its_call}[MCP task result pending]placeholder, no result is saved, and a later prompt (serve) or start (run --continue) tries again. The real result then replaces the placeholder where it sits. A configured server that failed to connect at startup is kept dormant in the catalog for this. Only a terminal status, a-32602, or an expired TTL resolves a call.run_continue_while_the_server_is_down_leaves_the_task_pending_then_delivers_it(the auditor's probe made into a test),serve_with_the_server_down_leaves_the_task_pending_then_delivers_ita_task_on_a_server_no_longer_configured_is_not_resumed(serve),run_continue_does_not_resume_a_task_on_a_server_no_longer_configured(run)serve_abort_sends_tasks_cancel_for_sticky_taskwas flaky: the cancel flag could be read after it was created but before it was written.got ""); with the fix it passes.~/.claude/mcp-task-journal.key). A session opened where the key differed (another service replica on shared storage, a wiped~/.claude, another machine, an upgrade) silently lost every journal entry: no resume, no journaled results.mcp_resume::JournalAuth, held by the session's store (active_journal/append_journal). Service mode (segments sealed under the tenant key): no MAC at all, because the storage already authenticates every line, which is stronger. Local: an HMAC key in a0600sidecar beside the session (<session>.jsonl.mcp-task-journal.json, see item 16; or inside a segmented session's directory). The session's first journal write creates it, so a session that never uses tasks gets no extra file. Concurrent first writers agree on one key (item 11). The key moves, trashes and restores with the session. Sessions without a key (from before this change, or not yet journaled) are read as before, and opening one writes nothing. The next journal write makes the key and records the MACs of the entries already there as accepted, so nothing that resumed before stops resuming.mcp_tasks_resume::a_session_copied_to_a_fresh_home_still_resumes_and_shows_its_journal(copied to a fresh$HOME: resumes the pending task; copied again: the journaled result reaches the model andget_messageswith no re-poll),mcp_tasks_service::a_task_left_in_flight_on_one_replica_resumes_on_another(twoserve --serviceprocesses on one store: the owner is killed mid-task and the peer resumes it). Both fail ona896d67. Alsosession_store::tests::{a_session_keeps_its_journal_key_beside_it_and_refuses_planted_entries, a_session_without_a_journal_key_keeps_its_existing_entries}.[MCP task result pending]prefix, so a real tool result that began that way counted as unanswered and was resumed again.serveresumer,run --continue) journals anmcp_task_placeholderentry for itstool_useid, sealed like the rest of the journal and reserved against clientappend_custom.answered_ids,pending,splice,materializeandresume_into_turnuse that set and never look at the text.mcp_tasks_resume::a_real_result_that_reads_like_a_placeholder_is_still_an_answer(fails ona896d67: the answered call gets polled again),mcp_resume::tests::a_placeholder_is_known_by_its_mark_not_its_contentpublish_privateused one fixed<sidecar>.tmp, andcreate_privateunlinks whatever is there, so writers clobbered each other's temp. The auditor's probe saw 36/600 writers believe a key that wasn't on disk, and 353 error; their entries were then silently refused.file_lockon the sidecar itself, then re-read once the lock is held: whoever got there first wrote it, and everyone after uses that key. It is written through the locked descriptor, in place. There is no temp file to share, and none is left beside the session. Every journal write adopts the key on disk (one small read), so a store opened before another writer made the key still seals under it. A torn or overwritten sidecar is replaced the same way.publish_privateis gone.session_store::tests::concurrent_first_journal_writers_all_use_one_key: 4 writer processes (this test binary re-run asjournal_key_writer_child) and 8 threads journal at once, released by a go file. The test asserts that all 12 entries carry amacthat verifies against the one key on disk. It fails 3/3 with the oldpublish_privatelogic swapped back in.a_trashed_session_takes_its_journal_key_along_and_brings_it_back: after delete the key is gone from the session dir and present in.trash; after restore it is back and the journal verifies.session_segments_sealing::mcp_task_journal_entries_are_sealed_and_read_backnow journals throughappend_journal, which is where a key would be made. It asserts no key file inside or beside the session dir and nomacon any entry. Withjournal_key_pathreturning a path for codec storage, it fails (["mcp-task-journal.json", "000001.jsonl"]).try_recvdrain) was untested.select!. While a hostbashruns, a prompt is refused as busy, so it can't be queued. So the dependence is now structural instead, and the flush is removed. The flush only mattered when a resumer restarts at a prompt (needs_retry): the previous resumer's finished results might not have landed in the journal yet, and the new one would poll those tasks again.start_mcp_resumer!(&carried)now counts those results as answers whatever the channel holds. Placeholder marks don't need the flush: a placeholder reaches the path only inside a run, and the run-end drain journals its mark before the next prompt.pending's unit tests cover a passed-in result answering its call.writecan corrupt the sidecar, not just delete it.JournalAuthnow say that deleting the key or overwriting it with anything unparseable makes the session keyless. The next journal write then makes a new key that accepts every line already in the file, planted ones included.path.with_extension(...)gavework.1andwork.2one key file.<session>.jsonl.mcp-task-journal.json). A key under the old name is read, and the next journal write carries it over (same bytes) rather than making a new key. The suffixed key is handled by name in trash, restore and hard delete.sessions_named_alike_keep_separate_journal_keys,a_journal_key_under_the_old_name_is_carried_over(also checks that a line planted before the carry-over is still refused)Residual risk (items 5 and 9), stated honestly
A session
.jsonlis ordinary file content. Locally, the journal key is a file beside the session that the agent's user can read. A model withwritecan also delete the key, or corrupt it by overwriting it with anything that does not parse. Either way the session becomes keyless, so the entries there are accepted, and its next journal write makes a new key that accepts every line already in the file, planted ones included. So a model that can also read files and goes looking can forge an entry. The transcript itself is unauthenticated, too: a model that can write the file could always plant an ordinarytool_resultmessage. Locally, the seal turns "append a line" into a deliberate read-then-forge; it does not close forgery.In service mode the store is out of the tools' reach (they run in the tenant's sandbox), so forgery is closed. All of this is documented in ARCHITECTURE "Durable MCP tasks".
The key is made lazily, at the first journal write, so a session that never uses tasks gets no extra file and nothing else in the session layout changes. The trade-off: a session that has never journaled has no key yet, so lines planted into it are accepted at the next open. That is no weaker than the admitted risk above, since a model that can write the file can already plant ordinary transcript messages. A session that has journaled refuses planted lines unless the model also deletes or reads the key.
Verification
mcp_tasks*,session_segments*,serve_harness_deadlinesandrun_session_managementtargets pass locally. The previous round ran the full suite.-D warnings, all targets) is clean;cargo fmtanddprint checkpass.a896d67):slow_compute,confirm_delete,multi_input,failing_jobandtest_tool_with_taskpass, with no straytasks/cancel.🤖 Generated with Claude Code
https://claude.ai/code/session_01JimHGjsfk2Ktm5GxyZJKKk