refactor(infinity-protocol)!: session_id becomes root_thread_id; add ThreadRef - #115
refactor(infinity-protocol)!: session_id becomes root_thread_id; add ThreadRef#115MingweiSamuel wants to merge 4 commits into
Conversation
Deploying infinity with
|
| Latest commit: |
43ca0b1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3c1da8a4.infinity-dc7.pages.dev |
| Branch Preview URL: | https://mingwei-strkind2.infinity-dc7.pages.dev |
85abb33 to
6d22148
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed runtime-safety/API-correctness issues (UTF-8 byte slicing panics in TUI rendering and inconsistent/unsafe handling of ThreadRef/remote naming/parsing) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR is stage 3 of the string-ID → typed-ID migration, implementing the decision that a session is its root thread by renaming session_id → root_thread_id across the daemon wire protocol and introducing ThreadRef / RemoteName to replace ad-hoc "remote/uuid" composite strings while keeping the serialized value format stable.
Changes:
- Refactors
infinity-protocolmessage schemas and shared data types to useThreadRefand typedRemoteName, with protocol field renames for the session/root-thread family. - Updates Rust daemon + sub-agents (Slack bot, CLI/ACP, Lambda, RAP bridge/client) to use typed thread IDs in APIs and internal maps, removing string-splitting logic for remote-prefixed IDs.
- Updates TypeScript clients (
infinity-web,infinity-ui) to the new JSON field names/types.
File summaries
| File | Description |
|---|---|
| infinity-web/src/App.tsx | Updates client message field names (session_id → root_thread_id) and related handling for connect/migrate flows. |
| infinity-ui/src/types.ts | Updates TS wire types for renamed fields and SwitchModel’s corrected thread_id naming. |
| crates/rap-client/src/toolset_loader.rs | Switches toolset-loading APIs to typed ThreadId<str> parameters. |
| crates/rap-client/src/notifier.rs | Switches notifier APIs to typed ThreadId<str> parameters. |
| crates/infinity-slack-bot/tests/dataflow_sim.rs | Updates test fixtures to new protocol field names and ThreadRef types. |
| crates/infinity-slack-bot/src/session_store.rs | Stores Slack thread→session mapping as ThreadRef while preserving on-disk JSON format. |
| crates/infinity-slack-bot/src/flow.rs | Adapts Slack dataflow to root_thread_id naming in Connected. |
| crates/infinity-slack-bot/src/daemon_sidecar.rs | Types daemon commands with ThreadRef and updates Connected/UserInput handling. |
| crates/infinity-slack-bot/src/daemon_client.rs | Types daemon client APIs with ThreadRef and updates connect/send input payloads. |
| crates/infinity-rap-bridge/src/lib.rs | Switches RAP bridge connect API to typed ThreadId<str>. |
| crates/infinity-protocol/src/lib.rs | Introduces RemoteName + ThreadRef and applies protocol-wide field/type renames to use typed IDs. |
| crates/infinity-protocol/Cargo.toml | Adds strkind dependency for new typed string kinds. |
| crates/infinity-daemon/tests/rap_callback_flow.rs | Updates daemon tests to ThreadRef/root_thread_id protocol changes. |
| crates/infinity-daemon/tests/keep_alive.rs | Updates keep-alive tests to ThreadRef and typed ID generation. |
| crates/infinity-daemon/src/session/tests.rs | Updates session manager tests for HashMap<ThreadRef, SessionInfo> session maps. |
| crates/infinity-daemon/src/session/observer.rs | Emits ThreadRef in replay/history messages instead of string IDs. |
| crates/infinity-daemon/src/session/mod.rs | Updates session manager wire messages and session listing maps to ThreadRef. |
| crates/infinity-daemon/src/session/display.rs | Updates display/event conversion to attribute messages with ThreadRef. |
| crates/infinity-daemon/src/remote.rs | Types remote configs/maps with RemoteName/ThreadRef and replaces string prefixing with ThreadRef ops. |
| crates/infinity-daemon/src/rap_servers.rs | Updates broadcast thread_id to ThreadRef. |
| crates/infinity-daemon/src/migrate.rs | Refactors migration orchestration to typed ThreadRef/RemoteName and removes composite-string juggling. |
| crates/infinity-daemon/src/memory_store.rs | Updates thread tree/session info to store ThreadRef for thread relationships. |
| crates/infinity-daemon/src/ids.rs | Changes IdSource::generate() to return typed ThreadId. |
| crates/infinity-daemon/src/client_handler.rs | Replaces remote-id string surgery with ThreadRef prefix/strip operations and updates message routing. |
| crates/infinity-agent-lambda/src/event_handler.rs | Updates toolset loading callsites to pass typed thread IDs. |
| crates/infinity-agent-cli/tests/tui_snapshots.rs | Updates CLI snapshot test events to carry ThreadRef. |
| crates/infinity-agent-cli/tests/tui_reflow_snapshots.rs | Updates snapshot test session maps to use ThreadRef keys. |
| crates/infinity-agent-cli/tests/tui_flow_snapshots.rs | Updates CLI flow tests for ThreadRef-typed session IDs in events/maps. |
| crates/infinity-agent-cli/tests/common/mod.rs | Updates harness types/channels to use ThreadRef instead of String IDs. |
| crates/infinity-agent-cli/src/terminal.rs | Keys terminal state/maps by ThreadRef and updates display attribution handling accordingly. |
| crates/infinity-agent-cli/src/session_picker.rs | Types session picker IDs as ThreadRef and updates rendering logic. |
| crates/infinity-agent-cli/src/main.rs | Updates remote config creation/checks for typed RemoteName. |
| crates/infinity-agent-cli/src/daemon_client.rs | Updates daemon client wiring and --session prefix matching for ThreadRef. |
| crates/infinity-agent-cli/src/acp_server.rs | Types ACP↔daemon session bookkeeping with ThreadRef on the daemon side while preserving ACP string IDs. |
| Cargo.lock | Records the new strkind dependency in the lockfile. |
Review details
Suppressed comments (1)
crates/infinity-daemon/src/client_handler.rs:374
- Same issue here: defaulting a missing
active_remote_nameto an emptyRemoteNamecan cause incorrect stripping/prefixing and masks an invariant violation. Considerexpecting the remote name whenremote_proxy_txis set, instead of synthesizing one.
let rn = active_remote_name.clone().unwrap_or_else(|| "".into());
match &msg {
- Files reviewed: 34/35 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
All the other Copilot comments are correct but also somewhat irrelevant, so up to you if you want to address here or later. |
|
I will put in separate PR |
…ThreadRef Stage 3 of the string-ID to typed-ID migration (#108), implementing the decision that a session IS its root thread. This is the one deliberate wire-format break of the migration: JSON field names change; values are unchanged (ThreadRef serializes as the historical composite string). Verified end-to-end: full test suite + Playwright web e2e (7/7) against the live daemon. * infinity-protocol: `session_id` renamed to `root_thread_id` across ClientMessage/DaemonMessage (`new_session_id` to `new_root_thread_id`; SwitchModel's mislabeled `session_id` is now honestly `thread_id`). New `ThreadRef { remote: Option<RemoteName>, id: ThreadId }` replaces the ad-hoc `"remote/uuid"` string encoding: Display/FromStr/serde use the composite form, `.prefixed()`/`.strip()` replace split_once logic; Ord derives (local-before-remote, then id). New `RemoteName` kind; typed `location`/`to`/`on` fields, `SessionInfo.remote`, `RemoteInfo.name`, `SubthreadInfo`, and sessions maps keyed by `ThreadRef`. Client-facing fields are `ThreadRef`; daemon-to-daemon variants (Emigrate family) are plain `ThreadId` * infinity-daemon: `IdSource::generate() -> ThreadId`; client_handler's is_remote_session/strip_id string surgery replaced by ThreadRef operations; remote.rs session maps and control workers typed (RemoteConfig.name: RemoteName, remotes.json unchanged); migrate.rs orchestration fully typed (composite-string juggling eliminated) * rap-client: `ToolsetLoader::load_toolsets` and `RapNotifier::request_migration` take `&ThreadId`; rap-bridge connect too * infinity-agent-cli (sub-agent): terminal state keyed by ThreadRef, session picker/daemon client typed, ACP boundary documented (ACP-side ids stay String; acp-sessions.json format unchanged); zero terminal snapshot changes * infinity-slack-bot + TS (sub-agent): SessionStore/DaemonCommand/client typed with ThreadRef (persisted JSON unchanged); infinity-ui types.ts + infinity-web App.tsx field renames — including the App.tsx conflation `UserInput { session_id: p.thread_id }` which the rename makes honest BREAKING CHANGE: daemon wire protocol field renames (session_id family); old clients/daemons cannot interop across this version for the renamed messages. TS clients updated in the same change. Deferred to the &ThreadId<str> loosening rebase: Stage-3's new `&ThreadId` params (handle_emigrate, connect_remote_session, load_toolsets, etc.). Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #115
The Rust protocol (crates/infinity-protocol) recently renamed the field in `ClientMessage::UserInput` from `root_thread_id` to `thread_id`, but the web interface was still sending the old field name, which would fail deserialization on the daemon side. - infinity-ui/src/types.ts: rename the `UserInput` variant field to `thread_id` to match the protocol. - infinity-web/src/App.tsx: update both `UserInput` senders (pending-input flush on Connected, and the normal send path) to use `thread_id`. Verified: `tsc --noEmit` passes for infinity-web, prettier format check passes, and all 7 Playwright web e2e tests pass (`cargo test -p infinity-daemon --features e2e-web --test web_e2e`). The pre-existing `streamdown/styles.css` tsc error in infinity-ui is unrelated to this change. Co-authored-by: Infinity 🤖 <infinity@hydro.run> PR: #115
e470bdc to
9a8a235
Compare
Stage 3 of the string-ID to typed-ID migration (#108), implementing the
decision that a session IS its root thread. This is the one deliberate
wire-format break of the migration: JSON field names change; values are
unchanged (ThreadRef serializes as the historical composite string).
Verified end-to-end: full test suite + Playwright web e2e (7/7) against
the live daemon.
session_idrenamed toroot_thread_idacrossClientMessage/DaemonMessage (
new_session_idtonew_root_thread_id;SwitchModel's mislabeled
session_idis now honestlythread_id). NewThreadRef { remote: Option<RemoteName>, id: ThreadId }replaces thead-hoc
"remote/uuid"string encoding: Display/FromStr/serde use thecomposite form,
.prefixed()/.strip()replace split_once logic; Ordderives (local-before-remote, then id). New
RemoteNamekind; typedlocation/to/onfields,SessionInfo.remote,RemoteInfo.name,SubthreadInfo, and sessions maps keyed byThreadRef. Client-facingfields are
ThreadRef; daemon-to-daemon variants (Emigrate family) areplain
ThreadIdIdSource::generate() -> ThreadId; client_handler'sis_remote_session/strip_id string surgery replaced by ThreadRef
operations; remote.rs session maps and control workers typed
(RemoteConfig.name: RemoteName, remotes.json unchanged); migrate.rs
orchestration fully typed (composite-string juggling eliminated)
ToolsetLoader::load_toolsetsandRapNotifier::request_migrationtake&ThreadId; rap-bridge connect toosession picker/daemon client typed, ACP boundary documented (ACP-side
ids stay String; acp-sessions.json format unchanged); zero terminal
snapshot changes
typed with ThreadRef (persisted JSON unchanged); infinity-ui types.ts +
infinity-web App.tsx field renames — including the App.tsx conflation
UserInput { session_id: p.thread_id }which the rename makes honestBREAKING CHANGE: daemon wire protocol field renames (session_id family);
old clients/daemons cannot interop across this version for the renamed
messages. TS clients updated in the same change.
Deferred to the &ThreadId loosening rebase: Stage-3's new
&ThreadIdparams (handle_emigrate, connect_remote_session, load_toolsets, etc.).