Skip to content

R4 P0: C2S 门禁设计收口与吸收清单验真 - #1892

Open
Kizunad wants to merge 1 commit into
mainfrom
refactor/plan-refactor-c2s-gate-v1
Open

R4 P0: C2S 门禁设计收口与吸收清单验真#1892
Kizunad wants to merge 1 commit into
mainfrom
refactor/plan-refactor-c2s-gate-v1

Conversation

@Kizunad

@Kizunad Kizunad commented Aug 3, 2026

Copy link
Copy Markdown
Owner

摘要

  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md 单次促升为 active plan,完成 R4 P0 docs closeout。
  • 以当前 ClientRequestV1 权威枚举重算并逐项冻结 104 变体门禁矩阵,纠正 plan-refactor-master-v1 建轨时 113 变体的历史快照。
  • 冻结穷尽式 GateSpec、欧氏平方 reach、fail-closed mutation barrier、内部拒绝原因与 R6 wire 所有权边界。
  • 逐项验真原吸收清单 24 项:14 完整吸收、1 已闭环回归样本、7 部分吸收、2 移出 R4。

总纲与依赖

本 PR 对齐 docs/plans-skeleton/plan-refactor-master-v1.md §2/§3/§4/§5/§6:R4 implementation 仍属于 Wave 2。#1287 已合入;R6 P1 尚未落地,因此本 PR 严格限定 P0 文档收口,不实施 gate 代码、不修改 R6 契约文件。

验证

  • ClientRequestV1 enum ↔ P0 matrix:104/104,顺序与集合精确一致。
  • absorption ledger:14 + 1 + 7 + 2 = 24。
  • git diff --check origin/main...HEAD
  • 最终 HEAD:83f56d37f2837bd3bdc2dd930a98b22667330827

测试说明

Docs-only;未运行 Cargo。未运行 scripts/test-tmux-shutdown-order.shscripts/test-server-lifecycle.sh

主导模型:claude-sonnet-5

@coderabbitai

coderabbitai Bot commented Aug 3, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • 文档

    • 新增 C2S 请求统一门禁与处理器拆分计划,明确校验规则、拒绝原因、资源预算及分阶段路线。
    • 更新 C2S/S2C 变体基线与契约要求,统一记录最新数量和版本基准。
  • 测试与质量

    • 新增门禁矩阵检查,验证请求变体、编号、顺序及配置的一致性。
    • 增加覆盖多种枚举结构和异常场景的自动化测试。
    • 将门禁矩阵检查接入端到端测试流程。

Walkthrough

新增 C2S 门禁重构计划,冻结 104 个 ClientRequestV1 变体基线。新增矩阵一致性检查脚本、单元测试和 CI 步骤。更新相关计划中的 C2S/S2C 基线。

Changes

C2S 门禁计划与契约

Layer / File(s) Summary
门禁契约与变体基线
docs/plan-refactor-c2s-gate-v1.md, docs/plans-skeleton/plan-refactor-master-v1.md, docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
定义 RequestGate、校验顺序、资源预算、owner 规则、handler 分域和覆盖 104 个变体的门禁矩阵,并更新 C2S/S2C 基线。
实施阶段与验收边界
docs/plan-refactor-c2s-gate-v1.md
补充计划吸收清单、P0–P4 交付物、跨轨依赖、验收场景、实施顺序和完成条件。
矩阵解析与一致性校验
scripts/check_c2s_gate_matrix.py, scripts/tests/check_c2s_gate_matrix_test.py, .github/workflows/e2e.yml
解析 ClientRequestV1 和计划矩阵,检查 serde 配置、编号连续性、变体集合、重复项及顺序;新增单元测试,并在 E2E 工作流中执行检查。

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • Kizunad/Bong#1301:更新该 PR 引入的 C2S gate 计划,并为同一矩阵增加校验工具。
  • Kizunad/Bong#1893:共同修改 C2S gate 计划中的库存拾取授权和跨轨依赖契约。
  • Kizunad/Bong#1894:共同涉及 C2S gate 的会话状态、所有权和 craft 请求校验契约。

Poem

兔耳旁,矩阵排成行,
Rust 变体不再躲藏。
CI 轻敲检查门,
重复与漂移无处藏。
门禁计划向前跑,
小兔挥爪庆新章。

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 标题准确概括了 C2S 门禁设计收口和吸收清单验真的主要变更。
Description check ✅ Passed 描述与文档收口、104 项变体矩阵、吸收清单验真及验证结果直接相关。
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
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/plan-refactor-c2s-gate-v1
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch refactor/plan-refactor-c2s-gate-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 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 83f56d37f2837bd3bdc2dd930a98b22667330827
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Resolve and wire coffin ownership before declaring P0 closed

docs/plan-refactor-c2s-gate-v1.md:149 · strict-maintainability

P0 is marked complete as a design closeout, but matrix row 27 still specifies CoffinBreak authorization as the unresolved alternative "owner 或明确公共破坏策略". The absorbed source plan confirms this is a real unresolved A/B product decision (docs/plans-skeleton/plan-bughunt-coffin-reclaim-owner-gate-v1.md:58-64). Production state also cannot currently support the proposed owner gate: server/src/coffin/mod.rs:113-122 defines CoffinEntity without any owner identity, CoffinRegistry::insert at lines 132-145 accepts and records no owner, and reclaim_occupied at lines 200-214 reconstructs a coffin without one. Nevertheless, this plan classifies the issue as fully absorbed at line 244 and commits P1 to connecting all 14 fully absorbed items at lines 286-288. Consequently P1 has neither a frozen authorization rule nor an authoritative owner producer/lifecycle to consume; an implementation can choose incompatible break behavior or cannot implement OwnershipRule at all while still appearing to follow the completed P0 matrix.

Root cause: the plan declares design closure and full absorption before resolving the coffin-break ownership policy and before specifying the domain state that produces authoritative coffin ownership, leaving a required production gate disconnected from its source of truth.

[major] Bound gate-denial alerts and logging per client

docs/plan-refactor-c2s-gate-v1.md:99 · performance

The new P0 contract requires P1 to emit an EventAlert for every gate rejection (docs/plan-refactor-c2s-gate-v1.md:99) and to log every rejection (:97), but defines no per-player request or denial budget, coalescing, or backpressure anywhere in the plan. The real ingress path processes every queued bong:client_request event in handle_client_request_payloads (server/src/network/client_request_handler.rs:541), deserializes each payload (:557), and has no request throttle; repository search finds rate limiting for chat only, not this channel. A connected client can repeatedly send a valid-version request that deterministically fails a cheap gate such as OutOfReach or TargetNotFound. Following this frozen design makes every input perform gate work, append a log record, serialize an S2C payload, and enqueue an alert packet. Thus request volume directly creates unbounded per-tick CPU, log output, and outbound packets, allowing one client to drive tick latency and output queues without reaching any gameplay handler. Existing validation does not prevent this because version-valid requests are accepted and denial is precisely the new response path; the PR introduces/exposes it by making per-denial feedback and logging part of the completed P0 contract and P1 acceptance criteria.

Root cause: the frozen gate contract treats every rejected c2s message as an independently logged and acknowledged event but omits resource bounds at the untrusted request boundary. p1 needs an explicit per-client budget/backpressure policy, with denial alerts and logs rate-limited or coalesced, plus a flood/burst acceptance test that proves bounded work and output.

[major] Frozen Euclidean reach contract breaks existing Workbench boundary behavior

docs/plan-refactor-c2s-gate-v1.md:61 · correctness

The new frozen contract requires every player-to-world-target reach check to use 3D Euclidean squared distance and assigns Workbench a 3.0 radius. The current production helper server/src/craft/workbench.rs:73-81 deliberately uses Chebyshev distance, and its regression test at server/src/craft/workbench.rs:388-390 explicitly accepts a player at offset [3.0, 3.0, 3.0]. Under the proposed contract that same established input has distance_squared = 27 > 9 and is rejected. The nearby statement at line 74 that P1 must preserve existing gameplay radii does not prevent this because changing the metric changes the accepted region even when the numeric radius remains 3.0. This PR turns the skeleton's previously open metric question into the contradictory completed P0 contract, so a subsequent conforming implementation would introduce the behavior change.

Root cause: the plan treats preserving a numeric radius as preserving reach behavior, but freezes a global euclidean metric without preserving domain-specific existing metrics. for workbench, replacing the established chebyshev predicate with euclidean distance shrinks the accepted interaction volume and contradicts the plan's behavior-preservation requirement.

[major] Update the master and R6 contracts to the 104-variant baseline

docs/plan-refactor-c2s-gate-v1.md:17 · strict-maintainability

The promoted plan states that the authoritative enum now has 104 variants and that the old 113 count "cannot continue". However, the parent plan still defines completion as "113 C2S variants" in docs/plans-skeleton/plan-refactor-master-v1.md:101, and the R6 plan still requires positive/negative samples for "113 C2S" variants in docs/plans-skeleton/plan-refactor-wire-s2c-v1.md:25. Those are live acceptance contracts consumed by later phases, not historical prose. Thus the same repository now has incompatible authoritative completion counts: R4/P3 derives 104 from the enum while the master and dependent R6 gates require 113. No implementation or test can simultaneously demonstrate both exact contracts without ignoring one document or inventing nine variants.

Root cause: the pr corrected a duplicated canonical variant count only in the promoted r4 plan, leaving the parent and cross-track acceptance contracts stale. this creates an already-observable inconsistency in duplicated sources of truth and makes plan completion evidence ambiguous.

[major] P1 cannot wire the exhaustive gate before P3 declares the remaining variants

docs/plan-refactor-c2s-gate-v1.md:286 · wiring

The plan freezes an exhaustive ClientRequestV1::gate_spec() in which every enum variant must return Spec or a reasoned NoGate and says the gate runs on the production path before dispatch (lines 45-67 and 76-89). However, P1 says to install that production gate while wiring only group A's 14 requests plus one migration sample, while P3 is deferred to a later PR to declare all variants (lines 286-292). Rust cannot compile the promised exhaustive method with the other current variants omitted. Making those variants compile via temporary NoGate arms would route requests that this same matrix identifies as requiring gates around the mutation barrier, such as inventory and session requests, and would defeat the no-default-bypass contract until P3. Existing downstream checks do not supply the promised unified pre-dispatch barrier, which the plan itself identifies as currently absent.

Root cause: the staged delivery splits installation of an enum-exhaustive production dispatcher from declaration of most of the enum's required routing metadata, even though those two pieces must be introduced atomically for the production gate to be both compilable and fail-closed.

[major] Target-first rejection ordering exposes an entity existence and location oracle

docs/plan-refactor-c2s-gate-v1.md:84 · security

The frozen evaluation order resolves the attacker-supplied target before checking dimension and distance (docs/plan-refactor-c2s-gate-v1.md:82-89), and the contract says only the first failure is returned (:91). Requests such as WorkbenchOpen, ContainerOpen, NpcInspectRequest, and GiveDanToElder carry client-controlled protocol entity IDs, which production resolves through the global EntityManager::get_by_id path (for example server/src/network/client_request_handler.rs:2450). The plan then defines distinct TargetNotFound, WrongDimension, and OutOfReach reasons (docs/plan-refactor-c2s-gate-v1.md:97) and explicitly maps them to distinguishable requester-visible messages such as “目标不在此界” and “距离太远” (:99). An untrusted client can therefore probe guessed/sequential entity IDs: an absent ID yields TargetNotFound, an existing entity in another dimension yields WrongDimension, and an existing same-dimension entity outside reach yields OutOfReach. Neither the proposed authoritative lookup nor the later ownership/state checks establish that the requester is allowed to know the target exists, so those later checks do not prevent the oracle. This exposure is introduced by the PR's newly frozen gate ordering and rejection contract and contradicts its stated non-disclosure guarantee.

Root cause: the gate design treats global target existence, dimension, and distance failures as requester-visible diagnostics before establishing a visibility/capability boundary for the target. distinct early failures turn authoritative target resolution into an entity-existence and location oracle. the contract should collapse all pre-authorization target failures to one indistinguishable external response (while retaining internal diagnostics), or establish requester visibility/authority before emitting differentiated reasons.

