fix: isolate think scratchpads by conversation

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Summary

Fixes #4588. Builds on merged #4570 and preserves its synchronization and sequential/64-way concurrency coverage.

• Make think toolsets stateless and bind their scratchpads to the actual conversation ID, calling agent and configured/extra toolset ordinal through the host-owned runtime. Forward this binding through Code Mode.
• Store thoughts atomically in the session store: mutex-protected in-memory state and SQLite migration 31 with ordered snapshots and root-owned foreign-key cleanup. Establish root persistence through the existing synchronous observer before starting the producer.
• Resolve durable ancestry for resumed nested children and their delegated descendants, with explicit ownership/cancellation/store errors instead of silent resets.
• Preserve Origin in Session.Clone() (pkg/session/branch.go:82) and assert it in the existing scalar-copy test (pkg/session/clone_test.go:23,55). This is the explicitly approved three-line, two-file micro-fix; the server workaround and Fork/Branch behavior are unchanged.
• Add credential-free store, public-handler and real-runtime regressions for overlapping distinct conversations sharing the same team/raw think instance, same-ID reload/clone, new-ID forks, restart, delegation, Code Mode, deletion and observer ordering.

Approved design: #4588 (comment). No config/schema changes or cache fixes.

Retention and limitations

Output remains exactly "Thoughts:\n" + strings.Join(thoughts, "\n").

Thoughts survive turns, runtime closure and compaction. SQLite retains them across reloads/restarts; the in-memory store retains them for its lifetime. Same-ID clones share the durable scratchpad; new-ID branches/forks start empty. JSON export/import does not transport this state, and there is no legacy backfill.

Raw thoughts are stored before response-output redaction: session database and backup privacy policies apply. Deletion is logical, not secure erasure or backup removal. Root deletion cleans up all root-owned thoughts, including aborted/unpersisted children. Persisted-child deletion prunes known subtrees; never-persisted descendants beneath a deleted non-root child remain root-owned until root deletion. Stop writers before deletion.

Retention is lifecycle-bounded, not hard-size-bounded: no quota, TTL, eviction or output cap. Full output and storage grow with retained thoughts. Ordinal identity requires unchanged agent/toolset topology and ordering; no remapping/portability guarantees. Scoped persistence/ownership/unsupported-store failures are tool errors; only genuinely unscoped Nop hosts use an ephemeral single thought.

Validation

Final author validation on reviewed tree c852a05122fce07a5c023257ddfc488192a5f8b1:

CGO_ENABLED=1 task --force build
CGO_ENABLED=1 task test
CGO_ENABLED=1 task --force lint
CGO_ENABLED=1 go test -race -shuffle=on -count=100 -timeout=5m ./pkg/tools/builtin/think
CGO_ENABLED=1 go test -v -race -shuffle=on -count=20 ./pkg/tools/builtin/think ./pkg/runtime/toolexec
CGO_ENABLED=1 go test ./pkg/session -run Think -race -count=20 -shuffle=on
CGO_ENABLED=1 go test ./pkg/runtime -run Think -race -count=3 -shuffle=on
CGO_ENABLED=1 go test ./pkg/runtime ./pkg/session ./pkg/runtime/toolexec ./pkg/tools/builtin/think -race -count=3 -shuffle=on
CGO_ENABLED=1 go test ./pkg/runtime -run '^TestThink(ResumedChildDelegates|OverlappingConversationIsolation|RootIgnoresInheritedCandidate)$' -race -count=10 -shuffle=on
CGO_ENABLED=1 go test ./pkg/runtime -run '^TestThinkRetentionReloadCloneFork$' -race -count=1
CGO_ENABLED=1 go test ./pkg/session -run '^TestClone_CopiesScalarFields$' -race -count=20 -shuffle=on
GOOS=js GOARCH=wasm go build ./pkg/tools/builtin/think
GOOS=js GOARCH=wasm go test -exec="$(go env GOROOT)/lib/wasm/go_js_wasm_exec" ./pkg/tools/builtin/think
git diff --cached --check

All final commands PASS, no race reports; lint reports zero issues and clean project cops/tidy. Fresh independent review of the complete 16-file final tree APPROVE: all three earlier findings confirmed fixed. The reviewer independently ran canonical commands, repeated affected/targeted races, Node-backed think tests and task test-wasm-js. It also reproduced the real Clone regression on both stores through a read-only negative overlay removing only the Origin copy; no test-side compensation, skip or weakened assertion.

Historical disposition is retained: earlier canonical validation failed the real Clone regression and unchanged TestFileCache_dedupSkipsRedundantWrite (pkg/cache/cache_test.go:118); unchanged-base cache control failed 28/30 runs. The latest canonical cache result is CACHED PASS, not fresh proof the cache flake is absent or fixed. Earlier runtime tests excluding Clone were supplemental, not full validation. Full-repository race tests were NOT run; the affected-package races above are not described as full-repository validation.

