revert: #18790 (orchestration incarnation reap fallback and bundled Freebuff agent) - #22601
Conversation
…reebuff agent) This reverts commit 0677271. #18790 was merged as one squash commit that carried two unrelated changes: a process-incarnation fallback for reaping leaked orchestration worker terminals, and an unannounced "Freebuff" third-party agent (catalog entry, icon, locale strings, README rows). The Freebuff agent was never meant to ship, so the whole PR is reverted; the reap fix should be re-submitted on its own. Until that re-land, a worker whose durable terminal handle goes stale is again reported missing on release/stop instead of being re-found through its process incarnation, so its terminal can leak on Remote Server. The mobile session page closure pin moves 4218 -> 4219: the revert drops the freebuff icon #22119 pinned (-1), and #22452 had already added two src/shared modules without re-pinning (+2).
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughFreebuff was removed from shared agent definitions, desktop and mobile catalogs, icons, locale entries, and supported-agent documentation. Worker terminal observation and release no longer remint terminal handles from process incarnations. Release paths now use the durable terminal handle, and missing or unattached liveness settling is limited to recovery mode. Related incarnation-recovery test suites were removed. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to After an unproven worker release, following the displayed retry instructions may only return the same result instead of retrying the close. Correct the instructions before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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[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: efc5f84b-9f2e-4662-b832-4be87e1a6330
⛔ Files ignored due to path filters (1)
src/shared/agent-icons/freebuff.pngis excluded by!**/*.png
📒 Files selected for processing (41)
README.mdconfig/scripts/locale-translation-policy.mjsconfig/scripts/mobile-web-app-session-terminal-closure.test.mjsdocs/readme/README.es.mddocs/readme/README.fr.mddocs/readme/README.ja.mddocs/readme/README.ko.mddocs/readme/README.pt.mddocs/readme/README.zh-CN.mddocs/site/content/docs/agents/supported.mdxmobile/src/components/mobile-agent-icon-assets.tsmobile/src/tasks/mobile-tui-agents.tssrc/main/runtime/orca-runtime-get-orchestration-dispatch-authority.tssrc/main/runtime/orca-runtime-terminal-handle-incarnation.test.tssrc/main/runtime/orchestration/worker-terminal-process-liveness.test.tssrc/main/runtime/orchestration/worker-terminal-process-liveness.tssrc/main/runtime/rpc/methods/orchestration-worker-release-incarnation-fallback.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-reap-fixed.func.test.tssrc/main/runtime/rpc/methods/orchestration/worker/structured-worker-release-stop.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-control.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-observation.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-release-completion.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-release-runtime-incarnation.test.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-stop.tssrc/main/runtime/rpc/methods/orchestration/worker/worker-terminal-release-lease.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/lib/agent-catalog.tsxsrc/renderer/src/lib/agent-favicon-assets.tssrc/renderer/src/lib/agent-status.tssrc/shared/agent-kind.tssrc/shared/skills-cli-agent-keys.tssrc/shared/telemetry-property-schemas.tssrc/shared/tui-agent-config.tssrc/shared/tui-agent-display-names.tssrc/shared/tui-agent-selection.tssrc/shared/tui-agent.ts
💤 Files with no reviewable changes (33)
- docs/readme/README.ja.md
- docs/readme/README.ko.md
- docs/readme/README.fr.md
- docs/readme/README.pt.md
- src/shared/telemetry-property-schemas.ts
- src/shared/agent-kind.ts
- config/scripts/locale-translation-policy.mjs
- src/renderer/src/lib/agent-favicon-assets.ts
- src/shared/skills-cli-agent-keys.ts
- src/renderer/src/lib/agent-catalog.tsx
- README.md
- src/renderer/src/lib/agent-status.ts
- docs/readme/README.zh-CN.md
- src/renderer/src/i18n/locales/en.json
- src/main/runtime/rpc/methods/orchestration/worker/worker-release-runtime-incarnation.test.ts
- src/main/runtime/rpc/methods/orchestration-worker-release-incarnation-fallback.test.ts
- src/shared/tui-agent-config.ts
- docs/site/content/docs/agents/supported.mdx
- src/shared/tui-agent-display-names.ts
- mobile/src/components/mobile-agent-icon-assets.ts
- docs/readme/README.es.md
- mobile/src/tasks/mobile-tui-agents.ts
- src/main/runtime/orchestration/worker-terminal-process-liveness.test.ts
- src/renderer/src/i18n/locales/fr.json
- src/renderer/src/i18n/locales/ja.json
- src/renderer/src/i18n/locales/es.json
- src/main/runtime/rpc/methods/orchestration-worker-release-reap-fixed.func.test.ts
- src/renderer/src/i18n/locales/zh.json
- src/shared/tui-agent.ts
- src/shared/tui-agent-selection.ts
- src/renderer/src/i18n/locales/ko.json
- src/main/runtime/orca-runtime-get-orchestration-dispatch-authority.ts
- src/main/runtime/orca-runtime-terminal-handle-incarnation.test.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| archive: { source: args.archiveSource, status: args.archiveStatus }, | ||
| lastError: unknown.release_error ?? stop.reason, | ||
| recovery: releaseUnknownRecovery(dispatchId) | ||
| recovery: `Inspect with: orca orchestration worker-show --dispatch ${dispatchId} --json — then repeat worker-release with the same --retry-request.` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
rg -n -C4 'releaseUnknownRecovery|retry-request|retryRequest' src/main/runtime --type=ts -g '!**/*.test.ts' | head -120
rg -n -C6 'orchestrationRequestId|requestId.*replay|replay' src/main/runtime/rpc --type=ts -g '!**/*.test.ts' | head -120Repository: stablyai/orca
Length of output: 24207
Use releaseUnknownRecovery for the structured release path.
When a structured-worker close is not proven, the current recovery text tells the caller to reuse --retry-request. The shared releaseUnknownRecovery helper states that reusing the prior request ID only replays the existing release_unknown receipt. The caller must use a fresh request ID to retry the close.
🐛 Suggested fix
-import type { WorkerReleaseReceipt } from './worker-release-completion'
+import { releaseUnknownRecovery, type WorkerReleaseReceipt } from './worker-release-completion'
@@
- * proven close may settle; an unproven one reports `release_unknown` and stays retryable under the
- * same request id.
+ * proven close may settle; an unproven one reports `release_unknown` and is retried with a fresh
+ * request id (replaying the prior id returns the same receipt).
@@
- recovery: `Inspect with: orca orchestration worker-show --dispatch ${dispatchId} --json — then repeat worker-release with the same --retry-request.`
+ recovery: releaseUnknownRecovery(dispatchId)![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Reverting #18790 also reverts an unrelated, pre-existing bug fix it bundled in — the structured-worker release_unknown recovery guidance. As written, a failed structured-worker close is permanently unretryable with the same request id. The rest of the revert is faithful and complete; details below.
Reviewed changes
- Removes the bundled Freebuff agent — drops
freebufffrom the shared TUI-agent registries, renderer catalog/favicon/status, mobile icon assets and favicon domains, locale JSONs, the locale translation policy, the READMEs, and the docs supported-agents table, and deletessrc/shared/agent-icons/freebuff.png. - Removes the process-incarnation reap fallback — deletes
resolveTerminalHandleByProcessIncarnation, inlinesmatchesProcessIncarnationback intoclassifyWorkerTerminalProcessIncarnation, and removes theterminalHandle/liveTerminalHandleplumbing fromworker-observation,worker-control,worker-stop,worker-release-completion, andworker-terminal-release-lease. - Restores the pre-fix release-completion shape — the missing/unattached + provably-exited settle path moves back inside
mode === 'recovery', so interactive releases reportrelease_unknownagain. - Deletes 6 fallback tests and repins the mobile session-page closure census (
4218 -> 4219, with a measured-note paragraph for the #22452 delta and the removed icon module).
I verified the revert leaves no dangling references (matchesProcessIncarnation, resolveTerminalHandleByProcessIncarnation, and freebuff no longer appear outside the census file's historical note), that no commit after #18790 touched any reverted file (so no later fix is clobbered), and that the surviving affected tests pass (worker-release-recovery, worker-release, worker-observation, orca-runtime-terminal-handle-incarnation, orca-runtime-process-incarnation-liveness — 63 tests).
The leak coming back on Remote Server hosts is documented and intentional, and the untested saved-freebuff preference degrades safely through the existing isTuiAgent / normalizeDisabledTuiAgents guards, so neither is a finding.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| archive: { source: args.archiveSource, status: args.archiveStatus }, | ||
| lastError: unknown.release_error ?? stop.reason, | ||
| recovery: releaseUnknownRecovery(dispatchId) | ||
| recovery: `Inspect with: orca orchestration worker-show --dispatch ${dispatchId} --json — then repeat worker-release with the same --retry-request.` |
There was a problem hiding this comment.
This hunk reverts more than Freebuff or the terminal-leak fallback: #18790 also fixed a pre-existing bug here. The old inline string tells the caller to retry with the same --retry-request, but a same-request-id replay only returns the recorded release_unknown receipt, so a structured worker whose close failed is permanently unretryable. releaseUnknownRecovery(dispatchId) is still defined in worker-release-completion.ts:310 and used by the terminal and federated release paths; keeping that call here preserves the fix while still removing Freebuff and the incarnation fallback.
Technical details
# Structured-worker release guidance regressed to the unretryable same-request-id form
## Affected sites
- `src/main/runtime/rpc/methods/orchestration/worker/structured-worker-release-stop.ts:39` — `recovery` string restored to "repeat worker-release with the same --retry-request"
- `src/main/runtime/rpc/methods/orchestration/worker/structured-worker-release-stop.ts:14` — doc comment restored to "stays retryable under the same request id"
## Required outcome
- A structured-worker close failure must stay retryable under a fresh request id, matching `releaseUnknownRecovery` and the terminal (`worker-release-completion.ts:183,267,295`) and federated (`federated-worker-release.ts:100`) paths.
## Suggested approach
- Restore `releaseUnknownRecovery(dispatchId)` and its import instead of the inline string; revert only the Freebuff and process-incarnation-fallback hunks.
## Context
- #18790's body records this as a CodeRabbit-driven fix: "stopStructuredWorkerForRelease told the caller to repeat the release with the same --retry-request, which only replays the stale release_unknown receipt and made a structured-worker close failure permanently unretryable."
- No surviving test pins the structured-worker message; `worker-release.test.ts:409,445` only asserts the terminal path's `'fresh request ID'`, so this regression will not be caught by CI.
1 participant
ELI5
This undoes PR #18790 in full. That PR was merged as a single commit that did two unrelated things: it fixed a leak of orchestration worker terminals, and it quietly added a new third-party coding agent called "Freebuff" to Orca. We never meant to ship the Freebuff agent, so we are taking the whole PR back out. The leak fix should come back as its own PR.
What Changed
What users see, before and after:
Mechanism: this is
git revertof0677271709(the #18790 squash commit). The changed lines are the exact inverse of that commit; the revert applied with no conflicts on today's main.One test pin also moves. The mobile session page closure census (
config/scripts/mobile-web-app-session-terminal-closure.test.mjs) counts the modules the session page bundles. #22119 raised it by one for the Freebuff icon, so the revert lowers it by one. Main was also already two over the pin: #22452 addedsrc/shared/main-agent-status.tsandsrc/shared/agent-turn-outcome.tsto that page but only touchedsrc/shared, so the mobile bundle job never ran. The count is measured, not worked out by arithmetic: 4,220 on main, 4,219 with this revert. The pin goes 4,218 -> 4,219, and both moves are written up in the file's history.Why
This change will also ship in a release: the same revert is being applied to the v1.4.210 release branch (
release/adhoc-1.4.210-daily-202609221832-cherrypicks), so that build will not include Freebuff either.Linked Issue
Reverts #18790.
Visual Proof
N/A. This removes code: one agent entry disappears from lists that are data-driven, and a background cleanup path goes away. There is no new UI to show.
Testing
Run locally on macOS:
pnpm tc: pass.vitestover the orchestration worker/runtime/CLI tests, the shared TUI-agent/agent-kind/telemetry tests, the renderer agent catalog and launch-routing tests, and the locale script tests: 3,238 passed. The other 2, both CLI task-list tests that fail under a localORCA_STRUCTURED_SESSION=1shell variable, pass once it is unset (a local environment leak, not related to this change).mobile/): 577 passed.ORCA_MOBILE_WEB_APP_DEPS_REQUIRED=1: the session closure census passes at the new pin, and all bundle and Chrome render checks pass. The WebKit render checks could not run locally because WebKit is not installed, so CI covers them.verify:localization-catalog,verify:localization-runtime-catalog,verify:localization-extraction,verify:localization-coverage,check:readme-local-links: pass.check:code-quality:changed,oxlint, andoxfmt --checkon changed files: pass.git grep -i freebufffinds nothing outside the census file's historical note (and nothing existed before fix(orchestration): reap leaked worker terminals via process-incarnation fallback — stops an unbounded PTY/process leak on Remote Server (OOM / cgroup PID exhaustion) #18790).AI Disclosure
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
Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance
freebuffis no longer a known agent id, so anyone who picked it will no longer see it offered. I did not separately test how a savedfreebuffpreference behaves after this change.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)