Skip to content

fix: fence MCP S3 writes after a Cloud turn is stopped - #505

Merged
ShotaroKataoka merged 3 commits into
mainfrom
fix/mcp-write-fence
Nov 3, 2026
Merged

ShotaroKataoka merged 3 commits into
mainfrom
fix/mcp-write-fence

Conversation

@ShotaroKataoka

Copy link
Copy Markdown
Contributor

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_python saved 4 slides/*.json 1.7 s after a cancel, and in #502's dev run the previews and thumbnail of an earlier generate_pptx landed 1.5 s after the stop (5 PutObject, 4 DeleteObject, 1 UpdateItem). notifications/cancelled cannot stop these calls: the server runs with stateless_http=True and the tool bodies run in asyncio.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), and composition._call_mcp / prefetch. Strands merges traceparent into the same dict.

_meta was chosen over an HTTP header because:

  • _meta is part of the JSON-RPC body, which the AgentCore MCP proxy forwards unchanged. A custom header would need the X-Amzn-Bedrock-AgentCore-Runtime-Custom-* prefix and a requestHeaderAllowlist change in infra.
  • Strands supports client-level meta directly.
  • No transport code changes.

Advancing the fence on a stop

The agent records the decks its tool calls name, with a BeforeToolCallEvent hook on deck_id (compose_slides names 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's finally when 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, through begin_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_turn reaches the MCP runtime (#502's dev runs exited 0.5 s and 1.1 s after SIGTERM; fence_turn takes about 0.2 s warm, longer cold). So stop_guard.hold() keeps the thread advance() 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_turn and the choke point (servers/remote)

fence_turn raises writeFence on 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). Every put_object / delete_object / delete_objects / copy_object first runs write_fence.check(). This covers the storage methods (upload_file, put_*_json, delete_object_keys, delete_files, …) and the direct storage._s3.delete_object calls in server.py, generate.py and server_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 a deck_id argument. It lives in a ContextVar, which asyncio.to_thread copies into the tool's thread. Background threads and pools started by tools get it through write_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 updatedAt update

update_deck (which writes pptxS3Key / htmlS3Key / updatedAt / thumbnailS3Key) carries attribute_not_exists(writeFence) OR writeFence < :gen for a fenced request, and a ConditionalCheckFailedException becomes StaleTurn. 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

  • Turn start writes nothing to DynamoDB. The turn mints its generation locally; only a stop writes the deck item. The Cloud invoke payload has no deckId and a new deck is created mid-turn by 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.
  • The comparison is "at or below the fence". A stopped turn's generation is the fence, and any later turn is larger.

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_turn tool, under the MCP runtime role, which already has grantReadWriteData on the deck table for update_deck. Authorization comes from the key: the item is addressed by the caller's JWT sub and the condition requires it to exist. fence_turn is in no mode's allowed_tools, so no model is offered it; the agent calls it by name with call_tool_sync.

Unchanged behaviour

  • Requests without a generation are unchanged: every write goes through. This covers the local server, other MCP clients, and older agents.
  • servers/local is untouched, so Local mode is unaffected.
  • No change to the Web UI, the API Lambda, or Memory handling (fix(agent): stop a Cloud turn for good on SIGTERM and on cancel #502's gate already drops the agent's own Memory writes).

Known limits

  • The fence call is best effort. On SIGTERM the process waits for it until the watchdog exits (3 s after the signal); it normally takes about 0.3 s. If it fails or is still running at exit, it is logged and that stop's stragglers can still write.
  • A stop during init_deck_workspace can still create the new, empty deck (the deck is unknown until the call returns).
  • The generation uses the wall clock. A new turn must start later than the stopped turn started, which holds unless two microVMs' clocks are skewed by more than the stopped turn's age.
  • KB sync (S3 Vectors) is not behind the S3 wrapper. In practice it is skipped, because generate_pptx and finalize_html_deck upload first and stop on StaleTurn.
  • A fenced generate_pptx logs ERROR generate_pptx failed with a traceback on the MCP runtime (the StaleTurn is returned as a tool error), so each stop adds one ERROR line to error searches.

Checklist

  • make all passes (ruff + pytest): make lint all checks passed, make test 2130 passed, 18 skipped
  • ash scan --mode local --fail-on-findings passes
  • Docs updated if behavior/paths changed (docs/en/architecture.md, servers/remote/README.md)
  • CHANGELOG.md [Unreleased] updated for user-facing changes
  • Commit messages follow Conventional Commits

Testing

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; advance only 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, and fence_turn fences the caller's decks and requires a generation; a real Strands MCPClient over 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_runtime passes the meta; the entrypoint advances on cancel and releases on a normal finish; stop_all runs on_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 fake fence_turn takes 1 s, longer than the turn takes to shut down. The test requires exactly one fence_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/Dockerfile copies agent modules by name and did not include the new turn_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's writeFence, and AgentCore Memory events.

Run Stop point fence_turn on MCP (after SIGTERM) Writes blocked by the fence S3 writes after SIGTERM Memory events after SIGTERM
1 3 s after the compose card (composers just started) +0.30 s none in flight 0 0
2 20 s after the compose card (layout composer mid model call); follow-up message on the same deck 45 s later +0.33 s none in flight 0 (before the follow-up) 0
3 0.7 s after the compose card +0.30 s none in flight 0 0
4 0.5 s after the layout composer's first activity +0.36 s none in flight 0 0
5 0.3 s after "4 / 4 created"; generate_pptx had reached MCP 0.38 s before SIGTERM +0.37 s generate_pptx's PPTX upload at +1.51 s 0 0

Results:

  • _meta crosses the AgentCore MCP proxy. In all 5 runs, fence_turn on the MCP runtime received the agent's generation unchanged. In run 5 the stopped turn's generate_pptx carried the same generation and was fenced.
  • No S3 write to the deck after a stop in any run. The only MCP-side write span after SIGTERM was 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_pptx tried to upload the PPTX 1.51 s after SIGTERM, logged write fence: deck …: generation … is at or below the fence …; skipping this request's writes and stopped with StaleTurn. No PPTX, preview, thumbnail or deck-record update followed; pptxS3Key and updatedAt kept their pre-stop values.
  • Normal turns still write. Writes before each stop landed. Run 2's follow-up turn on the fenced deck ran to completion (15 S3 writes: 4 slides/*.json, 5 compose/*.json, PPTX, 4 previews, thumbnail), update_deck's conditional update moved pptxS3Key and thumbnailS3Key, and no skipping appeared.
  • The agent process exited 0.64–3.06 s after SIGTERM; fence_turn arrived before exit in every run.

Test decks were soft-deleted, their Memory events deleted, and MCP runtime sessions stopped afterwards.

Boundaries touched

None.

…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
@ShotaroKataoka
ShotaroKataoka merged commit 68b5dee into main Nov 3, 2026
17 of 18 checks passed
@ShotaroKataoka
ShotaroKataoka deleted the fix/mcp-write-fence branch November 3, 2026 11:04
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