Skip to content

refactor: retire the Deep Research workflow - #5554

Merged
Astro-Han merged 4 commits into
apache:mainfrom
Astro-Han:refactor/deep-research-retirement
Sep 21, 2026
Merged

Astro-Han merged 4 commits into
apache:mainfrom
Astro-Han:refactor/deep-research-retirement

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Retire the standalone Deep Research workflow before the first release. Remove its command, specialized tools and prompts, durable execution state machine, Host query/subscription, progress UI, and handoff action. This also removes the now-unused desktop mode-start path and the feature's tests and documentation.

Existing chats and reports remain readable. Legacy Sessions keep their persisted explore permission boundary and can be copied with their conversation and report artifacts. The historical workflow table remains part of the validated SQLite schema so existing State Roots can upgrade and retain their stored events; existing research runs cannot resume. Historical artifact metadata is accepted without loading the retired workflow implementation. Generic search, agent tools, and artifact handling remain available.

Refs #3268. The remaining contributor review stays in that issue.

Compatibility

Runtime Host epoch advances from 169 to 170 because deep-research.query and its subscription domain are removed. Desktop and Host must be upgraded together. Existing access credentials retire the removed deep-research.query grant while retaining their identity and other permissions. Creating deep_research is rejected; other unknown Desktop start modes retain their existing fallback behavior.

Verification

  • Affected builds and Desktop typechecks passed.
  • Focused Core, Storage, Runtime, Runtime Host, and Desktop tests passed, including legacy report reopening, read-only Session restart, two-client conversation/report copying, retired-mode rejection, and IPC recovery.
  • npm run format, npm run lint, renderer architecture checks, protocol epoch guard, ASF headers, and npm run check:asf-source passed.
  • Storybook built; the command palette, legacy Session context, and empty chat passed render/play, DOM, console, and accessibility checks in light and dark themes. Comparison baseline: e2a7cf233.
  • Storage workspace: 1,405 tests passed, 8 skipped. A regression test preserves retired research events through upgrade from the released fixture, reopen, and backup restore. It fails without the retained historical table definition.
  • Runtime Host startup, recovery, and storage maintenance: 32 tests passed. Access credential migration and metadata: 14 tests passed, including retirement of the historical research grant through rewrite and reopen.
  • Full repository tests and packaged Electron E2E were not run locally. Linux released State Root qualification and Windows packaged upgrade require the new CI run.

Command palette comparison, light

Command palette comparison, dark

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the removal and compatibility tests, checked the remaining call paths, and ran the verification above.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes, described under Summary above
  • No

@github-actions github-actions Bot added the effort/XXL Over 2500 readable lines label Sep 21, 2026
Remove the specialized research runtime, protocol, tools, storage writer, desktop entry points and progress UI. Preserve legacy chats and report artifacts under their existing permission boundaries, and advance the Runtime Host compatibility epoch.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the refactor/deep-research-retirement branch from c339bea to b3f7d78 Compare September 21, 2026 04:45

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seat: kabi-opus-review-orchestrator. Review of exact head b3f7d78b03e2b3dd17a8df9bec6a249c9daebe50, based on 1c18700a7 (behind main by 0). All 14 checks green, 1 skipped. Still a draft.

Verdict

No P0–P2. 1×P3. This is a careful retirement. The part that usually goes wrong in a 7,000-line deletion — something still reaching for what was removed — does not go wrong here, and I verified that by building rather than by reading.

The strongest evidence I can offer: a clean build from nothing

npm run clean && npm run build across all ten workspaces: 0 errors. That matters more than a passing test suite for a deletion, because a stale dist will happily resolve an import whose source no longer exists — the removed modules have to be genuinely absent from the emitted output before the compile means anything. They were.

Full workspace suite: desktop, cli, core, ui, mcp, computer-use, website, eval all passed. runtime, storage and runtime-host each reported failures — six in total, and all six reproduce on origin/main without this PR:

Failing test Workspace On 1c18700a7
keeps direct-only tools out of the cell snapshot runtime fails
keeps provider-native tools out of the cell snapshot runtime fails
uses normal partial-execution semantics before an unknown tool failure runtime fails
does not dispatch tools inherited from Object.prototype runtime fails
rejects a second authority for the same storage root in another process storage fails
WorkHub v2 keeps its attachment and browser tool ceiling visible in direct and Code Mode runtime-host fails

I checked each one against main individually rather than assuming, because four of them are tool-registry tests and a tool-removing PR is exactly the thing that would break them. None are attributable to this change. They are pre-existing in my environment; CI is green, so they are likely local to it.

The migration claim is true, and I confirmed it by breaking it

The description says the retained historical table is what lets existing State Roots upgrade, and that the regression test "fails without the retained historical table definition." A test that would pass anyway proves nothing, so I removed the workflow_deep_research_events block from sqlite-workflow-schema.ts and rebuilt.

preserves retired research events through upgrade, reopen, and backup fails — along with four other migration tests. Restoring the block: 40/40 pass. The claim holds and the test is not vacuous. It is also a good test on its own terms: it restores a real released v0.1.6 fixture, writes a research event, reopens under require_current, and reads it back through both the upgraded root and a backup-restored copy.

The residual references are all deliberate

56 mentions survive outside docs. I read every source-side one, and each is load-bearing with a comment saying why: the reserved historical mode:deep_research label in session-start-mode.ts; the explicit rejection in create-session-input.ts; the ['deep-research.query', { kind: 'release' }] grant retirement, which follows the same shape turn.regenerate used at epoch 167; the retained schema table; the retained workflow_deep_research domain mapping; and the legacy-Session story. protocol-compatible-changes/session-catalog-deep-research-import.json stays too, correctly — it is a historical record stamped "epoch": 68, not live configuration.

P3 — epoch 170 has no changelog entry, and 170 is probably already taken

packages/runtime-host/src/protocol/index.ts bumps to 170 with no // 170: line. Removing an operation is the harder break to explain after the fact, and there is a precedent directly in the list for the exact shape needed:

// 167: Removed the turn.regenerate operation. Older peers can no longer
// safely interoperate because they may submit or advertise that operation.

There is a second, concrete reason to write it now. PR #5552 also bumps to 170, for an unrelated change, and as of f86a7a3e8 it has added its own entry: "170: External Session import input may carry an optional Host-resolved workspace target." If that lands first, this PR's number has to move, and the entry then has to say 171 and describe this break. Writing it while the reasoning is fresh is cheaper than reconstructing it during a rebase.

On the shared epoch number — not a defect, and the repo already handles it

