feat(orchestration): receive typed v2 harness-session stream#4652
Conversation
Add a hand-rolled SessionEnvelopeV2 beside the v1 mirror: a typed HarnessEvent carrying a snake_case, adjacently-tagged HarnessEventKind (user_prompt, agent_message, agent_thinking, tool_call, tool_result, approval_request, status, lifecycle, error, unknown) plus per-kind payload structs. Decoding folds any unrecognised/garbled kind to Unknown rather than failing the parse, so a future wire kind never silently routes a DM to Master. session_key() keeps the shared wrapper_session_id routing key (v1 parity). Also add the persisted run-state/event fields to OrchestrationSession (status_state, current_detail, active_call_id) and OrchestrationMessage (event_kind, tool_name, call_id), and derive Default on both structs + ChatKind so existing call sites keep the new fields at their NULL default.
Additively add the v2 columns to the orchestration store: sessions gains status_state/current_detail/active_call_id, messages gains event_kind/tool_name/call_id. New columns are in SCHEMA_DDL for fresh DBs and back-filled on older stores via an idempotent, table_info-guarded add_column_if_missing (SQLite has no ADD COLUMN IF NOT EXISTS), so a v1 store upgrades in place with existing rows defaulting NULL. upsert_session COALESCEs the run-state fields (a content event never wipes the last status; a status event overwrites), insert_message writes the event columns, and every session/message row-reader SELECT is updated in lockstep via shared map_session_row/map_message_row mappers. Adds a migration test proving an old-schema DB opens, upgrades, reads legacy rows as NULL, and re-opens idempotently.
Version-dispatch classify_message: try v2, then v1, then Master (both envelope versions discriminate on envelope_version, so they coexist and persist into one session model, keyed on the shared wrapper_session_id). classify_v2 switches on event.kind, mapping each to the persisted fields: content events (prompts/messages/thinking/tool_call/tool_result/approval/error) become thread messages that advance the monotonic wake ordinal; status/lifecycle/unknown are session-state-only and stamp seq 0 so they still dedupe + ack but never spuriously wake the front-end graph. status/approval/lifecycle/error write the session run-state columns; error also records orchestration:last_error. The store-assigned ingest ordinal (the tinyhumansai#4583 line-0 wake fix) is preserved untouched — the wire event.seq is retained on ClassifiedMessage for parity/debugging only, never for persisted ordering. Adds tests for every v2 kind, v1/v2 coexistence, status-without-seq-advance, and error last_error.
derive_status now reads the persisted status_state, mapping running|running_tool -> running, waiting_approval -> waiting-approval, idle, stopped, errored; v1/legacy sessions (no run-state) keep the original recency heuristic. A running/running_tool session that has gone silent downgrades to stopped (staleness fallback) so a crashed instance never sits on a stuck green dot, while waiting-approval/errored are honoured even when stale. SessionSummary.current_task now prefers the harness status.detail (current-activity / active tool) over the last-message preview. Adds tests for the run-state mapping, staleness downgrade, and recency fallback.
Thread ..Default::default() through the OrchestrationSession/OrchestrationMessage literals in the wake-graph reply persist, subconscious-directive persist, and their test helpers so the additive v2 model fields land at their NULL default without changing behaviour.
📝 WalkthroughWalkthroughAdds v2 harness session handling with typed events and persistence, remote approval attention items, a publish-identity RPC and UI flow, chat rendering by event kind, localized identity copy, and a vendored dependency update. ChangesHarness v2, remote approvals, and identity publish
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant SelfIdentityCard
participant OrchestrationSidebar
participant TinyPlaceOrchestrationTab
participant orchestrationClient
participant Backend
SelfIdentityCard->>OrchestrationSidebar: publish / republish click
OrchestrationSidebar->>TinyPlaceOrchestrationTab: onPublishIdentity()
TinyPlaceOrchestrationTab->>orchestrationClient: publishIdentity()
orchestrationClient->>Backend: orchestration_publish_identity
Backend-->>orchestrationClient: refreshed SelfIdentity
orchestrationClient-->>TinyPlaceOrchestrationTab: SelfIdentity or error
sequenceDiagram
participant DM
participant Ingest
participant Store
participant Attention
DM->>Ingest: raw v1/v2 envelope
Ingest->>Store: persist session/message and run-state
Store->>Attention: waiting_approval sessions
Attention->>Attention: remote_approval_signals()
Attention->>Attention: assemble_attention()
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
… unread These typed v2 rows are persisted for relay dedup but are run-state signals, not conversation. Exclude them from list_messages_by_session, unread_count, and latest_message_preview (roster task line) so the visible thread and unread badge show only real content; v1 rows (NULL event_kind) are unaffected.
The core already serializes eventKind/toolName/callId on each message; surface them on OrchestrationMessage + HarnessEventKind, and thread them through ChatMessage/mapMessage so the renderer can differentiate typed v2 rows. v1 rows leave the fields undefined.
MessageBubble now branches on eventKind: tool_call (▶, monospace command), tool_result (↳, monospace output), agent_thinking (italic muted, ∴), error (✕, coral), approval_request (⚠, amber), each with a left accent; the tool name renders as a mono chip and long bodies scroll. Legacy v1 rows keep the plain dot style. Tests cover each kind + the v1 fallback.
…tion queue Add a 4th attention source: peer agent sessions parked on a tool-approval decision (Phase 1 persists status.state='waiting_approval' + the prompt in current_detail + active_call_id). They share the Approval kind with the local gate, so they lead the queue and count under counts.approvals, but their action opens the orchestration chat window (OpenSession) rather than the local approval surface. Namespaced 'remote-approval:<sessionId>' to avoid list-key collision with local 'approval:<requestId>' items.
gather_remote_approval_signals mirrors gather_unread_signals (best-effort store read; failure logs warn + empty vec, never sinks the queue). handle_attention reads it as the 4th source and passes it to assemble_attention. Debug-logs each waiting session under [orchestration_rpc].
Assert an approval-kind item with an open-session action renders the amber Review verb + its summary and routes open-session on click.
Line-wrap + import ordering the formatter applied after the phase-2 commits; keeps format:check green (pre-push only runs rust:check, so these weren't gated).
…erable Wraps the signal_register_encryption_key primitive (upserts the directory card — minting a minimal one if none exists — and writes the wallet's Signal encryption key into its metadata) then re-reads self_identity, so a handle-holding-but-unpublished agent can be made messageable in one call. No @handle registration and no x402 payment; new paid handles stay in the tiny.place registry surface.
…card SelfIdentityCard flagged an un-messageable identity but offered no fix. Add a button (shown only while undiscoverable) that calls orchestration_publish_identity to publish the directory card + Signal key, then adopts the refreshed identity so the card flips to discoverable in place. Threaded Tab -> Sidebar -> Card; the tab owns the RPC + in-flight/error state.
Native translations for the 3 new self-identity keys (makeDiscoverable, publishing, publishFailed) and the reworded undiscoverableHint across all 13 non-English locales, keeping i18n:check parity green.
"Make discoverable" published the directory card + encryption key but not the Signal prekey bundle, so a peer could resolve the address yet 404 on /keys/<addr>/bundle when initiating the first DM — discoverable but not actually messageable. publish_identity now also calls signal_provision (signed pre-key + one-time pre-keys) so a fresh identity is deliverable-to in one action.
…er hangs self_identity + signal_key_status did unbounded directory lookups (reverse, get_agent). Fast relay + no card hid it; a degraded relay with a real card to fetch made them stall, pinning the identity card on "Loading identity…" forever. Wrap each in a 5s timeout that degrades to not-published/no-handle, so the card always resolves and the publish action stays reachable.
Once card+key are up the identity reads discoverable and the make-discoverable button hides — but the prekey bundle can still be missing, leaving no way to re-provision. Add a subtle "Republish keys" button shown while discoverable that calls the same publishIdentity (card + key + bundle).
coderabbitai
Bot
added
agent
![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: 75299f039a
ℹ️ 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".
| current_detail = COALESCE(excluded.current_detail, sessions.current_detail), | ||
| active_call_id = COALESCE(excluded.active_call_id, sessions.active_call_id)", |
There was a problem hiding this comment.
When a later v2 status event omits or clears detail/active_call_id (for example transitioning from running_tool to idle/stopped), classify_v2 stores those fields as None; this COALESCE then preserves the previous row instead of applying the fresh status. In that scenario the roster keeps showing the old current task/call id even after the remote harness has gone idle, so content events should preserve old values but status updates need a way to clear them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 811647c — a status event is now flagged authoritative and overwrites status_state/current_detail/active_call_id via a new store::apply_run_state UPDATE (SET, not COALESCE), so a running_tool → idle transition clears the stale detail/active_call_id. Content events keep COALESCE so they never wipe a live status. Tests cover both directions.
| seq, | ||
| body: b.body, | ||
| timestamp, | ||
| event_kind: Some(kind_str), |
There was a problem hiding this comment.
For a future or garbled v2 event such as kind: "teleport", decoded() deliberately maps it to HarnessEventKind::Unknown and advances_seq is set false, but this line still persists the raw kind string. The store readers only hide rows whose event_kind is exactly status, lifecycle, or unknown, so these forward-compatible unknown events will leak into the chat thread/unread count/preview instead of staying hidden as intended.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 811647c — an Unknown-folded event now persists the literal event_kind = "unknown" (via a kind_override) instead of the raw wire kind, so the store readers' hide-set (status/lifecycle/unknown) keeps a forward/garbled kind out of the thread, unread count, and preview. Test asserts kind:"teleport" → event_kind "unknown".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
app/src/components/intelligence/OrchestrationChatPrimitives.tsx (1)
145-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDecorative glyphs have no text alternative for screen readers.
Glyphs like
▶,↳,∴,⚠,✕convey the event kind but are rendered as plain text with noaria-hidden/title/visually-hidden label. Screen readers will announce the raw symbol rather than its meaning (e.g., "tool call", "error"). Consider addingaria-hidden="true"to the glyph span plus a visually-hidden label, or atitleattribute using the event kind.🤖 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 `@app/src/components/intelligence/OrchestrationChatPrimitives.tsx` around lines 145 - 176, The decorative glyph rendering in MessageBubble lacks an accessible text alternative, so screen readers will announce the raw symbols instead of the event meaning. Update the glyph branch in MessageBubble to hide the symbol from assistive tech with aria-hidden and provide an accessible label for the event kind, either via a visually hidden text node or a title/label derived from bubbleStyle(message.eventKind) and message.eventKind.app/src/components/intelligence/OrchestrationChatPrimitives.test.tsx (1)
78-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGood coverage of the new bubble-kind styling; consider extending to
approval_request/user_prompt.Tests thoroughly cover
tool_call,tool_result,agent_thinking,error, and the legacy v1 fallback.approval_request(with its⚠glyph and amber accent) anduser_promptstyling aren't exercised — worth adding for fullbubbleStylecoverage.🤖 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 `@app/src/components/intelligence/OrchestrationChatPrimitives.test.tsx` around lines 78 - 115, The MessageBubble test suite covers most bubble styles but is missing coverage for approval_request and user_prompt. Add assertions in OrchestrationChatPrimitives.test.tsx that render MessageBubble with eventKind set to approval_request and user_prompt, and verify their unique styling symbols/classes from bubbleStyle are applied, including the ⚠ glyph for approval_request and the user_prompt presentation.app/src/lib/orchestration/orchestrationClient.ts (1)
285-293: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDoc comment omits prekey bundle provisioning.
The JSDoc says this publishes "directory card + Signal encryption key," but per
handle_publish_identity(schemas.rs), the RPC also provisions the X3DH prekey bundle — required for a peer's first DM to succeed. Worth mentioning in the comment so callers understand the full scope of what "Make discoverable" repairs.✏️ Suggested doc tweak
/** - * Make this agent discoverable: publish (or refresh) its directory card + Signal - * encryption key for the wallet's current identity, then return the updated + * Make this agent discoverable: publish (or refresh) its directory card, Signal + * encryption key, and X3DH prekey bundle for the wallet's current identity, then return the updated * {`@link` SelfIdentity}. No `@handle` registration and no payment — it repairs the * common "has an identity but card/key aren't published" gap that makes every * inbound DM 404. Powers the SelfIdentityCard's "Make discoverable" action. */🤖 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 `@app/src/lib/orchestration/orchestrationClient.ts` around lines 285 - 293, The JSDoc for publishIdentity is incomplete: it mentions only the directory card and Signal encryption key, but the RPC also provisions the X3DH prekey bundle. Update the comment on publishIdentity in orchestrationClient to describe the full scope of handle_publish_identity, including prekey bundle provisioning, so callers understand what “Make discoverable” refreshes.app/src/components/intelligence/SelfIdentityCard.tsx (1)
140-187: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueActual publish error message is discarded — only a generic label is shown.
publishErrorcarries the real failure message (set fromerror.messageinTinyPlaceOrchestrationTab.publishIdentity), but both render sites only show the staticidentity.publishFailedstring, neverpublishErroritself. Tests assert this generic text, so it looks intentional, but consider at least exposing the raw message via atitleattribute for troubleshooting.💡 Optional: surface raw error via title
{publishError ? ( <span data-testid="tinyplace-self-identity-republish-error" + title={publishError} className="text-[10px] text-coral-600 dark:text-coral-300"> {t('tinyplaceOrchestration.identity.publishFailed')} </span> ) : null}🤖 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 `@app/src/components/intelligence/SelfIdentityCard.tsx` around lines 140 - 187, The publish failure UI in SelfIdentityCard is hiding the real error details and only rendering the generic publishFailed label. Update the two publishError render sites in SelfIdentityCard to surface the actual publishError value from TinyPlaceOrchestrationTab.publishIdentity, while keeping the existing translated fallback text for users. If you don’t want to change visible text, at least attach publishError as a title on the error element so the raw message is available for troubleshooting.
🤖 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 `@app/src/components/intelligence/OrchestrationChatPrimitives.test.tsx`:
- Around line 78-115: The MessageBubble test suite covers most bubble styles but
is missing coverage for approval_request and user_prompt. Add assertions in
OrchestrationChatPrimitives.test.tsx that render MessageBubble with eventKind
set to approval_request and user_prompt, and verify their unique styling
symbols/classes from bubbleStyle are applied, including the ⚠ glyph for
approval_request and the user_prompt presentation.
In `@app/src/components/intelligence/OrchestrationChatPrimitives.tsx`:
- Around line 145-176: The decorative glyph rendering in MessageBubble lacks an
accessible text alternative, so screen readers will announce the raw symbols
instead of the event meaning. Update the glyph branch in MessageBubble to hide
the symbol from assistive tech with aria-hidden and provide an accessible label
for the event kind, either via a visually hidden text node or a title/label
derived from bubbleStyle(message.eventKind) and message.eventKind.
In `@app/src/components/intelligence/SelfIdentityCard.tsx`:
- Around line 140-187: The publish failure UI in SelfIdentityCard is hiding the
real error details and only rendering the generic publishFailed label. Update
the two publishError render sites in SelfIdentityCard to surface the actual
publishError value from TinyPlaceOrchestrationTab.publishIdentity, while keeping
the existing translated fallback text for users. If you don’t want to change
visible text, at least attach publishError as a title on the error element so
the raw message is available for troubleshooting.
In `@app/src/lib/orchestration/orchestrationClient.ts`:
- Around line 285-293: The JSDoc for publishIdentity is incomplete: it mentions
only the directory card and Signal encryption key, but the RPC also provisions
the X3DH prekey bundle. Update the comment on publishIdentity in
orchestrationClient to describe the full scope of handle_publish_identity,
including prekey bundle provisioning, so callers understand what “Make
discoverable” refreshes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 183e05d7-31fb-490b-9203-cebf2f43af06
📒 Files selected for processing (34)
app/src/components/intelligence/AttentionQueueItem.test.tsxapp/src/components/intelligence/OrchestrationChatPrimitives.test.tsxapp/src/components/intelligence/OrchestrationChatPrimitives.tsxapp/src/components/intelligence/OrchestrationSidebar.test.tsxapp/src/components/intelligence/OrchestrationSidebar.tsxapp/src/components/intelligence/SelfIdentityCard.test.tsxapp/src/components/intelligence/SelfIdentityCard.tsxapp/src/components/intelligence/TinyPlaceOrchestrationTab.test.tsxapp/src/components/intelligence/TinyPlaceOrchestrationTab.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/lib/orchestration/orchestrationClient.tsapp/src/lib/orchestration/useOrchestrationChats.tssrc/openhuman/orchestration/attention.rssrc/openhuman/orchestration/ingest.rssrc/openhuman/orchestration/ops.rssrc/openhuman/orchestration/schemas.rssrc/openhuman/orchestration/store.rssrc/openhuman/orchestration/types.rssrc/openhuman/tinyplace/manifest.rssrc/openhuman/tinyplace/mod.rsvendor/tinyplace
…sites Post-merge with upstream/main: new master/session echo paths construct OrchestrationSession/OrchestrationMessage without the additive v2 columns (status_state/current_detail/active_call_id, event_kind/tool_name/call_id). Fill with ..Default::default() (None) — upsert_session COALESCEs run-state so a content echo never wipes a live status.
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 `@src/openhuman/orchestration/ops.rs`:
- Around line 338-368: Wrap the read-then-write flow in thread_reply_to_origin
with store::in_immediate_txn so the MAX(seq)+1 lookup and the subsequent
upsert_session and insert_message happen under the same write lock. Use the
existing store::with_connection block and the store helpers next_session_seq,
upsert_session, and insert_message as the place to apply the transactional
guard, matching the concurrency protection used by persist_outgoing_reply.
🪄 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: f876dcf6-5dec-4d05-bb00-1bb144997b56
📒 Files selected for processing (7)
src/openhuman/orchestration/ops.rssrc/openhuman/orchestration/schemas.rssrc/openhuman/orchestration/store.rssrc/openhuman/orchestration/tools.rssrc/openhuman/orchestration/types.rssrc/openhuman/tinyplace/manifest.rssrc/openhuman/tinyplace/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/openhuman/tinyplace/mod.rs
- src/openhuman/tinyplace/manifest.rs
- src/openhuman/orchestration/schemas.rs
- src/openhuman/orchestration/types.rs
- src/openhuman/orchestration/store.rs
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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 `@src/openhuman/orchestration/ops.rs`:
- Around line 338-368: Wrap the read-then-write flow in thread_reply_to_origin
with store::in_immediate_txn so the MAX(seq)+1 lookup and the subsequent
upsert_session and insert_message happen under the same write lock. Use the
existing store::with_connection block and the store helpers next_session_seq,
upsert_session, and insert_message as the place to apply the transactional
guard, matching the concurrency protection used by persist_outgoing_reply.
🪄 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: f876dcf6-5dec-4d05-bb00-1bb144997b56
📒 Files selected for processing (7)
src/openhuman/orchestration/ops.rssrc/openhuman/orchestration/schemas.rssrc/openhuman/orchestration/store.rssrc/openhuman/orchestration/tools.rssrc/openhuman/orchestration/types.rssrc/openhuman/tinyplace/manifest.rssrc/openhuman/tinyplace/mod.rs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/openhuman/tinyplace/mod.rs
- src/openhuman/tinyplace/manifest.rs
- src/openhuman/orchestration/schemas.rs
- src/openhuman/orchestration/types.rs
- src/openhuman/orchestration/store.rs
🛑 Comments failed to post (1)
src/openhuman/orchestration/ops.rs (1)
338-368: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Confirm seq uniqueness + that in_immediate_txn is the established seq-race guard. rg -nP 'UNIQUE|PRIMARY KEY|CREATE (UNIQUE )?INDEX' src/openhuman/orchestration/store.rs | rg -i 'seq|messages' rg -nP '\bin_immediate_txn\b' -C2 src/openhuman/orchestration/store.rs src/openhuman/orchestration/ops.rsRepository: tinyhumansai/openhuman
Length of output: 2834
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- src/openhuman/orchestration/ops.rs around thread_reply_to_origin ---' sed -n '320,390p' src/openhuman/orchestration/ops.rs | cat -n echo echo '--- src/openhuman/orchestration/ops.rs around persist_outgoing_reply ---' sed -n '920,950p' src/openhuman/orchestration/ops.rs | cat -n echo echo '--- src/openhuman/orchestration/store.rs relevant seq/index semantics ---' sed -n '70,100p' src/openhuman/orchestration/store.rs | cat -n echo sed -n '138,155p' src/openhuman/orchestration/store.rs | cat -n echo sed -n '1438,1475p' src/openhuman/orchestration/store.rs | cat -nRepository: tinyhumansai/openhuman
Length of output: 9950
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- src/openhuman/orchestration/ops.rs thread_reply_to_origin callsite ---' rg -n "thread_reply_to_origin|persist_outgoing_reply|in_immediate_txn|next_session_seq|insert_message" src/openhuman/orchestration/ops.rs -C 3Repository: tinyhumansai/openhuman
Length of output: 8112
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- insert_message / next_session_seq definitions ---' rg -n "fn (insert_message|next_session_seq)\b|INSERT OR IGNORE|ON CONFLICT|seq INTEGER|UNIQUE index on \(agent_id, session_id, seq\)" src/openhuman/orchestration/store.rs -C 4 echo echo '--- thread_reply_to_origin caller context around wake handling ---' sed -n '788,812p' src/openhuman/orchestration/ops.rs | cat -nRepository: tinyhumansai/openhuman
Length of output: 6143
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- messages table / indexes in src/openhuman/orchestration/store.rs ---' rg -n "CREATE TABLE messages|CREATE UNIQUE INDEX|CREATE INDEX.*messages|PRIMARY KEY.*messages|UNIQUE.*messages|seq.*messages" src/openhuman/orchestration/store.rs -C 3 echo echo '--- broader context around messages schema ---' sed -n '180,240p' src/openhuman/orchestration/store.rs | cat -n echo echo '--- any migration adding uniqueness on (agent_id, session_id, seq) for messages ---' rg -n "(agent_id, session_id, seq)|messages.*seq|seq.*messages" src/openhuman/orchestration/store.rs -C 2Repository: tinyhumansai/openhuman
Length of output: 7905
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- call graph / other writers to origin_session_id path ---' rg -n "origin_session_id|thread_reply_to_origin|advance_cursor|clear_pending_ask|persist_outgoing_reply|next_session_seq\\(" src/openhuman/orchestration/ops.rs -C 3 echo echo '--- drain / wake flow around origin and peer sessions ---' sed -n '740,820p' src/openhuman/orchestration/ops.rs | cat -nRepository: tinyhumansai/openhuman
Length of output: 11986
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- seq usage in orchestration ops.rs ---' rg -n "\bseq\b|latest_seq\(|has_new_work\(|newest_inbound\(|advance_cursor\(" src/openhuman/orchestration/ops.rs -C 2 echo echo '--- messages retrieval / ordering logic in store.rs ---' rg -n "ORDER BY .*seq|MAX\(seq\)|seq DESC|seq ASC|SELECT .*messages" src/openhuman/orchestration/store.rs -C 2Repository: tinyhumansai/openhuman
Length of output: 14230
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- latest_seq / newest_inbound definitions in src/openhuman/orchestration/ops.rs ---' rg -n "fn (latest_seq|newest_inbound|has_new_work)\b" src/openhuman/orchestration/ops.rs -C 6 echo echo '--- message ordering tests mentioning seq in src/openhuman/orchestration/ops.rs ---' rg -n "seq|ordering|newest_inbound|latest_seq" src/openhuman/orchestration/ops.rs -C 1 | sed -n '1,120p'Repository: tinyhumansai/openhuman
Length of output: 6544
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- definitions for latest_seq, newest_inbound, has_new_work in src/openhuman/orchestration/ops.rs ---' rg -n "fn (latest_seq|newest_inbound|has_new_work)\b" src/openhuman/orchestration/ops.rs -C 8 echo echo '--- any ordering assumptions on message seq in src/openhuman/orchestration/store.rs ---' rg -n "ORDER BY .*seq|seq DESC|seq ASC|MAX\(seq\)|latest_seq|newest_inbound" src/openhuman/orchestration/store.rs -C 4Repository: tinyhumansai/openhuman
Length of output: 11068
Wrap the
MAX(seq)+1write instore::in_immediate_txn.thread_reply_to_originuses the same read-then-write path aspersist_outgoing_reply, but without the write lock, so concurrent writers on the same(agent_id, session_id)can stamp duplicateseqvalues and make reply ordering/cursoring ambiguous.🤖 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/ops.rs` around lines 338 - 368, Wrap the read-then-write flow in thread_reply_to_origin with store::in_immediate_txn so the MAX(seq)+1 lookup and the subsequent upsert_session and insert_message happen under the same write lock. Use the existing store::with_connection block and the store helpers next_session_seq, upsert_session, and insert_message as the place to apply the transactional guard, matching the concurrency protection used by persist_outgoing_reply.
… unknown kind Addresses two Codex review notes on the v2 ingest path: - A later `status` snapshot (e.g. running_tool → idle) carries no detail/ active_call_id, but upsert_session COALESCEs run-state so the roster kept showing the stale task/call id forever. Mark the `status` event as an authoritative snapshot and overwrite the three run-state columns via a new store::apply_run_state UPDATE (SET, not COALESCE) so None can clear. Content events stay on COALESCE and never wipe a live status. - A forward/garbled v2 kind (e.g. "teleport") folded to Unknown but persisted its raw kind string as event_kind, which the store readers don't hide → it leaked into the thread/unread/preview. Persist the literal "unknown" instead. Tests: status clears stale detail/active_call_id; content event preserves live status; unknown kind persists as "unknown"; authoritative flag set only on status.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/orchestration/store.rs (1)
1055-1059: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreview assertion can't prove the "newest visible is the call" claim.
Both surviving visible rows (
v1andcall) use the fixture body"hi", solatest_message_previewwould still equalSome("hi")even if it wrongly returned the olderv1row or a hidden row leaked through. Give the newest visible row a distinct body so the assertion actually pins ordering + exclusion.💚 Make the preview assertion discriminating
let mut call = msg("call", "`@a`", "h1", 2); call.event_kind = Some("tool_call".into()); + call.body = "running build".into(); call.timestamp = "2026-07-02T00:00:02Z".into(); insert_message(conn, &call)?;// Roster preview skips the hidden rows → newest visible is the call. assert_eq!( latest_message_preview(conn, "`@a`", "h1")?.as_deref(), - Some("hi"), + Some("running build"), );🤖 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 1055 - 1059, The preview assertion in the roster preview test is ambiguous because the visible rows share the same body, so it cannot prove that `latest_message_preview` returns the newest visible message. Update the test fixture in the block around `latest_message_preview(conn, "`@a`", "h1")` so the newest visible row (the call) has a distinct body from the older visible row, then adjust the `assert_eq!` to expect that unique value. This will make the assertion in the roster preview test actually verify both ordering and hidden-row exclusion.
🤖 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 1055-1059: The preview assertion in the roster preview test is
ambiguous because the visible rows share the same body, so it cannot prove that
`latest_message_preview` returns the newest visible message. Update the test
fixture in the block around `latest_message_preview(conn, "`@a`", "h1")` so the
newest visible row (the call) has a distinct body from the older visible row,
then adjust the `assert_eq!` to expect that unique value. This will make the
assertion in the roster preview test actually verify both ordering and
hidden-row exclusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8f8f965e-6613-4b9e-8269-41d89f67484d
📒 Files selected for processing (2)
src/openhuman/orchestration/ingest.rssrc/openhuman/orchestration/store.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/openhuman/orchestration/ingest.rs
sanil-23
left a comment
sanil-23
left a comment
There was a problem hiding this comment.
Review — request changes
High-quality, unusually well-tested Rust: the additive migration, COALESCE-vs-SET run-state semantics, seq-ordinal wake fix, and the forward-compatible Unknown fold are all correct and thoroughly covered, and both Codex notes were properly addressed. Two things to resolve before merge.
Blocking
B1 — Merge-gate diff-coverage is red (48% < 80%); the new frontend paths ship without tests. The PR CI Gate → "Enforce >= 80% coverage on changed lines" step exits 1. Uncovered:
TinyPlaceOrchestrationTab.tsx:163-177— thepublishIdentitycallback (the core of the feature). The tab test only stubspublishIdentity: vi.fn(); nothing clicks the button and asserts the success-adoption / error paths.orchestrationClient.ts:292—publishIdentitywrapper (0%).useOrchestrationChats.ts:94-96— theeventKind/toolName/callIdmapping inmapMessage.SelfIdentityCard.tsx:148,152— the discoverable-branch "Republish" button'spublishinglabel +publishError.OrchestrationChatPrimitives.tsx:125,139— theapproval_requestanduser_promptbubbleStylebranches.
Add tests driving the publish flow (success → card flips discoverable; failure → tinyplace-self-identity-publish-error), a mapMessage test asserting v2 rows carry the typed fields and v1 rows leave them undefined, and MessageBubble renders for the two missing kinds.
Major
M2 — approval_request never populates current_detail/active_call_id, so the remote-approval attention summary is stale/empty (inline on ingest.rs). Correctness relies on a companion status event; the code also contradicts its own comments + the PR description.
Minor
- m3 —
last_erroris plumbed through the core status DTO but the TSOrchestrationStatusinterface omits it (andingestCursorLag), so the renderer can't consume it (inline). - m4 —
mark_chat_readusesMAX(timestamp)over all rows incl. now-hidden status/lifecycle/unknown, whileunread_countexcludes them; an out-of-order visible message could be missed from unread (inline). - m5 —
bubbleStylederives the dot background viastyle.dot.replace('text-','bg-'); works only because the glyph-less kinds use standard palette colors that havebg-variants.
Looks good
v2 envelope + Unknown fold (never routes a future kind to Master), the table_info-guarded additive migration with its legacy-schema test, the authoritative-status SET-not-COALESCE fix, the in_immediate_txn concurrent-seq serialization test, the best-effort four-source attention assembly, and the security posture (decrypted bodies stay workspace-internal + never logged, dedupe/linked-sender gate before the non-idempotent decrypt).
Automated CodeRabbit-style review.
| 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.
M2 (major): this arm sets status_state = "waiting_approval" but leaves status_detail/active_call_id unset, and the event stays non-authoritative → COALESCE upsert. But attention::remote_approval_signals reads s.current_detail as the item summary, and both RemoteApprovalSignal's doc-comment (attention.rs:144-159) and the PR description claim the approval "persists the prompt in current_detail and the in-flight active_call_id." It writes neither, so the "needs you" queue shows the previous status.detail (e.g. a stale compiling) or nothing — unless a companion status{waiting_approval, detail, active_call_id} event happens to arrive.
Suggest making the approval carry its own detail (it already has display + call_id):
HarnessEventKind::ApprovalRequest(p) => {
b.status_detail = non_empty(p.display.clone());
b.active_call_id = p.call_id.clone().and_then(non_empty);
b.body = p.display;
b.tool_name = non_empty(p.tool_name);
b.call_id = p.call_id.and_then(non_empty);
b.status_state = Some("waiting_approval".to_string());
b.authoritative_status = true; // own the run-state so detail/call_id land
}Either fix this, or reconcile the comments/PR text with the companion-status assumption. Is that companion event guaranteed by the emitter?
| const identity = await orchestrationClient.publishIdentity(); | ||
| if (!mountedRef.current) return; | ||
| debug('[tinyplace-orchestration] publish identity ok discoverable=%s', identity.discoverable); | ||
| setSelfIdentity(identity); |
There was a problem hiding this comment.
B1 (blocking): this publishIdentity callback (163-177) is 0% covered — the coverage gate fails on it. The tab test only adds publishIdentity: vi.fn() to the mock; add a test that clicks the publish button and asserts (a) success adopts the echoed identity → the status chip flips data-discoverable="true", and (b) a rejected publishIdentity surfaces tinyplace-self-identity-publish-error.
3 participants
Summary
Adds the OpenHuman receiver + renderer for the coding-agent → "Master Session" bridge: the core now ingests a coding agent's typed v2 harness-session stream (Claude/Codex) over tiny.place Signal DMs, classifies each event, and the orchestration tab renders it as a differentiated activity log (tool calls, results, thinking, approvals) instead of undifferentiated chat. Also makes an agent's own identity fully messageable in one action and hardens the identity card against a slow relay.
Companion to the SDK/emitter side (tiny.place #229 codex bridge, #230 v2 events). Proven end-to-end live: a real
codexsession streamed into OpenHuman, decrypted, classified, and rendered.Problem
Solution
Receive (Phase 1) —
src/openhuman/orchestration/SessionEnvelopeV2+ adjacently-taggedHarnessEventKind(snake_case, fail-softUnknown{raw}fold); discriminate onenvelope_version.classify_messagetries v2 → v1 → Master.event_kind,tool_name,call_id; sessionstatus_state,current_detail,active_call_id) via idempotent migration. Content events advancelast_seq(wake the graph); status/lifecycle/unknown persist atseq=0(relay dedup) but don't surface in-thread — preserves orchestration: wake cursor keyed on wrapper_session_id but seq=env.message.line → silent message drop #4583 ordinal semantics.Render (Phase 2) —
app/src/…/OrchestrationChatPrimitives.tsx▶tool_call (mono) /↳tool_result (mono) /∴agent_thinking (italic) /⚠approval /✕error; tool-name chip,data-event-kind. Store-level filter hides status/lifecycle/unknown from the thread, unread count, and preview.Attention (Phase 3) — remote sessions parked on an approval fold into the "needs you" queue (
RemoteApprovalSignal→AttentionItemkindApproval, actionOpenSession).Messageable identity (Phase 3b)
orchestration_publish_identityRPC — "Make discoverable" now publishes the directory card + encryption key and provisions the X3DH prekey bundle (signal_provision), so a fresh identity is deliverable-to in one action. A "Republish keys" affordance stays reachable when already discoverable.self_identity+signal_key_statusbound their directory lookups (reverse, get_agent) with a 5s timeout that degrades to not-published/no-handle, so the card always resolves instead of hanging on a slow relay.Submission Checklist
cargo checkclean;cargo clippyno new warnings;cargo fmtcleanpnpm typecheck,eslint,prettier --checkclean; Vitest green (SelfIdentityCard 11, renderer, attention)pnpm i18n:check) green across all localesImpact
Unknownrather than hard-failing.Related
fromcomment/test — current protocol is base58 (defensive unify already handles both) #4669 (recipient resolves the peer's messaging key instead of its wallet address).self_identity's bounded lookups (15s → 5s) and dedupe the doubleget_agent; makepublish_identitynot block its return on the slow re-read; fold prekey-bundle presence into thediscoverablesignal.AI Authored PR Metadata (required for Codex/Linear PRs)
Summary by CodeRabbit