[AGE-4290] fix(sdk): Read the session facts in the agent service, not off the wire

[mmabrouk]

Context

Fixes #6661. Rename a session, ask the agent what the session is called, and it answers with the old name. It keeps answering the old name on the next turn too, even when you tell it to use only the facts it was given for this turn.

The three per-turn session facts (the agent's display name, the session's name, the first-turn flag) shipped in #6638. The API stamps them on request.meta.session_context in its shared invoke prelude, the SDK renders them into one turnContext string, and the runner prepends that to the prompt. All three halves work.

The playground never reaches the first half. It posts a turn straight to {origin}/services/agent/v0/invoke, which traefik routes to the agent service, so the API's prelude never runs. Every playground turn reached the agent with no session facts at all. The agent then answered from the only place a name appears, its own earlier reply, which is why the name looked frozen at the value from before the rename.

The same request body carries a second problem. meta is client input on that path, so a browser could hand the agent forged facts. The deployed service accepted them. See the live evidence below.

Changes

The agent service now resolves the three facts itself, once per turn, in agenta/sdk/agents/platform/session_context.py. It reads them from the backend with the caller's own credential, the same connection the service already uses for tool and vault resolution:

Fact	Read
agent_name	GET /api/workflows/{workflow_id}
session_name	GET /api/sessions/streams/?session_id=
first_turn	POST /api/sessions/turns/query, limit 1

The workflow read and the session reads run concurrently. Resolution hangs off AgentComposition.resolve_session_context, so it is injectable like every other resolver on that seam.

The service no longer reads request.meta.session_context at all. A service cannot tell the API's stamp apart from a forged copy in its own request body, so it trusts neither. The API keeps stamping the key: that is the wire contract for a workflow service built on an SDK older than this change, and removing it would break those during a rolling deploy.

Two details worth a look in review.

The artifact reference has three names. A playground turn labels it application, a native workflow invocation labels it workflow, and an evaluator labels it evaluator. All three are workflow-backed and name the same row. The resolver takes the id from the run context, which already normalizes them, and falls back to scanning those three keys when a bare-SDK run has no tracing context. Reading only workflow, the way the API's own _resolve_agent_name does, reports no agent name for every playground turn.

The session name and the first-turn flag are one fact pair. If either read fails, both report UNKNOWN. An unread name beside first_turn=False renders "This session has no name yet", which tells an already-named session to rename itself. A wrong fact is worse than a missing one, and the renderer already drops any fact it does not have.

Every read is best-effort. These facts shape prompt text, so a backend that cannot answer degrades to the pre-#6638 prompt rather than failing the turn.

Why this option

The brief asked for three options in order.

(a) Resolve in the services container. Chosen. Every turn that goes through the SDK's agent handler passes here, whether the API proxied it or a browser posted it directly. Resolving there fixes both paths with one rule, and it is the only place that can honestly refuse to trust the request body. It also means the API's stamp is no longer consumed by the built-in service. That is deliberate, and the reason to keep the stamp is version skew, stated above.

Cost is three concurrent requests per agent turn, so the turn waits for the slowest rather than the sum. I have not measured the added latency and make no claim about it. The backend address comes from SDK configuration, so whether it is an in-cluster hop is a deployment property. The 500 ms budget below is the bound that actually matters: past it the facts are dropped rather than waited on.

(b) Render the turn context in the runner from the heartbeat response. The runner does receive the stream row on every beat, so it could recover session_name. It cannot reach the other two. The workflow display name is not on that row, and the first-turn flag is not either. It would also put a second renderer next to the SDK's, so the prompt text could drift between the two paths. Rejected.

(c) Route the playground through the API invoke path. This is the structurally cleanest answer and it deletes the whole class of bug. It also moves every playground turn onto a different transport, which changes streaming, cancellation, and session admission all at once. That is not a change to make inside a release fix. Worth doing on its own, separately.

Tests

Unit, all green:

Suite	Result
sdks/python agents unit	1322 passed, 4 skipped, 1 pre-existing failure
services unit	163 passed

The pre-existing failure is test_streaming.py::test_cli_stream_terminal_only_on_empty_request. It fails the same way on the branch point, 3e120dc46a.

New coverage:

