Repository navigation
fix: fence MCP S3 writes after a Cloud turn is stopped - #505
Merged
Merged
Conversation
…the deck A tool call the agent had already sent kept running on the MCP runtime after a stop and wrote to S3 (dev: run_python saved 4 slides 1.7 s after a cancel; generate_pptx's background previews landed 1.5 s after StopRuntimeSession). - agent/turn_fence.py: each turn mints a generation (time_ns at turn start) and sends it as MCP _meta["sdpm/generation"] on every Presentation Maker client (orchestrator, composer groups; Strands MCPClient meta, so direct compose calls carry it too). It records the decks its tool calls name and, when the turn is stopped (stop event set in the entrypoint's finally, or SIGTERM via stop_guard.begin_turn(on_stop=...)), calls fence_turn once. - servers/remote: fence_turn raises writeFence on the caller's own deck item (conditional UpdateItem, forward only). AwsStorage's S3 client checks the request's fence before every put/delete/copy (one choke point, including the direct _s3.delete_object calls); the fence is bound in offloaded_tool and carried into background threads and upload pools. update_deck is conditional on the fence. Stale requests write nothing and get a "Cancelled" error. Requests without a generation are unchanged. - No IAM change: the advance goes through the MCP runtime role; the agent role keeps no DynamoDB access. Checks: - make lint: All checks passed - make test: 2129 passed, 18 skipped - ash scan --mode local --fail-on-findings: exit 0 - infra unchanged (no npm test / tsc needed)
…ess exits on SIGTERM The fence call ran on a daemon thread that nothing waited for. A turn that shut down quickly (PR-1's dev runs exited 0.5-1.1 s after SIGTERM) let the process exit before fence_turn reached the MCP runtime, leaving the deck unfenced. A non-daemon thread would not help: after the lifespan shutdown uvicorn restores the default SIGTERM handler and re-raises the signal. stop_guard.hold() now keeps the thread TurnFence.advance() returns (from stop_all on SIGTERM, and from agent_stream's finally for a cancel followed by a stop), and the app lifespan's shutdown joins the held threads until the watchdog's exit (SIGTERM + 3 s). Tests: the SIGTERM end-to-end test's fake fence_turn now takes 1 s and must still be recorded (it failed before this fix); a unit test covers the wait and its bound. Checks run on this commit: - make lint: All checks passed - make test: 2130 passed, 18 skipped - ash scan --mode local --fail-on-findings: exit 0 - infra unchanged (no infra npm test / tsc needed)
The agent image COPYs its modules by name and missed the new turn_fence.py, so the dev agent (v266) failed at import: ModuleNotFoundError: No module named 'turn_fence'. Checks: make lint: All checks passed; make test: 2130 passed, 18 skipped
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.
Summary
Stopping a Cloud turn (cancel, or StopRuntimeSession → SIGTERM, #502) does not stop the Presentation Maker MCP calls the turn already sent. They keep running on the MCP runtime and write to S3. On dev,
run_pythonsaved 4slides/*.json1.7 s after a cancel, and in #502's dev run the previews and thumbnail of an earliergenerate_pptxlanded 1.5 s after the stop (5 PutObject, 4 DeleteObject, 1 UpdateItem).notifications/cancelledcannot stop these calls: the server runs withstateless_http=Trueand the tool bodies run inasyncio.to_thread.This PR adds a per-deck write fence. Each agent turn carries a generation; when a turn is stopped, the agent raises the fence on the decks the turn touched, and the MCP server skips every S3 write and deck-record update from a request at or below the fence.
Design
Generation
At the start of each turn the agent mints a generation,
time.time_ns(), in the entrypoint handler (agent/turn_fence.py,TurnFence). A later turn always gets a larger number, whether it runs in the same process or in a new microVM after StopRuntimeSession.Transport: MCP
_meta["sdpm/generation"]The generation is set as Strands
MCPClient(meta=…)on every Presentation Maker client the turn builds: the orchestrator's client (_MCP_FACTORIES[0]) and each composer group's client (composer_mcp_factory). Strands falls back to the client-level meta for every call: model-driven calls (MCPAgentTool→call_tool_async), compose's direct calls (call_tool_with_retry,_start_composing,generate_pptx), andcomposition._call_mcp/ prefetch. Strands mergestraceparentinto the same dict._metawas chosen over an HTTP header because:_metais part of the JSON-RPC body, which the AgentCore MCP proxy forwards unchanged. A custom header would need theX-Amzn-Bedrock-AgentCore-Runtime-Custom-*prefix and arequestHeaderAllowlistchange in infra.Advancing the fence on a stop
The agent records the decks its tool calls name, with a
BeforeToolCallEventhook ondeck_id(compose_slidesnames the deck its composers write). When the turn is stopped,TurnFence.advance()calls a new remote-only MCP tool,fence_turn(deck_ids), once, on a daemon thread, through the turn's own client, so the request carries the turn's generation. Two triggers:basic_agent.agent_stream'sfinallywhen the turn's stop event is set (a cancel, a newer request on the session, or the client going away).stop_guard.stop_all()on SIGTERM, throughbegin_turn(on_stop=fence.advance). It fires immediately, before the 3 s exit watchdog.On SIGTERM the process waits for the fence call. The turn can end, and uvicorn finish its graceful shutdown, before
fence_turnreaches the MCP runtime (#502's dev runs exited 0.5 s and 1.1 s after SIGTERM;fence_turntakes about 0.2 s warm, longer cold). Sostop_guard.hold()keeps the threadadvance()returns, and the app lifespan's shutdown joins the held threads until the watchdog's exit time (SIGTERM + 3 s). A non-daemon thread would not be enough: right after the lifespan shutdown, uvicorn restores the default SIGTERM handler and re-raises the signal, which ends the process without joining threads.A turn that ends normally calls
release()and fences nothing.Server:
fence_turnand the choke point (servers/remote)fence_turnraiseswriteFenceon the caller's own deck item (USER#<sub>/DECK#<id>) with a conditional UpdateItem:attribute_exists(PK) AND (attribute_not_exists(writeFence) OR writeFence < :gen). The fence never moves back, and a caller can fence only decks under its own sub. A generation more than 5 minutes in the future is rejected, so a bad client cannot lock its own decks.The choke point is AwsStorage's S3 client, wrapped in
storage/aws.py(_FencedS3). Everyput_object/delete_object/delete_objects/copy_objectfirst runswrite_fence.check(). This covers the storage methods (upload_file,put_*_json,delete_object_keys,delete_files, …) and the directstorage._s3.delete_objectcalls inserver.py,generate.pyandserver_utils.py. The check is one consistent GetItem on the deck item.The fence for a request is bound in
offloaded_tool's runner when the request has a generation and adeck_idargument. It lives in a ContextVar, whichasyncio.to_threadcopies into the tool's thread. Background threads and pools started by tools get it throughwrite_fence.carry:_run_in_background(run_python's compose and previews),schedule_webp_background(generate_pptx's previews and thumbnail), the compose upload pool, and html_deck's upload pool.A stale request writes nothing. The first check raises
StaleTurn("Cancelled: the agent turn that made this call was stopped, so its writes to deck X were skipped."); the flag is then sticky, so later writes skip without another read. The tool call returns an MCP error with that text.Conditional
updatedAtupdateupdate_deck(which writespptxS3Key/htmlS3Key/updatedAt/thumbnailS3Key) carriesattribute_not_exists(writeFence) OR writeFence < :genfor a fenced request, and aConditionalCheckFailedExceptionbecomesStaleTurn. A write that passed the check just before the fence moved therefore cannot repoint the deck. What remains: an S3 object put in the few milliseconds between the GetItem and the PutObject can land, but the deck record does not point at it.Deviations from a "advance at turn start" design
init_deck_workspace, so the agent cannot name the deck at turn start. Advancing at turn start would also fence the previous turn when it finished normally, dropping its background previews/thumbnail if the next message arrived within seconds.IAM
No IAM or infra change. The agent role still has no DynamoDB access (removed in #499); adding it back just to write one attribute would widen the agent's permissions for every turn. Instead the advance goes through the MCP server's
fence_turntool, under the MCP runtime role, which already hasgrantReadWriteDataon the deck table forupdate_deck. Authorization comes from the key: the item is addressed by the caller's JWT sub and the condition requires it to exist.fence_turnis in no mode'sallowed_tools, so no model is offered it; the agent calls it by name withcall_tool_sync.Unchanged behaviour
servers/localis untouched, so Local mode is unaffected.Known limits
init_deck_workspacecan still create the new, empty deck (the deck is unknown until the call returns).generate_pptxandfinalize_html_deckupload first and stop onStaleTurn.generate_pptxlogsERROR generate_pptx failedwith a traceback on the MCP runtime (theStaleTurnis returned as a tool error), so each stop adds one ERROR line to error searches.Checklist
make allpasses (ruff + pytest):make lintall checks passed,make test2130 passed, 18 skippedash scan --mode local --fail-on-findingspassesdocs/en/architecture.md,servers/remote/README.md)CHANGELOG.md[Unreleased]updated for user-facing changesTesting
Unit and integration tests
tests/test_remote_write_fence.py(new): without a generation every write goes through; a current generation writes; a stale generation writes nothing and the skip is sticky; the conditional update loses to a fence that moves between the check and the update;advanceonly moves forward and only on the owner's item; meta validation; background threads and pools carry the fence; over MCP, a stale call is cancelled, a current call writes, a call without meta writes, andfence_turnfences the caller's decks and requires a generation; a real StrandsMCPClientover streamable HTTP to the server's own stateless FastMCP app on 127.0.0.1 carries the generation end to end.tests/test_agent_turn_fence.py(new): a stopped turn fences the decks it named, once; a finished turn fences nothing; a stop before bind does nothing; the orchestrator and composer clients carry the meta;mcp_agentcore_runtimepasses the meta; the entrypoint advances on cancel and releases on a normal finish;stop_allrunson_stop; shutdown waits for the held fence call, but not past the watchdog's exit time.tests/test_agent_stop_guard.py: in the SIGTERM end-to-end test the fakefence_turntakes 1 s, longer than the turn takes to shut down. The test requires exactly onefence_turn(["deck1"]), recorded at least 1 s after the signal (before this fix the process exited first and none was recorded), and that every client of the turn, composers included, and every direct call before the signal carry the same generation.tests/test_remote_html.py: tool count is now 21 (fence_turn).Dev verification
Deployed SdpmRuntime + SdpmAgent to the dev account (infra unchanged, so SdpmData / SdpmAuth / SdpmWebUi were not redeployed). The first deploy surfaced a bug:
agent/Dockerfilecopies agent modules by name and did not include the newturn_fence.py, so the agent failed at import. Commit c094195 adds it; the run below used the fixed image.Procedure: in the Cloud Web UI (Playwright), start a new deck on the fast path asking for 4 slides, then press "Stop generation" at a chosen point after the "Composing slides…" card appears (the Web UI sends StopRuntimeSession; 200 every time). Checked the agent and MCP runtime logs, the MCP-side S3/DynamoDB write spans in
aws/spans, S3 LastModified under the deck's prefixes, the deck item'swriteFence, and AgentCore Memory events.fence_turnon MCP (after SIGTERM)generate_pptxhad reached MCP 0.38 s before SIGTERMgenerate_pptx's PPTX upload at +1.51 sResults:
_metacrosses the AgentCore MCP proxy. In all 5 runs,fence_turnon the MCP runtime received the agent's generation unchanged. In run 5 the stopped turn'sgenerate_pptxcarried the same generation and was fenced.fence_turn's own UpdateItem. Run 5 reproduces fix(agent): stop a Cloud turn for good on SIGTERM and on cancel #502's straggler case:generate_pptxtried to upload the PPTX 1.51 s after SIGTERM, loggedwrite fence: deck …: generation … is at or below the fence …; skipping this request's writesand stopped withStaleTurn. No PPTX, preview, thumbnail or deck-record update followed;pptxS3KeyandupdatedAtkept their pre-stop values.slides/*.json, 5compose/*.json, PPTX, 4 previews, thumbnail),update_deck's conditional update movedpptxS3KeyandthumbnailS3Key, and noskippingappeared.fence_turnarrived before exit in every run.Test decks were soft-deleted, their Memory events deleted, and MCP runtime sessions stopped afterwards.
Boundaries touched
None.