Skip to content

chore: remove registerComputerUseActionTools()#16598

Merged
siddseethepalli merged 1 commit into
mainfrom
do/remove-register-computer-use-action-tools
Mar 14, 2026
Merged

chore: remove registerComputerUseActionTools()#16598
siddseethepalli merged 1 commit into
mainfrom
do/remove-register-computer-use-action-tools

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

@siddseethepalli siddseethepalli commented Mar 14, 2026

Summary

  • Delete assistant/src/tools/computer-use/registry.ts — the sole export (registerComputerUseActionTools) was only used by tests
  • Inline direct tool registration in checker.test.ts using allComputerUseTools + registerTool
  • Remove the dedicated test in registry.test.ts that validated the deleted function

Original prompt

Remove registerComputerUseActionTools() — tools/computer-use/registry.ts.

🤖 Generated with Claude Code


Open with Devin

…/registry.ts

Computer-use tools are provided by the bundled skill. This function only
existed for backward compat in tests — inline the registration directly.

Co-Authored-By: Claude <noreply@anthropic.com>
@siddseethepalli siddseethepalli merged commit 79d65a2 into main Mar 14, 2026
@siddseethepalli siddseethepalli deleted the do/remove-register-computer-use-action-tools branch March 14, 2026 03:36
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

dvargasfuertes pushed a commit that referenced this pull request May 28, 2026
…32365)

* tools: drop executionMode from base-list dedup; owner is sufficient

The filter at registry.ts:getAllToolDefinitions and
tasks/tool-sanitizer.ts:getRegisteredToolNames was

  t.executionMode !== "proxy" && getToolOwner(t.name)?.kind !== "skill"

Two clauses, one job: keep proxy/projection tools out of the base tool
list so they don't double-up when a skill projects them. The first
clause is redundant — every tool the LLM should NOT see by default is
already excluded by the owner clause:

  - computer_use_*: not core-registered at all (#16598 removed
    registerComputerUseActionTools). They enter the registry only via
    registerSkillTools("computer-use", …) during skill projection,
    where their owner becomes {kind: "skill", id: "computer-use"} —
    owner-clause excludes them.
  - meet_*: registered via registerExternalTools with skill owner —
    owner-clause excludes them.
  - ui_show / ui_update / ui_dismiss: registered as core (no owner) →
    SHOULD appear in the base list. The old filter excluded them via
    executionMode, then conversation-tool-setup.buildToolDefinitions
    spliced them back in unconditionally. Net result identical.
  - app_open: same shape as ui_*.

After this PR:

  - registry.ts:getAllToolDefinitions and tool-sanitizer.ts both
    filter on owner alone.
  - conversation-tool-setup.buildToolDefinitions stops splicing
    allUiSurfaceTools + coreAppProxyTools — they flow through
    getAllToolDefinitions naturally, identical final tool set.
  - registry.ts snapshot drops the vestigial allComputerUseTools.map
    entry from manifestToolNames (no-op since #16598).
  - registry.test.ts "core registry includes app_open proxy tool"
    flips its assertion to expect app_open in
    getAllToolDefinitions, which is the new (correct) behavior.

executionMode stays on Tool definitions: it still drives dispatch
routing via resolveExecutionTarget (host vs sandbox) and the
tool-approval-handler proxyToolResolver gate. That role survives.
This PR removes the field's secondary role as the base-list dedup
flag — which is what PR #32362 had to rename to "projectedBySkill"
to express. With this PR landed, that rename is no longer needed:
the field can simply be deleted in the follow-up.

### AGENTS.md compliance

- assistant/src/tools/AGENTS.md — no new tools added; refactor of
  existing registry/dispatch surface only.

* address review feedback + fix CI

review feedback:
- delete redundant buildToolDefinitions() wrapper; replace 4 callers
  with direct getAllToolDefinitions() calls (conversation.ts, btw-routes.ts,
  + 2 test files).
- delete historical comment about allComputerUseTools in registry.ts.

ci fixes (both pre-existing on main from #32361):
- add home/feed/query route-policy entry (settings.read), matching the
  POST endpoint introduced in #32361 but never registered.
- regenerate openapi.yaml to include the new /v1/home/feed/query path
  (445 paths, 507 operations).

---------

Co-authored-by: vellum-apollo-bot[bot] <242025090+vellum-apollo-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant