Skip to content

Commit ff34c0c

Browse files
author
David Noble
committed
A decoded number with nothing to read it against is a load error, and a json.Number is falsy at zero
Phase 2 of the any-slot plan (#712). readAgainstField no longer hands a decoder artifact forward: a number in a slot with no action, in a slot no parameter declares, or one its parameter's type cannot hold is an error at load naming the slot, the parameter, and the declared type; the framework-slot reader refuses a non-text value the same way. scalarTruthy gains the json.Number guard rail, with the numeric cases split out to stay under the complexity gate. Two tests pin the refusals on hand-rewritten documents. Red set: the recovery-stack seam, phase 3. Claude-Session: https://claude.ai/code/session_01SqQVFZz7AdiLZkGoAcUqRW
1 parent f5c2435 commit ff34c0c

5 files changed

Lines changed: 179 additions & 31 deletions

File tree

‎docs/plans/fix/712-any-slot-types.md‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,9 @@ candidate's own URI and carries that catalog into the graph. Two fixture finding
199199
mint their own `anySlotResource` on a plain [ResourceBase]; and a fixture minted outside the catalog is exactly
200200
what Decision 8 refuses, so the old fixture asserted the defect. Red set now: the two phase-2 items only.
201201

202+
**Phase 2 complete 2026-09-06.** Red set: the recovery-stack seam alone, which is phase 3's (finding 6, `Result` at
203+
`recovery_stack.go`).
204+
202205
### State as of 2026-09-01
203206

204207

@@ -739,17 +742,22 @@ Two things Phase 1 changed about the plan itself:
739742
compares only the result's literal digits, read with `UseNumber` so the probe does not re-run the very
740743
float64 conversion it exists to detect.
741744

742-
### Phase 2: Resolve at the two authorities
745+
### Phase 2: Resolve at the two authorities -- COMPLETE (2026-09-06)
743746

744-
- [ ] Resolve an enveloped value at the decoder in `graph.go`, so no envelope survives `LoadGraph`. This is
745-
what makes a graph with a number in an `any` slot loadable again (finding 5).
746-
- [ ] Leave bare-literal resolution in `readAgainstField`, reading against the declared type. It must not move
747+
- [x] Resolve an enveloped value at the decoder in `graph.go`, so no envelope survives `LoadGraph`. This is
748+
what makes a graph with a number in an `any` slot loadable again (finding 5). Landed as `readSlotValue` on
749+
the assembly path (`assembleUnits` → `assembleNode` → `assembleBindings`), the decoder's own seam.
750+
- [x] Leave bare-literal resolution in `readAgainstField`, reading against the declared type. It must not move
747751
to the decoder; that would guess ahead of the field and regress #711.
748-
- [ ] Turn each of the four pass-through routes into an error rather than a guess, including the [Convert]
752+
- [x] Turn each of the four pass-through routes into an error rather than a guess, including the [Convert]
749753
failure that is deferred to dispatch today. The load-time message names the slot, the parameter, and the
750-
declared type.
751-
- [ ] Give `scalarTruthy` a `json.Number` case as defense in depth, so the truthiness inversion cannot recur
752-
if a new decoder path appears. The resolution above is the fix; this is the guard rail.
754+
declared type. `readAgainstField` returns an error on the three reachable routes (the fourth cannot arise);
755+
the framework-slot reader refuses a non-text value the same way. Pinned by
756+
`TestLoadGraph_ABareNumberInAnUndeclaredSlotIsRefused` and `TestLoadGraph_ANumberAParameterCannotHoldIsRefusedAtLoad`,
757+
both on documents rewritten by hand, since the serializer never writes a bare number where it would be refused.
758+
- [x] Give `scalarTruthy` a `json.Number` case as defense in depth, so the truthiness inversion cannot recur
759+
if a new decoder path appears. The resolution above is the fix; this is the guard rail. `numberTruthy`;
760+
the numeric cases moved to `numericTruthy` to keep the switch under the complexity gate.
753761

754762
### Phase 3: Write the envelope
755763

