Repository navigation
fix(orchestration): call the Orca session ID orca_session_id everywhere agents see it - #24230
Conversation
…id:<id> The database already names this identity orca_session_id, but agents saw and typed session:<id>, which collides with the Claude/Codex session ID, Session History and terminal sessions. The prefix is now orca_session_id:. No alias, migration or version handshake for the old session: spelling: restructured native chat is experimental. The coordinator-address triggers compile the prefix into their body, and migrate-v42 stamped them once, so an existing database would keep remembering session:<id> for new Runs. They are now recreated on every open, like the mail routing trigger. Provider-id refusals now name the "Orca session ID" instead of "Orca address"/"Orca id".
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe canonical Orca session address prefix changes from Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to Stored legacy addresses are upgraded to the canonical spelling, while new inputs using the retired prefix are rejected. The repository contract supports this behavior, leaving no actionable merge risk. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The normal upgrade preserves stored mail and routing ownership without an identified expansion of caller authority. Remaining uncertainty concerns older-version rollback, mixed-version access, and subsequent recovery—not a demonstrated security vulnerability. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the problem, user-visible behavior, implementation, migration, testing, and visual results. However, it omits the required Linked Issue with an actual issue number and does not include several required template sections, including Review, Agent skill upstream boundary, Notes, and Checklist. Resolution Add an actual issue reference after ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Renames the agent-facing Orca session address from session:<id> to orca_session_id:<id>, and makes the coordinator-address triggers follow the running build instead of the prefix compiled when the database first migrated.
- Prefix constant renamed —
ORCA_SESSION_ADDRESS_PREFIXis noworca_session_id:; the codec, CLI own-session check, injected-address helper, SQL address helper, mail-target/recipient resolvers, and agent-facing text all derive from it. - Coordinator triggers recreated on every open —
createRunCoordinatorAddressTriggersis extracted frommigrate-v42and run aftermigrateon eachOrchestrationDbopen, so an existing v42 database stops rememberingsession:<id>for a new Run coordinator.migrate-v42keeps only columns and indexes; no schema bump. - Refusal text — the two provider-id refusals and the malformed-recipient refusal now say "Orca session ID".
- Tests — existing assertions moved to the new spelling; a new test stamps a database with old-prefix triggers, reopens, and asserts the new prefix is used.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…vider-id refusals The send refusal handed back orca_session_id:<id> as the Orca session ID while the caller refusal called the bare id by that name. Both now say "This session's Orca session ID is orca_session_id:<id>"; the caller refusal also names the bare value ORCA_AGENT_SESSION_ID accepts, since that input takes only the bare id. Tests pin both refusal texts, that session:<id> no longer parses as a session, and that the older-build trigger rewrite actually changes the trigger SQL.
| throw new OrchestrationError( | ||
| CODES.providerId, | ||
| `${sessionId} is the provider's own session id, which changes on /clear. This session's Orca id is ${found.orcaSessionId}; use that instead. No effects were applied.`, | ||
| `${sessionId} is the provider's own session id, which changes on /clear. This session's Orca session ID is ${ORCA_SESSION_ADDRESS_PREFIX}${found.orcaSessionId}; set ${ORCA_AGENT_SESSION_ID_ENV}=${found.orcaSessionId} instead. No effects were applied.`, |
There was a problem hiding this comment.
orca_session_id:<id> the “Orca session ID,” but ORCA_AGENT_SESSION_ID requires the bare <id>. An agent that copies the displayed value into that variable is refused again as an unknown session. Show the bare ID as the session ID and reserve the prefixed form for its address.
| `${sessionId} is the provider's own session id, which changes on /clear. This session's Orca session ID is ${ORCA_SESSION_ADDRESS_PREFIX}${found.orcaSessionId}; set ${ORCA_AGENT_SESSION_ID_ENV}=${found.orcaSessionId} instead. No effects were applied.`, | |
| `${sessionId} is the provider's own session id, which changes on /clear. This session's Orca session ID is ${found.orcaSessionId}; set ${ORCA_AGENT_SESSION_ID_ENV}=${found.orcaSessionId} instead. No effects were applied.`, |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the delta since the prior pullfrog review: the authored commit 7a685dbc54 (the merge bb6ec0eb02 only brings origin/main forward and adds no authored change here).
- Aligned the two provider-id refusals —
session-recipient.tsandorchestration-session-caller.tsboth now spell the identity asorca_session_id:<id>; the caller-side refusal points at the bare valueORCA_AGENT_SESSION_IDaccepts because that path reads only a bare id from the env var the CLI sends asagentSessionId. - Pinned the refusal text exactly —
orchestration-session-caller.test.tsandorchestration-session-recipient.test.tsnow assert the full strings instead ofstringContaining, and the recipient case covers both theorca_session_id:<provider id>and bare spellings. - Added the retired-spelling test —
orca-session-address.test.tsassertssession:<id>no longer parses as an address nor satisfiesisOrcaSessionId. - Guarded the trigger-rewrite fixture —
run-coordinator-orca-session-address.test.tsnow asserts its older-build rewrite actually changes the trigger SQL, so the reopen cannot silently prove nothing.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Preserve handle provenance before removing retired addresses. · orchestration-db.ts:29-37
src/main/runtime/orchestration/db/orchestration-db.ts:29-37
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve handle provenance before removing retired addresses.
The open path recreates the triggers and refills the cache with
INSERT OR IGNORE. It does not remove existingsession:rows. Exact handle lookup can therefore continue routing a retired address to its historicalrun:mailbox.Do not delete every
session:%row.run_coordinator_handlesdoes not record whether a row came fromcoordinator_handleor a generated session address. The binding path also accepts arbitrary handle strings, and the cache intentionally retains historical handles. Current run metadata cannot distinguish all of these cases safely.Add provenance or enforce a disjoint handle namespace before removing only generated retired addresses. The migration must also define how to handle already ambiguous rows to preserve the no-alias contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: d42df367-b37b-41b6-b3a5-aff19d541165
📒 Files selected for processing (7)
src/main/runtime/orchestration/run-coordinator-orca-session-address.test.tssrc/main/runtime/rpc/methods/orchestration/messaging/session-recipient.tssrc/main/runtime/rpc/orchestration-session-caller.test.tssrc/main/runtime/rpc/orchestration-session-caller.tssrc/main/runtime/rpc/orchestration-session-recipient.test.tssrc/main/runtime/structured-chat-coordinator-mail.test.tssrc/shared/orca-session-address.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/runtime/rpc/orchestration-session-caller.ts
- src/main/runtime/rpc/methods/orchestration/messaging/session-recipient.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The only commit since the prior pullfrog review (7a685dbc54) is 5d8a69c644, a merge of origin/main into the PR branch. A scoped git diff 7a685dbc54 5d8a69c644 -- <all 24 PR file paths> is empty, so the merge brought in base-branch churn only, with no PR-authored change and no conflict resolution touching the PR's files. The prior review stands unchanged.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…s after the rename Existing native chats were addressed as session:<id>, so agents and stored mail still use that spelling. Input now accepts session:<id> and treats it as orca_session_id:<id> (the codec parses both; nothing writes the old one), and schema v43 rewrites every stored session:<id> address once: message senders and recipients, remembered Run coordinator addresses, structured pointer operation keys, and coordinator loop handles. Subjects, bodies and payloads are left as written.
… still migrate once The old prefix is not accepted as input: no respelling helper, no special case. A session:<id> recipient is refused like any unknown terminal handle (terminal_not_found), including after v43 rewrote a Run's remembered session:<id> coordinator address, so the old spelling no longer routes. Schema v43 still rewrites stored session:<id> addresses once.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The delta since the prior pullfrog review is commit 809d6235 ("keep chats reachable at their session:<id> address after the rename"), which turns the one-way rename into a forward migration with read-only legacy aliases.
- Legacy alias codec —
orca-session-address-prefix.tsgainsrespellLegacyOrcaSessionAddressandisSpelledAsOrcaSessionAddress;parseOrcaSessionAddressrespells first, sosession:<id>still parses to the bare id whileformatOrcaSessionAddressonly ever emits the current prefix. - Alias acceptance on input — the CLI own-session check and
sessionAddressForHost, the recipient resolver (mayNameSession/readSessionRecipient), and the party resolver all treat the legacy spelling as the same Session address, respelling it before lookup or storage. - Schema v43 —
migrateV43rewrites storedsession:<id>addresses once inmessages,coordinator_runs, and the two keyed tables (dedup viaUPDATE OR IGNORE+DELETE); idempotent and inside the existing migrate transaction. - Coordinator triggers —
createRunCoordinatorAddressTriggersnow owns the trigger form and runs on every open, so the compiled prefix always matches the running build. - Tests — a new v42-seeded migration test asserts the rewrite, dedup, chat read, reply address, and rerun no-op; the trigger test asserts its own fixture actually changed the SQL.
I traced the migration's dedup against the real PKs (run_coordinator_handles(run_id, terminal_handle), structured_pointer_operations(mailbox_handle)), confirmed deliveries.mailbox_handle only holds run:/dispatch: role addresses so it is correctly not migrated, and verified runs.coordinator_handle never holds a session address (only mailboxAddressOf does, which the keyed table covers). The prior CodeRabbit provenance concern is superseded: every session: row in those tables is a generated address, so the rewrite cannot rename a literal handle. No new findings.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 094f14af-6cfa-4b58-a925-c5fa0439568b
📒 Files selected for processing (11)
src/cli/orchestration-session-caller-cli.test.tssrc/cli/session-caller-flags.tssrc/main/runtime/orchestration/db/contract-constants.tssrc/main/runtime/orchestration/db/schema/migrate-v43.tssrc/main/runtime/orchestration/db/schema/migrate.tssrc/main/runtime/rpc/methods/orchestration/messaging/session-recipient.tssrc/main/runtime/rpc/orchestration-legacy-session-address-migration.test.tssrc/main/runtime/rpc/orchestration-session-recipient.test.tssrc/shared/orca-session-address-prefix.tssrc/shared/orca-session-address.test.tssrc/shared/orca-session-address.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| for (const [table, column] of ADDRESS_COLUMNS) { | ||
| this.db.exec(`UPDATE ${table} SET ${column} = ${respelled(column)} WHERE ${isLegacy(column)}`) | ||
| } | ||
| for (const [table, column] of KEYED_ADDRESS_COLUMNS) { | ||
| this.db.exec(` | ||
| UPDATE OR IGNORE ${table} SET ${column} = ${respelled(column)} WHERE ${isLegacy(column)}; | ||
| DELETE FROM ${table} WHERE ${isLegacy(column)}; | ||
| `) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP "session:\\$\\{|'session:'|\"session:\"" src/main --type=ts -g '!**/*.test.ts' -C2
rg -nP '\b\w+_handle\b\s+TEXT' src/main/runtime/orchestration/db -C1Repository: stablyai/orca
Length of output: 13181
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- migrate-v43 ---'
cat -n src/main/runtime/orchestration/db/schema/migrate-v43.ts
printf '%s\n' '--- migration references ---'
rg -n -C3 'ADDRESS_COLUMNS|KEYED_ADDRESS_COLUMNS|orca_session_id:|session:' src/main/runtime/orchestration
printf '%s\n' '--- orchestration schemas ---'
cat -n src/main/runtime/orchestration/db/schema/create-core-tables-sql.ts
printf '%s\n' '--- required diff stat ---'
git diff --stat 3ab3c9239fba5eaa36c3c4ce10aab4cf346ee812 809d6235bede57dd7dd1af990abaf73a44d7de3
printf '%s\n' '--- required diff for migration ---'
git diff --unified=30 3ab3c9239fba5eaa36c3c4ce10aab4cf346ee812 809d6235bede57dd7dd1af990abaf73a44d7de3 -- src/main/runtime/orchestration/db/schema/migrate-v43.tsRepository: stablyai/orca
Length of output: 41582
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- legacy migration test fixture ---'
sed -n '600,715p' src/main/runtime/orchestration/orchestration-legacy-session-address-migration.test.ts
printf '%s\n' '--- v42 migration at head ---'
cat -n src/main/runtime/orchestration/db/schema/migrate-v42.ts
printf '%s\n' '--- relevant current address construction and consumers ---'
rg -n -C4 'ORCA_SESSION_ADDRESS_PREFIX|sessionAddress|SessionAddress|from_handle|to_handle|coordinator_handle|mailbox_handle|terminal_handle|caller_handle|recipient|owner' src/main/runtime/orchestration/db src/main/runtime/orchestration/messaging src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- legacy prefix occurrences at required base ---'
git grep -n -E 'session:|orca_session_id:' 3ab3c9239fba5eaa36c3c4ce10aab4cf346ee812 -- src/main/runtime/orchestration ':!**/*.test.ts'
printf '%s\n' '--- legacy prefix occurrences at reviewed head ---'
git grep -n -E 'session:|orca_session_id:' 809d6235bede57dd7dd1af990abaf73a44d7de3 -- src/main/runtime/orchestration ':!**/*.test.ts'Repository: stablyai/orca
Length of output: 45665
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- locate legacy migration test ---'
git ls-files | rg 'legacy.*session.*address|session.*address.*migration'
printf '%s\n' '--- base v42 migration ---'
git show 3ab3c9239fba5eaa36c3c4ce10aab4cf346ee812:src/main/runtime/orchestration/db/schema/migrate-v42.ts | cat -n
printf '%s\n' '--- current test files mentioning earlier build ---'
rg -l -F 'earlier build of this step stamped v42' src/main
printf '%s\n' '--- fixture declaration and focused body ---'
file=$(rg -l -F 'earlier build of this step stamped v42' src/main | head -n1)
if [ -n "$file" ]; then
rg -n -C8 'earlier build of this step stamped v42|CREATE TABLE|CREATE TRIGGER|session:<id>|coordinator_principal|coordinator_actor|principal|actor' "$file"
fi
printf '%s\n' '--- current schema table declarations ---'
rg -n -C3 '^CREATE TABLE|coordinator_runs|dispatch_contexts|tasks|messages|deliveries|structured_pointer_operations' src/main/runtime/orchestration/db/schema src/main/runtime/orchestration/dbRepository: stablyai/orca
Length of output: 45663
🏁 Script executed:
#!/bin/bash
set -o pipefail
testfile=src/main/runtime/rpc/orchestration-legacy-session-address-migration.test.ts
printf '%s\n' '--- dedicated migration test address references ---'
rg -n -C6 'session:|orca_session_id:|formatOrcaSessionAddress|ADDRESS|messages|coordinator_runs|run_coordinator_handles|structured_pointer_operations|dispatch_contexts|tasks' "$testfile"
printf '%s\n' '--- address formatter uses in non-test source ---'
rg -n -C3 'formatOrcaSessionAddress|ORCA_SESSION_ADDRESS_PREFIX|structuredSessionPointerCallerKey|insertMessage|createStructuredPointer|mailboxHandle|coordinatorHandle' src/main -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- coordinator-runs schema and stores ---'
rg -n -C5 'CREATE TABLE.*coordinator_runs|coordinator_runs|coordinator_handle' src/main/runtime/orchestration/db -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- migration orchestration ---'
cat -n src/main/runtime/orchestration/db/schema/migrate.ts | sed -n '1,100p'Repository: stablyai/orca
Length of output: 45667
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- deliveries mailbox_handle ---'
rg -n -C5 '\bmailbox_handle\b' src/main/runtime/orchestration/db src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- dispatch and task handle declarations/writes ---'
rg -n -C4 'assignee_handle|creator_handle|created_by_terminal_handle|terminal_handle|from_handle|to_handle' src/main/runtime/orchestration/db/schema src/main/runtime/orchestration/db/dispatch src/main/runtime/orchestration/db/tasks src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts' | head -n 500
printf '%s\n' '--- gate/question persistence ---'
rg -n -C5 'question_threads|decision_gate|requester|caller|asker_handle|created_by_terminal_handle' src/main/runtime/orchestration/db src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts' | head -n 500Repository: stablyai/orca
Length of output: 42725
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- mailbox delivery API and callers ---'
sed -n '1,125p' src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts
rg -n -C6 'enqueue|create|deliver|mailboxHandle|mailbox_handle' src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts' | rg -C3 'delivery|mailboxHandle|mailbox_handle|enqueue|create'
printf '%s\n' '--- task and gate write boundaries ---'
rg -n -C8 'INSERT INTO tasks|created_by_terminal_handle|INSERT INTO question_threads|asker_handle|createDecision|decision_gates' src/main/runtime/orchestration/db src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 45663
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- exact delivery callers ---'
rg -n -C8 'getOrCreateMailboxDelivery|requireMailboxConsumer' src/main -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- mailbox consumer implementation ---'
cat -n src/main/runtime/orchestration/db/messages/mailbox-consumer.ts
printf '%s\n' '--- task creation inputs ---'
rg -n -C10 'createTask\(|createdByTerminalHandle|created_by_terminal_handle' src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- gate creation implementation ---'
rg -n -C12 'export function createGate|function createGate|INSERT INTO decision_gates' src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts'Repository: stablyai/orca
Length of output: 41471
🏁 Script executed:
#!/bin/bash
set -o pipefail
printf '%s\n' '--- question-thread backfill call and ordering ---'
rg -n -C8 'backfillLegacyQuestionThreads|asker_handle' src/main/runtime/orchestration/db/schema src/main/runtime/orchestration/db -g '*.ts' -g '!**/*.test.ts'
printf '%s\n' '--- question-thread consumers and writers ---'
rg -n -C8 'asker_handle|question_threads' src/main/runtime/orchestration/db src/main/runtime/rpc src/main/runtime/orchestration -g '*.ts' -g '!**/*.test.ts' | head -n 700
printf '%s\n' '--- migration calls around question backfill and v43 ---'
rg -n -C10 'backfillLegacy|migrateV20|migrateV43|migrateV41' src/main/runtime/orchestration/db/schema -g '*.ts'Repository: stablyai/orca
Length of output: 41987
Migrate legacy addresses in question_threads.asker_handle.
migrateV43 does not update question_threads.asker_handle. When an older database is replayed, backfillLegacyQuestionThreads copies messages.from_handle into this column before v43 runs. A legacy session:<id> sender can therefore remain in the question thread. legacy-reply-operation.ts uses that value as the reply recipient, so the reply can target the obsolete address.
Suggested fix
['messages', 'from_handle'],
['messages', 'to_handle'],
- ['coordinator_runs', 'coordinator_handle']
+ ['coordinator_runs', 'coordinator_handle'],
+ ['question_threads', 'asker_handle']
] as const🧰 Tools
🪛 OpenGrep (1.30.0)
[ERROR] 35-35: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
[ERROR] 38-41: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
Comments Outside DiffThese findings could not be posted inline.
|
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The delta since the prior pullfrog review is commit cec36b3022 ("refuse session:<id> input again; stored addresses still migrate once"), which reverses the read-only legacy aliases added in 809d6235 while keeping the one-time storage migration and the per-open trigger recreation.
- Legacy input aliases removed —
LEGACY_ORCA_SESSION_ADDRESS_PREFIX,respellLegacyOrcaSessionAddress, andisSpelledAsOrcaSessionAddressare gone;parseOrcaSessionAddressand the CLI'ssessionAddressForHostaccept onlyorca_session_id:<id>, sosession:<id>falls through to the terminal-handle path and is refused (terminal_not_found). Grep confirms no remaining references to the removed helpers. - Storage migration retained —
migrateV43still respells every storedsession:<id>once, so pre-upgrade mail, Run routing, replies, and remembered coordinator handles stay reachable at the current spelling. - Tests retargeted — the alias acceptance/reach tests are replaced by refusal assertions in the CLI, recipient, migration, and codec suites, with the provider-id refusal text pinned exactly. The four affected suites run clean locally (178 passed, 4 skipped).
ℹ️ Rollback-written session:<id> rows are now hard-refused, not aliased
The documented rollback gap changes character once the input alias is gone. If a user rolls back to a v1.4.214–v1.4.218 build that writes new session:<id> addresses, a later upgrade skips them (user_version is already 43), and --to session:<id> now refuses them as an unknown terminal instead of reaching the recipient through the alias 809d6235 briefly provided. This is a scope confirmation rather than a request — the PR body already defers the rewrite-on-every-open fix — but since the alias softened exactly this case, it is worth confirming the harder behavior is intended.
Technical details
# Rollback-written legacy rows become unreachable via direct `--to` input
## Affected sites
- `src/main/runtime/orchestration/db/schema/migrate-v43.ts:26-29` — `if (current >= 43) return`, so a v43 DB never re-runs the rewrite.
- `src/main/runtime/rpc/methods/orchestration/messaging/session-recipient.ts:50` — `session:<id>` no longer matches the current prefix, so it resolves as a terminal handle and is refused.
## Required outcome
- Confirm the no-alias contract is intended even when a rollback has written fresh `session:<id>` rows that the one-time migration cannot see. If not, either re-run the rewrite on open or retain a read-only alias.
## Open questions for the human
- Is the hardening acceptable, or does the rollback path need the migration to run on every open (as the PR body notes would close the gap)?DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Review status: ready for your call (not merged)The problem. Agents address each other by the Orca session ID, spelled What changes for you. Agents now see and type What the review changed (head now cec36b3):
Verified:
Not verified / deferred:
|
1 participant
The problem
When one agent messages another through Orca orchestration, it names the recipient by its Orca session ID, the ID Orca mints for every agent session. Agents see and type that ID as
session:<id>(for exampleorca orchestration send --to session:3f9c…)."Session" alone means several different things in Orca: the Claude/Codex session ID (the "Copy Session ID" action), Session History, and terminal sessions. So an agent or user reading
session:<id>can reasonably think it is the Claude or Codex session ID, which it is not, and paste the wrong one.The database already calls this identity
orca_session_id(thecoordinator_orca_session_id,assignee_orca_session_idandcreator_orca_session_idcolumns from #22522, which followed the team discussion that "sessionId itself is confusing"). Only the text form agents see still says plainsession:. The goal is that every agent is referred to by its Orca session ID, spelledorca_session_id:<id>.What this changes for you
--to session:3f9c…--to orca_session_id:3f9c…--from session:3f9c…--from orca_session_id:3f9c…from_handle,to_handle)session:3f9c…orca_session_id:3f9c…--to session:3f9c…A bare Orca session ID (no prefix) is still accepted wherever it was before. Terminal handles (
term_…) and terminal-agent wording are unchanged.Scope: Stored
session:<id>addresses are migrated once toorca_session_id:<id>; the old prefix is no longer accepted as input. That is,session:no longer names an Orca session anywhere:--to session:<id>gets the generic refusal below, a chat's--from session:<id>is refused, and a terminal agent's--from session:<id>is treated like any other unrecognised sender name (as--from foo:baris today), not as a session. Your existing chats stay reachable: they keep the mail sent to them before the upgrade, keep receiving mail for the Runs they coordinate, and replies to their old mail reach them. Typing--to session:<id>is refused like any unknown terminal handle ("Terminal session: has no live pane or durable Run/Dispatch mailbox."), and nothing is stored. There is no alias and no version handshake with remote hosts.Rolling back to v1.4.214–v1.4.218 after this build has run: the older build skips migration on the v43 database and cannot see unread mail the migration already respelled to
orca_session_id:<id>until you upgrade again.Deferred: if you upgrade, roll back to v1.4.214–v1.4.218, then upgrade again, mail that older build stored at a
session:<id>address during the rollback is not migrated, because the one-time migration has already run (the database is already at v43). Running the same rewrite on every open would close this; it is left out to keep the migration one-time.What changed
The prefix constant
ORCA_SESSION_ADDRESS_PREFIX(src/shared/orca-session-address-prefix.ts) is noworca_session_id:. Everything that formats or parses the address already derives it from that constant: the shared parse/format codec, the CLI's own-session check, the SQL helper that builds'<prefix>' || id, the mail-target and recipient resolvers, and the address injected into agent-facing text.One place did not follow the constant on existing databases: the two triggers that remember a Run coordinator's session address (so mail to it is routed into the Run mailbox) have the prefix compiled into their SQL body, and migrate-v42 created them once. An existing v42 database would therefore keep remembering
session:<id>for every new Run, so mail to the coordinator'sorca_session_id:<id>would not route until the next app restart backfilled it. The triggers are now recreated on every open, right after migrate, the same way the coordinator mail routing trigger already is, so the trigger body always matches the running build. migrate-v42 keeps only the columns and indexes. The triggers need no schema version bump.Stored addresses: schema v43 (src/main/runtime/orchestration/db/schema/migrate-v43.ts) rewrites
session:<id>toorca_session_id:<id>once, in every column that holds an Orca session address: message sender and recipient (messages.from_handle,messages.to_handle), remembered Run coordinator addresses (run_coordinator_handles.terminal_handle), structured pointer operation keys (structured_pointer_operations.mailbox_handle) and coordinator loop handles (coordinator_runs.coordinator_handle). It matches the exact prefix, leaves subjects, bodies and payloads as written, and spells both prefixes as fixed literals rather than the live constant, because a migration rewrites what one particular build wrote. Where a keyed table already holds the new spelling, it keeps that row and drops the old one. Running it again changes nothing. It only rewrites data, so the schema version-skew check needs nothing new, and an older build that opens a v43 database skips migration, as it already does for any newer schema.The two provider-ID refusals now use the same wording: "This session's Orca session ID is orca_session_id:". The recipient refusal adds "address it by that instead". The caller refusal adds "set ORCA_AGENT_SESSION_ID= instead", because that environment variable is where the caller's ID comes from and it takes only the bare ID. The refusal's machine-readable
orcaSessionIdfield stays the bare ID. The malformed-recipient refusal says "does not name an Orca session ID".Tests and code comments are updated to the new spelling. A new test stamps an existing database with triggers compiled for the old
session:prefix, reopens it, and checks a new Run's coordinator is remembered atorca_session_id:<id>. It fails on main's migrate-v42-only trigger install (it recordssession:<id>) and passes with this change.Left as
session:on purpose, because they are not the Orca session address: the workspace-session IPC channels (session:get,session:set, …) andbrowser:session:*channels, the AI Vault list and search keys, the desktop-script cache key, the native-chat subscription tokens (claude:session:…), the structured pointer idempotency key (trusted-local:orchestration:session:<id>, an internal ledger key whose shape change would orphan in-flight entries), mobile test fixtures, and two historical comments quoting what an earlier build wrote.The bundled skill guides and CLI help never spelled the address, so they are unchanged (the guide and skill-bundle manifest checks pass). Mobile does not parse or format the address.
Testing
orca_session_id:<id>an agent would paste. The trigger regression test builds its old-prefix setup fromORCA_SESSION_ADDRESS_PREFIXand asserts the rewrite actually changed the trigger SQL, so a future prefix change cannot make it pass without testing anything.session:<id>, a Run remembered undersession:<id>, mail a chat sent assession:<id>, keyed rows in both spellings, a coordinator loop handle) goes through the realmigrate. Then the chat reads the old mail through its owncheck; mail to the chat's new address lands in that Run's mailbox; a reply to the old mail reaches the original sender at its new address;--to session:<id>is refused withterminal_not_foundand the generic text, including for the Run whose remembered address wassession:<id>; subjects and bodies that spellsession:<id>are untouched; no address column keeps the old spelling; running the step again changes nothing. On a fresh database--to session:<id>gets the same generic refusal, and the address codec still refuses to parsesession:<id>.--to session:<id>for a Run remembered under that spelling is accepted and routed to the Run, so the refusal depends on the migration rewriting that row.mainis merged in (at 3ab3c92). Locallytc:nodeis clean. 614 test files ran (612 passed, 2 skipped) across src/main/runtime/orchestration, src/main/runtime/rpc, src/cli, tests/e2e/cross-version-wire, orca-runtime, structured-chat coordinator mail and the address codec: 7027 tests passed, 16 skipped, 0 failed.agent-session-resume-marker-downgrade(restart offers, unrelated to orchestration addresses), could not run locally: the harness's 45-secondtarextraction of an old release timed out on a heavily loaded machine. CI covers it.oxlint(full), anti-slop on the changed files, andcheck:code-quality:changedpass.verify:bundled-skill-guidesandverify:skill-bundle-manifestpass.Screenshots
Real app on a Mac, restructured Claude chats, real Claude CLI, isolated profile.
Upgrade from the build before this PR, same profile. On the old build (base 5cda0f4), chat A mails chat B at
session:<B>and B leaves the second mail unread:After relaunching the same profile on this PR (head cec36b3; the database goes from schema 42 to 43), B reads the mail it left unread, now from
orca_session_id:<A>:Typing the old
--to session:<B>is refused with the generic unknown-terminal error and nothing is delivered:The new spelling reaches B:
A still gets mail for the Run it coordinated before the upgrade, including B's reply to the old mail (shown after A acknowledges an older Run delivery it had read but never acknowledged before the upgrade; that wait is existing Run delivery behaviour, identical without the upgrade):
Fresh profile (head 7a685db; the address and refusal code is unchanged since):