[major] Completed P0 claims an enum-matrix script check that is not present

docs/plan-refactor-c2s-gate-v1.md:284 · testing

The new plan marks P0 complete at line 282 and then states at line 284 that a script compared all 104 matrix rows with all 104 ClientRequestV1 variants with no omissions or duplicates. However, this PR only adds the plan document and deletes its skeleton; no comparison script, test, output, or other reproducible evidence is added. Repository searches find no gate-matrix sweep/comparison implementation, while the actual enum remains separately maintained in server/src/schema/client_request.rs:35. A matrix row can therefore be renamed, duplicated, or omitted while all existing tests still pass, despite the completed phase claiming that exact regression protection/evidence.

Root cause: p0 is declared complete based on an asserted enum-to-matrix script check, but the change does not provide the claimed check or any durable result. the plan and rust enum remain independent sources, so the stated no-gap/no-duplicate acceptance evidence is not reproducible or regression-protected.

@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: 5

🤖 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/plan-refactor-c2s-gate-v1.md`:
- Around line 328-330: 在 active plan
中补充“开放问题”章节,列出所有未决项;若无未决项,明确写出“无开放问题”及验证依据。针对每项开放问题,在实施前新增对应的“§N.1
决议”,记录结论、实施方案、边界条件,并同时标注文件:行号与 plan 章节双锚点。
- Around line 352-354: 补充 §10 末尾“单次 consume-plan 全自动到 merge”章节,明确一次提交
`/consume-plan` 后系统按 PR-2 至 PR-5 串行执行、等待每阶段门禁、完成 merge,并最终迁移至
`docs/finished_plans/`;同时定义任一阶段失败、门禁未通过或进入暂停状态时立即停止且不得归档,保留 §10.4 现有终态条件。

In `@docs/plans-skeleton/plan-refactor-wire-s2c-v1.md`:
- Line 25: 修正 P4 计划中 ServerDataType 基线的来源或数值:核对 agent schema 的 ServerDataType
union 计数;若 144 包含旁路 channel 或其他服务端 Rust 变体,明确标注其来源和范围,否则将基线改为当前 agent union
的实际数量,避免按错误总量规划验证。

In `@scripts/check_c2s_gate_matrix.py`:
- Around line 12-29: Update enum_variants() and VARIANT_RE to recognize struct,
unit, and tuple ClientRequestV1 variants, while tracking whether the enum’s
closing brace was found. Reject unrecognized top-level variants and raise an
error for truncated or otherwise unclosed enums, even when some variants were
parsed. Add fixture tests covering all three variant forms and malformed or
truncated enum definitions.
- Around line 9-11: 补齐 scripts/check_c2s_gate_matrix.py(9-11 行)的跨仓 parity
gate:增加 agent 的 ClientRequestV1 TypeBox schema 输入,规范化并对比 Rust enum、matrix 与
agent schema 的变体集合,同时统一声明 authority、projection 边界及差异 allowlist。更新
docs/plan-refactor-c2s-gate-v1.md(243 行),将 104/104 证据扩展为 Rust、matrix、agent
schema、gate registry 的完整契约校验,或明确标注 server-only 变体。更新
docs/plans-skeleton/plan-refactor-master-v1.md(101 行),不要将 Rust 数量表述为全部跨仓
ClientRequestV1 变体。
🪄 Autofix (Beta)

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: f410b153-9386-4149-b095-bd580c4f81cf

📥 Commits

Reviewing files that changed from the base of the PR and between 3dfa7a0 and e90a8ae.

📒 Files selected for processing (5)
  • docs/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • scripts/check_c2s_gate_matrix.py
💤 Files with no reviewable changes (1)
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: e2e
🧰 Additional context used
📓 Path-based instructions (7)
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/

一个 PR 只允许修改一个 plan;/consume-plan 只能追加 Finish Evidence 或执行允许的 plan 归档移动,不得自动修改其他 docs 文件、CLAUDE.md 或 worldview 文档。

Files:

  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plan-refactor-c2s-gate-v1.md
docs/plans-skeleton/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

新建 plan 前必须先阅读 docs/CLAUDE.md;骨架 plan 必须包含主题和 P0/P1 等阶段总览。

Files:

  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
**/*.{rs,ts,tsx,java,json,md}

📄 CodeRabbit inference engine (CLAUDE.md)

唯一真货币是骨币;矿物是交易筹码,灵石是燃料/衰变物,金银不是货币。

Files:

  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plan-refactor-c2s-gate-v1.md
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: 禁止 git stash push 后不执行对应 git stash pop;不得留下孤儿 WIP stash。
每个逻辑单元使用中文 atomic commit;agent 产生的每个 commit 必须包含真实模型 ID 的 Model: trailer。
未经明确确认不得执行 force push、hard reset、amend、交互式 rebase、批量删除/移动文件或依赖版本/生产配置修改;严禁 --no-verify--no-gpg-sign 及关闭签名。
PR review 只能通过独立评论 /review 触发;不得等待 Codex,review 修改后必须重新等待 re-review。

Files:

  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plan-refactor-c2s-gate-v1.md
  • scripts/check_c2s_gate_matrix.py
docs/plan-*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/plan-*.md: Active plan 每个阶段必须写出可核验的模块路径、类型/函数、测试、schema、Redis key 或跨仓库契约 symbol。
plan 归档前必须所有阶段标记为 ✅ YYYY-MM-DD 并补充严格标题为 ## Finish Evidence 的证据章节。

Files:

  • docs/plan-refactor-c2s-gate-v1.md
**/*.{rs,ts,tsx,java,py}

📄 CodeRabbit inference engine (CLAUDE.md)

新增 skill/cast/主动能力必须同时提供独立 animation、particle/VFX、SFX、HUD 反馈和 hotbar/SkillBar PNG icon;仅实现 server 或 schema 不算完成。

Files:

  • scripts/check_c2s_gate_matrix.py
**/*.{nbt,bbmodel,json,py}

📄 CodeRabbit inference engine (CLAUDE.md)

NBT 建筑、worldgen layout、复杂模型和视觉资产必须经过三轮打磨;提交信息标注 (round N/3),终轮包含 <PROMISE>...已 3 轮打磨...</PROMISE>

Files:

  • scripts/check_c2s_gate_matrix.py
🧠 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-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-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-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-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-wire-s2c-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
🪛 LanguageTool
docs/plan-refactor-c2s-gate-v1.md

[uncategorized] ~301-~301: 您的意思是“"不"齐”?
Context: ...questV1变体穷尽声明Spec/NoGate(reason)`、补齐声明所需 authority/state adapter,并把 product...

(BU)


[uncategorized] ~306-~306: 您的意思是“"不"洞”?
Context: ...StateGateId adapter 保留单一事实源,不再承担任何枚举声明补洞。 - 测试抓手:enum↔registry 穷尽编译门继续常绿;...

(BU)


[uncategorized] ~332-~332: 您不可用“是”来表达个形容词。大部分人都平常用“很”来代替“是”。您是不是想表达"很纯"?
Context: ...。 ## §10 实施工作流 ### §10.1 适用边界 本 plan 是纯 server/network 逻辑重构,不产出 NBT、worldgen la...

(SHI_ADHECTIVE_ERROR)


[uncategorized] ~339-~339: 您的意思是“"不"枚”?
Context: ...变。 4. PR-4 / P3:删重复门禁并收敛领域 adapter,不补枚举声明欠账。 5. PR-5 / P4:bot/e2e 与符合条件的吸...

(BU)

🪛 Ruff (0.16.0)
scripts/check_c2s_gate_matrix.py

[warning] 28-28: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 36-36: Use a list comprehension to create a transformed list

(PERF401)


[warning] 38-38: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (2)
docs/plan-refactor-c2s-gate-v1.md (1)

1-242: LGTM!

Also applies to: 244-327, 331-351

scripts/check_c2s_gate_matrix.py (1)

1-8: LGTM!

Also applies to: 30-45, 46-95

Comment thread docs/plan-refactor-c2s-gate-v1.md
Comment thread docs/plan-refactor-c2s-gate-v1.md
- ⬜ P2 client 桥接层收敛:枚举前缀剥离收敛到单点(含 forge-session 修复);`ServerDataRouter` 注册表整备(分域注册文件,不再单个 1547 行 switch 追加)。
- ⬜ P3 旁路归一批次:28 channel 逐批收编入 server_data envelope 或登记豁免(资源包/握手类可豁免);删除散装 receiver。
- ⬜ P4 契约 pin 全量化:双向 sample 对拍测试补齐(113 C2S + 144 S2C 每变体至少一条正反 sample,schema 改动连 sample 一起改);emit 迁移到 builder 的长尾批次。
- ⬜ P4 契约 pin 全量化:双向 sample 对拍测试补齐(届时现行 `ClientRequestV1` 全部变体,2026-08-03 基线 104;以及届时现行 `ServerDataType` 全部变体,2026-07-27 侦察基线 144;每变体至少一条正反 sample,schema 改动连 sample 一起改);emit 迁移到 builder 的长尾批次。

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/sh
set -eu

python3 - <<'PY'
from pathlib import Path
import re

text = Path("agent/packages/schema/src/server-data.ts").read_text(encoding="utf-8")
block = text.split("export const ServerDataType = Type.Union([", 1)[1]
block = block.split("]);", 1)[0]
tags = re.findall(r'Type\.Literal\("([^"]+)"\)', block)

print(f"ServerDataType literal count: {len(tags)}")
if len(tags) != 144:
    raise SystemExit("ServerDataType count does not match the documented 144 baseline")
PY

Repository: Kizunad/Bong

Length of output: 248


🏁 Script executed:

#!/bin/sh
set -eu

echo "== plan =="
sed -n '1,80p' docs/plans-skeleton/plan-refacor-wire-s2c-v1.md 2>/dev/null || true

echo "== file match =="
fd -a 'plan-refactor-wire-s2c-v1\.md|server-data\.ts' . | sed 's#^\./##'

echo "== schema outline/search =="
wc -l agent/packages/schema/src/server-data.ts
python3 - <<'PY'
from pathlib import Path
text = Path("agent/packages/schema/src/server-data.ts").read_text(encoding="utf-8")
print("contains ServerDataType:", "export const ServerDataType = Type.Union([" in text)
print("line 190-305:")
for i,line in enumerate(text.splitlines(), 1):
    if 190 <= i <= 305:
        print(f"{i}: {line}")
PY

echo "== enumerate all Type.Literal values in server-data.ts =="
python3 - <<'PY'
import re
from pathlib import Path
text = Path("agent/packages/schema/src/server-data.ts").read_text(encoding="utf-8")
print("all exported/labeled datatypes:")
for i,line in enumerate(text.splitlines(), 1):
    if "export const " in line or "Type.Union" in line:
        print(f"{i}: {line}")
print("ServerDataType total literals:", len(re.findall(r'Type\.Literal\("([^"]+)"\)', text)))
PY

echo "== search for 144 and ServerDataType =="
rg -n "144|ServerDataType|ClientRequestV1|Client.*base|基线" docs/plans-skeleton docs agent packages 2>/dev/null | head -n 200

Repository: Kizunad/Bong

Length of output: 50368


🏁 Script executed:

#!/bin/sh
set -eu

echo "== plan exact =="
sed -n '1,70p' docs/plans-skeleton/plan-refactor-wire-s2c-v1.md

echo "== schema TypeScript exports from server-data.ts =="
python3 - <<'PY'
import re
from pathlib import Path
text = Path("agent/packages/schema/src/server-data.ts").read_text()
lines = text.splitlines()
for i,line in enumerate(lines, 1):
    if i < 20 or line.startswith(("export const ", "export type ") or "Type.Union" in line):
        if i <= 400:
            print(f"{i}: {line}")
PY

echo "== Rust ServerData enum search =="
fd -a 'server_data\.(rs|rs)$$' . | sed 's#^\./##'
rg -n "enum ServerData|ServerData(v1)?|ServerDataType|Type::" server agent 2>/dev/null | sed -n '1,240p'

