From 4f9a07481dea485376c46bc645dc5d76e3131a98 Mon Sep 17 00:00:00 2001 From: tmoonlight Date: Sat, 10 Oct 2026 01:44:24 +0800 Subject: [PATCH] Preserve storage diagnostics while recording session errors --- .../default-acemodel-responses-protocol/verification.md | 4 ++++ openspec/changes/persist-session-error-notices/design.md | 1 + .../specs/session-storage/spec.md | 5 +++++ openspec/changes/persist-session-error-notices/tasks.md | 1 + .../changes/persist-session-error-notices/verification.md | 6 ++++++ src/domain/session/session_manager.cpp | 8 ++++++++ tests/agent/characterization_context_test.cpp | 4 +++- tests/hooks/hook_agent_loop_test.cpp | 5 ++++- tests/web/web_server_smoke_test.cpp | 2 ++ 9 files changed, 34 insertions(+), 2 deletions(-) diff --git a/openspec/changes/default-acemodel-responses-protocol/verification.md b/openspec/changes/default-acemodel-responses-protocol/verification.md index bb622eb2..30555e07 100644 --- a/openspec/changes/default-acemodel-responses-protocol/verification.md +++ b/openspec/changes/default-acemodel-responses-protocol/verification.md @@ -20,3 +20,7 @@ HTTP 回归使用本机 fixture，确认实际端点和请求体；浏览器检查使用 fixture API。未发送真实 ACEModel 付费请求。已验证源代码与 Web 构建，未更新当前运行的已安装 Desktop 软件。当前仓库规则忽略 `openspec/` 中新建文件，变更文档保存在本地既定目录中。 构建出现既有共享 Intermediate 目录警告；使用串行构建避免并发链接冲突。未提交或发布本次改动。 + +## 发布集成补验（2026-10-10） + +既有运行中推理设置 HTTP fixture 使用 ACEModel 身份但仅实现 Chat Completions；显式设置其协议为 `chat_completions`，避免继承新的目录默认值。相关 41 项定向回归及 Windows 全量回归通过（6229 通过、9 跳过、0 失败、1 项禁用 benchmark）。 diff --git a/openspec/changes/persist-session-error-notices/design.md b/openspec/changes/persist-session-error-notices/design.md index 460ee9ea..96e4fb13 100644 --- a/openspec/changes/persist-session-error-notices/design.md +++ b/openspec/changes/persist-session-error-notices/design.md @@ -14,6 +14,7 @@ See proposal.md for the incident. Agent error messages use `TranscriptWriter::di 2. Use UUID identity for error records when available; retain existing content-derived IDs for legacy records and all other roles. A timestamp alone is insufficient for two equal failures within the same clock interval. 3. Distinguish durable errors by `role=error`, `metadata.transcript_only=true`, and a nonempty ID. Reconcile these by ID in both replay paths; bypass content-only deduplication for different error IDs. Unmarked legacy errors retain occurrence/sequence semantics. 4. Reuse the existing provider-history filter, and test both the actual next provider request and resumed history. No UI style changes or additional context protocol is required. +5. When a storage failure causes an error notice, retain that original storage diagnostic while attempting to save the notice. Failed compaction preserves the previous history prefix and context window; its new transcript-only error is visible without pretending the failed checkpoint was stored. ## Risks / Trade-offs diff --git a/openspec/changes/persist-session-error-notices/specs/session-storage/spec.md b/openspec/changes/persist-session-error-notices/specs/session-storage/spec.md index 21ebc4a0..6ba2616d 100644 --- a/openspec/changes/persist-session-error-notices/specs/session-storage/spec.md +++ b/openspec/changes/persist-session-error-notices/specs/session-storage/spec.md @@ -25,3 +25,8 @@ The system SHALL persist each emitted conversation error notice with its origina #### Scenario: Compatibility with legacy transient errors - **WHEN** an older daemon emits identical error events without a persisted identity marker - **THEN** the client SHALL retain distinct event occurrences and deduplicate replays of the same sequence + +#### Scenario: Error notice cannot be written after a storage failure +- **WHEN** a storage operation fails and recording its error notice also fails +- **THEN** the system SHALL retain the original storage diagnostic and visible error notice +- **AND** it SHALL NOT claim the failed operation or error notice was durably saved diff --git a/openspec/changes/persist-session-error-notices/tasks.md b/openspec/changes/persist-session-error-notices/tasks.md index 7447e0ed..c2a79fc9 100644 --- a/openspec/changes/persist-session-error-notices/tasks.md +++ b/openspec/changes/persist-session-error-notices/tasks.md @@ -7,3 +7,4 @@ - [x] 2.1 Document the additive daemon message contract and verify strict OpenSpec validation plus diff checks. - [x] 2.2 Run relevant Windows native regression tests, full Web tests, Web build, and a browser check of restored red error cards; record results and runtime limitations. +- [x] 2.3 Preserve the original storage failure when persisting its error notice also fails, update blocked-input/failed-compaction expectations for transcript-only errors, and pass focused plus full Windows release regressions. diff --git a/openspec/changes/persist-session-error-notices/verification.md b/openspec/changes/persist-session-error-notices/verification.md index 96eab9b5..5a0ddd3d 100644 --- a/openspec/changes/persist-session-error-notices/verification.md +++ b/openspec/changes/persist-session-error-notices/verification.md @@ -9,3 +9,9 @@ - Windows native regression: 141/141 passed. Covered error disk reload/resume, unique event/API identity, original diagnostic metadata, next-turn provider filtering, actual compaction requests, legacy message identity, turn timing, recovery callbacks, session resume, and conversation history. Results: `build/validation/session-errors/native-tests.json`. An initial run exposed a missing non-streaming method in the new test stub and callback/event ordering drift; both were corrected before the final passing run. The browser check uses isolated simulated responses, not the affected remote server. This change has not been packaged, published, or installed into a running Desktop process. Older errors that were never persisted are not reconstructed. + +## Release integration validation (2026-10-10) + +- Full Windows regression exposed two old assumptions that failures leave no visible error record, plus a diagnostic regression when saving the notice also failed. Retained the original storage error under the session lock, and updated blocked-input and failed-compaction assertions to require transcript-only errors while preserving the original history prefix and context window. +- Focused release regressions: 41 passed, zero failures. +- Final full Windows regression: 6239 inventory entries, 6238 executed, 6229 passed, 9 skipped, zero failures; one disabled benchmark. Ran all suites with `run_fast_tests.py --profile full --shards 6` and isolated profiles. Report: `C:/Users/shao/AppData/Local/Temp/acecode-release-1.0.3/windows-full-final.json`. diff --git a/src/domain/session/session_manager.cpp b/src/domain/session/session_manager.cpp index 0a67e586..68f9f8a8 100644 --- a/src/domain/session/session_manager.cpp +++ b/src/domain/session/session_manager.cpp @@ -16,6 +16,7 @@ #include "session_usage_ledger.hpp" #include "utils/atomic_file.hpp" #include "utils/logger.hpp" +#include "utils/scope_exit.hpp" #include "utils/uuid.hpp" #include "utils/utf8_path.hpp" @@ -341,6 +342,13 @@ bool SessionManager::ensure_created() { bool SessionManager::try_on_message(const ChatMessage& msg) { std::lock_guard lk(mu_); + // Recording an error notice must not replace the storage failure that + // caused it, even when that notice also cannot be written to disk. + std::string original_error = msg.role == "error" && msg.metadata.is_object() && + msg.metadata.value("transcript_only", false) ? last_error_ : std::string{}; + ScopeExit preserve_error([this, original_error = std::move(original_error)]() mutable { + if (!original_error.empty()) last_error_.swap(original_error); + }); last_error_.clear(); if (!started_ || !ensure_created()) return false; diff --git a/tests/agent/characterization_context_test.cpp b/tests/agent/characterization_context_test.cpp index 219860cc..ff2a460c 100644 --- a/tests/agent/characterization_context_test.cpp +++ b/tests/agent/characterization_context_test.cpp @@ -244,7 +244,9 @@ TEST(AgentLoopContextGolden, FailedCheckpointAppendKeepsHistoryAndWindowUntilDur } EXPECT_EQ(h.session->last_error(), "failed to append compact checkpoint"); const auto history_after = h.loop->messages(); - ASSERT_EQ(history_after.size(), history_before.size()); + ASSERT_EQ(history_after.size(), history_before.size() + 1); + EXPECT_EQ(history_after.back().role, "error"); + EXPECT_TRUE(history_after.back().metadata.value("transcript_only", false)); for (std::size_t i = 0; i < history_before.size(); ++i) { SCOPED_TRACE(i); EXPECT_EQ(serialize_message(history_after[i]), serialize_message(history_before[i])); diff --git a/tests/hooks/hook_agent_loop_test.cpp b/tests/hooks/hook_agent_loop_test.cpp index 99e2bf26..11c53ad1 100644 --- a/tests/hooks/hook_agent_loop_test.cpp +++ b/tests/hooks/hook_agent_loop_test.cpp @@ -331,7 +331,10 @@ TEST(HookAgentLoop, UserPromptSubmitBlockPreventsPersistenceAndProviderCall) { })); } EXPECT_EQ(provider->turn_count(), 0); - EXPECT_TRUE(h.loop->messages().empty()); + // The blocked input stays out of history; its error remains visible only. + ASSERT_EQ(h.loop->messages().size(), 1u); + EXPECT_EQ(h.loop->messages()[0].role, "error"); + EXPECT_TRUE(h.loop->messages()[0].metadata.value("transcript_only", false)); ASSERT_TRUE(terminal_events->busy.is_object()); const std::string turn_id = terminal_events->done.value("turn_id", std::string{}); diff --git a/tests/web/web_server_smoke_test.cpp b/tests/web/web_server_smoke_test.cpp index 1759b03f..ebd5e427 100644 --- a/tests/web/web_server_smoke_test.cpp +++ b/tests/web/web_server_smoke_test.cpp @@ -11949,6 +11949,8 @@ void configure_reasoning_http_model(WebServerFixture& fx) { profile.api_key = "test-key"; profile.model = "reasoning-model"; profile.models_dev_provider_id = "acemodel"; + // This fixture's local server implements Chat Completions explicitly. + profile.api_protocol = "chat_completions"; profile.context_window = 128000; acecode::ModelReasoningOptions reasoning; reasoning.supported = true;