Skip to content

feat(mcp): surface scheduled mcp_tool tasks before they break (#390) - #557

Merged
padak merged 1 commit into
mainfrom
feat/mcp-tool-task-detection
Aug 11, 2026
Merged

feat(mcp): surface scheduled mcp_tool tasks before they break (#390)#557
padak merged 1 commit into
mainfrom
feat/mcp-tool-task-detection

Conversation

@padak

@padak padak commented Aug 5, 2026

Copy link
Copy Markdown
Member

Phase 2 of the MCP removal, part 1 of 2: find the tasks, do not rewrite them.

agent --type mcp_tool is removed in v0.85.0. It is the one deprecated surface
whose users are absent when it breaks: an interactive tool call warns on
every invocation right up to removal, but a scheduled task was warned once --
when it was created -- and then runs unattended. At removal it simply starts
failing on its next cron tick.

  • kbagent doctor gains an mcp_tool_tasks check listing each affected task,
    the tool it calls and the native command that replaces it. A tool the parity
    map does not know reports native_command: null rather than inventing one.
    Filesystem only -- no API call, no MCP spawn -- and skips when there is no
    agents.json.
  • kbagent agent list notes the affected tasks under the table and points at
    doctor. The marker is deliberately NOT in the Type cell: Rich truncated it to
    "DEPRECA…" there, which is worse than not flagging it. --json gains an
    additive per-task deprecation key, present only on affected tasks so every
    existing consumer sees a byte-identical payload.

Deliberately NOT included: automatic rewriting. ParityEntry carries only the
command name, no argument mapping, while mcp_tool params are the MCP tool's own
input dict -- different key naming and, for several tools, different
structure. Guessing the argv of a scheduled WRITE task that then runs
unattended is the worst place to be wrong; a bad rewrite beats a clear failure
in no scenario. Part 2 will emit a reviewable migration plan with explicit
TODOs instead.


Open in Devin Review

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

Open in Devin Review

Comment thread src/keboola_agent_cli/services/doctor_service.py
Comment thread src/keboola_agent_cli/commands/agent.py Outdated
Phase 2 of the MCP removal, part 1 of 2: find the tasks, do not rewrite them.

`agent --type mcp_tool` is removed in v0.85.0. It is the one deprecated surface
whose users are absent when it breaks: an interactive `tool call` warns on
every invocation right up to removal, but a scheduled task was warned once --
when it was created -- and then runs unattended. At removal it simply starts
failing on its next cron tick.

- `kbagent doctor` gains an `mcp_tool_tasks` check listing each affected task,
  the tool it calls and the native command that replaces it. A tool the parity
  map does not know reports `native_command: null` rather than inventing one.
  Filesystem only -- no API call, no MCP spawn -- and skips when there is no
  agents.json.
- `kbagent agent list` notes the affected tasks under the table and points at
  doctor. The marker is deliberately NOT in the Type cell: Rich truncated it to
  "DEPRECA…" there, which is worse than not flagging it. `--json` gains an
  additive per-task `deprecation` key, present only on affected tasks so every
  existing consumer sees a byte-identical payload.

Deliberately NOT included: automatic rewriting. ParityEntry carries only the
command name, no argument mapping, while mcp_tool params are the MCP tool's own
`input` dict -- different key naming and, for several tools, different
structure. Guessing the argv of a scheduled WRITE task that then runs
unattended is the worst place to be wrong; a bad rewrite beats a clear failure
in no scenario. Part 2 will emit a reviewable migration plan with explicit
TODOs instead.
@padak
padak force-pushed the feat/mcp-tool-task-detection branch from 0230a74 to d5a3ff7 Compare August 11, 2026 23:02
@padak
padak merged commit 946b36f into main Aug 11, 2026
4 checks passed
@padak
padak deleted the feat/mcp-tool-task-detection branch August 11, 2026 23:11
padak added a commit that referenced this pull request Aug 12, 2026
…#390) (#581)

Decision: `kbagent agent migrate-mcp-tasks` will not be built. Migrating a scheduled `--type mcp_tool` task before v0.85.0 is manual, or AI-assisted.

The reasoning that ruled out automatic rewriting also argues against a plan-generator: the parity map knows which native command replaces each tool but not how to map arguments -- mcp_tool params are the MCP tool's own `input` dict (`componentId`) against CLI flags (`--component-id`), with different shapes for several tools. An agent reading `--help` next to the task's actual input does that mapping better than any generator we could ship, with a human confirming. Detection already landed in #557, which is the part that could not be improvised.

gotchas.md now states plainly that no command exists and why, then gives the four-step recipe. keboola-expert.md gets a one-line trigger pointing at it, kept short because that file is at 61.6 KB of its 62 KB budget.

Two corrections from review are folded in. The recipe originally ended with `agent update --type cli_command --argv ...`, which does not work -- that command cannot change a task's action, so the documented path exited 2. It now says create-new, verify with one `agent run`, delete-old. That path changes the task ID, and `Trigger.task_id` chains tasks by ID, so the recipe also says to repoint any chained task BEFORE deleting or the chain breaks silently.

tests/test_mcp_migration_recipe.py pins the recipe to the CLI's real flags by reading `params` off the command object rather than scraping `--help` -- rendered help is wrapped and truncated by terminal width, so the first version of that test passed locally and failed on all three CI runners.

Residual risk, stated rather than hidden: there is no telemetry, so "probably nobody uses this" is a judgement about the user base, not a measurement. If it is wrong the failure mode is a silently dead cron task -- which `kbagent doctor` now surfaces, and that is why the decision holds without the command.
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