echo "== agent package source files near ServerDataV1 =="
rg -n "ServerDataV1|ServerDataType" docs/plans-skeleton/plan-refactor-wire-s2c-v1.md agent/packages/schema src 2>/dev/null | sed -n '1,200p'

echo "== precise literal count in ServerDataType union block =="
python3 - <<'PY'
import re
from pathlib import Path
text = Path("agent/packages/schema/src/server-data.ts").read_text()
block = text.split("export const ServerDataType = Type.Union([",1)[1]
block = block.split("]);",1)[0]
vals = re.findall(r'Type\.Literal\("([^"]+)"\)', block)
print("count", len(vals))
PY

Repository: Kizunad/Bong

Length of output: 44261


修正 ServerDataType 基线或补全说明。

P4 的 ServerDataType 侦察基线写为 144,但 agent schema 的 ServerDataType literal 计数为 100。若这 144 包含旁路 channel 或其他服务端 Rust 变体,请标注来源;否则将基线改为当前 agent union 数量,避免 P4 按错误总量留空验证。

🤖 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-wire-s2c-v1.md` at line 25, 修正 P4 计划中
ServerDataType 基线的来源或数值:核对 agent schema 的 ServerDataType union 计数;若 144 包含旁路
channel 或其他服务端 Rust 变体,明确标注其来源和范围,否则将基线改为当前 agent union 的实际数量,避免按错误总量规划验证。

Comment thread scripts/check_c2s_gate_matrix.py
Comment thread scripts/check_c2s_gate_matrix.py Outdated
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: e90a8ae566e32ca65f9fc468626ef19230850ec3
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Wire the enum-to-matrix contract check into automated tests

docs/plan-refactor-c2s-gate-v1.md:299 · testing

The PR defines the changed P0 contract as matrix variants = ClientRequestV1 variants and says scripts/check_c2s_gate_matrix.py provides its regression check, but the checker is only referenced by this plan and is not invoked by any test or workflow. The repository's E2E workflow runs server, schema, client, agent, smoke, and bot tests, but no matrix checker. A concrete regression—adding a compiling ClientRequestV1 variant while omitting its matrix row (or deleting/reordering a row)—therefore leaves every existing automated check green unless someone manually runs the script, despite the plan's stated automatic inclusion requirement.

Root cause: the new consistency assertion was implemented only as an opt-in standalone script and was not registered in ci or an automated test target, so the observable enum-to-matrix contract has no enforced regression protection.

[major] Unit or tuple request variants silently bypass the matrix check

scripts/check_c2s_gate_matrix.py:12 · correctness

enum_variants() only appends lines matching ^ Name {$. A concrete future declaration such as HealthCheck, or LegacyRequest(u8), inside ClientRequestV1 is therefore silently omitted. With the matrix left unchanged, both parsed lists remain identical and main() prints success and returns 0, even though the new enum variant has no matrix row. The later set/order checks cannot catch a variant that the parser never collected, and there is no independent variant-count assertion. This behavior is introduced by the new checker and contradicts the PR's stated contract that future/current variants are automatically included in the exhaustive comparison.

Root cause: the enum parser treats only struct-style rust variants as variants instead of parsing every top-level clientrequestv1 variant or failing closed on unsupported variant syntax. consequently, valid unit and tuple variants bypass the security-sensitive gate-matrix completeness check.

[major] 104-variant baseline ignores the TypeBox source of truth

docs/plan-refactor-c2s-gate-v1.md:14 · schema-contracts

The new plan declares server/src/schema/client_request.rs::ClientRequestV1 to be the authoritative 104-variant set and marks P0 complete based on scripts/check_c2s_gate_matrix.py, but that checker only compares the Rust mirror with the Markdown matrix. Bong's IPC source of truth is the TypeBox schema, and its actual ClientRequestV1 union in agent/packages/schema/src/client-request.ts:1135-1223 does not contain multiple Rust/matrix variants, including AlchemyLearnRecipeFragment, CoffinBreak, CoffinMenuReclaim, QiScatterBeadUse, SupplyCoffinOpen, ExternalContainerMove, CraftStart, and GiveDanToElder. Therefore the advertised checker can report that all 104 requests match while the authoritative schema and generated artifacts expose a different request set. No surrounding check in this script reads TypeBox or generated JSON Schema, so the mismatch is not prevented.

Root cause: the plan reverses the repository's schema authority by treating the rust serde enum as canonical, and the new validation checks only that mirror against documentation instead of validating typebox, generated json schema, rust, and consumers together.

@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 `@agent/packages/schema/src/client-request.ts`:
- Around line 1184-1212: Update the entity ID fields in
SupplyCoffinOpenRequestV1, ContainerOpenRequestV1, WorkbenchOpenRequestV1, and
GiveDanToElderRequestV1 to use Type.Integer({ minimum: 0 }) instead of bare
Type.Integer(), matching the existing non-negative entity ID schema convention.
- Around line 1382-1430: 在 client-request.ts 的 ClientRequestV1 samples
定义中,为新增的各个 variant(包括 qi_scatter_bead_use、装备/背包/战斗、搜索、容器及 lingtian_start_*
等)逐一补充符合当前 schema 的正向和反向 sample;同时完善 inventory_move_intent 与 cancel_search
的完整正反对拍,并确保 samples 与最新 schema 变更保持一致。
- Around line 1237-1300: 将 LingtianStartTillRequestV1 和
LingtianStartHarvestRequestV1 中的 mode 从 Type.String() 改为覆盖实际协议允许值的
Type.Union([Type.Literal(...),
...]),分别使用各自对应的有限取值集合;同步确保生成的静态类型反映这些字面量约束,不要引入未定义的模式值。
🪄 Autofix (Beta)

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: 9a714cd4-6739-41fd-bf70-019ef1f4f43c

📥 Commits

Reviewing files that changed from the base of the PR and between e90a8ae and 353c013.

⛔ Files ignored due to path filters (1)
  • agent/packages/schema/generated/client-request-v1.json is excluded by !**/generated/**
📒 Files selected for processing (5)
  • .github/workflows/e2e.yml
  • agent/packages/schema/src/client-request.ts
  • docs/plan-refactor-c2s-gate-v1.md
  • scripts/check_c2s_gate_matrix.py
  • scripts/tests/check_c2s_gate_matrix_test.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: e2e
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{rs,ts,tsx,java,py}

📄 CodeRabbit inference engine (CLAUDE.md)

新增 skill/cast/主动能力必须同时提供独立 animation、particle/VFX、SFX、HUD 反馈和 hotbar/SkillBar PNG icon;仅实现 server 或 schema 不算完成。

Files:

  • scripts/tests/check_c2s_gate_matrix_test.py
  • agent/packages/schema/src/client-request.ts
  • scripts/check_c2s_gate_matrix.py
**/*.{nbt,bbmodel,json,py}

📄 CodeRabbit inference engine (CLAUDE.md)

NBT 建筑、worldgen layout、复杂模型和视觉资产必须经过三轮打磨;提交信息标注 (round N/3),终轮包含 <PROMISE>...已 3 轮打磨...</PROMISE>

Files:

  • scripts/tests/check_c2s_gate_matrix_test.py
  • scripts/check_c2s_gate_matrix.py
**/*

📄 CodeRabbit inference engine (CLAUDE.md)

**/*: 禁止 git stash push 后不执行对应 git stash pop;不得留下孤儿 WIP stash。
每个逻辑单元使用中文 atomic commit;agent 产生的每个 commit 必须包含真实模型 ID 的 Model: trailer。
未经明确确认不得执行 force push、hard reset、amend、交互式 rebase、批量删除/移动文件或依赖版本/生产配置修改;严禁 --no-verify--no-gpg-sign 及关闭签名。
PR review 只能通过独立评论 /review 触发;不得等待 Codex,review 修改后必须重新等待 re-review。

Files:

  • scripts/tests/check_c2s_gate_matrix_test.py
  • docs/plan-refactor-c2s-gate-v1.md
  • agent/packages/schema/src/client-request.ts
  • scripts/check_c2s_gate_matrix.py
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/

一个 PR 只允许修改一个 plan;/consume-plan 只能追加 Finish Evidence 或执行允许的 plan 归档移动,不得自动修改其他 docs 文件、CLAUDE.md 或 worldview 文档。

Files:

  • docs/plan-refactor-c2s-gate-v1.md
docs/plan-*.md

📄 CodeRabbit inference engine (CLAUDE.md)

docs/plan-*.md: Active plan 每个阶段必须写出可核验的模块路径、类型/函数、测试、schema、Redis key 或跨仓库契约 symbol。
plan 归档前必须所有阶段标记为 ✅ YYYY-MM-DD 并补充严格标题为 ## Finish Evidence 的证据章节。

Files:

  • docs/plan-refactor-c2s-gate-v1.md
**/*.{rs,ts,tsx,java,json,md}

📄 CodeRabbit inference engine (CLAUDE.md)

唯一真货币是骨币;矿物是交易筹码,灵石是燃料/衰变物,金银不是货币。

Files:

  • docs/plan-refactor-c2s-gate-v1.md
  • agent/packages/schema/src/client-request.ts
agent/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Agent TypeScript 改动应运行对应包的 npm test;schema 源码改动后必须先构建 @bong/schema 的 dist。

Files:

  • agent/packages/schema/src/client-request.ts
agent/packages/schema/src/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

修改 schema 源码后必须执行 cd agent && npm run build -w @bong/schema``,因为 agent 使用构建产物 dist/

Files:

  • agent/packages/schema/src/client-request.ts
agent/packages/schema/**/*.{ts,json}

📄 CodeRabbit inference engine (CLAUDE.md)

每个 TypeBox/serde schema variant 都必须有正反 sample 对拍;schema 变更必须同步修改 samples。

Files:

  • agent/packages/schema/src/client-request.ts
**/*.{rs,ts,tsx,java}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{rs,ts,tsx,java}: 六境界必须使用“醒灵→引气→凝脉→固元→通灵→化虚”,不得使用练气、筑基、金丹、元婴等旧称。
命名不得使用末法禁词玄、陨、星、仙、太、古,除明确允许的俗世矿名例外。

Files:

  • agent/packages/schema/src/client-request.ts
🪛 Ruff (0.16.0)
scripts/tests/check_c2s_gate_matrix_test.py

[warning] 10-10: Assertion should be broken down into multiple parts

Break down assertion into multiple parts

(PT018)


[warning] 46-46: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

scripts/check_c2s_gate_matrix.py

[warning] 27-27: Too many branches (13 > 12)

(PLR0912)


[warning] 45-45: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 53-53: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 56-56: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 61-61: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 68-68: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 90-90: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 94-94: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 117-117: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 137-137: Too many branches (17 > 12)

(PLR0912)


[warning] 159-159: zip() without an explicit strict= parameter

Add explicit value for parameter strict=

(B905)

🔇 Additional comments (19)
docs/plan-refactor-c2s-gate-v1.md (3)

132-243: LGTM!


295-310: LGTM!


132-136: 📐 Maintainability & Code Quality

无需拆分此更改。

docs/plans-skeleton 文件是骨架文档,不在 /consume-plan 归档消费路径内;按根 CLAUDE.md 定义,一 PR 一份 plan 约束的是 active plan 实施与 /consume-plan 归档行为,不禁止 docs-only 骨架基线更新与此 plan 的联动补充。

			> Likely an incorrect or invalid review comment.
agent/packages/schema/src/client-request.ts (6)

545-556: LGTM!


652-675: LGTM!


937-949: LGTM!


1352-1370: LGTM!


1439-1445: LGTM!


1214-1225: 🗄️ Data Integrity & Integration

无需为 ExternalContainerMoveRequestV1 增加 revision 字段。

该请求仅使用 session_id/instance_id/from/to,协议、Rust enum、TypeBox schema 及 proto 定义均不含独立 revision;校验与并发控制依赖资源锁和 server 重读,不在 schema 层面定义 revision 字段。

			> Likely an incorrect or invalid review comment.
scripts/check_c2s_gate_matrix.py (8)

1-20: LGTM!


23-69: LGTM! 解析逻辑对 struct/unit/tuple/多行 tuple 及未知顶层语法(如显式 discriminant)均能正确处理并 fail closed,与配套测试用例的断言一致。


72-83: LGTM!


86-117: LGTM! 已补齐 TypeBox union 成员、wire literal 与生成 JSON Schema 的解析,覆盖此前缺失的跨仓校验输入。


120-134: LGTM!


137-181: LGTM!


182-195: LGTM!


197-204: LGTM!

scripts/tests/check_c2s_gate_matrix_test.py (1)

1-52: LGTM! 三个用例覆盖 struct/unit/tuple 及未知顶层语法的 fail-closed 场景,与 check_c2s_gate_matrix.py::parse_enum_variants 的实际实现行为一致。

.github/workflows/e2e.yml (1)

119-123: 🩺 Stability & Availability

无需修改。

生成文件 agent/packages/schema/generated/client-request-v1.json 已受 git 追踪且未被 gitignore 排除。

Comment on lines +1184 to +1212
export const SupplyCoffinOpenRequestV1 = Type.Object(
{
v: Type.Literal(1),
type: Type.Literal("supply_coffin_open"),
entity_id: Type.Integer(),
},
{ additionalProperties: false },
);
export type SupplyCoffinOpenRequestV1 = Static<typeof SupplyCoffinOpenRequestV1>;

export const ContainerOpenRequestV1 = Type.Object(
{
v: Type.Literal(1),
type: Type.Literal("container_open"),
entity_id: Type.Integer(),
},
{ additionalProperties: false },
);
export type ContainerOpenRequestV1 = Static<typeof ContainerOpenRequestV1>;

export const WorkbenchOpenRequestV1 = Type.Object(
{
v: Type.Literal(1),
type: Type.Literal("workbench_open"),
entity_id: Type.Integer(),
},
{ additionalProperties: false },
);
export type WorkbenchOpenRequestV1 = Static<typeof WorkbenchOpenRequestV1>;

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 | 🟡 Minor | ⚡ Quick win

为 entity_id 类字段补充非负约束。

SupplyCoffinOpenRequestV1.entity_id(Line 1188)、ContainerOpenRequestV1.entity_id(Line 1198)、WorkbenchOpenRequestV1.entity_id(Line 1208)、GiveDanToElderRequestV1.elder_entity_id(Line 1336)均使用裸 Type.Integer()

上游参照片段 container-interaction.tsStartSearchRequestV1.container_entity_id 使用 Type.Integer({ minimum: 0 })。这些新字段与既有 entity id 约定不一致,允许负数 ID 通过客户端 schema 校验。

请为这些字段补充 { minimum: 0 }

🛡️ 建议修复
 export const SupplyCoffinOpenRequestV1 = Type.Object(
   {
     v: Type.Literal(1),
     type: Type.Literal("supply_coffin_open"),
-    entity_id: Type.Integer(),
+    entity_id: Type.Integer({ minimum: 0 }),
   },
   { additionalProperties: false },
 );

 export const ContainerOpenRequestV1 = Type.Object(
   {
     v: Type.Literal(1),
     type: Type.Literal("container_open"),
-    entity_id: Type.Integer(),
+    entity_id: Type.Integer({ minimum: 0 }),
   },
   { additionalProperties: false },
 );

 export const WorkbenchOpenRequestV1 = Type.Object(
   {
     v: Type.Literal(1),
     type: Type.Literal("workbench_open"),
-    entity_id: Type.Integer(),
+    entity_id: Type.Integer({ minimum: 0 }),
   },
   { additionalProperties: false },
 );
 export const GiveDanToElderRequestV1 = Type.Object(
   {
     v: Type.Literal(1),
     type: Type.Literal("give_dan_to_elder"),
     pill_instance_id: Type.Integer({ minimum: 0, maximum: JS_SAFE_INTEGER_MAX }),
-    elder_entity_id: Type.Integer(),
+    elder_entity_id: Type.Integer({ minimum: 0 }),
   },
   { additionalProperties: false },
 );

Also applies to: 1331-1341

🤖 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 `@agent/packages/schema/src/client-request.ts` around lines 1184 - 1212, Update
the entity ID fields in SupplyCoffinOpenRequestV1, ContainerOpenRequestV1,
WorkbenchOpenRequestV1, and GiveDanToElderRequestV1 to use Type.Integer({
minimum: 0 }) instead of bare Type.Integer(), matching the existing non-negative
entity ID schema convention.

Comment thread agent/packages/schema/src/client-request.ts Outdated
Comment on lines +1382 to +1430
ZhenfaPlaceRequestV1,
ZhenfaTriggerRequestV1,
ZhenfaDisarmRequestV1,
QiScatterBeadUseRequestV1,
LearnSkillScrollRequestV1,
TechniqueScrollUseRequestV1,
InventoryMoveIntentRequestV1,
EquipFalseSkinRequestV1,
ForgeFalseSkinRequestV1,
InventoryDiscardItemRequestV1,
TreasureActivateRequestV1,
DropWeaponIntentRequestV1,
RepairWeaponIntentRequestV1,
PickupDroppedItemRequestV1,
RemainsLootRequestV1,
MineralProbeRequestV1,
FreshnessProbeRequestV1,
ApplyPillRequestV1,
SelfAntidoteRequestV1,
DuoSheRequestV1,
QiColorInspectRequestV1,
UseLifeCoreRequestV1,
JiemaiRequestV1,
ChargeCarrierRequestV1,
ThrowCarrierRequestV1,
AnqiContainerSwitchRequestV1,
UseQuickSlotRequestV1,
QuickSlotBindRequestV1,
SkillBarCastRequestV1,
SkillBarBindRequestV1,
SkillConfigIntentRequestV1,
CombatReincarnateRequestV1,
CombatTerminateRequestV1,
CombatCreateNewCharacterRequestV1,
StartExtractRequestV1,
CancelExtractRequestV1,
StartSearchRequestV1,
CancelSearchRequestV1,
SupplyCoffinOpenRequestV1,
ContainerOpenRequestV1,
WorkbenchOpenRequestV1,
ExternalContainerMoveRequestV1,
ExternalContainerCloseRequestV1,
LingtianStartTillRequestV1,
LingtianStartRenewRequestV1,
LingtianStartPlantingRequestV1,
LingtianStartHarvestRequestV1,
LingtianStartReplenishRequestV1,
LingtianStartDrainQiRequestV1,

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 定位 schema samples 目录及现有正反样例,核对是否覆盖新增 variant
fd -t d -i sample agent/packages/schema
rg -n "alchemy_learn_recipe_fragment|coffin_break|coffin_menu_reclaim|qi_scatter_bead_use|supply_coffin_open|container_open|workbench_open|external_container_move|external_container_close|lingtian_start|jiemai|craft_start|craft_cancel|give_dan_to_elder" agent/packages/schema -g '*sample*'

Repository: Kizunad/Bong

Length of output: 181


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "## repo files under agent/packages/schema"
git ls-files agent/packages/schema | sed -n '1,160p'

echo
echo "## sample files"
git ls-files agent/packages/schema/samples | sed -n '1,240p'

echo
echo "## sample files mentioning new-ish terms"
rg -n "QiScatterBeadUseRequest|inventory_move|external_container_(move|close)|lingtian_start|JiemaiRequest|SupplyCoffinOpen|ContainerOpen|WorkbenchOpen|Equipment|UseQuickSlot|QuickSlotBind|SkillBar|CombatReincarnate|StartExtract|CancelExtract|StartSearch|CancelSearch" agent/packages/schema/samples || true

echo
echo "## schema relevant new variants and lines"
rg -n "QiScatterBeadUseRequest|InventoryMoveIntentRequest|EquipFalseSkinRequest|ForgeFalseSkinRequest|InventoryDiscardItemRequest|TreasureActivateRequest|DropWeaponIntentRequest|RepairWeaponIntentRequest|PickupDroppedItemRequest|RemainsLootRequest|MineralProbeRequest|FreshnessProbeRequest|ApplyPillRequest|SelfAntidoteRequest|DuoSheRequest|QiColorInspectRequest|UseLifeCoreRequest|JiemaiRequest|ChargeCarrierRequest|ThrowCarrierRequest|AnqiContainerSwitchRequest|UseQuickSlotRequest|QuickSlotBindRequest|SkillBarCastRequest|SkillBarBindRequest|SkillConfigIntentRequest|CombatReincarnateRequest|CombatTerminateRequest|CombatCreateNewCharacterRequest|StartExtractRequest|CancelExtractRequest|StartSearchRequest|CancelSearchRequest|SupplyCoffinOpenRequest|ContainerOpenRequest|WorkbenchOpenRequest|ExternalContainerMoveRequest|ExternalContainerCloseRequest|LingtianStart" agent/packages/schema/src/client-request.ts agent/packages/schema -g '*.ts' || true

Repository: Kizunad/Bong

Length of output: 50368


补齐新增 ClientRequestV1 变体的正反 sample

client-request.ts 中的新增 variant 需要逐个补充 sample,包括 qi_scatter_bead_useequip_false_skinforge_false_skininventory_discard_itemtreasure_activatedrop_weapon_intentrepair_weapon_intentuse_quick_slotquick_slot_bindskill_bar_castskill_bar_bindskill_config_intentcombat_reincarnatecombat_terminatecombat_create_new_characterstart_extractcancel_extractsupply_coffin_opencontainer_openworkbench_openexternal_container_moveexternal_container_closelingtian_start_*jiemaicharge_carrierthrow_carrieranqi_container_switchstart_searchcancel_search 等;已有部分 inventory_move_intentcancel_search 样例仍需补齐完整正反对拍,并按 schema 变更要求同步到 samples。

🤖 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 `@agent/packages/schema/src/client-request.ts` around lines 1382 - 1430, 在
client-request.ts 的 ClientRequestV1 samples 定义中,为新增的各个 variant(包括
qi_scatter_bead_use、装备/背包/战斗、搜索、容器及 lingtian_start_* 等)逐一补充符合当前 schema 的正向和反向
sample;同时完善 inventory_move_intent 与 cancel_search 的完整正反对拍,并确保 samples 与最新 schema
变更保持一致。

Source: Coding guidelines

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 353c013b79dae604e2e075d4b8772dc27b0784fc
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Add negative contract tests for the four-way gate-matrix checker

scripts/tests/check_c2s_gate_matrix_test.py:14 · testing

The newly wired CI step runs the checker only against the repository's currently consistent files, while ParserTests contains only three parse_enum_variants tests. There is no negative end-to-end validation for the checker's claimed contract: mismatched sets, reordered variants, duplicate variants, non-contiguous matrix rows, missing TypeBox literals, or generated-schema drift. A concrete regression such as deleting the comparisons at scripts/check_c2s_gate_matrix.py:162-194 (or making main() return success after parsing) would still pass every unittest and would also pass the workflow's positive-only invocation at .github/workflows/e2e.yml:121, so CI would stop rejecting schema/matrix drift without any test failure.

Root cause: the test suite exercises only the rust enum parser and never drives the four-representation validation logic with deliberately inconsistent fixtures. the positive repository check proves only that today's inputs agree; it does not regression-protect the checker's failure behavior, which is the observable contract this pr adds to ci.

[major] Craft quantity schema omits the server's maximum of 64

agent/packages/schema/src/client-request.ts:1316 · schema-contracts

CraftStartRequestV1.quantity is constrained only with minimum: 1, so the TypeBox source and generated JSON Schema accept every positive JavaScript integer, including 65. The Rust mirror independently enforces MAX_CRAFT_QUANTITY_V1, which resolves to crate::craft::MAX_CRAFT_QUANTITY = 64, in deserialize_craft_quantity (server/src/schema/client_request.rs:735-748). Thus a payload such as {"type":"craft_start","v":1,"recipe_id":"x","quantity":65} validates against the IPC source of truth but is rejected by the server decoder. The new matrix/checker only compares discriminator sets/order and cannot detect this field-constraint drift.

Root cause: the newly added typebox contract omitted the server's canonical maximum craft quantity, leaving the producer/schema boundary broader than the rust serde consumer contract.

[major] Pin the new craft quantity boundary in schema tests

agent/packages/schema/src/client-request.ts:1316 · testing

CraftStartRequestV1 introduces the observable contract that quantity must be an integer >= 1, but the schema test suite contains no craft_start positive or boundary/negative pin. The new matrix checker cannot protect this constraint: typebox_wire_literals() extracts only each type literal and main() compares variant names/order, so changing this line to Type.Integer() (and regenerating the JSON schema) would still pass both the checker and its parser-only unit tests while accepting quantity: 0 and negative quantities. Existing ClientRequestV1 tests cover other request families but do not mention craft_start, so no surrounding test rejects that concrete regression.

Root cause: the pr adds request payload validation contracts but adds only union-membership/order checking, not dedicated schema behavior tests for the new fields and their boundaries. consequently, field-level constraints can regress consistently across source and generated artifacts without any test failing.

[major] zhenfa_place permits remote cross-dimension world mutation

agent/packages/schema/generated/client-request-v1.json:1452 · security

An authenticated client can now send the newly published {"v":1,"type":"zhenfa_place","x":...,"y":...,"z":...,"kind":...,"qi_invest_ratio":...} contract with arbitrary integer coordinates (the changed schema has no proximity or dimension field/constraint). The production dispatcher forwards those coordinates directly as ZhenfaPlaceRequest.pos (server/src/network/client_request_handler.rs:1630-1658) without resolving an authoritative target or checking the player's position/dimension. The consumer then checks inventory/realm/qi but never queries player position or dimension before place_zhenfa_anchor_block mutates the Overworld layer at the supplied position (server/src/zhenfa/mod.rs:1573-1782). Thus a player with the required item/resources can place array/trap blocks at arbitrary remote Overworld coordinates, including while located in another dimension. TypeBox/JSON validation does not prevent this because all integers are accepted, and the later domain checks do not add a spatial authority check. This PR exposes the request through the generated client schema, making the unsafe server-supported operation part of the distributed C2S contract.

Root cause: the generated trust-boundary contract publishes zhenfa_place with attacker-controlled world coordinates while the real production handler treats those coordinates as authoritative and performs mutation without a server-side same-dimension/reach gate.

[major] Matrix-only changes bypass the new C2S contract gate

.github/workflows/e2e.yml:119 · wiring

The new contract step runs only inside the e2e workflow, but the workflow's pull_request.paths filter (lines 16-23) includes agent, client, server, proto, scripts, compose, and the workflow itself, not docs/**. The checker reads docs/plan-refactor-c2s-gate-v1.md as one of its four authoritative inputs. A pull request that changes only that matrix can therefore add, remove, reorder, or rename a row without starting this workflow, so the newly added step at line 119 never validates the changed contract. No other workflow references check_c2s_gate_matrix.py.

Root cause: the checker is registered in a path-filtered workflow without adding its markdown matrix input to the workflow trigger paths, leaving one producer of the validated contract disconnected from ci.

[major] TypeBox union parser silently skips valid unrecognized members

scripts/check_c2s_gate_matrix.py:91 · schema-contracts

typebox_union_members() builds the authoritative union member list by retaining only lines that match a bare Identifier, and silently ignores every other non-empty line in the Type.Union block. For example, adding a valid inline member such as Type.Object({ v: Type.Literal(1), type: Type.Literal("new_request") }), while leaving the generated artifact stale causes that entire source member to be omitted; the collected members can still equal the unchanged Rust enum and generated JSON Schema, so main() exits successfully. The new parser tests exercise unsupported Rust syntax only and do not cover this TypeBox fail-open path. This defeats the PR's stated exhaustive TypeBox-source-to-Rust/matrix/generated-schema contract and allows the source-of-truth schema to drift without the CI gate detecting it.

Root cause: the typebox union parser treats unmatched syntax as ignorable whitespace instead of rejecting any non-comment, non-member content inside the authoritative union. consequently, valid typebox member expressions outside the parser's narrow bare-identifier form disappear from validation rather than failing closed.

[major] Trigger the matrix check when its documentation input changes

scripts/check_c2s_gate_matrix.py:12 · wiring

The checker makes docs/plan-refactor-c2s-gate-v1.md one of its four authoritative inputs via PLAN_PATH, but the e2e workflow's push and pull_request path filters include agent/**, client/**, server/**, proto/**, scripts/**, and the workflow itself, not docs/** (.github/workflows/e2e.yml:3-22). A PR that adds, removes, renumbers, or reorders only matrix rows therefore never starts the job containing python3 scripts/check_c2s_gate_matrix.py. The checker cannot catch that drift, despite the new plan explicitly claiming CI enforces the four-way contract. This is exposed by this PR because it newly makes a documentation file a required machine-checked contract input.

Root cause: the ci registration for the new checker is not triggered by changes to every file the checker consumes; specifically, its documentation matrix input is outside the workflow path filters.

[major] New C2S request constraints have no semantic regression tests

agent/packages/schema/generated/client-request-v1.json:4019 · testing

The PR adds new request-shape constraints, including craft_start.quantity with minimum: 1 here, plus complex external_container_move locations and bounded IDs, but the schema tests contain no references to craft_start, craft_cancel, external_container_move, external_container_close, lingtian_start_till, lingtian_start_renew, lingtian_start_planting, lingtian_start_harvest, lingtian_start_drain_qi, qi_scatter_bead_use, alchemy_learn_recipe_fragment, coffin_break, coffin_menu_reclaim, supply_coffin_open, container_open, workbench_open, or give_dan_to_elder. The generated-artifact freshness test only checks that committed JSON matches the TypeBox source, and the new C2S matrix checker only compares discriminator membership/order. Consequently, changing the TypeBox source to accept {v:1,type:"craft_start",recipe_id:"r",quantity:0} and regenerating this file would still satisfy those tests even though it violates the newly changed minimum-one contract; analogous shape regressions in the other new variants are also unpinned.

Root cause: the pr adds and regenerates many observable clientrequestv1 contracts without dedicated positive and negative schema tests for their fields, boundaries, required properties, and closed-object behavior. existing parity/freshness checks prove representation synchronization, not the semantic constraints of the new request variants.

[major] Make qi-scatter coordinates atomic in the wire schema

agent/packages/schema/generated/client-request-v1.json:1673 · strict-maintainability

The new generated qi_scatter_bead_use schema requires only item_instance_id; x, y, and z are independently optional at lines 1673-1681. Therefore a payload such as {"type":"qi_scatter_bead_use","v":1,"item_instance_id":7002,"x":1} is schema-valid. The real server consumer in server/src/network/client_request_handler.rs:1700-1708 accepts coordinates only when all three are present or all three are absent and otherwise drops the request without dispatching ScatterBeadUseRequest. Existing tests cover only all-present and all-absent payloads, so they do not prevent this producer/consumer mismatch. This contract is newly exposed by adding this request variant to the generated schema in the pull request.

Root cause: a logically atomic optional coordinate tuple was modeled as three independent optional properties, while the production handler enforces an all-or-none invariant that the schema does not express. as a result, schema-valid client requests can be silently rejected by the server.

[major] Invalid Lingtian modes are accepted and executed as manual

agent/packages/schema/src/client-request.ts:1245 · correctness

Both newly added Lingtian request schemas accept any string for mode. A payload such as {"type":"lingtian_start_harvest","v":1,"x":0,"y":64,"z":0,"mode":"invalid"} therefore passes the TypeBox contract. The production handler forwards parse_session_mode(&mode) at server/src/network/client_request_handler.rs:2584-2587, and that parser maps every value other than case-insensitive auto to SessionMode::Manual (server/src/network/client_request_handler.rs:13752-13756). The same conversion is used for till requests at lines 2540-2545. Consequently malformed but schema-valid modes start a manual gameplay session instead of being rejected; existing validation does not prevent this because the schema is open-ended and the parser deliberately defaults.

Root cause: the typebox source represents the closed manual/auto state as an unrestricted string while the server parser converts unknown values to a real state rather than rejecting them.

[major] Schema-valid partial bead coordinates are dropped by the server

agent/packages/schema/src/client-request.ts:942 · wiring

QiScatterBeadUseRequestV1 declares x, y, and z as three independently optional fields, so the TypeBox source of truth and generated schema accept payloads such as {"v":1,"type":"qi_scatter_bead_use","item_instance_id":1,"x":10}. The real server ingress deserializes that payload, but client_request_handler.rs:1700-1708 only accepts all three coordinates or none and drops every partial tuple as malformed before emitting ScatterBeadUseRequest. No TypeBox validation prevents this because each coordinate is individually optional. This mismatch is introduced by the newly added union member and makes a schema-valid request unreachable through the production consumer.

Root cause: the producer-side schema models a logically atomic optional coordinate tuple as three unrelated optional properties, while the server consumer enforces all-or-none tuple presence.

[major] Rust serde wire renames bypass the new matrix contract

scripts/check_c2s_gate_matrix.py:169 · strict-maintainability

main() constructs the Rust wire list as [snake_case(variant) for variant in enum] instead of deriving it from the enum's serde contract. For example, changing ClientRequestV1's existing #[serde(..., rename_all = "snake_case")] to rename_all = "camelCase" (or adding a variant-level #[serde(rename = "...")]) leaves the parsed variant names unchanged, so this checker can still report success while Rust deserializes a different discriminator from TypeBox and the generated JSON Schema. parse_enum_variants() explicitly skips every #[...] line, and the new tests only exercise variant shapes; they do not pin enum- or variant-level serde naming. Thus the new CI step does not enforce the claimed Rust-wire side of the four-way contract and accepts a concrete cross-stack schema break introduced after this PR.

Root cause: the checker treats a locally implemented pascalcase-to-snake_case conversion as the authoritative rust wire representation and discards the serde attributes that actually define that representation.

[major] Generated craft_start contract omits the server's quantity cap

agent/packages/schema/generated/client-request-v1.json:4018 · wiring

The changed craft_start.quantity schema only sets minimum: 1, so payload {"type":"craft_start","v":1,"recipe_id":"craft.example.herb_knife.iron","quantity":65} validates against this generated ClientRequestV1 artifact. The real Rust ingress rejects that same payload: server/src/schema/client_request.rs:734-748 binds the maximum to crate::craft::MAX_CRAFT_QUANTITY, which is 64 (server/src/craft/session.rs:34), and server/src/schema/client_request.rs:2407-2408 explicitly pins 65 as invalid. The separate craft contract also declares and tests the 64 cap (agent/packages/schema/src/craft.ts:21,115; agent/packages/schema/tests/craft.test.ts:54-62). Thus schema-driven producers or validators can accept a request that the production consumer drops during deserialization; none of the surrounding generated constraints supplies the missing upper bound.

Root cause: the clientrequestv1 craft-start representation was generated from craftstartrequestv1, whose quantity field carries only a minimum, instead of reusing the canonical craft quantity constraint with maximum 64. this leaves the generated producer/validator contract out of sync with the rust wire consumer and the existing craft schema.

[major] Add contract tests for the new NPC request schemas

agent/packages/schema/generated/client-request-v1.json:1349 · testing

The generated union adds the npc_inspect_request, npc_dialogue_choice, and npc_trade_request contracts here, including required fields and bounds such as npc_entity_id >= 0, nonblank/length-bounded option_id, safe item IDs, and nonblank/length-bounded requested_item_id (lines 1343-1432). A repository-wide search of schema tests finds no references to any of these three discriminator values. The newly added checker also cannot protect these constraints: generated_typebox_wires() reads only variant["properties"]["type"]["const"] (scripts/check_c2s_gate_matrix.py:111-116), and its unit tests cover only Rust enum parsing (scripts/tests/check_c2s_gate_matrix_test.py:14-46). Consequently, a concrete regression such as replacing npc_entity_id: Type.Integer({ minimum: 0 }) with an unbounded integer, dropping the nonblank pattern, or making one of these fields optional in the TypeBox source and regenerating this file would still satisfy the matrix checker and all current tests, while malformed NPC requests would become valid on the wire.

Root cause: the pr adds observable validation contracts for the npc request variants but provides no positive, boundary, or negative validation tests for them; the only new automated gate verifies discriminator membership and ordering rather than request payload shape.

[major] Failed external-container moves can permanently drop the player's item

agent/packages/schema/generated/client-request-v1.json:3208 · concurrency-atomicity

The newly generated external_container_move contract requires an attacker-controlled from location and explicitly accepts container, equip, or hotbar locations (lines 3208 and 3230 onward). On the reachable player-to-external-container path, handle_external_container_move does not verify that instance_id is at from: it scans every player container and removes the first matching item (server/src/network/client_request_handler.rs:18497-18506). If placing into the external container then fails, for example because the requested target cells are occupied, rollback only searches for the declared from when it is a container (18564-18577). Sending an item that is actually in main_pack with from:{"kind":"hotbar","index":0} and an occupied external target therefore removes the real item, fails placement, finds no rollback container, and returns after resync with the item absent from both inventories. Schema validation does not prevent this because the forged location is structurally valid, and the existing tests exercise valid container origins rather than this rollback case.

Root cause: the new move request exposes a non-atomic remove-then-place implementation whose rollback trusts the unverified client-supplied from location instead of retaining and restoring the actual source container/location from which the item was removed.

@Kizunad
Kizunad force-pushed the refactor/plan-refactor-c2s-gate-v1 branch from 353c013 to 46c8406 Compare August 3, 2026 07:16
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: infrastructure_failure
Reviewed head: 46c8406fdba53db7e75f2c45b57fdf3c0bfc7ee1
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

The review could not complete safely. No approval or code finding was inferred from the failed stages.

  • find:wiringinfra_error: 2 occurrence(s) in 2/5 failed batch(es); samples: batch-1: after 3 attempts: timeout after 300000ms | batch-2: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"}],"stderr":""} {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":524,"terminalReason":"api_error"}],"stderr":""}
  • find:strict-maintainabilityinfra_error: 1 occurrence(s) in 1/5 failed batch(es); samples: batch-2: after 3 attempts: timeout after 300000ms
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"}],"stderr":""}

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: infrastructure_failure
Reviewed head: 46c8406fdba53db7e75f2c45b57fdf3c0bfc7ee1
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

The review could not complete safely. No approval or code finding was inferred from the failed stages.

  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:0:attempt-1infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:1:attempt-2infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:2:attempt-3infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:3:attempt-4infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:4:attempt-5infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:0:attempt-6infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:1:attempt-7infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:2:attempt-8infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:3:attempt-9infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:4:attempt-10infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:0:attempt-11infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:1:attempt-12infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:2:attempt-13infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:3:attempt-14infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2:4:attempt-15infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f0ddeac80f5b7b092cde9b87de974bebb909a4bd8fc902cb23a9f91548dbe9ad:2infra_error: could not collect 5 valid votes after 15 attempts
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:0:attempt-1infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:1:attempt-2infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:2:attempt-3infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:3:attempt-4infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:4:attempt-5infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:0:attempt-6infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:1:attempt-7infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:2:attempt-8infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:3:attempt-9infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:4:attempt-10infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:0:attempt-11infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:1:attempt-12infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:2:attempt-13infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:3:attempt-14infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1:4:attempt-15infra_error: after 3 attempts: claude exited 1
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"result","subtype":"success","isError":true,"apiErrorStatus":403,"terminalReason":"api_error"}],"stderr":""}
  • validate:f6b47ce51871f40a32950fbb126f7a94b4c3b82b778c5970beb0a8a6d23199aa:1infra_error: could not collect 5 valid votes after 15 attempts

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 46c8406fdba53db7e75f2c45b57fdf3c0bfc7ee1
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Lingtian replenish required and closed-shape contract is omitted

agent/packages/schema/tests/client-request-new-contracts.test.ts:43 · testing

lingtian_start_replenish is the only newly added Lingtian request omitted from the requests table, so this suite never verifies its required x, y, z, and source fields or its closed top-level shape. The pre-existing tests in agent/packages/schema/tests/schema.test.ts:1802 and :1814 only validate one accepted source and one rejected source. An incorrect implementation that made a coordinate optional or allowed additional properties would therefore pass all current tests, despite the new request contract requiring all four fields and additionalProperties: false.

Root cause: the exhaustive-looking new-contract table skips one newly added union variant, and existing source-enum tests do not cover that variant's required-field or closed-object guarantees.

[major] Do not deduplicate generated schema wires before validating duplicates

scripts/check_c2s_gate_matrix.py:148 · schema-contracts

generated_typebox_wires() recursively extracts every generated type.const, then immediately returns list(dict.fromkeys(wires(document))) at scripts/check_c2s_gate_matrix.py:148. Consequently, a generated schema whose anyOf contains duplicate wire variants, for example alpha, alpha, beta, is reduced to alpha, beta before main() calls duplicates(generated). The checker therefore reports success whenever the deduplicated sequence matches TypeBox, despite the generated JSON Schema containing a duplicate union member.

Root cause: the generated-schema representation is deduplicated before validation, which destroys the evidence needed by the duplicate-contract check and allows malformed or drifted generated unions to pass the ci gate.

[major] Recipe-fragment request has no production producer

agent/packages/schema/src/client-request.ts:545 · wiring

AlchemyLearnRecipeFragmentRequestV1 is added to the authoritative ClientRequestV1 union with the wire discriminator alchemy_learn_recipe_fragment and an item_instance_id, and the Rust handler consumes that discriminator by emitting LearnRecipeFragmentIntent (server/src/network/client_request_handler.rs:884-896). However, there is no production client or agent producer for this discriminator: repository-wide client/agent matches find it only in the TypeBox declaration and schema tests. The actual alchemy fragment drop path still calls sendAlchemyLearnRecipe(id) (client/src/main/java/com/bong/client/alchemy/AlchemyScreen.java:724-732), whose sender emits the older alchemy_learn_recipe payload with a recipe string (client/src/main/java/com/bong/client/network/ClientRequestSender.java:428-430). Therefore dragging a recipe fragment never traverses the newly added instance-ID request path, so the server-side fragment consumer cannot be reached through the real UI; schema tests only inject the missing payload directly.

Root cause: the pr registers a new cross-stack request contract and server consumer but does not wire the existing alchemy fragment ui action to a matching client encoder/sender. the production producer remains connected to the legacy recipe-id request, leaving alchemy_learn_recipe_fragment disconnected outside tests.

[major] Pin entity IDs to the Rust i32 wire range

agent/packages/schema/tests/client-request-new-contracts.test.ts:15 · correctness

The new contract table supplies ordinary positive entity_id values for supply_coffin_open, container_open, and workbench_open, and the generic assertions only remove required fields or add an unknown property. Their new TypeBox schemas use unconstrained Type.Integer(), while the Rust mirror stores each field as i32 (server/src/schema/client_request.rs:520-537). Consequently a payload such as {v:1,type:"workbench_open",entity_id:2147483648} passes the TypeBox source-of-truth validator but cannot deserialize into the Rust request. Nothing in this test catches either signed-32-bit boundary, so the changed cross-stack contract remains green despite rejecting a schema-valid request at the server boundary.

Root cause: the new entity-id contracts were tested only for presence and closed shape, without pinning the numeric range shared by typebox and rust. the typebox producer contract is therefore wider than the rust i32 consumer contract.

[major] Reconcile the P0-only claim with runtime changes in this PR

docs/plan-refactor-c2s-gate-v1.md:21 · correctness

This plan says the PR “只完成 P0 docs closeout” and that runtime implementation remains blocked, and later repeats that P0 does not modify the runtime gate. However, this pull request also changes the production handler: server/src/network/client_request_handler.rs:1642-1657 now applies an Overworld/6-block Zhenfa placement gate, and the same changed file alters Lingtian mode rejection and external-container mutation handling. The matrix still labels ZhenfaPlace as “空间门缺” at line 177. Thus the document's phase status, scope, and current-state matrix do not describe the code being merged, and the selected runtime fixes bypass the plan's stated P1 prerequisite and atomic full-gate rollout.

Root cause: the plan was written against an earlier origin/main snapshot and marked p0 complete without reconciling concurrent production changes in the same pull request, leaving its claimed deliverables and implementation status stale.

[major] Cover Zhenfa placement gate through the client-request path

server/src/network/client_request_handler.rs:1642 · testing

The handler now rejects every zhenfa_place request unless the requester has position and dimension state, is in Overworld, and is within squared distance 36 of the block center (server/src/network/client_request_handler.rs:1642-1655). The repository-visible tests cover schema/protobuf round trips and Zhenfa event handling, but no test exercises a real client_request payload through this new handler branch with an accepted boundary case and rejected cross-dimension, non-Overworld, missing-component, and out-of-range cases. A concrete incorrect implementation that dispatches the request without this gate would therefore pass the current tests despite violating the changed authorization contract.

Root cause: a new security-sensitive c2s reach/dimension gate lacks production-path regression coverage. tests that deserialize or directly inject zhenfaplacerequest cannot detect omission or misordering of the handler-side gate.

[major] New request boundary constraints are not regression-pinned

agent/packages/schema/tests/client-request-new-contracts.test.ts:66 · testing

The new parameterized suite supplies only small valid values for every bounded identifier (item_instance_id, session_id, instance_id, hoe_instance_id, and pill_instance_id) and checks only required-field removal plus an extra top-level property. The later boundary test covers craft_start.quantity, modes, and one scatter-coordinate partial case, while the NPC test only checks the lower bound for npc_entity_id and whitespace strings. Consequently, concrete incorrect schema changes such as removing the nonnegative/JavaScript-safe upper bounds from the newly added instance/session IDs, or removing the 64/128-character upper bounds from NPC strings, would still pass this suite. No surrounding tests found cover those bounds for these new contracts. These constraints are observable IPC validation behavior introduced by this PR, and the project policy requires positive, maximum, and off-by-one negative pins for changed schema contracts.

Root cause: the shared test table validates presence and closed shapes but does not encode the numeric and string boundary constraints of the new request schemas, leaving multiple changed wire contracts unprotected against permissive regressions.

[major] Recipe-fragment learning has no production client producer

agent/packages/schema/generated/client-request-v1.json:722 · wiring

The changed generated union exposes alchemy_learn_recipe_fragment as a production C2S request requiring item_instance_id at lines 708-730, and the server has a real dispatch branch in server/src/network/client_request_handler.rs:884-896. However, a repository-wide search for alchemy_learn_recipe_fragment finds only the TypeBox declaration, schema tests, and server-side schema/handler tests; there is no client ClientRequestProtocol encoder, ClientRequestSender method, or UI/item-use handler that emits it. The existing alchemy screen only emits alchemy_learn_recipe (client/src/main/java/com/bong/client/alchemy/AlchemyScreen.java:731), while recipe-fragment client code in InventorySnapshotHandler.java:381,421 only renders fragment metadata. Current tests manually construct JSON or deserialize fixtures, so they do not exercise a production client-to-server path.

Root cause: the pr registers the new request in the shared schema and wires its server consumer, but omits the client-side producer and interaction entry point. consequently a normal client can never send the request, leaving recipe-fragment learning unreachable despite schema and synthetic server tests passing.

[major] Rust still accepts craft_start payloads rejected by the source schema

agent/packages/schema/src/client-request.ts:1345 · schema-contracts

A {"v":1,"type":"craft_start","recipe_id":"r"} request fails the changed TypeBox/generated contract because quantity is required here, but the Rust mirror still marks quantity with #[serde(default = "default_craft_quantity", ...)] (server/src/schema/client_request.rs:664-668), and its regression test explicitly expects the omitted field to deserialize as quantity 1 (server/src/schema/client_request.rs:2397-2401). The production handler then dispatches the resulting CraftStartIntent (server/src/network/client_request_handler.rs:2781-2796). Thus a client that talks directly to the server can submit a payload the IPC source of truth declares invalid and still start a craft; TypeBox tests cannot prevent this because they only exercise the agent schema.

Root cause: the pull request adds craft_start to the authoritative typebox union with mandatory quantity, but does not align the rust serde mirror, which preserves an omitted-field compatibility default. validation therefore differs across the typebox-to-rust boundary.

[major] New C2S contracts lack required boundary and per-variant regression tests

agent/packages/schema/src/client-request.ts:1228 · testing

The new schemas introduce bounded IDs such as ExternalContainerMoveRequestV1.session_id/instance_id (0..JS_SAFE_INTEGER_MAX) and several other newly bounded item IDs, but client-request-new-contracts.test.ts supplies only the interior value 1 and checks required fields/unknown top-level fields. It never checks 0, JS_SAFE_INTEGER_MAX, -1, or JS_SAFE_INTEGER_MAX + 1 for these new contracts. Consequently, concrete incorrect changes such as changing minimum to 1 or deleting maximum would leave the entire current TypeBox test suite green while changing which C2S payloads are accepted. The same suite also omits the newly added jiemai discriminator entirely, so an incorrect required field or an open object shape for that parameterless request would pass; the static matrix checker only compares discriminator membership/order and cannot detect field-constraint drift.

Root cause: the regression suite exercises representative happy-path values but does not pin the newly declared numeric boundaries or every new request contract. this leaves observable c2s validation behavior unprotected, contrary to the required saturated schema tests for boundary, off-by-one, invalid-input, and per-variant coverage.

[major] Test strict Lingtian mode rejection and dispatch

server/src/network/client_request_handler.rs:2556 · testing

Unknown Lingtian till and harvest modes are now required to be rejected before StartTillRequest/StartHarvestRequest is sent (server/src/network/client_request_handler.rs:2556-2569 and 2606-2619). The changed handler has no corresponding tests for manual, auto, case-insensitive forms, and invalid modes proving both event emission and no-op rejection. A regression that restores the previous fallback of treating every unknown mode as manual would pass the existing visible tests while violating the new request contract.

Root cause: the strict mode-parsing behavior changes an externally supplied request value from fail-open defaulting to rejection, but the event-dispatch boundary and invalid-input branch are unprotected by regression tests.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: a83703389412707cde0e744affce01bf303bea2f
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Stacked attributes bypass the variant wire-rename guard

scripts/check_c2s_gate_matrix.py:53 · schema-contracts

A future valid Rust variant declaration such as #[serde(rename = "renamed")] followed by #[cfg(feature = "x")] and then Variant, reaches this parser. Line 53 overwrites pending_attribute for every attribute, so by the time lines 56-57 inspect it, only the cfg attribute remains and the unsupported serde rename is accepted. The matrix is then compared against the Rust identifier Variant, while the actual wire discriminator is renamed, and main() exits successfully. Rust/Serde permits stacked attributes, no surrounding validation rejects this shape, and the added test covers only a serde rename immediately adjacent to the variant, so this incorrect implementation still passes the new suite.

Root cause: the parser stores only the most recent top-level variant attribute instead of accumulating and validating all attributes attached to the pending variant. this makes the new purported fail-closed wire-contract check order-dependent and allows a serde rename to be hidden by any following attribute.

[major] Inline-attributed variants are silently omitted from the gate matrix inventory

scripts/check_c2s_gate_matrix.py:52 · strict-maintainability

parse_enum_variants treats every depth-1 line whose stripped text starts with # as an attribute-only line and immediately continues. Rust permits an outer attribute and its variant on the same line, for example #[cfg(feature = "x")] NewRequest { v: u8 }, or #[serde(rename = "new_request")] NewRequest,. For that valid declaration, the checker never appends NewRequest; if the Markdown matrix also omits it, the set/order checks in main() pass even though ClientRequestV1 gained a variant. The new tests only cover attributes on their own line, so they do not expose this accepted omission. This defeats the changed plan's stated contract that future variants are automatically included and that unknown top-level syntax fails closed.

Root cause: the parser identifies an attribute solely by code.startswith("#") and neither parses nor rejects tokens following the closing attribute bracket, allowing a complete variant declaration on the same line to be silently discarded from the canonical enum inventory.

[minor] Plan edits unnecessarily trigger the full 45-minute E2E job

.github/workflows/e2e.yml:12 · performance

A documentation-only commit that changes docs/plan-refactor-c2s-gate-v1.md now matches the push and pull_request path filters at .github/workflows/e2e.yml:12 (and the corresponding PR filter). That launches the entire E2E Redis Smoke job, including Java/Gradle client tests, Node dependency installation and schema/agent tests, Rust release build and server tests, Redis setup, smoke E2E, and bot scenarios. The newly added matrix checker itself is a small static check and does not require this full environment, so each plan edit consumes a full GitHub-hosted runner and can add up to the job's 45-minute timeout to documentation iteration.

Root cause: the workflow path filter couples a planning-document change to the full end-to-end job instead of a focused matrix-contract job. because the workflow's existing job has broad build, integration, and bot stages, ordinary edits to this large plan cause substantial avoidable ci latency and runner consumption.

[major] Stacked attributes bypass the tested serde-rename rejection

scripts/tests/check_c2s_gate_matrix_test.py:53 · testing

The new negative test only covers a #[serde(rename = "other")] attribute immediately adjacent to the variant. A valid declaration such as #[serde(rename = "other")]\n #[doc = "variant docs"]\n Variant, is accepted by parse_enum_variants: each top-level attribute overwrites pending_attribute, so by the time Variant is parsed only the doc attribute remains and the serde rename is never rejected. main() then compares the Rust identifier Variant with the Markdown row and returns success even though the actual wire discriminator is other. The workflow's direct checker invocation does not compensate because it uses the same faulty parser. This contradicts the stated P0 contract that variant-level wire renames fail closed, and the current suite would pass this concrete incorrect implementation.

Root cause: the parser regression tests cover only one immediately adjacent attribute and omit valid stacked variant attributes. consequently, the checker's single-slot pending_attribute state can discard a previously seen serde rename, allowing wire-schema drift through the newly added ci contract.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: infrastructure_failure
Reviewed head: 7a55c8c0fc74985a9696121818ab58db52267626
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

The review could not complete safely. No approval or code finding was inferred from the failed stages.

  • find:correctnessinfra_error: 1 occurrence(s) in 1/2 failed batch(es); samples: batch-0: after 3 attempts: stdout limit exceeded (1000000 bytes)
    • diagnostic: {"events":[{"type":"system","subtype":"init","model":"cc-review","apiKeySource":"ANTHROPIC_API_KEY","claudeCodeVersion":"2.1.220"},{"type":"non_json_output"}],"stderr":""}

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: request_changes
Reviewed head: 7a55c8c0fc74985a9696121818ab58db52267626
Policy: project-review-policy.v2
Policy SHA-256: 3d5a5de3227540025123a516e2856b2c183ac6bf029b6863404d5d30d3f00c70

Validated findings

[major] Remove unrelated item textures from the P0 gate-plan closeout

client/src/main/resources/assets/bong-client/textures/gui/items/herb_knife_iron.png:1 · strict-maintainability

This PR's declared P0 deliverable says it "only adds docs, absorption evidence, and the enum/matrix static checker" (docs/plan-refactor-c2s-gate-v1.md:297-300), and the implementation-workflow section explicitly says this plan produces no textures (docs/plan-refactor-c2s-gate-v1.md:334). Nevertheless, it also adds four production client item textures: this file plus rabbit_pelt.png, yu_yi_zhi.png, and zhenfa_trap_iron.png. These assets are unrelated to the C2S gate matrix contract and are not mentioned, validated, or owned by any P0 acceptance item. They therefore expand the shipped client/resource surface in a plan-closeout PR without an explicit migration or current plan requirement.

Root cause: unrelated gameplay assets were bundled into a docs-and-static-checker phase even though the changed plan explicitly excludes texture output and defines a narrower p0 file/deliverable boundary. this obscures the authoritative purpose and validation surface of the change; the assets should be moved to the gameplay/asset change that owns and verifies them, or the plan and acceptance evidence must explicitly absorb that scope.

[major] Reject trailing variant declarations instead of silently skipping them

scripts/check_c2s_gate_matrix.py:98 · correctness

A valid Rust declaration can place more than one variant on a line, for example pub enum ClientRequestV1 {\n Alpha, Beta,\n}. At line 98 the suffix check accepts this line because the suffix starts with ,, but line 100 appends only the first regex capture (Alpha) and never verifies that the remainder contains no second declaration. With a matrix containing only Alpha, parse_enum_variants() returns ["Alpha"] and main() reports success even though the production enum also contains Beta. The same bypass applies to Alpha { v: u8 }, Beta, and tuple forms. Rustfmt would normally split these, but this checker runs as its own CI gate and the parser explicitly promises to fail closed on unsupported top-level enum syntax; no surrounding validation in this script detects the skipped variant, and the added tests cover unknown discriminant syntax but not multiple valid declarations on one line.

Root cause: the line-oriented parser validates only the first character of the text following the first variant name and then discards the rest of that line. it neither consumes one complete rust variant declaration nor rejects trailing non-comment tokens, so additional valid variants can be silently omitted from the inventory and therefore from the required gate matrix.

[major] Plan matrix parsing is mocked out of every regression test

scripts/tests/check_c2s_gate_matrix_test.py:105 · testing

The new test suite never exercises matrix_variants() against plan text or a temporary plan file. Instead, both the failure matrix and success test patch matrix_variants (lines 105 and 113) to return already-parsed values. A concrete incorrect implementation such as returning the current 104-row baseline unconditionally would therefore pass every unittest and the workflow's live checker invocation while the repository is initially in sync; after a plan row is renamed, omitted, duplicated, or malformed, that implementation would continue reporting success. The tests of main() only prove validation after parsing and cannot expose this broken document-to-row boundary.

Root cause: the plan-side parser, which is a changed observable contract of the new ci gate, has no direct regression test; all contract tests mock away that boundary. add parser tests using representative markdown for valid rows and malformed, missing, duplicate, reordered, and unrelated numbered tables, or an end-to-end test with controlled enum and plan files.

@Kizunad
Kizunad force-pushed the refactor/plan-refactor-c2s-gate-v1 branch 2 times, most recently from 68b7b93 to ecc44f0 Compare August 3, 2026 16:16
仅重放原 stripped head 相对基线的 P0 文档与检查器文件,保留主线 v2.1 审查策略。

Model: cc-sonnet-high
Co-Authored-By: Claude <noreply@anthropic.com>
@Kizunad
Kizunad force-pushed the refactor/plan-refactor-c2s-gate-v1 branch from ecc44f0 to 1d042f7 Compare August 3, 2026 18:05

@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: 1

🤖 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 `@scripts/check_c2s_gate_matrix.py`:
- Around line 92-94: Update the variant-level serde attribute validation in
scripts/check_c2s_gate_matrix.py:92-94 to fail closed for every #[serde(...)]
attribute, or at minimum reject alias, skip, other, rename, and variant-level
rename_all instead of only rename. Add rejection test cases for alias, skip,
other, and variant-level rename_all in
scripts/tests/check_c2s_gate_matrix_test.py:54-87.
🪄 Autofix (Beta)

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: aa7f0efc-4d0f-4851-9cac-5d20c4a285ea

📥 Commits

Reviewing files that changed from the base of the PR and between 353c013 and 1d042f7.

📒 Files selected for processing (7)
  • .github/workflows/e2e.yml
  • docs/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
  • docs/plans-skeleton/plan-refactor-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • scripts/check_c2s_gate_matrix.py
  • scripts/tests/check_c2s_gate_matrix_test.py
💤 Files with no reviewable changes (1)
  • docs/plans-skeleton/plan-refactor-c2s-gate-v1.md
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: e2e
🧰 Additional context used
📓 Path-based instructions (5)
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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • docs/plan-refactor-c2s-gate-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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
  • scripts/tests/check_c2s_gate_matrix_test.py
  • scripts/check_c2s_gate_matrix.py
  • docs/plan-refactor-c2s-gate-v1.md
**/*.{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:

  • scripts/tests/check_c2s_gate_matrix_test.py
  • scripts/check_c2s_gate_matrix.py
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Python 文件保存后应由 ruff 自动格式化,worldgen 修改应通过仓库规定的 regen、raster 校验和构建流程。

Files:

  • scripts/tests/check_c2s_gate_matrix_test.py
  • scripts/check_c2s_gate_matrix.py
🧠 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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-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-master-v1.md
  • docs/plans-skeleton/plan-refactor-wire-s2c-v1.md
🪛 LanguageTool
docs/plan-refactor-c2s-gate-v1.md

[uncategorized] ~301-~301: 您的意思是“"不"齐”?
Context: ...questV1变体穷尽声明Spec/NoGate(reason)`、补齐声明所需 authority/state adapter,并把 product...

