Skip to content

fix(plugin): Fix OpenClaw commit failure semantics - #4441

Open
sponge225 wants to merge 1 commit into
volcengine:mainfrom
sponge225:fix-openclaw-commit-failure-semantics
Open

fix(plugin): Fix OpenClaw commit failure semantics#4441
sponge225 wants to merge 1 commit into
volcengine:mainfrom
sponge225:fix-openclaw-commit-failure-semantics

Conversation

@sponge225

Copy link
Copy Markdown
Collaborator

Description

修复 OpenClaw 插件在 session commit Phase 2 失败后的 compact 语义。

当前服务端语义是:Phase 1 归档成功后,commit 边界已经成立;如果 Phase 2 后续失败,archive 会进入 .failed.json 终态并被后续 context/commit 跳过,不应阻塞后续 commit。此前 OpenClaw 插件会把 wait=true 返回的 Phase 2 failed 统一当成 compact 失败,导致用户再次 compact 时看到 commit_no_archive,表现为 session 像是无法继续压缩。

本 PR 将 failed + archived=true 明确处理为“压缩边界已建立,但 Phase 2 抽取降级失败”。同时保留 timeout 的未完成语义:timeout 不是终态,archive 仍可能 pending 并被 context 读取路径回放原文,因此不把 timeout + archived=true 报告为 compacted。

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Fixes #4419

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • 调整 OpenClaw commitOVSession:当服务端返回 status=failedarchived=true 时,认为 Phase 1 边界已成立,返回成功;timeout 仍返回失败。
  • 调整 OpenClaw compact:当 failed + archived=true 时返回 compacted=truereason=commit_archived_phase2_failed,并保留 commit details;当 timeout 时继续返回 commit_timeout
  • 更新 session commit API 文档和 OpenClaw 插件文档,明确 failed archive 是 terminal/skippable,不阻塞后续 commit;pending/timeout 不等同于终态失败。
  • 补充并精简单测,覆盖 failed/timeout 与 archived 状态组合。

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

执行过的验证:

  • npm test -- tests/ut/context-engine-compact.test.ts
  • npm run typecheck
  • git diff --check

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

不适用。

Additional Notes

这次修复没有修改服务端 archive 状态机,只让 OpenClaw 插件的返回语义与现有服务端行为和测试约定对齐。

@sponge225
sponge225 force-pushed the fix-openclaw-commit-failure-semantics branch from c7a3da5 to 4c7ae15 Compare August 28, 2026 09:56
@sponge225 sponge225 changed the title Fix OpenClaw commit failure semantics fix(plugin): Fix OpenClaw commit failure semantics Aug 28, 2026

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

archived=true proves that Phase 1 moved messages, but it does not currently prove that a compacted representation is available. Server get_session_context() deliberately excludes overviews from failed archives and merges their raw messages.jsonl back into current_messages. This branch can therefore return ok=true, compacted=true with an empty summary and the same uncompressed messages/tokens after Phase 2 fails. The added test does not exercise that contract: its getSessionContext fake always returns no messages and estimatedTokens: 0, so it cannot distinguish actual reduction from a false success report. Please add an integration-faithful regression where a failed archive contains raw messages and no authoritative overview, and require compacted=true only when the returned context demonstrates a usable compacted representation/token reduction. Otherwise this should remain a failure/degraded result until the server exposes explicit partial-success semantics.

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction to one sentence in my previous review: current _collect_session_context_components() does not merge the failed archive raw messages back. It stops at the newest failed terminal, excludes that archive and everything older, and only returns newer pending archives plus the live tail. The blocking conclusion still holds, but the concrete failure is stronger: failed + archived=true can be reported as compacted=true with no authoritative overview while the archived conversation is omitted from the returned context. The regression should therefore use a failed archive containing the only prior conversation, no overview, and an empty/new live tail, then assert that the plugin must not report successful compaction with an empty summary or silently drop that conversation. Apologies for the earlier inaccurate characterization of the read path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: Failed memory extraction leaves archived sessions uncompactable

2 participants