This PR is intentionally draft for CI and follow-up. No merge requested.

[Copilot]

🟡 Changes recommended

Root cleanup can leave orphaned thoughts when SQLite foreign keys are disabled, and in-memory ownership checks scale quadratically.

2 open findings

• Root deletion leaves orphaned scratchpads when foreign keys are disabled · New
• Ownership validation becomes O(N²) across retained conversations · New

What changed in this PR

Isolates think scratchpads by conversation, agent, and toolset while adding durable session-store persistence.

Changes:

• Moves scratchpad ownership from toolsets into atomic session stores.
• Adds runtime lineage binding, Code Mode forwarding, and extensive regressions.
• Preserves session origin during cloning and documents retention semantics.

File	Description
pkg/​tools/​runtime.go	Adds scratchpad runtime capability.
pkg/​tools/​builtin/​think/​think.go	Makes think toolsets stateless.
pkg/​tools/​builtin/​think/​think_test.go	Tests output, concurrency, and errors.
pkg/​session/​think_scratchpad.go	Implements memory and SQLite storage.
pkg/​session/​think_scratchpad_test.go	Tests persistence, deletion, and concurrency.
pkg/​session/​store.go	Adds memory state and deletion cleanup.
pkg/​session/​migrations.go	Adds scratchpad table migration.
pkg/​session/​migrations_pinned_test.go	Updates migration digest.
pkg/​session/​clone_test.go	Verifies origin cloning.
pkg/​session/​branch.go	Preserves origin in clones.
pkg/​runtime/​toolexec/​dispatcher.go	Forwards scratchpad appends.
pkg/​runtime/​toolexec/​dispatcher_test.go	Tests dispatcher forwarding.
pkg/​runtime/​tool_dispatch.go	Resolves scratchpad ownership and ancestry.
pkg/​runtime/​think_test.go	Adds runtime integration regressions.
pkg/​runtime/​loop.go	Persists roots before producers start.
docs/​tools/​think/​index.md	Documents scope and retention.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Fixed in d63148c. Ownership validation now uses a sessionID -> rootID map guarded by the existing scratchpad mutex, rather than scanning retained roots and keys. Successful appends register ownership; deletion prunes entries for deleted session IDs or owning roots.

TestThinkScratchpadOwnershipAcrossKeys, TestThinkScratchpadOwnershipDeletion, and TestThinkScratchpadRejectedAppendDoesNotClaimOwnership cover agent/toolset separation, ownership reuse and pruning, unrelated survival, and rejected/cancelled appends not claiming ownership. Repeated store and affected-package race tests passed, and fresh independent delta review approved the published tree. Constant-time ownership validation follows from the map lookup, not a timing assertion.

[aheritier]

👋 This PR has merge conflicts with the base branch. Please rebase or merge the latest base branch and resolve them. I've moved it to draft and added ; it'll be picked back up automatically once the conflicts are cleared.

[aheritier]

👋 This PR has merge conflicts with the base branch. Please rebase or merge the latest base branch and resolve them. I've moved it to draft and added status/needs-rebase; it'll be picked back up automatically once the conflicts are cleared.

[aheritier]

🤖 Automated implementer agent — this comment was posted by the implementer bot from Docker Agentic Platform, not by a human developer

Addressed both conflict reports (6186000062, 6186004782) in signed merge commit 6826d3fc190f7fc9ce8c04abff17ed2898dfc57f by integrating current main at aa8fc5df9d7e994d57dca696531c9abcb5b7dae6. Existing reviewed commits are unchanged.

The two mechanical conflicts were resolved by retaining both AppendThought and PrepareConfirmation in pkg/runtime/tool_dispatch.go, and retaining both independently appended dispatcher tests unchanged in pkg/runtime/toolexec/dispatcher_test.go. Automatic confirmation changes and all existing scratchpad fixes remain intact. Fresh independent review approved the exact published tree.

New-tree validation passed: canonical build/test/lint; shuffled race runs for think ×100, think/toolexec ×20, complete session/runtime/toolexec ×20, store scratchpad ×20, runtime think ×3 and confirmation ×20; think WASM build/Node tests and whole task test-wasm-js. Initial build failed because this recycled sandbox lacked GCC; installed the build tools and reran successfully. Full-repository race testing was not run. Old-head CI is not counted as validation of this head; new CI is pending.

Verified local, remote branch and PR heads equal 6826d3fc190f7fc9ce8c04abff17ed2898dfc57f; GitHub reports the signature valid and mergeable: true (merge state blocked, not a claim that CI is green). PR remains open and ready. No PR merge performed.