I want to record this accurately because my first reading was wrong. Two open PRs advertising epoch 170 for incompatible reasons looks alarming, and I initially took it for a serious problem. It is not, because scripts/protocol-epoch-check.mjs exists for precisely this case (#3313) and runs on the merge result against the current base. I exercised its decision function both ways:

  • base 169 → head 170: Protocol changed and the epoch moved: 169 -> 170. Both PRs pass today.
  • base 170 → head 170: fails, with …must land with an epoch the current base has not seen: rebase onto current main and set the epoch past 170.

So whichever of #5552 and #5554 merges second gets a clear, mechanical CI failure telling it to renumber. The hazard the guard was built to stop — two incompatible protocols merging without a git conflict because both wrote the same text to the same line — is genuinely stopped. This is a scheduling note for whoever merges second, not a finding against this PR.

未验证

No Electron run, no packaged E2E, no Windows or Linux, no real upgrade of an actual user's State Root beyond the committed v0.1.6 fixture. npm run check:asf-source failed for me at creates reproducible candidates from committed files only with spawnSync gpg ENOENT — my machine has no gpg; everything else in that check passed. Renderer architecture check passed, including --strict-base against 1c18700a7, which is the run that enforces base monotonicity rather than only comparing the ledger.

Approving per the request, on no P0–P2 and terminal green CI at this exact head. Two things that approval does not cover: whether Deep Research should be retired is a product decision and belongs to you, not to me; and this is still a draft.

简体中文

结论:无 P0–P2,1×P3。 这是一次谨慎的退休。七千行删除里通常出事的地方 —— 还有东西在伸手够已经删掉的东西 —— 这里没有出事;而且我是用构建验的,不是靠读的

最强证据:从零干净构建。 npm run clean && npm run build 覆盖全部十个 workspace,0 错误。对删除类 PR,这比测试全绿更重要:陈旧的 dist 会让一个源文件已不存在的 import 照样解析成功,必须先确认被删模块真的不在产物里,编译通过才有意义。

全量测试:desktop/cli/core/ui/mcp/computer-use/website/eval 全过。runtime/storage/runtime-host6 条失败,全部在 origin/main 上同样失败(逐条单独跑过控制组,没有假设):cell snapshot 的 direct-only / provider-native 两条、unknown tool 的部分执行语义、Object.prototype 继承工具不派发、storage 的跨进程第二 authority、以及 runtime-host 的 WorkHub v2 工具上限。其中四条正是工具注册表相关 —— 而这是一个删工具的 PR,所以我没有偷懒,逐条对 main 验了。结论:与本单无关,是我环境里的既有失败(CI 是绿的)。

迁移主张为真,且我用「破坏它」验证过。 描述说保留的历史表是既有 State Root 能升级的原因,并说「去掉保留的表定义,回归测试就会失败」。能被任何情况下通过的测试等于没测,所以我把 sqlite-workflow-schema.ts 里的 workflow_deep_research_events 块删掉重建:preserves retired research events through upgrade, reopen, and backup 确实失败,另有 4 条迁移测试一并失败;恢复后 40/40 全过。主张成立,测试不空。该测试本身也写得好:用真实的已发布 v0.1.6 fixture、写入研究事件、以 require_current 重开,并且分别从升级后的 root 和备份还原出的副本各读一次

残留引用全部是刻意的。 文档之外还有 56 处,源码侧我逐个读过,每处都有注释说明为什么留:session-start-mode.ts 保留历史标签 mode:deep_research;create-session-input.ts 显式拒绝;['deep-research.query', { kind: 'release' }] 的授权退役 —— 与 turn.regenerate 在 epoch 167 的做法同形;保留的 schema 表;保留的 workflow_deep_research 域映射;以及遗留会话的 story。protocol-compatible-changes/session-catalog-deep-research-import.json 也该留 —— 它是标着 "epoch": 68 的历史记录,不是活配置。

P3 —— epoch 170 没有变更说明条目,而且 170 很可能已经被占。 移除一个操作是更难事后解释的那类破坏,而列表里就有现成的同形先例:// 167: Removed the turn.regenerate operation…。还有一个具体理由:#5552 也提到 170,且截至 f86a7a3e8 它已经补上了自己的条目(「170: 外部 Session 导入输入可携带可选的 Host 解析 workspace 目标」)。若它先落地,本单的号就得往后挪,条目也得改写成 171 并描述本单这次破坏 —— 趁推理还新鲜写下来,比 rebase 时重新回忆便宜。

关于共用 epoch 号 —— 不是缺陷,仓库已经处理了。 这点我要写准,因为我最初读错了:两个开着的 PR 同时宣称 epoch 170 且理由互不兼容,看起来很吓人,我一开始当成了严重问题。它不是,因为 scripts/protocol-epoch-check.mjs 正是为这个场景(#3313)而建,并且跑在合并结果上、对照当前 base。我把它的判定函数两个方向都跑了:base 169 → head 170 通过(所以两个 PR 今天都过);base 170 → head 170 失败,并明确提示「必须落在 base 没见过的 epoch 上:rebase 到当前 main 并把 epoch 设到 170 之后」。所以 #5552#5554后合的那个会收到清晰的机械报错。该守卫要拦的东西 —— 两个分支把相同文本写到同一行,git 三方合并无冲突地放过两套不兼容协议 —— 确实被拦住了。这是给后合者的排期提示,不是对本单的发现。

未验证:未跑 Electron、未跑打包 E2E、无 Windows/Linux、除已提交的 v0.1.6 fixture 外没有真实用户 State Root 的升级。npm run check:asf-sourcecreates reproducible candidates from committed files only 处失败,原因是 spawnSync gpg ENOENT —— 我机器上没装 gpg,该检查其余部分全过。渲染层架构检查通过,含对 1c18700a7--strict-base(这一档才强制基线单调性,而非只比对台账)。

按请求给出批准,依据是无 P0–P2 且该 head 上 CI 终态全绿。批准不覆盖的两件事:Deep Research 该不该退休是产品决定,属于你,不属于我;以及它目前仍是 draft


Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

@Astro-Han
Astro-Han marked this pull request as ready for review September 21, 2026 05:34
@Astro-Han
Astro-Han merged commit b48ae6c into apache:main Sep 21, 2026
15 checks passed
@Astro-Han
Astro-Han deleted the refactor/deep-research-retirement branch September 21, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XXL Over 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants