Conversation
|
CI attribution update for head
I replayed all three tests in an isolated worktree at current |
|
Baseline dependency follow-up:
Reference: #5251 |
cocolord
left a comment
There was a problem hiding this comment.
评审提交:377952efe6311b5eefecf32a80316e17f773e2fe。这是 policy-11 whole-PR、exact-head 评审。我检查了全部 26 个文件,沿着 CLI/Turn → Python admission → TypeScript owner → spend/void transaction → receipt/index → settlement readback/rolling window 走完正负路径,并独立运行了 focused tests、静态检查、base/head 失败归因和两个 readback 反例。写侧设计明显正向,但 readback 仍有可复现的跨实例缺口,因此不能 approve。
动机
这个改动解决的是明确且高价值的问题:同一个 goal_id 被重建后,旧 Goal A 的延迟 quota spend、replay、repair、void 或 settlement 证据不能落到新 Goal B。PR 给出的 before/after 可观察收益是成立的——写侧现在会在当前 GoalRef 和双锁 witness 不匹配时先拒绝,再把 B 的 GoalRef 写入 record、quota event、index row、transaction receipt 和响应;rolling-window 也按实例隔离。这里不是“为了抽象而抽象”,而是修复 append-only accounting 可能跨生命周期归属的 correctness boundary。
不过当前实现只完整关闭了 mutation/replay/void,未关闭 readback 和 inferred recovery。PR 正文与 RFC 宣称 quota_settlement 已 exact-owned/M3-qualified,比可执行行为更强;这个差距本身就是 blocker。另请把 Related to #4447 改为真正承载 Goal-instance/quota-owner 验收的 issue,或明确解释依赖关系:#4447 是 semantic vocabulary convergence tracker,不能单独作为这 1,981 行机制与 M3 qualification 的收益/验收来源。
改动思路
入口层在 quota spend-slot、void-slot 和 turn run-once 捕获 source-session 当前 GoalRef。quota_accounting_admission 按 run-index → source guard 顺序持锁并生成两个 cross-runtime witness;parseQuotaAccountingOwner 校验 GoalRef、profile、路径和 witness,withQuotaAccountingOwner claim 两把锁并调用既有 decideFirstPartyHostRuntime(require_current)。因此 stale A 在写入前失败,B 的 transaction owner 能覆盖 replay、prepared repair、void target 和最终 artifact commit。
accounting_artifact_transaction 把 GoalRef 纳入 request digest、receipt 校验、effect identity 冲突和所有持久 projection;goal_quota_with_spend_ledger 则让 current exact instance 只累计自己的行。这个方向复用了现有 first-party host 和 artifact transaction owner,机制成本虽大但与高严重度一致。问题出在 settlement_readback:它没有复用上述 owner,只接受一个 nullable goal_ref,且直到 identity 已解析后才在 findSpend 上做可选比较。
具体改动
阻塞问题
- [P1] 请在 identity inference 之前把 settlement readback 纳入 current exact-owner fence。
findSpend的goalRef === null || ...让 alias-only/未更新 caller 把任意 exact-source row 当作自己的;传入 GoalRef 时,也只比较持久行与请求值,不读取 registry、不 claim source guard、也不验证该 GoalRef 仍是 current。更早的resolveIdentity/inferPersistedIdentity完全看不到 GoalRef,会先从同 alias 的所有 run 中选 Turn。独立 exact-head 反例得到两个错误结果:一是无 GoalRef 请求直接返回 A 的 exactspend_run;二是当前 B 的infer_turn_instance_id请求在只有 A 记录时仍返回found=true,并选中 A 的 Turn 后报告spend_required。后者可让下游把 A 的 settlement tuple 带进一个由 B admission 合法通过、最终却 stamp 为 B 的新 spend。
最小修复应让 readback 使用与 spend/replay/void 相同的 typed alias/exact owner:source 请求在 guard 下验证 current GoalRef;alias 请求不能消费带 GoalRef 的记录;并在 resolveIdentity/inferPersistedIdentity 选择候选前应用 owner,而不是只过滤最终 spend row。请审计所有生产 read_heartbeat_settlement caller——当前 refresh-state、Todo completion、live decision、checkpoint、reward-memory、native-child 等多条路径仍未传 GoalRef。补四个 durable case:无 GoalRef + exact A、B inference + 仅 A、B 发布后 delayed A read、legacy alias + legacy row;前三者 fail closed/not found,最后一个保持原行为。完成前不要把 inventory 标为 m3_qualified。
关键代码讲解
quota_accounting_admission是 Python 锁顺序与 witness 生产者;source 必须有 exact GoalRef,legacy 保留原 index-lock 行为。withQuotaAccountingOwner是 TypeScript 写侧 authority owner;它校验并 claim 两个 witness,在require_current通过后才执行 transaction,finally 中按逆序释放。commitQuotaAccountingArtifactTransaction把 GoalRef 纳入 existing receipt、effect row、prepared repair 和四类 projection 的一致性检查,避免同 effect id 跨实例重放。evaluateQuotaSpendCommit/evaluateQuotaVoidCommit在同一 owner 下完成 lookup、target validation 和 commit;legacy wire-shape tests 证明未携带 GoalRef 的记录不新增字段。readQuotaSettlementFromRequest目前仅把request.goal_ref传给findSpend;它没有 current authority,也没有在 identity/event 候选阶段做 owner 隔离,这是 whole-PR 中唯一但关键的断口。
对主干的风险
独立验证结果:提交内 130 个 TypeScript quota tests、36 个 Python spend/void/rolling-window tests、10 个 owner-inventory/registry-census tests全部通过;TypeScript typecheck、Ruff 和 git diff --check 通过。GitHub 红项是 test-shard (3)、test-shard (4),聚合 pytest 和 merge-gate 随之失败;我在 immutable base 3ec049e138917a8cce4f84197ba196d26445b2b0 与 exact head 上重跑同三个 assertion,均为相同失败,因此不把它们归因于本 PR,也不把这点当作功能正确性的替代证据。
真正的主干风险是 silent scope escape:readback 不报 conflict,而是给出看似正常的 found/settled/spend_required。这会影响 quota 自身,也会影响消费 readback 的恢复、Todo、checkpoint 与 live-decision 路径。现有 submitted readback test 只覆盖“显式 A 匹配、显式 B 不匹配”,所以 130/130 仍无法捕捉 null-owner 和 pre-inference 两个反例。
代码量方面,26 文件共 +1,981/-176,其中约 980 行 production、946 行 tests;对高风险跨语言持久化 race 来说,测试占比和机制总体可接受。最高价值的收敛不是再加新层,而是让 readback 复用已经引入的 QuotaAccountingOwner,避免写侧 typed union、读侧 nullable wildcard 两套权威。domain wording 保持 Goal/Turn/quota 中性;没有把 advisory 当 obligation,但 RFC/inventory 的“qualified”是机器与 rollout 声明,必须等负路径真实通过。
语义与 CI 对齐
写侧的 alias | exact_source union、goal_instance_conflict 与 require_current 一致;readback 的 JsonObject | null 则把“legacy owner”与“未提供过滤条件”混为一类。CI 和 inventory tests 只验证声明结构与现有 positive cases,无法证明 scope 完整。应先把 readback 的状态规则对齐,再保留 exact_goal_ref_enforced/m3_qualified 声明。
我的整体评价
结论是 REQUEST_CHANGES。这项需求本身有清晰收益,写侧实现和大部分验证也值得保留;若只看 stale write、replay、repair、void 与 rolling-window,我会认为方向明显正向。当前不能 approve 的原因不是泛泛要求更多测试,而是核心承诺中的 readback/inference 有两个可复现反例,并且 inventory 已提前宣称 whole owner qualified。
请先让 readback 共享 current exact-owner 边界,补齐上述四个正反例,并修正或解释实际 task anchor。修复后重跑 130 TS quota、36 Python quota、owner inventory/census、typecheck、Ruff,以及这两个 reviewer counterexample;若 exact head 不再跨实例且 legacy parity 保持,我愿意重新审查。本次 review 不修改 PR,也不授权 merge。
English verdict: REQUEST_CHANGES on exact head 377952efe6311b5eefecf32a80316e17f773e2fe. The exact GoalRef write/replay/repair/void fence is a valuable and mostly well-tested improvement, and the current CI shard failures reproduce unchanged on the exact base. However, settlement readback still treats a missing GoalRef as a wildcard and applies GoalRef only after identity inference, so an unscoped caller can consume an exact A row and Goal B can infer Goal A's persisted Turn. Reuse the typed current-owner boundary for readback, add the negative cases, and keep quota_settlement unqualified until they pass.
| optionalString(run.goal_id) === identity.goal_id && | ||
| normalizeAgentId(run.agent_id) === identity.agent_id && | ||
| ( | ||
| goalRef === null |
There was a problem hiding this comment.
[P1] goalRef === null currently matches every spend row, including exact-source rows. Because resolveIdentity / inferPersistedIdentity run before this filter and the request carries no source-authority proof, an alias-only caller can consume Goal A state and current Goal B can select Goal A’s persisted Turn identity. Please reuse a typed alias/exact quota owner for readback, validate current source authority before inference, filter every identity/event candidate by that owner, and audit production callers that still omit GoalRef. Add durable cases for alias + exact A, B inference + only A, delayed A after B publication, and legacy alias + legacy row before retaining the m3_qualified claim.
There was a problem hiding this comment.
Addressed in ec7f51aaf on top of origin/main@0644abaaa.
settlement_readbacknow decodes the same typedalias | exact_sourceowner used by spend, replay, repair, and void.- It filters every event and run candidate before
resolveIdentityandinferPersistedIdentity. Alias requests accept only legacy rows with no GoalRef. - Exact reads claim both admission witnesses and verify
require_current; nested checkpoint, refresh-state, native-child, monitor, and prior-Turn recovery paths use borrowed or already-adopted admission without releasing the enclosing transaction. - I audited all 19 production
read_heartbeat_settlementcalls. Every source-aware call now carries bothregistry_pathandgoal_ref; already-locked callers also carrysource_admission. - The durable counterexamples now prove: alias plus exact A fails closed with
receipt_missingand no spend row; current B plus only A returnsfound=false; delayed A with a distinct Turn ID cannot replace current B inference; legacy alias plus legacy rows still settles. - The exact checkpoint test replaces A with B after context capture and confirms that A cannot append a checkpoint.
Validation at this head: full TypeScript 3,587 total, 3,557 passed, 30 optional PostgreSQL skipped; focused readback 88 passed; Python settlement compatibility 199 passed; exact checkpoint/native-child/external-delivery 34 passed; spend/void/rolling-window 36 passed; inventory/census 10 passed; typecheck, Ruff, docs governance, and the 260-site manifest check passed.
The PR body now uses #5206 as the task anchor. The only selected premerge failure reproduces unchanged on clean origin/main@0644abaaa (interaction-contract-state-machine-smoke.py), as does the separate repository-hygiene fixture finding.
|
#5344 also picked up the separate signed test-only fix |
|
Hi @Duang777, the DCO If the log confirms a missing |
|
Exact-head update: merged Exact-head validation now passes: full TypeScript 3,587 total, 3,557 passed, and 30 optional PostgreSQL skipped; combined Python target suite 279 passed; TypeScript typecheck; Ruff; docs governance; 260-site manifest check; and diff-driven standard premerge with 5 direct checks plus 19 of 19 selected checks. The separate repository-hygiene |
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
|
Hi @Duang777, the DCO If the log confirms a missing |
a23624c to
2d82f93
Compare
|
DCO history correction completed with the approved The PR now contains one authored contribution commit, Final-head local gates pass: TypeScript typecheck, the 260-site manifest check, 10 inventory/census tests, and standard premerge with 5 direct checks plus 19 of 19 selected checks. The three previous shard assertions now pass on current main, so #5344 is no longer a dependency for this PR. |
Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
|
CI follow-up for head
The commit is signed off and was pushed without rewriting history. |
Goal and delivered outcome
goal_id. A delayed Goal A request could spend, replay, repair, void, or read settlement state after the same alias had been recreated as Goal B.require_current, and retains the owner fence through readback, receipt, artifact, and index operations.a7e6b826a->70c74576b.Scope and continuation
source_session_v1.alias | exact_sourceowner before identity inference. Alias requests cannot consume exact rows. Exact requests verify the current source GoalRef under the same two-lock admission used by writes.quota_settlementinventory row. Unsupported providers and every other unqualified M3 owner remain blocked. The RFC activation hold andexecution_authority: falseremain unchanged.Validation
a23624c967785ffb7b3cee39c2052db08c0399ef70c74576b38cd85bccff6418aae5169c1c57ca5eunitpassednpm run test:control-plane: 3,587 tests, 3,557 passed, 30 optional PostgreSQL tests skipped, 0 failed.integrationpassedquota_settlement_readback.test.ts: 88 passed. Durable cases cover alias plus exact A, B inference with only A, delayed A after B publication using a distinct Turn ID, and legacy alias plus legacy rows.integrationpassedstaticpassed70c74576b, TypeScript control-plane typecheck, the 260-site project-registry I/O manifest check, and 10 inventory/census tests passed. Ruff, docs governance, andgit diff --checkalso passed for the feature diff.premergepassed70c74576b, diff-driven standard premerge ran 5 direct checks and all 19 selected risk, smoke, and public-boundary checks.repository_hygienebaseline failuretests/test_contract_scan_missing_roots.py:46private-IP fixture fails unchanged on the main baseline. This PR does not modify that file.The three previously failing shard assertions pass on current main. This change does not qualify PostgreSQL quota storage or any external provider path.
See validation disclosure guidance.
Frontend / visual evidence
Type of change
LoopX area
Technical direction
quota_settlementowner qualification.Shared-authority RFC fixture impact
Boundary checklist
none.Signed-off-bytrailer.