Skip to content

skeleton: agent 叙事管道计划 v1(§10.1.1 促升) - #1903

Merged
Kizunad merged 14 commits into
mainfrom
docs/plan-agent-narration-pipeline-v1
Aug 5, 2026
Merged

skeleton: agent 叙事管道计划 v1(§10.1.1 促升)#1903
Kizunad merged 14 commits into
mainfrom
docs/plan-agent-narration-pipeline-v1

Conversation

@Kizunad

@Kizunad Kizunad commented Aug 4, 2026

Copy link
Copy Markdown
Owner

flash-review cluster 3(29 confirmed source,含 4 条 #1470 重复证据)促升的 skeleton。草稿经独立审计(9 findings 已修订)。

🤖 Generated with Claude Code

https://claude.ai/code/session_014Az49y7KGz1xT91uYMMbUx

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Kizunad, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c75b9056-158d-4fef-9fd4-c216845c1aa3

📥 Commits

Reviewing files that changed from the base of the PR and between 0c34137 and 19dd59f.

📒 Files selected for processing (1)
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📝 Walkthrough

Summary by CodeRabbit

  • 新功能
    • 新增开发命令操作员权限控制,非授权人员执行相关命令时将收到提示。
    • 统一季节命令与开发命令的权限管理,并支持离线模式下的明确授权。
    • 增强端到端测试环境的运行标识与操作员校验,提升复用服务时的安全性。
  • 文档
    • 新增 Tiandao 叙事流水线 v1 规划。
    • 更新重构阶段、系统契约、迁移及生产启用计划,明确依赖与验收要求。
  • 测试
    • 增加开发命令权限、离线授权及季节命令集成验证。

Walkthrough

新增 Tiandao narration pipeline v1 计划,并更新重构计划的阶段依赖与 schema 规则。开发命令现在使用共享操作员权限门控。E2E 脚本和测试覆盖操作员环境、命令范围及执行拦截。

Changes

计划与评审规则更新

Layer / File(s) Summary
Narration pipeline 计划
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
定义 narration pipeline 的范围、统一契约、处理流程、P0-P5 交付、验收标准和实施决策。
重构计划依赖与激活顺序
docs/plan-refactor-inventory-core-v1.md, docs/plans-skeleton/plan-refactor-*.md
调整 R6、R10、R5、R2 和 R9 的阶段依赖、schema 生成链、atomic activation 及 dropped-loot 迁移顺序。
评审规则范围
.github/review-policy/bong.v2.json
限制同一分支的重复问题,并区分验收范围外扩展与重大规格问题。

开发命令操作员门控

Layer / File(s) Summary
E2E 操作员环境配置
scripts/bot-e2e.sh, scripts/e2e-redis.sh
为每次 E2E 运行生成操作员 roster 和运行标签,并同步离线信任配置。
共享开发命令权限资源
server/src/cmd/dev/mod.rs
新增 DevCommandPermissions,注册开发命令作用域,并拦截非操作员的开发命令。
季节命令集成与权限验证
server/src/cmd/dev/season.rs, server/tests/dev_command_operator_gate.rs
/season 接入共享权限资源,并测试环境变量解析、命令范围和执行权限。

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant E2E as E2E 脚本
  participant Server as 开发服务器
  participant Permissions as DevCommandPermissions
  participant Client as 客户端
  participant Command as 开发命令拦截器

  E2E->>Server: 设置 BONG_OPERATORS 和离线授权
  Server->>Permissions: 初始化操作员权限
  Client->>Server: 建立连接并提交用户名
  Server->>Permissions: 同步 bong.dev 作用域
  Client->>Command: 执行开发命令
  Command->>Permissions: 检查 is_operator
  Permissions-->>Command: 返回授权结果
  Command-->>Client: 放行命令或发送权限提示
Loading

Possibly related PRs

Poem

兔子守着命令门,
操作员名单先登记。
非授权请求被拦下,
/season 依权限运行。
计划与测试同步前行。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 30.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed 标题明确指向 agent 叙事管道 v1 骨架计划,符合主要变更内容。
Description check ✅ Passed 描述说明了该 PR 促升 flash-review cluster 3 结果,并与新增计划及审计修订相关。
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 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/plan-agent-narration-pipeline-v1
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch docs/plan-agent-narration-pipeline-v1

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.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: af18b99e6c4a76dd16dd4cb01691e782e4e460c3
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] NarrationV1 validation is ordered before the value exists

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:199 · wiring

Stage 3 requires running the NarrationV1 validator "在任何 renderer ... 前" (before any renderer) at line 199, and the P3 checklist repeats that NarrationV1 validation is centralized without correcting the ordering (line 266). NarrationV1 is the renderer output envelope: its schema requires { v, narrations } (agent/packages/schema/src/narration.ts:24-29). Real producers instead deliver feature event payloads. For example, MutationNarrationRuntime.handlePayload validates MutationEventV1, calls renderMutationNarration, constructs { v: 1, narrations: [...] }, and only then invokes validateNarrationV1Contract (agent/packages/tiandao/src/mutation-narration-runtime.ts:127-152). Applying the planned order to a valid mutation event attempts to validate it as NarrationV1 before an envelope exists, so validation fails and the renderer/publisher production path is unreachable. The nearby registry requirement does not prevent this: it mentions payload validators, but the explicit stage contract still places the output contract before rendering.

Root cause: the plan conflates validation of the source event payload with validation of the rendered narrationv1 output and consequently specifies the output validator on the wrong side of the renderer boundary.

[major] Pub/Sub inputs cannot satisfy the proposed pre-ingest crash recovery

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:181 · concurrency-atomicity

The proposed ingest protocol says every Redis channel is acknowledged only after the pending write and that a crash before that write is recovered by source redelivery. That recovery path does not exist for the current production inputs: RedisIpc.connect() subscribes to channels and handles them through a plain on("message", ...) callback (agent/packages/tiandao/src/redis-ipc.ts:276, :751-764), whose client interface exposes no message acknowledgement (:218-228). Server event outputs are likewise ordinary Redis Publish commands (for example server/src/network/redis_bridge.rs:583-595); only special inputs such as player chat use a durable ListPush (:597-607). Therefore, for a normal subscribed narration source, a process crash after Redis delivers the message but before line 180's pending write permanently loses the event: returning from the callback is not an ack and the publisher cannot redeliver it. The open pending-store choice at line 312 does not repair this pre-write gap; even a Redis-backed pending store only helps after the subscriber has received and persisted the message. This contradicts the plan's no-silent-loss goal and its required pre-write crash fault-injection outcome. P0 must explicitly migrate each affected source transport to a durable claim/ack mechanism (and include the producer-side wiring in scope), or narrow the guarantee for Pub/Sub sources.

Root cause: the plan models redis pub/sub delivery as an acknowledged, replayable queue. pub/sub message callbacks have neither an acknowledgement operation nor source retention, so pending-store persistence cannot be made atomic with consumption and cannot recover a crash that occurs before the pending write.

[major] Mandatory consumed state has no in-scope production producer

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:281 · correctness

P5 requires cooldowns, cursors, and pending deletion to wait for a server consumed receipt (line 281), and the acceptance criteria require restart-recoverable pending work without unbounded growth. The current production bridge only deserializes NarrationV1, routes it, and sends it to clients (server/src/network/mod.rs:3295-3358); it emits no receipt. Moreover, NarrationV1 contains only v and narrations and rejects unknown fields (server/src/schema/narration.rs:16-22), so the server cannot correlate a receipt to the proposed event_id. Yet the plan limits implementation to agent and says an incompatible server contract should only produce a proposed bridge seam rather than an implementation (lines 12 and 247). Thus every successfully published narration reaches the published state but can never reach consumed; pending entries and visibility-dependent state never terminate. Existing validation cannot prevent this because it validates only the narration payload, and the listed tests cannot inject a production receipt path that does not exist.

Root cause: the plan makes server consumption acknowledgement mandatory for terminal state while excluding the server/schema producer and correlation wire contract needed to create that acknowledgement. p0 must either add an explicit cross-stack deliverable for a correlated server receipt (including schema, producer, agent consumer, and integration test) or define a bounded terminal protocol that does not depend on such a receipt.

[major] SpiritNiche privacy gate has no authorization data or enforceable consumer

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:212 · security

The plan assigns #1738 to the agent route stage and requires revealed/revealed_by filtering before fan-out (lines 209-221), then makes non-disclosure of unrevealed SpiritNiche coordinates an acceptance criterion (line 299). However, the actual agent input NicheIntrusionEventV1 contains only v, niche_pos, intruder_id, items_taken, and taint_delta (agent/packages/schema/src/social.ts:201-210), and WorldStateV1 has no niche or reveal-state collection (agent/packages/schema/src/world-state.ts:226-243). The server producer likewise drops niche_owner, reveal state, and authorized viewers when constructing the Redis payload (server/src/social/mod.rs:3368-3377). Therefore an intrusion at an unrevealed niche reaches the proposed route stage with no data from which it can distinguish the owner/revealer from unauthorized recipients. The existing server narration selector only supports broadcast, zone, or a single player target (server/src/network/mod.rs:3361-3378), so it cannot enforce the proposed recipient policy either. Yet the plan limits a server-incompatible selector to merely proposing a minimal cross-repository seam (line 247) and excludes the six cross-cluster follow-ups from acceptance, leaving no phase that actually adds the missing authorization data and consumer behavior.

Root cause: the plan places spiritniche authorization enforcement in an agent-only routing layer even though neither of its declared inputs carries niche ownership/reveal authorization and the downstream server selector cannot consume a recipient policy. the required schema/producer/consumer contract change is excluded rather than owned, so the plan's stated #1738 security deliverable and acceptance criterion are unreachable.

[major] Dimension-aware routing is not representable by the existing narration contract

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:19 · schema-contracts

The plan states that AGENT_NARRATE is consumed by the server selector according to scope/target/dimension and later requires compatibility tests for that tuple. However, the actual public wire schema agent/packages/schema/src/narration.ts:23-29 contains only v and narrations, each narration contains only scope, target, text, style, and kind (:11-20), and rejects additional properties. The Rust mirror has the same fields and deny_unknown_fields (server/src/schema/narration.rs:15-20,23-38). Finally, server/src/network/mod.rs:3361-3378 routes solely on scope and target; it neither accepts nor evaluates a dimension. Thus an implementation following the documented tuple cannot send dimension through NarrationV1 (validation rejects it), while omitting it cannot satisfy the claimed server-side dimension gate. The document acknowledges a possible future seam but still presents dimension-aware selector delivery as the current output contract and makes it a P4/P5 acceptance requirement.

Root cause: the plan treats dimension as part of the existing server narration selector contract even though it is absent from both typebox and rust wire schemas and from the selector implementation. the required schema/bridge change is deferred outside the implementation scope instead of being represented as a prerequisite deliverable.

[major] Skeleton is created despite missing mandatory workflow documents

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:31 · schema-contracts

The document says this skeleton must follow plan-refactor-master-v1.md §10.1.1, but that section requires both the repository-root CLAUDE.md and docs/CLAUDE.md to exist and be read before a skeleton is created; if either path is missing, it explicitly says not to create the skeleton (docs/plans-skeleton/plan-refactor-master-v1.md:124-128). Read-only repository globbing finds neither /workspace/CLAUDE.md nor /workspace/docs/CLAUDE.md. The new file nevertheless creates the skeleton now and defers reading those files until “before draft becomes repository skeleton” in P0, even though this diff itself adds it at the canonical repository skeleton path. This violates the plan workflow's creation precondition and leaves the cited Plan evolution/consume-plan contract unavailable.

Root cause: the new skeleton relies on two mandatory repository-relative workflow documents that are absent and postpones their prerequisite check until after the skeleton has already been created.

[major] #1702 is excluded despite being directly absorbed by the bounded-concurrency deliverable

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:157 · performance

The new plan labels #1702 (agent-unbounded-narration-concurrency) as an external follow-up that is not phase-locked or included in acceptance. That contradicts this plan's own production scope: Stage 1 requires every Redis channel to enter the unified ingress (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:179), Stage 2 requires bounded global concurrency and queue capacity (:190), P2 explicitly prohibits unbounded void handlePayload bypasses (:260), and acceptance requires all keys to stay within explicit concurrency/queue limits (:292). The current production pattern is reachable, for example AnqiNarrationRuntime.onMessage starts void this.handlePayload(...) for every message without a limit (agent/packages/tiandao/src/anqi-narration.ts:175-177). Therefore completing this plan necessarily owns the same resource-bound defect represented by #1702. Excluding it leaves its source issue without this plan's required fix/acceptance evidence or creates a second runtime-concurrency owner implementing the same boundary; the surrounding statement that follow-ups may merely reuse the interface does not resolve that overlap.

Root cause: the source ledger partitions an unbounded narration-runtime concurrency defect away from the plan even though bounded narration concurrency and removal of every unbounded runtime bypass are explicit, universal deliverables of this plan. the ownership and acceptance ledger therefore disagrees with the implementation scope.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: fd08227d4581cc6db5f08f0e8c828f2b9b7a727a
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] P0 incorrectly marks the still-deferred F05 privacy decision as closed

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:240 · strict-maintainability

P0 says F04/F05/F07 已由本次保守修订收口 (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:240). Yet the open-questions section still marks privacy/fan-out as [DEFERRED F05] and says the recipient durability/atomicity policy can only be selected after the authorization contract lands (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:320). The implementation workflow also forbids assuming a recipient policy before that contract (:356).

Root cause: the phase gate labels f05 closed while the same plan leaves f05's policy explicitly deferred and makes later work conditional on unresolved authorization data and an enforcing consumer. this gives implementers conflicting completion criteria and can allow p1/p5 acceptance to proceed without a defined privacy/fan-out protocol.

[major] Redis client resolution cannot satisfy the plan's lossless and single-side-effect guarantees

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:219 · correctness

The plan defines published as terminal as soon as the Redis PUBLISH promise resolves and then deletes pending state (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:218-220), while also promising that a temporarily unavailable consumer leaves an item replayable (:182) and that retries produce only one visible narration (:285). Redis Pub/Sub resolution only reports that Redis processed PUBLISH; the existing call returns a subscriber count (agent/packages/tiandao/src/redis-ipc.ts:937-940) and can resolve successfully with zero subscribers. Thus, with the server subscriber disconnected, the proposed path marks the item published and deletes it even though nobody received it. Conversely, if Redis delivers the message and the agent crashes after resolve but before persisting the terminal transition/deleting pending, restart recovery republishes it. The current wire cannot deduplicate that replay: NarrationV1 contains only v and narrations (agent/packages/schema/src/narration.ts:23-29), and the server routes each received narration directly without an event/correlation ID (server/src/network/mod.rs:3295-3348). The plan explicitly defers a receipt contract, so neither surrounding validation nor the proposed tests can make these guarantees true.

Root cause: the design treats redis client acknowledgement as durable end-to-end confirmation even though plain pub/sub provides neither subscriber delivery acknowledgement nor a wire-level idempotency key. that acknowledgement boundary cannot simultaneously guarantee no loss during consumer unavailability and no duplicate visible side effect during crash recovery. the plan must either weaken acceptance to explicit at-least-once/possible-loss semantics or include a production consumer acknowledgement/deduplication contract before deleting pending state.

[major] Bound the dead-letter sink used by terminal failure paths

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:220 · performance

A realistic sustained failure path reaches this store: every publish rejection is retried only up to a maximum and then moved to dead_letter (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:220); malformed items and queue-overflow policy may also choose the same terminal state (:201, :323). The plan bounds the active queue and concurrency, but nowhere defines a dead-letter capacity, retention/TTL, eviction/archive policy, cleanup owner, or a test asserting the dead-letter store remains bounded. Thus a Redis outage, persistently invalid producer, or full queue can continuously append terminal records after retries stop, producing unbounded persistent storage growth despite the acceptance claim that overload "不会无界增长" (:286).

Root cause: the proposed resource-bound contract applies only to pending work and worker concurrency; it treats dead-letter as an indefinitely retained terminal sink without specifying any lifecycle or storage bound. since multiple normal failure policies continuously feed that sink, bounded pending does not bound total pipeline storage.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 2cf80723bc17ba7bdb9fc433ace08217e54057af
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] Add a positive broadcast case to the centralized route contract

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:268 · testing

P4 introduces a centralized route policy with explicit player, zone, and broadcast transitions (lines 208-210), but its compatibility-test deliverable names only zone and player targets here, and the acceptance suite likewise names only canonical zone/player targets plus fail-closed negative cases (lines 293 and 302). A concrete implementation that correctly routes player/zone events but rejects every legitimate scope="broadcast" event would therefore satisfy all listed route compatibility cases. Existing tests exercise the pre-migration runtime/server broadcast paths, not the new pipeline's route-to-publish path, so they do not prove that the centralized route preserves broadcast after old direct publishers are removed. Add a positive pipeline integration case for a legitimate broadcast event, alongside the existing zone/player and fail-closed cases.

Root cause: the planned regression matrix does not cover every variant of the changed narrationscope routing contract; it omits the positive broadcast transition even though p4 centralizes and replaces all runtime routing.

[major] #1590 misclassifies the documented offline:<name> target as incompatible

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:109 · schema-contracts

The plan states that halfstep-rechallenge-narration.ts:167-169 forwards a raw payload.char_id that is not a normalized player target, and therefore assigns #1590 to P4 target repair. The cited renderer does forward payload.char_id, but the producer contract explicitly defines player values as offline:<name> (server/src/schema/halfstep_rechallenge.rs:13-14,21-24). The actual server boundary deliberately normalizes this representation: normalize_player_target strips the offline alias prefix and lowercases the result (server/src/network/agent_bridge.rs:285-302), then player routing compares it against normalized usernames and char IDs (server/src/network/agent_bridge.rs:305-334). For example, offline:Azure is normalized to azure and matches username Azure; surrounding validation does not reject or lose this route. Thus the cited call path is already compatible with the real consumer, while P4 explicitly requires #1590 to be changed and the raw-ID path removed.

Root cause: the plan evaluates the producer value as though the agent must emit the server's final normalized lookup key, overlooking that normalization is intentionally performed by the server selector and that offline:<name> is the documented producer representation. this makes the source finding and its p4 deliverable factually incorrect and can direct implementation to replace a working cross-boundary identity contract without any alternative identity being present in the payload.

[major] The Section 8 gate makes P0 impossible to enter

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:307 · strict-maintainability

Section 8 is titled “Open questions(进入 P0 前必须收口)”, so its questions are a precondition for entering P0. However, the P0 checklist at lines 241 and the PR-1 workflow at line 353 assign F06/F08/F09 resolution to P0 itself, and several other Section 8 items explicitly say “P0 必须冻结”. Following the heading literally prevents PR-1/P0 from starting until work that PR-1/P0 is responsible for has already completed; following the checklist instead violates the stated gate. The plan therefore has no internally consistent entry criterion for its first implementation phase.

Root cause: the document conflates pre-p0 prerequisites with decisions that are deliverables of p0, defining the same decisions on both sides of the phase gate. separate true prerequisites from p0 decision work, or rename the gate to require closure before dependent phases rather than before p0.

[major] The new plan is missing from the master ownership matrix

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:48 · strict-maintainability

The new document declares an independent Agent implementation owner outside R1-R10 (line 48), but it does not register this new skeleton in the canonical master plan's coverage matrix. docs/plans-skeleton/plan-refactor-master-v1.md:76-83 says that matrix guarantees every active/skeleton plan has an owner, requires each newly added skeleton to be classified immediately, and lists the Agent-track skeletons without agent-narration-pipeline. Thus the repository's stated coverage audit will report this newly introduced plan as unassigned, contradicting both this plan's ownership claim and the master plan's completeness contract.

Root cause: the pr creates a new independently owned skeleton without updating the repository's canonical plan-to-owner source of truth, leaving two planning documents inconsistent at introduction time.

[major] In-memory pending option contradicts mandatory process-restart recovery

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:308 · correctness

The plan repeatedly makes post-ingest process-restart recovery a required P1 and acceptance contract: stage 1 says items written before a worker/process crash are recovered from the pending store (lines 178-181), P1 requires “内部重启恢复” (lines 244-247), and acceptance requires non-terminal pending items to recover after process restart (line 286). However, open question 8.1 explicitly permits either an in-memory or persistent pending store while stating that either can recover already-written events (line 308). A concrete P0 choice of an in-memory queue therefore satisfies the documented option but loses every queued event when the process exits, making the required restart-recovery tests and acceptance criterion impossible. The Pub/Sub pre-ingest disclaimer does not prevent this: the affected event was successfully written to pending before the crash.

Root cause: the plan conflates successful insertion into an in-process memory queue with durable pending persistence. process-restart recovery requires a store whose contents survive the process (and whose durability/restore behavior is specified); an in-memory option can provide retry only while the same process remains alive.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md`:
- Around line 389-391: 将 §10 的最后一节标题从“10.6 单次 consume-plan 收口”重命名为“10.6 单次
consume-plan 全自动到 merge”,保留现有正文内容,并确保该章节仍是 §10 的最后一节。
- Around line 332-336: 在该容量与生命周期策略表中新增 pending policy row,明确其
max_items、max_bytes、单条 max_payload_bytes、retention_ttl、cleanup/archive owner
及满载时的处理策略。同步扩展验收证据,持续注入 pending 写入与恢复场景,验证 item/byte/TTL 上限、过期清理以及满载 overflow
telemetry,并继续断言 pending 始终有界。
- Around line 365-383: 在“每 PR 的验证与文档更新”和“独立实施上下文”中补充可执行提交门禁:每个逻辑单元使用一个 atomic
commit,commit message 必须为中文,且 agent 生成的每个 commit 包含调用前解析的精确模型 ID 对应的 Model
trailer。将 Agent 示例中的 model: "opus" 改为精确模型 ID,并要求该 ID 写入 commit trailer,不得仅写入 PR
body;同时明确禁止 --no-verify、--no-gpg-sign、未经确认的 force push、hard reset、amend 和交互式
rebase。
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 99c26e9a-77b6-4a77-8f2d-6506b7c7a65a

📥 Commits

Reviewing files that changed from the base of the PR and between 460eea7 and b2e0271.

📒 Files selected for processing (1)
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md

📄 CodeRabbit inference engine (docs/CLAUDE.md)

docs/**/*.md: 新 plan 头部必须写明接入面:进料、出料、复用的共享类型/event/schema、server/agent/client 跨仓库契约,以及对应的 worldview.md 锚点。
涉及真元、灵气、衰减、逸散、半衰、距离损耗、排斥或吸力的 plan,必须调用 qi_physics;新物理常数和公式必须先扩展 qi_physics,不得在功能 plan 中重复实现。
所有真元/灵气流动必须遵守守恒律并通过 qi_physics::ledger::QiTransfer;释放使用 qi_release_to_zone,吸收使用 qi_excretion,不得凭空生成或销毁真元。
涉及玩家可感知行为的 plan,必须在对应机制阶段中内联可直接实现的粒子、音效、HUD、环境、动画和 narration 规格;不得将视听内容笼统推迟到独立阶段。纯 server 逻辑 plan 例外。
每份 plan 必须列出开放问题;实施前必须追加 §N.1 决议,逐项给出结论、实施方案、边界条件,并以文件:行号和 plan 章节双锚点落地。
scope 大于或等于 4 个 PR 的 plan,必须在末尾包含 §10 实施工作流,并按依赖顺序在一个 plan 内序列化多个 PR,不得拆成多个 plan。
涉及 NBT 建筑、worldgen layout 或复杂视觉资产的 TODO,必须完成三轮提交:(round 1/3)(round 2/3)(round 3/3);终轮提交必须包含拼写准确的 <PROMISE> 担保块。
§10 最末必须包含“单次 consume-plan 全自动到 merge”章节,明确用户提交 /consume-plan 后即可等待最终归档至 docs/finished_plans/

引用世界观内容时统一使用 worldview.md §X L<line> 格式;不得自动修改 docs/worldview.md 或主动回写 docs/library/

Files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
docs/plans-skeleton/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

新 plan 必须先读取 docs/CLAUDE.md;骨架、Active 和 Finished plan 必须遵循三态流转及规定的阶段状态、Finish Evidence 结构。

Files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: commit message 必须使用中文,每个逻辑单元一个 atomic commit;agent 生成的 commit 必须带真实模型 Model: <精确模型 id> trailer。
禁止使用 --no-verify--no-gpg-sign、关闭签名配置、未经确认的 force push、hard reset、amend 或交互式 rebase。
禁止留下 auto-stash 产生的孤儿 WIP stash;自动 stash 流程完成后必须恢复自己的 stash。

Files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
🧠 Learnings (3)
📚 Learning: 2026-07-17T00:31:10.779Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:1-1
Timestamp: 2026-07-17T00:31:10.779Z
Learning: 在审查该仓库 `docs/plans-skeleton/` 下的“docs-only skeleton plan”创建类 PR 时:先核对 `docs/CLAUDE.md` 中“Plan 消费规范”,并逐份查看本计划文档里的“§10 实施工作流”,确认后续实施阶段是否会遵循“每个 PR 只修改一个 plan”,且实施/归档时不会出现跨 plan 的修改。该规则不适用于用户显式指定、共享调研基线且计划之间存在互相交叉引用的 skeleton plan 同批创建 PR;对这类情况应按实际交叉引用关系放宽,确保仍能按独立或约定的序列化方式推进。

Applied to files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-17T00:31:13.643Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:81-85
Timestamp: 2026-07-17T00:31:13.643Z
Learning: 审核 docs/plans-skeleton/*.md 下的 skeleton 草案 PR 时:不得要求作者在“§N 开放问题(P0 决策门前需收口)”核查完成之前提前填写对应的“§N.1 决议”。仅当计划进入 active 且 P0 实施前,已由 Explore agent 并行核查代码现状后,才允许追加“§N.1 决议”,且该决议需包含结论、实施方案、边界条件,并使用“文件:行号 + plan 章节”的双锚点格式。

Applied to files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-22T02:11:59.191Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1247
File: docs/plans-skeleton/plan-bughunt-animal-air-spawn-gravity-v1.md:0-0
Timestamp: 2026-07-22T02:11:59.191Z
Learning: 在 `docs/plans-skeleton/` 目录下的计划状态行中,若起草日期同时涉及 UTC 与本地日期(例如时区换算后可能跨到不同日期),请在该行中同时标注 UTC 起草日与本地起草日。这样可以避免基于 UTC 的时间基准在 GitHub/CodeRabbit 审查时将本地日期误判为“未来日期”。

Applied to files:

  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
🪛 LanguageTool
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md

[grammar] ~133-~133: Ensure spelling is correct
Context: ...ce issue 清单与唯一聚类 ### 4.1 publish-error-boundary(5 个 source ID;其中 4 个为 #1470 重复证据) `#1470...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~145-~145: Ensure spelling is correct
Context: ... P4 raw-ID 修复范围。 ### 4.3 agent-publish-idempotency(10 个 confirmed source ID) #1616 #1619 #1`...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~191-~191: Ensure spelling is correct
Context: ...batch/event id,但这不等于当前 Pub/Sub receiver 已实现可见副作用去重。 2. 以 ordering_key 对同一 player/entity/z...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~192-~192: Ensure spelling is correct
Context: ...yer/entity/zone 的事件串行化;不同 key 可 bounded 并发,设置全局并发上限和队列上限。 3. 将 feature cooldown、lastStatusByFaction、...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~193-~193: Ensure spelling is correct
Context: ...confirm 后 commit 的 side effect;pipeline 在事件成功前不调用不可逆状态推进。 4. 明确 stale event、重复 event、跨 restart ti...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[uncategorized] ~297-~297: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... privacy closure。两项保持 §8 contract gap 与 open source issue。 - agent producer/consumer schem...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


[grammar] ~310-~310: Ensure spelling is correct
Context: ...cceptance,由各自 owner 提供独立证据。 ## 8. Open questions(P0 内收口;P0 退出前冻结依赖决策) 本节不构成 P0 entry block...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (2)
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md (2)

33-33: 📐 Maintainability & Code Quality

无需额外修改。

本节已声明读取了计划工作流依赖的规则,其余 flow 已在 §10.6 与 Finish Evidence 收尾规范中对应。


211-215: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

Authorization Bypass (CWE-862): Missing Authorization

为 player/zone target 增加授权边界。

当前 route 设计验证 canonical (scope, target) 的格式和存在性,但没有证明 producer 有权向该 player 或 zone 发布。

具体路径是:Redis/producer payload → source validator → NarrationV1 validator → route → AGENT_NARRATE。如果不可信或跨租户输入可以到达 ingress,攻击者可以提交格式正确的目标并向其他玩家或区域发送 narration。

请补充 producer 身份认证与 target authorization,或明确并验证 ingress 只接受受信 producer。必须增加未授权 player/zone target 的负向测试。现有的 fail-closed broadcast 规则和 #1738 deferred 说明不能替代该授权控制。

#!/bin/bash
set -euo pipefail

rg -n -C 8 \
  'RuntimeRedis|AGENT_NARRATE|publishNarrations|validateNarrationV1Contract|scope|target|authorize|authorization|authz' \
  agent server || true

Comment thread docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
Comment on lines +365 to +383
### 10.3 每 PR 的验证与文档更新

- agent 代码:`cd agent/packages/tiandao && npm test`;schema source 改动先 `cd agent && npm run build -w @bong/schema`,再跑 `cd agent && npm run build`。
- 每 PR 只完成对应阶段;测试证据写入当前 active plan 的阶段记录,最后一个 PR 才补 `## Finish Evidence` 并归档。
- 任一实现发现需要 server/schema/client 跨栈扩展时,停在 §8 contract gap,不以 mock receipt、测试专用 authorization 或未消费 dimension 字段代替生产链。

### 10.4 独立实施上下文

主线不亲自实施 PR;每个 PR 按 `docs/CLAUDE.md §6.4` 启动独立实现 subagent,prompt 必须限定该 PR 范围、必读本节、门禁与禁止扩展的 §8 contract gap,并以 `ultrathink` 结尾:

```text
Agent(
subagent_type: "claude",
model: "opus",
prompt: "<当前 PR 范围、依赖、测试与禁止项>\n\nultrathink"
)
```

subagent 只负责实现、测试、push 与创建该 PR,不等待 review、不 merge;主线只接收 PR URL、HEAD 与简短门禁证据。执行模型、validator 与 reviewer 的真实模型 ID 必须按当时路由记录在 commit trailer/PR body,不能以模板占位名冒充实际上游模型。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

把提交与模型身份规则写成可执行门禁。

§10.3-10.4 没有明确以下强制规则:

  • 每个逻辑单元使用一个 atomic commit。
  • commit message 必须使用中文。
  • agent 生成的 commit 必须包含 Model: <精确模型 id> trailer。
  • 禁止 --no-verify--no-gpg-sign、未经确认的 force push、hard reset、amend 和交互式 rebase。

此外,model: "opus" 不是精确模型 ID,且 Line 383 允许把真实模型 ID 写入 PR body。请改为调用前解析的精确模型 ID,并要求每个 agent commit 写入 trailer。否则 subagent 可能生成不符合仓库规则的提交。

建议补充实施门禁
-  model: "opus",
+  model: "<调用前由当前路由解析的精确模型 ID>",
...
+ - 每个逻辑单元使用一个 atomic commit,commit message 必须使用中文。
+ - 每个 agent 生成的 commit 必须包含 `Model: <精确模型 id>` trailer,不能只写入 PR body。
+ - 禁止使用 `--no-verify`、`--no-gpg-sign`、未经确认的 force push、hard reset、amend 或交互式 rebase。
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
### 10.3 每 PR 的验证与文档更新
- agent 代码:`cd agent/packages/tiandao && npm test`;schema source 改动先 `cd agent && npm run build -w @bong/schema`,再跑 `cd agent && npm run build`
- 每 PR 只完成对应阶段;测试证据写入当前 active plan 的阶段记录,最后一个 PR 才补 `## Finish Evidence` 并归档。
- 任一实现发现需要 server/schema/client 跨栈扩展时,停在 §8 contract gap,不以 mock receipt、测试专用 authorization 或未消费 dimension 字段代替生产链。
### 10.4 独立实施上下文
主线不亲自实施 PR;每个 PR 按 `docs/CLAUDE.md §6.4` 启动独立实现 subagent,prompt 必须限定该 PR 范围、必读本节、门禁与禁止扩展的 §8 contract gap,并以 `ultrathink` 结尾:
```text
Agent(
subagent_type: "claude",
model: "opus",
prompt: "<当前 PR 范围、依赖、测试与禁止项>\n\nultrathink"
)
```
subagent 只负责实现、测试、push 与创建该 PR,不等待 review、不 merge;主线只接收 PR URL、HEAD 与简短门禁证据。执行模型、validator 与 reviewer 的真实模型 ID 必须按当时路由记录在 commit trailer/PR body,不能以模板占位名冒充实际上游模型。
### 10.3 每 PR 的验证与文档更新
- agent 代码:`cd agent/packages/tiandao && npm test`;schema source 改动先 `cd agent && npm run build -w `@bong/schema``,再跑 `cd agent && npm run build`
- 每 PR 只完成对应阶段;测试证据写入当前 active plan 的阶段记录,最后一个 PR 才补 `## Finish Evidence` 并归档。
- 任一实现发现需要 server/schema/client 跨栈扩展时,停在 §8 contract gap,不以 mock receipt、测试专用 authorization 或未消费 dimension 字段代替生产链。
- 每个逻辑单元使用一个 atomic commit,commit message 必须使用中文。
- 每个 agent 生成的 commit 必须包含 `Model: <精确模型 id>` trailer,不能只写入 PR body。
- 禁止使用 `--no-verify``--no-gpg-sign`、未经确认的 force push、hard reset、amend 或交互式 rebase。
### 10.4 独立实施上下文
主线不亲自实施 PR;每个 PR 按 `docs/CLAUDE.md §6.4` 启动独立实现 subagent,prompt 必须限定该 PR 范围、必读本节、门禁与禁止扩展的 §8 contract gap,并以 `ultrathink` 结尾:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 365 -
383, 在“每 PR 的验证与文档更新”和“独立实施上下文”中补充可执行提交门禁:每个逻辑单元使用一个 atomic commit,commit
message 必须为中文,且 agent 生成的每个 commit 包含调用前解析的精确模型 ID 对应的 Model trailer。将 Agent
示例中的 model: "opus" 改为精确模型 ID,并要求该 ID 写入 commit trailer,不得仅写入 PR body;同时明确禁止
--no-verify、--no-gpg-sign、未经确认的 force push、hard reset、amend 和交互式 rebase。

Source: Coding guidelines

Comment on lines +389 to +391
### 10.6 单次 consume-plan 收口

用户单次提交 `/consume-plan` 后,主线按 PR-1 → PR-6 串行完成实现、review 收敛与 merge,不要求用户逐 PR 接力。所有 in-scope source 有修复/测试证据、#1738 明确保留为 open contract gap、全部阶段标记完成并补齐 `## Finish Evidence` 后,才可迁入 `docs/finished_plans/`;不得以 skeleton/关联 PR/日志可见替代真实实现关闭。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

使用规定的 consume-plan 章节名称。

仓库规则要求 §10 最末包含“单次 consume-plan 全自动到 merge”章节。当前标题是 10.6 单次 consume-plan 收口。正文虽然描述了自动串行实现、review、merge 和归档流程,但标题不符合规定名称。

请将该节重命名为“单次 consume-plan 全自动到 merge”,并保持它为 §10 的最后一节。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 389 -
391, 将 §10 的最后一节标题从“10.6 单次 consume-plan 收口”重命名为“10.6 单次 consume-plan 全自动到
merge”,保留现有正文内容,并确保该章节仍是 §10 的最后一节。

Source: Coding guidelines

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: b2e027129d2d63962a3505ce9c85a42d2e77d54e
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] Pipeline phases absorb unrelated world-model and query-tool defects

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:187 · strict-maintainability

The plan assigns #1746 to P1 at this line and again to P3, even though the cited changed behavior is world-model.ts::cloneZoneSnapshot dropping ZoneSnapshot.status. It likewise assigns query-player.ts relationship result bounding (#1674) to P3 and the mixed-window calculation in query-zone-history.ts (#1607) to P4. The repository confirms these are separate canonical logic defects: agent/packages/tiandao/src/world-model.ts:1089-1096 clones a world-model snapshot while omitting status; agent/packages/tiandao/src/tools/query-player.ts:119-129 exposes the relationships array without a bound; and agent/packages/tiandao/src/tools/query-zone-history.ts:111-127 computes a bounded localDelta but obtains trendDelta from the model's separate trend window. None is caused by, or requires, the proposed ingest/dedupe/validate/route/publish transport lifecycle. Folding these independent fixes into pipeline phases means P1/P3/P4 cannot remain the claimed focused infrastructure changes, and ownership of the actual world-model/tool contracts is obscured.

Root cause: the source ledger was clustered by broad agent/narration adjacency rather than by a shared implementation root cause, so unrelated world-model cloning and query-tool correctness fixes were made deliverables of the narration transport abstraction instead of receiving minimal, authoritative fixes in their own domains.

[major] Register the new skeleton in the authoritative master matrix

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:30 · correctness

The new plan explicitly leaves master registration for a later PR (Master matrix note ... remains pending, line 30). However, docs/plans-skeleton/plan-refactor-master-v1.md:78 says every newly added skeleton must be categorized in the master matrix immediately, and line 95 says the coverage audit reports any uncategorized plan as an error. The master’s explicit Agent-track list at line 82 does not contain plan-agent-narration-pipeline-v1. Therefore, adding this file in the current diff leaves it unassigned in the authoritative coverage matrix and makes the mandated plan coverage audit fail; declaring an implementation owner inside this plan does not satisfy the separate master registration contract.

Root cause: the change intentionally defers the authoritative master ownership-matrix update even though the master plan requires new skeleton registration in the same intake and treats missing registration as a coverage error.

[major] published can become terminal before its business-state commit

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:175 · concurrency-atomicity

The envelope declares published to be terminal (line 175), while feature cooldown/cursor/status updates are deferred until publish confirmation (lines 193 and 277). These are separate durable changes, but the plan specifies neither an atomic transaction nor a recoverable intermediate published_pending_commit state. A reachable sequence is: Redis PUBLISH resolves, the envelope is persisted as terminal published, and the process crashes before committing lastStatusByFaction, cooldown, or lastProcessedStateCursor. On restart, recovery only covers pending/nonterminal events (lines 183 and 290), so this terminal item is not revisited and the business state remains stale. Reversing the write order merely creates the already-documented duplicate-publish window. The required tests cover a crash after resolve before terminal persistence (lines 288 and 305), but not a crash after terminal persistence before the related state commit, so they do not prevent this partial state.

Root cause: the proposed state machine treats publisher confirmation as a terminal durable transition while describing dependent business-state updates only as later “confirm 后 commit” side effects. it does not define an atomic commit boundary or restart reconciliation protocol spanning the terminal envelope state and those related updates.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
server/src/cmd/dev/season.rs (2)

130-147: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

缺少 SeasonCmd::Query 的测试用例。

handle_season 处理三个 SeasonCmd variant。测试覆盖了 SetAdvance,没有覆盖 QueryQuery 分支的独有行为是不改变 WorldSeasonState、不发送 SeasonChangedEvent、但仍返回状态消息。

按编码规范,每个 enum variant 至少需要一条用例。

依据编码规范:“schema、enum 和状态机必须有专属 pin 测试;TypeBox/serde variant 要有正反 sample 对拍,每个 enum variant 和状态转换至少有一条用例。”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/src/cmd/dev/season.rs` around lines 130 - 147, 为 handle_season 增加覆盖
SeasonCmd::Query 的专属测试用例,验证查询返回当前状态消息,同时不修改 WorldSeasonState 且不发送
SeasonChangedEvent;保留现有 Set 和 Advance 测试,并遵循已有测试的断言与构造方式。

Source: Coding guidelines


233-246: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

DevCommandPermissions::default() 让拒绝路径依赖真实环境变量。

Defaultfrom_connection_mode(None),会读取进程的 BONG_OPERATORSBONG_OPERATORS_ALLOW_OFFLINE。该单元测试不隔离环境变量。若运行环境中 BONG_OPERATORS 恰好包含 Aliceslash_season_command_rejected_for_non_op_player 就不再测试拒绝路径。

集成测试 server/tests/dev_command_operator_gate.rsENV_MUTEXScopedEnvVars 做了隔离,这里没有。

建议在 server/src/cmd/dev/mod.rs 增加一个不读环境变量的测试构造器,让拒绝意图显式化。

♻️ 建议的确定性测试构造器

server/src/cmd/dev/mod.rsimpl DevCommandPermissions 中新增:

#[cfg(test)]
pub fn deny_all() -> Self {
    Self {
        allowed_usernames: HashSet::new(),
        usernames_are_authenticated: true,
    }
}

然后在本文件中使用:

         if allow_alice {
             app.insert_resource(DevCommandPermissions::allow_user("Alice"));
         } else {
-            app.insert_resource(DevCommandPermissions::default());
+            app.insert_resource(DevCommandPermissions::deny_all());
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/src/cmd/dev/season.rs` around lines 233 - 246, Make the rejection
setup in setup_command_app deterministic by replacing the false branch’s
DevCommandPermissions::default() with the test-only
DevCommandPermissions::deny_all() constructor. Add deny_all to the
DevCommandPermissions impl in mod.rs under cfg(test), initializing an empty
allowed_usernames set with usernames_are_authenticated enabled, so the test does
not read environment variables.
♻️ Duplicate comments (3)
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md (3)

388-390: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

使用规定的 consume-plan 章节名称。

请将 §10 最后一节标题改为“单次 consume-plan 全自动到 merge”,并保持该节为 §10 的最后一节。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 388 -
390, 将 §10 的最后一节标题从“单次 consume-plan 收口”改为“单次 consume-plan 全自动到
merge”,保持该节内容不变,并确保它仍是 §10 的最后一节。

Source: Coding guidelines


329-335: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

pending 补齐生命周期和溢出策略。

当前表格只定义 dead_letter。Line 335 却要求 pending 也受独立上限约束。

请增加 pending policy row,至少定义 max_itemsmax_bytes、单条 max_payload_bytes、retention/TTL、cleanup owner 和满载策略。同步增加写入、恢复、过期清理和 overflow telemetry 测试。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 329 -
335, 在“backpressure 与优先级 [DECISION-NEEDED F09]”的 dead-letter policy table 中新增
pending policy row,明确有限的
max_items、max_bytes、max_payload_bytes、retention_ttl、cleanup owner 与满载策略,并确保该策略与
pending 的写入、恢复及过期清理流程一致;补充测试覆盖 pending 写入、恢复、TTL 清理和容量溢出,并验证 overflow telemetry
可查询且 pending 不超过各项上限。

372-382: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

将提交身份和提交命令写成强制门禁。

model: "opus" 不是精确模型 ID。当前文本也没有强制中文 commit、每个逻辑单元一个 atomic commit、每个 agent commit 的 Model: <精确模型 id> trailer,以及禁止 --no-verify--no-gpg-sign、未经确认的 force push、hard reset、amend 和交互式 rebase。

请把这些规则写入 §10.4。模型 ID 必须写入每个 agent 生成的 commit trailer,不能只写入 PR body。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 372 -
382, 在 §10.4 补充强制提交与提交命令门禁:要求使用中文 commit message、每个逻辑单元一个 atomic commit,并在每个
agent 生成的 commit trailer 中记录实际精确模型 ID(不要使用“opus”等占位名,也不能只记录在 PR body)。同时明确禁止使用
--no-verify、--no-gpg-sign、未经确认的 force push、hard reset、amend 及交互式
rebase,并保留现有执行模型、validator 和 reviewer 的真实模型 ID 记录要求。

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/review-policy/bong.v2.json:
- Line 133: Update the exact contract assertion for the p0-doc-scope policy in
.github/scripts/review-consumer.test.mjs to match the revised text at the policy
entry, preserving the existing assert.equal-based validation and all unrelated
expectations.

In `@docs/plan-refactor-inventory-core-v1.md`:
- Line 9: 为所有 scope ≥4 PR 的计划补齐或重排统一的“§10 实施工作流”:在
docs/plan-refactor-inventory-core-v1.md:9 为 R10
P0-P4、docs/plans-skeleton/plan-refactor-c2s-gate-v1.md:23 为 R4
P0-P4、docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md:22-23 为 R9
P0-P4、docs/plans-skeleton/plan-refactor-persistence-slices-v1.md:28-31 为 R3
P0-P5、docs/plans-skeleton/plan-refactor-wire-s2c-v1.md:21-25 为 R6 P0-P5
添加前置依赖与串行 PR 顺序、每 PR 独立 subagent、测试和 review gate、atomic commit、中文 commit、精确
Model trailer 及禁止命令;在 docs/plans-skeleton/plan-refactor-master-v1.md:44-80 重排
§10,同时保留 flash-review 清算职责。所有计划的最后一节标题必须是“单次 consume-plan 全自动到 merge”。

In `@docs/plans-skeleton/plan-refactor-master-v1.md`:
- Around line 57-80: 统一跨轨 activation 顺序,避免 master 与子计划声明冲突的启动门:在
docs/plans-skeleton/plan-refactor-master-v1.md(57-80)确认并记录 R10 P2b
的完整前置,或明确仅保留现有 R6 P3 gate;随后同步
docs/plan-refactor-inventory-core-v1.md(90-98),删除未被 master 权威覆盖的 R5 P3、R10 P3、R4
pickup consumer 启动前置,确保所有子计划遵循 master 的 Wave/dependency 顺序。

In `@scripts/bot-e2e.sh`:
- Around line 26-43: Remove the independent hard-coded operator roster in
scripts/bot-e2e.sh lines 26-43 by deriving the complete list from the scenario
definitions, or assert after roster generation that every bot username used in
this run is covered. In scripts/e2e-redis.sh lines 1239-1249, eliminate the
hard-coded NRift suffix by consuming the scenario-produced bot username or the
same shared constant; preserve the existing run-tag flow at line 1311.

In `@server/src/cmd/dev/mod.rs`:
- Around line 244-263: 更新 gate_dev_commands 中的命令根解析逻辑,先去除前导空白和前导“/”再提取根命令,以确保
CommandExecutionEvent.command 中的“/qi”等形式能匹配
DevCommandRoots。保持该函数仅发送权限提示,不在此处消费事件或额外拒绝命令,继续沿用现有的 operator 校验与执行拦截流程。

In `@server/tests/dev_command_operator_gate.rs`:
- Around line 89-93: 补充 DevCommandPermissions::from_connection_mode 的专属测试,覆盖
ConnectionMode 的 BungeeCord 和 Velocity variant,并断言其授权结果符合 Online/Offline
的预期。调整或新增 setup_app 测试场景,使 NetworkSettings 缺失时传入 None,覆盖
from_connection_mode(None) 的 fail-open 分支并断言结果。保持现有 Online、Offline 用例不变,确保每个
ConnectionMode variant 及缺失配置路径都有测试。
- Around line 382-390: 将生产注册清单中的 identity 补入 registry_pin::COMMAND_NAMES 和
COMMAND_TREE_PATHS,使其与 IdentityCmd::assemble_graph 注册的根级命令一致;随后在生产命令树 pin 测试中移除
expected_roots.insert("identity") 硬编码补丁,并保留基于 COMMAND_NAMES 生成 expected_roots
的校验逻辑。

---

Outside diff comments:
In `@server/src/cmd/dev/season.rs`:
- Around line 130-147: 为 handle_season 增加覆盖 SeasonCmd::Query
的专属测试用例,验证查询返回当前状态消息,同时不修改 WorldSeasonState 且不发送 SeasonChangedEvent;保留现有 Set 和
Advance 测试,并遵循已有测试的断言与构造方式。
- Around line 233-246: Make the rejection setup in setup_command_app
deterministic by replacing the false branch’s DevCommandPermissions::default()
with the test-only DevCommandPermissions::deny_all() constructor. Add deny_all
to the DevCommandPermissions impl in mod.rs under cfg(test), initializing an
empty allowed_usernames set with usernames_are_authenticated enabled, so the
test does not read environment variables.

---

