Skip to content

fix(agents): wait for a terminal status in getResult() - #156

Draft
ling-senpeng13 wants to merge 1 commit into
mainfrom
fix/getresult-wait-for-terminal-status
Draft

fix(agents): wait for a terminal status in getResult()#156
ling-senpeng13 wants to merge 1 commit into
mainfrom
fix/getresult-wait-for-terminal-status

Conversation

@ling-senpeng13

@ling-senpeng13 ling-senpeng13 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #155.

Problem

AgentRuntime.run() could return status: "RUNNING" for an execution that had already reached a terminal state on the server. Wrong, not late — the failing calls return in 2–4 seconds, nowhere near any timeout.

run() takes its result from the SSE stream, and AgentStream.getResult() read the status endpoint once. The stream ending does not mean the workflow ended, so that single read can land mid-flight and the non-terminal status becomes the returned result. The comment above it already said "Poll the server for the real terminal status" — it just never polled.

It shows up most clearly with guardrails. Server-side the workflow reaches FAILED with reasonForIncompletion set, after the expected retry iterations and a TERMINATE task, and GET /agent/{id}/status returns FAILED:

status: FAILED
reasonForIncompletion: "Do not include secrets."
  1  gr_secrets_loop                    [DO_WHILE]           CANCELED
  …  (3 guardrail retry iterations)
 13  gr_secrets_guardrail_terminate__3  [TERMINATE]          COMPLETED

The SDK reported RUNNING. A guardrail that worked perfectly looks like it never fired, and callers branching on result.status take the wrong branch.

Change

getResult() now reads until the status is terminal, via a new _fetchTerminalStatus():

  • Reuses the existing TERMINAL_STATUSES set, so this path agrees with wait(), liveness, and schedules rather than inventing its own notion of "done".
  • Bounded by RESULT_SETTLE_TIMEOUT_MS (30s). On expiry it returns the last status seen rather than throwing — callers get the best available answer, never a hang. This bounds reconciliation only; it is not a run timeout.
  • Only a successful-but-non-terminal read is retried. An unreachable or erroring endpoint returns immediately with whatever was seen last, which preserves the previous behaviour on that path — a broken endpoint cannot burn the settle budget.

The non-streaming path never had this bug: _pollForCompletion() already loops until isComplete. That asymmetry between the two paths was the defect, and this closes it.

Tests

Three regression cases in src/agents/__tests__/stream.test.ts:

case asserts
RUNNINGRUNNINGFAILED result is FAILED with the right error; 3 status reads. Returned RUNNING before this change.
terminal on first read result is COMPLETED; exactly 1 read — no added latency for the common case
status endpoint returns 500 falls back to stream inference in 1 read — no 30s stall

src/agents/__tests__/stream.test.ts passes 25/25. The settle test takes ~1s of real time, confirming it genuinely polled rather than short-circuiting.

End-to-end verification

Built the SDK, installed it into the released conductor-ai-e2e-typescript-4.0.0-rc4 bundle, and ran against a live server with streaming enabled and no server-side change:

stock 4.0.0-rc4 this branch
Suite 8: agent output secrets blocked FAIL — status=RUNNING PASS
Suite 8: max_retries escalation FAIL — status=RUNNING PASS
Suite 8: tool input raise — SQL injection blocked FAIL (full run) PASS
full Suite 8 3 failed / 4 passed 7 passed / 7

And the complete 196-test suite, run twice with identical results, to check for
regressions:

stock 4.0.0-rc4 this branch (2 runs)
passed 174 178
failed 8 4
remaining failures guardrails 3, streaming 4, +1 flake streaming 4

Nothing that passed on stock fails on this branch.

A note on the third guardrail test

tool input raise — SQL injection blocked passes when run in isolation but fails in a full
run, so I initially took it for contention and said in #155 that it was not part of this
bug. That was wrong — this branch fixes it too, across both full runs.

It is the same race with a different hit rate: under full-suite load the workflow runs longer
relative to stream close, so the single status read lands mid-flight more often. Worth knowing
because it means the bug's visibility scales with load, and a green isolated run is not
evidence that a caller is safe.

Scope

Draft, and deliberately narrow — it fixes the reported status-reporting race only.

I checked whether it also helps the four Suite 16 streaming failures on the same setup. It does not — they fail identically before and after, so they are a separate problem and are not addressed here.

One pre-existing issue I hit, unrelated to this change: npm run test:unit crashes on Node 26 with ReferenceError: clearTimeout is not defined in src/sdk/clients/worker/Poller.ts:76. It reproduces on a clean checkout of main. I ran the stream.test.ts suite directly instead. Happy to file it separately.

AgentRuntime.run() could return status "RUNNING" for an execution that had
already finished on the server — wrong, not late: the failing calls returned
in 2-4s, nowhere near any timeout.

run() takes its result from the SSE stream, and getResult() read the status
endpoint once. The stream ending does not mean the workflow ended, so that
single read can land mid-flight and the non-terminal status is returned
verbatim. The code comment already said "poll the server for the real
terminal status" — it just never polled.

Most visible with guardrails: server-side the workflow reaches FAILED with
reasonForIncompletion set, after the expected retry iterations and a
TERMINATE task, while the SDK reports RUNNING — a guardrail that worked
perfectly looks like it never fired, and callers branching on result.status
take the wrong branch.

Read until the status is terminal, bounded by RESULT_SETTLE_TIMEOUT_MS (30s)
so a long-running execution still returns rather than hanging. Reuses the
existing TERMINAL_STATUSES set, so this path now agrees with wait(),
liveness, and schedules.

Only a *successful but non-terminal* read is retried. An unreachable or
erroring endpoint returns immediately with what was seen last — preserving
the previous behaviour on that path, so a broken endpoint cannot burn the
settle budget.

The non-streaming path never had this bug: _pollForCompletion() already
loops until isComplete. That asymmetry was the defect.

Tests: three regression cases — a RUNNING/RUNNING/FAILED sequence resolving
to FAILED (the reported bug), a terminal-first status doing exactly one read,
and an erroring endpoint falling back to stream inference in one read.

Verified end to end against the 4.0.0-rc4 e2e bundle on a live server, with
streaming enabled and no server change: Suite 8 goes from 3 failed / 4 passed
to 7 passed / 7.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.36364% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/agents/stream.ts 96.36% 2 Missing ⚠️
Flag Coverage Δ
integration-v4-sm 41.53% <45.45%> (+0.05%) ⬆️
integration-v5-sdkdev 43.99% <45.45%> (-0.02%) ⬇️
unit 73.93% <96.36%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/agents/stream.ts 86.17% <96.36%> (+1.34%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AgentRuntime.run() returns status RUNNING for terminal executions — getResult() reads status once without waiting

1 participant