Skip to content

Commit 2178d6e

Browse files
authored
fix(mothership): explain a Chat turn the worker ends without a reason (#8478)
* fix(mothership): explain a Chat turn the worker ends without a reason When the worker rebuilds an ended run from its log (a resume or reattach that reaches a run that already ended, for example at its deadline), it sends an error terminal with no error event. Sim then fell back to the generic "An unexpected error occurred while processing the response." The turn now says the run had already ended and can be continued by sending a message. A reason the worker reports, a replay refusal, and a Stop all still take precedence. Also: - Log the Go stream's error text under errorMessage/detail so it no longer overwrites the log line's own message (stream.ts, buffer.ts). - Rename STREAM_TIMEOUT_MS to CHAT_RUN_DEADLINE_MS and document it as the worker's default run deadline, now only the base of USAGE_SETTLE_MS. - Update the byte-budget doc: a reader behind the ring trim is re-synced from the worker log; replay_gap is only the fallback. * fix(mothership): keep the ended-run message surface-neutral and below a replay refusal The fallback is shared by Chat, workflow execute and inbox, so it no longer tells the reader to send a message. A replay refusal now wins by guard rather than by spread order, with a test that covers the combination. * refactor(mothership): drop the unreachable refusal guard and reuse the stream-abort test helper
1 parent 088955f commit 2178d6e

7 files changed

Lines changed: 84 additions & 11 deletions

File tree

‎apps/sim/lib/billing/core/usage-analytics.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
} from '@/lib/billing/core/reporting-period'
99
import type { BillingEntity } from '@/lib/billing/core/usage-log'
1010
import { zonedWallClockToUtc } from '@/lib/core/utils/timezone'
11-
import { STREAM_TIMEOUT_MS } from '@/lib/mothership/constants'
11+
import { CHAT_RUN_DEADLINE_MS } from '@/lib/mothership/constants'
1212

1313
/**
1414
* Pure half of organization usage analytics: window resolution, the ledger scope
@@ -510,15 +510,15 @@ export function usageBucketTimestamps(
510510
* How long after a stretch of time ends before its ledger rows are final.
511511
*
512512
* Rows are stamped when inserted, but a cumulative model charge tops up its row's
513-
* cost in place for as long as its stream runs — which {@link STREAM_TIMEOUT_MS}
514-
* caps — plus the retry flushes that follow it. Past the cap and this margin a day or
515-
* hour can no longer change and is treated as settled.
513+
* cost in place for as long as its run lasts — which the worker's run deadline
514+
* ({@link CHAT_RUN_DEADLINE_MS}) caps — plus the retry flushes that follow it. Past
515+
* the cap and this margin a day or hour can no longer change and is treated as settled.
516516
*
517517
* Without a run deadline a Chat turn can top up its row for longer than that, so a
518518
* settled hour's cached aggregate can under-report that turn's later spend. This is
519519
* display only: invoices, threshold billing, and the usage gate read live ledger sums.
520520
*/
521-
export const USAGE_SETTLE_MS = STREAM_TIMEOUT_MS + 2 * 60 * 60 * 1000
521+
export const USAGE_SETTLE_MS = CHAT_RUN_DEADLINE_MS + 2 * 60 * 60 * 1000
522522

523523
const HOUR_MS = 60 * 60 * 1000
524524

‎apps/sim/lib/core/redis/byte-budget.server.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import type { Logger } from '@sim/logger'
1212
* execution's event history is read from a cursor, so the write that would breach
1313
* the ceiling is refused and the buffer stops growing. The copilot replay ring trims
1414
* its oldest events by bytes below its ceiling instead, refunding what it drops, so a
15-
* long run slides rather than refuses; a reader behind the trim gets a replay gap.
15+
* long run slides rather than refuses; a reader behind the trim is re-synced from the
16+
* worker's run log, and ends with a replay gap only when that log cannot serve it.
1617
* A live-update feed is bounded differently — see `lib/realtime/event-log.ts`, whose
1718
* readers already handle a prune by refetching, so it drops oldest-first instead.
1819
*

‎apps/sim/lib/mothership/constants.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,13 @@ export const CLIENT_TOOL_RESULT_TIMEOUT_MS = 60 * 60 * 1000
4545
/** Extra slack the resume gate allows past the slowest pending tool's watchdog. */
4646
export const TOOL_WATCHDOG_RESUME_GRACE_MS = 30_000
4747

48-
/** Timeout for the client-side streaming response handler (60 min). */
49-
export const STREAM_TIMEOUT_MS = 3_600_000
48+
/**
49+
* The worker's default deadline for one Chat run (60 min).
50+
*
51+
* Sim does not enforce it: stream legs have no wall clock. It is the base of
52+
* `USAGE_SETTLE_MS`, since it bounds how long a run tops up its model charge.
53+
*/
54+
export const CHAT_RUN_DEADLINE_MS = 3_600_000
5055

5156
/**
5257
* How long a workflow tool call waits for a browser to pick it up before the

‎apps/sim/lib/mothership/request/go/stream.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -413,7 +413,7 @@ export async function runStreamLoop(
413413
context.errors.push(failureMessage)
414414
logger.error('Received invalid stream event on shared path', {
415415
reason: parsedEvent.reason,
416-
message: parsedEvent.message,
416+
detail: parsedEvent.message,
417417
errors: parsedEvent.errors,
418418
})
419419
throw new FatalSseEventError(failureMessage)
@@ -458,7 +458,7 @@ export async function runStreamLoop(
458458
agentId: streamEvent.scope?.agentId,
459459
code: errorPayload.code,
460460
provider: errorPayload.provider,
461-
message: errorPayload.message,
461+
errorMessage: errorPayload.message,
462462
error: errorPayload.error,
463463
displayMessage: errorPayload.displayMessage,
464464
data: errorPayload.data,

‎apps/sim/lib/mothership/request/lifecycle/run.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2037,6 +2037,57 @@ describe('runCopilotLifecycle', () => {
20372037
}
20382038
)
20392039

2040+
it('reports a replay refusal over a reasonless error terminal', async () => {
2041+
const abortController = new AbortController()
2042+
const refusal = ownerRefusal()
2043+
mockRunStreamLoop.mockImplementationOnce(
2044+
async (_url: string, _init: RequestInit, context: StreamingContext): Promise<void> => {
2045+
context.completionStatus = MothershipStreamV1CompletionStatus.error
2046+
abortController.abort(refusal)
2047+
}
2048+
)
2049+
2050+
const result = await runWithStreamAbort(abortController)
2051+
2052+
expect(result).toEqual(
2053+
expect.objectContaining({
2054+
success: false,
2055+
cancelled: false,
2056+
error: refusal.userMessage,
2057+
errorCode: REPLAY_BUDGET_EXHAUSTED_CODE,
2058+
})
2059+
)
2060+
})
2061+
2062+
it('explains an error terminal that arrives without a reason as an already-ended run', async () => {
2063+
mockRunStreamLoop.mockImplementationOnce(
2064+
async (_url: string, _init: RequestInit, context: StreamingContext): Promise<void> => {
2065+
context.completionStatus = MothershipStreamV1CompletionStatus.error
2066+
}
2067+
)
2068+
2069+
const result = await runWithStreamAbort(new AbortController())
2070+
2071+
expect(result.success).toBe(false)
2072+
expect(result.cancelled).toBe(false)
2073+
expect(result.error).toEqual(expect.stringContaining('already ended'))
2074+
})
2075+
2076+
it('keeps a Stop a cancellation when the error terminal carries no reason', async () => {
2077+
const abortController = new AbortController()
2078+
mockRunStreamLoop.mockImplementationOnce(
2079+
async (_url: string, _init: RequestInit, context: StreamingContext): Promise<void> => {
2080+
context.completionStatus = MothershipStreamV1CompletionStatus.error
2081+
abortController.abort()
2082+
}
2083+
)
2084+
2085+
const result = await runWithStreamAbort(abortController)
2086+
2087+
expect(result.cancelled).toBe(true)
2088+
expect(result.error).toBeUndefined()
2089+
})
2090+
20402091
it('keeps a Stop a cancellation when a replay refusal follows it', async () => {
20412092
const abortController = new AbortController()
20422093
mockRunStreamLoop.mockImplementationOnce(
@@ -3326,6 +3377,7 @@ describe('runCopilotLifecycle', () => {
33263377
)
33273378

33283379
expect(result.success).toBe(false)
3380+
expect(result.error).toBeUndefined()
33293381
expect(result.errors).toEqual(['The provider is overloaded'])
33303382
})
33313383

‎apps/sim/lib/mothership/request/lifecycle/run.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,12 @@ const logger = createLogger('CopilotLifecycle')
9090

9191
const COPILOT_MODEL_CONTENT_PROJECTION_ERROR = 'Copilot model input could not be safely projected'
9292

93+
/**
94+
* Shown when the worker ends a turn with an error terminal but gives no reason. Every surface
95+
* (Chat, workflow execute, inbox) reports it, so it carries no surface-specific next step.
96+
*/
97+
const ENDED_RUN_MESSAGE = 'This run had already ended before it could continue.'
98+
9399
class CopilotModelContentProjectionError extends Error {
94100
constructor() {
95101
super(COPILOT_MODEL_CONTENT_PROJECTION_ERROR)
@@ -573,6 +579,14 @@ export async function runCopilotLifecycle(
573579
!refusal &&
574580
!turnWasAborted &&
575581
(backendFinishedTurn || (!context.completionStatus && context.errors.length === 0))
582+
// The worker sends an error terminal with no `error` event only when it replays a run
583+
// that already ended (for example at its deadline) to a resume or reattach, because
584+
// that replay does not carry the run's stored reason. Say so rather than leave the turn
585+
// to a generic failure; a reported reason or a replay refusal always wins.
586+
const endedWithoutReason =
587+
!turnWasAborted &&
588+
context.completionStatus === MothershipStreamV1CompletionStatus.error &&
589+
context.errors.length === 0
576590

577591
const result: OrchestratorResult = {
578592
success: succeeded,
@@ -591,6 +605,7 @@ export async function runCopilotLifecycle(
591605
toolCalls: buildToolCallSummaries(context),
592606
chatId: context.chatId,
593607
requestId: context.requestId,
608+
...(endedWithoutReason ? { error: ENDED_RUN_MESSAGE } : {}),
594609
...(refusal ? { error: refusal.userMessage, errorCode: refusal.code } : {}),
595610
errors: !succeeded && context.errors.length ? context.errors : undefined,
596611
usage: context.usage,

‎apps/sim/lib/mothership/request/session/buffer.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,7 +487,7 @@ export async function readEvents(
487487
logger.warn('Skipping corrupt outbox entry', {
488488
streamId,
489489
reason: parsed.reason,
490-
message: parsed.message,
490+
detail: parsed.message,
491491
errors: parsed.errors,
492492
})
493493
continue

0 commit comments

Comments
 (0)