[fix] Resume a gated turn under the config it was running (10/12) - #5689
Draft
ardaerzin wants to merge 8 commits into
Draft
[fix] Resume a gated turn under the config it was running (10/12)#5689ardaerzin wants to merge 8 commits into
ardaerzin wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change carries effective session configuration from SDK request hydration through runner interaction records and approval resumes. It adds redaction and size limits, preserves legacy references-only behavior, and updates web approval handling to answer interactions directly. ChangesEffective configuration capture
Runner and API replay
Web approval handling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SDK
participant Runner
participant InteractionStore
participant ApprovalAPI
participant MobileClient
SDK->>Runner: Send session run with effectiveParameters
Runner->>InteractionStore: Persist interaction parameters
MobileClient->>ApprovalAPI: Respond to approval interaction
ApprovalAPI->>InteractionStore: Read stored parameters
ApprovalAPI->>Runner: Resume with parameters and references
Runner-->>ApprovalAPI: Complete resumed turn
ApprovalAPI-->>MobileClient: Return interaction response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 15:13
c4025ac to
0cae609
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 15:13
6101ac7 to
f957a88
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 16:14
0cae609 to
e46be44
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 16:14
f957a88 to
f34d0e1
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 16:47
e46be44 to
9d900a8
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 16:47
f34d0e1 to
4223629
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 17:34
9d900a8 to
5321274
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 17:34
4223629 to
fed6233
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 17:54
5321274 to
1c4c784
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 17:54
fed6233 to
0e3c2cb
Compare
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/mobile/src/features/chat/useApprovalActions.ts (1)
69-105: 📐 Maintainability & Code Quality | 🔵 TrivialAdd unit test coverage for the
submitconflict-handling path.
approvalTargets.test.tscovers only the pureselectApprovalTargetsselector. The new logic insubmit— toleratingisInteractionConflict, countinganswered, and re-arming toidlewhen every target was already answered — is not covered by any test in this batch. This logic drives the mobile approval flow's resilience to already-answered gates.Add a test (with
queryInteractionsandrespondInteractionmocked) that exercises: all targets succeed, all targets conflict (re-arm to idle immediately), and a mixed success/conflict/non-conflict-failure case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dafe74b-d5cb-4dd4-bf47-b9e5ebb3215b
📒 Files selected for processing (39)
api/oss/src/core/sessions/interactions/dtos.pyapi/oss/src/tasks/asyncio/sessions/interactions_dispatcher.pyapi/oss/tests/pytest/unit/sessions/test_interactions_dispatcher.pydocs/design/agenta-mobile/plans/2026-07-27-mobile-approvals-steering.mddocs/design/agenta-mobile/plans/2026-07-29-effective-turn-config.mdsdks/python/agenta/sdk/agents/adapters/local.pysdks/python/agenta/sdk/agents/adapters/sandbox_agent.pysdks/python/agenta/sdk/agents/dtos.pysdks/python/agenta/sdk/agents/handler.pysdks/python/agenta/sdk/agents/interfaces.pysdks/python/agenta/sdk/agents/utils/effective_config.pysdks/python/agenta/sdk/agents/utils/wire.pysdks/python/agenta/sdk/agents/wire_models.pysdks/python/agenta/sdk/middlewares/running/resolver.pysdks/python/oss/tests/pytest/integration/agents/_fake_runner_backend.pysdks/python/oss/tests/pytest/unit/agents/conftest.pysdks/python/oss/tests/pytest/unit/agents/golden/run_request.pi_core.jsonsdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.pysdks/python/oss/tests/pytest/unit/agents/test_wire_contract.pysdks/python/oss/tests/pytest/unit/test_batch_fold_stream_contract_routing.pysdks/python/oss/tests/pytest/unit/test_invoke_real_handlers_negotiation_routing.pysdks/python/oss/tests/pytest/utils/test_resolver_middleware.pyservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/protocol.tsservices/runner/src/sessions/interactions.tsservices/runner/tests/unit/interactions-parameters.test.tsservices/runner/tests/unit/wire-contract.test.tsweb/mobile/src/features/chat/approvalStamp.tsweb/mobile/src/features/chat/approvalTargets.tsweb/mobile/src/features/chat/useApprovalActions.tsweb/mobile/src/lib/auth.tsweb/mobile/tests/unit/approvalStamp.test.tsweb/mobile/tests/unit/approvalTargets.test.tsweb/packages/agenta-chat/src/transport/agentResumeRequest.tsweb/packages/agenta-chat/tests/unit/transport/agentResumeRequest.test.tsweb/packages/agenta-entities/src/session/api/api.tsweb/packages/agenta-entities/src/session/core/schema.tsweb/packages/agenta-entities/src/session/index.tsweb/packages/agenta-entities/tests/unit/session-interaction-schema.test.ts
💤 Files with no reviewable changes (3)
- web/mobile/src/lib/auth.ts
- web/mobile/tests/unit/approvalStamp.test.ts
- web/mobile/src/features/chat/approvalStamp.ts
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 18:26
1c4c784 to
551dc6f
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 18:26
0e3c2cb to
86c74ff
Compare
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 20:04
551dc6f to
28906c3
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 20:04
86c74ff to
ea4d80e
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 3, 2026 22:29
28906c3 to
090ed5d
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 3, 2026 22:29
ea4d80e to
e13f3f2
Compare
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 4, 2026 18:26
090ed5d to
ff0eb26
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 4, 2026 18:26
e13f3f2 to
e1bd8b8
Compare
A references-only /invoke (no data.parameters, no data.revision) never hydrated its references, so the run silently used the service's registered default configuration instead of the referenced revision's. This is the mobile resume path; desktop was unaffected because it always sends inline data.parameters. The hydration gate consulted the revision returned by resolve_revision, which falls back to RunningContext.revision — pre-seeded by the decorator with the registered default config. That revision is always populated for the agent builtin, so the gate read "already configured" and skipped hydration. seed_empty_parameters_from_configuration also ran before the gate, so it could never have observed empty parameters anyway. Decide hydration purely from caller-supplied config (data.parameters or data.revision), and seed the registered default only after a hydration attempt has been made and come back empty, so it stays a fallback rather than a pre-emption.
Grounded in two live experiments on the EE dev stack: - A dirty-config run's session_interactions row carries only workflow + workflow_variant references (no workflow_revision), so a references-only resume hydrates the variant HEAD, not the draft the turn ran under. A committed run's revision reference is pinned and is therefore already immune to later commits. - A warm approval resume keeps the sandbox's acquire-time model and secrets while re-reading the permission map from the incoming (hydrated, committed) request -- a split brain where the approval UI enforced one policy and the resumed turn enforces another. Cold replay runs the committed config end to end against a draft transcript. Recommends stamping the effective parameters on the interaction row at gate creation (SDK emits, runner echoes, API replays inline), which needs no migration, with a detect-and-defer fallback for pre-change rows.
ardaerzin
force-pushed
the
feat/sessions-watch-and-liveness
branch
from
August 4, 2026 19:26
ff0eb26 to
e062f59
Compare
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 4, 2026 19:26
e1bd8b8 to
2ddfa94
Compare
A HITL gate answered from a client that cannot reproduce the turn's config (mobile, the M2 dispatcher) resumes references-only, so the SDK hydrates the referenced variant's HEAD revision instead of what the gated turn was running - wrong model, wrong instructions, and wrong tool permissions on a dirty run. The SDK is the only component holding the post-hydration config, so it emits it: the handler carries its resolved data.parameters onto SessionConfig, and request_to_wire puts it on the wire as the opaque effectiveParameters. Emitted only for a session run (nothing else can park a gate), so an ad-hoc run's payload stays byte-identical to the golden contract. Redacted (an MCP connection's static headers are the one place the schema permits a raw credential; the vault-key refs survive) and capped at 64 KB, dropped whole with a log line rather than truncated into invalid JSON. The runner echoes the blob onto the interaction row; the answering client replays it as data.parameters, which suppresses hydration and reproduces the turn.
The runner writes the durable row at the exact moment a gate parks, so it is where the
turn's config has to be recorded - but it had no access to it (there was no parameters
field on the /run wire at all). The SDK now stamps effectiveParameters; this echoes it
verbatim into the row's data.parameters, alongside data.request and data.references.
Opaque by design: the runner never reads inside the blob and derives no behavior from it.
It is deliberately NOT in configFingerprint - that hash decides warm resume vs cold replay,
and the blob is a projection of fields already hashed, so including it would let a cosmetic
serialization change evict every warm session. A turn with no stamped config omits the KEY
(not null, not {}): a legacy row keeps its exact shape, and an empty inline config would
still suppress hydration server-side and resume a toolless agent.
Two links in the same chain. The DTO: SessionInteractionData is a closed pydantic model with the default extra="ignore", and the postgres mapping round-trips through it on write and on read even though data is a schemaless json column - so the runner's new data.parameters was being dropped twice with no error anywhere. Declaring the field is the whole fix; no migration. The dispatcher: when the row carries a config, send it inline on the resume. The SDK resolver decides hydration purely from what the caller sent, so inline parameters suppress it and the run continues under the config the gate was raised against instead of the referenced variant's HEAD revision. References still ride along as attribution. A row written before the runner stamped configs has none and produces the byte-identical references-only body this dispatcher has always sent.
Records what landed on the backend lanes of the effective-turn-config plan and notes in the approvals round-trip that a pre-change row (or one over the stamp cap) is still answerable, degrading to reference hydration.
The lite resume answered a HITL gate with a references-only body, so the SDK
hydrated the referenced variant's HEAD revision. For a dirty run that is a
different model and — the security-relevant half — a different tool-permission
map than the gate was approved under.
The runner now stamps the turn's post-hydration config onto the interaction row
as data.parameters. Read it off the same row the references come from and send
it inline, which suppresses hydration and reproduces the turn exactly.
Emit the key ONLY when the stamped config is a non-empty object: an empty {}
also suppresses hydration and would run an unconfigured agent. Rows without it
(legacy, over-cap, pre-stamping runner) keep today's references-only path.
Declare parameters on the interaction zod schema too — objects strip unknown
keys by default, so an undeclared field would be silently dropped and the fix
would no-op with everything green.
The phone built its own /invoke resume from a stamped records replay. That lands as a
NEW turn (the keepalive trips approval-mismatch (history) -> evict + cold), so the parked
gate is never matched and the interaction row stays pending — the desktop keeps showing
"Approval needed to continue" even after the tool ran.
Call POST /sessions/interactions/{id}/respond instead: the backend CAS-flips the row to
responded and the interactions worker rebuilds the history server-side and replays the
gate's stamped effective config, so the resume lands warm. Approve-all fans out one
respond per pending gate; a 409 (already answered) settles to idle instead of erroring.
respondInteraction now throws instead of swallowing the failure — a mutation's caller has
to tell a real failure from an already-answered gate (isInteractionConflict).
Drops the mobile-only invoke plumbing: approvalStamp.ts and the invoke bearer header.
ardaerzin
force-pushed
the
feat/effective-turn-config
branch
from
August 4, 2026 21:27
2ddfa94 to
d13945a
Compare
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
1 participant
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.
Context
An approval answered from a phone could resume the turn under the wrong configuration. The gate carried references to the workflow, so the resume hydrated the referenced variant's HEAD revision rather than the config the paused turn was actually running. If the agent had been invoked with a draft config, or the variant moved on, the resumed turn ran as a different agent than the one you approved.
This is one contract across three tiers, which is why it is one PR: splitting it would leave a wire field written by nobody or read by nobody.
Changes
The SDK stamps the post-hydration config it is about to run onto the
/runwire aseffectiveParameters(session runs only, credential-stripped, capped at 64KB). The runner echoes it onto the interaction row of any gate that turn parks. The API replays it as the resume'sdata.parameters, which suppresses reference hydration server-side and reproduces the turn, tool permissions included.Also fixed here: the SDK hydrated references even when the caller had sent an inline config, so a caller-supplied configuration was silently discarded. Hydration is now decided from what the caller actually sent.
Tests / notes
protocol.tsandwire.pyand pinned by shared golden fixtures; both sides' contract tests are updated together.parametersand resume via references exactly as before.