Implement new session APIs#481
Draft
mattklein123 wants to merge 1 commit intomainfrom
Draft
Conversation
Signed-off-by: Matt Klein <mklein@bitdrift.io>
Contributor
Author
|
Depends on bitdriftlabs/api#147. Most of the diff is proto changes. |
Contributor
There was a problem hiding this comment.
Pull request overview
Implements shared-core support for the new async session/state-update protocol by refactoring bd-session to own durable session persistence and queueing of started sessions, wiring state updates through bd-api (handshake + mid-stream updates + ack), and updating logger-facing APIs to remain synchronous by bridging through the async logger task.
Changes:
- Refactors
bd-sessioninto an asyncStrategywith file-backed persistence and a durable started-sessions queue, plus updated tests. - Adds protocol bindings and transport support for
StateUpdateRequest/Responseand workflowon_new_sessionproto surface. - Reworks logger + API integration to propagate session + opaque entity updates (rename from opaque user) via state-update messages.
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| logger-cli/src/service.rs | Updates RPC to trigger new session via async logger handle. |
| logger-cli/src/logger.rs | Bridges CLI session rotation through async logger task. |
| bd-workspace-hack/Cargo.toml | Bumps miniz_oxide workspace-hack pin. |
| bd-workflows/src/config.rs | Rejects on_new_session transition rules (not supported yet). |
| bd-workflows/Cargo.toml | Bumps linux bench dep gungraun. |
| bd-state/src/lib.rs | Adds ENTITY_ID_KEY constant for bd-state persistence. |
| bd-session/src/persistence.rs | Adds file-backed persistence for session state + pending started sessions. |
| bd-session/src/lib_test.rs | Adds integration tests for async session/state-update behavior. |
| bd-session/src/lib.rs | Refactors Strategy into async orchestrator with durable queue + callbacks. |
| bd-session/src/fixed_test.rs | Updates fixed strategy tests for new async/session persistence model. |
| bd-session/src/fixed.rs | Refactors fixed backend into transition provider (no direct persistence). |
| bd-session/src/activity_based_test.rs | Updates activity-based tests for new async/session persistence model. |
| bd-session/src/activity_based.rs | Refactors activity-based backend into transition provider with durable queue integration. |
| bd-session/Cargo.toml | Updates deps (drops bd-key-value, adds proto/persistence dependencies). |
| bd-proto/src/protos/workflow/workflow.rs | Regenerates workflow proto bindings with on_new_session rule type. |
| bd-proto/src/protos/client/api.rs | Regenerates client API proto bindings with StateUpdate* messages. |
| bd-logger/src/test/setup.rs | Updates logger test setup to new session strategy constructors. |
| bd-logger/src/test/logger_integration.rs | Adjusts integration tests for new session + entity ID APIs and bindings. |
| bd-logger/src/test/embedded_logger_integration.rs | Updates embedded logger integration test session construction. |
| bd-logger/src/test/crash_handler_integration.rs | Updates crash handler integration test to new session API return types. |
| bd-logger/src/logger_test.rs | Updates logger unit tests for entity ID updates + session id result type. |
| bd-logger/src/logger.rs | Renames opaque user → entity API, adds sync session bridging through async task. |
| bd-logger/src/builder.rs | Wires entity-ID watch channel and passes session/entity channels into API. |
| bd-logger/src/async_log_buffer_test.rs | Updates async log buffer tests for async session API. |
| bd-logger/src/async_log_buffer.rs | Adds state update message types for entity/session requests and flush changes. |
| bd-error-reporter/src/reporter_test.rs | Adjusts mocks/tests for session ID now returning Result. |
| bd-error-reporter/src/reporter.rs | Updates error reporter to tolerate session ID resolution failures. |
| bd-crash-handler/src/monitor_test.rs | Updates tests for async session ID and new session strategy creation. |
| bd-crash-handler/src/lib.rs | Makes crash monitor session ID retrieval async and fall back on failure. |
| bd-client-common/src/payload_conversion.rs | Adds StateUpdateRequest → ApiRequest conversion. |
| bd-client-common/src/file_system.rs | Adds atomic file write helper + async delete helpers; refactors RealFileSystem deletes. |
| bd-api/src/lib.rs | Removes OPAQUE_USER_ID_KEY constant (migration to state updates). |
| bd-api/src/api_test.rs | Adds tests for handshake/midstream state updates (session + opaque entity). |
| bd-api/src/api.rs | Implements state-update transport (handshake + midstream + ack) using bd-session + entity watch. |
| bd-api/Cargo.toml | Adds dependency on bd-session. |
| PLAN.md | Adds implementation plan document for the state-update/session refactor. |
| Cargo.toml | Bumps ctor version in workspace deps. |
| Cargo.lock | Updates lockfile for new deps/versions and regenerated crates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+170
to
+172
| if let Some(logger) = logger { | ||
| let handle = logger.lock().new_logger_handle(); | ||
| handle.start_new_session().unwrap(); |
Comment on lines
95
to
98
| pub fn start_new_session(&self) { | ||
| let handle = self.logger.lock().new_logger_handle(); | ||
| handle.start_new_session(); | ||
| handle.start_new_session().unwrap(); | ||
| } |
Comment on lines
+278
to
297
| pub fn register_opaque_entity_id(&self, opaque_entity_id: Option<&str>) { | ||
| let opaque_entity_id = opaque_entity_id.map(str::to_string); | ||
| *self.pending_entity_id.lock() = Some( | ||
| opaque_entity_id | ||
| .clone() | ||
| .map_or(PendingEntityIdUpdate::Clear, PendingEntityIdUpdate::Set), | ||
| ); | ||
|
|
||
| if let Err(e) = | ||
| self | ||
| .tx | ||
| .try_send_state_update(async_log_buffer::StateUpdateMessage::SetEntityId( | ||
| opaque_entity_id.clone(), | ||
| )) | ||
| { | ||
| log::debug!("failed to persist entity ID state: {e:?}"); | ||
| } | ||
|
|
||
| self.opaque_entity_updates.send_replace(opaque_entity_id); | ||
| } |
Comment on lines
+337
to
+356
| // Preserve pre-start entity ID updates from the public logger handle. If nothing updated the | ||
| // watch channel yet, seed it from the persisted bd-state value before the API starts. | ||
| let pending_entity_id = pending_entity_id.lock().take(); | ||
| if pending_entity_id.is_none() { | ||
| let initial_opaque_entity_id = { | ||
| let reader = state_store.read().await; | ||
| reader | ||
| .get(StateScope::System, ENTITY_ID_KEY) | ||
| .and_then(|value| { | ||
| let Value_type::StringValue(entity_id) = value.value_type.as_ref()? else { | ||
| return None; | ||
| }; | ||
|
|
||
| (!entity_id.is_empty()).then(|| entity_id.clone()) | ||
| }) | ||
| }; | ||
| if let Some(initial_opaque_entity_id) = initial_opaque_entity_id { | ||
| opaque_entity_updates_tx.send_replace(Some(initial_opaque_entity_id)); | ||
| } | ||
| } |
| } | ||
|
|
||
| let tmp_path = path.with_extension("tmp"); | ||
| tokio::fs::write(&tmp_path, data).await?; |
Comment on lines
+305
to
+314
| .blocking_recv() | ||
| .map_err(|e| anyhow!("failed to receive session ID from async logger: {e}"))? | ||
| } | ||
|
|
||
| pub fn request_start_new_session(&self) -> anyhow::Result<()> { | ||
| let (response_tx, response_rx) = bd_completion::Sender::new(); | ||
| self.try_send_state_update(StateUpdateMessage::StartNewSession(response_tx))?; | ||
| response_rx | ||
| .blocking_recv() | ||
| .map_err(|e| anyhow!("failed to receive start-new-session ack from async logger: {e}")) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implement the shared-core side of the new session API changes.
bd-sessionpersistence, mutation flow, and tests for the async session modelbd-api,bd-logger,bd-crash-handler,bd-error-reporter, andlogger-cli