(BU)


[uncategorized] ~306-~306: 您的意思是“"不"洞”?
Context: ...StateGateId adapter 保留单一事实源,不再承担任何枚举声明补洞。 - 测试抓手:enum↔registry 穷尽编译门继续常绿;...

(BU)


[uncategorized] ~332-~332: 您不可用“是”来表达个形容词。大部分人都平常用“很”来代替“是”。您是不是想表达"很纯"?
Context: ...。 ## §10 实施工作流 ### §10.1 适用边界 本 plan 是纯 server/network 逻辑重构,不产出 NBT、worldgen la...

(SHI_ADHECTIVE_ERROR)


[uncategorized] ~339-~339: 您的意思是“"不"枚”?
Context: ...变。 4. PR-4 / P3:删重复门禁并收敛领域 adapter,不补枚举声明欠账。 5. PR-5 / P4:bot/e2e 与符合条件的吸...

(BU)

🪛 Ruff (0.16.0)
scripts/tests/check_c2s_gate_matrix_test.py

[warning] 13-13: Assertion should be broken down into multiple parts

Break down assertion into multiple parts

(PT018)


[warning] 51-51: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)


[warning] 56-56: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)


[warning] 64-64: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)


[warning] 73-73: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)


[warning] 80-80: Use pytest.raises instead of unittest-style assertRaisesRegex

