refactor: consolidate session-tree + session-inbox into single session worker - #111
Conversation
…n worker Both surfaces (`session-tree::*` and `session-inbox::*`) now register from one binary (`iii-session`). They share the same engine connection, runtime, and release cadence. Wire-level function ids are unchanged, so callers in harness/web, turn-orchestrator, and provider-router don't need to migrate. Library exposes `session::tree` and `session::inbox` modules; the `session` worker carries the `harness-types` subcrate previously inside session-tree. Configuration merges into one `SessionConfig` (store_backend + engine_url + state_scope). Workers that depended on the two old workers now declare a single `session` dep (harness, turn-orchestrator, provider-router). Release pipeline, registry, and create-tag workflow updated to the unified worker name.
📝 WalkthroughWalkthroughThis PR consolidates two separate session worker packages (session-inbox and session-tree) into a unified session worker exposing both function namespaces (session-tree::* and session-inbox::*) from a single binary, with unified configuration, updated dependencies across the codebase, and comprehensive documentation. ChangesSession Worker Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
skill-check — worker6 verified, 19 skipped (no docs/). 72 errors across the verified workers.
|
Aligns with the bin==name policy from #118 — `iii worker add` extracts the binary by worker name, so the published archive must contain a binary named `session`, not `iii-session`.
skill-check Terminology.EmDash flags every '—' in session/README.md, session/skill.md, and session/skills/*.md. Updates the hand-written README plus the docs/ sources, then mirrors the substitutions into the rendered files so the next render is a no-op.
`cargo fmt --check` flagged `session/src/tree/mod.rs` tuple wrapping and `turn-orchestrator/tests/common/mod.rs` chained `.join(...)` calls. Applied rustfmt; no functional change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@session/tests/common/mod.rs`:
- Around line 7-10: The current binary lookup in session/tests/common/mod.rs
only checks candidate = dir.join("target").join(profile).join("session") (where
dir = Path::new(env!("CARGO_MANIFEST_DIR"))), which fails for workspace builds;
update the probe to also check the workspace-level target directory (e.g., find
the workspace root from dir or its ancestors and also test
workspace_root.join("target").join(profile).join("session")) and use the first
existing path before panic so both per-crate and workspace targets are
supported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0c757c5-f12c-4555-b1c9-92d7e361e802
⛔ Files ignored due to path filters (1)
session/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
.github/workflows/create-tag.yml.github/workflows/release.ymlharness/iii.worker.yamlprovider-router/iii.worker.yamlsession/Cargo.tomlsession/README.mdsession/config.yamlsession/docs/companions.mdsession/docs/intro.mdsession/docs/leaves/append.mdsession/docs/leaves/clone.mdsession/docs/leaves/compact.mdsession/docs/leaves/create.mdsession/docs/leaves/export_html.mdsession/docs/leaves/fork.mdsession/docs/leaves/messages.mdsession/docs/leaves/tree.mdsession/docs/quickstart.mdsession/iii.worker.yamlsession/skill.mdsession/skills/append.mdsession/skills/clone.mdsession/skills/compact.mdsession/skills/create.mdsession/skills/export_html.mdsession/skills/fork.mdsession/skills/messages.mdsession/skills/tree.mdsession/src/main.rssession/src/tree/mod.rssession/tests/common/mod.rssession/tests/manifest.rssession/tests/tree_integration.rssession/tests/tree_restart_e2e.rssession/tests/tree_skill.rs
✅ Files skipped from review due to trivial changes (18)
- session/skills/clone.md
- session/skills/create.md
- session/skills/append.md
- session/skill.md
- session/README.md
- session/skills/export_html.md
- session/docs/leaves/append.md
- session/docs/leaves/export_html.md
- session/docs/leaves/clone.md
- session/docs/leaves/create.md
- session/skills/messages.md
- provider-router/iii.worker.yaml
- session/docs/leaves/messages.md
- session/skills/compact.md
- session/docs/intro.md
- .github/workflows/release.yml
- session/docs/leaves/compact.md
- session/src/tree/mod.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- session/tests/manifest.rs
- harness/iii.worker.yaml
- .github/workflows/create-tag.yml
- session/iii.worker.yaml
- session/tests/tree_integration.rs
- session/Cargo.toml
- session/tests/tree_skill.rs
- session/tests/tree_restart_e2e.rs
- session/src/main.rs
| let dir = Path::new(env!("CARGO_MANIFEST_DIR")); | ||
| for profile in ["debug", "release"] { | ||
| let candidate = dir.join("target").join(profile).join("session-inbox"); | ||
| let candidate = dir.join("target").join(profile).join("session"); | ||
| if candidate.is_file() { |
There was a problem hiding this comment.
Harden fallback binary lookup for workspace targets (Line 7).
The fallback only probes <crate>/target/..., which can fail in workspace setups where artifacts are under <workspace>/target/.... Consider checking both locations before panicking.
Suggested patch
- let dir = Path::new(env!("CARGO_MANIFEST_DIR"));
- for profile in ["debug", "release"] {
- let candidate = dir.join("target").join(profile).join("session");
- if candidate.is_file() {
- return candidate;
- }
- }
+ let manifest_dir = Path::new(env!("CARGO_MANIFEST_DIR"));
+ let target_roots = [
+ manifest_dir.join("target"),
+ manifest_dir.parent().unwrap_or(manifest_dir).join("target"),
+ ];
+ for target_root in target_roots {
+ for profile in ["debug", "release"] {
+ let candidate = target_root.join(profile).join("session");
+ if candidate.is_file() {
+ return candidate;
+ }
+ }
+ }Also applies to: 15-15
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@session/tests/common/mod.rs` around lines 7 - 10, The current binary lookup
in session/tests/common/mod.rs only checks candidate =
dir.join("target").join(profile).join("session") (where dir =
Path::new(env!("CARGO_MANIFEST_DIR"))), which fails for workspace builds; update
the probe to also check the workspace-level target directory (e.g., find the
workspace root from dir or its ancestors and also test
workspace_root.join("target").join(profile).join("session")) and use the first
existing path before panic so both per-crate and workspace targets are
supported.
Summary
session-treeandsession-inboxinto a singlesessionworker (binaryiii-session). Both surfaces register from one process and share engine connection, runtime, and release cadence.session-tree::*,session-inbox::*) are unchanged — no caller migration needed in harness/web, turn-orchestrator, or provider-router.session::treeandsession::inboxmodules;harness-typessubcrate moves with it.SessionConfigmerges the three knobs (store_backend,engine_url,state_scope).harness,turn-orchestrator,provider-router) declare a singlesession: "^0.1.0"dep instead of the two old workers. Release pipeline, registry index, and create-tag worker list collapse to one entry.Test plan
cargo buildinsession/— cleancargo testinsession/— 76 passed, 1 ignored (gatedrestart_e2e)cargo build --testsinturn-orchestrator/,provider-router/,harness/— cleaniii-session --manifestemits valid JSON with all three default-config keyssession/v*) and create-tag choice list resolvesessiondep against a live engineSummary by CodeRabbit
New Features
session-tree::*) and per-session inbox (session-inbox::*) functionality into a single deployable package, eliminating the need for separate worker configurations.Chores