From bff6b102c1b2f27a3994445a5c567ad5c2e31b76 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 22:24:45 -0700 Subject: [PATCH 1/4] Hold the request line's keys to a contract, and the skill to the line The request line is read by programs nothing in this repo runs: the basecamp-connect skill, and the session or practice driving the connector. Its keys are what those readers match on. A renamed json tag passed every test, because the tests decode the line into the same struct that wrote it, and the skill's example of the line could drift from the real one with nothing noticing. Pin the names the line must carry, and hold the skill's example to the line's keys exactly, so a renamed or dropped key fails here, and a key added to the line has to be documented where the reader learns it. --- internal/connector/handoff.go | 5 ++ internal/connector/request_contract_test.go | 91 +++++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 internal/connector/request_contract_test.go diff --git a/internal/connector/handoff.go b/internal/connector/handoff.go index b48a869c4..8accc92ba 100644 --- a/internal/connector/handoff.go +++ b/internal/connector/handoff.go @@ -23,6 +23,11 @@ import ( // HandoffLine is one trusted request, as the reader gets it. It is the // instruction a worker used to pull, written out: the agent's own mention // stripped, nothing more than the record carries. +// +// Its keys are a contract with whoever reads the lines, the basecamp-connect +// skill first: renaming or dropping one breaks a reader nothing here runs. +// requestLineContract holds the names, and the skill's example is held to +// the line exactly, so a key added here is documented where readers learn it. type HandoffLine struct { Type string `json:"type"` // "request" EventID int64 `json:"event_id"` diff --git a/internal/connector/request_contract_test.go b/internal/connector/request_contract_test.go new file mode 100644 index 000000000..0862a3b3d --- /dev/null +++ b/internal/connector/request_contract_test.go @@ -0,0 +1,91 @@ +package connector + +import ( + "encoding/json" + "os" + "path/filepath" + "regexp" + "slices" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// requestLineContract is every key a "type":"request" line carries, nested +// keys as parent.child. The basecamp-connect skill, and any session or +// practice that drives the connector, keys on these names: renaming or +// dropping one breaks a reader that nothing here runs, so it is a breaking +// change made on purpose, here, or not at all. Adding a key is not. +var requestLineContract = []string{ + "acknowledge", + "content", + "content_updated_at", + "event_id", + "event_type", + "recording.bucket_id", + "recording.project_name", + "recording.recording_id", + "recording.title", + "recording.type", + "recording.url", + "reply_to.kind", + "reply_to.recording_id", + "requester_id", + "requester_name", + "role", + "trigger", + "type", +} + +// The line the connector writes carries every key the contract names, under +// that name. +func TestTheRequestLineKeepsItsContract(t *testing.T) { + got := writtenLineKeys(t) + for _, key := range requestLineContract { + assert.Contains(t, got, key, "the request line no longer carries %q, which its readers key on", key) + } +} + +// The skill's own example of the line names exactly the keys the line +// carries, so the session reading the line is taught the names it will see, +// and a key added to the line is documented where the reader learns it. +func TestTheSkillDocumentsTheRequestLineItGets(t *testing.T) { + skill, err := os.ReadFile(filepath.Join("..", "..", "skills", "basecamp-connect", "SKILL.md")) + require.NoError(t, err) + example := regexp.MustCompile("(?s)### 2\\. The request line\\s*```json\\n(.*?)```").FindSubmatch(skill) + require.NotNil(t, example, "the skill's request line example is where this test looks for it") + assert.Equal(t, writtenLineKeys(t), lineKeys(t, example[1])) +} + +// writtenLineKeys is the keys of a request line as the connector writes one, +// every optional field set. +func writtenLineKeys(t *testing.T) []string { + t.Helper() + raw, err := json.Marshal(HandoffLine{ + Type: "request", + Recording: HandoffRecording{ProjectName: "set, so omitempty keeps it"}, + RequesterName: "set, so omitempty keeps it", + }) + require.NoError(t, err) + return lineKeys(t, raw) +} + +// lineKeys is a line's keys, nested ones as parent.child, sorted. +func lineKeys(t *testing.T, raw []byte) []string { + t.Helper() + var obj map[string]any + require.NoError(t, json.Unmarshal(raw, &obj)) + var keys []string + for k, v := range obj { + if nested, ok := v.(map[string]any); ok { + for nk := range nested { + keys = append(keys, k+"."+nk) + } + continue + } + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} From 07db0cb7be63b37654532784f7fff571a5ac5669 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Mon, 5 Oct 2026 22:38:52 -0700 Subject: [PATCH 2/4] See every key the line can carry, at any depth The fixture set only the two optional fields that exist today, so a key added later with omitempty stayed out of the comparison, and keys were read one level deep. Fill every field by reflection, and walk nested objects to their full dotted path. --- internal/connector/request_contract_test.go | 54 ++++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/internal/connector/request_contract_test.go b/internal/connector/request_contract_test.go index 0862a3b3d..d4d342377 100644 --- a/internal/connector/request_contract_test.go +++ b/internal/connector/request_contract_test.go @@ -4,9 +4,11 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "regexp" "slices" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -59,33 +61,59 @@ func TestTheSkillDocumentsTheRequestLineItGets(t *testing.T) { } // writtenLineKeys is the keys of a request line as the connector writes one, -// every optional field set. +// with every field set, so a key that omitempty would drop is still seen — +// including one added after this test was written. func writtenLineKeys(t *testing.T) []string { t.Helper() - raw, err := json.Marshal(HandoffLine{ - Type: "request", - Recording: HandoffRecording{ProjectName: "set, so omitempty keeps it"}, - RequesterName: "set, so omitempty keeps it", - }) + var line HandoffLine + fill(t, reflect.ValueOf(&line).Elem()) + raw, err := json.Marshal(line) require.NoError(t, err) return lineKeys(t, raw) } -// lineKeys is a line's keys, nested ones as parent.child, sorted. +// fill sets every field of v to a value that is not its zero. +func fill(t *testing.T, v reflect.Value) { + t.Helper() + if v.Type() == reflect.TypeFor[time.Time]() { + v.Set(reflect.ValueOf(time.Date(2026, 10, 6, 0, 0, 0, 0, time.UTC))) + return + } + switch v.Kind() { + case reflect.Struct: + for i := range v.NumField() { + if v.Type().Field(i).IsExported() { + fill(t, v.Field(i)) + } + } + case reflect.String: + v.SetString("x") + case reflect.Int, reflect.Int64, reflect.Int32: + v.SetInt(1) + case reflect.Bool: + v.SetBool(true) + default: + t.Fatalf("the request line has a %s field; teach fill to set one", v.Kind()) + } +} + +// lineKeys is a line's keys, nested ones as their full dotted path, sorted. func lineKeys(t *testing.T, raw []byte) []string { t.Helper() var obj map[string]any require.NoError(t, json.Unmarshal(raw, &obj)) var keys []string - for k, v := range obj { - if nested, ok := v.(map[string]any); ok { - for nk := range nested { - keys = append(keys, k+"."+nk) + var walk func(prefix string, obj map[string]any) + walk = func(prefix string, obj map[string]any) { + for k, v := range obj { + if nested, ok := v.(map[string]any); ok { + walk(prefix+k+".", nested) + continue } - continue + keys = append(keys, prefix+k) } - keys = append(keys, k) } + walk("", obj) slices.Sort(keys) return keys } From a59fecc8f843216e133a33a4fba6c1c5fa21bc80 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 13:08:54 -0700 Subject: [PATCH 3/4] Hold the line to the contract both ways, and its required keys at zero values A key added to the line had to reach the skill but not the contract, and a required key that gained omitempty vanished from real lines at its zero value while the filled fixture still showed it. The line's keys now equal the contract's, and a zero-valued line keeps every key but the two documented as optional. --- internal/connector/request_contract_test.go | 24 +++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/internal/connector/request_contract_test.go b/internal/connector/request_contract_test.go index d4d342377..0e95f296b 100644 --- a/internal/connector/request_contract_test.go +++ b/internal/connector/request_contract_test.go @@ -40,13 +40,25 @@ var requestLineContract = []string{ "type", } -// The line the connector writes carries every key the contract names, under -// that name. +// optionalKeys are the contract's keys a line may leave out: the ones whose +// value is not always known. +var optionalKeys = []string{"recording.project_name", "requester_name"} + +// The line the connector writes carries exactly the keys the contract names, +// under those names: a key dropped or renamed breaks a reader, and a key +// added is added to the contract on purpose. func TestTheRequestLineKeepsItsContract(t *testing.T) { - got := writtenLineKeys(t) - for _, key := range requestLineContract { - assert.Contains(t, got, key, "the request line no longer carries %q, which its readers key on", key) - } + assert.Equal(t, requestLineContract, writtenLineKeys(t)) +} + +// Every key but the optional ones is there at its zero value too. A false +// acknowledge or an empty title is a real line, and a key that omitempty +// drops there is gone from the line its readers get. +func TestARequestLineAtItsZeroValuesKeepsItsRequiredKeys(t *testing.T) { + raw, err := json.Marshal(HandoffLine{}) + require.NoError(t, err) + want := slices.DeleteFunc(slices.Clone(requestLineContract), func(k string) bool { return slices.Contains(optionalKeys, k) }) + assert.Equal(t, want, lineKeys(t, raw)) } // The skill's own example of the line names exactly the keys the line From b5b21a9e3afd47737b4107175d7ec5cc15ab5e93 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Tue, 6 Oct 2026 13:56:21 -0700 Subject: [PATCH 4/4] Find the skill's example under CRLF, and count an empty object as a key --- internal/connector/request_contract_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/connector/request_contract_test.go b/internal/connector/request_contract_test.go index 0e95f296b..d21d3ac58 100644 --- a/internal/connector/request_contract_test.go +++ b/internal/connector/request_contract_test.go @@ -61,13 +61,16 @@ func TestARequestLineAtItsZeroValuesKeepsItsRequiredKeys(t *testing.T) { assert.Equal(t, want, lineKeys(t, raw)) } +// skillExample finds the request line example in the skill. +var skillExample = regexp.MustCompile("(?s)### 2\\. The request line\\s*```json\\r?\\n(.*?)```") + // The skill's own example of the line names exactly the keys the line // carries, so the session reading the line is taught the names it will see, // and a key added to the line is documented where the reader learns it. func TestTheSkillDocumentsTheRequestLineItGets(t *testing.T) { skill, err := os.ReadFile(filepath.Join("..", "..", "skills", "basecamp-connect", "SKILL.md")) require.NoError(t, err) - example := regexp.MustCompile("(?s)### 2\\. The request line\\s*```json\\n(.*?)```").FindSubmatch(skill) + example := skillExample.FindSubmatch(skill) require.NotNil(t, example, "the skill's request line example is where this test looks for it") assert.Equal(t, writtenLineKeys(t), lineKeys(t, example[1])) } @@ -118,7 +121,7 @@ func lineKeys(t *testing.T, raw []byte) []string { var walk func(prefix string, obj map[string]any) walk = func(prefix string, obj map[string]any) { for k, v := range obj { - if nested, ok := v.(map[string]any); ok { + if nested, ok := v.(map[string]any); ok && len(nested) > 0 { walk(prefix+k+".", nested) continue } @@ -129,3 +132,10 @@ func lineKeys(t *testing.T, raw []byte) []string { slices.Sort(keys) return keys } + +// An empty object is a key like any other, and the skill example is found +// whatever line endings the checkout gave the file. +func TestTheContractWalkSeesEmptyObjectsAndCRLF(t *testing.T) { + assert.Equal(t, []string{"a.b", "metadata"}, lineKeys(t, []byte(`{"metadata":{},"a":{"b":1}}`))) + assert.NotNil(t, skillExample.FindSubmatch([]byte("### 2. The request line\r\n\r\n```json\r\n{}\r\n```"))) +}