Replace assertRaisesRegex with pytest.raises

(PT027)

scripts/check_c2s_gate_matrix.py

[warning] 49-49: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 53-53: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 57-57: Too many branches (17 > 12)

(PLR0912)


[warning] 60-60: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 62-62: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 82-82: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 93-93: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 97-97: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 100-100: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 105-105: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 112-112: Avoid specifying long messages outside the exception class

(TRY003)


[warning] 124-124: Use a list comprehension to create a transformed list

(PERF401)


[warning] 126-126: Avoid specifying long messages outside the exception class

(TRY003)

🔇 Additional comments (5)
docs/plan-refactor-c2s-gate-v1.md (2)

328-332: 补齐开放问题和对应决议登记。

当前 plan 没有独立的“开放问题”章节。P0 决议不能替代未决项清单。

请列出未决项。若无未决项,请明确写出“无开放问题”和验证依据。实施前请为每项补充 §N.1 决议,并记录结论、实施方案、边界条件、文件:行号和 plan 章节双锚点。


352-354: 定义单次 /consume-plan 的自动闭环和停止条件。

§10.4 只定义了归档终态。它没有说明用户提交一次 /consume-plan 后会自动串行执行 PR-2 至 PR-5、等待门禁、merge 并归档。

请明确任一阶段失败、门禁未通过或暂停时立即停止,且不得归档。

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

