Skip to content

fix(mq): keep terminal receivers terminal on result settlement - #398

Merged
KonghaYao merged 1 commit into
mainfrom
fix/mq-terminal-receiver-guard
Nov 12, 2026
Merged

KonghaYao merged 1 commit into
mainfrom
fix/mq-terminal-receiver-guard

Conversation

@KonghaYao

@KonghaYao KonghaYao commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

问题

MQ 结果结算的 receiver 提升语句只看 target work_state = 'idle'、结果行存在与 child active,不看 target 自身是否已终态。SQL 级可复现:terminal-root(error + idle)+ active-child + 结果行 → target 被改写成 error + working,违反「终态线程的 work_state 是派生值」的一致性约束。

SQL 改动

两条提升语句在原有 work_state = 'idle'、结果行存在与 child active 条件之外,增加 target agent_status = 'active' 守卫:

  • 本机:PROMOTE_RECEIVER_ON_TERMINAL_SQL(peri-resources/src/sessions/sqlite_store/session_data/mq.rs:135-139)
  • 远端:PROMOTE_RECEIVER_WORK_INTENT_SQL(peri-resources/src/sessions/remote/session_mq.rs:109-113)
UPDATE threads SET work_state = 'working'
    WHERE id = ?1 AND work_state = 'idle' AND agent_status = 'active'
      AND EXISTS (SELECT 1 FROM thread_mq WHERE thread_id = ?1 AND id = ?2)
      AND EXISTS (SELECT 1 FROM threads WHERE id = ?3 AND agent_status = 'active')

结果行照常接纳:终态 target 只失去「被提升为 working」这一效果,MQ 行、child 终态与同批其他写入不变。设计文档 docs/design/thread-mq.md §5.2 同步一句守卫语义。

测试

  • local:sessions::sqlite_store::session_mq_tests::terminal_settlement::finalize_does_not_promote_a_terminal_receiver
  • remote:sessions::remote::session_mq_tests::settlement_tests::remote_finalize_does_not_promote_a_terminal_receiver

两者都对 error / done / cancelled + idle 三种终态 target 断言:结果行数为 1、child 写入 done + idle、target 的 (agent_status, work_state) 完全不变。

回归有效性已实测:临时移除两条 SQL 守卫后,两条新测试分别失败为 ("error", "working")(local 与 remote 一致);恢复守卫后通过。

验证矩阵

检查 结果
test --locked -p peri-resources --lib -- sessions::sqlite_store::session_mq_tests::terminal_settlement 10 passed
test --locked -p peri-resources --lib -- sessions::remote::session_mq_tests:: 34 passed
test --locked -p peri-resources --lib -- sessions::resources::tests::mq:: 7 passed
test --locked -p peri-resources --lib(完整) 748 passed, 34 ignored
fmt --all -- --check 通过
./scripts/check-changed.sh 通过
git diff --check 通过
pre-commit hook(check / clippy / fmt / layer-imports / 依赖门) 通过

未覆盖项

  • 不做历史数据清理:已落库的「终态 + working」残留不在本次范围。
  • 不改 schema、不建表、不新增索引。
  • 不改 close 语义与 settle_retained_mq 结算路径。

Summary by CodeRabbit

  • Bug Fixes
    • Receiving a completed task’s result no longer changes a thread that has already finished, failed, or been cancelled back to a working state.
    • Results continue to be delivered, and the completed task continues to be settled as requested.

@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Local and remote settlement now promote a receiver from idle to working only when the receiver is active. Regression tests cover Error, Done, and Cancelled receivers and verify result insertion and child settlement.

Changes: Receiver settlement

Layer / File(s) Summary
Guard receiver promotion during settlement
peri-resources/src/sessions/remote/session_mq.rs, peri-resources/src/sessions/sqlite_store/session_data/mq.rs, peri-resources/src/sessions/remote/session_settlement_test.rs, peri-resources/src/sessions/sqlite_store/session_mq_terminal_test.rs, docs/design/thread-mq.md
Both receiver-promotion SQL statements now require the receiver to be active. Tests verify that finalization inserts the result and settles the child without changing a terminal receiver’s status or idle work state. The design document reflects the local SQLite condition.

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

Change: Bug fix

Possibly related PRs

  • KonghaYao/peri#339: Adds the child-terminal settlement path that this change updates.
  • KonghaYao/peri#382: Addresses terminal threads receiving working intent, including an overlapping terminal-settlement path.
Merge Risk: Minimal · up to 2e9f9

Settling a child result no longer changes a terminal receiver back to working. The result is still delivered and the child is still settled. No merge-blocking risk remains.

Security architecture risk: Minimal · up to 2e9f9

The change prevents completed, failed, or cancelled work from being marked for automatic resumption while preserving result delivery. It tightens an existing state check, and the reviewed changes introduce no material security risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct changed effect is confined to whether a designated receiver row gains working intent. The SQL still uses bound identifiers and requires the designated result row and active child; the added condition reduces eligible writes rather than expanding authority over additional threads.

Trust Boundaries and Controls

  • observed — Settlement retains writable-access and payload-ID validation before writes. Existing session-existence and foreign-message ownership checks remain in place, including remote batch guards that abort conflicting effects.

Resilience and Maintainability Implications

  • observed — The inspected queue selects messages by source and execution policy and hands them off using an execution-bound receive permit, not a working-state prerequisite. Thus result handoff remains separate from the durable automatic-resumption intent restricted by this PR.
Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the main change: the receiver-promotion SQL now keeps terminal receivers terminal during result settlement. The raw summary confirms the guard added to PROMOTE_RECEIVER_ON_TERMINAL_SQL…
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.
Full details: Title check

Explanation

The title names the main change: the receiver-promotion SQL now keeps terminal receivers terminal during result settlement. The raw summary confirms the guard added to PROMOTE_RECEIVER_ON_TERMINAL_SQL and PROMOTE_RECEIVER_WORK_INTENT_SQL, plus tests for error, done, and cancelled targets. The title follows the conventional commit style and stays under the length guidance.

Finishing Touches

Generate unit tests (beta)

  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2aefabe3-39e7-4dc2-96eb-40ad54dab77a
Commits

Reviewing files that changed from the base of the PR and between f530d26 and 2e9f9bd.

Files selected for processing: 5

  • docs/design/thread-mq.md
  • peri-resources/src/sessions/remote/session_mq.rs
  • peri-resources/src/sessions/remote/session_settlement_test.rs
  • peri-resources/src/sessions/sqlite_store/session_data/mq.rs
  • peri-resources/src/sessions/sqlite_store/session_mq_terminal_test.rs

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

@KonghaYao
KonghaYao merged commit 7bf4cee into main Nov 12, 2026
7 checks passed
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