• test_session_context_http.py, 13 tests over a mocked backend: the three reads and their exact request shapes, the credential on every read, an unconfigured backend, a run with no session id, a draft run with no artifact, a half-read session reporting neither fact, an unreadable workflow costing only the agent name, blank names reading as unnamed, and unexpected body shapes reading as unknown.
• test_agent_composition_seam.py: the resolved facts reach the backend as turn text, the resolver receives the session id and the artifact id for all three reference families, the run context wins over the raw reference, a client-supplied session_context is ignored, and a client-supplied one cannot survive a failed resolve.

ruff format and ruff check clean at the CI-pinned 0.15.12. No web changes.

Live evidence

Local EE dev stack, harness Pi core, model openrouter/openai/gpt-4.1-mini through an OpenRouter connection, sandbox local. Both runs drive POST /agent/v0/invoke with the exact body the playground sends. Before is the deployed service on the unmodified SDK. After is the same stack, same API, same runner, with the agent service run from this branch.

The stored row read QA-v0.115.3 Vermilion Quay in both runs.

Check	Before	After
Session name, after the first rename	assistant	QA-v0.115.3 Sapphire Ledger
Session name, "use only THIS turn's facts", after a second rename	assistant	QA-v0.115.3 Vermilion Quay
The agent's own name	assistant	QA-v0.115.3 ctx 9ee0c5eb
A forged meta.session_context naming the session FORGED Ashen Vault	FORGED Ashen Vault	QA-v0.115.3 Vermilion Quay

The agent-name row matters on its own. That name never appears anywhere in the conversation, so a correct answer proves the workflow read arrived rather than the model reading its own history.

The forged row is the security half. The deployed service repeats a name the client invented. With this change the service reports the stored name and ignores the forgery.

What to QA

• Open an agent in the playground and have one exchange. Rename the session with Alt+R to something that has never appeared in the chat. Ask "What is this session named? Answer with only the name." It answers the new name.
• Rename again, then ask the same question. It answers the newest name, not the previous one.
• Ask the agent "What is your name?". It answers the agent's display name from the header, not a guess.
• Rename the agent itself, then ask again in a new turn. It answers the new agent name.
• Regression: start a fresh session and send the first message. The agent still names the session once, as it did before, rather than treating an unnamed session as already named.

Not covered

• The API path is unchanged and was not re-tested end to end. The service resolves the facts there too, now, so a HITL resume and a trigger fire get them from the new read rather than from meta. Unit tests cover the resolution; there is no live run of a trigger fire in this PR.
• The first_turn flag has no live assertion. It is prompt text with no user-visible surface to check, and unit tests pin it.
• A caller whose role grants RUN_SESSIONS but not VIEW_SESSIONS or VIEW_WORKFLOWS now loses the facts, because the reads carry the caller's own credential rather than server privilege. That degrades to the pre-feat(agents): give every turn current agent and session context #6638 prompt.
• The browser tab title staleness the issue mentions is a separate frontend concern and is untouched here.
• A live approval reply continues the pending harness prompt rather than submitting a new one, so it does not pick up freshly resolved facts. A cold resume does, because it submits a new prompt.
• An SDK user who drives the harness or session interfaces directly bypasses this handler and must supply SessionConfig.session_context themselves. This PR does not refresh every possible SDK session.
• "One source of truth" describes what the handler consumes, not the whole pipeline. On the API path the facts are computed twice, once by the stamp and once here, and the two can disagree through timing, permissions, or artifact-family handling. Only the handler's copy is rendered.
• Each invocation builds its own HTTP client, with no pooling across turns. That matches every other adapter in agents/platform/, so changing it belongs to the package rather than to this module alone. Filed as a follow-up, not done here. No fact is cached, and none should be: a rename has to show on the very next turn.
• The release gate shares the blind spot this bug lived in. Its invoke helper posts to the same service endpoint, so no gate cell exercised the API stamp, and no cell renames a session between two turns. This PR adds no gate cell. The driver that produced the live table above is written for that shape and is the obvious seed for one.

Codex review follow-up (12b6c6b)

Three findings from the xhigh review, all landed with tests.

P1, one total deadline and a complete exception boundary. The reads inherited the tool resolver's 30 s timeout, which is per-operation. A backend that trickles bytes resets it on every chunk while elapsed time grows without bound, and Codex reproduced 0.48 s against a 0.1 s setting. Client construction and teardown also sat outside the catches, so a constructor exception escaped.

The whole optional operation now runs under one budget, 500 ms by default and settable with AGENTA_AGENT_SESSION_CONTEXT_TIMEOUT. Expiry cancels the outstanding reads. The handler applies the same budget to an injected resolver, which has none of its own. A cancellation from outside propagates rather than being swallowed, because the caller is going away and this optional work should go with it.

P2, malformed session data is UNKNOWN, not unnamed. {"stream": []} beside a valid turns answer used to yield (None, False), which renders "This session has no name yet. Name it with rename_session" at an already-named session. That is the exact bug this PR exists to fix. A numeric name did the same. Both envelopes and both field types are now validated before the pair is asserted, and anything untrusted sends the whole pair to UNKNOWN.

The legitimate cases still work. An absent or null stream means no session row yet, which the response model produces via response_model_exclude_none, and a null or blank name is a real unnamed session. Both still get their naming instruction.

P2, competing artifact families decline rather than pick. A request carrying application=A and workflow=B names two different artifacts. The family validator that would reject it runs only during reference hydration, which an inline-config run bypasses, so the selector silently preferred workflow. It now reports no agent name when the families disagree, and still resolves when they agree, which is the normal overlapping case.

API comments corrected. _stamp_session_context claimed "a UI turn, a HITL resume, and a trigger fire are all covered". A UI turn was never covered. Both that docstring and core/sessions/context.py now say what the stamp actually serves and why it stays.

New tests covered the total budget, cancellation of the outstanding reads, a constructor exception, the env override, the malformed stream and turns bodies, the legitimately unnamed shapes, and the malformed workflow bodies, plus competing and agreeing artifact families and a slow or raising injected resolver.

Live re-run after these changes, same stack and same five cells, all green. The service logged no resolver warning, so every read landed inside the budget.

Two things the review asked to file rather than fix here, both now open:

• #6668, an inline-config invoke can name two competing workflow artifact families, because the family validator runs only during reference hydration. Pre-existing. This PR declines to report an agent name when the families disagree, so a wrong name never reaches the prompt, but every other consumer of that id still picks silently.
• #6669, the release gate never renames a session between turns and asserts nothing about the session facts. That is the coverage gap that let this class of bug ship.

Codex round 2 (a5780c8)

The competing-families check never ran on the path that matters. Codex posted an ambiguous inline request through the real service app and got Your name is "Name of B". The check was there, but the run-context early return sat above it, and the run context is built from those same references and prefers the workflow family. So the early return handed back the exact silent preference the check existed to refuse. The check now runs first.

My round-1 test passed because a bare handler has no ambient run context, which is precisely why it could not see this.

The cover is now services/oss/tests/pytest/unit/agent/test_session_context_resolution.py, which posts to the service's real /invoke route with the real platform resolver and stubs only the backend HTTP, the harness, and auth. Driving the route matters: it is the route that builds the tracing context from the request's own references, so the run context under test is the real one rather than something the test hands in. Four cells: the facts render end to end, a rename shows on the very next turn, a forged meta.session_context is ignored, and competing families render no agent name while the session half still lands.

Reverting the check ordering makes it fail with assert 'Agent B' not in ..., the same result the review reported through its own probe. The seam test fails alongside it with B's id. Both reproduce the finding rather than restating the fix.

One deadline owner, and a bounded unwind. run_optional is shared by the module and the handler. It runs the work as its own task, which buys two things a bare asyncio.wait_for cannot. The unwind is bounded by a grace period instead of awaited without limit. And a teardown that raises can no longer replace an in-flight CancelledError with an ordinary exception that the boundary then swallows, which was a real regression the round-1 fix introduced.

The default budget is now 2 s, up from 500 ms. Codex is right that a fresh client per turn pays connection and TLS setup, and that the stream read does Redis work before its Postgres lookup. Silent degradation on an ordinary slow day is the worse failure, and 2 s still cannot meaningfully delay runs that take seconds.

Smaller items. A non-finite override such as inf no longer disables the deadline. Reference ids are canonicalized, so two families spelling one artifact differently do not read as a disagreement. The docstring claiming the streams GET route omits null fields was wrong: that route registers no response model at all.

Test totals at this head: SDK agents 1359 passed, services unit 167 passed, same single pre-existing failure. Codex root-caused that one for us as a missing tsx, where pnpm emits the literal string undefined and the NDJSON parser rejects it. Live re-run of all five cells green with the 2 s budget and no resolver warning in the service log.

Three items from the review I did not take here. Connection pooling, because every adapter in agents/platform/ builds a client per call and the handler has no lifespan hook to own a shared one; that is a package-level change. Tighter callable typing, because the six sibling aliases in handler.py use the same loose form. And metrics, filed as #6671: the review is right that a log warning alone makes this degrade invisibly, but an outcome counter and a duration histogram carry naming and cardinality decisions that do not belong in a release fix.

Codex round 3 (047738e)

The artifact-ambiguity blocker is closed, confirmed by the review's own probe through the real service route. Three items from that round are fixed here, two of them defects the previous round's rework introduced.

A resolver that raises synchronously escaped the boundary. The call was evaluated before entering the guard, so a factory that raises before returning its awaitable propagated and broke the turn. The composition field is typed Callable[..., Awaitable[...]], which admits exactly that. run_optional now takes the callable and calls it inside the boundary.

Two deadlines nested on the default path. The handler wrapped a resolver that wrapped itself, so when the outer budget expired it cancelled the inner wrapper, and the grace period then watched that wrapper finish rather than the client holding the connections. The mechanism built to bound cleanup was guarding the wrong object. The handler now calls read_session_context, which carries no bound, and resolve_session_context stays as the bounded entrypoint for a caller that reaches past the handler. Exactly one deadline owns a turn, and a test asserts that by recording every owner that fires.

A detached cleanup could be collected mid-unwind. A done callback is not a reference, so an abandoned task could be garbage-collected and report Task was destroyed but it is pending!. Detached tasks now live in a set that each entry removes itself from on completion.

Both new tests were checked against their own regressions. Re-evaluating the resolver before the guard makes the first fail with the factory's RuntimeError; restoring the nesting makes the second fail with two owners instead of one.

Smaller items. The docstring correction about the streams route landed in the module this time, not only in a test comment. Two assertions were too loose to fail on broken code: the cancellation test now requires all three reads to cancel rather than at least one, and the slow-teardown test is bounded by budget plus grace rather than a full second.

The service tests own the SDK singleton. The route runs the instrumented handler, which reads ag.tracing, and a worker that has run nothing to initialize it returns 500 on NoneType.get_current_span in the cell that proves the artifact check works. A fixture now initializes it, asserts it, and restores whatever was there, so a process global does not follow every later test in the worker. This started as a suspected parallel-runner flake; the causal claim was later withdrawn, and the historical failures stay unexplained. The hardening is worth having either way.

What the abandonment path does and does not promise. Cancelling the task raises into the async with, so the client's own close is entered every time, and a test pins that alongside the caller not waiting for it. If that close itself hangs there is nothing further to force: httpx exposes no way past aclose. The review could not produce a stock-httpx teardown hang either, which is why bounding admission is deferred rather than done.

The service tests now take the SDK singleton through a fixture that restores whatever was there. Setting a process global and leaving it installed would follow every later test in the same worker.

Test totals at this head: SDK agents 1365 passed, services unit 167 passed across three consecutive runs, same single pre-existing failure.

The review returned SHIP WITH NITS on this round, with no production blocker. It independently probed each closed item: a synchronous CancelledError still propagates through the new boundary rather than degrading to an ordinary failure, exactly one deadline owner fires on each path, and a detached task survives a forced garbage collection with no destroyed-task diagnostic. It also measured the deferred admission bound against stock httpx, where five trickling resolutions each finished inside a 0.2 s budget with every connection closed and nothing left detached, which is why that stays hardening rather than a fix. The remaining nits were three inaccurate comments, corrected here.

Deferred, with the review's agreement on each. Bounding admission while a cleanup is still outstanding stays open: the leak needed an injected infinite teardown and could not be reproduced against stock httpx. Metrics are #6671. Connection pooling and the callable typing remain package-level concerns. The review also found that a slug-only reference sidesteps an id-to-id ambiguity check, which is pre-existing and now recorded in #6668.

[linear-code]

AGE-4290

[mmabrouk]

@coderabbitai review

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
agenta-documentation	Ready	Preview	Sep 8, 2026 6:52pm UTC

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[CLAassistant]

All committers have signed the CLA.

[github-actions]

📘 Docs preview

Status	✅ Ready
Preview	https://pr-6667-agenta-docs-preview.mahmoud-637.workers.dev/docs
Inspect	Actions run
Commit	0c1fc39fc6afade1b91ffffb26601d135feb90b6

This comment updates in place on every push.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Summary

Summary by CodeRabbit

• Bug Fixes

  • Fixed session names and first-turn behavior in the playground and agent service.
  • Session details now update immediately after a session is renamed.
  • Improved consistency for turns started directly from the playground.
  • Invalid, unavailable, or malformed session details no longer prevent turns from continuing.
  • Conflicting session references no longer produce misleading agent details.
  • Approval replies continue pending prompts without submitting duplicate context or user messages.
  • Client-provided session details are ignored in favor of verified server data.

• Documentation

  • Updated protocol documentation for server-resolved session details and approval replies.

Walkthrough

The agent service now resolves workflow and session facts from the Agenta backend for each turn. The handler ignores client-supplied session metadata, supports injected resolvers, and derives artifact identifiers from request references. Documentation and tests cover timeouts, malformed responses, ambiguity, cancellation, and failed reads.

Changes

Session context resolution

Layer / File(s)	Summary
Backend session-context resolver sdks/python/agenta/sdk/agents/platform/session_context.py, sdks/python/agenta/sdk/agents/platform/__init__.py, sdks/python/oss/tests/pytest/unit/agents/platform/test_session_context_http.py	The resolver reads workflow, session-stream, and turn data with strict validation. It applies a configurable timeout and reports malformed or incomplete fact pairs as unknown.
Handler composition integration sdks/python/agenta/sdk/agents/handler.py, sdks/python/oss/tests/pytest/unit/agents/test_agent_composition_seam.py	AgentComposition accepts a session-context resolver. The handler derives artifact identifiers, ignores request metadata, bounds resolver execution, and passes resolved facts to SessionConfig.
Protocol and service validation docs/design/agent-platform-instructions/status.md, docs/design/agent-workflows/documentation/protocol.md, api/oss/src/core/sessions/context.py, api/oss/src/core/workflows/service.py, services/oss/tests/pytest/unit/agent/*	The documentation describes direct playground requests, service-side resolution, API metadata compatibility, and proxy-only stamping. Service tests cover current names, renamed sessions, forged metadata, and competing artifact families.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to acea5

The service now derives session facts for playground turns, but the forged-metadata regression test does not distinguish a client-supplied first-turn value from the backend result. This is a bounded test-coverage gap that could allow a future regression in first-turn prompt behavior to go undetected.

Sequence Diagram(s)

sequenceDiagram
  participant Playground
  participant AgentHandler
  participant SessionResolver
  participant AgentaBackend
  Playground->>AgentHandler: POST /invoke with session id
  AgentHandler->>SessionResolver: Resolve session context
  SessionResolver->>AgentaBackend: Read workflow and session facts
  AgentaBackend-->>SessionResolver: Return validated backend data
  SessionResolver-->>AgentHandler: Return SessionContext or no context
  AgentHandler->>AgentHandler: Build SessionConfig

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 42.75% which is insufficient. The required threshold is 60.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 131 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies issue [#6661]. Each turn resolves the current session name in the agent service, so playground turns can use renamed session values instead of stale context. The unrelated browser tab…
Out of Scope Changes check	✅ Passed	The changes are within the stated scope. Resolver injection, credential use, timeout handling, malformed-data safety, artifact-family handling, forged-context rejection, documentation, and tests suppo…
Description check	✅ Passed	The description clearly explains the session-context bug, the service-side resolution, security behavior, compatibility rationale, tests, and scope. It is directly related to the changeset.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: the SDK now reads session facts in the agent service instead of trusting request data.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/release-1153-session-context-path

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Railway Preview Environment

Status	Destroyed (PR closed)

Updated at 2026-09-08T19:04:08.042Z

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 78203041-6422-4ddb-9f89-503e18b944e8

📥 Commits

Reviewing files that changed from the base of the PR and between a5780c8 and acea55f.

📒 Files selected for processing (1)

• services/oss/tests/pytest/unit/agent/test_session_context_resolution.py

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.