Skip to content

fix(acp): persist conversation errors for cold clients - #826

Open
dawNotPoi wants to merge 7 commits into
xintaofei:mainfrom
dawNotPoi:fix/acp-persist-last-error
Open

dawNotPoi wants to merge 7 commits into
xintaofei:mainfrom
dawNotPoi:fix/acp-persist-last-error

Conversation

@dawNotPoi

@dawNotPoi dawNotPoi commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #600.

  • Persist ACP terminal and nonterminal errors on the conversation row. Clear the stored error when a new prompt begins; linking alone does not clear it, so a failed history fork preserves the prior error.
  • Preserve the session identity seen by the lifecycle worker through manager cleanup. This covers history-load failures before a link and a new conversation whose first prompt binds a row, then exits before queued events drain.
  • Save a trusted ACP session ID to parser branch ID alias when Gemini/Cline detail loading normalizes history. Session-aware SQL writes target one row, and S1 aliases follow the preserving row through bind splits and either fork ordering. If S2 is already normalized, its alias, branch ID, and error stay on S2.
  • Use per-process dispatcher sequence and session-aware updates so a delayed S1 error cannot overwrite S2 after another client rebinds the original numeric row.
  • Carry S1's last error connection ID and scope sequence onto its preserving row in either fork ordering. This keeps the live Alert identity across split/detail reload and rejects older late S1 events; S2 clears the old identity.
  • Return the stored error, revision, and emitting connection ID through the shared detail API. After merging upstream/main ef1c376, cold-loaded session errors appear in the existing Alerts history with a localized “Last error” label, without replaying a toast or desktop notification. A live error keeps its original Alert and diagnostics through a detail refetch and in-place reconnect; dismissing that Alert does not resurrect it. Action verdicts and transcript-only errors are not restored. A new prompt retires the old detail revision, while a later error from another client can appear.
  • Store only the user-visible message and stable code in SQLite. Diagnostic details remain in the live snapshot.

Verification

  • Frontend after upstream merge: full Vitest suite 520 files / 7,747 tests passed; ESLint and production build passed. Focused tests cover cold load, action exclusion, live → S1 split → detail → reconnect/dismissal, another client’s new error, prompt retirement, and conversation switching.
  • Rust server: 4,291 passed / 1 ignored; the real-production-input legacy Gemini split test passed. CI-equivalent server all-targets Clippy with test-utils and warnings denied passed.
  • Rust desktop: all-targets Clippy with test-utils and warnings denied passed. The earlier delegation_columns integration fixture compiled with test-utils before this upstream merge.
  • Regressions cover real SQLite close/reopen after manager cleanup and S1 split, preserved error identity and scope in both fork orderings, older event rejection, alias normalization and cold recovery, alias/exact collision single-row writes, S1 without an alias while S2 has one, stale events after cross-client rebind, failed fork preservation, and DB-to-detail recovery.
  • git diff --check passed. Repository-wide cargo fmt reports pre-existing formatting differences in untouched code; this PR avoids unrelated formatting changes.

Scope and limits

An error is recoverable after backend restart and cold Web/mobile detail load once its event is committed to SQLite. A process crash before receipt or commit cannot be recovered. Old pre-migration rows normalized to parser branch IDs have no stored ACP UUID to backfill: successful session/load with the known branch ID stays on the row, while a failed load that falls back to a new Gemini ACP UUID splits to a new row because built-in agents provide no transcript continuation. An unknown old UUID cannot be inferred safely. The project assumes one backend process per SQLite data root. The shared API and UI state were tested, but no physical phone was manually tested.

@dawNotPoi
dawNotPoi marked this pull request as ready for review September 24, 2026 07:00
@dawNotPoi

Copy link
Copy Markdown
Contributor Author

@xintaofei Ready for review. This fixes #600 by persisting ACP session errors for cold Web/mobile detail loads and restarts, while keeping live Alerts deduplicated through fork and reconnect. S1 error identity and event ordering survive both fork orderings, so dismissed alerts stay dismissed and older events cannot overwrite newer errors. Independent review found no blocker; all seven CI checks passed on c94d4b7, alongside 4,291 Rust and 7,747 frontend tests locally. Scope limits and the untested physical phone path are documented in the PR body. I would appreciate your review.

@dawNotPoi
dawNotPoi force-pushed the fix/acp-persist-last-error branch from c94d4b7 to d554868 Compare September 28, 2026 07:05
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.

智能体出错时应该持久化last_error,这样冷启动的客户端比如手机端/web能获知会话真实的错误状态。

1 participant