fix(statedb): preserve detected session-id keys on stale full-table saves

[lazyoft]

Problem

A freshly-launched session's claude_session_id (and the gemini/opencode/codex equivalents) can be silently wiped from state.db, surfacing as "no session ID detected" on a session whose Claude transcript is actively being written. Callers that resolve a session's transcript by its persisted id (e.g. a coordinator mapping its workers to ~/.claude/projects/<uuid>.jsonl) intermittently lose the mapping within seconds of launch.

Root cause

The per-tool conversation ids are detected asynchronously and persisted shortly after launch. But any concurrent full-table save — SaveWithGroups → SaveInstances → saveInstancesOnce — rebuilds each row's tool_data from its in-memory Instance. If that writer's snapshot has not yet observed the id, the rebuilt blob omits it (omitempty zero-value).

MergeToolDataExtras preserved only keys unknown to the typed schema; for a known key it let the new blob win "including absence-by-omitempty". So an omitted id cleared the previously-persisted value instead of being preserved.

The hottest such writer is the rename hook (hook_name_sync.go): on every Claude hook that renames a session it does LoadWithGroups then SaveWithGroups over the whole snapshot. Other full-rewrite writers (internal/ui/home.go, web_mutator.go, group_cmd.go, session_move.go) hit the same path less frequently. The launch path itself is already hardened (InsertSessionAndVerify, #1031), so the clobberer is always a different writer.

Fix

Treat the detected session-id keys as sticky in MergeToolDataExtras: {claude,gemini,opencode,codex}_session_id and their *_detected_at. When the new blob omits a sticky key but the old row has it, carry the old value forward. Semantics:

• non-empty new value still wins — a real resume/fork that changes the id;
• explicit empty present in the new blob ("claude_session_id":"") is honored as an intentional clear;
• only outright omission preserves.

This is a one-line change to the merge guard (if known[k] && !sticky[k]) plus a small key set; it does not touch the save/sweep contract.

Design note

Deliberate clears must now use an explicit empty (or a single-row clear) rather than relying on an omit. UpdateClaudeSessionsWithDedup clears in-memory only; with sticky keys a cleared id is restored from disk on reload and re-cleared in memory each load. Since duplicates "shouldn't occur in normal use" and tmux env is authoritative, that is a harmless redundant in-memory re-dedup, not a functional regression.

Tests

internal/statedb/tool_data_extras_test.go:

• repoints the typed-absence test to a non-sticky key (latest_prompt), since claude_session_id absence behavior intentionally changed;
• adds sticky coverage: preserve-on-omit (all four tools + a *_detected_at), non-empty-new-wins, and explicit-empty-clears.

go test ./internal/statedb/ passes; gofmt/go vet clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved how session-related data is merged so previously saved values are retained when newer data omits them.
  • Non-sticky fields still follow the usual overwrite rules, and explicit empty values are respected as intentional clears.
• Tests
  • Expanded test coverage for missing, updated, and intentionally cleared session-related values during merges.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bd8e1e0a-d4b9-4d99-ad3e-82b8dc65f14f

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5ae31 and cd781b5.

📒 Files selected for processing (2)

• internal/statedb/tool_data_extras.go
• internal/statedb/tool_data_extras_test.go

---

📝 Walkthrough

Walkthrough

MergeToolDataExtras now preserves selected session-id and detected-at JSON fields when new tool data omits them, while still allowing explicit new values and explicit empty values to replace existing ones. Tests were updated for omission, override, and explicit-clear cases.

Changes

Sticky session-id merge semantics

Layer / File(s)	Summary
Sticky merge contract internal/statedb/tool_data_extras.go	Documentation, the sticky-key list, and the merge loop now treat the listed session-id and timestamp keys as preserved-on-omission exceptions.
Merge behavior tests internal/statedb/tool_data_extras_test.go	Tests now cover non-sticky omission, sticky omission preservation, non-empty override, and explicit empty clear behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• asheshgoplani/agent-deck#1140: Both changes touch tool_data session-id fields and their companion timestamps during merge and persistence handling.

Suggested reviewers

• asheshgoplani

🚥 Pre-merge checks | ✅ 5 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.57% which is insufficient. The required threshold is 70.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Test_coverage_per_surface	⚠️ Warning	Only internal/statedb unit tests cover the sticky merge; no CLI/TUI/Web/remote regression tests assert the stale-save omit-preserves contract at any surface.	Add at least one regression per applicable surface (or a shared harness exercised by CLI/TUI/Web/remote) that forces a stale save and verifies sticky session IDs survive.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title uses Conventional Commits format and accurately describes the merge behavior fix for sticky session-id keys.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Remote_parity	✅ Passed	The PR only changes internal/statedb merge logic and tests; no internal/ui/ or cmd/agent-deck TUI code is part of the patch, so the RemoteSession parity check is not applicable.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[asheshgoplani]

🤖 autopilot: CI green + Claude review clean; FLAGGED for local Codex dual-review before merge (data-sensitive area).

The change modifies MergeToolDataExtras in internal/statedb/tool_data_extras.go to preserve sticky session-id keys (claude_session_id, gemini_session_id, etc.) when the new blob omits them — fixing the t-0133 race where a concurrent full-table save could wipe a live session's mapping. The logic is correct and well-tested, but it touches the statedb merge path which is directly on the data-critical layer. Holding for local Codex dual-review per policy.

---

Generated by Claude Code

[asheshgoplani]

🤖 autopilot: CI green + Claude review clean; FLAGGED for local Codex dual-review before merge (data-sensitive area).

Review summary: The fix correctly reorders the sticky-key preservation logic in MergeToolDataExtras so that detected session-id keys (claude_session_id, etc.) are not wiped by concurrent full-table saves. Logic is sound, test coverage is thorough, and no data-loss risk is introduced. However, this touches internal/statedb directly on the merge path for every save, which puts it in the data-sensitive category requiring a second local review pass before autopilot merge.

---

Generated by Claude Code

[asheshgoplani]

🤖 autopilot: CI green + Claude review clean; FLAGGED for local Codex dual-review before merge (data-sensitive area: internal/statedb sticky-key merge logic).

---

Generated by Claude Code

[asheshgoplani]

🤖 autopilot 2026-06-29: CI still green; still FLAGGED for local Codex dual-review before merge (data-sensitive area — internal/statedb/tool_data_extras.go). No change from yesterday's flag.

---

Generated by Claude Code

[asheshgoplani]

🤖 autopilot: CI green + Claude review clean; FLAGGED for local Codex dual-review before merge (data-sensitive area: statedb tool_data merge logic for session-ID sticky keys — modifies the MergeToolDataExtras path). Codex should verify that omitempty JSON tags produce full key-absence (not "") for zero values, to ensure the three-way omit/new-wins/explicit-clear logic works as intended.

---

Generated by Claude Code

[asheshgoplani]

🤖 autopilot: CI green + Claude review clean; FLAGGED for local Codex dual-review before merge (data-sensitive area). This PR modifies internal/statedb/statedb.go (new SetArchived targeted UPDATE) and internal/statedb/tool_data_extras.go (sticky session-id keys) — statedb changes require local dual-review per autopilot policy.

---

Generated by Claude Code