Skip to content

fix(provider): 回放签名按块数对齐,长度不符整体丢弃 - #234

Merged
ProjectNyaser merged 1 commit into
masterfrom
fix/replay-meta-align-guard
Oct 4, 2026
Merged

ProjectNyaser merged 1 commit into
masterfrom
fix/replay-meta-align-guard

Conversation

@ProjectNyaser

@ProjectNyaser ProjectNyaser commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

背景

.tmp/zhipu-bugreport-reply.md 第六节备案的回放块下标对齐隐患:DSHana 回放块按下标对齐,担心剪枝/压缩让 content 与 replay.blocks 长度不等时签名错位。

核实结论

  • 宿主 assembler 自身剪枝不产生错位:dsh-llm 的 assembler.assembled() 在 replay.blocks.length !== all.length 时整体丢弃 replayState,等长时 blocks 与 replay 用同一个 kept 同步 filter。一条消息内要么等长、要么 replay 为 undefined。
  • 唯一可达的不等来自中断提交:agent.ts 用 live.interruptedBlocks() 落盘内容(会裁掉 tool-call 与空白块),却仍带完整的 live.replayState(按 all.length 校验)⇒ 落盘消息 content 短于 replay.blocks。
  • DSHana 读侧此前静默错位:assistantToHanaContent 的 metaBlocks[i] 只判对象存在,不校验长度。
  • compaction / deriveMessages / projection / 迁移 / serialize 均已逐条排查,不可达(证据见会话报告)。写侧 stream.ts 的 index 分配与 content 顺序一致,streamed.has(index) 不会误判,不改。

改动

  • packages/provider/lib/messages.ts:metaBlocks 仅在 replay.blocks.length === blocks.length 时采用,否则整体丢弃(与宿主 assembler 同口径)。只对齐长度,不逐块校验类型,其余行为不变。
  • tests/cordis/provider-messages.test.mjs:+3 例(长度不等整体丢弃 / 短 meta 不错位 / 非 hana 信封不采用)。

验证

单文件 node --test(沙箱下目录模式 spawn 受阻,按文件在进程内跑):

  • provider-messages:10/10 pass
  • provider-adapter:16/16 pass
  • provider-stream:5 pass / 1 skipped
  • git diff --check:干净

判别力验证:把守卫临时改回无长度校验的写法,新增用例 2 例失败;恢复后全绿——用例确实锁住了该缺陷。

遗留(宿主侧,本 PR 不改)

真实缺口在 DSH 中断提交路径(agent.ts 内容与 replayState 口径不一致),属 vendored 上游代码,需另行跟进/上报。本 PR 是 DSHana 侧的纵深防御:不再因长度不等而错挂签名。

Summary by CodeRabbit

  • Bug Fixes
    • Prevented replay signatures from being applied to the wrong assistant message content when replay data is incomplete or its block count does not match. In these cases, signatures are omitted while the converted content and its ordering remain unchanged.
    • Replay signatures are also omitted when the replay data is not in the expected format, helping keep displayed message content accurate.

assistantToHanaContent 此前按下标取 replay.blocks 里的签名,不做长度校验。
落盘消息一旦出现 content 与 replay.blocks 不等(例如宿主中断提交路径以
interruptedBlocks() 裁掉 tool-call 却保留完整 replayState),签名会挂到别的块上。
现按宿主 assembler 同一口径:块数一致才采用签名,否则整体丢弃。
新增 provider-messages 三例覆盖长度不等与错位防护。
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: Nyasers/DSHana/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ae5fb4d1-7c68-4631-bcb8-f5cf68e1c52e
📥 Commits

Reviewing files that changed from the base of the PR and between 23754fa and 8c40d7d.

📒 Files selected for processing (2)
  • packages/provider/lib/messages.ts
  • tests/cordis/provider-messages.test.mjs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

assistantToHanaContent now uses replay metadata only when the replay block count matches the assistant content block count. Tests cover mismatched counts and invalid replay metadata.

Changes

Replay signature alignment

Layer / File(s) Summary
Validate replay block alignment
packages/provider/lib/messages.ts, tests/cordis/provider-messages.test.mjs
The conversion ignores replay metadata when its block count differs from the assistant content block count. Tests verify that content conversion and order remain intact, and that signatures are omitted for mismatched or invalid replay metadata.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 8c40d

No actionable issue remains from this review; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8c40d

The change prevents potentially misaligned replay signatures from being attached while preserving message content. No new privilege or destination is evident. Risk remains low rather than minimal because downstream handling of unsigned replay has not been established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct behavioral exposure is assistant history submitted through the provider with mismatched replay and content counts. The change affects copied signature fields, not request identity, tool declarations, or destination selection in the inspected call path.

Trust Boundaries and Controls

  • observed — Caller-supplied assistant content and replay metadata reach the conversion boundary. The new control rejects mismatched metadata as a whole but continues forwarding converted content. Identity resolution remains separate and fails explicitly on resolution errors; the local converter does not define downstream authorization or signature-verification policy.

Resilience and Maintainability Implications

  • inferred — Whole-message metadata rejection contains count-mismatch signature misassociation without mutating the stored content. It does not repair the upstream persistence inconsistency or protect against equal-length metadata reordering.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: replay signatures are used only when block counts match, and are discarded when they do not.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ProjectNyaser
ProjectNyaser merged commit 107323b into master Oct 4, 2026
2 checks passed
@ProjectNyaser
ProjectNyaser deleted the fix/replay-meta-align-guard branch October 4, 2026 11:20
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.

1 participant