Repository navigation
fix(core): a persisted shape's accepted key set can shrink without anything noticing #4998
Description
Activity
take
I've taken this and started looking at the enumeration options.
I'd like to go with option 2, with two small additions so the snapshot is actually enforceable:
- Give
defineObjectShapea stable name/id so accepted-key snapshots can be compared per shape (not just an anonymous bag of sets). - Add an explicit register helper for the hand-built
ExactObjectShapevalues indurable-tool-result-projection.ts, so those 7 shapes are covered too.
Then commit a snapshot of each shape's accepted key set (
allowed ∪ retired) and fail when that set shrinks (growth stays allowed).Why not the others for a first cut:
- Option 1 forces exporting a lot of internals across ~24 files.
- Option 3 can cover everything via source extraction, but it's a heavier build-time step than we need if registry + names get us there.
Does this direction look right before I implement?
- Give
I've taken this and started looking at the enumeration options.
I'd like to go with option 2, with two small additions so the snapshot is actually enforceable:
- Give
defineObjectShapea stable name/id so accepted-key snapshots can be compared per shape (not just an anonymous bag of sets). - Add an explicit register helper for the hand-built
ExactObjectShapevalues indurable-tool-result-projection.ts, so those 7 shapes are covered too.
Then commit a snapshot of each shape's accepted key set (
allowed ∪ retired) and fail when that set shrinks (growth stays allowed).Why not the others for a first cut:
- Option 1 forces exporting a lot of internals across ~24 files.
- Option 3 can cover everything via source extraction, but it's a heavier build-time step than we need if registry + names get us there.
Does this direction look right before I implement?
One detail I noticed: the seven hand-built shapes are created inside decoder branches, so an import-based snapshot may miss them. Defining them as module-level constants could make enumeration more reliable.
Reacted by Changsu Seong- Give
@Totoro-qaq Good catch — agreed.
If those seven shapes stay inside decoder branches, an import-time snapshot/registry can miss them. I'll hoist them to module-level constants first, then register via the helper, so enumeration does not depend on hitting those branches.
Will implement option 2 with that adjustment unless anyone objects to the overall direction.
Reacted by Totoro
Removing a key from an exact object shape silently makes every stored record carrying that key undecodable. This is not hypothetical — it shipped in #4879 and was caught in review: dropping
partialOutputRetainedfromTURN_STATE_MESSAGE_SHAPEmade 67 of 67turn_staterows in a real user store faildecodeMessage, which would have left those Sessions unopenable after upgrade. Fixed in3a4371a74by adding aretiredlist todefineObjectShape.The invariant, stated once: an exact shape carries two contracts whose variance is opposite — what it emits may shrink freely, what it accepts may only grow.
allowedexpressed both as one set, so a type-level field deletion was silently translated into a persisted-data-level deletion.Why the type system made it worse rather than better:
Covers<Expected, Actual>requires the optional list to exactly cover the type's optional keys, so once the field leftTurnStateMessage, keeping the key inoptionalwas a compile error. The compiler pushed toward the breaking change.What is still unguarded:
retirednames the rule but nothing enforces it. There are 242defineObjectShapecall sites across 24 files;session.tsalone holds 26 shapes and exports none of them, so no test can enumerate what any shape accepts.durable-tool-result-projection.tsbuilds 7ExactObjectShapevalues structurally, bypassing theCovers<>check entirely. The epoch does not cover this either — it rejects an incompatible peer, not a row already on local disk.Wanted: a check that fails when a shape's accepted key set (
allowed ∪ retired) shrinks, and passes when it grows. The obstacle is enumeration, and it is a design decision rather than an implementation detail:defineObjectShape(no export churn, but a side effect on import and no help for the 7 hand-built shapes);A committed snapshot of the accepted sets, compared per shape, is what turns the rule from a comment into a failing test.
Not a good first issue: the fix touches a core schema authority and needs the enumeration decision made first.
简体中文
从一个精确对象形状里移除一个键，会静默地让每一条携带该键的已存记录无法解码。 这不是假设 —— 它在 #4879 里真的发生过，并在 review 中被抓到：把
partialOutputRetained从TURN_STATE_MESSAGE_SHAPE移除后，某个真实用户存储里 67 条turn_state全部decodeMessage失败，那会让这些 Session 在升级后打不开。已在3a4371a74通过给defineObjectShape增加retired列表修复。不变量，只说一遍：一个精确形状承载着两个变型方向相反的契约 —— 它发出什么可以自由收缩，它接受什么只能增长。
allowed把两者表达成了同一个集合，于是一次类型层面的字段删除，被静默翻译成了一次持久化数据层面的删除。类型系统为什么帮了倒忙：
Covers<Expected, Actual>要求 optional 列表恰好覆盖该类型的可选键；所以一旦字段离开TurnStateMessage，把这个键留在optional里反而是编译错误。编译器把人推向了那个破坏性改动。目前仍然没有守卫：
retired给规则起了名字，但没有任何机制强制它。仓库里有 242 处defineObjectShape调用、分布在 24 个文件；仅session.ts就有 26 个形状，一个都没有导出，所以没有任何测试能枚举出某个形状接受什么。durable-tool-result-projection.ts还以结构体方式手工构造了 7 个ExactObjectShape，完全绕过Covers<>检查。epoch 也顶不上 —— 它拒绝的是不兼容的对端，不是一条已经躺在本机磁盘上的行。想要的：一个检查 —— 当某个形状的接受键集合（
allowed ∪ retired）收缩时失败，增长时通过。障碍在枚举，而这是一个设计决策而非实现细节：defineObjectShape内部把形状注册进一个模块级注册表（无需导出、但引入了 import 副作用，且救不了那 7 个手工形状）；把接受集合提交为快照、逐形状比对，才是让这条规则从注释变成会失败的测试的那一步。
不是 good first issue：改动触及核心 schema 权威，且需要先把枚举方案拍板。