Duplicate comments:
In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md`:
- Around line 388-390: 将 §10 的最后一节标题从“单次 consume-plan 收口”改为“单次 consume-plan 全自动到
merge”,保持该节内容不变,并确保它仍是 §10 的最后一节。
- Around line 329-335: 在“backpressure 与优先级 [DECISION-NEEDED F09]”的 dead-letter
policy table 中新增 pending policy row,明确有限的
max_items、max_bytes、max_payload_bytes、retention_ttl、cleanup owner 与满载策略,并确保该策略与
pending 的写入、恢复及过期清理流程一致;补充测试覆盖 pending 写入、恢复、TTL 清理和容量溢出,并验证 overflow telemetry
可查询且 pending 不超过各项上限。
- Around line 372-382: 在 §10.4 补充强制提交与提交命令门禁:要求使用中文 commit message、每个逻辑单元一个
atomic commit,并在每个 agent 生成的 commit trailer 中记录实际精确模型 ID(不要使用“opus”等占位名,也不能只记录在
PR body)。同时明确禁止使用 --no-verify、--no-gpg-sign、未经确认的 force push、hard reset、amend
及交互式 rebase,并保留现有执行模型、validator 和 reviewer 的真实模型 ID 记录要求。
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9f8f7062-3cd8-44bb-860d-2849d5b0b998

📥 Commits

Reviewing files that changed from the base of the PR and between b2e0271 and 29bce3a.

📒 Files selected for processing (13)
  • .github/review-policy/bong.v2.json
  • docs/plan-refactor-inventory-core-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • scripts/bot-e2e.sh
  • scripts/e2e-redis.sh
  • server/src/cmd/dev/mod.rs
  • server/src/cmd/dev/season.rs
  • server/tests/dev_command_operator_gate.rs
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: e2e
⚠️ CI failures not shown inline (2)

GitHub Actions: Review consumer tests / test: skeleton: agent 叙事管道计划 v1(§10.1.1 促升)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m[[ "$(git -C _central-contract rev-parse HEAD)" == '2986b95388b3ac3f8bf28b8ebdd60a19d2e4364c' ]]�[0m
 �[36;1m[[ "$(git -C 9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 rev-parse HEAD)" == '9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784' ]]�[0m
 �[36;1mnode --check .github/scripts/review-consumer.test.mjs�[0m
 �[36;1mCENTRAL_REVIEW_CONTRACT_DIR=_central-contract \�[0m
 �[36;1m  PROVIDER_CANARY_CONTRACT_DIR=9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 \�[0m
 �[36;1m  node --test .github/scripts/review-consumer.test.mjs�[0m
 �[36;1m(�[0m
 �[36;1m  cd _central-contract�[0m
 �[36;1m  npm test�[0m
 �[36;1m)�[0m
 �[36;1mgit diff --check�[0m
 shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
 env:
   BWRAP_EXECUTABLE: /home/runner/work/Bong/Bong/_sandbox-package/root/usr/bin/bwrap
 ##[endgroup]
 ✔ shadow caller pins the central workflow and preserves the trusted trigger gate (5.809278ms)
 ✔ shadow caller job exactly matches the reviewed reusable contract (1.430291ms)
 ✔ consumer CI checks out and tests the exact central workflow contract (0.879811ms)
 ✔ checked-out central workflow matches the complete immutable publication contract (2.736814ms)
 ✔ central publication contract rejects every input and secret set drift (3.237405ms)
 ✔ caller publication contract rejects mapping, secret, and permission drift (1.604421ms)
 ✔ central permission contract rejects unbound and non-block permission forms (1.549429ms)
 ✔ checked-out provider canary release matches its caller interface (1.760644ms)
 ✔ provider canary publication contract rejects every input and secret set drift (1.420236ms)
 ✔ provider canary remains dispatch-only, minimally permissioned, and secret-isolated (2.388429ms)
 ✖ Bong policy is bounded declarative data with canonical project rules (2.230745ms)
 ℹ tests 11
 ℹ suites 0
 ℹ pass 10
 ℹ fail 1
 ℹ cancelled 0
 ℹ skipped 0
 ℹ todo 0
 ℹ duration_ms 84.962885
 ✖ failing tests:
 test at .github/scripts/r...

GitHub Actions: Review consumer tests / 0_test.txt: skeleton: agent 叙事管道计划 v1(§10.1.1 促升)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m[[ "$(git -C _central-contract rev-parse HEAD)" == '2986b95388b3ac3f8bf28b8ebdd60a19d2e4364c' ]]�[0m
 �[36;1m[[ "$(git -C 9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 rev-parse HEAD)" == '9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784' ]]�[0m
 �[36;1mnode --check .github/scripts/review-consumer.test.mjs�[0m
 �[36;1mCENTRAL_REVIEW_CONTRACT_DIR=_central-contract \�[0m
 �[36;1m  PROVIDER_CANARY_CONTRACT_DIR=9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 \�[0m
 �[36;1m  node --test .github/scripts/review-consumer.test.mjs�[0m
 �[36;1m(�[0m
 �[36;1m  cd _central-contract�[0m
 �[36;1m  npm test�[0m
 �[36;1m)�[0m
 �[36;1mgit diff --check�[0m
 shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
 env:
   BWRAP_EXECUTABLE: /home/runner/work/Bong/Bong/_sandbox-package/root/usr/bin/bwrap
 ##[endgroup]
 ✔ shadow caller pins the central workflow and preserves the trusted trigger gate (5.809278ms)
 ✔ shadow caller job exactly matches the reviewed reusable contract (1.430291ms)
 ✔ consumer CI checks out and tests the exact central workflow contract (0.879811ms)
 ✔ checked-out central workflow matches the complete immutable publication contract (2.736814ms)
 ✔ central publication contract rejects every input and secret set drift (3.237405ms)
 ✔ caller publication contract rejects mapping, secret, and permission drift (1.604421ms)
 ✔ central permission contract rejects unbound and non-block permission forms (1.549429ms)
 ✔ checked-out provider canary release matches its caller interface (1.760644ms)
 ✔ provider canary publication contract rejects every input and secret set drift (1.420236ms)
 ✔ provider canary remains dispatch-only, minimally permissioned, and secret-isolated (2.388429ms)
 ✖ Bong policy is bounded declarative data with canonical project rules (2.230745ms)
 ℹ tests 11
 ℹ suites 0
 ℹ pass 10
 ℹ fail 1
 ℹ cancelled 0
 ℹ skipped 0
 ℹ todo 0
 ℹ duration_ms 84.962885
 ✖ failing tests:
 test at .github/scripts/r...
🧰 Additional context used
📓 Path-based instructions (5)
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: commit message 必须使用中文,每个逻辑单元一个 atomic commit;agent 生成的 commit 必须带真实模型 Model: <精确模型 id> trailer。
禁止使用 --no-verify--no-gpg-sign、关闭签名配置、未经确认的 force push、hard reset、amend 或交互式 rebase。
禁止留下 auto-stash 产生的孤儿 WIP stash;自动 stash 流程完成后必须恢复自己的 stash。

Files:

  • scripts/bot-e2e.sh
  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plan-refactor-inventory-core-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • scripts/e2e-redis.sh
  • server/tests/dev_command_operator_gate.rs
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • server/src/cmd/dev/mod.rs
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
  • server/src/cmd/dev/season.rs
docs/**/*.md

📄 CodeRabbit inference engine (docs/CLAUDE.md)

docs/**/*.md: 新 plan 头部必须写明接入面:进料、出料、复用的共享类型/event/schema、server/agent/client 跨仓库契约,以及对应的 worldview.md 锚点。
涉及真元、灵气、衰减、逸散、半衰、距离损耗、排斥或吸力的 plan,必须调用 qi_physics;新物理常数和公式必须先扩展 qi_physics,不得在功能 plan 中重复实现。
所有真元/灵气流动必须遵守守恒律并通过 qi_physics::ledger::QiTransfer;释放使用 qi_release_to_zone,吸收使用 qi_excretion,不得凭空生成或销毁真元。
涉及玩家可感知行为的 plan,必须在对应机制阶段中内联可直接实现的粒子、音效、HUD、环境、动画和 narration 规格;不得将视听内容笼统推迟到独立阶段。纯 server 逻辑 plan 例外。
每份 plan 必须列出开放问题;实施前必须追加 §N.1 决议,逐项给出结论、实施方案、边界条件,并以文件:行号和 plan 章节双锚点落地。
scope 大于或等于 4 个 PR 的 plan,必须在末尾包含 §10 实施工作流,并按依赖顺序在一个 plan 内序列化多个 PR,不得拆成多个 plan。
涉及 NBT 建筑、worldgen layout 或复杂视觉资产的 TODO,必须完成三轮提交:(round 1/3)(round 2/3)(round 3/3);终轮提交必须包含拼写准确的 <PROMISE> 担保块。
§10 最末必须包含“单次 consume-plan 全自动到 merge”章节,明确用户提交 /consume-plan 后即可等待最终归档至 docs/finished_plans/

引用世界观内容时统一使用 worldview.md §X L<line> 格式;不得自动修改 docs/worldview.md 或主动回写 docs/library/

Files:

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plan-refactor-inventory-core-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
docs/plans-skeleton/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

新 plan 必须先读取 docs/CLAUDE.md;骨架、Active 和 Finished plan 必须遵循三态流转及规定的阶段状态、Finish Evidence 结构。

Files:

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
server/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

server/**/*.rs: 所有真元/灵气流动必须通过 qi_physics::ledger::QiTransfer;不得直接修改玩家或区域真元,必须保持 SPIRIT_QI_TOTAL 守恒。
不得自行定义真元衰减、抽取或半衰期常数;必须复用或先扩展 qi_physics::constants
离屏战死必须调用 release_dormant_qi_to_zone 并通过 ledger.transfer(ReleaseToZone) 返还真元;不得直接删除带有真元的快照。
使用境界名称 醒灵 → 引气 → 凝脉 → 固元 → 通灵 → 化虚,严禁使用旧称练气、筑基、金丹、元婴。
新 zone 必须先核对 docs/worldview.md 区域表和 server/zones.json 中的既有 ID。
禁止使用 armor stand 或 invisible mob 作为碰撞箱或交互载体;实体必须采用 Marker + 自定义渲染,交互通过 C2S 请求处理。

Files:

  • server/tests/dev_command_operator_gate.rs
  • server/src/cmd/dev/mod.rs
  • server/src/cmd/dev/season.rs
**/*.{rs,ts,tsx,java,py}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rs,ts,tsx,java,py}: 每个新增函数、组件或协议必须测试 happy path、边界、错误分支和全部状态转换;测试应断言外部可观察行为而非内部实现。
schema、enum 和状态机必须有专属 pin 测试;TypeBox/serde variant 要有正反 sample 对拍,每个 enum variant 和状态转换至少有一条用例。

Files:

  • server/tests/dev_command_operator_gate.rs
  • server/src/cmd/dev/mod.rs
  • server/src/cmd/dev/season.rs
🧠 Learnings (3)
📚 Learning: 2026-07-17T00:31:10.779Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:1-1
Timestamp: 2026-07-17T00:31:10.779Z
Learning: 在审查该仓库 `docs/plans-skeleton/` 下的“docs-only skeleton plan”创建类 PR 时:先核对 `docs/CLAUDE.md` 中“Plan 消费规范”,并逐份查看本计划文档里的“§10 实施工作流”,确认后续实施阶段是否会遵循“每个 PR 只修改一个 plan”,且实施/归档时不会出现跨 plan 的修改。该规则不适用于用户显式指定、共享调研基线且计划之间存在互相交叉引用的 skeleton plan 同批创建 PR;对这类情况应按实际交叉引用关系放宽,确保仍能按独立或约定的序列化方式推进。

Applied to files:

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-17T00:31:13.643Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:81-85
Timestamp: 2026-07-17T00:31:13.643Z
Learning: 审核 docs/plans-skeleton/*.md 下的 skeleton 草案 PR 时:不得要求作者在“§N 开放问题(P0 决策门前需收口)”核查完成之前提前填写对应的“§N.1 决议”。仅当计划进入 active 且 P0 实施前,已由 Explore agent 并行核查代码现状后,才允许追加“§N.1 决议”,且该决议需包含结论、实施方案、边界条件,并使用“文件:行号 + plan 章节”的双锚点格式。

Applied to files:

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-22T02:11:59.191Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1247
File: docs/plans-skeleton/plan-bughunt-animal-air-spawn-gravity-v1.md:0-0
Timestamp: 2026-07-22T02:11:59.191Z
Learning: 在 `docs/plans-skeleton/` 目录下的计划状态行中,若起草日期同时涉及 UTC 与本地日期(例如时区换算后可能跨到不同日期),请在该行中同时标注 UTC 起草日与本地起草日。这样可以避免基于 UTC 的时间基准在 GitHub/CodeRabbit 审查时将本地日期误判为“未来日期”。

Applied to files:

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
🪛 LanguageTool
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md

[grammar] ~129-~129: Ensure spelling is correct
Context: ...ce issue 清单与唯一聚类 ### 4.1 publish-error-boundary(5 个 source ID;其中 4 个为 #1470 重复证据) `#1470...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~141-~141: Ensure spelling is correct
Context: ... P4 raw-ID 修复范围。 ### 4.3 agent-publish-idempotency(8 个 confirmed source ID) #1616 #1619 #1`...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~190-~190: Ensure spelling is correct
Context: ...batch/event id,但这不等于当前 Pub/Sub receiver 已实现可见副作用去重。 2. 以 ordering_key 对同一 player/entity/z...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~191-~191: Ensure spelling is correct
Context: ...yer/entity/zone 的事件串行化;不同 key 可 bounded 并发,设置全局并发上限和队列上限。 3. 将 feature cooldown、lastStatusByFaction、...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~309-~309: Ensure spelling is correct
Context: ...follow-up 由各自 owner 提供独立证据。 ## 8. Open questions(P0 内收口;P0 退出前冻结依赖决策) 本节不构成 P0 entry block...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🔇 Additional comments (12)
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md (1)

3-50: LGTM!

Also applies to: 61-308, 340-368

docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md (1)

15-19: LGTM!

Also applies to: 38-38

docs/plans-skeleton/plan-refactor-master-v1.md (1)

9-10: LGTM!

Also applies to: 99-99

docs/plans-skeleton/plan-refactor-persistence-slices-v1.md (1)

59-59: LGTM!

docs/plans-skeleton/plan-refactor-wire-s2c-v1.md (1)

17-18: LGTM!

Also applies to: 38-40

scripts/bot-e2e.sh (1)

316-317: LGTM!

server/src/cmd/dev/mod.rs (4)

116-127: LGTM!


78-101: 🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift

Authorization Bypass (CWE-1188): Insecure Default Initialization of Resource

NetworkSettings 缺失时授权 fail-open,需确认注册顺序。

第 81 行只在 Some(ConnectionMode::Offline) 时收紧信任。若 connection_modeNoneusernames_are_authenticated 变为 true

None 的来源是第 183-187 行:register_operator_gatedev::register(app) 执行的那一刻读取 NetworkSettings。Bevy 插件注册有顺序性。若生产 Appdev::register 之后才插入 NetworkSettings(例如网络插件后置),则即使服务器实际运行在 Offline 模式,客户端自报用户名也会被当作已认证身份。

结合第 96 行的默认 roster {"Admin", "admin"},后果是:任何人以用户名 Admin 连接 Offline 服务器即可获得 bong.dev 作用域,并执行 /gm/give/qi/realm 等全部开发命令。

集成测试 setup_appserver/tests/dev_command_operator_gate.rs 第 72-78 行)先插入 NetworkSettings 再调用 dev::register,因此该测试无法覆盖这条路径。

建议两项修复:把 None 视为不可信(fail-closed),并把连接模式读取推迟到 Startup/PostStartup 阶段。

