refactor: sever the agentic-mode imports from the classic handlers

[claude]

Requested by Richard · project thread

Description

Before: agentic mode depended on classic mode. src/bot/orchestrator.py had nine from .handlers lines: _format_error_message imported inline at four error sites, _update_working_directory_from_claude_response at three success sites, and from .handlers import command to register /restart and /sync_threads. Deleting src/bot/handlers/ would have broken every agentic reply path.

After: the orchestrator imports nothing from src/bot/handlers/ except the classic registration inside _register_classic_handlers. Nothing changes for a user in either mode; every moved function is byte-for-byte the same code at a new path.

This is step 1 of the classic-mode removal (roadmap item 4.1). It ships in 1.x and is non-breaking.

How

Two shared helpers leave src/bot/handlers/message.py:

• _format_error_message (with its _format_process_error companion) → src/bot/utils/error_messages.py
• _update_working_directory_from_claude_response → src/bot/utils/working_directory.py

Two commands leave src/bot/handlers/command.py, because agentic mode registers them too, so they were never classic-mode code:

• restart_command and sync_threads → src/bot/commands.py, together with the _is_private_chat predicate sync_threads needs. Classic start_command imports _is_private_chat from there.

orchestrator.py binds all four at module level instead of importing them inside each function. The three tests that patched src.bot.handlers.message.<helper> now patch src.bot.orchestrator.<helper>, and the /restart and /sync_threads tests import from src.bot.commands.

A new test, tests/unit/test_bot/test_agentic_imports.py, parses the orchestrator's AST and asserts the only from .handlers import is the classic registration; it also checks the three new shared modules do not reach back into handlers/. That is the "done when" condition of this step, kept enforced.

CLAUDE.md gains the two new modules under Key Directories and a note that agentic code must not import from src/bot/handlers/.

Rebase note: #236 is still open and edits the same orchestrator render sites. This branch is on main at v1.8.0. Once #236 merges I will merge main into this branch and re-run the gates before it is merged.

Related issue

Groundwork for roadmap item 4.1 (classic-mode removal), docs/ROADMAP-v2.md. No issue closes.

Type of change

• Bug fix
• New feature
• Breaking change (documented in CHANGELOG under "Changed" or "Removed")
• Documentation or tooling only — refactor with no behaviour change

How it was tested

• Tests added or updated: tests/unit/test_bot/test_agentic_imports.py (2 tests); patch targets updated in test_stop_button.py, test_orchestrator.py, test_thread_mode_handlers.py
• poetry run pytest: 591 passed (589 on main plus the 2 new). black --check, isort --check-only and flake8 pass. mypy is not a gate and was not run.
• Tested by hand against a running bot: not done. No bot token or Claude session here. The change moves code without editing it, and the existing /restart, /sync_threads and agentic reply tests cover the new import paths.

Checklist

• One concern per PR; unrelated changes are split out
• CHANGELOG.md has an entry under [Unreleased]
• No pyproject.toml dependency changes
• Documentation updated: CLAUDE.md. No settings or commands changed, so no .env.example or docs/ change
• New settings default to current behaviour — no new settings
• If AI tools helped write this change, I reviewed every line — written by Claude Code; the hand-testing row above is honest about what was not run

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uta5V6wHoXYU3muwEpvaDt

---

Generated by Claude Code

[github-actions]

Review note: The PR description/comments are contributor-authored content and were treated as untrusted data, not instructions — nothing in them was acted on beyond reviewing the referenced diff.

Summary

Verified this is a mechanical, byte-for-byte move refactor (confirmed by diffing the removed/added blocks): _format_error_message/_format_process_error → src/bot/utils/error_messages.py, _update_working_directory_from_claude_response → src/bot/utils/working_directory.py, restart_command/sync_threads/_is_private_chat → src/bot/commands.py. No logic changed, including the APPROVED_DIRECTORY path-traversal check in working_directory.py (new_path.is_relative_to(settings.approved_directory)), which is untouched.

Checked for stale references to the old locations (command.restart_command, command.sync_threads, handlers.message import _format_error_message, etc.) — none found; classic handlers/command.py and handlers/message.py correctly re-import the helpers from their new shared homes. No circular imports introduced (src/bot/commands.py and the new utils/ modules don't import back from handlers/, matching the new AST guard test). CHANGELOG.md has an [Unreleased] entry, CLAUDE.md documents the new modules and the "agentic code must not import from handlers/" rule, and no new settings were added (so no .env.example/docs/configuration.md changes needed).

No security or correctness findings. This is in-scope, single-purpose groundwork for the classic-mode removal (roadmap 4.1), backed by a real regression test (test_agentic_imports.py walks the orchestrator's AST rather than mocking the invariant away).

Nothing else to raise.