Repository navigation
feat(threads): per-thread working directory in the conversation store - #310
Conversation
Introduce a persistent thread store backed by an index, along with a bus for broadcasting thread events. This gives sessions a durable place to keep thread state and a way to notify subscribers of changes. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
…rs,crates/tinyagents-session/sr Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Add a store test that binds a working directory on thread creation, verifies a later title upsert preserves it, then rebinds and clears it and checks the listed thread reflects the cleared value. The module re-exports update_thread_working_dir so the test can exercise the rebind path. Auto-committed-on: dragonfly Co-authored-by: Medulla <medulla@tinyhumans.ai>
Tiny Sweeper reviewTiny Sweeper reviewed this change across 6 lane(s) and found 1 active actionable finding(s). Detailed lane evidence and any incomplete work are listed below. State: Ready for maintainer review Review snapshot
Completeness: Complete What changedThe review could not produce a supported behavioral summary; inspect the cited changed surface and lane details below. FeaturesNone identified with supported citations. TestsNo supported feature-to-test mapping was produced. Test execution is not inferred. Findings
Before mergeNone. Agent review detailscritique
security
tests
commits
description
e2e
Evidence and run details
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (12)
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughConversation threads now store an optional working directory. Store operations can set or clear it, and thread-list and thread-summary results include the value. ChangesThread Working Directory
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The working-directory persistence change is mergeable after normal checks; no actionable issue remains from this review. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change currently stores directory metadata rather than granting filesystem or execution access. However, creation and update handle directory strings differently, and the future host’s directory authorization and execution behavior are not available for review. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
A rabbit hops beside the thread, Comment |
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.0166 · 326,735 in / 12,941 out · 42,448 cached (13%) · gpt-5.6-luna, glm-5.3-flash
critique: $0.0070 · 138,562 in / 5,269 out · 20,572 cached (15%) · gpt-5.6-luna
security: $0.0077 · 154,748 in / 5,225 out · 21,876 cached (14%) · gpt-5.6-luna
tests: $0.0001 · 11,032 in / 599 out · 0 cached (0%) · glm-5.3-flash
description: $0.0007 · 10,654 in / 271 out · 0 cached (0%) · glm-5.3-flash
| /// when `None`. | ||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub personality_id: Option<String>, | ||
| /// Optional working directory the thread's agent acts in, chosen when the |
There was a problem hiding this comment.
Test the JSON round trip of a bound working_dir on ConversationThread
The skip_serializing_if on working_dir on the wire record is the mechanism that makes "omitted means default" true, and the doc comment states it as the contract, but no test in this diff serializes a ConversationThread with working_dir: Some(...) back into the field (only the None case is touched in types_tests.rs, and that only asserts the CreateConversationThread encoding compiles — the previous tests already covered the None path). If the serde attribute is dropped or renamed, the wire record would silently change shape for any bound thread and nothing would fail. A serde_json::from_value(serde_json::to_value(thread)?) round trip asserting Some("/projects/alpha") pins both skip-when-none and preserve-when-some.
[RULE] uncovered-branch ·
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 289f97103b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[serde(default)] | ||
| pub working_dir: Option<String>, |
There was a problem hiding this comment.
Omit absent working directories from create payloads
When a CreateConversationThread with working_dir: None is serialized, #[serde(default)] still emits "workingDir": null; it does not implement the advertised omission behavior. This makes the new public wire shape differ from ConversationThread and from the log entry, and the updated serialization test only initializes the field without checking its output. Add skip_serializing_if = "Option::is_none" and assert that the key is absent.
AGENTS.md reference: AGENTS.md:L66-L70
Useful? React with 👍 / 👎.
| ConversationPurgeStats, ConversationStore, append_message, delete_messages_from, delete_thread, | ||
| ensure_thread, get_messages, list_threads, purge_threads, update_message, update_thread_labels, | ||
| update_thread_title, | ||
| update_thread_title, update_thread_working_dir, |
There was a problem hiding this comment.
Document the new working-directory API
Update threads/README.md alongside this new export: its threads.jsonl format still omits working_dir, and both its store-method and free-function public-surface lists omit update_thread_working_dir. As a result, the module's designated design/API documentation is immediately stale for this public behavior change.
AGENTS.md reference: AGENTS.md:L78-L82
Useful? React with 👍 / 👎.
|
Host side: tinyhumansai/openhuman#7048 (pinned to this branch). Merge this first, then the gitlink moves to the merge commit. |
Summary
Adds an optional per-thread working directory to the conversation thread store (
tinyagents-session::threads), so a host can bind the folder a thread's agent acts in when the conversation starts.ConversationThread.working_dir/CreateConversationThread.working_dir:Option<String>, omitted from the wire record whenNone. Oldthreads.jsonllogs read back unchanged.Upsertlog entry carriesworking_dir. The fold follows the same rule aspersonality_id: a later upsert without the field keeps the bound value, so renames, label updates and channel touches never drop it.ConversationStore::update_thread_working_dir(thread_id, Option<String>, updated_at)plus its free-function shim. Clearing is written as an empty string, because an absent field means "keep"; the fold turns""back intoNone.Validation and policy (which folders are allowed, and only before the first message) stay in the host. This crate only persists the value.
Why
OpenHuman is adding a "where this conversation works" picker above the composer on a new chat. The host side is in tinyhumansai/openhuman (PR to follow, pinned to this branch).
Tests
working_dir_binds_survives_upserts_and_clears: bind on create, keep across a title update, rebind, clear, and read back fromlist_threads.cargo test -p tinyagents-session --lib threads::passes (115 tests);cargo clippy -p tinyagents-session --all-targets -D warningsandcargo fmt --checkare clean.Summary by CodeRabbit