Repository navigation
fix(native-chat): /clear starts nothing; the new chat's first message starts its agent - #23935
Merged
Merged
Conversation
A journal write and its delivery to open readers were two calls, and some writers made only the first. A failed start whose lease could not be handed back, a provider revision with no frame behind it, and eviction's settlement were all journaled without reaching an open chat. A journal handle now reports every durable change, and the host's session map binds that report to the session's readers when the handle is set. Writers no longer publish what they append; the per-writer publish calls are deleted.
…dle enters only through the map
…ackfill cannot supply its rewrite The seeded record had no surface tab id, so the next open backfilled one and that rewrite alone made the no-op transaction write. The test passed with the legacy-lease rewrite signal removed.
The handoff removal deleted it alongside the terminal-owner tests, but it covers the surfaced-PTY block that still guards resume, including an agent whose ownership is unknown.
Each commit now delivers itself, so the publish a provider frame still sends afterwards found every reader caught up but still read rows and rebuilt the timeline for each one. A caught-up reader now skips the read.
# Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
A send to a chat with no running agent restarted the agent inside the send
call, before the message was recorded, so the client waited for the whole
start and a failed restart refused the message. Claude held prompts sent
during startup, and those could settle as "unconfirmed".
A send is now accepted inside the session's serialized queue: one ledger row
and one submission row marked handoverRecorded, published, answered pending.
A per-session delivery loop exists while a message is queued. It starts the
agent through the same serialized attach a hold uses, waits outside the queue
for a Claude child to prove its start, and hands the oldest queued message
over as its own serialized step, writing dispatch{pending} before the adapter
call. A start it needed and did not get writes one error-tone row and rejects
every queued message with the same words; a start Stop cancelled writes none.
Settlement follows from the rows. A queued message is provably unwritten, so a
close, an eviction or an exit rejects it. A handed-over message stays in doubt.
A queued row at or below the sequence a handle found when it opened was left
by an earlier process and is rejected at open, with no latch. Stop withdraws
queued messages with no writer lease and no fence. An attach failure keeps the
conversation open, and the attach adopts its journal. Owed work counts the
loop and queued rows.
A compaction or rewind found prepared when a conversation opens was started
under a child this process no longer has, so the open settles it rather than
leaving it to refuse every send until a view attaches. The open cursor is
scoped to its epoch, because sequences restart when an epoch is replaced.
Deleted: restart-before-admission, recordFailedRestart, the fence rebase,
Claude's startup gate, the attach's forget on failure and its own crash
boundary. Clients without agent-session.accepted-send.v1 get their reply held
until the handover; the desktop and paired desktop lists advertise it.
A child that proved its start and then exited before its message was handed over left the message queued: the exit settlement returned early when nothing else was in flight. Delivery then started another child for it, and a child that died the same way started another, without end and without a row. A retried settlement for an earlier generation, run by the attach that delivery started, did the opposite: with that generation's turn unfinished it rejected the message queued for the child being attached. The settlement now takes the rejection for queued messages from its caller. The unexpected exit and the eviction pass one, and it applies even with no other work in flight; the retry for an earlier generation passes none.
…tion open The attach now writes into the conversation's own open journal, but a failed transcript import still closed it as if it were the attach's provisional one. The conversation stayed indexed with a closed journal, so every later send answered "could not be recorded" and every attach failed again until the app restarted. The import now closes only a journal the attach opened for itself.
Every conversation open now goes through the recovering open, including the read restore of every chat at startup, which used to replay its journal once. The recovering open replayed it twice: once to probe it and again inside the open. The probe is now handed to the open as its load.
…s no child behind A failed attach now keeps the conversation open, but a failure after `onAttached` indexed the child (the rewind or compaction recovery, or the attach's own success record) left that entry claiming a child the failure path had already released. The next send found the phantom, skipped the start, and wrote at a fence the journal had moved past, so the message stayed queued for good. The entry now drops the released child and its event sink, and follows the record's fence, as a failure before indexing already did.
… outlasts the send's answer The error strip for a message the host accepted and then did not deliver matched the entry before the outbox reconciled, so a Stop's withdrawal, which the reconcile drops, showed "Orca could not send your message" with nothing to retry. It now reads the reconciled entry. A rejection the journal records before the send's own pending answer lands is final as well: that answer no longer puts the entry back to dispatching with no Retry.
…mble wait is left unknown, not torn down The preamble waits for its submission to be delivered while the worker's agent starts. When that wait ran out it threw operation_unknown, and the failed-start teardown then closed the session, which rejected the very preamble the host was about to deliver. It now reports a turn start nobody observed yet: the worker is start-unknown with its session kept, the host delivers the preamble when the agent starts, and the worker's report settles the dispatch as for any unobserved start. The receipt no longer suggests reading a screen a structured worker lacks.
…as not sent A remount reads an entry it left dispatching as unconfirmed. When the journal had rejected it meanwhile, as a failed start or a quit now does, the reconcile left it unconfirmed: it blocked every later message behind a Retry and no reason, and the delivery probe, seeing the journal already answered, never ran. The reconcile now settles it as rejected like a dispatching one.
…start is not resent
…box comments Three places still gave the removed fence check as a reason: the launch replay said admission puts the ledger ahead of the fence, the launch surface said a send must name the lease it was admitted against, and the direct-mailbox path said the lease fence decides whether delivery is safe. Admission now checks only the writer lease.
A conversation now outlives any number of provider children, so the child is one record on the conversation's entry instead of five loose fields beside its journal. It is written in one place: indexed only once an attach has fully succeeded, and ended through one function that an exit, a failed re-attach, a Stop and an eviction all share, matched on the child's generation and fence. - A failed attach writes no child, so there is nothing to unwind: the field unwind and the fence patch after it are gone. - Conversation writes read the record's fence, the way mutation admission already does; a child's own writes use its fence. The four stored-fence patches, and the settlement retry's overwrite of the conversation's fence, are gone. - The owed wind-down is its own tombstone, carrying the child it is owed for, and is no longer dropped when an attach replaced the whole entry. - Stop on a child still proving its start stops only the child: its lease goes back and the chat is told it is idle, but the journal, the holders and the readers stay. Close is that stop plus the conversation's close. - The settlement retry uses the conversation's own journal, opened through the host's one open.
…tes-by-target # Conflicts: # src/main/native-chat/agent-session-wire/structured-agent-session-host-types.ts # src/main/runtime/agent-session-record-store-legacy-handoff-lease.test.ts # src/renderer/src/lib/worktree-agent-activation-gate.test.ts
… or child failed A queued message was settled by whichever path happened to end the child first: the loop, the unexpected exit, eviction's work settlement, the open's leftover rule, and the startup branch that rejected every pending row. That gave two failure rows with different tones for one start, a loop that could hand over to a different child than the one it waited on, and a Claude start that died while starting reading unlike every other failed start. - The loop remembers the child it waited on. At handover, if that child is gone or replaced, it reads how it ended: a Stop continues; anything else writes one failure row and rejects every queued message with the same words, then stops. A child still starting whose start the adapter says did not land fails the same way. The exit, eviction and the settlement retry only settle the handed-over and legacy rows of the child that ended. - One failure row, always an error, keyed by the start. A start a view began that dies with nothing queued writes the same row through the same builder, so a second report revises it. - The open no longer rejects leftovers; the loop's first step does, and the open wakes it. - `awaitStarted` answers why a start did not land, so the row says it even when the loop sees the failure before the exit is processed. - Quit closes every conversation the way closing a chat does: what is still queued is rejected as closed, with or without a child, and a start the loop already has in flight is waited for so the child it produces is stopped rather than left behind.
… stop The eviction step reads a stop's result through `stopAgentSessionProviderRoot` and hands that verdict to the child's ending, so the host never forms a second view of whether the root is gone. Every ending carries it: a stop's comes from that reading, an exit's root is gone by definition, and a failed re-attach passes what its release saw. The end-of-child record can therefore also carry a stop whose root was not seen to go, which nothing ends on yet.
…as it The host now lists agent-session.accepted-send.v1 among its own runtime capabilities, the same string capable clients already send. A client can then tell a host that answers a send at acceptance, and admits a Stop with no writer before a turn starts, from an older one that still restarts the agent inside the send. Additive: an older client ignores a capability it does not know.
… and /compact in its menu Once the host sends a `/` list for a chat, the menu shows that list instead of its own. The at-rest list started from Claude's text-driven commands, which is empty, so a Claude chat whose Claude was not running lost /model, /effort, /clear and /compact from its menu. It now starts from exactly the menu a chat at rest showed before, then adds what the folders hold. A scan that never answers also held the next one off for good, freezing the menu until relaunch; one unanswered for 30 seconds is now given up on and the next read scans again.
…r piles up Giving up on a scan after 30 seconds threw away every scan that took longer than that, so a slow folder never updated the menu, and a folder that stayed hung started another stuck walk every 30 seconds, each holding one of Node's few file-system threads. Each scan is now numbered and its answer is kept whenever it is newer than the one already kept, however long it took; an older answer that lands after a newer one changes nothing. A new scan starts beside an overdue one, but never more than two run at once.
…er answer A failed scan marked itself as the newest answer, so an older scan that answered after it was ignored and the menu stayed without custom commands until the next scan. A failure now only starts the 10-second wait. Test pins that the wait runs from when a scan lands, a failed one included.
…rst-send-mainmerge
3 tasks done
7 of 9 tasks
Contributor
Author
|
#23674, which this PR is ordered after, is ready to merge at |
…rst-send-mainmerge # Conflicts: # src/main/claude/claude-structured-session-adapter.ts
…e file AgentSessionRecordStore.open hardened permissions, loaded the file, marked every lease unreconciled, built the transaction queue and persisted a pending rewrite. That is the queue's load lifecycle, and the queue already applies the same unreconciled rule when it reloads an externally changed file. Move it to AgentSessionStoreTransactionQueue.open beside fromLoadedStore, and drop the exported wrapper that existed only for the store's open. No behavior change; the store's public API is unchanged. Brings the record store back under max-lines after commitConversationClear.
…arts-on-first-send
…member The at-rest `/` surface's read and change listener travel together as `atRestCommands`, which the Claude catalog already is, so the adapter types stay within their line limit after main's growth.
brennanb2025
marked this pull request as ready for review
September 30, 2026 22:54
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
|
||
| async function describe(filePath: string): Promise<string | undefined> { | ||
| try { | ||
| const markdown = await readFile(filePath, 'utf8') |
There was a problem hiding this comment.
When an at-rest Claude chat scans a workspace containing a large .claude/commands/*.md file, readFile loads the entire file before applying the 256 KiB limit. Opening the chat can therefore allocate memory proportional to the file’s size and potentially exhaust the host process. Bound the read itself.
How this was verified: An at-rest menu read reaches the workspace command scanner, which reads each Markdown file in full before truncating its contents.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 6522a79e-3f5d-4e3d-b3cd-f38efc45c6d5
📒 Files selected for processing (23)
src/main/claude/claude-at-rest-commands.tssrc/main/claude/claude-command-folder-scan.tssrc/main/claude/claude-structured-session-adapter.tssrc/main/claude/claude-structured-session-state.tssrc/main/codex/codex-provider-retry-idle-sweep.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter-router.tssrc/main/native-chat/agent-session-wire/structured-agent-session-adapter.tssrc/main/native-chat/agent-session-wire/structured-agent-session-at-rest-commands.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-at-rest-commands.tssrc/main/native-chat/agent-session-wire/structured-agent-session-client-delivery.tssrc/main/native-chat/agent-session-wire/structured-agent-session-delivery-loop.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.tssrc/main/native-chat/agent-session-wire/structured-agent-session-rewind-at-rest.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-subscribers.tssrc/main/native-chat/agent-session-wire/structured-conversation-command.test.tssrc/main/native-chat/agent-session-wire/structured-conversation-command.tssrc/main/runtime/agent-session-conversation-clear-commit.test.tssrc/main/runtime/agent-session-record-store.tssrc/main/runtime/rpc/methods/structured-chat-tab-table.test.tssrc/main/runtime/structured-agent-session-startup-reconcile.test.tssrc/main/runtime/structured-chat-coordinator-mail.test.tssrc/main/runtime/structured-claude-runtime-adapter.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
Comment on lines
+54
to
+59
| async function describe(filePath: string): Promise<string | undefined> { | ||
| try { | ||
| const markdown = await readFile(filePath, 'utf8') | ||
| return ( | ||
| summarizeSkillMarkdown(markdown.slice(0, MAX_COMMAND_FILE_BYTES)).description ?? undefined | ||
| ) |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Limit the read before parsing the command file.
describe calls readFile on the whole file. It truncates the file only after the read. MAX_COMMAND_FILE_BYTES therefore does not limit memory or I/O. For example, a large .md file under .claude/commands in a cloned repository is read completely on every scan, and a scan can repeat every 10 s. slice also counts UTF-16 code units, not bytes. Open the file and read at most MAX_COMMAND_FILE_BYTES bytes.
Proposed fix
-import { opendir, readFile } from 'node:fs/promises'
+import { open, opendir } from 'node:fs/promises'
@@
async function describe(filePath: string): Promise<string | undefined> {
+ const handle = await open(filePath, 'r').catch(() => null)
+ if (!handle) {
+ return undefined
+ }
try {
- const markdown = await readFile(filePath, 'utf8')
- return (
- summarizeSkillMarkdown(markdown.slice(0, MAX_COMMAND_FILE_BYTES)).description ?? undefined
- )
+ const buffer = Buffer.alloc(MAX_COMMAND_FILE_BYTES)
+ const { bytesRead } = await handle.read(buffer, 0, MAX_COMMAND_FILE_BYTES, 0)
+ return summarizeSkillMarkdown(buffer.subarray(0, bytesRead).toString('utf8')).description ?? undefined
} catch {
return undefined
+ } finally {
+ await handle.close()
}
}
Comment on lines
+104
to
+124
| // Stopped before the marker, so nothing the old agent does can land after the clear. The | ||
| // stop releases the lease, which moves its fence: the marker is written at the new one. | ||
| await context.stopAgent(sessionId) | ||
| const fence = store.getRecord(sessionId)!.lease.runtimeFence | ||
| const completed = { | ||
| command, | ||
| runtimeFence: ctx.fence, | ||
| runtimeFence: fence, | ||
| operationId: clientOperationId, | ||
| callerKey: caller.callerKey, | ||
| ...(replacementSessionId ? { replacementSessionId } : {}) | ||
| } | ||
| if (ids) { | ||
| const attach: AgentSessionAttachParams = { | ||
| envelope: { | ||
| sessionId: ids.sessionId, | ||
| clientOperationId: ids.attachOperationId, | ||
| expectedRuntimeFence: null, | ||
| payloadFingerprint: '' | ||
| }, | ||
| location: record.location, | ||
| accountHome: record.accountHome, | ||
| provider: record.provider, | ||
| agent: record.provider, | ||
| runtimeKind: 'native', | ||
| launchArgs: record.launchArgs, | ||
| // The options the user chose, which any restart of this chat would replay too. | ||
| options: record.options | ||
| } | ||
| attach.envelope.payloadFingerprint = computeAgentSessionPayloadFingerprint({ | ||
| method: 'agentSession.attach', | ||
| sessionId: ids.sessionId, | ||
| fields: attachFingerprintFields(attach) | ||
| }) | ||
| const acquired = await host.attach(caller, attach) | ||
| const replacement = store.getRecord(ids.sessionId) | ||
| // An earlier try started this replacement and a restart or the idle sweep has since | ||
| // stopped it: attach can't replay a settled start, but the new conversation is at rest. | ||
| const startedAndAtRest = | ||
| replacement !== null && | ||
| agentSessionLeaseIsReleased(replacement.lease) && | ||
| store.getOperationRow(caller.callerKey, attach.envelope.clientOperationId)?.outcome | ||
| .status === 'succeeded' | ||
| if (!acquired.ok && !startedAndAtRest) { | ||
| if ( | ||
| !isDefinitiveAgentSessionCreateRefusal(acquired.refusal.code) && | ||
| replacement?.lease.claimStatus !== 'released' | ||
| ) { | ||
| throw new Error(acquired.refusal.message) | ||
| } | ||
| // The refusal's message is Orca's log text; the result keeps its situation instead. | ||
| const failed = { | ||
| ...base, | ||
| replacementSessionId: undefined, | ||
| phase: 'committed' as const, | ||
| state: 'completed' as const, | ||
| ...conversationCommandFailure( | ||
| structuredAgentSessionStartFailureFact({ | ||
| refusal: acquired.refusal, | ||
| newSession: true | ||
| }), | ||
| { ...structuredAgentSessionFailureWordsContext(record), command: 'clear' } | ||
| ) | ||
| } | ||
| await store.setConversationCommand(sessionId, ctx.fence, failed) | ||
| return { ok: true, value: failed } | ||
| } | ||
| // Carry the source's drafts to the replacement, the same for every client version: | ||
| // the cards stay visible where the user now is, and no text rides the wire. | ||
| // Bookkeeping — a failure is reported and never fails the clear. | ||
| await carryQueuedMessagesToClearReplacement(ctx, { | ||
| replacementSessionId: ids.sessionId, | ||
| replacementJournal: context.sessions.get(ids.sessionId)?.journal, | ||
| callerKey: caller.callerKey, | ||
| operationId: clientOperationId | ||
| }) | ||
| } | ||
| const completed = { | ||
| ...base, | ||
| // Only has to be new: the marker is what points at it, and a same-id resend replays it. | ||
| replacementSessionId: `clear-${randomBytes(20).toString('hex')}`, | ||
| phase: 'committed' as const, | ||
| state: 'completed' as const | ||
| } | ||
| await store.setConversationCommand(sessionId, ctx.fence, completed) | ||
| await store.commitConversationClear({ | ||
| sessionId, | ||
| fence, | ||
| command: completed, | ||
| claimKeyId: context.deps.claimKeyId, | ||
| now: context.now() | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -nP -C4 'runtimeFence\s*(\+|=)' src/main/runtime src/main/native-chat | head -80
rg -nP -C3 'stopAgent\s*[:=(]' src/main/native-chat/agent-session-wireRepository: stablyai/orca
Length of output: 17928
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- command implementation ---'
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-conversation-command.ts
printf '%s\n' '--- host serialization and stop binding ---'
sed -n '230,310p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
sed -n '1,125p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts
printf '%s\n' '--- stop implementation and serializer ---'
rg -n -C5 'function stopStructuredAgentSessionAgentUnderSerialize|stopStructuredAgentSessionAgentUnderSerialize|serialize\s*[:=]|serialize\(' src/main/native-chat/agent-session-wire src/main/runtime
printf '%s\n' '--- commit fence check ---'
rg -n -C8 'commitConversationClear|commitConversationClearRecord|agent_session_checkpoint_stale|runtimeFence ===' src/main
printf '%s\n' '--- runtimeFence mutations in relevant source ---'
rg -n -C5 'runtimeFence\s*(=|\+=|\+\+)|nextAgentSessionFence|minimumNextFence' src/main/runtime src/main/native-chat --glob '!**/*.test.ts' | head -240Repository: stablyai/orca
Length of output: 42432
🏁 Script executed:
#!/bin/bash
set -e
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-conversation-command.ts
printf '\n--- host/lifetime ---\n'
sed -n '250,300p' src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
sed -n '1,100p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts
printf '\n--- direct definitions ---\n'
rg -n -C8 'stopStructuredAgentSessionAgentUnderSerialize|serialize\s*[:=]|serialize\(' src/main/native-chat/agent-session-wire
printf '\n--- clear commit ---\n'
rg -n -C10 'commitConversationClear|commitConversationClearRecord|agent_session_checkpoint_stale|runtimeFence ===' src/main
printf '\n--- fence writers ---\n'
rg -n -C6 'runtimeFence\s*(=|\+=|\+\+)|nextAgentSessionFence|minimumNextFence' src/main/runtime src/main/native-chat --glob '!**/*.test.ts' | head -260Repository: stablyai/orca
Length of output: 45661
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- symbols and fence consumers ---'
rg -n -C12 'stopStructuredAgentSessionAgentUnderSerialize|commitConversationClear|commitConversationClearRecord|agent_session_checkpoint_stale|runtimeFence' src/main/native-chat/agent-session-wire src/main/runtime --glob '!**/*.test.ts' | head -500
printf '%s\n' '--- serializer declarations/usages ---'
rg -n -C8 'serialize' src/main/native-chat/agent-session-wire --glob '*.ts' | head -320Repository: stablyai/orca
Length of output: 41838
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- existing full inspection log ---'
cat /tmp/coderabbit-shell-logs/shell-output-jzhLWI
printf '%s\n' '--- direct definitions from discovered files ---'
rg -l 'function stopStructuredAgentSessionAgentUnderSerialize|const stopStructuredAgentSessionAgentUnderSerialize|commitConversationClearRecord|commitConversationClear' src/main | xargs -r -n1 sh -c 'echo "--- $0"; rg -n -C12 "stopStructuredAgentSessionAgentUnderSerialize|commitConversationClearRecord|commitConversationClear" "$0"'Repository: stablyai/orca
Length of output: 33022
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- per-session task serializer ---'
rg -n -C12 'class .*Task|serialize\(.*sessionId|serialize<|tasks\.serialize|create.*Tasks' src/main --glob '*.ts' --glob '!**/*.test.ts' | head -260
printf '%s\n' '--- lease renewal implementation ---'
rg -n -C18 'renewLeases|function renew.*Lease|renew.*Lease' src/main/runtime --glob '*.ts' --glob '!**/*.test.ts' | head -320
printf '%s\n' '--- restart reconcile implementation/callers ---'
rg -n -C18 'reconcileRestartLeases|restoreStructuredAgentSessionsOnRestart|restart.*reconcil|reconcile.*Lease' src/main --glob '*.ts' --glob '!**/*.test.ts' | head -420
printf '%s\n' '--- clear commit fence assertion ---'
sed -n '35,95p' src/main/runtime/agent-session-conversation-command-record.tsRepository: stablyai/orca
Length of output: 42893
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- restart reconciler ---'
sed -n '1,260p' src/main/native-chat/agent-session-wire/structured-agent-session-restart-reconcile.ts
printf '%s\n' '--- restart restore ---'
sed -n '1,90p' src/main/native-chat/agent-session-wire/structured-agent-session-restart-restore.ts
printf '%s\n' '--- host restore wiring ---'
rg -n -C14 'createStructuredAgentSessionHostRestore|reconcileLeases:|reconcileRestartLeases' src/main/native-chat/agent-session-wire src/main/runtime --glob '*.ts' --glob '!**/*.test.ts'
printf '%s\n' '--- recovery release transition ---'
sed -n '40,115p' src/main/native-chat/agent-session-wire/structured-agent-session-recovery-resolution.ts
rg -n -C12 'releaseUnprovenAgentSessionOwner|transitionHandoff' src/main/runtime src/main/native-chat/agent-session-wire --glob '*.ts' --glob '!**/*.test.ts' | head -220Repository: stablyai/orca
Length of output: 42032
Serialize restart lease reconciliation with /clear.
restoreOneStructuredAgentSessionRead reconciles leases before it acquires the session serializer. A restart reconciliation can release an owner and advance runtimeFence. If that happens after /clear reads the fence, commitConversationClearRecord can reject the clear with agent_session_checkpoint_stale.
Idle-sweep operations already run inside the serializer, and lease renewal does not advance runtimeFence. Keep restart reconciliation under the same serialization boundary, or retry /clear automatically after a stale-fence refusal.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Important
The /clear redesign traces correct — stop-before-commit cannot write a stale-fence marker, removing the other-window guard is safe, and a cleared chat's failed first start lands on the message with Retry and no /clear wording. One resilience gap in the new at-rest / catalog is worth closing before merge: if two folder scans hang, that menu freezes indefinitely, which contradicts the stated "given up on after 30 seconds" behavior. Details inline.
Reviewed changes
/clearis one store write that starts nothing.structured-conversation-command.tsnow stops the source's agent, mints a randomclear-<40hex>replacement id, and commits the replacement record + "cleared, continued in X" marker + tab move in one transaction viastore.commitConversationClear. The old operation-log scan, derived ids, other-window refusal, start-and-wait block, and/clearfailure wording are deleted.- The replacement's agent starts on its first message. The new record (
agent-session-record-founding.ts) carries an emptyproviderHandleChain,claimStatus: 'released',runtimeFence: 1, and copied location/provider/account/options/launchArgs; the existing send path starts it fresh. - Failed first start is worded
startFailedon a never-started record.structured-agent-session-delivery-loop.tsderivesnewSessionfromproviderHandleChain.length === 0, so a cleared chat's failed first start reads "couldn't start", not "couldn't restart". - At-rest
/surface for a Claude chat whose agent isn't running. Newclaude-at-rest-commands.tscatalog (10s TTL, 30s overdue, max 2 in flight) andclaude-command-folder-scan.ts, exposed as a newatRestCommandsadapter member, fanned out by the router, withclientDelivery.readCommandsfalling back to it andAgentSessionSubscribers.republishCommands()pushing changes to open panes. - Queued drafts carry after the commit.
carryQueuedMessagesToClearReplacementnow opens the replacement journal lazily only when there are drafts, under its own lock and after the single store write.
ℹ️ Nitpicks
structured-conversation-command.ts:104-105— the comment says "the stop releases the lease, which moves its fence." That holds only when the source had a running child or an owed wind-down; for an already-at-rest sourcestopAgentno-ops and the fence does not move (the marker is simply written at the existing fence). Consider softening to "moves its fence when the source had an agent to release."claude-command-folder-scan.ts:54-59—MAX_COMMAND_FILE_BYTESis applied afterawait readFile(filePath, 'utf8'), so it bounds the parsed surface, not memory: a huge.mdis fully resident first. A streamed/open+readbounded read would make the cap real.structured-agent-session-client-delivery.ts:61-65—watchAtRestCommandsoverwritesstopAtRestCommandUpdateswithout calling the previous value. There is exactly one construction and call site today, so it cannot leak, but calling it twice would orphan the first listener.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| const due = | ||
| entry.inFlight === 0 | ||
| ? now - entry.scannedAt >= CLAUDE_AT_REST_COMMANDS_TTL_MS | ||
| : entry.inFlight < MAX_SCANS_IN_FLIGHT && now - entry.lastStartedAt >= SCAN_OVERDUE_MS |
Contributor
There was a problem hiding this comment.
If two scans hang, this key is frozen forever. inFlight is decremented only in .finally, which runs when scan() settles, and scan() has no timeout — so once both slots are occupied inFlight < MAX_SCANS_IN_FLIGHT is permanently false, due never becomes true again, and the / surface stays at its last value until relaunch. This contradicts the comment above ("a hung folder can't freeze the menu") and the PR body's "a scan that has not answered in 30 seconds is given up on".
Technical details
# Two hung at-rest scans permanently freeze one workspace/account key
## Affected sites
- `src/main/claude/claude-at-rest-commands.ts:63-69` — the `due` predicate: `entry.inFlight === 0 ? TTL : entry.inFlight < MAX_SCANS_IN_FLIGHT && ...`
- `src/main/claude/claude-at-rest-commands.ts:90-109` — `scan()` is un-raced (`resolveWorkspacePath`, `discoverSkills`, `opendir`, `readFile`) and `inFlight` is only released in `.finally`
## Required outcome
- A scan that never settles must not permanently hold its in-flight slot: after `SCAN_OVERDUE_MS` a hung scan should be abandoned (or the overdue branch should ignore it) so a later read can still start a fresh scan and refresh the list.
## Suggested approach (optional)
- Race `scan(record)` against a timeout that rejects after `SCAN_OVERDUE_MS`, or track each in-flight scan's start time and let the overdue branch start a new scan regardless of how many are hung (bounded by a hard cap on concurrent scans).
- Either way, make the module comment at lines 11-12 match the real behavior.
brennanb2025
added a commit
that referenced
this pull request
Oct 1, 2026
# Conflicts: # src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts
Closed
7 of 9 tasks
brennanb2025
added a commit
that referenced
this pull request
Oct 1, 2026
7 of 9 tasks
7 of 9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Lands after #23674, which accepts a conversation command's own reply after the stop moves the fence.
ELI5
Typing
/clearin a structured chat used to start a brand-new agent right away, before the chat actually switched over. If that start failed,/clearfailed with "Codex couldn't start. Run /clear again." and the chat stayed where it was. Now/clearonly records "this chat continues in a new, empty conversation" and switches the tab. The new conversation's agent starts when you send your first message there, and if that start fails you see why on that message, with a Retry button.What Changed
The problem the user saw, and why.
/clearin a structured chat (Claude or Codex) did two things in this order: it started the new conversation's agent, and only then recorded that the chat had been cleared and moved the tab. Because the start came first:/clearcould fail on the agent start alone, for example "Codex is not signed in for the selected account. Sign in, then run /clear again." or "Codex couldn't start. Run /clear again.", leaving the old chat in place./clearwaited for the agent to start before the tab switched./clearsafe, but it needed a lot of machinery to do it: scanning the operation log to find which earlier try to finish, a second id derived from the operation id to recognise that try's start, a check for a start that had already finished, and a rule that refused another window's/clearwhile a leftover agent from an unfinished try was still running.What is different for the user.
/clearis instant and cannot fail because an agent failed to start. The tab switches to an empty conversation with the composer ready./clear. Retry sends the same message again and starts the agent./clearon a chat whose agent is not running no longer starts that old agent first just to clear it. The one exception is a Codex chat whose rewind was cut off (a quit or crash mid-rewind): only its agent can settle that rewind, so/clearstarts it first, exactly as sending a message or/compactalready does. Before this, such a/clearwould have been refused every time./menu keeps your skills and custom commands in the new conversation before its first message. Claude reports that list only while it runs, so without this the menu would have dropped to Orca's built-in commands until the first message. For a Claude chat whose Claude is not running, the host now reads the list where Claude reads it, on the machine that runs the chat, without starting anything: the project's and the account's.claude/commandsfolders, the Claude skills Orca's skill discovery already finds there, plus/model,/effort,/clearand/compact, which the menu always offered. This also fills the menu of a chat stopped after 30 idle minutes, and after a relaunch. A running Claude's own list still wins. A file you add shows up within about 10 seconds, the next time anything reads the menu./clearis recorded rather than before.The mechanism.
/clearfirst stops the old conversation's agent, if one is running (the same stop the 30-minute idle sweep makes), and only then writes, so nothing the old agent does lands after the clear. If the write then fails, the chat is left usable: its next message starts the agent again./clearis now one write to the session store. In a single transaction it creates the new conversation's record, writes the "cleared, continued in X" marker on the old record, and moves the tab. Either all of it lands, or none of it does.--resume, Codex with a new thread) instead of resuming the old one./clear. The host mints it once and stores it; every reader (other windows, the phone, session lists, read state, tab restore, orchestration) gets it from the stored marker or the tab move, and none ever computed it. The tab id, which host-shared read state is keyed on, does not change across/clear./list is read by the Claude adapter (claude-at-rest-commands.ts,claude-command-folder-scan.ts): every*.mdbelow each commands folder, named by its path with:between folders (frontend/test.mdis/frontend:test), symlinks not followed, at most 24 levels and 1,000 entries, unreadable folders skipped. One scan answers for a workspace and account for 10 seconds, a scan that finds something new is pushed to the panes showing those chats, and a scan that has not answered in 30 seconds is given up on. It answers only for chats this machine runs (structured Claude runs only on the local host, never over SSH or WSL), so no chat is ever shown another machine's folders./clear, and the "... Run /clear again." failure it recorded.structured-conversation-command.tsgoes from 280 lines to about 125.Why this over the alternatives.
/clearand keep hardening the retry: that is what fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524 did, and each fix added another rule about a started-but-unrecorded conversation. The problem is the ordering, so every such rule treats a symptom./clear: no orphans, but/clearwould still wait on a start, and a failed start after the switch would need its own "cleared, but the agent didn't start" state./clear(this PR): there is no window between a start and a record, so nothing can be orphaned and nothing needs retry logic. The comparable desktop agent apps we checked do the same: clearing writes one marker and starts nothing, and the agent is started when the next message is sent.Differences from the common pattern
This PR moves /clear to the common pattern: clearing starts nothing, the first message afterwards starts the agent, a start failure is recorded against that message, the old agent is stopped before the clear is recorded, and a resting chat's
/menu is read from the agent's command and skill folders without starting it. Like the common approach, a /clear while a turn is running is refused rather than interrupting it; that refusal is unchanged here.Mixed versions
/clearresult never carries an error (the field was always optional); the new conversation is published at rest instead of starting (the same state Orca's idle stop produces today); and a start failure arrives as a failure row plus a rejected message, which clients have rendered since fix(native-chat): turn facts come from the turn record, and /compact is a message the chat sends #23059/fix(native-chat): the host writes chat failures for a person, with a typed fact beside them #23116 (older clients show the plain reason)./clearcommits instantly, a resend under the same id replays the committed answer, and its first message starts the agent on the host./clearfrom before fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524 stays inert as fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524 made it; a committed failed/clearblocks nothing; a started-but-unrecorded conversation from fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524's era stays inert (no tab, nothing points at it) and the startup lease check settles it; fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524's leftover operation-log rows are read by nothing and age out after 24 hours./clearresult and tab swap, and its first message carries a valid counter value./list is read on that host too; older clients simply receive a list where they used to get none, which they already render. Folder workspaces and git worktrees are treated alike; nothing here reads git.Deferred items
/clears), the "recovering" lease on such an orphan that only a same-operation retry could settle (nothing retries now; the startup lease check handles it), a same-operation retry after a crash mid-start recording "couldn't start", the retry-scan and derived-id questions raised in review of fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524, and the empty-composer "Send your message to try again" after a cleared chat's new agent died while starting with no message waiting (a cleared chat now starts only for a message, so its failure lands on that message)./clearwrite and the carry (before this PR the carry ran first, and a crash re-ran the whole/clear); the fix would be to notice on the next open that a carry is still owed./menu: a command file added on disk appears the next time anything reads the chat (switching to it, a new message), not when the/menu is opened, because the menu does not ask the host; making the menu ask on open is a renderer change. Also, commands from Claude plugins are not listed (the menu never listed them at rest before either), a command file'sargument-hintis not shown, and plugin skills come from the default~/.clauderather than a non-default account's folder (that is how Orca's existing skill discovery reads them)./clearfailure before this PR. It shares its cause with the create-path wording below and will be fixed with it.Linked Issue
Follow-up to #23524 (merged). This PR now targets
main.Visual Proof
Live Electron QA on a separate Mac, with a scripted stand-in for Claude (no real Claude or Codex). First run at 9199f97; the
/menu and running-agent/clearshots further down come from a second run after the later changes./clearis instant (154 ms from Enter to this) and starts nothing: an empty conversation, composer ready, no "starting" indicator, and no Claude process running.The first message starts Claude (a new process appears only now) and is answered.
A first start that dies keeps the message, marked not sent, with Retry. Nothing mentions
/clear. The red row's wording is the existing start-failure text (see Deferred items).Retry starts Claude and the message is answered.
Quitting and relaunching between
/clearand the first message restores the tab to the empty conversation, and the first message then works.A second live run, on a scripted Claude that reports NO list of its own, so everything below comes from the folders on disk (at 7251f4e; the later commits only change how an at-rest scan retries after a failure, and are unit-tested). Right after
/clear, with no Claude running, the/menu shows/model,/effort,/clear,/compact, the project's command/qa-cmd, and, further down, the account's command/qa-user-cmdand the project's skill/qa-skill.After quitting and relaunching, the same menu, with nothing remembered from before.
A command file added on disk (
frontend/qa-new2.md) is listed as/frontend:qa-new2once the chat is read again (here, after switching tabs away and back; see Deferred items)./clearwhile Claude is running: the old agent was stopped 83 ms after Enter, the empty conversation showed at about 0.5 s, and no notice or console error appeared. The first message afterwards was answered.Also passed: two
/clears in a row start nothing and the next message makes exactly one start; a model picked before the first message is saved and applied at the first start (the scripted Claude does not log which model it received, so that last hop is unverified). Mobile was not run: no emulator route there avoids a live Orca. This change is host-side only and changes nothing in the mobile app.Testing
structured-conversation-command.test.ts:/clearstarts no agent for the old or new conversation and founds the new record at rest with everything copied; a failed store write leaves nothing behind and the old chat usable; a same-id resend replays; a chain of two clears starts neither; the first message starts a fresh conversation and delivers; a start failure (not signed in, and an unclassified one) shows one row without "/clear", keeps the message as not sent, and Retry starts it; a restart between/clearand the first message; an option picked before the first message is used by the first start; an older build's orphan under the same operation id does not collide; an older build's unfinished/clearstill blocks nothing.structured-chat-tab-table.test.ts: after a/clearfrom one window, another window and a phone both list the new conversation the stored record names, under the same tab, and both session lists show nothing unread for it until its first message runs. Making any reader recompute the id instead of reading the stored one turns it red.agent-session-conversation-clear-commit.test.ts: the one write lands record, marker and tab move together on disk, a refused write lands none of them, and an existing record is never overwritten.Real Claude adapter with a scripted CLI: the cleared chat's first message launches Claude under its own id with no
--resumeand replays the saved model.Real Codex adapter with a fake app-server: the successor's first message calls
thread/start, notthread/resume; orchestration mail waiting on a cleared coordinator reaches the new conversation without the user sending anything.structured-agent-session-at-rest-commands.test.ts, with real folders on disk and a Claude that never reports a list: after/clearthe new conversation's menu shows/model,/effort,/clear,/compact, the account's command and the project's skill; after a relaunch the same, with nothing remembered; a command file added on disk is pushed to a pane already showing the chat, namedfrontend:test; a chat another machine runs is answered with nothing from this machine's folders; a scan that hangs is retried after 30 seconds. With the at-rest list removed three go red, with the push removed three go red, with the host check removed one goes red, and against the first version of the scan the menu test and the hang test go red.structured-conversation-command.test.ts: "stops a running source's agent before it writes the marker" (red before the change: the old agent was still running at the write).structured-agent-session-rewind-at-rest.test.ts:/clearon an at-rest Codex chat with a cut-off rewind settles the rewind and commits (red on the previous head: refused).Queued messages across
/clear(structured-agent-session-queued-messages/-command/-gate.test.ts): the carry lands on the new conversation paused; a/clearwith no queued messages opens nothing. Three tests from feat(native-chat): hold mid-turn messages in a host-owned queue #23726 were already red onmainbecause they expected an older build's unfinished/clearto hold messages back, which fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524 made inert, or held/clearin flight through the agent start it no longer makes; they now test what/clearis.Each new test was run against the previous head (red) and against this change with one piece reverted (red): starting the old agent before clearing, a counter value of 0, writing the record and marker in two writes, the "couldn't start" wording, not copying options, an id derived from the operation id, and carrying the old provider conversation over.
Re-verified after merging main with fix(native-chat): chat failure messages appear in the app's language #23674 (which keeps only the reply a pane still waits on), live on a second Mac with a scripted Claude, at
59c14454da5:/clearwith Claude running stopped the old process 41-46 ms after Enter and showed the empty chat within about 0.3 s, with no notice and no console errors, and the next message started a fresh agent and was answered; a resting/clearwas empty in 0.18 s and started nothing; the/menu after/clearlisted all 7 entries; the failed-start Retry is unchanged. 190 tests across the 11 changed test files pass after the merge./clearwith Claude running (after the merge)/menu after/clearSeen, not caused by this PR:
/clearwhile a reply is still streaming is refused ("Wait for pending work and messages to finish before using this command."), as on main. The refusal line sits under the open "No matching commands or skills" menu and stays after the turn ends; pressing Enter again then clears. Tracked as a follow-up.I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Main merged in (b4b708c, 8dab53f, then 07e9fdf). What that needed:
commitConversationClear(the clear, its new record and the tab move in one write), now uses the new store'stransact. The earlier split of the queue for the line limit went with the queue./members an adapter had (readAtRestCommands,onAtRestCommandsChanged) are one member,atRestCommands, typed asStructuredAgentSessionAtRestCommands, which the Claude catalog already is. The adapter types stay within their line limit./clearrefusal check read the chat strip's child records, and this PR passes itat-restwhen no agent runs. The call now passes both.tc:nodepasses, and 16 changed and related test files pass (239 tests). After the earlier merge:tc:webpassed, 14 changed and related test files passed (248 tests), and oxlint, the changed-code gate, the reliability gates and the anti-slop audit are clean. The /clear paths QA'd live at 59c1445 (the command, the commit, the replacement) are unchanged by the merge, so that QA stands.Same on macOS, Linux, Windows and SSH hosts: the host that runs the agent does all of this.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)