Skip to content

fix(api): always propagate invocationId in callback broadcasts (#454)#455

Merged
zts212653 merged 2 commits intomainfrom
fix/454-callback-invocation-id
Apr 12, 2026
Merged

fix(api): always propagate invocationId in callback broadcasts (#454)#455
zts212653 merged 2 commits intomainfrom
fix/454-callback-invocation-id

Conversation

@mindfn
Copy link
Copy Markdown
Collaborator

@mindfn mindfn commented Apr 12, 2026

What

  • callbacks.ts: unconditionally propagate invocationId in text broadcast, rich_block system_info broadcast (post-message), and create-rich-block broadcast
  • callback-document-routes.ts: add invocationId to generate-document file block broadcast
  • callback-routes.test.js: 4 regression tests covering all callback broadcast paths

Why

Callback-origin websocket payloads were missing invocationId, forcing the frontend into heuristic content-matching to deduplicate late-arriving callback bubbles. Since every callback route already has invocationId in scope (required for auth), propagating it is zero-cost and gives the frontend an exact-match key.

Closes #454

Original Requirements

Plan / ADR

Tradeoff

放弃了 PR #410 的客户端 heuristic 方案(time+content fence),选择协议层修复。原因:铲屎官明确要求根治而非缓解,且 invocationId 已在 callback auth scope 内,零新增字段。

Test Evidence

node --test packages/api/test/callback-routes.test.js # 80 passed, 0 failed
pnpm check # green
pnpm lint # green (only pre-existing web warnings)
pnpm -r --if-present run build # success

Open Questions

  • 全量 test:public 有 pre-existing exit code 2 failure(main 上同样存在),与本 PR 无关

本地 Review: [x] gpt52 已 review 并放行(两轮,P2 已修复)
云端 Review: [ ] PR 创建后在 comment 中触发(见下方模板)

mindfn and others added 2 commits April 12, 2026 19:36
Three callback broadcast paths omitted invocationId from system_info
payloads, forcing the frontend to rely on heuristic matching for
rich_block events. This made late callbacks across invocation
boundaries produce duplicate bubbles.

- post_message rich_block SSE: add invocationId
- create-rich-block broadcast: add invocationId
- post_message text: make invocationId unconditional (always present via callback auth)
- callback-document-routes file block: add invocationId

[宪宪/Opus-46🐾]
Covers the P2 from gpt52 review — the /api/callbacks/generate-document
path now has a dedicated test asserting its broadcast includes invocationId.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mindfn mindfn requested a review from zts212653 as a code owner April 12, 2026 12:02
@mindfn
Copy link
Copy Markdown
Collaborator Author

mindfn commented Apr 12, 2026

@codex review

Please review latest commit 1d61a23 for P1/P2 only.

规则:任何 P1/P2 必须给"可执行复现":

  • 优先:新增/更新一个 failing test(最小复现)
  • 否则:给确定性复现步骤(命令 + 输入 + 预期/实际)
    没有证据的一律降级为 P3 建议,不算缺陷。

审查标准(详见 AGENTS.md "Review guidelines" section):

  • P0 数据丢失/安全漏洞 | P1 逻辑错误/测试缺失/架构违规
  • P2 性能/重复/命名 | P3 风格偏好
  • 禁止 any、文件 200 行警告/350 硬上限、新功能必须有测试

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner

Hi @mindfn — summarizing the maintainer view from both reviews:

  • This is the right fix layer. Compared with fix(web): late callback dedup across invocation boundaries #410, this removes heuristic surface instead of adding more client-side coordination state.
  • The scope is exactly what we wanted: small diff, protocol-level invariant, and no new frontend reconciliation machinery.
  • The coverage looks complete from our side: post-message text broadcast, post-message rich-block broadcast, create-rich-block, and generate-document now all propagate invocationId.
  • The regression tests match the contract we asked for, and CI is green.

We don't have additional P1/P2 concerns from the maintainer side. From our side, this is the version worth merging.

Thanks for pivoting quickly from the heuristic mitigation to the protocol fix — that's exactly the kind of course correction we hope to see in community contributions.

— 砚砚 / GPT-5.4

@zts212653 zts212653 merged commit ac1f5a7 into main Apr 12, 2026
5 checks passed
@zts212653 zts212653 deleted the fix/454-callback-invocation-id branch April 12, 2026 12:21
mindfn added a commit that referenced this pull request Apr 12, 2026
…#455)

* fix(api): always propagate invocationId in callback broadcasts (#454)

Three callback broadcast paths omitted invocationId from system_info
payloads, forcing the frontend to rely on heuristic matching for
rich_block events. This made late callbacks across invocation
boundaries produce duplicate bubbles.

- post_message rich_block SSE: add invocationId
- create-rich-block broadcast: add invocationId
- post_message text: make invocationId unconditional (always present via callback auth)
- callback-document-routes file block: add invocationId

[宪宪/Opus-46🐾]

* test(#454): add generate-document invocationId broadcast regression test

Covers the P2 from gpt52 review — the /api/callbacks/generate-document
path now has a dedicated test asserting its broadcast includes invocationId.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
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.

fix(api,ws): always propagate callback invocationId to avoid late-callback duplicate bubbles

2 participants