25-25: 修正或说明 ServerDataType 的 144 项基线。

当前文本仍将 ServerDataType 写为 144 项。此前核验显示 agent schema 的 ServerDataType union 为 100 项。

如果 144 包含 Rust-only 或旁路变体,请说明来源和范围。否则,请使用当前 agent union 的实际数量。

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

101-101: LGTM!

.github/workflows/e2e.yml (1)

119-123: LGTM!

Comment on lines +92 to +94
if any("serde" in attribute and "rename" in attribute for attribute in pending_attributes):
raise RuntimeError("ClientRequestV1 variant-level serde rename is unsupported")
pending_attributes.clear()

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -U \
  '^\s*#\[serde\([^]]*(alias|skip|other|rename|rename_all)' \
  server/src/schema/client_request.rs

Repository: Kizunad/Bong

Length of output: 974


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- files ---\n'
git ls-files | rg '(^|/)check_c2s_gate_matrix(\.py)?$|(^|/)check_c2s_gate_matrix_test\.py$' || true

printf '\n--- checker outline/contents ---\n'
wc -l scripts/check_c2s_gate_matrix.py
sed -n '1,150p' scripts/check_c2s_gate_matrix.py

printf '\n--- tests relevant section ---\n'
sed -n '1,140p' scripts/tests/check_c2s_gate_matrix_test.py

printf '\n--- schema excerpts client_request.rs relevant ---\n'
sed -n '1,50p' server/src/schema/client_request.rs
sed -n '760,800p' server/src/schema/client_request.rs

Repository: Kizunad/Bong

Length of output: 241


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '--- files ---\n'
git lg ls-files | rg '(^|/)check_c2s_gate_matrix(\.py)?$|(^|/)check_c2s_gate_matrix_test\.py$' || true

printf '\n--- checker outline/contents ---\n'
wc -l scripts/check_c2s_gate_matrix.py
sed -n '1,150p' scripts/check_c2s_gate_matrix.py

printf '\n--- tests relevant section ---\n'
sed -n '1,140p' scripts/tests/check_c2s_gate_matrix_test.py

printf '\n--- schema excerpts client_request.rs relevant ---\n'
sed -n '1,50p' server/src/schema/client_request.rs
sed -n '760,800p' server/src/schema/client_request.rs

Repository: Kizunad/Bong

Length of output: 241


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- files ---'
git lg ls-files | rg '(^|/)check_c2s_gate_matrix(\.py)?$|(^|/)check_c2s_gate_matrix_test\.py$' || true

printf '%s\n' ''
printf '%s\n' '--- checker outline/contents ---'
wc -l scripts/check_c2s_gate_matrix.py
sed -n '1,150p' scripts/check_c2s_gate_matrix.py

