Repository navigation
Conversation
…again Waking a sleeping terminal agent in its pane registers the pane's launch config and then clears the sleeping record it woke from. Clearing that record also deleted the launch config it had just registered, so the next sleep captured a record without it. The second wake then resumed the agent without its saved permission flags, custom args, and env. Clearing a sleeping record now clears only the record. The launch-config registry stays owned by pane retirement and the existing identity checks.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 13 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the primary change: preserving saved launch flags after a woken agent sleeps again. Description check The description is detailed and covers the user impact, causes, implementation, affected paths, linked issue, testing, known gaps, and scope. It omits some template sections, including Visual Proof an… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is detailed and covers the user impact, causes, implementation, affected paths, linked issue, testing, known gaps, and scope. It omits some template sections, including Visual Proof and checklist confirmations, but the core required information is present.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
⚔️ Resolve merge conflicts: 💡
-
- Resolve merge conflict in branch
sta-11575-wake-drops-launch-flags
- Resolve merge conflict in branch
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…re its first prompt A woken pane registers its launch config with a launch token, and sleep capture only trusted that entry when a hook had brought the same token. Claude reports its token as soon as it starts, but Codex waits for the first prompt. A Codex pane that woke and slept again without a prompt was captured with no launch config, so the next wake dropped its saved flags (for example --ask-for-approval and --sandbox). A resume spawn now pins the session it resumed on its registry entry, and sleep capture accepts the entry without a token only when that pinned session matches the pane's session. Fresh launches and a different session in the same pane still get no stale flags.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8c267a6d-587f-419d-8b91-2d37fea69479
📒 Files selected for processing: 7
src/renderer/src/components/terminal-pane/pty-connection-agent-session-resume.spec.tssrc/renderer/src/components/terminal-pane/pty-connection-sleeping-resume-banner.spec.tssrc/renderer/src/components/terminal-pane/pty-connection/deferred-cold-restore-and-snapshot.tssrc/renderer/src/components/terminal-pane/pty-connection/fresh-spawn-start.tssrc/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.tssrc/renderer/src/store/slices/agent-status-launch-config.tssrc/renderer/src/store/slices/agent-status-wake-launch-config.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
| tabId: entry.tabId ?? getTabIdFromPaneKey(entry.paneKey) ?? undefined, | ||
| terminalHandle: entry.terminalHandle, | ||
| launchToken: undefined, | ||
| launchToken: pinnedSessionMatches ? registryEntry?.identity.launchToken : undefined, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🔎 Supported by static analysis
🏁 Script executed:
sed -n 90,150p src/renderer/src/store/slices/agent-status-launch-config.ts
sed -n 1,130p src/renderer/src/store/slices/agent-status-wake-launch-config.test.ts
rg -n 'removeAgentLaunchConfig|clearAgentLaunchConfig|registryEntryMatchesStatus' src/renderer/src | head -50Repository: stablyai/orca
Length of output: 14065
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- launch config and matcher ---'
nl -ba src/renderer/src/store/slices/agent-status-launch-config.ts | sed -n '1,155p'
printf '%s\n' '--- live entry builder ---'
nl -ba src/renderer/src/store/slices/agent-status-live-entry-builder.ts | sed -n '130,215p'
printf '%s\n' '--- live reducer ---'
nl -ba src/renderer/src/store/slices/agent-status-live-reducer.ts | sed -n '1,125p'
printf '%s\n' '--- pane identity cleanup ---'
nl -ba src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts | sed -n '1,255p'
printf '%s\n' '--- launch actions and provider-session actions ---'
nl -ba src/renderer/src/store/slices/agent-status-launch-actions.ts | sed -n '1,120p'
nl -ba src/renderer/src/store/slices/agent-status-provider-session-actions.ts | sed -n '1,120p'
printf '%s\n' '--- relevant PR diff ---'
git diff --unified=35 01456c14ecbe489461e87e99a59ef91a99953800 5fcf147fea4c3847ed43218aa2ad86f5b2f7bbb6 -- src/renderer/src/store/slices/agent-status-launch-config.ts src/renderer/src/store/slices/agent-status-live-entry-builder.ts src/renderer/src/store/slices/agent-status-live-reducer.ts src/renderer/src/store/slices/agent-status-runtime.ts src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.tsRepository: stablyai/orca
Length of output: 42462
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- matcher and capture ---'
nl -ba src/renderer/src/store/slices/agent-status-launch-config.ts | sed -n '1,145p'
printf '%s\n' '--- identity cleanup ---'
nl -ba src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts | sed -n '150,230p'
printf '%s\n' '--- reducer and builder ---'
nl -ba src/renderer/src/store/slices/agent-status-live-entry-builder.ts | sed -n '155,200p'
nl -ba src/renderer/src/store/slices/agent-status-live-reducer.ts | sed -n '50,90p'
printf '%s\n' '--- relevant callers and cleanup tests ---'
rg -n -C 8 -F -- 'clearAgentLaunchConfig' src/renderer/src/components/terminal-pane/pty-connection src/renderer/src/store/slices | sed -n '1,260p'
printf '%s\n' '--- launch-token and provider-session comments/tests ---'
rg -n -C 6 -i -- 'reuse|stale|pinned|launch token|provider session|manual run' src/renderer/src/store/slices/agent-status-* src/renderer/src/components/terminal-pane/pty-connection/pane-agent-identity.ts | sed -n '1,300p'Repository: stablyai/orca
Length of output: 42247
Do not validate a launch token against itself.
registryEntryMatchesStatus correctly treats a missing launch token as stale when a registry token exists. However, getLaunchConfigForEntry supplies that same registry token whenever the pinned provider session matches, so the check becomes self-validation.
A different manual or mixed Codex run can reuse the provider-session ID. The live reducer does not retire the registry when the session ID is reused and the launch token is missing. The pane cleanup also clears the registry only on confirmed visible-pty or process-exit paths. If those paths do not run, a later sleep can capture the previous run’s permission mode, arguments, or environment.
Preserve the pinned-session behavior for the known hibernation-wake record, but do not use provider-session equality as general launch proof. Retire the registry at the manual-run/process-identity boundary when no matching launch token exists.
The previous commit pinned the resumed session as the registry entry's providerSession. That also changed how agent hooks match the entry: when a resume did not take and the pane started a new session, the new session's first hook no longer matched, and its saved flags were dropped where they used to be kept. The resumed session now goes in its own field, resumedProviderSession, which only sleep capture reads. Hook matching is back to how it was. The field is set only when the resume really ran: after a local spawn or cold restore that main did not report as resume-unavailable. Remote panes do not report that outcome, so they do not get the field.
…estart restores them (#33523) A woken sleeping agent keeps its launch config (flags like permission mode, model, custom args) so a later Orca restart restores it with the same flags; consuming the sleeping record no longer deletes the pane's launch config (retirement and launch-token/session matching still govern it). Supersedes #31756 Co-authored-by: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com>
|
Superseded by #33523 (merged, with your commits and co-author credit): a woken agent keeps its launch config so flags survive a later restart. |
ELI5
When a sleeping terminal agent woke up in its tab, Orca saved how it was launched and then deleted that save in the same step. The second sleep and wake therefore resumed the agent without its saved flags (permission mode, custom args, env). Fixes STA-11575.
Cause
clearSleepingAgentSessionsByPaneKeydeleted both the sleeping record and the pane's launch-config registry entry. The in-place wake path registers the config and then consumes the record, so the config it had just registered was erased. Later hook status carries only the launch token, so the next sleep captured a record with no launch config. This coupling dates from #5916.Second cause: even with the registry kept, sleep capture (
getLaunchConfigForEntry) checks the registry entry without a launch token, and every entry registered by a spawn carries one, so capture always rejected it. Capture then fell back to the record, which only gets a launch config from a hook that carries the token. Claude sends that hook as soon as it starts; Codex waits for the first prompt. So a Codex pane that woke and slept again before any prompt still lost its flags.Fix
Clearing a sleeping record now clears only the record. Launch configs still retire with the pane (
removeAgentStatus*,dropAgentStatus*,dropHibernatedAgentStatusPane,retireAgentPaneAuthority, and the worktree-level clears) and on identity mismatch. Registry reuse is guarded byregistryEntryMatchesStatus(launch token plus pinned provider session).For the second cause: the registry entry gets a capture-only field,
resumedProviderSession, naming the session the spawn really resumed. It is set only when the resume ran: after a local spawn that main did not report as resume-unavailable (registerEffectiveLaunchConfigin fresh-spawn-start), and after a local cold-restore reattach under the same condition (apply-reattach-payload). The pre-spawn registration inapplyColdRestoreAgentResumeStartupdoes not set it, since the outcome is not known yet. IngetLaunchConfigForEntryonly, sleep capture returns the entry's config when that field equals the pane's session and the agent, tab, leaf and terminal handle still match. Otherwise it falls through to the unchanged token check. Hook matching (registryEntryMatchesStatus) never reads the field, so a resume that did not take, or a /new session, keeps its flags through its token hook exactly as before.Caller audit of
clearSleepingAgentSession/clearSleepingAgentSessionsByPaneKey:sleeping-record-accessclearSleepingRecordProviderDuplicates(fresh spawn, reattach)resume-sleeping-agent-session.ts(8 sites)sleeping-agent-session-launch.tswake-sleeping-agents-in-background.tsagent-status-listeners.ts(legacy worker recovery)removeAgentStatuson PTY exit.Affected paths
Cold restore after app restart, manual workspace sleep and wake, auto-hibernation wake, SSH and remote panes, and phone-triggered wakes of a windowed desktop.
Not affected by the first cause: waking into a new tab, and a windowless server (it reads the config from the live PTY).
Known gaps: new-tab resumes (and history resumes) do not set the field, because forks share that path and get a new session id. Remote panes do not set it either, because the remote transport does not report whether the resume ran. In both cases a Codex pane that sleeps before its first prompt can still lose its flags; that is left for a follow-up.
Verification
agent-status-wake-launch-config.test.ts: sleep, wake, sleep, wake keeps the flags; a Codex-style pane with no token hook keeps them across two wakes (fails with the capture branch removed); a resumed launch whose hook reports a different session keeps its flags (fails with the earlier providerSession pin). Negative cases: no resumed session, or a different one, gets no flags at capture. A pty spec checks that a resume main declined does not set the field (fails without the gate).pnpm tc:web, oxlint, oxfmt, the changed-code quality gate, and the pr.yml check scripts are green.claude --resume <id>(flags lost). On the fixed build, wakes 1 and 2 both ranclaude --dangerously-skip-permissions --model haiku --resume <id>. Claude itself restored the bypass mode from its transcript, so the screen looked the same; the difference is in the process args and the saved record.--ask-for-approval on-request --sandbox read-only): the base build and the build with only the first fix both ran wake 2 ascodex ... resume <id>without the flags. Codex still asked for approval after wake 2 because it restores its mode from the session, so the screen did not show the loss. That run found the second cause. Real-app re-verification of the combined fix is pending.Not in scope
Model picker flags on resume (#21152) are excluded by design (#9085).