‎pkg/op/any_slot_types_test.go‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -408,3 +408,97 @@ func newAnySlotResource(runtimeEnvironment *RuntimeEnvironment, identity any) (R
408408
func init() {
409409
AnnounceResource(reflect.TypeFor[*anySlotResource](), newAnySlotResource, nil)
410410
}
411+
412+
// typedSlotFixture carries a method with a declared `bool` parameter, for the load-time refusals of #712 phase 2.
413+
type typedSlotFixture struct{ ProviderBase }
414+
415+
// Take accepts a flag; the method does nothing with it.
416+
//
417+
// Parameters:
418+
// - `flag`: the declared-`bool` parameter the refusal tests write a number into.
419+
//
420+
// Returns:
421+
// - `error`: always nil.
422+
func (p *typedSlotFixture) Take(flag bool) error { return nil }
423+
424+
func init() {
425+
AnnounceProvider(reflect.TypeFor[typedSlotFixture](), NewProviderFlags(SurfaceWorkflow, PlacementQualified),
426+
func(runtimeEnvironment *RuntimeEnvironment) (any, error) {
427+
return &typedSlotFixture{ProviderBase: NewProviderBase(runtimeEnvironment)}, nil
428+
},
429+
map[string]MethodMetadata{
430+
"Take": {ParameterNames: []string{"flag"}},
431+
})
432+
}
433+
434+
// loadWithSlotRewritten serializes a one-node graph, rewrites one slot's document form by textual replacement, and
435+
// loads the result. The rewrite stands in for a hand-edited or foreign document, which is the only way a bare
436+
// number reaches a slot the serializer would have enveloped.
437+
func loadWithSlotRewritten(t *testing.T, graph *Graph, from, to string) error {
438+
t.Helper()
439+
document := string(serializeGraph(t, graph, "json"))
440+
if !strings.Contains(document, from) {
441+
t.Fatalf("the document does not contain %q:\n%s", from, document)
442+
}
443+
_, err := LoadGraph(formatIdentityEnvironment(t), []byte(strings.Replace(document, from, to, 1)), "json")
444+
return err
445+
}
446+
447+
// TestLoadGraph_ABareNumberInAnUndeclaredSlotIsRefused pins #712 phase 2's second route: a number in a slot no
448+
// parameter declares has nothing to be read against, and the loader says so rather than handing a decoder
449+
// artifact forward.
450+
func TestLoadGraph_ABareNumberInAnUndeclaredSlotIsRefused(t *testing.T) {
451+
452+
action, err := ReceiverRegistry().BuildAction("anySlotFixture.keep")
453+
if err != nil {
454+
t.Fatalf("BuildAction: %v", err)
455+
}
456+
node, err := NewNode(NewNodeSpec().WithID("keep").WithAction(action).
457+
WithSlot("value", NewImmediateBinding("kept")).
458+
WithSlot("extra", NewImmediateBinding(int64(5))))
459+
if err != nil {
460+
t.Fatalf("NewNode: %v", err)
461+
}
462+
graph, err := NewGraph(NewGraphSpec().WithOrigin(NewOriginBase("test", "home", NewAnnotationMap(nil))).WithUnits(node))
463+
if err != nil {
464+
t.Fatalf("NewGraph: %v", err)
465+
}
466+
467+
err = loadWithSlotRewritten(t, graph, `{"$int64":"5"}`, "5")
468+
if err == nil {
469+
t.Fatal("LoadGraph accepted a bare number in a slot no parameter declares; want a refusal")
470+
}
471+
for _, want := range []string{`"extra"`, "keep", "nothing to read it against"} {
472+
if !strings.Contains(err.Error(), want) {
473+
t.Errorf("refusal %q does not name %s", err, want)
474+
}
475+
}
476+
}
477+
478+
// TestLoadGraph_ANumberAParameterCannotHoldIsRefusedAtLoad pins #712 phase 2's third route: a number the declared
479+
// type cannot hold fails at load, naming the slot, the parameter, and the type -- not at dispatch.
480+
func TestLoadGraph_ANumberAParameterCannotHoldIsRefusedAtLoad(t *testing.T) {
481+
482+
action, err := ReceiverRegistry().BuildAction("typedSlotFixture.take")
483+
if err != nil {
484+
t.Fatalf("BuildAction: %v", err)
485+
}
486+
node, err := NewNode(NewNodeSpec().WithID("take").WithAction(action).WithSlot("flag", NewImmediateBinding(true)))
487+
if err != nil {
488+
t.Fatalf("NewNode: %v", err)
489+
}
490+
graph, err := NewGraph(NewGraphSpec().WithOrigin(NewOriginBase("test", "home", NewAnnotationMap(nil))).WithUnits(node))
491+
if err != nil {
492+
t.Fatalf("NewGraph: %v", err)
493+
}
494+
495+
err = loadWithSlotRewritten(t, graph, `"value":true`, `"value":5`)
496+
if err == nil {
497+
t.Fatal("LoadGraph accepted a number in a bool slot; want a refusal at load")
498+
}
499+
for _, want := range []string{`"flag"`, "Take", "bool"} {
500+
if !strings.Contains(err.Error(), want) {
501+
t.Errorf("refusal %q does not name %s", err, want)
502+
}
503+
}
504+
}

‎pkg/op/framework_slots.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package op
55

66
import (
77
"encoding"
8+
"fmt"
89
"reflect"
910
)
1011

@@ -39,30 +40,30 @@ func frameworkSlotType(name string) (reflect.Type, bool) {
3940
// readFrameworkSlot parses a bare document value back through a framework slot's declared type.
4041
//
4142
// The declared types marshal as text (a policy writes its name), so a string reads back through
42-
// [encoding.TextUnmarshaler]. A value of any other shape, or a type without a text form, passes through unchanged and
43-
// fails at its consumer, which names the slot.
43+
// [encoding.TextUnmarshaler]. Anything else is an error naming the slot and the type: no guessing (#712 phase 2).
4444
//
4545
// Parameters:
4646
// - `value`: the decoded document value.
4747
// - `declared`: the slot's framework-declared type.
4848
//
4949
// Returns:
50-
// - `any`: the typed value, or `value` unchanged.
51-
func readFrameworkSlot(value any, declared reflect.Type) any {
50+
// - `any`: the typed value.
51+
// - `error`: a value that is not text, a declared type without a text form, or text the type refuses.
52+
func readFrameworkSlot(value any, declared reflect.Type) (any, error) {
5253

5354
text, isString := value.(string)
5455
if !isString {
55-
return value
56+
return nil, fmt.Errorf("declared %s by the framework, and the document holds a %T, not its text form", declared, value)
5657
}
5758

5859
target := reflect.New(declared)
5960
unmarshaler, ok := target.Interface().(encoding.TextUnmarshaler)
6061
if !ok {
61-
return value
62+
return nil, fmt.Errorf("declared %s by the framework, a type with no text form to read %q back through", declared, text)
6263
}
6364
if err := unmarshaler.UnmarshalText([]byte(text)); err != nil {
64-
return value
65+
return nil, err
6566
}
6667

67-
return target.Elem().Interface()
68+
return target.Elem().Interface(), nil
6869
}

‎pkg/op/node.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func readSlotValue(value any, method *Method, name string, catalog *ResourceCata
371371
return decodeTypeWrapper(value, catalog)
372372
}
373373

374-
return readAgainstField(value, method, name), nil
374+
return readAgainstField(value, method, name)
375375
}
376376

377377
// assembleNode constructs a [*Node] from a [nodeData] payload during deserialization.
@@ -440,27 +440,31 @@ func assembleNode(p *nodeData, catalog *ResourceCatalog) (*Node, error) {
440440
// - `name`: the slot name, which is the parameter name.
441441
//
442442
// Returns:
443-
// - `any`: the value read against the field, or the value unchanged.
444-
func readAgainstField(value any, method *Method, name string) any {
443+
// - `any`: the value read against the field; a non-number passes through unchanged.
444+
// - `error`: a number with nothing to read it against, or one its parameter's type cannot hold.
445+
func readAgainstField(value any, method *Method, name string) (any, error) {
445446

446447
if declared, ok := frameworkSlotType(name); ok {
447448
return readFrameworkSlot(value, declared)
448449
}
449-
if method == nil || !isDecodedNumber(value) {
450-
return value
450+
if !isDecodedNumber(value) {
451+
return value, nil
452+
}
453+
// From here the value is a decoder artifact -- a number whose type only the field can supply -- and every
454+
// route that once handed it forward unchanged is an error instead (#712 phase 2): no guessing.
455+
if method == nil {
456+
return nil, fmt.Errorf("a number (%T) with no action to declare the slot: nothing to read it against", value)
451457
}
452-
453458
parameter, declared := method.ParameterByName(name)
454459
if !declared || parameter.Type == nil {
455-
return value
460+
return nil, fmt.Errorf("a number (%T) in a slot %s declares no parameter for: nothing to read it against", value, method.Name())
456461
}
457-
458462
converted, err := Convert(nil, value, parameter.Type)
459463
if err != nil {
460-
return value
464+
return nil, fmt.Errorf("parameter %q of %s is declared %s, and the document's number cannot be read as one: %w",
465+
parameter.Name, method.Name(), parameter.Type, err)
461466
}
462-
463-
return converted
467+
return converted, nil
464468
}
465469

466470
// isDecodedNumber reports whether a decoded slot value is a number a codec may have mistyped.

‎pkg/op/truthiness.go‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33

44
package op
55

6-
import "reflect"
6+
import (
7+
"encoding/json"
8+
"reflect"
9+
)
710

811
// IsTruthy reports whether `value` is truthy under Python / Starlark truth semantics.
912
//
@@ -56,10 +59,28 @@ func IsTruthy(value any) bool {
5659
// - `bool`: the truthiness, when `value` is a built-in scalar.
5760
// - `bool`: true when `value` was a built-in scalar.
5861
func scalarTruthy(value any) (truthy, isScalar bool) {
59-
6062
switch v := value.(type) {
6163
case bool:
6264
return v, true
65+
case string:
66+
return v != "", true
67+
case json.Number:
68+
return numberTruthy(v)
69+
}
70+
return numericTruthy(value)
71+
}
72+
73+
// numericTruthy is [scalarTruthy]'s case for the Go numeric types, exactly typed: a named type with a numeric
74+
// underlying kind is not a scalar here, as it never was.
75+
//
76+
// Parameters:
77+
// - `value`: the value under test.
78+
//
79+
// Returns:
80+
// - `truthy`: whether the number is non-zero.
81+
// - `isScalar`: false when `value` is not one of the built-in numeric types.
82+
func numericTruthy(value any) (truthy, isScalar bool) {
83+
switch v := value.(type) {
6384
case int:
6485
return v != 0, true
6586
case int8:
@@ -84,8 +105,28 @@ func scalarTruthy(value any) (truthy, isScalar bool) {
84105
return v != 0, true
85106
case float64:
86107
return v != 0, true
87-
case string:
88-
return v != "", true
108+
}
109+
return false, false
110+
}
111+
112+
// numberTruthy is [scalarTruthy]'s case for a decoder artifact.
113+
//
114+
// Defense in depth (#712 phase 2): the decoder should never let a json.Number reach a truthiness check, but if a
115+
// new path does, a zero is falsy however it was decoded.
116+
//
117+
// Parameters:
118+
// - `number`: the undecoded number.
119+
//
120+
// Returns:
121+
// - `truthy`: whether the number is non-zero.
122+
// - `isScalar`: false when the number parses as neither an integer nor a float.
123+
func numberTruthy(number json.Number) (truthy, isScalar bool) {
124+
125+
if integer, err := number.Int64(); err == nil {
126+
return integer != 0, true
127+
}
128+
if float, err := number.Float64(); err == nil {
129+
return float != 0, true
89130
}
90131

91132
return false, false

0 commit comments

Comments
 (0)