printf '%s\n' ''
printf '%s\n' '--- tests relevant section ---'
sed -n '1,140p' scripts/tests/check_c2s_gate_matrix_test.py

printf '%s\n' ''
printf '%s\n' '--- schema excerpts client_request.rs relevant ---'
sed -n '1,50p' server/src/schema/client_request.rs
sed -n '760,800p' server/src/schema/client_request.rs

Repository: Kizunad/Bong

Length of output: 12717


🌐 Web query:

serde RenameAll applies to enum variant names only enum variant attributes alias other

💡 Result:

In Serde, the rename_all attribute acts differently depending on where it is applied, and it does not affect alias attributes [1][2][3]. When rename_all is used as a container attribute on an enum, it applies the specified casing convention (e.g., camelCase, snake_case) to the enum's variant names [1][4]. It does not affect the fields within those variants [4][5]. If you need to rename the fields of enum variants using a casing convention, you must either: 1. Apply rename_all individually to each struct-like variant [4][2]. 2. Use rename_all_fields if available/appropriate for your specific Serde version (note: rename_all_fields is specifically intended to apply a casing rule to the fields of variants) [1]. Regarding aliases, the alias attribute on an enum variant provides an alternative name for deserialization that is independent of any rename_all or rename attributes [6]. If a variant has a rename_all rule applied (via the container) or a rename attribute, the alias attribute still functions as an additional valid name for that variant during deserialization [6]. Explicit rename attributes on a specific variant will override the casing convention applied by rename_all at the container level [3]. Top results: [1][4][2][3][6]

Citations:


🌐 Web query:

serde enum variant syntax alias skip default other documentation example

💡 Result:

In Serde, enum variants support several attributes to control their serialization and deserialization behavior [1][2][3]. ### Alias The #[serde(alias = "name")] attribute allows you to deserialize a variant from a different name in addition to its Rust name [1][3]. This can be repeated to specify multiple aliases [1][3]. ### Skip The #[serde(skip)] attribute prevents a variant from being serialized or deserialized [1][3]. Note: In some data formats, especially non-self-describing ones like bincode, using #[serde(skip)] on an enum variant can lead to unexpected behavior or errors because the serializer still expects to be able to represent that variant [4][5]. ### Other The #[serde(other)] attribute is used during deserialization to catch any unknown variant values and map them to a specific "catch-all" variant [5]. It is typically applied to a unit variant in an enum. If the deserializer encounters a value that does not match any other defined variants, it will deserialize it into the variant marked with #[serde(other)] [5]. ### Default While #[serde(default)] is a standard field attribute for structs (to fill missing fields with Default::default()) [6][7], it is not a direct variant attribute for enums in the same way. However, for enum variants that contain fields (e.g., struct-like variants), you can apply #[serde(default)] to those individual fields to provide default values during deserialization [2][7]. ### Example Below is an example demonstrating the use of alias and other: rust use serde::{Deserialize, Serialize}; #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] enum Status { Active, #[serde(alias = "inactive", alias = "disabled")] Suspended, #[serde(other)] Unknown, } fn main() { // Deserializing "inactive" maps to Status::Suspended let s: Status = serde_json::from_str("\"inactive\"").unwrap(); println!("{:?}", s); // Output: Suspended // Deserializing "anything_else" maps to Status::Unknown due to #[serde(other)] let s: Status = serde_json::from_str("\"anything_else\"").unwrap(); println!("{:?}", s); // Output: Unknown } Top results for this query include the official Serde documentation for variant and field attributes [1][2][6].

Citations:


对所有 variant-level serde attribute 执行 fail-closed。

scripts/check_c2s_gate_matrix.py:92-94 只拒绝 renamealias 可以增加未登记的 wire 名称,skipother 也会改变反序列化集合;rename_all 也需要在 variant 上拒绝,因为它可能覆盖 enum 级别的 rename_all

  • 拒绝所有 variant-level #[serde(...)],或至少拒绝 aliasskipotherrename 以及 variant 级别的 rename_all
  • scripts/tests/check_c2s_gate_matrix_test.py:54-87 补充 aliasskipother 和 variant-level rename_all 的拒绝用例。
建议修复
-            if any("serde" in attribute and "rename" in attribute for attribute in pending_attributes):
-                raise RuntimeError("ClientRequestV1 variant-level serde rename is unsupported")
+            if any("serde" in attribute for attribute in pending_attributes):
+                raise RuntimeError("ClientRequestV1 variant-level serde attribute is unsupported")
📝 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
if any("serde" in attribute and "rename" in attribute for attribute in pending_attributes):
raise RuntimeError("ClientRequestV1 variant-level serde rename is unsupported")
pending_attributes.clear()
if any("serde" in attribute for attribute in pending_attributes):
raise RuntimeError("ClientRequestV1 variant-level serde attribute is unsupported")
pending_attributes.clear()
🧰 Tools
🪛 Ruff (0.16.0)

[warning] 93-93: Avoid specifying long messages outside the exception class

(TRY003)

📍 Affects 2 files
  • scripts/check_c2s_gate_matrix.py#L92-L94 (this comment)
  • scripts/tests/check_c2s_gate_matrix_test.py#L54-L87
🤖 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/check_c2s_gate_matrix.py` around lines 92 - 94, Update the
variant-level serde attribute validation in
scripts/check_c2s_gate_matrix.py:92-94 to fail closed for every #[serde(...)]
attribute, or at minimum reject alias, skip, other, rename, and variant-level
rename_all instead of only rename. Add rejection test cases for alias, skip,
other, and variant-level rename_all in
scripts/tests/check_c2s_gate_matrix_test.py:54-87.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Central review

Decision: infrastructure_failure
Reviewed head: 1d042f726787a9aaef722f85df2c2c0302c39df4
Policy: project-review-policy.v2
Policy SHA-256: ba594fb26bae4f60ebc26a470e1a7b55bfa5bf3ecb1313dd89c22721e772bcbf

The review could not complete safely. No approval or code finding was inferred from the failed stages.

  • find:correctnessschema_error: 1 occurrence(s) in 1/2 failed batch(es); samples: batch-0: schema validation failed: candidate-0: candidate taxonomy must exactly equal assigned dimension "correctness"

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