Repository navigation
feat(opencode-agents): open one OpenCode session per call - #179
Conversation
ba09746 to
89320c9
Compare
|
Since #177 and this PR both handle the judge/session problem: is the plan to land #177 first (for the For context, a trial merge of main + #177 + this PR conflicts in |
|
@jeffdupont Keep only #177's This PR already opens a session per call, so a judge does not enter the agent's session. Leaving the raise in place would block that. a29afc3 records the block-array history on the span and does not bring the raise over. #177 and #180 stay open. |
jeffdupont
left a comment
There was a problem hiding this comment.
Thanks for this — the per-call session model is a nice simplification. A few things need addressing before merge:
1. Run can hang indefinitely when the message POST fails (server.py:116-126, _read_until_done; client created with timeout=None at server.py:66)
While the post task is pending, the loop awaits lines.__anext__() with no timeout and only checks post.done() after another SSE line arrives. A real /event stream never closes, so if POST /session/{id}/message returns e.g. 400 (bad provider/model id) without a session.idle, the handler hangs until some unrelated bus frame shows up — or forever. Reproduced with a MockTransport whose /event stays open after server.connected and the POST returns 400. Suggest racing the next line against the post task (asyncio.wait(..., return_when=FIRST_COMPLETED)), raising immediately on a ≥400 status, and a finite default timeout for the non-SSE calls. Tests miss this because every mocked /event body is finite.
2. Judges are re-enabled but templates aren't rendered (handler.py:690-731)
This PR drops the skip_judges=True guidance, but nothing in the package calls parse_template. run_judges passes variables={"message_history", "response_to_evaluate"} and expects the handler to substitute them, so the judge sees literal {{message_history}} and produces a degraded score that's still tracked. It also regresses agent instructions using {{ldContext.*}} (on main, the events callable could render them). Either land rendering here / merge #182 first, or keep the skip_judges advice until #182 lands. test_a_judge_opens_its_own_session only asserts two sessions opened — it should assert the rendered judge prompt and a parsed score.
3. Abort on early exit / session.error depends on GC (handler.py:579-590, 664-683)
The async generators are iterated with bare async for and never aclose()d, so the finally (cancel post, POST /abort, close stream/owned client) only runs whenever asyncio's asyncgen finalizer gets to it. Reproduced: breaking after the first TEXT_MESSAGE_CONTENT sent zero aborts until a sleep + gc.collect(). With an injected client this can race the caller closing it. Wrap with contextlib.aclosing(...) and add tests asserting the abort happens immediately.
4. 50 ms drain can truncate output/usage (server.py:17)
Once the POST returns, any 50 ms gap ends the turn even without this session's session.idle. On a loaded or remote server the final message.updated (with usage) can arrive later and get silently dropped. Prefer session.idle as the authoritative end with a much longer fallback, or reconcile from the POST response body.
5. Judges run as a full OpenCode agent with tools (handler.py:486-510)
A judge prompt containing the evaluated (possibly user-influenced) response runs with bash/edit tools in the server's cwd. At minimum document it; ideally allow a tool-less agent for judges (e.g. judge_agent=).
Nits
server.py:133-135/handler.py:113: asession.idlewith nosessionIDis treated as this session's end; require an exact match.handler.py:524:kwargs.pop("base_url")raises a bareKeyError;create_opencode_handlerraises a clearTypeErrorfor the same case.
Also flagging merge order with #182 and #177 — #182 should land before (or with) this, and this makes most of #177's judge guard redundant.
|
Addressed in d29fe06.
Nits: an idle event with no session id does not end the turn. OpenCode tests: 42 passed. Not merged. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d29fe06. Configure here.
Co-authored-by: alexis <alexis@launchdarkly.com>
Import AsyncIterable for the existing reducer and drop the unused test import. Co-authored-by: alexis <alexis@launchdarkly.com>
A failed message POST raises without waiting on an open event bus. Early exit and session.error abort through aclosing. This session's idle ends the turn, with a longer fallback so the final usage is kept. Agent instructions, including a judge prompt, are rendered. Co-authored-by: alexis <alexis@launchdarkly.com>
… stream A quiet bus and a failed message POST both leave the pending /event line task running. Cancel it and await it, swallowing CancelledError, before GET /event closes. Co-authored-by: alexis <alexis@launchdarkly.com>
The Bugbot fix push did not trigger the pull_request workflow.
c8d2911 to
491939d
Compare
jeffdupont
left a comment
There was a problem hiding this comment.
Re-reviewed at 491939d. Every point from my 20 Oct review is fixed. For each one, I removed the fix and checked that a test fails:
- Failed message POST no longer hangs: the next SSE line is raced against the POST (
server.py:155-166), and calls use finite timeouts. See the one ask below. - Judge prompts are rendered:
_system_textrunsparse_templateoninstructions. A messages-mode judge reaches the handler with its messages already collapsed intoinstructionsby core (judges.py:141-153), so it is covered too. Removing the render failstest_a_judge_opens_its_own_session. - Abort no longer waits for garbage collection: every generator level is wrapped in
aclosing. Turningaclosinginto a no-op failstest_breaking_aborts_without_waiting_for_gc. - No 50 ms drain: this session's
session.idleends the turn, with a 5 s quiet fallback. Setting the fallback back to 50 ms failstest_a_late_completion_survives_the_post_returning. - Judge tools: documented in the README and the docstring. That is the minimum I asked for; a tool-less judge agent can come later.
- Nits:
session.idleneeds an exactsessionIDmatch (a test fails without it), and a missingbase_urlraisesTypeError. Bugbot's cancelled-read finding is fixed and tested.
43 passed locally. CI is green, and the branch merges cleanly with main now that #177 has landed.
One ask (should fix, not blocking): test_a_failed_message_post_does_not_wait_for_the_bus passes even with the race removed. _OpenBus sends assistant_open and text_delta right away, and when the second frame arrives the loop sees the finished POST and raises. A real server sends nothing after rejecting the message, and that silence is the hang this fix prevents. With a silent bus for the 400 case (head = [] if script.message_status >= 400 else [...]), the test passes with the fix and times out at 2 s without it. Making that change means the test actually guards the fix.
Stack: #182 sits on this PR, and #180 is still open even though #182 says that route was rejected. Can #180 be closed?
A 400 used to be followed by assistant events, so the test passed even without racing the POST. A rejected message sends nothing. Co-authored-by: alexis <alexis@launchdarkly.com>
<!-- CURSOR_AGENT_PR_BODY_BEGIN -->
## Decision
Render `{{variables}}` in non-system config messages with
`parse_template`. History and the user turn stay raw. Instruction
rendering, including the judge prompt, is already on #179, which is
merged to main.
## Why
Configured messages were posted with placeholders still in them. The
other handlers render those messages. History and the latest user turn
are already concrete text.
## What this PR does not do
- It does not treat the model resolver as a writer of
`config["instructions"]`. `ModelResolver` only returns a provider id and
a model id.
- It does not add a messages-mode route.
- It does not change the session lifecycle from #179.
- It does not choose the AI Config key.
## Next
Observability #2547 passes `base_url` to `create_opencode_handler`. This
PR's base is main.
<!-- CURSOR_AGENT_PR_BODY_END -->
<div><a
href="https://cursor.com/agents/bc-1c257247-7e7f-505a-a3a8-201076b7d425?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-web-light.png"><img
alt="Open in Web" width="114" height="28"
src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a
href="https://cursor.com/background-agent?bcId=bc-1c257247-7e7f-505a-a3a8-201076b7d425&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source
media="(prefers-color-scheme: light)"
srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img
alt="Open in Cursor" width="131" height="28"
src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div>
Co-authored-by: Cursor Agent <cursoragent@cursor.com>
🤖 I have created a release *beep* *boop* --- <details><summary>launchdarkly-ai-server: 0.4.0</summary> ## [0.4.0](launchdarkly-ai-server-0.3.0...launchdarkly-ai-server-0.4.0) (2026-10-23) ### ⚠ BREAKING CHANGES * **evaluations:** keep the management API client internal ([#202](#202)) * **evaluations:** rename base uri to match spec ([#199](#199)) * **evaluations:** add test proving harness edge case matches spec ([#200](#200)) * **evaluations:** route handlers by provider and mode * **evaluations:** mark evaluations from code as experimental ([#157](#157)) ### Experimental * **evaluations:** add test proving harness edge case matches spec ([#200](#200)) ([fa7162e](fa7162e)) * **evaluations:** keep the management API client internal ([#202](#202)) ([17270ed](17270ed)) * **evaluations:** mark evaluations from code as experimental ([#157](#157)) ([1389765](1389765)) * **evaluations:** rename base uri to match spec ([#199](#199)) ([7d93f9c](7d93f9c)) * **evaluations:** route handlers by provider and mode ([f62591f](f62591f)) * **evaluations:** Set JSON as standard for output to pass to judge ([#203](#203)) ([ffc728b](ffc728b)) * **skills:** Correct the HTTP 403 advice now that server-side FDv2 is available to every account ([#178](#178)) ([b17e322](b17e322)) ### Features * **agui:** add a shared reader for handler streams ([#164](#164)) ([fc08dd5](fc08dd5)) * **AIC-3534:** add a TypeSafe Jev judge handler ([#149](#149)) ([1b41ea4](1b41ea4)) * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) * **langchain:** emit AG-UI text and tool events ([#163](#163)) ([ab5e9e1](ab5e9e1)) * **openai:** emit AG-UI text and tool events ([#161](#161)) ([2b1b2d4](2b1b2d4)) ### Bug Fixes * **AIC-3570:** re-resolve graph flags on every invoke and stream ([#187](#187)) ([8fdc4d4](8fdc4d4)) * **AIC-3571:** visit every child before its parent in reverse_traverse ([#189](#189)) ([87c73d9](87c73d9)) * **AIC-3572:** treat a variation without _ldMeta.enabled as disabled ([#184](#184)) ([7a0d260](7a0d260)) * **AIC-3574:** build the LaunchDarkly client off the event loop ([#185](#185)) ([bf03b6c](bf03b6c)) * **AIC-3576:** record deferred judge scores under the evaluated config ([#190](#190)) ([354d3f1](354d3f1)) * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) * **AIC-3596:** name the [otel] extra in the missing-OTel warning ([#186](#186)) ([90ce7bf](90ce7bf)) * **client:** match a handler's provides_for after normalizing its mode ([f62591f](f62591f)) * give native graph spans their LaunchDarkly identity ([#121](#121)) ([e863b6d](e863b6d)) </details> <details><summary>launchdarkly-ai-python: 0.3.0</summary> ## [0.3.0](launchdarkly-ai-python-0.2.0...launchdarkly-ai-python-0.3.0) (2026-10-23) ### ⚠ BREAKING CHANGES * **evaluations:** mark evaluations from code as experimental ([#157](#157)) ### Experimental * **evaluations:** mark evaluations from code as experimental ([#157](#157)) ([1389765](1389765)) </details> <details><summary>launchdarkly-ai-claude-agents: 0.2.6</summary> ## [0.2.6](launchdarkly-ai-claude-agents-0.2.5...launchdarkly-ai-claude-agents-0.2.6) (2026-10-23) ### Features * **claude:** emit AG-UI text and tool events ([#165](#165)) ([3f3a0ce](3f3a0ce)) * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) ### Bug Fixes * **AIC-3571:** visit every child before its parent in reverse_traverse ([#189](#189)) ([87c73d9](87c73d9)) * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) * **claude-agents:** fail the run when the CLI reports an API error as success ([#174](#174)) ([3d1a768](3d1a768)) * give native graph spans their LaunchDarkly identity ([#121](#121)) ([e863b6d](e863b6d)) </details> <details><summary>launchdarkly-ai-claude-messages: 0.2.5</summary> ## [0.2.5](launchdarkly-ai-claude-messages-0.2.4...launchdarkly-ai-claude-messages-0.2.5) (2026-10-23) ### Features * **claude:** emit AG-UI text and tool events ([#165](#165)) ([3f3a0ce](3f3a0ce)) * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) ### Bug Fixes * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) </details> <details><summary>launchdarkly-ai-openai-agents: 0.2.6</summary> ## [0.2.6](launchdarkly-ai-openai-agents-0.2.5...launchdarkly-ai-openai-agents-0.2.6) (2026-10-23) ### Features * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) * **openai:** emit AG-UI text and tool events ([#161](#161)) ([2b1b2d4](2b1b2d4)) ### Bug Fixes * **AIC-3571:** visit every child before its parent in reverse_traverse ([#189](#189)) ([87c73d9](87c73d9)) * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) * give native graph spans their LaunchDarkly identity ([#121](#121)) ([e863b6d](e863b6d)) </details> <details><summary>launchdarkly-ai-vercel-agents: 0.1.1</summary> ## [0.1.1](launchdarkly-ai-vercel-agents-0.1.0...launchdarkly-ai-vercel-agents-0.1.1) (2026-10-23) ### Features * **AIC-3408:** add Vercel AI SDK messages and agents adapters ([#103](#103)) ([64d679e](64d679e)) </details> <details><summary>launchdarkly-ai-litellm-agents: 0.1.1</summary> ## [0.1.1](launchdarkly-ai-litellm-agents-0.1.0...launchdarkly-ai-litellm-agents-0.1.1) (2026-10-23) ### Features * add LiteLLM messages and agents integrations ([#102](#102)) ([f2e9886](f2e9886)) </details> <details><summary>launchdarkly-ai-openai-messages: 0.2.5</summary> ## [0.2.5](launchdarkly-ai-openai-messages-0.2.4...launchdarkly-ai-openai-messages-0.2.5) (2026-10-23) ### Features * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) * **openai:** emit AG-UI text and tool events ([#161](#161)) ([2b1b2d4](2b1b2d4)) ### Bug Fixes * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) </details> <details><summary>launchdarkly-ai-vercel-messages: 0.1.1</summary> ## [0.1.1](launchdarkly-ai-vercel-messages-0.1.0...launchdarkly-ai-vercel-messages-0.1.1) (2026-10-23) ### Features * **AIC-3408:** add Vercel AI SDK messages and agents adapters ([#103](#103)) ([64d679e](64d679e)) </details> <details><summary>launchdarkly-ai-litellm-messages: 0.1.1</summary> ## [0.1.1](launchdarkly-ai-litellm-messages-0.1.0...launchdarkly-ai-litellm-messages-0.1.1) (2026-10-23) ### Features * add LiteLLM messages and agents integrations ([#102](#102)) ([f2e9886](f2e9886)) </details> <details><summary>launchdarkly-ai-langchain-agents: 0.2.6</summary> ## [0.2.6](launchdarkly-ai-langchain-agents-0.2.5...launchdarkly-ai-langchain-agents-0.2.6) (2026-10-23) ### Features * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) * **langchain:** emit AG-UI text and tool events ([#163](#163)) ([ab5e9e1](ab5e9e1)) ### Bug Fixes * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) * give native graph spans their LaunchDarkly identity ([#121](#121)) ([e863b6d](e863b6d)) </details> <details><summary>launchdarkly-ai-langchain-messages: 0.2.5</summary> ## [0.2.5](launchdarkly-ai-langchain-messages-0.2.4...launchdarkly-ai-langchain-messages-0.2.5) (2026-10-23) ### Features * **client:** add configure for SDK defaults ([#158](#158)) ([3736767](3736767)) * **langchain:** emit AG-UI text and tool events ([#163](#163)) ([ab5e9e1](ab5e9e1)) ### Bug Fixes * **AIC-3577:** send object tool results to the model as JSON ([#188](#188)) ([dead4bd](dead4bd)) </details> <details><summary>launchdarkly-ai-opencode-agents: 0.2.0</summary> ## [0.2.0](launchdarkly-ai-opencode-agents-0.1.0...launchdarkly-ai-opencode-agents-0.2.0) (2026-10-23) ### ⚠ BREAKING CHANGES * **opencode:** start and stop opencode serve in the handler ([#198](#198)) ### Features * **opencode-agents:** add an OpenCode handler ([#159](#159)) ([fc08e8c](fc08e8c)) * **opencode-agents:** emit the AG-UI text and tool subset ([#162](#162)) ([7677621](7677621)) * **opencode-agents:** open one OpenCode session per call ([#179](#179)) ([b225113](b225113)) * **opencode-agents:** render agent instruction templates ([#182](#182)) ([83175aa](83175aa)) * **opencode:** start and stop opencode serve in the handler ([#198](#198)) ([0748dcf](0748dcf)) ### Bug Fixes * **opencode-agents:** own the HTTP client ([#194](#194)) ([ed2d419](ed2d419)) * **opencode-agents:** skip judges and record structured history ([#177](#177)) ([6fadf1d](6fadf1d)) </details> <details><summary>launchdarkly-ai-typesafe: 0.1.1</summary> ## [0.1.1](launchdarkly-ai-typesafe-0.1.0...launchdarkly-ai-typesafe-0.1.1) (2026-10-23) ### Features * **AIC-3534:** add a TypeSafe Jev judge handler ([#149](#149)) ([1b41ea4](1b41ea4)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Decision
One OpenCode session per call. The handler takes
base_urlfor a runningopencode serve, opensGET /event, thenPOST /sessionandPOST /session/{id}/message, and reduces only that session with the existing_Reducer. A judge gets its own session. Agent instructions, including the judge prompt, are rendered withparse_template.Why
The reducer drops every event whose session id is not the one it was bound to.
create_opencode_handlerused to take onesession_idfor the whole process, so a judge prompt entered the agent's session, or the agent's answer was scored as the verdict. A Vega-local handler was rejected: the session boundary belongs in this package.A failed message POST used to wait forever on an open
/eventstream. Early exit waited on garbage collection before aborting. A 50ms drain could drop the finalmessage.updated.events=andsession_id=are removed. The package is not on PyPI and is still pre-v1.What this PR deliberately does not do
create_opencode_handlerstays("*", "agent").Next
#182 renders
{{variables}}in non-system config messages. History and the user turn stay raw. #180 stays open and is not part of this stack.