feat(orchestration): receive session_info harness events into the v2 mirror (B′2)#4710
Conversation
📝 WalkthroughWalkthroughThis PR adds v2 ChangesSession Info Enrichment
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Harness
participant Ingest as classify_v2/persist_message
participant Store as upsert_session
participant DB as sessions table
participant Reader as list_messages_by_session/unread_count
Harness->>Ingest: session_info event
Ingest->>Ingest: extract title, model, handle, repo, branch, capabilities
Ingest->>Store: enriched OrchestrationSession fields
Store->>DB: INSERT/UPDATE with COALESCE (preserve prior intro metadata)
Reader->>DB: query messages/unread count
DB-->>Reader: exclude event_kind = 'session_info' rows
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6d2850d12
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let ingest_seq = if classified.advances_seq { | ||
| store::next_session_seq(c, agent_id, &classified.session_id)? | ||
| } else { | ||
| 0 |
There was a problem hiding this comment.
For a new v2 session whose first envelope is session_info or status, stamping the row with seq = 0 does not actually keep it out of the wake path: ingest_one still publishes OrchestrationSessionMessage, seed_state loads the persisted row, and has_new_work compares latest 0 against a missing cursor (i64::MIN). That means a purely informational intro/status event can run the orchestration graph and send an unsolicited reply before any real content arrives; skip publishing/waking these events or initialize the cursor so 0 state rows are not considered new work.
Useful? React with 👍 / 👎.
| HarnessEventKind::ApprovalRequest(p) => { | ||
| b.body = p.display; | ||
| b.tool_name = non_empty(p.tool_name); | ||
| b.call_id = p.call_id.and_then(non_empty); | ||
| // Drive the roster dot to waiting-approval. | ||
| b.status_state = Some("waiting_approval".to_string()); |
There was a problem hiding this comment.
When a peer sends a v2 approval_request, the new attention path builds the remote approval summary from session.current_detail and its call identity from active_call_id, but this branch only stores status_state plus the message-level call_id. As a result, sessions parked on approval show up in the attention queue without the approval prompt (and without the active call id) even though the payload includes display and call_id; persist display into status_detail and the call id into active_call_id here.
Useful? React with 👍 / 👎.
…mirror Add the session_info HarnessEventKind to OpenHuman's hand-rolled v2 harness mirror so a wrapped agent's session intro/announce (spec §2b) is decoded, classified, and folded into the durable session record. - types.rs: SessionInfo variant (first, adjacently tagged) + SessionInfoPayload struct with wire-byte-identical snake_case field names to the TS emitter (spec §2a); enrich OrchestrationSession with title/model/handle/repo/branch/ capabilities. - ingest.rs: classify_v2 SessionInfo arm — advances_seq=false (seq=0), title doubles as the row body, model falls back to event.model, payload folds onto the session record. - store.rs: additive nullable session columns (capabilities as a JSON array, empty→NULL so COALESCE preserves a prior list); upsert COALESCEs the intro metadata so ordinary events never wipe it and a resumed=true re-intro refreshes rather than duplicates; add session_info to the no-bubble filter so the intro never renders as an in-thread chat bubble or counts as unread. Receiver behavior (spec §4): upsert keyed on wrapper_session_id, lazy-create on the first event of any kind (session_info is enrichment, not a prerequisite), idempotent on event.id, resumed=true updates in place. Tests: session_info decode + wire-field-name round-trip, model/event.model fallback, lazy-create + idempotent + resumed-refresh upsert, capabilities JSON codec (null-safe), and no-bubble hiding from thread/unread.
sanil-23
force-pushed
the
oh-sessinfo-recv
branch
from
c6d2850 to
aedc907
Compare
|
@coderabbitai review pls |
|
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
coderabbitai
Bot
added
agent
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/orchestration/store.rs (1)
241-250: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the new enrichment columns in the legacy migration test.
The migration adds six
sessionscolumns, but the existing pre-v2 schema test still asserts only the older status/event columns. Extending that test would catch regressions in the ALTER path for upgraded stores.Test coverage extension
for (table, column) in [ ("sessions", "status_state"), ("sessions", "current_detail"), ("sessions", "active_call_id"), + ("sessions", "title"), + ("sessions", "model"), + ("sessions", "handle"), + ("sessions", "repo"), + ("sessions", "branch"), + ("sessions", "capabilities"), ("messages", "event_kind"), ("messages", "tool_name"), ("messages", "call_id"), ] {🤖 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 `@src/openhuman/orchestration/store.rs` around lines 241 - 250, The legacy migration test needs to cover the new `sessions` enrichment columns added in `ensure_schema` via `add_column_if_missing`, since it currently only checks the older status/event columns. Update the pre-v2 schema migration test to assert that upgraded stores also gain `title`, `model`, `handle`, `repo`, `branch`, and `capabilities` on `sessions`, so regressions in the ALTER path are caught.
🤖 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.
Nitpick comments:
In `@src/openhuman/orchestration/store.rs`:
- Around line 241-250: The legacy migration test needs to cover the new
`sessions` enrichment columns added in `ensure_schema` via
`add_column_if_missing`, since it currently only checks the older status/event
columns. Update the pre-v2 schema migration test to assert that upgraded stores
also gain `title`, `model`, `handle`, `repo`, `branch`, and `capabilities` on
`sessions`, so regressions in the ALTER path are caught.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 71b00dbc-2b76-48b9-bb52-fe903e10ff14
📒 Files selected for processing (3)
src/openhuman/orchestration/ingest.rssrc/openhuman/orchestration/store.rssrc/openhuman/orchestration/types.rs
2 participants
Summary
session_infoHarnessEventKindto OpenHuman's hand-rolled v2 harness mirror so a wrapped coding-agent's session intro/announce is decoded, classified, and folded into the durable session record — the RECEIVE side of thesession_infofeature (Track B′).sessionstable +OrchestrationSessionwithtitle/model/handle/repo/branch/capabilities, populated from the intro payload (additive, nullable, migration-backfilled).session_infolikestatus/lifecycle:seq=0, does not advance the wake ordinal, and is hidden from the in-thread bubble/unread surfaces.SessionInfoPayloademitter (snake_case) so the mirror matches the wire.Problem
TS SDK v2 (
SessionEnvelopeV2+HarnessEventKind) is already merged upstream and the OpenHuman v2 receiver mirror already decodes the content/status/lifecycle kinds. Thesession_infointro event (spec §2b) — emitted the moment a wrapped session initialises to announce identity, repo/branch, model, and capabilities to its OpenHuman "Master" — had no receiver support: it would fold toUnknownand its metadata would be dropped. OpenHuman couldn't register or render a session's header/metadata from the intro.The mirror's
classify_v2match is exhaustive (no_ =>wildcard), so adding the enum variant requires the classification arm or it won't compile — this is the intended, compile-checked seam.Solution
types.rs—SessionInfo(SessionInfoPayload)as the first (adjacently-tagged,snake_case) variant;SessionInfoPayloadstruct with#[serde(default)]on every field andskip_serializing_if="Option::is_none"on optionals; six enrichment fields onOrchestrationSession.ingest.rs—classify_v2SessionInfoarm:advances_seq=false,titledoubles as the row body,modelfalls back to the frame'sevent.model(spec §2b), and the payload folds onto the session record. Threaded throughClassifiedMessage→persist_message→upsert_session.store.rs— additive nullable session columns (fresh DB viaSCHEMA_DDL, older store via the per-columnadd_column_if_missingguard);capabilitiesstored as a JSON array (empty →NULLsoCOALESCEpreserves a prior list); upsertCOALESCEs intro metadata so ordinary events never wipe it and aresumed=truere-intro refreshes rather than duplicates;session_infoadded to all four no-bubble filters.Receiver behavior (spec §4): upsert keyed on
wrapper_session_id; lazy-create on the first event of any kind (session_info is enrichment, not a prerequisite); idempotent onevent.id;resumed=trueupdates in place.Tradeoff (accepted): two sources of truth persist (TS SDK v2 + this Rust mirror) until the deferred SDK port; the wire contract is owned by the TS emitter and mirrored here.
Submission Checklist
model→event.modelfallback, lazy-create + idempotent + resumed-refresh upsert, capabilities JSON codec (null-safe/malformed), and no-bubble hiding from thread/unread.orchestration::unit tests. Rancargo test -p openhuman --lib openhuman::orchestration::locally → 124 passed, 0 failed. (Full-packagecargo testcompilation is blocked by a pre-existing, unrelated breakage intests/memory_tree_sync_raw_coverage_e2e.rs— arebuild_treearity drift already onmain, untouched here.)docs/TEST-COVERAGE-MATRIX.md(no existing row); this extends that untracked domain rather than a matrixed feature.serde_json/rusqliteare already dependencies. Tests use tempdir SQLite only.session_inforeceiver; companion to the existing v2 harness stream (openhuman feat(orchestration): receive typed v2 harness-session stream #4652, tiny.place #229/#230).Impact
OrchestrationSessiongains additive JSON fields over RPC (extra fields are ignored by older renderers — backward compatible).user_version/table_infoguarded migration; idempotent on re-open; existing rows defaultNULL/empty.session_infotoUnknownand safely ignores it; a newer receiver reads the enrichment. Wire field names match the TS emitter exactly.repo/branch/cwdare only ever received from the paired peer's DM (not broadcast).Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
sanil-23:oh-sessinfo-recvc6d2850d1(session_info addition; stacked on the v2 receiver mirror commits)Validation Run
pnpm --filter openhuman-app format:check— N/A (noapp/srcchange)pnpm typecheck— N/A (no TypeScript change)cargo test -p openhuman --lib openhuman::orchestration::→ 124 passed, 0 failedcargo fmt --checkclean on the three changed files; lib compiles (unit-test build green)app/src-taurichange)Validation Blocked
command:cargo test -p openhuman orchestration::(full package)error:unrelated pre-existing compile break intests/memory_tree_sync_raw_coverage_e2e.rs(rebuild_treetakes 3 args, test passes 4) — already onmain, not touched by this PRimpact:none on this change; validated via the--libscope above. CI Lite scopes Rust tests to the changedorchestrationdomain.Behavior Changes
session_infointro, enriching the session record (title/model/handle/repo/branch/capabilities) instead of dropping it asUnknown.Parity Contract
Unknownfold intact for genuinely unmodeled kinds.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes