-
Notifications
You must be signed in to change notification settings - Fork 9.7k
feat(chat): surface daemon failure_detail to sharpen run-error guidance #5321
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
32af894
feat(chat): surface daemon failure_detail to sharpen run-error guidance
xiaoche-hub f96cc46
chore(i18n): rebrand user-facing "AMR" to "Open Design Cloud"
xiaoche-hub 6d521e7
fix(chat): persist daemon failure classification onto assistant message
xiaoche-hub f4eced7
fix(chat): keep daemon failure classification on the live web reload/…
xiaoche-hub 50703b1
merge: resolve main into feat/run-failure-detail-surface-895
xiaoche-hub 14bc861
fix(web): update AMR tier-upgrade test for 3-arg resolveRunFailureUi
xiaoche-hub 67d2b3d
ci: re-trigger validate workspace on approved head
xiaoche-hub fda7539
test: align AMR rebrand assertions with Open Design Cloud copy
xiaoche-hub File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
239 changes: 239 additions & 0 deletions
239
apps/daemon/tests/run-failure-detail-persisted-message.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,239 @@ | ||
| import type { Server } from 'node:http'; | ||
| import { randomUUID } from 'node:crypto'; | ||
| import { chmod, mkdtemp, rm, writeFile } from 'node:fs/promises'; | ||
| import os from 'node:os'; | ||
| import path from 'node:path'; | ||
| import { afterEach, describe, expect, it } from 'vitest'; | ||
|
|
||
| import { startServer } from '../src/server.js'; | ||
|
|
||
| // #895 regression: the daemon classifies a failure's fine-grained cause | ||
| // (failure_category / failure_detail) at finalize. The live web error handler | ||
| // stamps that onto the assistant message, but a failure persisted purely on the | ||
| // daemon side — or a conversation reloaded before the web save lands — reads the | ||
| // stored message instead of the live SSE stream. Historically that stored | ||
| // `status:error` event only carried `{ detail, code }`, so a reload fell back to | ||
| // the coarse errorCode UI and lost the specific fix guidance. | ||
| // | ||
| // This asserts the STORED assistant message (not just the run-status DTO) | ||
| // carries `failureCategory` / `failureDetail` after a failed hard-quota run. | ||
|
|
||
| type StartedServer = { | ||
| url: string; | ||
| server: Server; | ||
| shutdown?: () => Promise<void> | void; | ||
| }; | ||
|
|
||
| type RunStatus = { id: string; status: string }; | ||
|
|
||
| type PersistedEvent = { | ||
| kind?: string; | ||
| label?: string; | ||
| detail?: string; | ||
| code?: string; | ||
| failureCategory?: string; | ||
| failureDetail?: string; | ||
| }; | ||
|
|
||
| type StoredMessage = { | ||
| id: string; | ||
| role: string; | ||
| events?: PersistedEvent[]; | ||
| }; | ||
|
|
||
| type RunHandles = { | ||
| projectId: string; | ||
| conversationId: string; | ||
| assistantMessageId: string; | ||
| status: RunStatus; | ||
| }; | ||
|
|
||
| describe('run failure classification persisted to assistant message', () => { | ||
| const originalEnv = snapshotEnv(); | ||
| let started: StartedServer | null = null; | ||
| let binDir: string | null = null; | ||
|
|
||
| afterEach(async () => { | ||
| await Promise.resolve(started?.shutdown?.()); | ||
| if (started?.server) { | ||
| await new Promise<void>((resolve) => started?.server.close(() => resolve())); | ||
| } | ||
| started = null; | ||
| if (binDir) await removeTempDir(binDir); | ||
| binDir = null; | ||
| restoreEnv(originalEnv); | ||
| }); | ||
|
|
||
| it('stamps failureCategory/failureDetail onto the stored error event on a hard-quota failure', async () => { | ||
| binDir = await mkdtemp(path.join(os.tmpdir(), 'od-failure-detail-msg-bin-')); | ||
| const fakeClaude = await writeHardQuotaClaude(binDir, 'claude-hard-quota'); | ||
|
|
||
| delete process.env.POSTHOG_KEY; | ||
| delete process.env.POSTHOG_HOST; | ||
| delete process.env.LANGFUSE_PUBLIC_KEY; | ||
| delete process.env.LANGFUSE_SECRET_KEY; | ||
| delete process.env.LANGFUSE_BASE_URL; | ||
| delete process.env.OPEN_DESIGN_TELEMETRY_RELAY_URL; | ||
|
|
||
| started = (await startServer({ port: 0, returnServer: true })) as StartedServer; | ||
| await putConfig(started.url, { | ||
| agentId: 'claude', | ||
| agentCliEnv: { claude: { CLAUDE_BIN: fakeClaude } }, | ||
| telemetry: { metrics: true, content: false, artifactManifest: false }, | ||
| privacyDecisionAt: Date.now(), | ||
| }); | ||
|
|
||
| const { projectId, conversationId } = await createConversation(started.url); | ||
| const run = await sendRunAndWait(started.url, projectId, conversationId); | ||
| expect(run.status.status).toBe('failed'); | ||
|
|
||
| // Read the STORED message the same way a reload does (daemon HTTP API), | ||
| // not the live stream, so this proves the daemon-owned persistence path. | ||
| const stored = await fetchAssistantMessage( | ||
| started.url, | ||
| projectId, | ||
| conversationId, | ||
| run.assistantMessageId, | ||
| ); | ||
| expect(stored).not.toBeNull(); | ||
|
|
||
| const errorEvent = [...(stored?.events ?? [])] | ||
| .reverse() | ||
| .find((event) => event.kind === 'status' && event.label === 'error'); | ||
| expect(errorEvent, 'persisted assistant message should carry a status:error event').toBeTruthy(); | ||
| expect(errorEvent?.failureCategory).toBe('rate_limit'); | ||
| expect(errorEvent?.failureDetail).toBe('hard_quota'); | ||
| }); | ||
| }); | ||
|
|
||
| function snapshotEnv(): Record<string, string | undefined> { | ||
| return { | ||
| LANGFUSE_PUBLIC_KEY: process.env.LANGFUSE_PUBLIC_KEY, | ||
| LANGFUSE_SECRET_KEY: process.env.LANGFUSE_SECRET_KEY, | ||
| LANGFUSE_BASE_URL: process.env.LANGFUSE_BASE_URL, | ||
| OPEN_DESIGN_TELEMETRY_RELAY_URL: process.env.OPEN_DESIGN_TELEMETRY_RELAY_URL, | ||
| POSTHOG_KEY: process.env.POSTHOG_KEY, | ||
| POSTHOG_HOST: process.env.POSTHOG_HOST, | ||
| }; | ||
| } | ||
|
|
||
| function restoreEnv(env: Record<string, string | undefined>): void { | ||
| for (const [key, value] of Object.entries(env)) { | ||
| if (value === undefined) delete process.env[key]; | ||
| else process.env[key] = value; | ||
| } | ||
| } | ||
|
|
||
| // Fake Claude CLI: emits the init frame, then dies with a hard-quota billing | ||
| // message on stderr (matches isHardQuotaText -> detail 'hard_quota', which is | ||
| // non-retryable so the run fails on the first attempt). | ||
| async function writeHardQuotaClaude(dir: string, name: string): Promise<string> { | ||
| const bin = path.join(dir, name); | ||
| await writeFile( | ||
| bin, | ||
| `#!/usr/bin/env node | ||
| if (process.argv.includes('--version')) { console.log('claude-code 1.0.0-hard-quota'); process.exit(0); } | ||
| if (process.argv.includes('--help')) { console.log('Usage: claude -p [--include-partial-messages]'); process.exit(0); } | ||
| console.log(JSON.stringify({ type: 'system', subtype: 'init', model: 'claude-quota-test' })); | ||
| process.stderr.write('You have exceeded your current quota. Please upgrade your plan to continue.\\n'); | ||
| setTimeout(() => process.exit(1), 20); | ||
| `, | ||
| 'utf8', | ||
| ); | ||
| await chmod(bin, 0o755); | ||
| return bin; | ||
| } | ||
|
|
||
| async function putConfig(url: string, patch: Record<string, unknown>): Promise<void> { | ||
| const response = await fetch(`${url}/api/app-config`, { | ||
| method: 'PUT', | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: JSON.stringify(patch), | ||
| }); | ||
| expect(response.status).toBe(200); | ||
| } | ||
|
|
||
| async function createConversation( | ||
| url: string, | ||
| ): Promise<{ projectId: string; conversationId: string }> { | ||
| const projectId = `failure_detail_msg_${randomUUID()}`; | ||
| const projectResponse = await fetch(`${url}/api/projects`, { | ||
| method: 'POST', | ||
| headers: { 'content-type': 'application/json' }, | ||
| body: JSON.stringify({ | ||
| id: projectId, | ||
| name: 'Failure detail persisted message smoke', | ||
| metadata: { kind: 'prototype' }, | ||
| skipDiscoveryBrief: true, | ||
| }), | ||
| }); | ||
| expect(projectResponse.status).toBe(200); | ||
| const projectBody = (await projectResponse.json()) as { conversationId: string; id: string }; | ||
| return { projectId, conversationId: projectBody.conversationId }; | ||
| } | ||
|
|
||
| async function sendRunAndWait( | ||
| url: string, | ||
| projectId: string, | ||
| conversationId: string, | ||
| ): Promise<RunHandles> { | ||
| const assistantMessageId = `assistant_failure_detail_msg_${randomUUID()}`; | ||
| const runResponse = await fetch(`${url}/api/runs`, { | ||
| method: 'POST', | ||
| headers: { | ||
| 'content-type': 'application/json', | ||
| 'x-od-analytics-device-id': 'failure-detail-msg-test', | ||
| 'x-od-analytics-session-id': 'failure-detail-msg-session', | ||
| 'x-od-analytics-client-type': 'web', | ||
| }, | ||
| body: JSON.stringify({ | ||
| projectId, | ||
| conversationId, | ||
| assistantMessageId, | ||
| clientRequestId: `client_failure_detail_msg_${randomUUID()}`, | ||
| agentId: 'claude', | ||
| message: 'please do the task', | ||
| currentPrompt: 'please do the task', | ||
| }), | ||
| }); | ||
| expect(runResponse.status).toBe(202); | ||
| const body = (await runResponse.json()) as { runId: string }; | ||
| const status = await waitForRun(url, body.runId); | ||
| return { projectId, conversationId, assistantMessageId, status }; | ||
| } | ||
|
|
||
| async function waitForRun(url: string, runId: string): Promise<RunStatus> { | ||
| const startedAt = Date.now(); | ||
| while (Date.now() - startedAt < 10_000) { | ||
| const response = await fetch(`${url}/api/runs/${encodeURIComponent(runId)}`); | ||
| expect(response.status).toBe(200); | ||
| const run = (await response.json()) as RunStatus; | ||
| if (run.status === 'failed' || run.status === 'succeeded' || run.status === 'canceled') { | ||
| return run; | ||
| } | ||
| await delay(100); | ||
| } | ||
| throw new Error(`run ${runId} did not finish`); | ||
| } | ||
|
|
||
| async function fetchAssistantMessage( | ||
| url: string, | ||
| projectId: string, | ||
| conversationId: string, | ||
| assistantMessageId: string, | ||
| ): Promise<StoredMessage | null> { | ||
| const response = await fetch( | ||
| `${url}/api/projects/${encodeURIComponent(projectId)}/conversations/${encodeURIComponent(conversationId)}/messages`, | ||
| ); | ||
| expect(response.status).toBe(200); | ||
| const body = (await response.json()) as { messages?: StoredMessage[] }; | ||
| return body.messages?.find((message) => message.id === assistantMessageId) ?? null; | ||
| } | ||
|
|
||
| function delay(ms: number): Promise<void> { | ||
| return new Promise((resolve) => setTimeout(resolve, ms)); | ||
| } | ||
|
|
||
| async function removeTempDir(dir: string): Promise<void> { | ||
| await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.