Repository navigation
fix(orchestration): keep a released worker's closing turn and never leave a blank pane - #34
Conversation
…eave a blank pane Released supervised workers "went dark" for the operator: - worker-release froze the archive and SIGTERMed the agent as soon as it was called, so a coordinator releasing on receipt of worker_done cut off the turn that sent it. Release now waits (bounded, 20 s) for a running agent's tui-idle before capturing and closing, and records a warning in the archive when the turn was still running at the bound. Recovery, structured workers and retried releases skip the wait. - A paired client keeps the emptied runtime-owned workspace active (by design since stablyai#21630), but its tab group body rendered nothing. The group now shows a New Terminal action when it has no tabs. - worker-list's nextAction told coordinators to worker-release a worker the user had retained, undoing worker-retain. Retained rows now owe no release. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review complete. No issues found — approved ✅. Adds a bounded closing-turn wait (with a cut-off archive warning) before worker terminal release so the agent's final message is captured, excludes Reviewed commit: 5017872 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5017872697
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: the help text promised the closing-turn wait for every release, but a worker on a connected server went through the execution host's federationRelease handler, which captured and closed at once. The host handler now waits before it inspects the attachment and passes the outcome to releaseRemoteAttachment, which records the cut-off warning the same way. The bound drops to 15 s so the host still answers inside the 30 s a coordinator (old or new) gives a federated release; no wire or deadline change. The wait's stored-archive check no longer throws, so an unreadable archive store is still reported by the release itself. federated-worker-release-host reuses retainedReason instead of its copy. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review complete. 🟡 3 medium Reviewed the latest changes. 💬 Inline comments (2)
📍 Outside the diff (1)
Reviewed commit: f110bd2 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f110bd2884
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
This delta threads the worker closing turn into the federation release path: it waits up to 15s for a closing turn before observation, records a cut-off warning when no stored archive exists, and moves retainedReason handling into worker-release-completion. Three medium-severity issues were found in the delta: the full wait can exceed the 30s federation deadline, hasStoredArchive swallows read errors and silently skips the wait, and concurrent releases of one dispatch skip coalescing and can double-close the terminal.
A newer Tenki review found issues on this pull request.
…n wait - A tui-idle wait that ends on a human-only prompt (satisfied: false) no longer counts as a finished turn; the archive records the cut-off. - An unreadable archive store no longer skips the wait; the release path still reports the read failure itself. - The coordinator gives a federated release the closing-turn bound plus 25 s, so a full wait still leaves the host room to capture and close. Older hosts never wait, which the worker-release help now says. - The emptied-workspace action needs an accepted, explicitly empty tab row for that workspace, so it cannot appear (and race the host into a duplicate terminal) before a paired workspace's inventory arrives. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review complete. No blocking issues — 1 nitpick below. Reviewed the latest changes; 2 prior findings remain open; 1 prior finding resolved. 🧹 Nitpicks (1) — 🟢 1 low
Delta fixes the closing-turn wait (archive read failures no longer skip it, prompt-blocked turns now count as cut_off) and raises the federated release budget to 40 s, with only a stale 30 s comment left behind. Reviewed commit: 4ac1064 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ac1064869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
ELI5
When a coordinator agent finishes with a helper agent ("worker"), it tells Orca to clean up the worker's terminal. Three things went wrong when that happened:
This PR fixes all three.
What Changed
1. Release waits for the worker's last turn. A worker reports "done" partway through its final turn, then keeps writing its closing message for a few seconds.
orca orchestration worker-releaseused to save the worker's output and stop the agent immediately, cutting that message off. In the run that prompted this, two of three workers lost it.tui-idleterminal wait, the same oneorca terminal wait --for tui-idleuses. Then it saves the output and closes the terminal. This applies both to workers on the coordinator's own machine and to workers on another connected Orca server; for the latter, the server running the worker does the wait.worker-readshows that warning.2. An emptied workspace offers "New Terminal" instead of a blank pane. When the server closes the last terminal in a workspace, a connected desktop keeps that workspace open.
3. Kept workers stay kept.
worker-listprojects anextActionper row, and it returnedworker-releasefor any settled worker that still owned its terminal. That included workers someone had explicitly kept withworker-retain, and workers retained because their identity couldn't be proven, where it caused a retry loop. Retained rows now get no next action.The
worker-releasehelp text now mentions the wait.Why
worker-releasehelp says so. There is no wire change.worker-retainor a user typing into the worker during the wait still wins, because the archive is only committed while the release is still pending. A test covers the retain case.Linked Issue
No tracking issue. This came from an operator report: three released workers in one supervised run "went dark." Related upstream context: stablyai#13047, stablyai#21630 and stablyai#21992.
Visual Proof
Captured from the new end-to-end test: a hidden-window Electron desktop client connected to a headless Orca server, Linux.
Ctrl + Tchip in both themes. Clicking it creates a terminal.The screenshots couldn't be attached from the build host because the
gh imageextension isn't installed there. They're available and will be added as a comment.Testing
New unit tests in
worker-release-closing-turn.test.ts:worker-readreturns;Three of the four fail on the unfixed code. The fourth guards the plain-shell skip.
New federated test
federated-release-closing-turn.test.ts: on the server running the worker, release waits before saving and closing, and records the cut-off warning at the bound. Both cases fail without the change.New projection test: a retained, owned, settled worker gets no
worker-releasenext action. It fails on the unfixed code.New Electron end-to-end test
tests/e2e/paired-emptied-workspace-empty-state.spec.ts. It runs a headless server and a desktop client, closes the last terminal on the server, expects the empty state, then clicks New Terminal and expects a terminal. It fails with the fix reverted ("element(s) not found") and passes with it.Existing suites:
pnpm testover the orchestration, runtime, shared fleet, CLI and tab-group suites passes (379 files).Checks:
pnpm tcandpnpm run check:code-quality:changedpass, including the design-system rules.oxlintis clean on the changed files.Not run: the full
pnpm lintand fullpnpm test.Platforms: Linux only. The shortcut chip uses the existing platform-aware shortcut helper.
I manually tested these changes locally (verified through the hidden-window end-to-end test and screenshots, not by hand)
Automated tests added/updated, or explained why not below
AI Disclosure
Written with Claude Opus 5.5 in Claude Code.
Review
Not in this PR (follow-ups)
orca orchestration worker-read; the desktop has no view for it.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
warningslist.Checklist
N/Awith reason (described above; images to follow in a comment)pnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (typecheck, changed-lines lint gate, targeted tests and the e2e build pass locally; full lint/test left to CI)🤖 Generated with Claude Code