fix(agent-tools): wire channel_service into hard_delete_channels (#1290)#1294
Conversation
🔍 Local review (cycle 1)Reviewed locally (
FIX → addressed in next commit: added dev-mode gate after confirmation, parity with CLI/web, TDD + mutation-verified. |
The hard_delete_channels tool built FilterDeletionService(db) without a
channel_service, so every call hit `raise RuntimeError("hard_delete requires
channel_service (dev mode)")`, was swallowed by the generic except, and was
returned to the user as "Ошибка удаления каналов". The operation could never
succeed regardless of arguments.
Existing tests mocked FilterDeletionService entirely, so the real path was
never exercised — that is why the bug went undetected since the tool's
introduction.
Fix: build the service with a ChannelService (pool=None, queue=None —
delete() is a pure DB op, identical to the CLI's _build_deletion_service in
src/cli/commands/filter.py). Adds a regression test that drives the real
service/DB (no service mock); mutation-verified red without the fix.
Co-Authored-By: Claude <noreply@anthropic.com>
…bled (#1290) Codex local review caught that the wired service made the irreversible hard-delete reachable from the agent without the dev-mode guard that BOTH the CLI (src/cli/commands/filter.py:227) and web (src/web/filter/handlers.py:31) enforce. With confirm=true an agent call could permanently erase channels in production despite developer mode being disabled — the "real dev-gate" the issue's acceptance criteria asked for was missing. Add the gate after require_confirmation (parity with CLI): refuse with a clear message unless agent_dev_mode_enabled == "1". Tests: - test_dev_mode_off_rejects_hard_delete: off → refusal, channel preserved (mutation-verified red without the gate) - test_real_service_path_actually_deletes: updated to enable dev-mode - mock tests updated to stub get_setting -> "1" Co-Authored-By: Claude <noreply@anthropic.com>
721d237 to
86c24d8
Compare
📋 Review summary — all cycles (local mode)PR #1294 —
Totals: 1 FIX (resolved), 1 SKIP (out-of-scope pre-existing), 0 UNVERIFIED. Issue #1290 acceptance criteria:
Local mode does not auto-merge. CI is green on |
Closes #1290
What
hard_delete_channelswas dead since introduction: the tool builtFilterDeletionService(db)withoutchannel_service, so every call hitraise RuntimeError(\"hard_delete requires channel_service (dev mode)\"). The error was swallowed by the genericexceptand returned to the user as\"Ошибка удаления каналов\". The operation could never succeed regardless of arguments.Root cause
src/agent/tools/filters.py—FilterDeletionService(db)missing thechannel_serviceargument;hard_delete_channels_by_pksraises immediately when it isNone.The existing tests mocked
FilterDeletionServiceentirely, so the real path was never exercised — that is exactly why the bug went undetected.Fix
Wire a
ChannelServicewhen building the service, mirroring the CLI's_build_deletion_serviceinsrc/cli/commands/filter.py.ChannelService.delete()is a pure DB op (works withpool=None, queue=None), so this matches the proven CLI path and restores agent/CLI parity.Test (TDD, mutation-verified)
test_real_service_path_actually_deletesdrives the realFilterDeletionService+:memory:DB (no service mock). Confirmed red on currentmain(Ошибка удаления каналов: hard_delete requires channel_service), green after the fix.FilterDeletionService(db)) turns the test red again.Tests run
tests/test_agent_tools_filters.py+tests/test_filter_deletion_service.py— 60 passedtest_agent_tool_smoke_contract.py,test_tool_permissions_autoderive.py,test_agent_tools_init.py,test_agent_tools_registry.py— 103 passed (registry/parity intact)ruff check— cleanNotes / risks
require_confirmation+destructiveHint) remains the real guard.