🔒️ 建议的 fail-closed 修正
     fn from_connection_mode(connection_mode: Option<&ConnectionMode>) -> Self {
         let allow_offline = truthy_env_var("BONG_OPERATORS_ALLOW_OFFLINE");
-        let usernames_are_authenticated =
-            !matches!(connection_mode, Some(ConnectionMode::Offline)) || allow_offline;
-        if matches!(connection_mode, Some(ConnectionMode::Offline)) && !allow_offline {
+        // 未知连接模式必须按不可信处理。
+        let mode_authenticates = matches!(
+            connection_mode,
+            Some(ConnectionMode::Online { .. }) | Some(ConnectionMode::BungeeCord) | Some(ConnectionMode::Velocity { .. })
+        );
+        let usernames_are_authenticated = mode_authenticates || allow_offline;
+        if !mode_authenticates && !allow_offline {
             tracing::warn!(
-                "BONG_OPERATORS ignored in offline mode; set BONG_OPERATORS_ALLOW_OFFLINE=1 to explicitly trust client-provided usernames"
+                ?connection_mode,
+                "BONG_OPERATORS ignored: connection mode does not authenticate usernames; set BONG_OPERATORS_ALLOW_OFFLINE=1 to explicitly trust client-provided usernames"
             );
         }
#!/bin/bash
# 目的:确认生产 App 中 NetworkSettings 与 dev::register 的注册顺序。
set -uo pipefail

echo "=== NetworkSettings 插入点 ==="
rg -n -C10 'NetworkSettings' server/src --glob '!**/tests/**'

echo "=== dev::register 调用点 ==="
rg -n -C10 'dev::register|cmd::register|cmd::dev::register' server/src

echo "=== main / app 构建 ==="
fd -e rs 'main.rs|lib.rs' server/src --max-depth 2 --exec ast-grep outline {} --items all

199-229: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Authorization Bypass (CWE-862): Missing Authorization

Reachability: Internal

一次性初始化对后续新增命令根 fail-open。

Local<bool> 让该系统只在第一个 EventLoopPreUpdate 帧扫描一次命令图。任何在该帧之后进入 CommandRegistry 的命令根不会被打上 bong.dev 作用域,因此对所有玩家可见且可达。

同时第 225 行用 vec![DEV_COMMAND_SCOPE] 覆盖节点已有的 scopes。若某个命令自行声明了更严格的作用域,该声明会被静默丢弃。建议改为追加去重,保留原有声明。

请确认本仓库中没有在 App 构建期之后调用 AddCommand 的路径。

♻️ 建议:作用域追加而非覆盖
         dev_roots.0.insert(name.clone());
-        command_registry.graph.graph[node].scopes = vec![DEV_COMMAND_SCOPE.to_string()];
+        let scopes = &mut command_registry.graph.graph[node].scopes;
+        if !scopes.iter().any(|scope| scope == DEV_COMMAND_SCOPE) {
+            scopes.push(DEV_COMMAND_SCOPE.to_string());
+        }
#!/bin/bash
# 目的:查找构建期之后的命令注册路径与既有 scope 声明。
set -uo pipefail

echo "=== add_command 调用点 ==="
rg -n -C4 '\.add_command::<' server/src

echo "=== 既有 scope 声明 ==="
rg -n -C4 'scopes\s*=|#\[scopes' server/src

231-242: 🩺 Stability & Availability

无需修改。

CommandScopes 的现有生成路径与 Username 同帧存在,Added<CommandScopes> 不会在该代码路径中产生后续遗漏。

server/tests/dev_command_operator_gate.rs (2)

442-484: LGTM!


34-68: 📐 Maintainability & Code Quality

不需要修改:当前 crate 使用 Rust 2021 edition。

server/Cargo.toml 声明 edition = "2021",因此 std::env::set_varstd::env::remove_var 不需要 unsafe;该变更仅在 2024 edition 生效。

"id": "p0-doc-scope",
"level": "major",
"text": "For pull requests whose changes are limited to docs/ plan documents (plan-* and plans-skeleton design-closure PRs), limit review scope to internal consistency, factual accuracy of code citations, alignment with plan-refactor-master-v1 scope and cross-track dependencies, and satisfaction of the acceptance criteria stated in the PR description or the document itself. Depth expansions beyond those criteria - new scenarios, extra edge-case enumerations, additional contract surfaces - are suggestions for the open-questions section, not major or blocker findings; an explicit open-questions entry with rationale is a valid resolution for a deferred design decision."
"text": "For pull requests whose changes are limited to docs/ plan documents (plan-* and plans-skeleton design-closure PRs), limit review scope to internal consistency, factual accuracy of code citations, alignment with plan-refactor-master-v1 scope and adjudications, cross-track dependencies, and satisfaction of the acceptance criteria stated in the PR description or the document itself. Findings addressed in a prior review round of the same branch, regardless of the pull-request number carrying that branch, must not be re-raised as major findings unless a later change newly contradicts or invalidates the resolution. Completeness expansions beyond the plan's stated acceptance criteria and evidence base - including demands for new scenarios, additional test rows, extra edge-case enumerations, protocol corners, or contract surfaces - are suggestions for the open-questions section, not major or blocker findings; an explicit open-questions entry with rationale is a valid resolution for a deferred design decision. Major findings in this scope are reserved for internal contradictions, conflicts with plan-refactor-master-v1 or binding adjudications, and specifications that a competent implementer could not execute as written."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

先同步更新 p0-doc-scope 的精确契约测试。

Line 133 修改了策略文本,但 .github/scripts/review-consumer.test.mjs 仍使用 assert.equal 校验旧文本。当前变更会导致该测试确定性失败。若新文本是预期行为,请在同一变更中更新测试期望值;否则请恢复旧文本。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/review-policy/bong.v2.json at line 133, Update the exact contract
assertion for the p0-doc-scope policy in
.github/scripts/review-consumer.test.mjs to match the revised text at the policy
entry, preserving the existing assert.equal-based validation and all unrelated
expectations.

Comment thread docs/plan-refactor-inventory-core-v1.md Outdated
- ✅ 2026-08-03 P0 完整契约面重写 + absorption audit
- ⬜ P1:inventory 拆分 + txn/capacity 骨架 + inventory-layout/dropped-loot 纯 migration helpers(依赖 R3 P1 seam)
- ⬜ P2:production writer 迁移分为 P2a metadata/provider + Public writer path 与 P2b OwnerOnly private-writer activation;P2a 依赖 R3 P2 atomic commit seam 与旧 dropped-loot compatibility,P2b 必须等 R3 P4 dropped-loot migration/hydration、R6 P1 recipient projection/page、R5 P3 + R6 P4、R10 P3 pickup txn 及 R4 pickup consumer 全部完成后才可启用。
- ⬜ P2:production writer 迁移分为 P2a metadata/provider + Public writer path 与 P2b OwnerOnly private-writer activation;P2a 依赖 R3 P2 atomic commit seam 与旧 dropped-loot compatibility,P2b 必须等 R3 P4 dropped-loot migration/hydration、R6 P3 recipient projection/page production activation、R5 P3 + R6 P4、R10 P3 pickup txn 及 R4 pickup consumer 全部完成后才可启用。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

为所有 scope ≥ 4 PR 的计划补齐 §10 实施工作流。

这些计划都定义了至少 4 个实施阶段,但没有在计划末尾提供统一的 §10 实施工作流。Master plan 的 §5 工作流§10 flash-review issue 消化流程 不能替代规定的实施工作流。

请在每份计划末尾补齐:

  • 前置依赖和串行 PR 顺序;

  • 每 PR 的独立 subagent、测试和 review gate;

  • atomic commit、中文 commit、精确 Model trailer 和禁止命令;

  • 最后一节标题“单次 consume-plan 全自动到 merge”。

  • docs/plan-refactor-inventory-core-v1.md#L9-L9:为 R10 P0-P4 补充 §10。

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md#L23-L23:为 R4 P0-P4 补充 §10。

  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md#L22-L23:为 R9 P0-P4 补充 §10。

  • docs/plans-skeleton/plan-refactor-master-v1.md#L44-L80:为总纲重排 §10,保留 flash-review 清算职责。

  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md#L28-L31:为 R3 P0-P5 补充 §10。

  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md#L21-L25:为 R6 P0-P5 补充 §10。

As per coding guidelines:scope 大于或等于 4 个 PR 的 plan 必须包含 §10 实施工作流,并以“单次 consume-plan 全自动到 merge”收尾。

📍 Affects 6 files
  • docs/plan-refactor-inventory-core-v1.md#L9-L9 (this comment)
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md#L23-L23
  • docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md#L22-L23
  • docs/plans-skeleton/plan-refactor-master-v1.md#L44-L80
  • docs/plans-skeleton/plan-refactor-persistence-slices-v1.md#L28-L31
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md#L21-L25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plan-refactor-inventory-core-v1.md` at line 9, 为所有 scope ≥4 PR
的计划补齐或重排统一的“§10 实施工作流”:在 docs/plan-refactor-inventory-core-v1.md:9 为 R10
P0-P4、docs/plans-skeleton/plan-refactor-c2s-gate-v1.md:23 为 R4
P0-P4、docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md:22-23 为 R9
P0-P4、docs/plans-skeleton/plan-refactor-persistence-slices-v1.md:28-31 为 R3
P0-P5、docs/plans-skeleton/plan-refactor-wire-s2c-v1.md:21-25 为 R6 P0-P5
添加前置依赖与串行 PR 顺序、每 PR 独立 subagent、测试和 review gate、atomic commit、中文 commit、精确
Model trailer 及禁止命令;在 docs/plans-skeleton/plan-refactor-master-v1.md:44-80 重排
§10,同时保留 flash-review 清算职责。所有计划的最后一节标题必须是“单次 consume-plan 全自动到 merge”。

Source: Coding guidelines

Comment thread docs/plans-skeleton/plan-refactor-master-v1.md Outdated
Comment thread scripts/bot-e2e.sh
Comment on lines +26 to +43
BOT_E2E_OPERATOR_TAGS=(
RGA RGB Clr Fog Give Atk RespawnSfx Cast SwordAV Sword Break Pill Cult Box Herbal Eqp ScDim
MCA MCB CE1 CE2 Req Scope Tol AmbSur Brew ProdAF Refund Resume Forge Craft ProdLG WoodDrop J1 Poi
)
BOT_E2E_OPERATORS=""
for bot_tag in "${BOT_E2E_OPERATOR_TAGS[@]}"; do
[ -z "$BOT_E2E_OPERATORS" ] || BOT_E2E_OPERATORS+=,
BOT_E2E_OPERATORS+="B${BOT_E2E_RUN_TAG}${bot_tag}"
done
# Reuse is safe only when the caller proves that the running offline server has this exact
# run-tag roster and the explicit username-trust opt-in. Otherwise force the fresh-launch path.
if [ "$REUSE" = "1" ] && [ "$AMBIENT_FIXTURE_MODE" != "1" ] && {
[ "${BONG_OPERATORS:-}" != "$BOT_E2E_OPERATORS" ] ||
[ "${BONG_OPERATORS_ALLOW_OFFLINE:-}" != "1" ]
}; then
echo "[bot-e2e] existing server operator roster does not match this run; disabling BOT_E2E_REUSE=1" >&2
REUSE=0
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

operator 用户名缺少单一事实来源。 两个脚本都在 shell 侧手工构造 operator 用户名,而实际 bot 用户名由 scripts/bot/run_scenarios.py 的场景定义独立构造。两侧没有共享来源,也没有一致性断言。任一侧改名或新增场景 bot,roster 就会漏掉该 bot,dev 命令被新的 operator 门控拒绝,故障表现只是场景断言失败,难以定位。

  • scripts/bot-e2e.sh#L26-L43BOT_E2E_OPERATOR_TAGS 硬编码 34 个后缀。请从场景定义导出该列表,或在生成 roster 后断言它覆盖本轮将要连接的全部 bot 用户名。
  • scripts/e2e-redis.sh#L1239-L1249NRift 后缀在此处硬编码,而第 1311 行只把 NORTH_RIFT_RUN_TAG 传给场景。请让场景侧输出其 bot 用户名,或在脚本中复用同一常量定义。
📍 Affects 2 files
  • scripts/bot-e2e.sh#L26-L43 (this comment)
  • scripts/e2e-redis.sh#L1239-L1249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/bot-e2e.sh` around lines 26 - 43, Remove the independent hard-coded
operator roster in scripts/bot-e2e.sh lines 26-43 by deriving the complete list
from the scenario definitions, or assert after roster generation that every bot
username used in this run is covered. In scripts/e2e-redis.sh lines 1239-1249,
eliminate the hard-coded NRift suffix by consuming the scenario-produced bot
username or the same shared constant; preserve the existing run-tag flow at line
1311.

Comment thread server/src/cmd/dev/mod.rs
Comment on lines +244 to +263
fn gate_dev_commands(
mut events: EventReader<CommandExecutionEvent>,
dev_roots: Res<DevCommandRoots>,
permissions: Res<DevCommandPermissions>,
usernames: Query<&Username>,
mut clients: Query<&mut Client>,
) {
for event in events.read() {
let command_root = event.command.split_whitespace().next().unwrap_or_default();
if !dev_roots.0.contains(command_root)
|| usernames
.get(event.executor)
.is_ok_and(|username| permissions.is_operator(username.0.as_str()))
{
continue;
}
if let Ok(mut client) = clients.get_mut(event.executor) {
client.send_chat_message("[dev] Command requires operator permission.");
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 目的:确认 CommandExecutionEvent.command 是否包含前导斜杠。
set -uo pipefail

fd -t f 'manager.rs' -p 'command' --exec rg -n -C10 'CommandExecutionEvent' {}
rg -n -C10 'CommandExecutionEvent\s*\{' ~/.cargo/registry 2>/dev/null | head -60
rg -n -C6 'ChatCommandC2s' server/src

Repository: Kizunad/Bong

Length of output: 245


🏁 Script executed:

#!/bin/bash
# 定位且检查与 dev 命令 gate、CommandExecutionEvent、相关测试的源代码上下文。
set -euo pipefail

echo "== files containing dev/mod.rs =="
fd -t f 'mod.rs' server/src/cmd/dev 2>/dev/null || true

echo "== repository occurrences of gate_dev_commands/event.command/CommandExecutionEvent =="
rg -n -C 5 'gate_dev_commands|CommandExecutionEvent|is_operator|dev_roots|CommandExecutionEvent\s*\{' server -g '*.rs' || true

echo "== relevant dev/mod.rs sections =="
if [ -f server/src/cmd/dev/mod.rs ]; then
  wc -l server/src/cmd/dev/mod.rs
  sed -n '220,280p' server/src/cmd/dev/mod.rs
  sed -n '1,180p' server/src/cmd/dev/mod.rs
fi

echo "== command execution test command snippets =="
sed -n '100,150p' server/src/cmd/dev/mod.rs || true

Repository: Kizunad/Bong

Length of output: 19291


🌐 Web query:

valence-rs CommandExecutionEvent command field manager command execution source

💡 Result:

Based on an examination of the Valence Rust framework (valence-rs) codebase, documentation, and public APIs, there is no entity or event named CommandExecutionEvent that contains fields like command, manager, or command execution source [1][2][3][4][5][6]. The valence-rs project utilizes Bevy ECS for event handling [1][7]. While it defines various events related to player and client actions (such as SneakEvent, DiggingEvent, InteractEntityEvent, and JumpWithHorseEvent) [2][3], the framework does not currently implement a generalized CommandExecutionEvent with the specified structure [2][3]. It is highly likely that the term CommandExecutionEvent is being conflated with other technologies: - A CommandExecutionEvent interface exists in the Microsoft VS Code API (for monitoring command execution) [8]. - An CommandExecutionEvent class exists within the AWS IoT Device SDK for Java [9]. - A custom CommandExecutionEvent class is documented in unrelated projects such as the Panther Project's execution environment [10]. If you are working with Valence, you may be looking for how to handle chat commands or protocol-level interactions, which are typically managed through different mechanisms in the framework [1][4][11].

Citations:


🏁 Script executed:

#!/bin/bash
# 检查 Cargo.lock / Cargo.toml 里的 valence 版本,并结合 crate metadata 解析 CommandExecutionEvent.command 的真实来源。
set -euo pipefail

echo "== valence versions in cargo manifests =="
for f in Cargo.toml server/Cargo.toml; do
  [ -f "$f" ] && { echo "--- $f"; rg -n 'valence|command' "$f" || true; }
done

echo "== lockfile valence entries =="
if [ -f Cargo.lock ]; then
  rg -n -C 3 'name = "valence"|name = "valence-|version = ' Cargo.lock | rg -C 3 'name = "valence' || true
fi

echo "== local Cargo registry src for CommandExecutionEvent =="
python3 - <<'PY'
from pathlib import Path
import re
for root in [Path.home()/'.cargo/registry/src']:
    if not root.exists():
        continue
    for p in root.rglob('*.rs'):
        text=p.read_text(errors='ignore')
        if 'CommandExecutionEvent' in text:
            print(f'--- {p}')
            idx=text.find('pub struct CommandExecutionEvent')
            if idx==-1: idx=text.find('struct CommandExecutionEvent')
            if idx==-1: idx=text.find('enum CommandExecutionEvent')
            if idx!=-1:
                end=text.find('\n}', idx)
                print(text[idx:end+2])
            else:
                print(text[text.find('CommandExecutionEvent'):text.find('CommandExecutionEvent')+800])
PY

echo "== source test non-operator command with leading slash if any =="
sed -n '430,485p' server/tests/dev_command_operator_gate.rs || true

echo "== deterministic matching behavior for gate extract =="
python3 - <<'PY'
samples = ["qi set 40", "/qi set 40", " qi set ", " qi set 40", ""]
dev_roots = {"qi"}
for command in samples:
    root = command.split(None, 1)[0].split()[0] if command.split() else ""
    # Rust split_whitespace().next() is equivalent for these sample strings
    print(f'{command!r} -> {root!r} included={root in dev_roots}')
PY

Repository: Kizunad/Bong

Length of output: 3192


🏁 Script executed:

#!/bin/bash
# 通过 git 源直接读取 valence 2b705351 中的 CommandExecutionEvent.command,避免本地本地 registry 缺失或版本问题。
set -euo pipefail

tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT

git clone --depth 1 --branch main --single-branch "https://github.com/valence-rs/valence" "$tmp/valence" >/dev/null || true
cd "$tmp/valence"
git checkout "2b705351" >/dev/null || true

echo "== Checkout status =="
git status --short
git rev-parse HEAD

echo "== Find and inspect CommandExecutionEvent in source =="
rg -n -C 15 'pub struct CommandExecutionEvent|struct CommandExecutionEvent|CommandExecutionEvent' . || true

Repository: Kizunad/Bong

Length of output: 12493


command 与校验逻辑解耦并处理前导斜杠

  1. gate_dev_commands 只发提示,不负责消费或拒绝执行;当前非 operator 执行仍由 valence_command 的 scope 检查拦截。若要把授权判定整合到同一处失败分支,需要在读取 CommandExecutionEvent 后直接消费或设置拒绝标记。

  2. 客户端命令通过 CommandExecutionC2s.command 进入 CommandExecutionEvent.command,可能包含前导 /。当前 split_whitespace().next() 会把 /qi 识别为根名,导致 dev_roots 匹配失败;应剥离前导空白和前导 / 后再取根名。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/src/cmd/dev/mod.rs` around lines 244 - 263, 更新 gate_dev_commands
中的命令根解析逻辑,先去除前导空白和前导“/”再提取根命令,以确保 CommandExecutionEvent.command 中的“/qi”等形式能匹配
DevCommandRoots。保持该函数仅发送权限提示,不在此处消费事件或额外拒绝命令,继续沿用现有的 operator 校验与执行拦截流程。

Comment on lines +89 to +93
fn online_mode() -> ConnectionMode {
ConnectionMode::Online {
prevent_proxy_connections: true,
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

测试未覆盖 ConnectionMode 的全部 variant,也未覆盖 NetworkSettings 缺失场景。

DevCommandPermissions::from_connection_modeConnectionMode 做匹配判定。当前测试只覆盖 OnlineOfflineBungeeCordVelocity 这两个 variant 没有用例,而它们同样不由服务器自身认证用户名,判定结果直接影响授权。

同时,setup_app 总是先插入 NetworkSettings,因此 from_connection_mode(None) 这条分支完全没有测试。这正是 server/src/cmd/dev/mod.rs 第 81 行 fail-open 的路径。

请补齐这两类用例。

依据编码规范:“schema、enum 和状态机必须有专属 pin 测试;……每个 enum variant 和状态转换至少有一条用例。”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/tests/dev_command_operator_gate.rs` around lines 89 - 93, 补充
DevCommandPermissions::from_connection_mode 的专属测试,覆盖 ConnectionMode 的 BungeeCord
和 Velocity variant,并断言其授权结果符合 Online/Offline 的预期。调整或新增 setup_app 测试场景,使
NetworkSettings 缺失时传入 None,覆盖 from_connection_mode(None) 的 fail-open
分支并断言结果。保持现有 Online、Offline 用例不变,确保每个 ConnectionMode variant 及缺失配置路径都有测试。

Source: Coding guidelines

Comment on lines +382 to +390
let mut expected_roots = registry_pin::COMMAND_NAMES
.iter()
.copied()
.collect::<BTreeSet<_>>();
expected_roots.insert("identity");
assert_eq!(
actual_roots, expected_roots,
"gate test must cover every root-level literal registered in production"
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 目的:核对 registry_pin::COMMAND_NAMES 是否遗漏 identity。
set -uo pipefail

rg -n -C20 'COMMAND_NAMES' server/src/cmd/registry_pin.rs
rg -n -C4 'IdentityCmd' server/src

Repository: Kizunad/Bong

Length of output: 24231


🏁 Script executed:

#!/bin/bash
set -uo pipefail

echo "== candidate files =="
fd -a 'dev_command_operator_gate\.rs|registry_pin\.rs|identity/command\.rs' server/src server/tests

echo
echo "== gate test context =="
sed -n '340,400p' server/tests/dev_command_operator_gate.rs

echo
echo "== full registry_pin COMMAND_NAMES in production const =="
sed -n '1,155p' server/src/cmd/registry_pin.rs

echo
echo "== identity command registration =="
sed -n '46,106p' server/src/identity/command.rs

echo
echo "== all occurrences of identity literal or register =="
rg -n 'literal\("identity"|add_command::<IdentityCmd>|identity::command|register\(.*identity' server/src server/tests

Repository: Kizunad/Bong

Length of output: 9431


🏁 Script executed:

#!/bin/bash
set -uo pipefail

echo "== pin tests =="
sed -n '1,280p' server/tests/dev_command_operator_gate.rs

echo
echo "== command pin references in tests =="
rg -n -C2 'COMMAND_(NAME|TREE|PATH)|pin|identity' server/tests/dev_command_operator_gate.rs server/src/cmd/registry_pin.rs

echo
echo "== deterministic probe: identity present in production pin vs runtime root check =="
python3 - <<'PY'
from pathlib import Path
import re

const_text = Path('server/src/cmd/registry_pin.rs').read_text()
test_text = Path('server/tests/dev_command_operator_gate.rs').read_text()

# Use a simple extraction that stays valid for the shown source shape.
m = re.search(r'pub const COMMAND_NAMES:\s*&\[&str\]\s*=\s*&\[(?P<body>.*?)\];', const_text, re.S)
if not m:
    raise SystemExit('cannot parse COMMAND_NAMES')
body = m.group('body')
pin_names = [s.strip().strip('"\'') for s in body.split(',') if s.strip() and s.strip().startswith('"') and s.strip().endswith('"')]
print('COMMAND_NAMES_count:', len(pin_names))
print('COMMAND_NAMES_contains_identity:', 'identity' in pin_names)
print('COMMAND_NAMES_first_few:', pin_names[:3])
print('identity adjacent pins:', [pin for i, pin in enumerate(pin_names) if 'identity' in pin_low(pin) or pin_low(pin_low(pin_names[i-1]) in ['zones'] and pin_low('zones') in pin)] for _ in [])

def pin_low(s):
    return s.lower()

test = (
    'expected_roots = registry_pin::COMMAND_NAMES\n' in test_text
    and 'expected_roots.insert("identity");' in test_text
)
print('test manually_inserts_identity:', test)
PY

Repository: Kizunad/Bong

Length of output: 14276


identity 补进 registry_pin::COMMAND_NAMES,并删除 gate 测试中的硬编码补丁。

/identity 已由 IdentityCmd::assemble_graph 注册为根级命令,但 server/src/cmd/registry_pin.rs 和生产命令树 pin 测试中没有 identity,导致当前测试在第 386 行手动插入后再去检查 runtime root。这会让 root 遗漏规避 pin 测试。将 identity 加入 registry_pin::COMMAND_NAMESCOMMAND_TREE_PATHS 后,expected_roots.insert("identity") 不应再需要。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/tests/dev_command_operator_gate.rs` around lines 382 - 390, 将生产注册清单中的
identity 补入 registry_pin::COMMAND_NAMES 和 COMMAND_TREE_PATHS,使其与
IdentityCmd::assemble_graph 注册的根级命令一致;随后在生产命令树 pin 测试中移除
expected_roots.insert("identity") 硬编码补丁,并保留基于 COMMAND_NAMES 生成 expected_roots
的校验逻辑。

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 29bce3a97d984a2bbe40dcb3e6bd22e3f1f02dbb
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[minor] Validate BOT_E2E_RUN_TAG before generating usernames and the operator roster

scripts/bot-e2e.sh:23 · strict-maintainability

A fresh launch always constructs every bot username as B${BOT_E2E_RUN_TAG}${tag} and passes the same unvalidated value into BONG_OPERATORS at scripts/bot-e2e.sh:315. The runner only rejects an overlong resulting username at scripts/bot/run_scenarios.py:39-43; it does not validate the tag's character set. For example, BOT_E2E_RUN_TAG=abc, produces operator configuration entries containing a comma and bot usernames such as Babc,Give, which are not valid Minecraft usernames and also alter the comma-separated allowlist. The server can therefore start with a roster that cannot authenticate the bots, causing the harness's normal scenario path to fail instead of rejecting the invalid run tag before startup.

Root cause: the new run-tag override is interpolated directly into both minecraft usernames and the comma-separated operator allowlist without enforcing the runner's stated length constraint or a username-safe character grammar. this makes malformed or oversized caller-provided tags reach server startup and bot execution.

[major] Add regression coverage for exact operator-roster reuse gating

scripts/bot-e2e.sh:36 · testing

The changed reuse contract is implemented only in the shell branch at scripts/bot-e2e.sh:36-42: when BOT_E2E_REUSE=1, the harness must require the exact generated BONG_OPERATORS roster and BONG_OPERATORS_ALLOW_OFFLINE=1, otherwise it must force a fresh launch. The repository’s existing operator-gate tests exercise server-side parsing and authorization, but do not invoke or source this harness branch, and no shell test covers mismatched roster, missing opt-in, or the subsequent fresh-launch environment export at scripts/bot-e2e.sh:313-317. A concrete incorrect implementation that removes either equality check (or leaves REUSE=1 after a mismatch) would therefore pass the current tests while running scenarios against a stale offline server with the wrong trusted usernames, producing false authorization results and potentially testing the wrong server state. Add a focused harness-level test that asserts each mismatch disables reuse and that a fresh launch receives the exact generated roster and opt-in.

Root cause: the pull request changes the bot harness’s server identity and operator-trust boundary, but regression coverage exists only for the server consumer and not for the producer-side reuse decision and startup environment wiring.

[major] Plan misstates master-matrix registration evidence

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:37 · correctness

The new plan says the master matrix row for agent-narration-pipeline and the skeleton are in the same docs-only revision at docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:37. However, this diff adds only docs/plans-skeleton/plan-agent-narration-pipeline-v1.md for that plan; docs/plans-skeleton/plan-refactor-master-v1.md is unchanged in the immutable diff batch. The current master file already contains the agent row independently, so the claimed same-revision ownership evidence cannot be supplied by this pull request.

Root cause: the plan documents an atomic ownership/matrix-registration fact that is not true of the submitted change set. reviewers and implementers cannot use the stated commit as evidence that the master matrix registration and skeleton were introduced together.

[major] Plan cites missing mandatory workflow documents

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:33 · correctness

The new plan states that the root CLAUDE.md and docs/CLAUDE.md exist and were read, and makes that evidence a P0 deliverable at docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:33 and :238. Read-only repository enumeration finds no CLAUDE.md at /workspace and no docs/CLAUDE.md; the plan therefore cites prerequisite workflow documents that are absent, so an implementer cannot perform the mandated workflow verification as written.

Root cause: the skeleton records repository-process documents as present/read without those files existing in the repository, while later phases depend on their referenced rules. this makes the stated p0 evidence and the implementation workflow factually unusable.

[major] Include every command-using bot tag in the generated operator roster

scripts/bot-e2e.sh:27 · wiring

A fresh self-start run exports BONG_OPERATORS from the fixed roster at scripts/bot-e2e.sh:27-28, but that roster contains SwordAV and not the scenario's separate Sword username (scripts/bot/scenarios/combat_weapon_equip_damage.py:78). The scenario then sends /clearinv and /give (combat_weapon_equip_damage.py:84-85,117), while the server's non-public command roots require the bong.dev scope and operator authorization (server/src/cmd/dev/mod.rs:221-227,235-256). Therefore the normal fresh bot-e2e.sh path creates the BSword bot without operator permission and its required commands are rejected, making this scenario fail. The same roster also omits NRift, although the preview scenario sends /preview_tp (terrain_north_rift_scorch_zone_identity.py:211,219) and that command is registered as a dev command (server/src/cmd/dev/preview_tp.rs:32-66).

Root cause: the newly introduced operator allowlist is manually enumerated but is not synchronized with all usernames generated by the bot scenario producer. fresh servers authorize only the listed run-tagged usernames, so omitted scenario users cannot traverse the command authorization path required by their scenarios.

[minor] Default BOT_E2E_REUSE can no longer reuse the previous script-started server

scripts/bot-e2e.sh:36 · correctness

在已有 server 上按脚本顶部记录的用法运行 BOT_E2E_REUSE=1 bash scripts/bot-e2e.sh 时,脚本会在启动前用本次 shell 的 PID 生成新的 BOT_E2E_RUN_TAG(23-24),再在 36-42 行将 REUSE 强制改回 0,因为已运行 server 的 BONG_OPERATORS 不会等于这次新生成的 roster。随后 287-299 行看到端口已占用且 REUSE != 1,直接退出,而不是复用 server。自起 server 的子 shell 环境(313-316)也不会回写到下一次调用,因此普通的“第二次运行复用上一轮 server”没有办法从脚本自身获得所需的 roster/tag。

Root cause: 复用资格依赖精确的 run-tag operator roster,但默认 run tag 每次按新 shell pid 重新生成;脚本既没有持久化/读取上一轮 roster,也没有更新顶部的复用用法来要求调用者显式提供匹配的 bot_e2e_run_tagbong_operators 和离线信任开关。结果是原本记录的 bot_e2e_reuse=1 用法对脚本自启动的上一轮 server 失效。

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 60e156f3295ea0b757cd3484d6afd6742f518f79
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] P0 gate references undefined F04 and ambiguously gates F07

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:251 · correctness

The new P0 checklist requires the implementer to “收口 F06/F08/F09; F04/F07 ... 收口” (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:251), but the plan defines no F04 anywhere, and its P0 exit rule only identifies F06/F08/F09 as P0-owned decisions (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:320). F07 is separately introduced as a draft decision for post-ingest recovery (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:190), while durable-transport choices remain an open question (docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:322). Consequently, an implementer cannot determine which decision F04 denotes or whether F07 must be frozen before P1/P0 exit.

Root cause: the p0 acceptance checklist references an undefined decision identifier and assigns f07 a closure requirement that is not aligned with the plan's stated p0-owned decision set and open-question treatment. the phase gate therefore has no executable, unambiguous contract for when implementation may proceed.

[major] R4 P2 adds prerequisites forbidden by the master ordering contract

docs/plans-skeleton/plan-refactor-c2s-gate-v1.md:22 · schema-contracts

plan-refactor-c2s-gate-v1.md:22 makes the R4 P2 inventory pickup consumer wait for the merged R10 P3 pickup/merge transaction, R5 P3 attrition API, and R6 P4 receipt API. However, the same pull request changes plan-refactor-master-v1.md:52-56 to declare the Wave table the sole authority for inter-track ordering, and plan-refactor-master-v1.md:75-79 explicitly says a track plan may not add a start/order dependency absent from that table. The Wave table does not name these three phase-level prerequisites for R4 P2, so implementers receive contradictory ordering contracts and cannot determine whether the child-plan gate is permitted by the authoritative plan.

Root cause: the r4 child plan retains an explicit phase-level cross-track prerequisite while the concurrently changed master plan prohibits child plans from introducing cross-track dependencies outside the wave table. the dependency must be amended into the master authority or removed/rephrased as a non-gating contract-first/activation condition.

[major] Validate BOT_E2E_RUN_TAG before generating the operator roster

scripts/bot-e2e.sh:24 · correctness

A caller-provided BOT_E2E_RUN_TAG is interpolated directly into every generated username and into the comma-separated BONG_OPERATORS value at scripts/bot-e2e.sh:24-33, with no character or length validation. For example, BOT_E2E_RUN_TAG='x,y' produces the bot username Bx,yRGA but the operator roster is split by the server's comma parser into Bx and yRGA; tags containing whitespace or invalid characters likewise produce invalid Minecraft usernames. An overlong tag is only rejected later by run_scenarios.py, after the script has already created fixture/state resources and started or selected the server. The plan's required contract is fail-fast validation before roster, fixture, or server creation, and this changed harness path does not implement it.

Root cause: the new roster generation treats an external run tag as trusted text and duplicates it across two independently parsed protocol boundaries without validating minecraft-safe characters, length, or the resulting username for every scenario. this allows malformed identities to reach server startup and can make operator authorization and bot login disagree or fail late.

[major] Exercise public roots through the event-time authorization gate

server/tests/dev_command_operator_gate.rs:391 · testing

The new suite verifies that public roots have empty command-tree scopes at server/tests/dev_command_operator_gate.rs:391-400, but every_dev_root_enforces_event_time_operator_authorization only sends events for roots whose scopes equal bong.dev at server/tests/dev_command_operator_gate.rs:453-467. It never sends /ping, /bong, /faction, or /identity through the event-time gate and asserts that no rejection is emitted. A concrete regression such as removing the !dev_roots.0.contains(command_root) exemption in gate_dev_commands would therefore pass all of these tests while rejecting non-operators' public commands.

Root cause: the changed authorization contract has two independent surfaces, command-tree visibility and event-time enforcement, but the public-root tests cover only the former. the event-time gate could accidentally apply operator authorization to public roots without any current regression test detecting the user-visible permission break.

[major] Assert that rejected commands do not execute

server/tests/dev_command_operator_gate.rs:467 · correctness

every_dev_root_enforces_event_time_operator_authorization only checks that a non-operator receives a rejection chat message and that an operator does not receive that message. It never checks that the non-operator's command produces no CommandResultEvent, state mutation, or other handler side effect. The production gate is a separate EventLoopUpdate system that sends the rejection message; this test would still pass if the command handler executed normally after the rejection was emitted.

Root cause: the test treats the rejection notification as proof of authorization enforcement, but notification and command cancellation are separate behaviors. a regression that lets unauthorized commands execute while retaining the chat message would be accepted by this suite, leaving the claimed permission contract unprotected.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

♻️ Duplicate comments (4)
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md (4)

398-400: 📐 Maintainability & Code Quality | 🟡 Minor

使用规定的 §10 末节标题。

当前标题是 10.6 单次 consume-plan 收口。仓库规则要求 §10 最末包含精确标题“单次 consume-plan 全自动到 merge”。

请将标题改为 ### 10.6 单次 consume-plan 全自动到 merge,并保持该节为 §10 的最后一节。当前版本仍未解决历史评论指出的同一问题。

As per coding guidelines, §10 最末必须包含“单次 consume-plan 全自动到 merge”章节。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 398 -
400, 将 §10 末节标题从“10.6 单次 consume-plan 收口”更新为精确标题“10.6 单次 consume-plan 全自动到
merge”,并确保该章节继续作为 §10 的最后一节。

Source: Coding guidelines


341-345: 🩺 Stability & Availability | 🟠 Major

pending 增加独立的容量和生命周期策略。

Line 343 只定义了 dead_letter。Lines 257 和 345 同时要求 pending 有界、可恢复,并验证其上限,但文档没有定义 pendingmax_itemsmax_bytesmax_payload_bytesretention_ttl、cleanup owner 或 overflow policy。

请增加 pending policy row,并补充写入、恢复、过期清理和满载测试。当前版本仍未解决历史评论指出的同一问题。

建议补充的 policy row
 | `dead_letter` | finite `max_items` + `max_bytes`;每条记录有 `max_payload_bytes` | finite `retention_ttl`;由 agent Tiandao pipeline cleanup/archive job 到期清理或归档 | conservative default:reject new terminal record + emit `dead_letter_overflow`,不重新塞回 pending;若 P0 选择覆盖/暂停,必须记录取舍并保持有界 | 持续注入 malformed、publish-exhaustion、queue-overflow,验证 item/byte/TTL 上限及 overflow telemetry |
+| `pending` | finite `max_items` + `max_bytes`;每条记录有 `max_payload_bytes` | finite `retention_ttl`;明确 cleanup/archive owner | 明确 reject、backpressure 或 requeue 边界,并记录 overflow telemetry | 持续注入 pending 写入、恢复、过期和满载场景,验证 item/byte/TTL 上限 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 341 -
345, 为该容量与生命周期策略表补充独立的 pending policy row,明确 max_items、max_bytes、每条记录的
max_payload_bytes、retention_ttl、cleanup/archive owner,以及满载时的 overflow policy 和
telemetry 行为。同步完善 pending 的写入、恢复、过期清理和满载测试要求,并让验收覆盖这些边界及与 dead_letter 各自上限的断言。

341-345: 🔒 Security & Privacy | 🟠 Major

Sensitive Data Exposure (CWE-359)

Reachability: Internal

限制 pending payload 的隐私保留周期。

Line 181 定义 payload 为原始事件或 renderer 输入。Line 190 允许该数据写入持久 pending store。当前表格没有 pending 的 retention、cleanup 或归档规则。

如果 payload 包含 player identity、target 或 world context,pending store 可能在处理目的结束后继续保留这些数据。请确认 payload 的字段分类和存储访问边界,并为 pending 固定有限 retention_ttl 与 cleanup owner。

历史评论已指出 pending retention 缺失;当前版本仍未补齐该控制。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 341 -
345, 补充 pending store 的数据治理约束:明确 payload 字段分类及存储访问边界,并在 pending 的容量/生命周期规则中增加有限
retention_ttl、明确 cleanup 或归档 owner,以及到期处理方式。同步更新验收标准,验证 pending payload 按 TTL
被清理或归档,且不影响现有 pending 上限与 telemetry 断言。

385-392: 📐 Maintainability & Code Quality | 🟠 Major

把 agent commit 门禁改成可执行的仓库规则。

model: "opus" 不是精确模型 ID。Line 392 还允许把真实模型 ID 写入 commit trailer/PR body,但 PR body 不能替代 commit trailer。

同时补充以下强制规则:

  • 每个逻辑单元使用一个 atomic commit。
  • commit message 必须使用中文。
  • 每个 agent commit 必须包含 Model: <精确模型 id> trailer。
  • 禁止 --no-verify--no-gpg-sign、未经确认的 force push、hard reset、amend 和交互式 rebase。

当前版本仍未解决历史评论指出的同一问题。

As per coding guidelines, commit trailer 是强制控制,PR body 只能作为补充记录。

建议的修订
 Agent(
   subagent_type: "claude",
-  model: "opus",
+  model: "<调用前由当前路由解析的精确模型 ID>",
   prompt: "<当前 PR 范围、依赖、测试与禁止项>\n\nultrathink"
 )
+ - 每个逻辑单元使用一个 atomic commit,commit message 必须使用中文。
+ - 每个 agent commit 必须包含 `Model: <精确模型 id>` trailer;PR body 只能补充记录。
+ - 禁止 `--no-verify`、`--no-gpg-sign`、未经确认的 force push、hard reset、amend 和交互式 rebase。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md` around lines 385 -
392, 修订 Agent 执行与提交门禁规则:将 Agent 配置中的 "opus" 替换为当时路由记录的精确模型 ID;要求每个逻辑单元使用一个
atomic commit、提交信息必须为中文,且每个 agent commit 必须包含 Model: <精确模型 id> trailer,真实模型 ID
不得仅记录在 PR body。明确禁止 --no-verify、--no-gpg-sign、未经确认的 force push、hard reset、amend
和交互式 rebase,并保留 agent 仅实现、测试、push、创建 PR 后返回 URL、HEAD 与门禁证据的职责范围。

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/plans-skeleton/plan-refactor-c2s-gate-v1.md`:
- Line 23: 在 P1/P2 的相关计划条目中补充可直接实现的拒绝回执体验规格:定义 R10 reason code 到 client
toast/HUD 的映射、优先级与未知原因的 fallback,并明确适用的音效、动画、环境、粒子和 narration 行为;同时保留
accepted/rejected outcome 通过 R6 emit API 传递,避免仅描述数据交付或将这些玩家可感知行为推迟到后续阶段。
- Line 23: 在该计划文档末尾补充“§10 实施工作流”,按依赖顺序定义至少 4 个 PR 的串行拆分边界、每个 PR
的范围与验证条件,并明确禁止未满足前置条件时提前合并。新增“单次 consume-plan 全自动到
merge”章节,描述从消费计划、自动执行与验证到合并的完整流程,同时保持现有 P0–P4、吸收清单和开放问题内容不变。
- Line 23: Update the owner plan for R6 to define the frozen API, phase, and
schema for the R6 emit contract, including request ID, revision, authoritative
item view, and rejected-reason fields for InventoryEventV1::Moved and
inventory_receive_rejected. Then synchronize this phase’s references to those
exact definitions, keeping the current change limited to declared, unwired,
test-only contract/pin work and preventing production traffic activation until
the R6 production seam is ready.

---

Duplicate comments:
In `@docs/plans-skeleton/plan-agent-narration-pipeline-v1.md`:
- Around line 398-400: 将 §10 末节标题从“10.6 单次 consume-plan 收口”更新为精确标题“10.6 单次
consume-plan 全自动到 merge”,并确保该章节继续作为 §10 的最后一节。
- Around line 341-345: 为该容量与生命周期策略表补充独立的 pending policy row,明确
max_items、max_bytes、每条记录的 max_payload_bytes、retention_ttl、cleanup/archive
owner,以及满载时的 overflow policy 和 telemetry 行为。同步完善 pending
的写入、恢复、过期清理和满载测试要求,并让验收覆盖这些边界及与 dead_letter 各自上限的断言。
- Around line 341-345: 补充 pending store 的数据治理约束:明确 payload 字段分类及存储访问边界,并在
pending 的容量/生命周期规则中增加有限 retention_ttl、明确 cleanup 或归档 owner,以及到期处理方式。同步更新验收标准,验证
pending payload 按 TTL 被清理或归档,且不影响现有 pending 上限与 telemetry 断言。
- Around line 385-392: 修订 Agent 执行与提交门禁规则:将 Agent 配置中的 "opus" 替换为当时路由记录的精确模型
ID;要求每个逻辑单元使用一个 atomic commit、提交信息必须为中文,且每个 agent commit 必须包含 Model: <精确模型 id>
trailer,真实模型 ID 不得仅记录在 PR body。明确禁止 --no-verify、--no-gpg-sign、未经确认的 force
push、hard reset、amend 和交互式 rebase,并保留 agent 仅实现、测试、push、创建 PR 后返回 URL、HEAD
与门禁证据的职责范围。
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8ea7db5b-380b-49fd-bc4a-5277c8f11967

📥 Commits

Reviewing files that changed from the base of the PR and between 60e156f and 482ebcf.

📒 Files selected for processing (2)
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: e2e
⚠️ CI failures not shown inline (2)

GitHub Actions: Review consumer tests / test: skeleton: agent 叙事管道计划 v1(§10.1.1 促升)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m[[ "$(git -C _central-contract rev-parse HEAD)" == '2986b95388b3ac3f8bf28b8ebdd60a19d2e4364c' ]]�[0m
 �[36;1m[[ "$(git -C 9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 rev-parse HEAD)" == '9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784' ]]�[0m
 �[36;1mnode --check .github/scripts/review-consumer.test.mjs�[0m
 �[36;1mCENTRAL_REVIEW_CONTRACT_DIR=_central-contract \�[0m
 �[36;1m  PROVIDER_CANARY_CONTRACT_DIR=9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 \�[0m
 �[36;1m  node --test .github/scripts/review-consumer.test.mjs�[0m
 �[36;1m(�[0m
 �[36;1m  cd _central-contract�[0m
 �[36;1m  npm test�[0m
 �[36;1m)�[0m
 �[36;1mgit diff --check�[0m
 shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
 env:
   BWRAP_EXECUTABLE: /home/runner/work/Bong/Bong/_sandbox-package/root/usr/bin/bwrap
 ##[endgroup]
 ✔ shadow caller pins the central workflow and preserves the trusted trigger gate (5.909751ms)
 ✔ shadow caller job exactly matches the reviewed reusable contract (1.508065ms)
 ✔ consumer CI checks out and tests the exact central workflow contract (1.162457ms)
 ✔ checked-out central workflow matches the complete immutable publication contract (2.902203ms)
 ✔ central publication contract rejects every input and secret set drift (2.965276ms)
 ✔ caller publication contract rejects mapping, secret, and permission drift (1.478561ms)
 ✔ central permission contract rejects unbound and non-block permission forms (1.513283ms)
 ✔ checked-out provider canary release matches its caller interface (1.594811ms)
 ✔ provider canary publication contract rejects every input and secret set drift (1.383463ms)
 ✔ provider canary remains dispatch-only, minimally permissioned, and secret-isolated (2.345693ms)
 ✖ Bong policy is bounded declarative data with canonical project rules (2.641652ms)
 ℹ tests 11
 ℹ suites 0
 ℹ pass 10
 ℹ fail 1
 ℹ cancelled 0
 ℹ skipped 0
 ℹ todo 0
 ℹ duration_ms 89.851639
 ✖ failing tests:
 test at .github/scripts/r...

GitHub Actions: Review consumer tests / 0_test.txt: skeleton: agent 叙事管道计划 v1(§10.1.1 促升)

Conclusion: failure

View job details

##[group]Run set -euo pipefail
 �[36;1mset -euo pipefail�[0m
 �[36;1m[[ "$(git -C _central-contract rev-parse HEAD)" == '2986b95388b3ac3f8bf28b8ebdd60a19d2e4364c' ]]�[0m
 �[36;1m[[ "$(git -C 9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 rev-parse HEAD)" == '9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784' ]]�[0m
 �[36;1mnode --check .github/scripts/review-consumer.test.mjs�[0m
 �[36;1mCENTRAL_REVIEW_CONTRACT_DIR=_central-contract \�[0m
 �[36;1m  PROVIDER_CANARY_CONTRACT_DIR=9dcee849e3a0b45bd9a8fe663b48ae3fb1d82784 \�[0m
 �[36;1m  node --test .github/scripts/review-consumer.test.mjs�[0m
 �[36;1m(�[0m
 �[36;1m  cd _central-contract�[0m
 �[36;1m  npm test�[0m
 �[36;1m)�[0m
 �[36;1mgit diff --check�[0m
 shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
 env:
   BWRAP_EXECUTABLE: /home/runner/work/Bong/Bong/_sandbox-package/root/usr/bin/bwrap
 ##[endgroup]
 ✔ shadow caller pins the central workflow and preserves the trusted trigger gate (5.909751ms)
 ✔ shadow caller job exactly matches the reviewed reusable contract (1.508065ms)
 ✔ consumer CI checks out and tests the exact central workflow contract (1.162457ms)
 ✔ checked-out central workflow matches the complete immutable publication contract (2.902203ms)
 ✔ central publication contract rejects every input and secret set drift (2.965276ms)
 ✔ caller publication contract rejects mapping, secret, and permission drift (1.478561ms)
 ✔ central permission contract rejects unbound and non-block permission forms (1.513283ms)
 ✔ checked-out provider canary release matches its caller interface (1.594811ms)
 ✔ provider canary publication contract rejects every input and secret set drift (1.383463ms)
 ✔ provider canary remains dispatch-only, minimally permissioned, and secret-isolated (2.345693ms)
 ✖ Bong policy is bounded declarative data with canonical project rules (2.641652ms)
 ℹ tests 11
 ℹ suites 0
 ℹ pass 10
 ℹ fail 1
 ℹ cancelled 0
 ℹ skipped 0
 ℹ todo 0
 ℹ duration_ms 89.851639
 ✖ failing tests:
 test at .github/scripts/r...
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md

📄 CodeRabbit inference engine (docs/CLAUDE.md)

docs/**/*.md: 新 plan 头部必须写明接入面:进料、出料、复用的共享类型/event/schema、server/agent/client 跨仓库契约,以及对应的 worldview.md 锚点。
涉及真元、灵气、衰减、逸散、半衰、距离损耗、排斥或吸力的 plan,必须调用 qi_physics;新物理常数和公式必须先扩展 qi_physics,不得在功能 plan 中重复实现。
所有真元/灵气流动必须遵守守恒律并通过 qi_physics::ledger::QiTransfer;释放使用 qi_release_to_zone,吸收使用 qi_excretion,不得凭空生成或销毁真元。
涉及玩家可感知行为的 plan,必须在对应机制阶段中内联可直接实现的粒子、音效、HUD、环境、动画和 narration 规格;不得将视听内容笼统推迟到独立阶段。纯 server 逻辑 plan 例外。
每份 plan 必须列出开放问题;实施前必须追加 §N.1 决议,逐项给出结论、实施方案、边界条件,并以文件:行号和 plan 章节双锚点落地。
scope 大于或等于 4 个 PR 的 plan,必须在末尾包含 §10 实施工作流,并按依赖顺序在一个 plan 内序列化多个 PR,不得拆成多个 plan。
涉及 NBT 建筑、worldgen layout 或复杂视觉资产的 TODO,必须完成三轮提交:(round 1/3)(round 2/3)(round 3/3);终轮提交必须包含拼写准确的 <PROMISE> 担保块。
§10 最末必须包含“单次 consume-plan 全自动到 merge”章节,明确用户提交 /consume-plan 后即可等待最终归档至 docs/finished_plans/

引用世界观内容时统一使用 worldview.md §X L<line> 格式;不得自动修改 docs/worldview.md 或主动回写 docs/library/

Files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
docs/plans-skeleton/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

新 plan 必须先读取 docs/CLAUDE.md;骨架、Active 和 Finished plan 必须遵循三态流转及规定的阶段状态、Finish Evidence 结构。

Files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: commit message 必须使用中文,每个逻辑单元一个 atomic commit;agent 生成的 commit 必须带真实模型 Model: <精确模型 id> trailer。
禁止使用 --no-verify--no-gpg-sign、关闭签名配置、未经确认的 force push、hard reset、amend 或交互式 rebase。
禁止留下 auto-stash 产生的孤儿 WIP stash;自动 stash 流程完成后必须恢复自己的 stash。

Files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
🧠 Learnings (3)
📚 Learning: 2026-07-17T00:31:10.779Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:1-1
Timestamp: 2026-07-17T00:31:10.779Z
Learning: 在审查该仓库 `docs/plans-skeleton/` 下的“docs-only skeleton plan”创建类 PR 时:先核对 `docs/CLAUDE.md` 中“Plan 消费规范”,并逐份查看本计划文档里的“§10 实施工作流”,确认后续实施阶段是否会遵循“每个 PR 只修改一个 plan”,且实施/归档时不会出现跨 plan 的修改。该规则不适用于用户显式指定、共享调研基线且计划之间存在互相交叉引用的 skeleton plan 同批创建 PR;对这类情况应按实际交叉引用关系放宽,确保仍能按独立或约定的序列化方式推进。

Applied to files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-17T00:31:13.643Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1218
File: docs/plans-skeleton/plan-skill-av-relink-v1.md:81-85
Timestamp: 2026-07-17T00:31:13.643Z
Learning: 审核 docs/plans-skeleton/*.md 下的 skeleton 草案 PR 时:不得要求作者在“§N 开放问题(P0 决策门前需收口)”核查完成之前提前填写对应的“§N.1 决议”。仅当计划进入 active 且 P0 实施前,已由 Explore agent 并行核查代码现状后,才允许追加“§N.1 决议”,且该决议需包含结论、实施方案、边界条件,并使用“文件:行号 + plan 章节”的双锚点格式。

Applied to files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
📚 Learning: 2026-07-22T02:11:59.191Z
Learnt from: Kizunad
Repo: Kizunad/Bong PR: 1247
File: docs/plans-skeleton/plan-bughunt-animal-air-spawn-gravity-v1.md:0-0
Timestamp: 2026-07-22T02:11:59.191Z
Learning: 在 `docs/plans-skeleton/` 目录下的计划状态行中,若起草日期同时涉及 UTC 与本地日期(例如时区换算后可能跨到不同日期),请在该行中同时标注 UTC 起草日与本地起草日。这样可以避免基于 UTC 的时间基准在 GitHub/CodeRabbit 审查时将本地日期误判为“未来日期”。

Applied to files:

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-agent-narration-pipeline-v1.md
🔇 Additional comments (7)
docs/plans-skeleton/plan-agent-narration-pipeline-v1.md (7)

47-51: LGTM!


191-191: LGTM!


252-252: LGTM!


321-321: LGTM!


367-367: LGTM!


29-32: 🗄️ Data Integrity & Integration

无需修改:source 计数已说明重叠关系。

当前文本已说明:本 plan 将 25 个 confirmed source 加上跨簇 #1702 得到 26 个 confirmed source;其中 #1475/#1509/#1527/#1538 共享 #1470,而 #1607/#1674/#1746 移入 §4.4,不计入本 plan 账本。


29-32: 📐 Maintainability & Code Quality

无需改动现有 plan 内容。

docs/CLAUDE.md 只将骨架列为 docs/plan-*.md 中的“将来要写”占位,并未要求 docs/plans-skeleton/**/*.md 骨架必须先读该文件;且该 skeleton 已在 §0.2 和 P0 记录中明确 root CLAUDE.md / docs/CLAUDE.md 缺失时立即阻塞。

			> Likely an incorrect or invalid review comment.

- ⬜ P0 设计收口 + 吸收清单验真:113 个变体普查(每个标注应有的门禁四元组现状);冻结 `GateSpec` 与拒绝回执语义;等 #1287(冷却重构,同文件大改)merge 定基线。
- ⬜ P1 门禁中间件落地:gate 层上线,先给"已知漏洞簇"的 ~20 个请求类型挂 spec(吸收清单全命中),旧内联校验保留并行断言一个版本期。
- ⬜ P2 巨石拆分批次 A:巨型 match 拆为按域 handler 注册表(combat/production/world/social/npc 五组),行为不变,bot 场景锁住;inventory pickup handler 必须从 ECS `CurrentDimension`、authoritative position/observation range 与 owner/private permission 构造 R10 `PickupAuthorization`,禁止仅凭 client XYZ/instance id 调用 txn,且把 R10 accepted/rejected outcome 交给 R6 emit API。**本批次的 inventory pickup consumer 仅在 R10 P3 pickup/merge txn、R5 P3 attrition API 与 R6 P4 receipt API 均已合入后实施;此前不得以 mock 或旧 R6 P1 schema 接线。**
- ⬜ P2 巨石拆分批次 A:巨型 match 拆为按域 handler 注册表(combat/production/world/social/npc 五组),行为不变,bot 场景锁住;inventory pickup handler 必须从 ECS `CurrentDimension`、authoritative position/observation range 与 owner/private permission 构造 R10 `PickupAuthorization`,禁止仅凭 client XYZ/instance id 调用 txn,且把 R10 accepted/rejected outcome 交给 R6 emit API。consumer 的 production activation 必须遵守 `plan-refactor-master-v1.md §3` Wave 表及其 atomicity/ownership invariants;在上游 artifact 尚未就绪时只允许 declared、unwired、test-only 的 contract/pin,不得把本阶段新增的 phase-level prerequisite 作为跨轨 start/order gate。

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

在 P2 内联拒绝回执的可直接实施规格。

P2 要求把 R10 的 accepted/rejected outcome 交给 R6 emit API。接入面又声明拒绝回执由 client 的 toast/HUD 消费。因此该 plan 不是纯 server 逻辑。请在 P1/P2 内联 reason code 到 HUD/toast 的映射、优先级、fallback,以及适用的音效、动画、环境、粒子和 narration 规格。当前文字只定义了数据交付,没有定义玩家可感知行为。

As per coding guidelines:涉及玩家可感知行为的 plan 必须在对应机制阶段内联可直接实现的粒子、音效、HUD、环境、动画和 narration 规格,不得笼统推迟。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-refactor-c2s-gate-v1.md` at line 23, 在 P1/P2
的相关计划条目中补充可直接实现的拒绝回执体验规格:定义 R10 reason code 到 client toast/HUD 的映射、优先级与未知原因的
fallback,并明确适用的音效、动画、环境、粒子和 narration 行为;同时保留 accepted/rejected outcome 通过 R6
emit API 传递,避免仅描述数据交付或将这些玩家可感知行为推迟到后续阶段。

Source: Coding guidelines


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="docs/plans-skeleton/plan-refactor-c2s-gate-v1.md"
rg -n '(^|[[:space:]])§10|实施工作流|单次 consume-plan 全自动到 merge' "$file"

Repository: Kizunad/Bong

Length of output: 150


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="docs/plans-skeleton/plan-refactor-c2s-gate-v1.md"

printf '--- file size and section markers ---\n'
wc -l "$file"
od -c "$file" | grep -E '§[0-9]+|Prerequisite|Phase|阶段|§10|实施工作流|单次 consume-plan 全自动到 merge' | head -80 || true

printf '\n--- outline of section headings ---\n'
rg -n '^(#+|§[0-9]+|[A-Z0-9]+)' "$file" | head -120

printf '\n--- P-phase references ---\n'
rg -n '\bP[0-9]\b|113|20\+|巨石拆分|Handler|consumer|pickup|吸收计划|§3|§10|实施工作流|单次 consume-plan 全自动到 merge' "$file"

Repository: Kizunad/Bong

Length of output: 241


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="docs/plans-skeleton/plan-refactor-c2s-gate-v1.md"

echo '--- file size and section markers ---'
wc -l "$file"
od -c "$file" | grep -E '§[0-9]+|Prerequisite|Phase|阶段|§10|实施工作流|单次 consume-plan 全自动到 merge' | head -80 || true

echo
echo '--- outline of section headings ---'
rg -n '^(#+|§[0-9]+|[A-Z0-9]+)' "$file" | head -120

echo
echo '--- P-phase references ---'
rg -n '\bP[0-9]\b|113|20\+|巨石拆分|Handler|consumer|pickup|吸收计划|§3|§10|实施工作流|单次 consume-plan 全自动到 merge' "$file"

Repository: Kizunad/Bong

Length of output: 3504


补齐 §10 实施工作流并给出 PR 拆分边界。

当前文档仅列 P0–P4、吸收清单和开放问题,没有结尾的 §10 实施工作流。若本计划 scope ≥ 4 个 PR,必须在末尾按依赖顺序串行拆分多个 PR,并包含“单次 consume-plan 全自动到 merge”章节。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-refactor-c2s-gate-v1.md` at line 23, 在该计划文档末尾补充“§10
实施工作流”,按依赖顺序定义至少 4 个 PR 的串行拆分边界、每个 PR 的范围与验证条件,并明确禁止未满足前置条件时提前合并。新增“单次
consume-plan 全自动到 merge”章节,描述从消费计划、自动执行与验证到合并的完整流程,同时保持现有 P0–P4、吸收清单和开放问题内容不变。

Source: Coding guidelines


🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for file in \
  docs/plans-skeleton/plan-refactor-master-v1.md \
  docs/plans-skeleton/plan-refactor-wire-s2c-v1.md \
  docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
do
  echo "===== $file ====="
  rg -n -C 8 'InventoryEventV1|PickupAuthorization|accepted|rejected|emit|unwired|test-only|atomic|production activation|P[0-4]' "$file"
done

Repository: Kizunad/Bong

Length of output: 25812


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "===== docs/CLAUDE.md if present ====="
if [ -f docs/CLAUDE.md ]; then
  wc -l docs/CLAUDE.md
fi

echo
echo "===== relevant ownership section around R6/R10/R4 ====="
sed -n '37,81p' docs/plans-skeleton/plan-refactor-master-v1.md | cat -n

echo
echo "===== relevant phase sections in R10 skeleton if present ====="
if [ -f docs/plans-skeleton/plan-refactor-inventory-core-v1.md ]; then
  wc -l docs/plans-skeleton/plan-refactor-inventory-core-v1.md
  rg -n -C 8 'PickupAuthorization|inventory receipt|InventoryEventV1|accepted|rejected|Moved|P[0-5]|phase|contract|production' docs/plans-skeleton/plan-refactor-inventory-core-v1.md
else
  echo "missing docs/plans-skeleton/plan-refactor-inventory-core-v1.md"
fi

echo
echo "===== broad search for InventoryEventV1 and PickupAuthorization definitions/usages ====="
rg -n 'InventoryEventV1|PickupAuthorization|InventoryEventHandler|inventory receipt|accepted|rejected' . \
  --glob '!target/**' --glob '!build/**' --glob '!node_modules/**' | head -n 200

Repository: Kizunad/Bong

Length of output: 50368


明确 R6 emit API 的契约和生产启用边界。

R6 拥有 *_emit.rs 公共层和生成/传输契约,R4 P2 只应引用 R6 owner plan 已定义的冻结 API 名称、phase 与 schema;目前 InventoryEventV1::Movedinventory_receive_rejected 还未定义 request idrevision、权威 item view 及 rejected reason/schema。请在 owner plan 中定义后同步本阶段引用;本阶段只提交 contract/pin,不得在生产接缝未就绪时切换 production traffic。

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/plans-skeleton/plan-refactor-c2s-gate-v1.md` at line 23, Update the
owner plan for R6 to define the frozen API, phase, and schema for the R6 emit
contract, including request ID, revision, authoritative item view, and
rejected-reason fields for InventoryEventV1::Moved and
inventory_receive_rejected. Then synchronize this phase’s references to those
exact definitions, keeping the current change limited to declared, unwired,
test-only contract/pin work and preventing production traffic activation until
the R6 production seam is ready.

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 482ebcfdbb490ccbbcee9bd7bad69f6b66a62ddd
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

Validated findings

[major] Pin suite leaves the new source and target variants unprotected

docs/plans-skeleton/plan-refactor-cast-av-contract-v1.md:22 · testing

P1 changes the observable cast_sync contract by adding required source, target, and phase fields, but its mandatory pin suite only requires positive coverage for “每个 phase/discriminant” and negative coverage for missing fields plus invalid/unknown phase. It never requires a positive case for every valid source and target variant/value, nor invalid/unknown source or target cases. Consequently, an implementation that accepts only one source (for example quick_slot but rejects skill_bar), maps a source variant incorrectly, or accepts an unknown target kind would still satisfy the stated suite as long as all phase cases use the one working source/target and the three missing-field tests pass. The master invariant at docs/plans-skeleton/plan-refactor-master-v1.md:74 explicitly requires every enum/discriminant variant and invalid/unknown variants for contract-first schemas, so the R9 phase plan does not provide the required acceptance evidence for its newly added fields.

Root cause: the test matrix is phase-centric even though the changed wire contract adds three independently observable dimensions. it omits variant-complete positive and negative pins for source and target, allowing incorrect schema definitions or mappings for those new fields to pass the required p1 gate.

[major] Use the real command-registration and execution path in the operator-gate integration test

server/tests/dev_command_operator_gate.rs:77 · testing

setup_app constructs the test application by calling dev::register(&mut app) directly and then manually adding only four production command types (PingCmd, BongCmd, FactionCmd, and IdentityCmd) at server/tests/dev_command_operator_gate.rs:77-81; it never invokes the real bong_server::cmd::register entry point (server/src/cmd/mod.rs:10-17) or drives the command plugin's actual input-to-CommandExecutionEvent path. Consequently, an incorrect production wiring such as removing dev::register/the operator gate from cmd::register, or breaking production command registration while leaving dev::register usable, would still pass the new suite because the harness injects the missing registration and directly sends CommandExecutionEvent in execute (server/tests/dev_command_operator_gate.rs:103-111).

Root cause: the regression suite tests an isolated manually assembled command graph rather than the repository's production registration and event producer, so it cannot detect a disconnected operator gate or a broken production command-wiring contract.

[major] Source ledger double-counts #1702

docs/plans-skeleton/plan-agent-narration-pipeline-v1.md:154 · schema-contracts

plan-agent-narration-pipeline-v1.md:144 already lists #1702 in the 13 IDs under narration-routing-and-delivery. However, plan-agent-narration-pipeline-v1.md:154 says the 25 remaining narration IDs are supplemented by the cross-cluster #1702, producing 26 confirmed IDs. Starting from the stated 30 raw IDs and excluding #1551, #1590, #1607, #1674, and #1746 leaves 25, and adding an ID already present in the ledger still leaves 25. The summary and acceptance language therefore claim one source and one source-to-deliverable mapping that the plan does not actually contain.

Root cause: the source-ledger accounting treats #1702 as both an existing member of the 25-item narration cluster and a newly added cross-cluster item. this duplicates the id in the narrative count without adding a distinct source entry, making the plan's stated 26-source coverage and later closure/accountability claims factually inconsistent.

[major] Public-root test self-validates privilege expansion

server/tests/dev_command_operator_gate.rs:398 · security

every_public_root_remains_reachable_to_non_operators derives its expected public set from dev::PUBLIC_COMMAND_ROOTS itself. If a privileged root such as qi is accidentally added to that production constant, scope_dev_command_roots will leave it unscoped and omit it from DevCommandRoots; this assertion will accept the same expanded set, while both later every_dev_root... tests explicitly skip roots in that constant. An online non-operator can then reach /qi, and no test in this file fails. The fixed security contract is that only bong, faction, identity, and ping are public, so the test needs an independent literal/pin rather than the authorization source it is meant to validate.

Root cause: the permission test uses the mutable production allowlist as its expected value, making an accidental privilege expansion self-validating and leaving the public-root authorization boundary unpinned.

[major] Inventory sequence adds an unauthorized R6 P3 prerequisite

docs/plan-refactor-inventory-core-v1.md:98 · wiring

docs/plan-refactor-inventory-core-v1.md:98 defines the authoritative sequence as ... R3 P4 ... → R6 P3 dropped-loot projection/page production activation → R5 P3 + R6 P4 ..., so an implementer following the arrow must wait for the R6 P3 production cutover before starting R5 P3 and R6 P4. This conflicts with plan-refactor-master-v1.md:53,57,77-80, which makes the Wave table the sole source of inter-track ordering and explicitly gates R6 P3 only on R10 P2a metadata and R3 P4 hydration; it does not make R5 P3 or R6 P4 downstream of that cutover. The same inventory plan also says at :94 that R6 P3's activation gate only references the master Wave table, making the local sequence internally inconsistent. The result is a concrete scheduling/wiring error: valid R5/R6 receipt-provider work is artificially blocked by an unrelated production activation, while the claimed activation gate is no longer the effective gate.

Root cause: the revised arrow sequence encodes an additional cross-track start/order dependency instead of merely recording the dependency needed for r10 p2b. master-plan §3 explicitly reserves inter-track sequencing authority to the wave table, so the child plan's local ordering makes the production cutover and provider phases disagree about which artifacts are prerequisites.

[major] Event-time gate test does not prove commands are blocked

server/tests/dev_command_operator_gate.rs:468 · security

For each protected root, the test injects only the bare root name and asserts solely that a rejection chat message appears. It never asserts that no CommandResultEvent is emitted or that command state remains unchanged. A gate implementation that sends the warning but does not stop downstream command handlers therefore passes; indeed bare roots that require arguments generally produce no meaningful operation regardless of authorization. The existing representative negative test covers only qi set 40 in offline mode, and the season test is operator-positive, so an online forged CommandExecutionEvent with a valid privileged command and observable side effect is not protected for these roots.

Root cause: the claimed event-time authorization test verifies the denial message rather than denial of command execution, and uses syntactically incomplete commands, so it does not pin the security property that unauthorized events cannot produce privileged effects.

server_kizuna and others added 6 commits August 5, 2026 20:25
将已审计的 Tiandao narration pipeline 草案整理为仓库 plans-skeleton,保留 source issue ledger、决策门与开放问题。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
移除文档末尾多余空行,保持 skeleton 文件通过仓库 whitespace 检查。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
按现有 Pub/Sub 与 narration wire 能力收窄恢复、确认和路由承诺,并补齐并发账本与实施工作流。

Model: cc-sonnet-high
Co-Authored-By: Claude <noreply@anthropic.com>
Model: cc-haiku-max
Co-Authored-By: Claude <noreply@anthropic.com>
补齐 broadcast 正向路由兼容案例,修正 offline:<name> 兼容性分类,明确 P0 退出决策门并移除内存 pending 选项。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
登记 Agent 轨归属,移出独立 world-model/query-tool follow-up,并明确业务状态提交先于 terminal published。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
server_kizuna and others added 8 commits August 5, 2026 20:26
将已审计的 Tiandao narration pipeline 草案整理为仓库 plans-skeleton,保留 source issue ledger、决策门与开放问题。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
移除文档末尾多余空行,保持 skeleton 文件通过仓库 whitespace 检查。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
按现有 Pub/Sub 与 narration wire 能力收窄恢复、确认和路由承诺,并补齐并发账本与实施工作流。

Model: cc-sonnet-high
Co-Authored-By: Claude <noreply@anthropic.com>
Model: cc-haiku-max
Co-Authored-By: Claude <noreply@anthropic.com>
补齐 broadcast 正向路由兼容案例,修正 offline:<name> 兼容性分类,明确 P0 退出决策门并移除内存 pending 选项。

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
补齐 operator roster、reuse gating、run tag 校验与流程证据边界。

Model: deepseek-v4-flash-max
Co-Authored-By: Claude <noreply@anthropic.com>
- 明确 F07 的 P0 归属并移除未定义 F04
- 对齐 R4 与总纲 Wave 顺序权威
- 补齐 bot roster 与 public-root event-time 验收断言

Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
Model: claude-haiku-4-5-20251001
Co-Authored-By: Claude <noreply@anthropic.com>
@Kizunad
Kizunad force-pushed the docs/plan-agent-narration-pipeline-v1 branch from 482ebcf to 19dd59f Compare August 5, 2026 12:27
@Kizunad
Kizunad merged commit 03fd9a6 into main Aug 5, 2026
1 check 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