Skip to content
Open
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
42 changes: 39 additions & 3 deletions internal/statedb/tool_data_extras.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,19 @@ import (
// setting a key wins over the old value (no silent override of intended
// updates). Only keys that are completely unknown to the typed schema AND
// absent from the new blob are carried forward.
//
// EXCEPTION — sticky detected session-id keys (stickyToolDataKeys): the
// per-tool conversation ids (claude_session_id / gemini_session_id /
// opencode_session_id / codex_session_id and their *_detected_at) are
// write-once-per-conversation identity, populated asynchronously after a
// session starts. They are treated like extras for the ABSENCE case: when the
// new blob OMITS them (omitempty zero-value) but the old row HAS them, the old
// value is carried forward, so an unrelated full-table save whose in-memory
// snapshot simply has not detected the id yet can no longer silently wipe a
// live session's mapping (t-0133). A NON-EMPTY new value still wins (a real
// resume/fork that changes the id), and an EXPLICIT empty value present in the
// new blob (`"claude_session_id":""`) is honored as an intentional clear — only
// outright OMISSION is treated as "unaware writer, preserve".
func MergeToolDataExtras(oldToolData, newToolData json.RawMessage) json.RawMessage {
if len(oldToolData) == 0 {
return newToolData
Expand All @@ -49,13 +62,14 @@ func MergeToolDataExtras(oldToolData, newToolData json.RawMessage) json.RawMessa
}

known := toolDataKnownKeys()
sticky := stickyToolDataKeys()
merged := false
for k, v := range oldMap {
if known[k] {
continue // typed schema is authoritative
if known[k] && !sticky[k] {
continue // typed schema is authoritative (except sticky identity keys)
}
if _, exists := newMap[k]; exists {
continue // new explicitly set this key
continue // new explicitly set this key (incl. explicit empty = intentional clear)
}
newMap[k] = v
merged = true
Expand All @@ -71,6 +85,28 @@ func MergeToolDataExtras(oldToolData, newToolData json.RawMessage) json.RawMessa
return out
}

// stickyToolDataKeys returns the typed tool_data keys that MergeToolDataExtras
// preserves from the old row when the new blob OMITS them (rather than letting
// omitempty-absence clear them). These are the per-tool conversation ids and
// their detection timestamps: identity that is detected asynchronously and is
// write-once for the life of a conversation. Keeping them sticky prevents a
// concurrent full-table save (e.g. the rename hook's LoadWithGroups +
// SaveWithGroups) whose snapshot has not yet observed the id from wiping a live
// session's session-id mapping (t-0133). A non-empty new value, or an explicit
// empty present in the new blob, still wins — only outright omission preserves.
func stickyToolDataKeys() map[string]bool {
return map[string]bool{
"claude_session_id": true,
"claude_detected_at": true,
"gemini_session_id": true,
"gemini_detected_at": true,
"opencode_session_id": true,
"opencode_detected_at": true,
"codex_session_id": true,
"codex_detected_at": true,
}
}

// toolDataKnownKeys returns the set of JSON keys that toolDataBlob explicitly
// models. Used by MergeToolDataExtras to distinguish agent-deck's
// authoritative schema from externally-managed extras.
Expand Down
80 changes: 74 additions & 6 deletions internal/statedb/tool_data_extras_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,86 @@ func TestMergeToolDataExtras_NewExplicitWinsOverOldUnknown(t *testing.T) {
}

func TestMergeToolDataExtras_TypedKeyAbsenceRespected(t *testing.T) {
// When the new tool_data omits a typed key (e.g., omitempty zero-value),
// the merge must NOT carry the old value forward. The typed schema is
// authoritative for typed fields. This protects intentional clears.
old := json.RawMessage(`{"claude_session_id":"abc"}`)
// When the new tool_data omits a NON-STICKY typed key (e.g., omitempty
// zero-value), the merge must NOT carry the old value forward. The typed
// schema is authoritative for those fields. (Sticky session-id keys are the
// deliberate exception, covered by the sticky tests below.)
old := json.RawMessage(`{"latest_prompt":"hello"}`)
new_ := json.RawMessage(`{}`)

merged := MergeToolDataExtras(old, new_)

var got map[string]json.RawMessage
_ = json.Unmarshal(merged, &got)
if _, present := got["claude_session_id"]; present {
t.Errorf("claude_session_id should be absent in merged when new omits it; got %s", got["claude_session_id"])
if _, present := got["latest_prompt"]; present {
t.Errorf("latest_prompt should be absent in merged when new omits it; got %s", got["latest_prompt"])
}
}

func TestMergeToolDataExtras_StickySessionIDPreservedWhenNewOmits(t *testing.T) {
// t-0133: a session-id key present in the old row but OMITTED by the new
// blob (omitempty zero-value, e.g. a concurrent full-table save whose
// in-memory snapshot has not yet detected the id) must be carried forward,
// not wiped. Covers all four tools + a *_detected_at companion.
old := json.RawMessage(`{` +
`"claude_session_id":"c-uuid","claude_detected_at":111,` +
`"gemini_session_id":"g-uuid",` +
`"opencode_session_id":"o-uuid",` +
`"codex_session_id":"x-uuid"}`)
new_ := json.RawMessage(`{"latest_prompt":"hi"}`)

merged := MergeToolDataExtras(old, new_)

var got map[string]json.RawMessage
if err := json.Unmarshal(merged, &got); err != nil {
t.Fatalf("merged JSON does not parse: %v", err)
}
for k, want := range map[string]string{
"claude_session_id": `"c-uuid"`,
"claude_detected_at": `111`,
"gemini_session_id": `"g-uuid"`,
"opencode_session_id": `"o-uuid"`,
"codex_session_id": `"x-uuid"`,
} {
if string(got[k]) != want {
t.Errorf("%s = %s, want %s (sticky: preserved from old when new omits)", k, got[k], want)
}
}
// The new blob's own key still lands.
if string(got["latest_prompt"]) != `"hi"` {
t.Errorf("latest_prompt = %s, want \"hi\"", got["latest_prompt"])
}
}

func TestMergeToolDataExtras_StickyNonEmptyNewWins(t *testing.T) {
// A real resume/fork writes a NEW non-empty session id; that must win over
// the old value (sticky only protects the omission case, never overrides an
// explicit update).
old := json.RawMessage(`{"claude_session_id":"old-uuid"}`)
new_ := json.RawMessage(`{"claude_session_id":"new-uuid"}`)

merged := MergeToolDataExtras(old, new_)

var got map[string]json.RawMessage
_ = json.Unmarshal(merged, &got)
if string(got["claude_session_id"]) != `"new-uuid"` {
t.Errorf("claude_session_id = %s, want \"new-uuid\" (non-empty new wins)", got["claude_session_id"])
}
}

func TestMergeToolDataExtras_StickyExplicitEmptyIsIntentionalClear(t *testing.T) {
// An EXPLICIT empty value present in the new blob (not omitted) is honored
// as an intentional clear — the escape hatch for a deliberate reset/dedup
// that wants the id gone. Only outright omission preserves.
old := json.RawMessage(`{"claude_session_id":"old-uuid"}`)
new_ := json.RawMessage(`{"claude_session_id":""}`)

merged := MergeToolDataExtras(old, new_)

var got map[string]json.RawMessage
_ = json.Unmarshal(merged, &got)
if string(got["claude_session_id"]) != `""` {
t.Errorf("claude_session_id = %s, want \"\" (explicit empty = intentional clear)", got["claude_session_id"])
}
}

Expand Down