refactor(conversations): Launch, part c: attach_conversation and its checks - #2245
Conversation
…checks
Moves the attach arm of the launch into `Fountain.Conversations.Launch`:
`attach_conversation/3` (the `@doc false` door stage 7b opened), the two
checks `check_attachable/4` and `check_attach_capacity/3`, and the private
helpers used only by them: `create_attached_conversation/3`,
`deliver_attach_prompt/3`, `check_sandbox_api_attach/2` and
`_unsafe_sandbox_runtime/1`.
`attach_conversation/3` had no caller outside `Launch` (it was already
`@doc false`, not part of `Conversations`' public surface), so it moves
in as a private `defp` with no delegate left behind. The three call sites
in `Launch.start_conversation/2` (the `sandbox_id` clause and both
`{:home, _}` branches) become local calls.
`check_attachable/4` and `check_attach_capacity/3` were already `defp` in
`Conversations`, called by nothing outside it; they move the same way.
`_unsafe_sandbox_runtime/1` had exactly one caller, `check_attachable/4`,
so it moves too, closing the cross-module `_unsafe_` call the move would
otherwise have opened (credo's unsafe_call_ownership check would have
required an ownership comment there; moving the caller and callee
together keeps the call local instead). `check_attach_capacity/3`'s
remaining cross-module `_unsafe_sandbox_at_capacity?/3` call (that
function stays in `Conversations`, shared with `turn_machine.ex`) gets
an ownership comment naming the scoped sandbox read.
`unbind_rotated_channel/2` and `restore_rotated_channel/2` (opened public
in `Launch` in stage 7b for `Conversations`' admission and
`fail_initial_start`) now have every caller inside `Launch`, since
`create_attached_conversation/3` and `deliver_attach_prompt/3` moved in;
their doc comments are updated to say so, but they stay `def` since
tightening their visibility is outside this stage's scope.
Doc-comment references to the moved functions' old location are updated
in `conversations.ex` and `reapply.ex`; no code path outside the moved
functions changes.
`start_conversation/2` (both the fresh path from stage 7b and the attach
path here) is now fully owned by `Launch`; `Conversations.start_conversation/2`
is a one-line delegate. Wake, interrupt, terminate and release stay in
`Conversations`/`ConversationServer` for their own stages.
No server change, no behaviour change, no changelog fragment.
Closes #2218
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>
BinaryBourbon
left a comment
There was a problem hiding this comment.
APPROVED — independent adversarial review
Reviewed head 144b3d6019daf8e9ba0f6295be9cd0de4f1873fe against base 72e753d292ceec9b6539da9710966c081bb6ea51. No actionable regressions found.
Covered all changed paths:
apps/fountain/lib/fountain/conversations.exapps/fountain/lib/fountain/conversations/launch.exapps/fountain/lib/fountain/conversations/reapply.ex
I traced explicit sandbox attachment, persistent-home reuse and its creation-race fallback through the context delegates and API caller. Checked tenant-scoped agent/sandbox lookup; environment, vault and inference allowlists; sandbox identity/runtime/reset/capacity checks; callback-credential isolation; suspension/billing/platform-inference gates; source and machine lock order; transactional conversation/allowance insertion; channel restoration and row removal on prompt refusal; and audit attribution after commit. The existing attach, controller, sandbox API access and execution-limit admission tests cover the relevant denial and rollback paths.
Local validation: an exact-revision Elixir AST comparison passed for all seven moved functions (10 clauses), normalizing metadata, def/defp, and the intentional Conversations/Launch call qualification changes. Existing Launch and remaining Conversations definitions were equivalent, and Reapply definitions were unchanged. I separately checked alias targets, the shared advisory-lock namespace, public helper exports, and repository-wide callers of both removed context names; no callers were left behind. git diff --check passed.
Verified current-head GitHub evidence: CI required and Detect secrets passed. Application tests and the full verification recipe were not rerun locally. This is an independent source review, not a Review Loop service verification attestation.
Stage 7c of #2175 (one owner per conversation lifecycle verb), stacked on stage 7b (#2243, since merged to main).
What moved
Into
Fountain.Conversations.Launch:attach_conversation/3— the@doc falsedoor stage 7b opened inConversations. It had no caller outsideLaunch(the three call sites arestart_conversation/2'ssandbox_idclause and both{:home, _}branches), so it moves in as a privatedefpwith no delegate left behind.check_attachable/4(all three clauses) andcheck_attach_capacity/3(both clauses) — alreadydefpinConversations, called by nothing outside it.create_attached_conversation/3,deliver_attach_prompt/3,check_sandbox_api_attach/2._unsafe_sandbox_runtime/1— its only caller wascheck_attachable/4. Moving it with its caller keeps that_unsafe_*call local instead of opening a cross-module call credo'sunsafe_call_ownershipcheck would have required a comment for.Attach still inserts the conversation
idle, reads the sandbox through the tenant-scopedConversations.get_sandbox/2(kept as the first fetch), reserves the source inside the insert transaction, and gates spend withFountain.Billing.check_spend/1— moved unchanged, none added (ADR 0030 decision 6).What stayed, and why
claim_sandbox,sandbox_retired?andprevent_sandbox_revivalstay inConversations'update_sandbox; attach keeps callingConversations.update_sandbox(retirement rejection at admission, Make sandbox reset and retirement atomic with turn admission #1767).check_attach_capacity/3's remaining cross-module_unsafe_sandbox_at_capacity?/3call stays inConversations(shared withturn_machine.ex), so it gets an# ownership: ...comment naming the scoped sandbox read instead of moving.resolve_vault_id,resolve_environment_id,resolve_inference_credential_id,resolve_permission_policy,resolve_parent_id,check_execution_limits,insert_conversation_row,reserve_inference,after_conversation_created,record_execution_allowance_created,broadcast_sidebar_updateandrequire_provider_commit_boundaryall stay@doc falsedoors inConversations: each still has at least one caller insideConversationsor another module besidesLaunch(checked by grep), so none is Launch-only yet.unbind_rotated_channel/2andrestore_rotated_channel/2(opened public inLaunchin stage 7b, forConversations' admission andfail_initial_start) now have every caller insideLaunch, sincecreate_attached_conversation/3anddeliver_attach_prompt/3moved in. Their doc comments are updated to say so, but they staydef— tightening their visibility is outside this stage's scope.Left for stage 8 or a follow-up
Nothing else stage 7b opened in
Conversationsbecame Launch-only as a result of this move (checked each caller above); no candidates to flag.Delegates
None added:
attach_conversation/3,check_attachable/4andcheck_attach_capacity/3were never called from outsideLaunchto begin with.Doc accuracy
Updated stale cross-module references to the moved functions' old location:
conversations.ex's comment abovecheck_sandbox_api_name/2,reapply.ex's three prose references tocheck_attachable/4, andLaunch.resolve_admission_inference/5's doc (it now serves two callers inside the same module instead of one inConversations).Proves unchanged
Run individually from
apps/fountainagainstfountain_test_2175_7c:test/fountain/conversations/attach_test.exstest/fountain/conversations/sandbox_mode_test.exstest/fountain/conversations/orphaned_home_test.exstest/fountain/conversations/termination_attach_order_test.exstest/fountain/conversations/retirement_admission_order_test.exstest/fountain/conversations/reset_admission_order_test.exstest/fountain/conversations_start_test.exstest/fountain/team_test.exstest/fountain_web/controllers/conversation_controller_test.exstest/fountain/audit_guardrail_test.exsRun together: 354 tests, 0 failures.
../../ee/test/fountain/credits_enforcement_test.exs: 11 tests, 0 failures.Gates
mix precommit --full(10 stages, 417s): PASSED, run before rebasing onto the merged 7b.origin/main(72e753d, 7b's squash-merge):mix precommit compile format credo(3 stages, 8s): PASSED. The rebase replayed only this stage's own commit (diffstat unchanged), so the full suite from before the rebase still applies.Lengths
conversations.ex: 4972 → 4397 (against the pre-stage base; the file also picked up unrelated shrinkage from Wake stages 3/3b merging underneath this branch).launch.ex: 783 → 1051.No server change, no behaviour change, no changelog fragment.
Closes #2218
🤖 Generated with Claude Code