Skip to content

Commit fb7d2e0

Browse files
author
gluck
committed
outfit: a skill is a .md file; form: terminal comes out
The loader read every file in the skills directory and named it by chopping its last suffix. filepath.Ext("howto.md.pre-bundle-20260629") is ".pre-bundle-20260629", so a backup left beside the real howto loaded as a skill called "howto.md". Any stray .bak or .swp did the same, under whatever name the chop produced. That invented collision is what node.terminal existed to preserve: provenance threaded through the tree, the diff and the wire so skills.howto.md could sit beside skills.howto rather than inside it. With the loader fixed the case does not arise, and the guard is again what it should be -- a leaf stands where a branch is wanted, so the rest of the key stays whole beside it. succession is the real prefix collision and needs no provenance: its value is prose, so it is a leaf and the guard fires. 115 lines removed. The fixture carried the invented skill; it is regenerated without it.
1 parent e83c2ab commit fb7d2e0

8 files changed

Lines changed: 216 additions & 243 deletions

File tree

‎api/form/form.go‎

Lines changed: 2 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,6 @@ func diffTrees(prev, next ptree) Patch {
229229
out.New = map[string]bool{}
230230
}
231231
out.Update[n.key], out.New[n.key] = child, true
232-
if n.terminal {
233-
if out.Term == nil {
234-
out.Term = map[string]bool{}
235-
}
236-
out.Term[n.key] = true
237-
}
238232
}
239233
return true
240234
}
@@ -249,12 +243,6 @@ func diffTrees(prev, next ptree) Patch {
249243
out.Update = map[string]Patch{}
250244
}
251245
out.Update[n.key] = child
252-
if n.terminal {
253-
if out.Term == nil {
254-
out.Term = map[string]bool{}
255-
}
256-
out.Term[n.key] = true
257-
}
258246
}
259247
case !old.branch && !n.branch:
260248
if !old.value.Equal(n.value) {
@@ -337,9 +325,7 @@ func applyToTree(t ptree, p Patch) (ptree, bool) {
337325
if cur := lookupExact(t.root, k); cur != nil && !cur.branch && cur.value.Equal(v) {
338326
continue
339327
}
340-
e := leafOrBranch(k, v)
341-
e.terminal = true
342-
t = ptree{root: setEntry(t.root, e)}
328+
t = ptree{root: setEntry(t.root, leafOrBranch(k, v))}
343329
changed = true
344330
}
345331
for k := range p.Object.Delete {
@@ -349,22 +335,6 @@ func applyToTree(t ptree, p Patch) (ptree, bool) {
349335
}
350336
for k, child := range p.Object.Update {
351337
cur := lookupExact(t.root, k)
352-
if cur != nil && cur.terminal {
353-
// The key ends here for whoever wrote it, so a patch that would
354-
// reach through it is naming siblings rather than fields. A flat
355-
// patch has no base and cannot know that when it splits its keys.
356-
// Only a suffix the node does not already hold is a sibling.
357-
// A field it does hold is an edit of that field.
358-
if flat, ok := flattenUnder(child); ok && allForeign(cur, flat) {
359-
for suffix, v := range flat {
360-
e := leafOrBranch(k+"."+suffix, v)
361-
e.terminal = true
362-
t = ptree{root: setEntry(t.root, e)}
363-
changed = true
364-
}
365-
continue
366-
}
367-
}
368338
var kids ptree
369339
leafHere := cur != nil && !cur.branch
370340
if cur != nil && cur.branch {
@@ -383,8 +353,7 @@ func applyToTree(t ptree, p Patch) (ptree, bool) {
383353
if !subChanged && cur != nil {
384354
continue
385355
}
386-
term := p.Object.Term[k] || (cur != nil && cur.terminal)
387-
t = ptree{root: setEntry(t.root, &node{key: k, kids: sub, branch: true, terminal: term})}
356+
t = ptree{root: setEntry(t.root, &node{key: k, kids: sub, branch: true})}
388357
changed = true
389358
}
390359
return t, changed
@@ -652,67 +621,3 @@ func setIn(node Value, segs []string, v Value) Value {
652621

653622
// flattenUnder reads a patch that only creates leaves as the suffixes it
654623
// names, so they can be written beside a terminal node rather than inside it.
655-
// It refuses anything that changes or removes: only a pure creation can be
656-
// re-anchored without knowing what it was diffed against.
657-
func flattenUnder(p Patch) (map[string]Value, bool) {
658-
out := map[string]Value{}
659-
var walk func(Patch, string) bool
660-
walk = func(q Patch, prefix string) bool {
661-
if q.Object == nil {
662-
return false
663-
}
664-
if len(q.Object.Delete) > 0 {
665-
return false
666-
}
667-
for k, v := range q.Object.Set {
668-
out[join(prefix, k)] = v
669-
}
670-
for k, c := range q.Object.Update {
671-
if !q.Object.New[k] {
672-
return false
673-
}
674-
if !walk(c, join(prefix, k)) {
675-
return false
676-
}
677-
}
678-
return true
679-
}
680-
if !walk(p, "") || len(out) == 0 {
681-
return nil, false
682-
}
683-
return out, true
684-
}
685-
686-
func join(prefix, k string) string {
687-
if prefix == "" {
688-
return k
689-
}
690-
return prefix + "." + k
691-
}
692-
693-
// allForeign reports whether every suffix names something the node does not
694-
// hold. One that it holds is a field, and the patch is editing it.
695-
func allForeign(n *node, flat map[string]Value) bool {
696-
if n == nil || !n.branch {
697-
return false
698-
}
699-
for suffix := range flat {
700-
head := suffix
701-
if i := indexDot(suffix); i >= 0 {
702-
head = suffix[:i]
703-
}
704-
if lookupExact(n.kids.root, head) != nil {
705-
return false
706-
}
707-
}
708-
return true
709-
}
710-
711-
func indexDot(s string) int {
712-
for i := 0; i < len(s); i++ {
713-
if s[i] == '.' {
714-
return i
715-
}
716-
}
717-
return -1
718-
}

‎api/form/patch.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,6 @@ type ObjectPatch struct {
3636
Delete map[string]Value `json:"Delete,omitempty"`
3737
Update map[string]Patch `json:"Update,omitempty"`
3838

39-
// Term names the Update keys where a caller's key ENDED. Splitting a
40-
// dotted key into segments loses that, and without it a later key
41-
// reaches through the node instead of landing beside it.
42-
Term map[string]bool `json:"Term,omitempty"`
43-
4439
// New names the Update keys that did not exist. Their leaves are
4540
// described rather than the subtree, so two patches adding different
4641
// fields under one parent compose; the flag is what lets Inverse remove

‎api/form/regression_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,17 @@ import (
1717
// its prefix. Gluck's store holds both: skills.howto is a skill, and
1818
// skills.howto.md is a different skill.
1919
func TestCollidingKeysAreBothAddressable(t *testing.T) {
20+
// succession is prose AND a namespace of sub-notes: a key that is a
21+
// proper prefix of another, written deliberately.
2022
s := form.FromMap(map[string]json.RawMessage{
21-
"skills.howto": json.RawMessage(`{"filePath":"/a.md"}`),
22-
"skills.howto.md": json.RawMessage(`{"filePath":"/b.md"}`),
23-
"succession": json.RawMessage(`["one"]`),
24-
"succession.plan": json.RawMessage(`"handover"`),
23+
"succession": json.RawMessage(`"the protocol"`),
24+
"succession.rule": json.RawMessage(`"you are a seat"`),
25+
"succession.authority": json.RawMessage(`"only the target may write"`),
2526
})
2627
for k, want := range map[string]string{
27-
"skills.howto": `{"filePath":"/a.md"}`,
28-
"skills.howto.md": `{"filePath":"/b.md"}`,
29-
"succession": `["one"]`,
30-
"succession.plan": `"handover"`,
28+
"succession": `"the protocol"`,
29+
"succession.rule": `"you are a seat"`,
30+
"succession.authority": `"only the target may write"`,
3131
} {
3232
got, ok := s.Get(k)
3333
require.True(t, ok, "%s is unreachable", k)

‎api/form/succession_test.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package form_test
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/jack-work/figaro/api/form"
11+
)
12+
13+
// A key that is a proper prefix of another, written deliberately: succession
14+
// is prose AND a namespace of sub-notes.
15+
func TestParentAndChildKeysBothSurvive(t *testing.T) {
16+
src := map[string]json.RawMessage{
17+
"succession": json.RawMessage(`"the protocol"`),
18+
"succession.rule": json.RawMessage(`"you are a seat"`),
19+
"succession.authority": json.RawMessage(`"only the target may write"`),
20+
}
21+
s := form.FromMap(src)
22+
for k, want := range src {
23+
got, ok := s.Get(k)
24+
require.True(t, ok, "%s unreachable", k)
25+
assert.JSONEq(t, string(want), string(got), k)
26+
}
27+
28+
// And across a reload, which is where the shape has to agree with itself.
29+
raw, err := json.Marshal(s)
30+
require.NoError(t, err)
31+
var back form.Snapshot
32+
require.NoError(t, json.Unmarshal(raw, &back))
33+
for k, want := range src {
34+
got, ok := back.Get(k)
35+
require.True(t, ok, "%s did not survive a reload", k)
36+
assert.JSONEq(t, string(want), string(got), k)
37+
}
38+
}

0 commit comments

Comments
 (0)