Skip to content

core: cloneAsOverride keeps its input's deep-readonly type, so a Tenant/User override clone does not type-check as mutable #5257

Description

@os-support-ai

Filed unassigned from the implementation of #5160. Not fixed there — that card only changes documentation, never the export surface, and this is the export surface.

What was measured

packages/core/src/utils/freeze-schema.ts declares:

exportdeclarefunctiondefineSystemView<Textendsobject>(schema: T): SystemView<T>;exportdeclarefunctioncloneAsOverride<T>(view: T): T;

SystemView< T > is DeepReadonly< T > & { readonly [SYSTEM_VIEW_MARKER]?: true }. Because cloneAsOverride returns T unchanged, cloning a System View returns something still typed deep-readonly, even though the implementation produces a plain mutable object (structuredClone, or a JSON round-trip fallback) and deliberately drops the marker symbol.

So the documented override flow does not type-check. Measured against the built packages/core/dist/index.d.ts with the #5138 snippet gate, over packages/core/README.md:

packages/core/README.md:121:15 TS2339: Property 'push' does not exist on type 'readonly { readonly name: string; }[]'.

That line in the README is:

constdraft=cloneAsOverride(userListView)draft.columns.push({name: 'name'})// ✅ allowed

The comment is correct about runtime and wrong about the type. The neighbouring line one block up is the intended opposite and is correctly rejected:

packages/core/README.md:116:22 TS2339: Property 'push' does not exist on type 'readonly { readonly name: string; }[]'.

...which is the userListView.columns.push(...) // ❌ TypeError (strict mode) demonstration — that one is correct documentation, and freeze-schema.ts's own JSDoc shows the same line for the same purpose.

Why this was invisible until now

packages/core/README.md imported defineView, a name core does not export (it was renamed to defineSystemView in objectstack#4115). Under the missing import, userListView was an error type, so neither .push line was ever checked. Fixing the import in #5160 is what surfaced both. The diagnostic count on that document went up, not down — the two TS2339s are pre-existing and were masked, not introduced.

Whose defect this is

The boundary in #5160 was "documentation changes, never the export surface", so the README was left alone and this was filed instead. There is a real decision here rather than an obvious patch:

  • A.cloneAsOverride< T >(view: T): Mutable< T > — strip DeepReadonly on the way out, so the returned clone types the way it behaves. Needs a Mutable/DeepMutable inverse of the existing DeepReadonly, and a consumer sweep.
  • B. Leave the signature and change the README to annotate or cast the draft. Teaches a cast around a type that is simply wrong about its own value — a lenient read at the consumer, which AGENTS.md #0.1 rules out.
  • C. Leave both and declare the README block a fragment. Hides a real signature defect behind the gate's fragment marker.

A looks right; A is a contract change and wants a maintainer ruling.

Related

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions