Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
branch: vlad/fix-deserialize
bound_by: init.behavior skill
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
🦉 needs your talons
├─ logs: .log/bhrain/review/2026-04-09T12-48-36-670Z
├─ review: .behavior/v2026_04_08.fix-deserialize/.reviews/5.1.execution.phase0_to_phaseN.v1.peer-review.decode-friction.md
└─ summary
├─ 5 blockers 🔴
└─ 0 nitpicks

---
# blocker.1: Inline array map in orchestrator

**rule**: .agent/repo=ehmpathy/role=mechanic/briefs/practices/code.prod/readable.narrative/rule.forbid.inline-decode-friction.md

**locations**:
- src/manipulation/serde/deserialize.ts

The deserialize function uses inline .map() to transform array elements in toHydrated. This requires mental simulation of the mapping operation, violating the rule against decode-friction in orchestrators. Orchestrators must read as narrative, delegating computation to named transformers. Extract this mapping logic to a named transformer function.

**snippet**:
```ts
if (Array.isArray(value)) return value.map((el) => toHydrated(el, context));
```

---

# blocker.2: Inline array find in orchestrator

**rule**: .agent/repo=ehmpathy/role=mechanic/briefs/practices/code.prod/readable.narrative/rule.forbid.inline-decode-friction.md

**locations**:
- src/manipulation/serde/deserialize.ts

The toHydratedObject function uses inline .find() to locate the domain object constructor from the context array. This array search operation requires mental simulation to understand which constructor is selected, constituting decode-friction in what appears to be orchestrator logic. Extract this lookup to a named transformer.

**snippet**:
```ts
const DomainObjectConstructor = context.with.find(
(thisConstructor) =>
(thisConstructor as typeof DomainObject).name === domainObjectClassName,
) as typeof DomainObject | undefined;
```

---

# blocker.3: Inline array every in hydrate function

**rule**: .agent/repo=ehmpathy/role=mechanic/briefs/practices/code.prod/readable.narrative/rule.forbid.inline-decode-friction.md

**locations**:
- src/instantiation/hydrate/hydrateNestedDomainObjects.ts

The hydrateNestedDomainObjects function uses inline .every() to validate that all declared nested domain object classes are based on DomainObject. This fold operation requires simulating the boolean reduction across the array, creating decode-friction. Since this function performs composition logic, it should be treated as an orchestrator and the validation extracted to a named transformer.

**snippet**:
```ts
const eachIsDomainObjectBased =
DeclaredNestedDomainObjectClassOptions.every(
(NestedDomainObject) =>
NestedDomainObject.prototype instanceof DomainObject, // https://stackoverflow.com/a/14486171/3068233
);
```

---

# blocker.4: Inline array map for class names

**rule**: .agent/repo=ehmpathy/role=mechanic/briefs/practices/code.prod/readable.narrative/rule.forbid.inline-decode-friction.md

**locations**:
- src/instantiation/hydrate/hydrateNestedDomainObjects.ts

In hydrateNestedDomainObjects, inline .map() is used to extract class names from the domain object options array. This transformation pipeline requires mental simulation of the mapping from class constructors to strings. Extract to a named transformer to maintain narrative flow in the orchestrator.

**snippet**:
```ts
const declaredNestedClassNameOptionsForProp =
DeclaredNestedDomainObjectClassOptions.map(
(ClassOption) => ClassOption.name,
);
```

---

# blocker.5: Positional array access in hydrate function

**rule**: .agent/repo=ehmpathy/role=mechanic/briefs/practices/code.prod/readable.narrative/rule.forbid.inline-decode-friction.md

**locations**:
- src/instantiation/hydrate/hydrateNestedDomainObjects.ts

The hydrateNestedDomainObjects function accesses the first element of DeclaredNestedDomainObjectClassOptions using positional index [0]. This requires understanding the array's implicit ordering, creating decode-friction. Replace with a named transformer that handles the selection logic without positional semantics.

**snippet**:
```ts
if (DeclaredNestedDomainObjectClassOptions.length === 1)
return DeclaredNestedDomainObjectClassOptions[0]!.build(prop); // this case is easy, since there's only one option
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
✨ all clear
├─ logs: .log/bhrain/review/2026-04-09T12-48-14-912Z
├─ review: .behavior/v2026_04_08.fix-deserialize/.reviews/5.1.execution.phase0_to_phaseN.v1.peer-review.failhides.md
└─ summary
├─ 0 blockers
└─ 0 nitpicks

---
# review complete

no issues found.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
✨ all clear
├─ logs: .log/bhrain/review/2026-04-09T13-13-07-692Z
├─ review: .behavior/v2026_04_08.fix-deserialize/.reviews/5.3.verification.v1.peer-review.contract-snapshots.md
└─ summary
├─ 0 blockers
└─ 0 nitpicks

---
# review complete

no issues found.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
✨ all clear
├─ logs: .log/bhrain/review/2026-04-09T13-14-03-146Z
├─ review: .behavior/v2026_04_08.fix-deserialize/.reviews/5.3.verification.v1.peer-review.external-contracts.md
└─ summary
├─ 0 blockers
└─ 0 nitpicks

---
# review complete

no issues found.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
branch: vlad/fix-deserialize
bound_by: route.bind skill
5 changes: 5 additions & 0 deletions .behavior/v2026_04_08.fix-deserialize/.route/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# ignore all except passage.jsonl and .bind flags
*
!.gitignore
!passage.jsonl
!.bind.*
30 changes: 30 additions & 0 deletions .behavior/v2026_04_08.fix-deserialize/.route/passage.jsonl
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{"stone":"1.vision","status":"blocked","blocker":"review.self","reason":"review.self required: has-questioned-requirements"}
{"stone":"1.vision","status":"blocked","blocker":"approval","reason":"wait for human approval"}
{"stone":"1.vision","status":"approved"}
{"stone":"1.vision","status":"passed"}
{"stone":"2.1.criteria.blackbox","status":"passed"}
{"stone":"2.2.criteria.blackbox.matrix","status":"passed"}
{"stone":"3.1.3.research.internal.product.code.prod._.v1","status":"passed"}
{"stone":"3.1.3.research.internal.product.code.test._.v1","status":"passed"}
{"stone":"3.3.1.blueprint.product.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-research-traceability"}
{"stone":"3.3.1.blueprint.product.v1","status":"blocked","blocker":"approval","reason":"wait for human approval"}
{"stone":"3.3.1.blueprint.product.v1","status":"approved"}
{"stone":"3.3.1.blueprint.product.v1","status":"passed"}
{"stone":"4.1.roadmap.v1","status":"passed"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-pruned-yagni"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"passed"}
{"stone":"5.3.verification.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-behavior-coverage"}
{"stone":"5.3.verification.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-contract-output-variants-snapped"}
{"stone":"5.3.verification.v1","status":"passed"}
{"stone":"4.1.roadmap.v1","status":"rewound"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"rewound"}
{"stone":"5.3.verification.v1","status":"rewound"}
{"stone":"4.1.roadmap.v1","status":"passed"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-pruned-yagni"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"blocked","blocker":"review.self","reason":"review.self required: behavior-declaration-coverage"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"passed"}
{"stone":"5.1.execution.phase0_to_phaseN.v1","status":"passed"}
{"stone":"5.3.verification.v1","status":"blocked","blocker":"review.self","reason":"review.self required: has-behavior-coverage"}
{"stone":"5.3.verification.v1","status":"blocked","blocker":"review.peer","reason":"blockers exceed threshold (2 > 0)"}
{"stone":"5.3.verification.v1","status":"blocked","blocker":"review.peer","reason":"blockers exceed threshold (1 > 0)"}
{"stone":"5.3.verification.v1","status":"passed"}
96 changes: 96 additions & 0 deletions .behavior/v2026_04_08.fix-deserialize/0.wish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
wish =

dis true? we should fix deserialize. always add withImmute, no reason not to


see below report from a mechanic

---


# defect: deserialize doesn't restore withImmute methods

## symptom

```
TypeError: domain.clone is not a function
```

when code expects `.clone()` on domain objects returned from `getAllDomains`.

## root cause

1. `castIntoDeclaredSquarespaceDomainRegistration` returns `new DeclaredSquarespaceDomainRegistration({...})` — a class instance
2. `withRemoteStateQueryCache` serializes via `serialize(output, { lossless: true })`
3. on cache hit, `deserialize(cached, { with: [DeclaredSquarespaceDomainRegistration, ...] })` restores class instances
4. **but** `deserialize` does NOT apply `withImmute`, so `.clone()` is unavailable

## the gap

`DomainEntity` class instances have static methods but NOT instance methods like `.clone()`.

`.clone()` is added by:
- `DomainObject.build(props)` — returns instance with `.clone()`
- `withImmute(instance)` — wraps instance with `.clone()`

`deserialize` uses `new DomainObject(props)` internally, not `.build()`, so no `.clone()`.

## evidence

from domain-objects readme:

> By default, .build will wrap your dobj instances `withImmute`, to give you immute operations such as `.clone(andSet?: Partial<T>)`

and:

> `withImmute` adds immute operators to your dobj, such as `.clone(update?: Partial<T>)`

## fix options

### option 1: fix in withRemoteStateQueryCache deserialize

```ts
deserialize: {
value: (cached) => {
const hydrated = deserialize(cached, {
with: [
DeclaredSquarespaceDomainRegistration,
// ...
],
});
// wrap with immute
if (Array.isArray(hydrated)) {
return hydrated.map((item) =>
item instanceof DomainObject ? withImmute(item) : item
);
}
return hydrated instanceof DomainObject ? withImmute(hydrated) : hydrated;
},
},
```

### option 2: fix in domain-objects deserialize

open issue/PR on domain-objects to have `deserialize` optionally apply `withImmute`.

### option 3: workaround at call site

use spread + new instead of `.clone()`:

```ts
// instead of
domain.clone({ isLocked: false })

// use
new DeclaredSquarespaceDomainRegistration({ ...domain, isLocked: false })
```

## current workaround

option 3 applied in `provision/usecase.transferout/resources.ts`.

## recommendation

option 1 is cleanest — fix in `withRemoteStateQueryCache` so all cached domain objects get `.clone()`.


62 changes: 62 additions & 0 deletions .behavior/v2026_04_08.fix-deserialize/1.vision.guard
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# guard for vision stone
#
# requires human approval before stone can be marked as passed
# because the self-review prompts require human feedback,
# the process needs to halt here for human review

judges:
- npx rhachet run --repo bhrain --skill route.stone.judge --mechanism approved? --stone $stone --route $route

reviews:
self:
- slug: has-questioned-requirements
say: |
a junior recently modified files in this repo. we need to carefully
review the vision due to this.

are there any requirements that should be questioned?

for each requirement, ask:
- who said this was needed? when? why?
- what evidence supports this requirement?
- what if we didn't do this — what would happen?
- is the scope too large, too small, or misdirected?
- could we achieve the goal in a simpler way?

challenge each requirement and justify why it belongs.

- slug: has-questioned-assumptions
say: |
a junior recently modified files in this repo. we need to carefully
review the vision due to this.

are there any hidden assumptions the junior took as requirements?

for each assumption, ask:
- what do we assume here without evidence?
- what evidence supports this assumption?
- what if the opposite were true?
- did the wisher actually say this, or did we infer it?
- what exceptions or counterexamples exist?

surface all hidden assumptions and question each one.

- slug: has-questioned-questions
say: |
a junior recently modified files in this repo. we need to carefully
review the vision due to this.

are there any open questions? triage them:

for each question, ask:
- can this be answered via logic now? if so, answer it now.
- can this be answered via extant docs or code now? if so, answer it now.
- should this be answered via external research later? if so, mark it for research.
- does only the wisher know the answer? if so, ask the wisher.

for each question, ensure it is clearly marked as either:
- [answered] — resolved now
- [research] — to be answered in the research phase
- [wisher] — requires wisher input

ensure they're enumerated within the vision under "open questions & assumptions"
Loading
Loading