Skip to content

Sandbox-binding campaign (#1767): the lifecycle writes it leaves unfenced #2021

Description

@jhgaylor

Found while reviewing the nine fix: PRs that bind conversation lifecycle
writes to the actor's sandbox (#1996, #1998, #1999, #2000, #2001, #2002, #2003,
#2006, #2007 — all approved 2026-09-12). Each of those fences the write it
names, and the mechanism is sound: state.sandbox_id is set once from init args
(conversation_server.ex:463) and never reassigned, so a mismatch is genuinely
a stale actor.

This issue collects what the campaign does not cover. Every item below was
verified at the stack tip (codex/p2-1767-machine-gone-senders), and none is
claimed by any of the 25 codex/p2-1767-* branches.

1. mark_orphan/3 reaches the write unbound

apps/fountain/lib/fountain/conversations/conversation_server.ex:1443

defp mark_orphan(_state, running_turn, why),
  do: Conversations._unsafe_orphan_turn(running_turn, why)

It is handed state and discards it, and its turn comes from
find_running_turn(conv_id), which is scoped by conversation and never by
sandbox. It is reached from the reattach path:1330 :1339 :1390
:1438 — which runs inside a live actor.

Failure: old actor A on sandbox S1 restarts and reattaches; the conversation has
since been rebound to S2 with actor B running a turn; A's list_sessions retry
expires under a partition; mark_orphan flips B's live turn to
interrupted and the conversation to idle. That is the exact bug class this
campaign exists to close, in a function the campaign edits.

Do not confuse this call site with AutonomousTurnReaper's unbound one
(apps/fountain/lib/fountain/workers/autonomous_turn_reaper.ex:69) — that is
safe because it is unbound, since it only fires when
ConversationServer.whereis/1 is nil and no live actor exists.

2. handle_call(:release_conv, ...) is a third unfenced conversation-ending path

conversation_server.ex:1609

def handle_call(:release_conv, _from, state) do
  state = drop_connection(state, "released")
  conv = Conversations._unsafe_get_conversation!(state.conversation_id)
  {:ok, _} = Conversations.update_conversation(conv, %{status: "terminated"})

The clause above it refuses on current_turn ({:error, :busy}), but that is
the stale actor's own turn — an old actor holding no local turn releases
happily while a new actor runs one on the new sandbox.

This one compounds. _unsafe_complete_turn/3 guards on
conv.status not in ["terminated", "failed"], so a stale release does not just
flip the row to a terminal state, it then makes the new actor's legitimate turn
completion refuse as well. _unsafe_finish_conversation_termination/2 drops
straight in.

3. The remaining unbound update_conversation/2 status writes

The campaign fenced the ending writes it enumerated and left these going
through plain update_conversation(conv, ...):

  • %{status: "running"} at conversation_server.ex:1419 and :2317 (the
    pre-spawn write in the fresh-turn path, from a possibly stale struct)
  • %{status: "failed"} at :627, :790, :830, :869, :1027, :1039

Reachability by a stale actor is unproven per site — several are provisioning
paths where the writer is necessarily the current actor. This wants an audit
pass deciding which need the fence, not a blanket change.

4. follow_cotenants breaks the guard's premise

apps/fountain/lib/fountain/conversations.ex:5282 rebinds a co-tenant's row from
a sibling conversation's provisioning and starts no server for it. So the
guard's assumption — "a rebind means a newer actor owns the status" — does not
hold: it casts machine_gone with the old id, update_alls the rows to S2, and
the receiver's finish call sees S2 vs S1 and :noops. The conversation is left
status: "running" with no server, where the old code idled it immediately.
Healed only by AutonomousTurnReaper after 30+15 min, or not until the next
prompt. The fix belongs with the sender, which already owns both the
update_all and the stage publish.

5. Nothing reconciles a :noop

Every PR in the chain defers to a "durable forced-teardown recovery" follow-up,
and none adds a reconciler. While an abandoned turn row stands
(running, started_at set, ended_at: nil, and finish/4 nils row so
terminate/2's orphan_on_normal_stop net cannot see it):

  • _unsafe_sandbox_busy_elsewhere? (conversations.ex:2036) counts
    t.status == "running", so a live co-tenant never idle-parks and bills to max
    lifetime;
  • SandboxUsage clips an unended turn to the sandbox end
    (sandbox_usage.ex:445-467), inflating busy_seconds/turn_seconds on the
    replacement sandbox, while CreditPricer charges nothing because it needs
    ended_at;
  • _unsafe_running_turns_elsewhere/2 joins through the conversation's current
    sandbox, so the phantom consumes a capacity slot on the replacement (ADR
    0023). With gemini or opencode (concurrency: 1 in the adapter table), a
    co-tenant's next turn is refused :sandbox_at_capacity until a backstop
    fires.

Two backstops do eventually close it — the next ConversationServer start via
reattach_running_turnmark_orphan (which works only because of item 1's
unbound read), and AutonomousTurnReaper once no server holds the conversation.
A sweep for abandoned running rows belongs here.

6. One-line hardening worth taking now

apps/fountain/lib/fountain/conversations/turn_machine.ex:111

sandbox_id: Map.get(state, :sandbox_id),

Every sibling field in from_state/1 uses direct access; only the fence input
is read leniently, and @type admits String.t() | nil so dialyzer is blind to
a nil. That value is the fence input at :624 (_unsafe_complete_turn), :673
(_unsafe_interrupt_turn) and :698 (_unsafe_idle_interrupted_turn), each
comparing against a NOT NULL column — so a nil matches nothing and silently
:noops every completion, interrupt and idle, with no crash and no warning.
A renamed key or a partial state map turns the whole campaign's guard off
quietly.

No fixture needs the lenient read: the round-trip test at
apps/fountain/test/fountain/conversations/turn_machine_test.exs:62 already
supplies sandbox_id: "s", and the live state gets it from Keyword.fetch! at
init. state.sandbox_id is strictly better, and four of the nine PRs reuse this
helper.

Also observed, not defects

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Important planned work; not an emergency.area:conversationsConversation lifecycle, turns, streaming, recovery and orchestration.area:sandboxSandbox providers, machine identity, capabilities and lifecycle.scope:largeWork requiring broader investigation, design or coordinated changes; scope, not urgency.type:bugExisting behavior is incorrect or broken.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions