feat(diagnostics): bridge generate failures to diagnose() hypotheses (#130)#165
Merged
feat(diagnostics): bridge generate failures to diagnose() hypotheses (#130)#165
Conversation
…130) Main catch in codesign:v1:generate now tags the thrown error with upstream_status / upstream_provider / upstream_baseurl / upstream_wire so the renderer can reason about the failure without re-parsing err.message. New diagnoseGenerateFailure() in @open-codesign/shared maps generate-time failures to the DiagnosticHypothesis shape already used by the connection -test path: - 404 (or bare "404 page not found" message) -> missing /v1 hint + baseUrl transform - 5xx with "not implemented" / "page not found" body -> gatewayIncompatible - 400 with "instructions" body -> openaiResponsesMisconfigured - 401 / 403 / 429 -> reuse existing hypotheses via diagnose(String(status)) Renderer applyGenerateError appends the most-likely-cause sentence to the toast description and, for the missing-/v1 case, attaches an "Apply fix" action that updates the provider baseUrl via config:v1:update-provider — one-click fix for the #130 Win11 relay-gateway failure instead of a dead-end error message. Deliberately out of scope (owned by parallel PRs): retry.ts RetryDecision, core/errors.ts remapProviderError, error-codes.ts additions, toPiContext, core/agent.ts first-turn retry. Signed-off-by: hqhq1025 <1506751656@qq.com>
3771d08 to
48b63c7
Compare
Contributor
There was a problem hiding this comment.
Findings
- [Blocker] Silent fallback when fix-apply IPC is unavailable — violates the project rule that errors must surface instead of failing silently; if
window.codesign?.config?.updateProvideris missing, clicking “Apply fix” does nothing with no user-visible error, evidenceapps/desktop/src/renderer/src/store.ts:1149
Suggested fix:const api = window.codesign?.config?.updateProvider; if (api === undefined) { get().reportableErrorToast({ code: 'GENERATE_FIX_APPLY_FAILED', scope: 'generate', title: tr('notifications.generationFailed'), description: tr('errors.rendererDisconnected'), }); return; }
Summary
- Review mode: initial
- 1 blocker found in modified lines.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.
Testing
- Not run (automation)
- Suggested tests: add a Vitest case for
applyGenerateBaseUrlFixpath wherewindow.codesign?.config?.updateProvideris unavailable, asserting an error toast is emitted.
open-codesign Bot
| providerId: string, | ||
| nextBaseUrl: string, | ||
| ): Promise<void> { | ||
| const api = window.codesign?.config?.updateProvider; |
Contributor
There was a problem hiding this comment.
[Blocker] This introduces a silent fallback (if (api === undefined) return;). Project constraints require surfacing every error. If preload/API wiring is missing, clicking Apply fix currently no-ops with no feedback.
Suggested minimal change:
const api = window.codesign?.config?.updateProvider;
if (api === undefined) {
get().reportableErrorToast({
code: 'GENERATE_FIX_APPLY_FAILED',
scope: 'generate',
title: tr('notifications.generationFailed'),
description: tr('errors.rendererDisconnected'),
});
return;
}…ack (#130) The Apply-fix toast button silently dropped clicks when `window.codesign?.config?.updateProvider` was undefined (older app builds) — violating the project rule that errors must surface, not fail silently. Also tightened the error title when IPC throws so users see "Could not apply fix" instead of the generic "Generation failed". - Missing IPC -> reportable error toast with code GENERATE_FIX_APPLY_UNAVAILABLE and a hint to edit Base URL in Settings. - IPC rejection -> reportable error toast with its own title, carries message + stack for the Report dialog. - Add three vitest cases covering success, rejection, and missing IPC. Signed-off-by: hqhq1025 <1506751656@qq.com>
Contributor
There was a problem hiding this comment.
Findings
- No actionable issues found in added/modified lines with >=80% confidence.
Summary
- Review mode: follow-up after new commits
- No new issues found in the latest diff.
docs/VISION.mdanddocs/PRINCIPLES.md: Not found in repo/docs.- Residual risk:
diagnoseGenerateFailurerelies on message-text heuristics; future provider message variations may reduce classification accuracy.
Testing
- Not run (automation)
- Suggested tests: add a renderer-level Vitest for
applyGenerateErrorthat asserts (1) 404 message path appends the most-likely-cause sentence and (2) missing-/v1hypothesis exposes an Apply-fix action.
open-codesign Bot
4 tasks
hqhq1025
added a commit
that referenced
this pull request
Apr 23, 2026
Some gateways (older sub2api / claude2api / anyrouter builds) mishandle OpenAI Responses API SSE events and treat `response.output_text.delta` / `response.completed` as `[DONE]`, cutting the stream short with no HTTP status — the error surfaces as a transport-level "terminated" / "premature close" / ECONNRESET. diagnoseGenerateFailure() now recognises this pattern and returns a `relayStreamingBug` hypothesis with actionable fix copy (upgrade the relay, switch wire to openai-chat, or use api.openai.com directly). Detection signal (b): wire=openai-responses AND baseUrl host is not *.openai.com AND no HTTP status attached AND the message matches a truncated-stream shape. HTTP-status errors still route to the existing gatewayIncompatible / serverError / keyInvalid paths. Refs #167. Stacked on #165 (introduces diagnoseGenerateFailure). Signed-off-by: hqhq1025 <1506751656@qq.com>
hqhq1025
added a commit
that referenced
this pull request
Apr 23, 2026
## Summary Fix #180 (implementation of #167): give users an actionable diagnostic when a third-party relay (older sub2api / claude2api / anyrouter builds) mishandles OpenAI Responses API SSE events and cuts the stream short with no HTTP status. ## What changed - `packages/shared/src/diagnostics.ts` — new `relayStreamingBug` hypothesis in `diagnoseGenerateFailure()`. Triggers when: - `wire === 'openai-responses'` - `baseUrl` host is **not** `*.openai.com` - No HTTP status is attached (transport-level failure, not a 4xx/5xx) - Message matches a truncated-stream shape: `stream ended`, `premature close`, `terminated`, `ECONNRESET`, `aborted` - `packages/i18n/src/locales/en.json` + `zh-CN.json` — `diagnostics.cause.relayStreamingBug` + `diagnostics.fix.relayStreamingBug`. - `packages/shared/src/diagnostics.test.ts` — 6 new tests covering positive + negative cases: - openai-responses + custom baseUrl + "terminated" → triggers - openai-responses + `api.openai.com` + "terminated" → does NOT trigger (official endpoint isn't the culprit) - openai-responses + custom baseUrl + 500 status → does NOT trigger (routes to `serverError`) - anthropic wire + "terminated" → does NOT trigger (wrong wire) - "premature close" + "ECONNRESET" message shapes both matched Detection signal used: **(b)** from the task spec — pattern-matching on baseUrl + wire + message, since the existing error path (`packages/providers/src/codex/client.ts`, `retry.ts`) does not yet attach a "stream closed without response.completed" context field. ## Stacked on #165 This PR builds on the `diagnoseGenerateFailure()` function introduced in #165 (branch `worktree-agent-a75d65c7`). **Merge #165 first**, then this PR. Rebasing onto main after #165 lands will be a no-op. ## Four-principle check - Compatibility: green — pure addition, no signature changes - Upgradeability: green — hypothesis code-path is additive, falls through to existing `serverError` / `unknown` when signals don't match - No bloat: green — ~25 LOC + 2 i18n strings per locale - Elegance: green — same shape as existing hypotheses, reuses the DiagnosticHypothesis surface ## Test plan - [x] `pnpm --filter @open-codesign/shared test` — 174 passed (8 files) - [x] `pnpm typecheck` — 10 packages green - [x] `pnpm lint` — biome clean (360 files) - [ ] Manual: reproduce with a gateway that truncates `response.*` SSE events, confirm the diagnostic panel renders the new cause + fix strings Refs #167, closes #180. Signed-off-by: hqhq1025 <1506751656@qq.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Test-connection 失败已经走 `diagnose()` 给出可操作 hypothesis(如 404 → `missingV1` + 自动加 `/v1`),但真实生成失败完全没接这套系统,用户只看到原始 IPC 错误文案,毫无方向。
本 PR 把 generate 失败接到 diagnostic hypothesis 系统,覆盖 404/4xx/5xx 多种场景,并为 404 missing-`/v1` 加自动 fix 按钮。
What changed
main 进程
shared
renderer
i18n(en + zh-CN)
Design trade-offs
Test plan
Follow-up
Closes #130
Refs #124, #134, #158