Skip to content

refactor(orchestration): derive delivery eligibility from messages - #19837

Merged
Jinwoo-H merged 9 commits into
mainfrom
orch-inbox-wedge
Sep 14, 2026
Merged

Jinwoo-H merged 9 commits into
mainfrom
orch-inbox-wedge

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 13 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​865 $\color{#cf222e}{\Huge{\mathbf{−}}}$​16 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​849
Prod 18 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​130 $\color{#cf222e}{\Huge{\mathbf{−}}}$​47 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​83

ELI5

An obsolete heartbeat could keep a worker’s completion hidden. Orca now checks whether a batch actually contains unread messages before letting it block the inbox. Batch IDs still identify exactly which messages an acknowledgement consumes.

What Changed

Keep the existing deliveries.status for acknowledgement and revocation. Derive replay and notification eligibility through one outstanding_deliveries view: status is outstanding and at least one member is unread. Reading or suppressing messages does not need to update batch status, and checking an obsolete batch needs no repair write.

Schema v41 keeps the table and its columns. It replaces the existing status-based unique index with a nonunique index under the same name; a trigger consulting the view enforces one eligible batch per mailbox. This avoids the previous table rebuild and replacement fencing flag.

Creation and acknowledgement validate ownership, generation, and worker lifecycle inside their transaction. Finished workers cannot consume mail awaiting rerouting. Existing remote states with uncertain liveness remain eligible. Replacement revokes all unacknowledged batches independently of message read state.

Consuming output retains its delivery ID, history does not expose an unrelated acknowledgement ID, and CLI help explains wake-only filtering and whole-batch acknowledgement. The unrelated notification-prefix change was removed.

Why

The stored batch status records acknowledgement or revocation; it is insufficient on its own to determine whether unread work remains. Deriving eligibility removes the synchronization problem without changing the historical storage representation. Partial batches replay their complete original membership, preserving FIFO and acknowledgement safety.

Linked Issue

Direct investigation request; no linked issue.

Visual Proof

N/A — database and CLI behavior verified through automated tests; no rendered UI changes or desktop interaction recording.

Testing

  • Automated delivery, lifecycle, migration, fencing, RPC and CLI regressions.
  • Broad local suite: 2,931 tests passed, 11 skipped across 207 files on simplified head a82838f5f0.
  • Historical database round trip: actual pre-v41 implementation at aac38d698ff7 creates the database; current code upgrades it and advances past a consumed batch; old code reopens, acknowledges both retained batches and writes a new batch; current code reopens and acknowledges that batch.
  • Packaged desktop downgrade manually exercised (not performed).

The round-trip test reuses the existing release-checkout harness and temporary SQLite files. Run it with ORCA_BACKGROUND_LAUNCH=1 pnpm test tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts. All test launches use background mode. No live database was modified.

AI Disclosure

Internal contributor workflow using Claude/Codex and independent Astra/Sol reviews.

Review

Previous Astra and Sol reviews identified and resolved the lifecycle race and inconsistent revocation. Final Astra review found no blocking issues and recommends this simpler version after CI passes. Astra independently ran 48 tests across six files, all passing, and endorsed the historical round-trip coverage. Local typecheck and changed-code quality checks also passed.

Agent skill upstream boundary

  • Not applicable; no upstream skill-installer content copied or translated.

Notes

The former one-way column-removal warning no longer applies: status remains readable and writable by the tested pre-v41 code. This is database compatibility evidence for that implementation, not a guarantee for every historical app release or packaged downgrade workflow. Older versions retain their original inbox replay bug; they do not acquire the new derived-eligibility behavior.

RPC shapes, execution-host ownership and remote liveness semantics remain unchanged. Stable batch identity, explicit revocation and real acknowledgement timestamps are preserved. No new acknowledgement timestamp is invented during migration.

Checklist

  • Explained behavior and design rationale.
  • Removed unrelated nudge wording and obsolete migration machinery.
  • Automated regression coverage and historical database round trip pass locally.
  • Cross-platform, SSH/remote, and compatibility impact considered.
  • Final Astra review complete: no blockers.
  • CI green for final test commit 52d8ecc034a: all eight test shards, cross-version compatibility, typecheck, static analysis, packaging, native smoke, Pullfrog, and aggregate verification passed. Shard 8 initially failed during pnpm setup with an npm registry ECONNRESET; it passed on retry without code changes. CI run.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 25d3310c-ebed-414e-bdfa-371228e75c8e

📥 Commits

Reviewing files that changed from the base of the PR and between d008782 and 0e70dbc.

📒 Files selected for processing (10)
  • docs/reference/orchestration-delivery-storage.md
  • src/main/runtime/orchestration/db/dispatch-context/dispatch-capability.ts
  • src/main/runtime/orchestration/db/federation/remote-dispatch-attachment-authority.ts
  • src/main/runtime/orchestration/db/messages/mailbox-consumer-lifecycle-fencing.test.ts
  • src/main/runtime/orchestration/db/messages/mailbox-consumer.ts
  • src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts
  • src/main/runtime/orchestration/db/runs/run-binding.ts
  • src/main/runtime/orchestration/db/runs/run-lookup.ts
  • src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts
  • src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/reference/orchestration-delivery-storage.md

Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The change moves mailbox delivery eligibility from deliveries.status to unread message state, acknowledged_at, and fenced. Schema v41 adds the derived view, indexes, trigger, migration logic, and compatibility handling. Delivery APIs now validate dispatch or attachment consumers. Runtime tests cover replay, suppression, fencing, rollback, and migration behavior. Orchestration check output now reports delivery IDs in formatted mode, and notification messages include an Orca prefix.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 0e70d

This increment renames and consolidates mailbox delivery fencing calls and consumer validation without introducing new defects that could be substantiated; the reviewed changes correctly special-case coordinator mailboxes and cover all active dispatch states. Two previously flagged concerns about the one-way v41 schema migration (a legacy receipt shape that can silently become replay-eligible, and derived schema objects that could bypass migration repair if left in a stale shape) remain unresolved since this round did not touch those migration files, so they should still be addressed or explicitly accepted before merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 46 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description is detailed, but it fails the required Linked Issue section and contains material contradictions with the changeset. It says deliveries.status remains and that the notification-prefi… Add a valid linked issue under Linked Issue. Correct the change summary, rationale, notes, and validation details to match the implemented schema v41 migration, fenced delivery state, notification-prefix change, and actual test counts.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: deriving orchestration delivery eligibility from messages.
Full details: Docstring Coverage

Explanation

Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 46 files. (1 skipped: 1 unsupported.)

Full details: Description check

Explanation

The description is detailed, but it fails the required Linked Issue section and contains material contradictions with the changeset. It says deliveries.status remains and that the notification-prefix change was removed, while the changes remove status, add fenced, and retain the notification prefix.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — a well-scoped, well-tested fix with one UX rough edge noted inline.

Reviewed changes

  • Retire fully-read deliveries — markAsRead / markAsReadAndDelivered now retire an outstanding mailbox delivery when every member is read, inside the existing MESSAGE_MUTATION_SAVEPOINT. Retirement records acknowledged (not fenced) so a late consumer --ack stays idempotent, and it is mailbox-scoped so unrelated mailboxes are untouched.
  • Repair obsolete heads before replay — getOrCreateMailboxDelivery retires an outstanding delivery whose messages are all read before falling through to selection, so check --wait --types worker_done advances past a suppressed heartbeat head instead of replaying it.
  • Wrong-ID --ack guidance — acknowledgeMailboxDelivery detects a message id passed to --ack, names the owning outstanding delivery only when it belongs to the caller's mailbox, and otherwise points at orchestration check; other mailboxes' ids are not exposed.
  • Expose deliveryId on history — the --all history result now carries the outstanding deliveryId, preserved through the formatted CLI output path.
  • Nudge attribution mitigation — formatMessagePointer prefixes [Orca orchestration notification].

The regressions are exact, not theatre: the lifecycle-retirement, obsolete-replay, transactional-rollback, mailbox-isolation, and RPC history tests all assert specific ids/statuses rather than truthiness, and the retireReadMailboxDelivery NOT EXISTS subquery is genuinely exercised. The root-cause chain (heartbeat delivery minted first, then worker_done suppresses it, leaving a fully-read outstanding head) is reproduced directly.

ℹ️ Nitpicks

  • formatMessagePointer also feeds structured-mailbox-pointer-delivery.ts:218, so the new [Orca orchestration notification] prefix now appears in native-chat structured-lane text blocks where sender identity is already unambiguous. Harmless, but the mitigation leaks past the PTY-only intent.
  • getOutstandingMailboxDelivery returns exposeDeliveryTimestamps(...), so getOrCreateMailboxDelivery applies it twice (once at the existing lookup, again on return) and hasOutstandingMailboxDelivery now pays a full-row fetch + timestamp transform for a boolean. Idempotent today, but a latent footgun if the exposure ever becomes non-idempotent.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Comment thread src/main/runtime/rpc/methods/orchestration/messaging/check-run.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior Pullfrog review at b6e7ab9.

  • Removed deliveryId from --all history — the history result no longer carries the outstanding deliveryId, and formatOrchestrationCheckText only prints Delivery <id> when a consuming check actually returned one. The >100 rows misleading-label concern from the prior review is fully resolved, with a regression asserting history has no deliveryId property.
  • Retired getOutstandingMailboxDelivery — getOrCreateMailboxDelivery reads the outstanding row directly again, retireReadMailboxDelivery now returns a boolean (result.changes > 0) that drives the replay-vs-advance decision, and hasOutstandingMailboxDelivery is back to a SELECT 1. This resolves the double-exposeDeliveryTimestamps footgun noted earlier.
  • Simplified wrong-ID --ack guidance — the isGeneratedId(...,'msg') special case is gone; a single stale_delivery message states the delivery_* contract without revealing another mailbox's delivery, and isGeneratedId import was reverted.
  • Consumer generation checked before repair — a fenced consumer can no longer retire an already-read delivery; a new regression asserts the consumer_fenced throw while leaving the delivery outstanding.
  • Refreshed CLI help and tests — help notes describe --types and replay behavior without the now-removed history deliveryId line, and the three exact-text nudge assertions were updated for the [Orca orchestration notification] prefix.

The changed unit tests (orchestration-retired-delivery.test.ts, check-delivery-history.test.ts, formatter.test.ts, orchestration-check-output.test.ts) pass locally (33 tests), and the retirement SQL is a correct correlated json_each subquery run only inside caller transactions.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior Pullfrog review at 733b18a.

  • Aligned the orchestration check spec test — src/cli/specs/orchestration.test.ts updates its check assertion to the revised --types note ("Without --wait it has no effect on consuming checks."), keeping the pre-existing spec test in sync with the note wording changed in the previous commit. The suite passes locally (6 tests).

No production code changed in this commit.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@Jinwoo-H
Jinwoo-H marked this pull request as draft September 10, 2026 05:54
@Jinwoo-H Jinwoo-H changed the title fix(orchestration): retire read deliveries and clarify mailbox recovery refactor(orchestration): derive delivery eligibility from messages Sep 10, 2026
@Jinwoo-H
Jinwoo-H marked this pull request as ready for review September 10, 2026 21:32

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: d6c4b4c8-eb5b-46ea-a5d4-428942f375dc

📥 Commits

Reviewing files that changed from the base of the PR and between 795d8e5 and d008782.

📒 Files selected for processing (31)
  • docs/reference/orchestration-delivery-storage.md
  • src/main/runtime/orchestration-dispatch-mailbox-delivery.test.ts
  • src/main/runtime/orchestration-message-delivery-identity.test.ts
  • src/main/runtime/orchestration/db.ts
  • src/main/runtime/orchestration/db/contract-constants.ts
  • src/main/runtime/orchestration/db/dispatch-mailbox-consumer-fencing.test.ts
  • src/main/runtime/orchestration/db/messages/mailbox-consumer.ts
  • src/main/runtime/orchestration/db/messages/role-mailbox-delivery.ts
  • src/main/runtime/orchestration/db/runs/run-delivery.ts
  • src/main/runtime/orchestration/db/schema/adopt-legacy-run.ts
  • src/main/runtime/orchestration/db/schema/delivery-v40-test-fixture.ts
  • src/main/runtime/orchestration/db/schema/derived-delivery-migration.test.ts
  • src/main/runtime/orchestration/db/schema/migrate-role-mailbox-delivery-v34.ts
  • src/main/runtime/orchestration/db/schema/migrate-v2-v12.ts
  • src/main/runtime/orchestration/db/schema/migrate-v35.ts
  • src/main/runtime/orchestration/db/schema/migrate-v41.ts
  • src/main/runtime/orchestration/db/schema/migrate.ts
  • src/main/runtime/orchestration/db/schema/schema-column-probes.ts
  • src/main/runtime/orchestration/dispatch-consumer-generation-migration.test.ts
  • src/main/runtime/orchestration/federation-acknowledgment-migration.test.ts
  • src/main/runtime/orchestration/nested-worker-depth-migration.test.ts
  • src/main/runtime/orchestration/orchestration-delivery-consumption.test.ts
  • src/main/runtime/orchestration/orchestration-derived-delivery.test.ts
  • src/main/runtime/orchestration/orchestration-federated-legacy-probe.test.ts
  • src/main/runtime/orchestration/orchestration-legacy-storage-db.test.ts
  • src/main/runtime/orchestration/orchestration-legacy-storage-test-fixture.ts
  • src/main/runtime/orchestration/orchestration-schema-version-skew.ts
  • src/main/runtime/orchestration/orchestration-version-skew-migration.test.ts
  • src/main/runtime/orchestration/types.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/check-delivery-history.test.ts
  • src/main/runtime/rpc/methods/orchestration/messaging/check-worker.ts
💤 Files with no reviewable changes (1)
  • src/main/runtime/orchestration/db.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

Comment thread src/main/runtime/orchestration/db/schema/migrate-v41.ts Outdated
Comment on lines +177 to +180
(storedVersion >= 41
? hasOrchestrationSchemaObject(db, 'idx_deliveries_unacknowledged_mailbox') &&
hasOrchestrationSchemaObject(db, 'outstanding_deliveries', 'view') &&
hasOrchestrationSchemaObject(db, 'trg_deliveries_one_outstanding', 'trigger')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 33738


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '130,215p' src/main/runtime/orchestration/orchestration-schema-version-skew.ts
printf '%s\n' '--- v41 migration candidates ---'
fd -i 'migrate-v41|.*v41.*' src
printf '%s\n' '--- relevant definitions and callers ---'
rg -n -C 6 "outstanding_deliveries|trg_deliveries_one_outstanding|idx_deliveries_unacknowledged_mailbox|resolveOrchestrationMigrationStartVersion|hasCompletePostV6Schema" src

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- files containing v41 migration ---'
rg -l "outstanding_deliveries|trg_deliveries_one_outstanding|idx_deliveries_unacknowledged_mailbox" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**'
printf '%s\n' '--- target file outline ---'
ast-grep outline src/main/runtime/orchestration/orchestration-schema-version-skew.ts

Repository: stablyai/orca

Length of output: 1526


🏁 Script executed:

#!/bin/bash
set -eu
sed -n '1,260p' src/main/runtime/orchestration/orchestration-schema-version-skew.ts

Repository: stablyai/orca

Length of output: 8661


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- target file ---'
sed -n '130,215p' src/main/runtime/orchestration/orchestration-schema-version-skew.ts
printf '%s\n' '--- relevant files ---'
rg -l "outstanding_deliveries|trg_deliveries_one_outstanding|idx_deliveries_unacknowledged_mailbox" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**'

Repository: stablyai/orca

Length of output: 3965


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- migration and skew definitions ---'
rg -n -C 12 "outstanding_deliveries|trg_deliveries_one_outstanding|idx_deliveries_unacknowledged_mailbox|hasCompletePostV6Schema|resolveOrchestrationMigrationStartVersion" src/main/runtime/orchestration

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- v41 migration ---'
cat -n src/main/runtime/orchestration/db/schema/migrate-v41.ts
printf '%s\n' '--- migration dispatch ---'
sed -n '1,80p' src/main/runtime/orchestration/db/schema/migrate.ts
printf '%s\n' '--- consumers of derived delivery objects ---'
rg -n -C 5 "outstanding_deliveries|trg_deliveries_one_outstanding|idx_deliveries_unacknowledged_mailbox|hasOutstandingMailboxDelivery|getOrCreateRunDelivery" src/main/runtime/orchestration/db src/main/runtime/orchestration --glob '!*.test.ts'

Repository: stablyai/orca

Length of output: 37674


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- v41 migration ---'
cat -n src/main/runtime/orchestration/db/schema/migrate-v41.ts
printf '%s\n' '--- migration dispatch ---'
sed -n '1,75p' src/main/runtime/orchestration/db/schema/migrate.ts
printf '%s\n' '--- derived delivery consumers ---'
rg -n -C 4 "outstanding_deliveries|hasOutstandingMailboxDelivery|getOrCreateRunDelivery" src/main/runtime/orchestration/db --glob '!*.test.ts'

Repository: stablyai/orca

Length of output: 16551


Validate v41 delivery object definitions before accepting the stored version.

hasCompletePostV6Schema checks only object names. A v41 database with incorrect definitions can therefore skip migration repair. A bad view can hide outstanding deliveries from hasOutstandingMailboxDelivery, and a bad trigger can allow multiple deliveries for one mailbox. Validate the index predicate, view predicates, and trigger condition. Add a skew test with incorrect same-named objects and verify repair after reopening.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The derived-eligibility redesign is technically sound and exceptionally well covered, but schema v41 is a one-way local upgrade with no rollback backup — shipped binaries that still read deliveries.status cannot reopen an upgraded database. This remains the single gating item to resolve before un-drafting; the rest of the review confirms the implementation itself, and enumerates the concrete resolution paths.

Reviewed changes

  • Derive eligibility from a view — outstanding_deliveries (a SQLite view joining deliveries to unread messages via json_each) replaces the stored status column; replay, notification, and the single-active-batch constraint all read this one source. No read-time repair or message↔delivery sync remains.
  • Schema v41 migration — drops deliveries.status and its partial unique index, adds a fenced INTEGER, and recreates the single-active-batch backstop as an AFTER INSERT trigger consulting the same view. Version-skew detection now requires the view, trigger, and idx_deliveries_unacknowledged_mailbox at >= 41, so a dropped view is rebuilt on reopen.
  • In-transaction consumer validation — the optional requireCurrentRunConsumer flag is replaced by requireMailboxConsumer, which validates run / dispatch / attachment generations (and their run_id) inside the delivery BEGIN IMMEDIATE transaction rather than re-reading them outside it.
  • Nudge attribution + CLI text — formatMessagePointer prefixes [Orca orchestration notification]; --types and --ack help notes clarified; Delivery <id> remains printed only on consuming checks.

⚠️ One-way migration has no rollback path

The migration is transactional, so a crash mid-upgrade rolls back cleanly — the risk is not a corrupt database, it is the landed upgrade followed by a downgrade. Options to resolve before merge:

Technical details
# One-way migration has no rollback path

## Affected sites
- src/main/runtime/orchestration/db/schema/migrate-v41.ts — `DROP TABLE deliveries` (dropping `status`) is irreversible; no pre-upgrade backup is created.
- docs/reference/orchestration-delivery-storage.md — already documents the downgrade limitation honestly.

## Required outcome
- Choose a rollback story before un-drafting/merging: either accept + document the one-way upgrade, or preserve a downgrade path.

## Suggested approach (optional)
- Snapshot the database file(s) — including `-wal` / `-shm` if journaled in WAL mode — before the destructive `DROP TABLE`, so a downgraded binary can be pointed at the pre-v41 copy; or
- keep `status` in the v41 table (unmaintained/stale but readable) so older binaries can still open the database, trading away the "no stored status" goal.

## Open questions for the human (optional)
- Is there a realistic downgrade flow (auto-update rollback / version pin) a user could hit? If not, accepting the one-way upgrade as documented may be the right call.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior Pullfrog review at d008782. This commit implements the lifecycle-race fix and streamlines revocation.

  • Validate consumers under the delivery lock — requireMailboxConsumer now limits dispatch consumers to status IN ('pending', 'dispatched') (ACTIVE_DISPATCH_CONSUMER_SQL) and attachment consumers to potentiallyLiveRemoteAttachmentSql() (the complement of WORKER_SETTLED_STATES), which closes the race where a settled worker could still consume mail awaiting reroute. start_unknown / stop_unknown remain eligible, matching the existing uncertain-liveness contract.
  • Widen batch revocation to every unacknowledged batch — fenceOutstandingMailboxDelivery is renamed to fenceUnacknowledgedMailboxDeliveries and now fences acknowledged_at IS NULL AND fenced = 0 rows instead of only view-visible ones. This is what lets historical adoption (adoptLegacyRunIfNeeded, no generation bump) revoke a batch, since delivery.fenced is then the only block on a late --ack.
  • Dropped the redundant pre-read guard — check-worker.ts no longer re-reads readCurrentGeneration() before reading a delivery, relying on the atomic in-transaction check, and threads consumerSource: activeDispatch ? 'dispatch' : 'attachment' through both the ack and the create paths.
  • Added exact lifecycle-fencing tests — mailbox-consumer-lifecycle-fencing.test.ts asserts consumer_fenced for create/ack after local completion/failure and remote stop/failure on a second connection, preserves unread state and generation, and verifies start_unknown / stop_unknown attachments stay deliverable.

The regressions are exact rather than theatre — they assert specific consumer_fenced codes, fenced: 1, null acknowledged_at, and unchanged generations — and the settlement paths in the it.each matrix genuinely exercise the new status / liveness filters.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@Tauri-EPO

Copy link
Copy Markdown
Contributor

One question on the wake side of this, since delivery eligibility moves onto the messages view here.

Today mailbox-pointer-eligibility.ts excludes from the pointer only the types some waiter filters. The guide's coordinator wait is check --wait --types "worker_done,escalation,question", so a routine heartbeat never matches a waiter, gets a pointer, and wakes the coordinator for a message that carries no decision (#14910). In this design, does a heartbeat still earn a pointer, or does eligibility for the wake now depend on the message type as well?

Asking because #15173 (mine) changes exactly that predicate and I would rather rebase it onto the answer here than race this PR.

…y without a column drop

The outstanding_deliveries view now reads status = 'outstanding' plus unread
membership, so v41 only drops uniqueness from idx_deliveries_one_outstanding
and adds the view and trigger. Older binaries can still open the database.
Removes the column-drop migration, the v40 test fixture and hasColumn guards,
the fenced skew probe, and the unrelated nudge-text change.
The compatibility caveat it existed to explain no longer applies; the view
and index comments carry the remaining rationale.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes — delta since the prior Pullfrog review at 0e70dbc (new head a4115b2).

  • Kept deliveries.status and made v41 non-destructive — migrateV41 now only drops the unique constraint from idx_deliveries_one_outstanding (same name and predicate) and creates the outstanding_deliveries view plus AFTER INSERT trigger; no column is removed. An older binary opens the upgraded DB, sees a higher user_version, skips migration, and still resolves its status queries and expected index name. This resolves the one-way-migration gate raised at d008782.
  • Derived objects rebuilt on every open — createTables now execs DERIVED_DELIVERY_SCHEMA_SQL, so a dropped view/trigger is recreated on reopen even though migrate early-returns on a v41 stamp; migrate drops both before the table-rebuilding steps and recreates them after.
  • Status replaces the fenced column — DeliveryRow/DeliveryStatus revert to status, fencing writes status = 'fenced' for status = 'outstanding' rows, and the view filters status = 'outstanding' AND EXISTS(unread member).
  • Nudge prefix and version-skew v41 requirements reverted — formatMessagePointer no longer adds [Orca orchestration notification], and the schema-version-skew probes return to the v40 object set.

I ran the affected suites locally against a4115b2: 928 passed / 10 skipped across 121 orchestration files, plus the focused migration, lifecycle-fencing, check-delivery-history, and CLI-output tests, and pnpm tc:node is clean. The downgrade regression test asserts the retained status column and non-unique index shape, and that a v40 target leaves the v41 stamp untouched.

ℹ️ PR description still describes the superseded column-drop design

The head commit reverses the approach (status retained, migration non-destructive), but the PR body still advertises the old one: the Notes section calls v41 "a one-way local database upgrade" with "the removed status column" and no rollback backup, ELI5/What-Changed say the duplicated waiting status is removed, and the "Why" list still claims nudges gain an [Orca orchestration notification] prefix that no longer exists in formatter.ts. The [ ] Database downgrade tradeoff accepted or resolved before merge checklist item is also still unchecked even though retaining status resolves that tradeoff. Updating the description before un-drafting would keep merge reviewers from gating on a limitation the code no longer has.

Technical details
# Stale PR description vs. head

## Affected sites
- PR body — "Notes" describes a one-way v41 upgrade with the removed `status` column and no rollback backup.
- PR body — "ELI5" / "What Changed" state the duplicated delivery waiting status is removed.
- PR body — "Why" item 6 and the testing section advertise the `[Orca orchestration notification]` prefix, absent from `src/main/runtime/orchestration/formatter.ts:112`.
- PR body — checklist `[ ] Database downgrade tradeoff accepted or resolved before merge` remains unchecked.

## Required outcome
- Description and checklist match the retained `status` column and the now downgrade-compatible v41 stamp, so merge is not gated on a resolved limitation.

## Suggested approach (optional)
- Rewrite the Compatibility note to mirror `docs/reference/orchestration-delivery-storage.md` and check the downgrade checklist item.

Pullfrog  | View workflow run | Using openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important

The new downgrade round-trip test is not registered in any CI lane — the sharded unit job excludes tests/e2e/cross-version-wire/**, and the dedicated cross-version-wire job runs an explicit five-file list that omits it. The compatibility evidence the PR body cites therefore never executes on a PR.

Reviewed changes — delta since the prior Pullfrog review at a4115b2.

  • Added a historical downgrade round-trip test — orchestration-delivery-downgrade.unit.test.ts materializes the pre-v41 tree at aac38d698, creates a database, has current code upgrade it and advance past a fully-read heartbeat, then confirms pre-v41 code can still reopen, acknowledge both retained batches, and write a new batch that current code consumes.
  • PR description updated to the retained-status design — the body no longer advertises the column-drop/one-way upgrade or the reverted nudge prefix, and the round trip is listed under Testing. The earlier informational note is resolved.

The test is genuinely falsifiable: the not.toBe(oldBatch.delivery.id) assertion fails if current code still replays a fully-read batch by stored status, and it passes locally in ~5.4s against the pinned checkout.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using openrouter/deepseek/deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

const PRE_V41 = 'aac38d698ff75ac4c8658addab48ef5a83617619'

test('pre-v41 code opens, acknowledges and writes a v41 database, then current code reopens it', async () => {
const checkout = await materializeReleaseCheckout(PRE_V41)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test never runs in CI. unit-tests.yml:59 explicitly excludes tests/e2e/cross-version-wire/**, and the dedicated cross-version-wire job in pr.yml:655-660 runs an explicit five-file list that does not include this file. Add its path to that job's vitest run argument list — it is the only lane that checks out with fetch-depth: 0, so the pinned aac38d698 resolves.

Technical details
# New compatibility test is registered in no CI lane

## Affected sites
- `.github/workflows/unit-tests.yml:59` — `--exclude=tests/e2e/cross-version-wire/**` removes this file from the sharded `test` job.
- `.github/workflows/pr.yml:655-660` — the `cross-version-wire` job enumerates five `.unit.test.ts` files and omits `orchestration-delivery-downgrade.unit.test.ts`.

## Required outcome
- `orchestration-delivery-downgrade.unit.test.ts` executes in CI, so the v41 forward/backward-compatibility contract it encodes is actually enforced.

## Suggested approach
- Append `tests/e2e/cross-version-wire/orchestration-delivery-downgrade.unit.test.ts` to the `vitest run` list in the `cross-version-wire` job. Its `actions/checkout` already uses `fetch-depth: 0`, which `materializeReleaseCheckout` needs to resolve `aac38d698` (a confirmed ancestor of `origin/main`).

@Jinwoo-H
Jinwoo-H merged commit 3ab2a1b into main Sep 14, 2026
57 of 59 checks passed
zpyoung added a commit to zpyoung/orca that referenced this pull request Sep 16, 2026
* fix(native-chat): wait for the runtime capability probe before resolving the creation launch route (#19819)

* fix(native-chat): wait for the runtime capability probe before resolving the launch route

A worktree created before the renderer's hydration-gated capability refresh
runs read the local capability set as null, which
resolveStructuredNativeChatSupport treats as a blocker, silently degrading
structured native chat to the legacy terminal-backed route. Creation submits
now await ensureLocalRuntimeCapabilities(), which probes the local runtime
when no answer has landed yet, so the route resolves on an actual answer.

Fixes #19154

* fix(native-chat): await the capability probe in the work-item direct launch route too

prepareDirectWorkItemAgentLaunch is the fourth creation-flow route owner and
already async; a pre-hydration submit-after-ready launch (fix-checks) read the
unprobed cache as unsupported and silently degraded to legacy. Draft-delivery
launches were unaffected (draft-prompt blocks structured before the capability
check). Same shape as the three creation-submit sites.

* fix(native-chat): keep the capability probe starting synchronously

The broken-bridge hardening wrapped the probe in Promise.resolve().then(...),
which deferred window.api.runtime.getStatus() by a microtask. The session-tabs
restore deliberately overlaps its inventory RPC with this refresh and relies on
the probe already being in flight when refresh returns, so the deferral broke it.

The bridge call is synchronous again; a synchronous throw becomes a rejection
instead, which is what the wrapper was actually for.

* fix(native-chat): hydrate local runtime capabilities at renderer boot

The capability cache's only writer was `useLocalStructuredSessionTabsSync`,
gated on workspaceSessionReady + terminalStartupRestorationReady + the
experimental flag. Every `resolveAgentLaunchRoute` reader treats an
unanswered cache as "unsupported", so the answer arriving seconds late is
what produces the bare-terminal create in #19154 — awaiting the probe at a
route decision guards four call sites but leaves the window open for the
three readers that are synchronous and cannot await.

Start the probe from the renderer boot chain, ungated, so the answer is
cached before any launch route is resolved. The per-call-site awaits stay
as the backstop for the residual window and for re-probing after a failed
probe.

Also: hoist the full-creation probe above its cancel gate so the gate stays
adjacent to createWorktree; pin the retry-after-failure, concurrent-ensure
and missing-bridge contracts; drop a stale microtask tick and correct two
comments that no longer described the code.

* test(native-chat): pin the cancel gate around the capability probe

The probe added an await to two composer creation paths. Full creation had
no gate between the route decision and createWorktree, so the earlier
revision opened a window where a dismissed composer still created a
worktree; the hoist that closed it was unpinned. Quick creation already
gated immediately before runBackgroundWorktreeCreation, so its inline
await is safe — pin that too, since nothing asserted it.

Both tests fail against origin/main (no probe) and the full-creation one
fails against the pre-hoist revision.

* fix(native-chat): close the folder-create cancel window the probe opened

The probe added the first `await` inside `submitFolderWorkspaceCreate`. On
`main` that function ran straight through to `createFolderWorkspace` with no
suspension of its own, so its caller's `isSubmissionCancelled()` gate and the
create call sat in the same turn. With the probe inline, a composer dismissed
while the probe is in flight still creates the folder workspace and launches
an agent — the same defect the full-creation hoist fixed on the git path.

Resolve capabilities in `folder-submit-orchestration` above its existing gate
and hand them down, so the create path's prefix is synchronous again. The
parameter stays optional: a caller without a cancel gate keeps the probe.

Both new tests fail against `origin/main` and against this branch's previous
head; the cancel-window one still fails with its probe-pending assertion
removed, so it pins the create, not just the probe.

* refactor(native-chat): require pre-resolved capabilities on the folder create path

The cancel-window fix in f492064432 left its invariant -- a caller that gates
on cancellation must resolve capabilities above its gate -- enforced only by a
comment, because `hostCapabilities` stayed optional with an inline probe as the
fallback. A future caller that owns a cancel gate and forgets the parameter
would silently reopen the window twice fixed already, and nothing would catch
it: the caller census test pins `resolveAgentLaunchRoute` callers, not this
function's, and `exactOptionalPropertyTypes` is off so even an explicit
`undefined` is legal.

Make it required and drop the now-unreachable inline probe. The sole
production caller already passes it, so runtime behaviour is unchanged: the
old ternary never evaluated its `await` when a value was supplied.

`null` keeps its meaning -- probed, genuinely unknown -- and still degrades to
the legacy route; only absence becomes impossible. The launch-route test that
covered the removed probe is replaced by one pinning that `null` contract with
the cache and the bridge both holding the structured capability, so only the
handed-in value can produce the legacy outcome. The cases in the sibling suite
are not about the route, so they go through one typed wrapper that supplies the
unknown answer rather than repeating it 21 times.

---------

Co-authored-by: Merge Sim <sim@local>

* fix(worktrees): retire the chat tab of a chat with no child when its workspace goes (#19970)

* fix(worktrees): retire the chat tab of a chat with no child when its workspace goes

Deleting a workspace left a chat tab behind for every structured session that
had no attached provider child at the time, and that tab came back at the next
launch pointing at a workspace that no longer exists.

A provider child is scoped to a VISIBLE pane, not to a tab: the hold that keeps
one is `enabled: isVisible && isWorktreeActive`, and dropping the last hold
evicts the child after the release grace. So the sweep's liveness predicate
selected only "the chat that is the visible pane in the active workspace, or was
moments ago" — which means deleting a workspace from the sidebar while a
different one is active left every chat in the target invisible to the sweep,
and the `live.length === 0` early return did nothing at all.

The durable reference is `visibleSessionIds` in the agent-session record store.
Both purges a removal already performs miss it: the renderer drops
`unifiedTabsByWorktree` and the main process drops the workspace metadata, and
neither touches that index. Startup replays it, restores the session from it and
republishes the tab. Worktree ids are path-derived, so a later workspace created
at the same path inherits the old chat.

Splits the two concerns the sweep conflated in one list. Liveness still decides
what to CLOSE and what to refuse over, unchanged. Membership — the same fenced
record filter minus the liveness clause — decides what to RETIRE, and covers
exactly the complement of the close list so each session's tab is handled once.

Retirement runs from `killAllProcessesForWorktree`, past every point that can
refuse, not from the structured sweep itself: that sweep is joined BEFORE the
unstopped-PTY verdict so a structured refusal can outrank a terminal one, and a
tab retired there would still be ahead of a gate that can refuse the whole
removal — leaving the workspace in place with its chats gone.

* fix(worktrees): retire tabs across all teardown outcomes

* test(worktrees): type teardown fixtures

---------

Co-authored-by: Merge Sim <sim@local>

* feat(ai-vault-search): public session search contract and transports (#20277)

* feat(ai-vault-search): define public contract and service seam

* feat(ai-vault-search): add IPC runtime relay and web transports

* fix(ai-vault-search): register search IPC at the core handler site

ai-vault.ts was two lines over the 300-line max-lines limit; the search
handlers belong with the other register*Handlers calls anyway.

* fix(ai-vault-search): withhold degraded-root paths from relay status

Status carried local filesystem paths over the relay while hits redact
theirs. redactStatusForTransport applies the same policy at the same
boundary: relay callers keep each root's reason and the array length as
the count, so the type only makes root optional.

* fix(ai-vault-search): close diagnostic path leak and remove test casts

* feat(ai-vault-search): carry an execution host id and per-host outcomes on hits

* feat(ai-vault-search): route desktop search by execution host scope, including runtimes

* feat(preload): accept an execution host scope on session search

* feat(web): answer only for the paired runtime on session search

* docs(ai-vault-search): describe execution-host routing and the all-hosts merge

* test(ai-vault-search): cover every host scope, the all-hosts merge and wire compat

* fix(ai-vault-search): resume every host mid-page so a merged page never drops a hit

* fix(ai-vault-search): decode the merged cursor with a schema instead of casts

CI's type-aware audit refuses type assertions; a zod record validates the
per-host entries and yields the typed map without one.

* refactor(ai-vault-search): defer cross-host merged search

* refactor(mobile): migrate settings reads to RpcOperation (#20499)

* refactor(mobile): migrate settings reads to RpcOperation

Replay the settings slice on the landed RPC foundation after rebasing onto main.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): refresh task parity snapshots after main rebase

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): correct rebased declaration parity hash

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): account for main task declaration

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): preserve raw RPC rejection timing

Return the transport promise directly and interpret replies separately so sibling Promise.all rejection order cannot change.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): refresh parity hashes after timing fix

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): use operation interpreter after raw request

* test(mobile): refresh settings migration parity hashes

Refresh hook and statement parity hashes for the two task declarations whose settings reads now use RpcOperation request and interpretation.

Changed declarations:
- useMobileTasksRuntimeHydration: settings.get replaced by settingsRead request/interpret.
- useMobileTasksWorkspaceCreateActions: settings.get response handling replaced by settingsRead request/interpret.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): give native chat one tail-follow owner so streaming stops jumping (#20493)

* fix(mobile): stabilize native chat tail following

* refactor(mobile): give native chat one tail-follow owner

Extract the streaming scroll contract into
use-mobile-native-chat-tail-follow, so intent and geometry have a single
writer instead of a state/ref pair hand-synced at five call sites.

No behaviour change: the existing guards pass untouched.

* fix(mobile): fence native chat tail follow through momentum

* fix(mobile): repin chat at measured tail

---------

Co-authored-by: Merge Sim <sim@local>

* fix(mobile): preserve delivery ambiguity across transport cutover (#20280)

* fix(mobile): preserve delivery ambiguity across transport cutover

Let physical close settle requests and retain its error as the cutover cause, copying only an existing delivery-unknown mark. Pin sent and unsent caller outcomes and both cutover predicate carriers.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(mobile): pin the RpcClient.close() settlement contract

close() was declared `() => void` with no stated obligation. That was harmless
while migrateTo rejected pendings itself; now that it does not, close() is the
retiring generation's only settlement path, so a type-compatible implementation
that leaves a request pending strands its caller for good.

States the obligation on the declaration and pins it for both trackers the real
implementations reject through. Dropping the delivery-unknown flag, dropping the
relay mark, or leaving pendings in the map each fail a test.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): read the cutover cause without a type assertion

main's new casting gate rejects `(error as Error).cause`; narrow instead so the
assertion still distinguishes a missing cause from an unmarked one.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(runtime): reject malformed file Base64 padding (#20283)

Require padded file-write payloads to end on a Base64 quartet boundary. Cover both RPC methods and padded final upload chunks, and document client compatibility evidence.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(tasks): preserve repository results under GitHub search quota (#20460)

* fix(tasks): preserve repository results under GitHub search quota

* fix(github): preserve search budget on count fallback

---------

Co-authored-by: m4air <m4air@Mac.localdomain>

* fix(ai-vault): stream oversized remote session transcripts (#20455)

* fix(ai-vault): stream oversized remote session transcripts

* fix(build): bundle streamed JSON parser in desktop main

---------

Co-authored-by: m4air <m4air@Mac.localdomain>

* fix(grok): stop replayed Claude/Cursor hooks reporting Grok panes as Claude (#20507)

* fix(grok): stop replayed Claude/Cursor hooks reporting Grok panes as Claude

Grok's hook discovery reads ~/.claude/settings.json (and the Cursor equivalent)
for vendor compatibility, and that is on by default. So inside every Grok pane
Orca's managed Claude hook fires in addition to Orca's managed Grok hook, and
both POST the same Grok envelope. The Claude-routed copy lands last and wins, so
the pane's agent type is resolved from the POST route as "claude" and no
Grok-specific normalization runs for it.

Guard the managed Claude and Cursor scripts on GROK_HOOK_EVENT, which Grok's hook
runner stamps into every hook subprocess it spawns — including replayed vendor
configs — after any user-supplied environment, so a hook cannot spoof it. This
mirrors the existing DEVIN_PROJECT_DIR guard in the same script, which solves the
identical problem for another agent that imports Claude hooks.

Placement is load-bearing: the guard sits after the stdin capture, so Grok's
writer never blocks, and before both the spool write and the HTTP POST, so a
replayed event cannot leave a spool entry that replays later. The Windows
variants jump to the stdin-drain label rather than exiting, because abandoning
stdin there hangs the writer.

The guard is scoped to agent === 'claude'; OpenClaude reuses ClaudeHookService
with its own settings file, which Grok does not replay, so it is unaffected.

Verified live against Grok 1.0.25 in a dev instance: the pane's reported agent
type goes from "claude" to "grok" on every turn-end, including the hidden
follow-up turns Grok runs when background work finishes.

The guard pushed hook-service.ts past the 300-line cap, so the script builder
moves to a sibling hook-script.ts. That mirrors the existing split under
src/main/cursor/, where the service owns install/status and the script module
owns script text.

* fix(agent-hooks): preserve Windows background worker stdin contract

* fix(browser): press keys through a US-layout CDP key table instead of a subprocess per keystroke (#15310)

Typing in the remote browser pane spawned an agent-browser process per
keystroke -- ~160ms each, so a 17-character password took seconds -- and
some keys arrived half-formed: F-keys, Insert and ContextMenu dispatched
windowsVirtualKeyCode 0, Shift+1 typed '1' instead of '!', and non-ASCII
printables reported success while typing nothing at all.

keypress now resolves the key name through a US-layout table and
dispatches the Input.dispatchKeyEvent pair over the electron debugger,
the same transport mouseClick already uses. Two fallbacks keep the old
behavior reachable:

- a single printable BMP character outside the table dispatches in
  process as an IME-style event (keyCode 229 with the character as text,
  the shape composed input already has when it reaches pages)
- anything else -- media keys, surrogate pairs, unrecognized names --
  goes to the helper exactly as before, and only that path pays for
  creating the helper session

Virtual key codes come from the table, never from the character's own
char code: charCodeAt puts '&' on 38 (VK_UP) and '.' on 46 (VK_DELETE),
which Blink runs as caret commands that swallow the character.

Dispatch failures normalize the way evaluate's already do -- a gone page
becomes browser_tab_not_found, anything else browser_error -- because
attach and sendCommand reject with plain Errors that the RPC layer would
report as runtime_error, and the pane only reclaims a dead page when it
sees a browser_* code.

Result shape is unchanged and no wire, schema or RPC surface moves, so
mixed-version client/server pairs see no difference. Pages can observe
the fidelity fixes: Shift+a now types 'A', Shift+1 now types '!',
Alt+<char> no longer carries text, and editing keys arrive as rawKeyDown.
Each matches what a real US keyboard produces.

Verified against the shipped agent-browser 0.27 binary on the same
browser: every difference is a fix, nothing regressed. macOS editing
shortcuts (Cmd+A) do not fire through either path -- Blink runs those off
the native responder chain and neither sends CDP `commands` -- so that
gap is unchanged, not introduced.

Co-authored-by: Neil <neil@stably.ai>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* fix(rpc): validate provider-specific fields in TaskProviderIdentity (#20284)

* fix(rpc): validate task provider identity fields

Validate provider-specific field types while preserving nullable scopes and unknown identity fields. Record the producer census and pin validation with regression tests.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(rpc): reject a blank GitHub owner or repo

normalizeTaskProviderIdentity treats a blank owner or repo as no identity at
all, but the schema accepted '' and whitespace-only, so the two disagreed about
the same payload. Refined rather than trimmed: trimming would rewrite the
parsed value and change what the handler receives.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(rpc): re-measure the identity evidence counts

The blank-field commit added seven tests, so the recorded 74/16/58 described the
commit before it. Re-ran both: 81 tests, and the discriminant-only mutation now
gives 17 failures / 64 passes.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(rpc): correct the remaining stale gate count

The blank-field commit moved the full-RPC total too; 2,463 was the count before
it. Re-ran: 278 files, 2,470 passed, one skipped.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* perf(terminal): batch file-link checks on their owning host (#20463)

* perf(terminal): batch file-link existence checks on their owning host

* test(relay): allow additive filesystem capabilities

* fix(web): keep terminal file links working under batched existence checks

createShellApi omitted pathsExist, so withFallback answered the new batch
call with a truthy proxy resolving to undefined and the whole hover batch
rejected — dropping every link on lines with an out-of-worktree path.

* test(web): assert the shim without type assertions

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: m4air <m4air@m4airs-MacBook-Air.local>
Co-authored-by: Neil <neil@stably.ai>

* feat(native-chat): support file drag and drop (#20494)

* feat(native-chat): support workspace file drops

* fix(native-chat): report OS file drops that attach nothing

#15782 is a silent failure on the Finder route, and that route still
swallowed every way it could fail:

- the preload handler returned with no feedback when the OS handed us
  file items `webUtils.getPathForFile` could read no path from (promised
  or virtual files). It now sends the existing `rejected` payload with a
  new `unresolved-paths` reason, which the global drop toast names.
- the composer's external-attach path dropped the batch with no notice
  when every path failed authorization, when an upload came back empty,
  and (new in this branch) when the owner changed mid-flight. Each exit
  now sets a notice; only a disabled composer stays quiet, because it has
  no notice surface.

Also stops `resolveNativeChatAttachmentOwnerForWorktree` throwing out of a
drop/IME handler when an SSH connection's generation is gone mid-attach —
that is an unknown owner, which the resolver already models as
`not-ready`.

* refactor(native-chat): one owner-identity check for composer attachments

The branch had two near-identical "is this still the same owner" helpers,
one per attach route, and they disagreed: the workspace-drop copy ignored
the SSH connection generation, so a reconnect between the drop and the IME
flush read as the same owner and the path landed on a new connection.

Collapses both onto one predicate in the pure ownership module (the
store/toast-free seam both routes already depend on), which compares the
full SSH expectation and never treats `not-ready` as a match.

* perf(file-explorer): resolve drag ownership at dragstart, not per render

The virtualized row list resolved the selection's source execution host on
every render — the virtualizer re-renders on every scroll frame, so a large
multi-selection paid a full projection scan plus a route allocation per
selected path per frame, and per visible row on top of that. Only
`onDragStart` ever read the result.

Rows now receive a resolver they call with the paths they are about to
drag. The three copies of the "stamp only if both halves resolve" guard
(explorer row, both combined-diff row shapes) collapse into one helper next
to the writer.

* fix(native-chat): refuse a guarded composer drop visibly

The drop handlers claimed the drag (preventDefault + stopPropagation) before
checking `disabled`, so a guarded composer told the browser it accepted the
drop, left the copy cursor up, and then did nothing — the same silent swallow
this branch exists to remove.

Dragover now answers `none` when the composer is guarded, so the cursor refuses
and no drop event follows. It still claims the event either way: the composer
sits inside the terminal surface, which accepts the same drag and would paste
the paths into the shell instead.

Drops `stopImmediatePropagation`. The capture-phase `stopPropagation` already
keeps the event off the editor below, so the stronger form only risked
suppressing unrelated listeners on the React root.

The fake DataTransfer in the test now starts at a dropEffect we never write, so
asserting `none` or `copy` proves the handler set it.

* fix(native-chat): decide attachment ownership per path, not per batch

A queued batch can mix sources — a workspace drop the target host owns and a
client-local paste it cannot read — because IME composition holds both until it
settles. Collapsing the batch to one verdict refused the whole thing on a remote
target, including the drop the user was entitled to make.

The verdict now follows the path it belongs to: owned paths attach, client-local
ones are refused, and the refusal is reported rather than dropped. A stale owner
still refuses everything, since that means the target moved under all of them.
Also guards the empty-batch case, which previously read as "every path owned".

* refactor(combined-diff): resolve drag ownership from the live workspace

The combined diff captured an execution host into the open-file record at tab
open and drilled it through three components to reach the row. That host was
never persisted, so after a restart every drag from a restored diff was refused
until the tab was reopened, and the capture failure was swallowed into an
undefined source with no trace.

Rows now resolve the owner the same way the source-control rows already do, from
the workspace the diff belongs to at the moment of the drag. That deletes the
prop drilling, the store capture and its bare catch, and leaves one way to
answer "who owns these paths" for every live listing.

The file explorer keeps its per-node owner: its tree is a cache that can still be
showing a previous host's listing, which is exactly what that field records.

* revert(file-explorer): drop the workspace-id tree reset

Resetting and reloading the tree when the workspace id changes at an unchanged
path is not needed for the drag source to be correct. The tree already records
the workspace whose root listing it committed, so a cache left over from a
previous workspace stamps that workspace and the composer refuses the drop —
the intended answer, reached without touching the reset rule.

That rule clears selection, the name filter and undo history, which is more
file-explorer behaviour change than this feature asked for.

* test(native-chat): stop the external-attach mock hiding new notices

The hook's test replaced the whole attachment-owner module with a hand-written
stub, so the two notices added alongside the owner-change guards resolved to
undefined. Calling them threw inside the async attach loop — an unhandled
rejection, which leaves every test in the file reported as passing while the run
as a whole fails. CI caught it; a local run reporting only pass/fail counts does
not.

The mock now spreads the real module, so a notice added later cannot go missing
from it, and both owner-change tests assert the string a user would read instead
of only asserting that nothing attached.

* test(native-chat): guard the last-path owner change on a one-file drop

The owner flipping while the final path is authorizing has no next loop
iteration to catch it, so the post-loop check is all that stands between a
single-file drop and a path attached to a host that no longer owns it — and a
one-file drop is the ordinary shape. No test covered that exit.

Removing the post-loop check now turns this red; before it, only the
multi-path exit was guarded.

* fix(native-chat): keep a mixed attachment batch in attach order

applyResolvedPaths partitioned a queued batch into a target-owned half and
a client-local half and concatenated them. An IME-delayed batch that mixed
a workspace drop with a paste made earlier in the same composition was
therefore inserted owned-first, so the dropped reference jumped ahead of
the pasted one in the draft.

Filter against the two verdicts in place instead. Membership is unchanged,
the order the user attached in survives, and the two intermediate arrays go
away.

* fix(file-explorer): name the owner of a dragged path whose row is hidden

A multi-selection outlives the rows that showed it. Nothing prunes
selectedPaths when a directory collapses, when the name filter narrows, or
when dotfiles are hidden, and the drag still carries every selected path.
Drag-source resolution read those owners from the row projection, which is
built from visible rows only, so one hidden path collapsed the whole drag to
an unstamped one and the composer refused it as coming from another
workspace.

The owner was never unknowable — the dir cache the projection is built from
still records which host listed that path. Fall back to it when the path has
no visible row. A path in neither (a name-filter synthetic node for a
directory that was never listed) still fails closed.

* fix(native-chat): ask which workspace the composer serves now

The IME-flush ownership check compared the workspace id captured when the
drop happened against the same captured value, so for a structured pane the
comparison could only ever hold. The live protection came from the host and
owner checks beside it; this one asked nothing.

Read the id through a ref so the check means what it reads as. A pane whose
structured target moves between the drop and the composition settling now
refuses the queued path instead of attaching it.

* fix(native-chat): ask which workspace an external attach lands on

The post-await ownership gate resolved the owner through the render closure, so
it re-asked the workspace the attach started in and compared the answer with
itself. A tab moved to another workspace mid-authorization passed the gate, and
the paths landed in a composer that no longer served that workspace.

Read the pane through a ref and compare the workspace identity as well as the
owner: two workspaces can both report a local owner, so the owner alone cannot
tell them apart.

* test(native-chat): read the real notice on a workspace drop

The drop tests hand-built their attachment-upload mock and hand-copied the
not-ready wording into it, so the assertion tracked the copy rather than the
string a user reads: rewording the real notice left all 15 tests green.

Spread the real module and override only the owner resolver, matching the two
sibling test files in this directory. Rewording the notice now fails the test.

* docs(native-chat): restore the hook's doc comment to the hook

The workspace comparison landed between the doc block and the function it
describes, leaving the comment attached to a type alias.

* test(native-chat): cover the upload window for a moved pane

The workspace-currency gate guards two windows and only the authorize loop was
covered. The upload window is the longer one: the paths go to the worktree the
attach captured, so a pane that moved workspaces meanwhile must not receive
remote paths living under the workspace it left.

* test(native-chat): pin the two untested attachment refusals

Refusing an already-blocked target at the drop rather than queueing it had no
test: queued paths that can never attach still spend the pending budget, and the
next legitimate drop is then turned away for being one too many.

Also pins the immediate already-false ownership verdict. Today's only caller
settles ownership synchronously so it cannot arrive false, but the hook exports
this entry point and the fallback is not a refusal — a false verdict is not
"owned", so a remote target blames client-local attachments for an ownership
failure. Verified: removing the branch reports the wrong notice.

* docs(native-chat): say which rule the ownership refusal follows

The per-path comment sat directly above the batch-wide ownership refusal while
describing the blocked-target logic below it, so the refusal read as a
contradiction of the line under it rather than as the file's stated rule.

Name the rule at the refusal: a failed ownership verdict refuses the whole
completion, the same way the pending-limit rejection does.

---------

Co-authored-by: Merge Sim <sim@local>

* fix: show unexpected signout notice only once across versions (#20526)

* test(mobile): consolidate the RPC migration's verification infrastructure (#20521)

* test(mobile): record main RPC hooks and regression schedules

Add scripted sender recordings, guarded main goldens, reply matrices, lifecycle schedules, settings caller fixtures, and targeted B-seed mutants.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): flush recording user actions through React act

Keep lifecycle updates in separate act boundaries while wrapping direct stateful user actions.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): compile recorded modules with the Node VM API

Use the same trusted-source execution boundary as existing mobile VM test harnesses.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): pool golden values and hoist pre-divergence checkpoints

Golden format version 2 stores each distinct observation field value once in
a `values` map keyed by a 12-hex sha256 of its sorted-key JSON, and a
checkpoint references five hashes. Output stays pretty-printed; the reader
rejects any other format version, resolves hashes back to values, and reports
the scenario, checkpoint, field and JSON path on a mismatch.

Generated variants now declare where their distinguishing input lands, so
checkpoints observed before that point are recorded once in a `.prelude`
scenario instead of once per reply partition. Reply matrices, interruption
schedules and lifecycle schedules share the primitive, which asserts each
variant's pre-divergence prefix matches the base. Equal-but-differently-reached
checkpoints are untouched.

17.71 MB / 3,599 checkpoints / 58.4% intra-file duplicates becomes
4.21 MB / 1,961 checkpoints / 23.6%, with every file's set of distinct
observations unchanged.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): make the recordings sense deadlines, the recorder, and every family

The goldens carried no temporal information, so a request deadline could be cut
to a third and all 61 files stayed byte-identical. Every threshold is now
straddled by two advances with a checkpoint between them: the 30 s request
deadline in both schedule drivers, the 120 ms search debounce in b1, and the 60 s
repo-metadata cache TTL. Shortening any of them moves an observation.

The record fence pinned product sources but excluded the whole recorder, so
--record could rewrite every golden from a modified runner and report the
baseline intact. Goldens now pin recorderSha256 over every non-markdown file in
the runner plus pilot-scenarios.json, and the fence exemption shrinks to the one
directory that digest covers.

Mutation evidence covered 3 of 13 mounted operations. There is now one anchored
mutant per adapter family, covering 11 operations and 51 of the 61 goldens; the
two omitted are the pure async loaders whose entire output is their settlement.
Anchors are asserted to match exactly one site, which caught the acceptance
mutant silently half-applying against three identical guards.

The archived-tree assertion pins each seed's visible state instead of merely
differing from main, and error observations carry code and cause when present.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): record settlement times instead of straddling deadlines

The previous commit made the reviewer's divide-by-three deadline mutant fail by
placing checkpoints on each side of the 30 s deadline. That is a patch: a timing
change that does not cross a hand-placed boundary stays invisible. Those
scenario edits are reverted, and pilot-scenarios.json and schedule-driver.ts are
byte-identical to what they were before them.

The real defect was that the projection had no temporal dimension, so every
settlement now carries startedAt and settledAt in virtual milliseconds on the
pinned fake clock. Any transition the product schedules for itself is recorded
at the time it actually fires, so a deadline or debounce change of any size, in
either direction, moves a recorded number.

A checkpoint's own clock is not recorded. It is always the sum of the scripted
advances, so it is a function of the scenario rather than of the code under
test; run-recording.ts asserts that equality at every checkpoint instead, which
costs no bytes and fails loudly if it ever drifts.

projectionVersion is 2 and all 61 goldens are re-recorded. With the added
timestamps stripped, the distinct-observation set is identical to the previous
recording, so the change is purely additive.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): probe the repo-metadata cache inside its TTL window

Recorded settlement times cover thresholds the product schedules for itself, but
not one it only consults when something else makes it act. The repo-metadata TTL
is the single such case: with probes only at 0 s and 60 s, a 20 s TTL and a 60 s
TTL are both expired at 60 s and record identically, so a 3x cache-lifetime
regression was invisible.

settings-repo-cache-expiry now probes the cache at 59 s as well. This is
coverage, not a substitute for recorded time: it bounds how small a TTL
reduction is visible rather than making the reduction itself observable, and the
README says so.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): record the reply shapes a host can send, not a cross product

The reply matrix froze ~26 malformed envelopes crossed against every consumed
field and three boundary kinds, which is 163,925 lines of JSON pinning accidents
on inputs no desktop produces. `successResponse` always sets `result`, so a JSON
wire has no explicit-undefined slot, and no mounted handler returns a number, a
string, an array, a bare `{}` or a boolean: `settings.get` returns
`{settings: ...}`, and the seed methods return an object or nothing.

Each family now runs nine witnessed partitions once, with no field cross: a
normal result, an absent result, `null`, an inner `{ok: false}` envelope with a
string or an object error, an inner envelope missing `ok`, an outer refusal,
`method_not_found`, and a transport rejection. `null` stays because
`linear.getIssue` returns it for a missing issue and b2 is a shipped null-result
bug; it is also what carries the one named delta these goldens record.

`run-step1-exit.ts` had zero callers and shelled out to the same two Vitest
files as `rpc-recording.mts`, so it and its README paragraph go, along with
`MUTATION_NAMES`, which only it read.

In the module loader, the `rpc-delivery-ambiguity` escape is measured dead: over
every scenario, mutant and reference run it was taken once, by the test that
existed to take it. Golden comparison already fails loudly if a mounted module
ever imports the marker, so both go. The history-panel exposure moves into a
declarative table beside the mutation anchors, leaving the loader with one
source-text mechanism and no per-file branch.

The VM stays. Mount adapters load product sources from an arbitrary `root`, and
the archived bcba08b3e4 tree is bare `mobile/src` and `src` with no package.json
and no node_modules, so no bundler-resolved import can reach it and the seed
rejection gate cannot run without it. Direct import also swaps a 38-module lazy
graph for a 338-module eager one behind 20 native mocks, because
`mobile-tasks-dependencies.ts` re-exports from `react-native` and four other
native packages and `export *` enumerates.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* fix(mobile): treat the recorded lockfile hash as provenance, not an oracle

Every golden pinned `lockfileSha256`, so any dependency bump on main failed all
61 comparisons on the merge commit while the traces were identical. A dependency
that changes behaviour changes the trace itself; one that does not must not fail
a candidate. `platform` already had this exemption — `lockfileSha256` joins it.

Recording still refuses to run unless the lockfile matches the pinned baseline,
so goldens are still produced under frozen conditions.

Verified against main's lockfile: 85 passed, previously 61 failed. The declared
mutation set still reports every mutant killed.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(rpc): cover repeat queries and settings refresh boundaries

Add three scenarios, preserve existing traces, remove unreachable archived checks, and document observed mutation kills and remaining adapter limitations.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(rpc): keep the known-open holes, drop the review transcript

The audit file was mostly a point-in-time record of mutation runs that had already
happened, in an artifacts directory, where it would go stale on the next scenario
change. The durable part is which holes are still open and why they cannot be
reached, which belongs beside the runner it describes.

Markdown is outside recorderSha256, so no re-record; 88 passed | 3 skipped.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* chore(mobile): stage the nine RPC probe scenarios and goldens

These existed only on one machine's /tmp. Landing them verbatim first so a
reboot cannot lose them; a follow-up commit moves them into the suite.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): fold the nine probe scenarios into the recording oracle

The probes were env-var invocations over loose /tmp manifests. They now live in
pilot-scenarios.json and rpc-foundation/goldens, so `pnpm --dir mobile test` runs
them with no flag to remember.

Re-records every golden against main (22f56f7c2a). Two causes:

- #20280 gave LogicalClientCutoverError the delivery-unknown mark and its cause,
  so nine cutover/interruption goldens now record `isRpcDeliveryUnknown: true`
  plus a `Connection closed` cause. The other 55 are byte-identical after 260
  commits of main.
- #20499 replaced the five anchored raw-envelope reads with typed operations, so
  those mutation anchors matched zero sites. Each is re-anchored at the same
  defect's new home; bot-overrides moves to the shared reader that now owns it.

probe-hole-witness.test.ts pins hole and closure together: a probe must kill its
mutation and every pre-probe scenario of the same operation must still survive
it, so a redundant probe fails instead of accumulating.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): list the recording harness in the raw-port inventory

main's #20026 boundary test fails on any non-test file that reaches the raw
request port and is not inventoried. The oracle's scripted transport is exactly
that — it drives the real tracker and logical client — so it belongs in OWNERS
beside the supervisor fakes, not in the step-4 pending backlog.

Also states what the oracle covers, the two holes it was blind to until the
probes, and the step-4 runbook.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): pin the goldens to the tree that recorded them

The inventory entry is a fenced product-tree edit, so --record refused against
main's sha. Baseline now names the branch commit the goldens were recorded from;
the next re-record after this lands bumps it to the merge commit.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): carry a SAFETY rationale on every recorder cast

main added a changed-code casting gate after this branch was cut, so 45 `as`
sites in the recorder read as new findings. Each now states why the assertion
holds; they cluster into five reasons — recorded observations are RecordedValue
by construction, parsed manifests and goldens are validated on the next lines,
interned pools resolve their own hashes, a VM-evaluated module has no static
type, and the mount adapters supply only the members each hook reads.

Re-records the goldens: the comments move recorderSha256.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* docs(rpc): state the measured blindness, not the assumed one

Applying each mutation to real product source shows the two holes are not equal.
The reorder is invisible to 83 of 84 tests and only a probe sees it. The refusal
blanking is also caught by the family reply matrix, because a refusal from cold
publishes null over a non-null initial value — an observational gap, not a
detection gap. Says so rather than letting the stronger claim carry both.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): close three ways the oracle could pass without checking

All four review findings were real; three let the oracle report green while
verifying less than it claimed.

- The baseline guard used `git diff --quiet`, which ignores untracked files, so
  an untracked module under mobile/src or src/shared could change resolution
  while a golden still recorded a pinned baseline header. Adds a
  `git ls-files --others` check over the same paths, recorder still exempt.
- The determinism loop read `Number(env ?? 2)` unvalidated, so
  RPC_FOUNDATION_DETERMINISM_RUNS=0 skipped the body and 57 tests passed having
  recorded and compared nothing. Now requires an integer >= 2.
- Cleanup-time observations were dropped: every checkpoint clones the effects
  array, so anything appended during dispose or the final flush never reached a
  golden. Warns and documents the six scenarios that hit it today; recording
  them changes every golden and is its own change.
- Two SAFETY rationales described each other's assertion. Swapped.

Goldens re-recorded for the recorder-digest change: 73 files, one header line
each, no recorded observation moved.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* test(mobile): record teardown observations as a cleanup checkpoint

Each checkpoint clones the effects array, so a rejection or state write produced
by dispose, the transport teardown or the final flush landed after the recording
was built and never reached a golden. An unmount leak is exactly what this
oracle exists to catch, so teardown now runs on the recorded path and anything
it observes becomes a checkpoint with id `cleanup`. State is captured before
dispose, since the operation is gone afterwards.

Six scenarios were dropping observations, across five goldens: projectRowDetailError,
projectMutating, hostLabelById, hostPlatform, workspaceAgent, workspaceAgentOverridden,
creatingKey, selectedAgent, agentOverridden and error. Those five gain a cleanup
checkpoint; the other 68 goldens change by their header line only, so no existing
observation moved.

Also fixes the README's own formatting, which failed `format:check`.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* Update README downloads badge

* refactor(attention): move the agent attention boundary off terminal panes (#20525)

* refactor(attention): move the agent attention boundary off terminal panes

The completion-attention pipeline asked PTY questions inline, so a non-terminal
agent surface was structurally excluded from unread, delivery and acknowledgement.

Extract a provider-neutral policy under `src/renderer/src/attention/` that reaches
every surface fact through an adapter, and move the PTY-shaped predicates
(`hasLivePtyForNotification`, `isCurrentLivePaneKey`, `isCurrentKnownPaneKey`,
`isVisibleForegroundPaneKey`, plus the leaf/foreground resolution the auto-ack scan
did inline) into `terminal-attention-surface.ts`, now their only call site.

Unread markers carry the reason that wrote them (`agent-completion`,
`terminal-bell`, `manual-mark-unread`, `legacy`); an unclassified boolean still on
live state reads as `legacy` rather than being guessed or migrated.

Main's `notifications:dispatch` closure becomes a delivery service with injected
collaborators; the IPC handler is a thin adapter. Behaviour is unchanged: unread is
still written before main's desktop gate, the tray dot before the cooldown/focus
gates, and mobile fan-out before the desktop early returns.

* fix(attention): recognize classified tab unread markers

* fix(grok): defer managed hook pane guard expansion to shell (#20534)

* fix(auth): report callback failures instead of cancellation (#20535)

* fix(auth): distinguish failed sign-ins from user cancellation

* test(mobile): fix conditional registration lint and refresh recorder fingerprints

* perf(skills): bound WSL installed skill discovery (#12314)

* perf(skills): bound WSL installed skill discovery

* fix(skills): preserve bounded discovery correctness

* fix(skills): bound WSL metadata prefilter reads

* fix(skills): isolate absent discovery cwd cache keys

* test(skills): adapt WSL discovery mocks to runner

* fix(skills): preserve filtered discovery fallbacks

* fix(skills): share filtered scans and preserve WSL inventory

* fix(skills): share WSL scans without losing skill aliases

* fix: preserve skill metadata and retire filtered peer caches

---------

Co-authored-by: m4air <m4air@Mac.localdomain>

* fix(git): skip upstream remote probes when the remote is absent (#18455)

* fix(git): skip upstream remote probes when the remote is absent

Issue and PR resolvers listed remotes by probing `git remote get-url
upstream` on every poll, including origin-only clones where that remote
cannot exist. List remotes once, cache against git config, and skip the
probe unless `upstream` is present.

* fix(git): avoid stale remote probe cache entries

* fix(github): observe origin repository probe failures

* fix(github): observe verified origin probe failures

* fix(github): skip missing upstream probe for PR lists

* test(github): scope the #9171 lazy-resolution guard to default-branch commands

The guard asserted that no git command runs for an open PR, using "no git at
all" as a proxy for "no default-branch resolution". Remote-name listing is a
separate concern, so allow it and keep every other command forbidden; the
symbolic-ref/rev-parse resolution this issue is about stays unreachable.

---------

Co-authored-by: Neil <neil@stably.ai>

* refactor(orchestration): derive delivery eligibility from messages (#19837)

* fix(orchestration): retire read deliveries and clarify mailbox recovery

* fix(orchestration): simplify delivery recovery and update nudge contracts

* test: align orchestration check help expectation

* refactor(orchestration): derive delivery eligibility from messages

* fix(orchestration): validate live consumers and simplify batch revocation

* refactor(orchestration): keep deliveries.status and derive eligibility without a column drop

The outstanding_deliveries view now reads status = 'outstanding' plus unread
membership, so v41 only drops uniqueness from idx_deliveries_one_outstanding
and adds the view and trigger. Older binaries can still open the database.
Removes the column-drop migration, the v40 test fixture and hasColumn guards,
the fenced skew probe, and the unrelated nudge-text change.

* docs(orchestration): drop delivery storage reference

The compatibility caveat it existed to explain no longer applies; the view
and index comments carry the remaining rationale.

* docs: revert unrelated formatter churn

* test(orchestration): verify historical database downgrade round trip

* fix(ssh): bound relay incumbent lsof probe (#18304)

* fix ssh relay incumbent probe timeout

* test ssh relay unconfirmed probe termination

* fix(ssh): preserve connect evidence while bounding lsof

* fix: preserve uncertainty when relay holder enumeration fails

* fix(ssh): supervise lsof helpers and preserve partial holder evidence

* test(ssh): prevent GC racing unconfirmed probe cleanup

* refactor(process): keep POSIX lsof supervision in process owner

* fix(ssh): confirm census cleanup and handle probe startup signals

* fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts

lsof warns to stderr about mounts it cannot stat, and any stderr byte forced
the holder enumeration to 'unavailable' — making the 'exited' verdict, and so
husk reaping, unreachable on those hosts. Pass -w to suppress the warnings.

* Revert "fix(ssh): keep lsof holder evidence usable on hosts with unstat-able mounts"

This reverts commit 10d7b0db375.

Passing -w is correct in isolation, but it activates a previously dormant
path: in the SSH docker e2e lsof warns about the container filesystem, so
every probe used to degrade to 'unavailable'. Suppressing the warning lets
holder enumeration succeed and the takeover/reaping path run for the first
time there, and 'e2e / ssh docker watcher isolation' then hung to the job
timeout (52m, vs 28m passing on the parent commit).

The underlying gap is real and still open: on hosts where lsof always warns,
the 'exited' verdict and husk reaping stay unreachable. Fixing it needs the
reaping path understood in that environment, not just the flag.

---------

Co-authored-by: m4air <m4air@Mac.localdomain>
Co-authored-by: Neil <neil@stably.ai>

* perf(remote): avoid repeated capability probes during file imports (#14555)

* perf: avoid repeated remote import capability probes

* test: cover cold remote import compatibility probe

* fix(remote): fence imports across runtime reconnects

* fix(remote): bind import proof to connection

* fix(remote): fence import routing by runtime identity

* test(remote): remove unsafe import fixture assertions

- type remote RPC mocks at declaration so call arguments stay checked
- narrow upload params before reusing generated temp paths

---------

Co-authored-by: Neil <neil@stably.ai>

* fix(grok): announce a completion once, when Grok is actually finished (#20523)

* fix(grok): announce a completion once, when Grok is actually finished

Orca pinged on every Grok turn-end. Grok runs turns the user never asked for:
when a background task finishes it wakes itself, does a little work, and ends
another turn. One request produced several pings.

Grok already reports, on every turn-end, whether it still has work outstanding.
Read that instead of trying to classify which turns are "real":

  backgroundTasks absent          -> silent, this is the session-end tail
  StopFailure / StopCancelled     -> announce, a failure is never hidden
  stopHookActive                  -> silent, a Stop hook is keeping it working
  a shell task or subagent running -> silent, the work is not done
  otherwise                       -> announce

Nothing here knows what an auto-wake turn is. A turn that ends with work
outstanding stays quiet; the later turn where that work is finally done is the
one that announces. That is also why this survives the case where Grok completes
a user's goal inside one of those turns — prefix-based suppression would have
silenced it.

Monitors and scheduled entries are deliberately not counted as outstanding work.
They can run indefinitely, so counting them would suppress a user's completion
permanently, and a lost ping is worse than an extra one.

Also registers StopCancelled, which Grok fires instead of Stop on a user
interrupt, a declined permission, --max-turns, or a no-progress bail-out. Orca
never subscribed to it, so those turns were reported as successes.

Also removes a stale notification matcher that searched for prose the shipping
binary never sends; the typed notification kind is matched instead, and neither
idle_prompt nor task_complete is treated as a completion.

Needs-input behaviour (permission prompts and ask_user_question waits) is
unchanged and stays ungated by background work.

* fix(grok): never hide a failed or cancelled turn behind the background-work gate

The announce predicate checked field-absence before terminal outcome. Grok's
StopFailure and StopCancelled payloads carry no background inventory at all, so
the absent-field branch — added so the session-end tail stays silent — fired
first and silenced every failure and every cancellation.

That inverted the rule it was meant to serve. Before this series a cancelled turn
at least surfaced as a (wrong) success; gated this way it surfaced as nothing.

Terminal outcome is now checked first, so a failure or cancellation announces
regardless of what other fields the payload happens to carry.

The existing tests passed straight through the bug because they built failure
payloads with a backgroundTasks field Grok never sends for those events. They now
model the real payload shapes, verified against the provider's payload
definitions and the captured envelopes.

* fix(grok): settle completion from provider lifecycle state

* fix(grok): fence stale turn ends without prompt ids

* feat(native-chat): add provider-aware Fast mode (#20506)

* feat(native-chat): add provider-aware fast mode

* chore: drop unrelated formatter churn from the merge

pnpm format reflowed pnpm-workspace.yaml quoting and a source-scan test
that this PR does not otherwise touch.

* fix(native-chat): review fixes for provider-aware fast mode

Review pass over the Fast mode work.

Claude reads its model catalog once per option write. The admit check, the
effort guard and the Fast guard each took their own `list_models`, so a model
write with Fast on paid two round trips for one list and let two guards answer
from two different catalogs. The guards are now pure over a single read.

Claude no longer refuses a Fast enable when the catalog identified nothing at
all. An empty list is not evidence against a model -- the same rule the model
admit-check already applies -- so a CLI that cannot answer would otherwise have
Fast refused on every model. A catalog that did list the model and stayed silent
about Fast is still not positive evidence and keeps refusing.

Codex refuses a direct `serviceTier` write instead of accepting one the next
turn discards. The turn derives the tier from `fastMode`; the key still restores
so a session persisted before Fast existed migrates.

Both option surfaces return a cached snapshot again. `SessionOptionsSurface` is
read through `useSyncExternalStore`, whose contract is a stable snapshot, and
rebuilding it per call breaks that for any consumer wired that way.

Also records two decisions that were emergent rather than stated: routing
Standard when Fast is on but no tier is named yet, and what a readback
disagreement does and does not prove.

Quality gate: merges the duplicate imports static analysis flagged, adds SAFETY
rationales for two pre-existing casts the changed-code gate now sees, and drops
a new assertion in favour of a checked narrowing.

* fix(native-chat): read Claude Fast state from the session frame

A fresh Claude session reports `fastModeState` while the settings readback still
has no `fastMode` boolean, so the two are not redundant -- the frame answers at a
moment the boolean has none. The picker fell back to "value unknown" and asked
the user to disambiguate what the provider had already reported, and the state it
reported had no reader at all.

Falls back to the frame only when neither a pick nor the settings readback
answers. `cooldown` throttles routing rather than clearing the pick, so it reads
as on; reading it as off would flip a control nobody touched.

Display only. The launch seed is untouched: an unset Fast preference still seeds
nothing, which its own guard continues to pin.

* perf(native-chat): skip the model catalog read when turning Fast off

Turning Fast off needs no support evidence, so the read only cost a
round trip — and restore replays a stored `false` on every acquire.

Also narrows the alias-matcher comment: the effort and admit guards
match on alias and resolved id only, so calling it the sole matcher
overstated it.

* fix(native-chat): clear a Claude Fast block once the child stops reporting it

The child omits fast_mode_disabled_reason entirely when nothing blocks Fast
and never sends a null, so requiring the key back latched the first reason
for the session's life: switching to a model that disallows Fast and back
retired the control for good, leaving a session running Fast with no way to
turn it off. A frame that reports state without a reason is the all-clear.

* test(native-chat): cover the mobile structured option hook

useMobileStructuredAgentOptions gained generation fencing, a pending-write
guard and a post-write options refresh with no test file. Pins the concurrency
contract and the fast mode round trip:

- a superseded options read is dropped instead of overwriting newer state
- an overlapping write is refused and the pending guard is released after
- an accepted same-fence write reads options back and applies the result,
  and a different-fence write does not
- a boolean fastMode pick reaches the wire encoded and is remembered decoded
- no Fast row when session support, catalog support or the model capability
  is missing

Each behaviour was ablated against the production logic to confirm it fails
without it. No production code changed.

* feat(native-chat): render a boolean session option as one toggle

On and Off were two radio rows under a header repeating the option name,
so a binary choice cost three lines and two clicks to read. It is now a
single switch row that owns its label, on desktop and mobile.

An unknown value keeps its caption: a switch cannot say "unset".

* fix(native-chat): resolve a boolean option's display value at the producer

A boolean session option reached the UI in three states while its control had
only two, so the renderer apologised for the gap with a "Current value unknown"
caption beside a switch that had already collapsed to off. For `thinking`, whose
catalog default is on, that caption sat next to a switch asserting the opposite
of what every composed dispatch assumes.

One expression fed both the displayed value and the option's provenance. Split
them: the boolean descriptor now always carries a value, resolved to the same
`values[id] ?? defaultValue` that buildNativeChatSessionOptionCommand already
composes, while `valueSource` is untouched and still records whether anything
confirmed it. `kind.currentValue` is required on the boolean arm so the third
state cannot come back.

The launch path is unaffected: resolveAgentSessionOptionLaunch and
buildNativeChatSessionOptionCommand build the composed `--model` argument from
the caller's picks and the catalog, never from a descriptor.

Both surfaces mark an unconfirmed value instead of captioning it, and the two
reasons stay distinct — `default` says the catalog value is what a launch will
send, `unreported` says nothing has told us anything. Only `unreported` is
reachable in the structured lane, where the agent may be routing a tier we have
never been told about, so the two never share a label.

* fix(native-chat): let assistive tech read the option value marker

The marker was aria-hidden next to an explicit aria-label, so the label
already won the accessible name and hiding it only cost screen reader
users the default-vs-unreported distinction that sighted users get. It is
now referenced by aria-describedby, which keeps the name Fast mode.

Mobile's summary row said "Not set" for a boolean while the sheet behind
it showed the switch on, so the two screens disagreed. A boolean always
has a value; the summary states it and the sheet's marker qualifies it.

* chore(i18n): drop the On/Off option strings the switch row retired

Replacing the On/Off radio pair removed the only call sites for these two
keys. i18next cannot rebuild a key with no call-site default, so leaving
them in the catalogs forced them into the boot bundle as dead weight.
Removing them shrinks it by two entries instead.

* fix(claude): install SessionEnd for capable versions (#20530)

* refactor(mobile): send the source-control domain through typed RpcOperations (#20544)

* test(mobile): record main's source-control RPC behaviour before migrating it

45 scenarios over 11 source-control senders, recorded from main so the step-4
migration has a frozen answer to compare against. Adapters mount the real
exported senders as plain functions, so no React host or device is needed.

The 73 existing goldens change header-only (`baseline`, `recorderSha256`): any
new scenario re-digests the recorder, and the pinned baseline had drifted from
main in `src/shared` so recording required bumping it. Content is byte-identical
on all 73 — verified field-by-field against HEAD.

Scenarios deliberately pin the empty-message cases (`sc-*-refused-empty-message`,
`sc-*-rejected-empty-message`), because a refusal with no message falls back to
the screen's copy while a transport error with no message does not, and the two
paths are easy to collapse when a call site moves behind an acceptance policy.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb

* refactor(mobile): send the source-control domain through typed RpcOperations

13 of the domain's 14 files now send through a declared operation instead of the
raw request port: 44 references to 0. The holdout is use-mobile-git-requests.ts,
whose single reference is a `(method: string, params)` dispatcher that five other
hooks feed `{ method, params }` action steps at runtime; typing it is a step model
change, not a call-site move, so its line stays at 1.

Fifteen operations over fourteen methods. Two of them read git.status, and that is
deliberate: the Changes screen publishes the host payload verbatim while
hosted-review preparation reads the normalized projection, which returns null when
`entries` is not an array and drops entries missing a path. Sharing the projecting
reader would change what the Changes list renders, so both are named.

Four loads still read the refusal envelope before interpreting, through
readMobileGitRefusal: two degrade to a capability-missing screen, one retries a
selector that is not visible yet, and one falls back from files.openDiff to
files.open. `isMobileGitUnavailable` consults …
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants