fix(chat-workflow): shrink tool_handler.py under its file-size ceiling (#14495)

[mrveiss]

Closes #14495

This unblocks every open PR. Dev_new_gui is currently red on the
code-quality required check because autobot-backend/chat_workflow/tool_handler.py
(4081 lines) exceeded its KNOWN_LARGE ceiling of 4063 in
scripts/check_python_file_size.py. Since Actions checks out the PR merge
commit, every open PR against Dev_new_gui inherits this failure regardless
of what it touches, until this lands.

Thinking Path

The squash-merge of #14470 (closing #14420) grew tool_handler.py past its
ceiling without lowering it. The ratchet in check_python_file_size.py only
ever shrinks — raising 4063 is exactly what it exists to refuse — so the fix
has to shrink the file.

A security review of this file had already flagged three oversized functions
(_dispatch_tool_call ~152 lines, _handle_browser_tool ~105 lines,
_process_single_command ~84 lines) as extraction candidates. Rather than
picking one to shave lines off arbitrarily, I looked for a cohesive unit
inside the biggest of the three, _dispatch_tool_call: it opens with six
sequential seam-enforcer calls (_enforce_forbidden_work,
_enforce_config_protection, _enforce_fact_forcing,
_enforce_pre_action_verifier, _enforce_repetition,
_enforce_work_item_approval) — each records a block/hold in
execution_results and returns a WorkflowMessage, or None to fall
through. None of the six touch self in their current form, which makes
them a clean, self-contained "dispatch guard" concern with zero coupling to
the rest of the mixin.

I moved all six, verbatim apart from mechanical changes, into a new module
chat_workflow/tool_dispatch_guards.py as plain functions, and left
ToolHandlerMixin with six thin delegating methods of the same name so
every existing call site and every existing test that calls or monkey-patches
mixin._enforce_forbidden_work(...) etc. keeps working unchanged.

What Changed

• New: autobot-backend/chat_workflow/tool_dispatch_guards.py (338 lines)
  — enforce_forbidden_work, enforce_config_protection,
  enforce_fact_forcing, enforce_pre_action_verifier, enforce_repetition,
  enforce_work_item_approval, moved out of tool_handler.py.
• autobot-backend/chat_workflow/tool_handler.py: the six _enforce_*
  methods are now one-line delegates to the module above; unused
  SENSITIVE_TOOLS import dropped (its only use moved with
  enforce_pre_action_verifier). 4081 → 3863 lines (218-line margin under
  the old ceiling, and the new one).
• scripts/check_python_file_size.py: KNOWN_LARGE for
  tool_handler.py lowered 4063 → 3863 — the ratchet only ever shrinks,
  and this tightens it to the size just achieved, not just far enough to pass.

Nothing was deleted as dead code — every line that left tool_handler.py
landed in tool_dispatch_guards.py; see the byte-diff evidence below.

One symbol needed a deferred import that didn't exist before:
enforce_work_item_approval calls _approval_category_for, which stays in
tool_handler.py (it's also imported there by chat_workflow/graph.py and
tests/unit/chat_workflow/test_governed_execution.py, so moving it would
widen the blast radius for no size benefit). tool_dispatch_guards.py
imports it back with a deferred, function-body import — the same pattern
graph.py already uses for this exact symbol — to avoid a load-time circular
import between the two modules.

Verification

Faithful-move evidence (diff-based, not asserted). I extracted the
original six function bodies from origin/Dev_new_gui, mechanically dedented
them one level and stripped the self, parameter (the same transform Python
requires to turn a method into a module function), and diffed against the
new module. After normalizing the only three cosmetic differences PEP8/the
move require — the ctx: "X" | None → ctx: "X | None" quoting style,
blank-line count between top-level functions vs. methods, and the one
_enforce_x → enforce_x cross-reference inside a docstring — the diff is
empty except for the one legitimately new deferred import
(enforce_work_item_approval's from chat_workflow.tool_handler import _approval_category_for, documented above and in a comment at the call site).
That import is the only non-mechanical line added across all six functions.

$ diff -B orig_enforcers_normalized.txt new_module_normalized.txt
IDENTICAL modulo blank-lines and the one documented deferred import

Deny-before-call ordering — unaffected on all three RBAC surfaces. This
PR does not touch _process_single_command, _handle_browser_tool, or
_handle_web_search_tool at all (grep confirms zero diff lines in those
functions). It also does not touch _dispatch_tool_call itself — only the
six methods it calls. Every one of the five if <enforcer>_msg is not None: yield <msg>; return guards in _dispatch_tool_call still runs, unchanged,
before the tool-specific branches (compose, respond, delegate, LLC
tools, and the uniform-builtin route that reaches
_process_single_command/_handle_browser_tool/_handle_web_search_tool).
The should_execute / _emit_before_tool_execute guard-before-call pattern
inside those three functions (lines ~2093, ~2435/2461ish, ~2668) is
byte-for-byte what it was in origin/Dev_new_gui — confirmed by git diff
showing no hunks in that range.

File size / ratchet:

$ wc -l autobot-backend/chat_workflow/tool_handler.py
3863 autobot-backend/chat_workflow/tool_handler.py

KNOWN_LARGE["autobot-backend/chat_workflow/tool_handler.py"] lowered to
3863, matching exactly — check_python_file_size.py's
_grandfathered_verdict passes a file that sits at its ceiling, and
repo_tests/python_file_size_ratchet_test.py::test_a_shrunk_file_must_lower_its_ceiling
requires exactly this.

Syntax: ast.parse() over the resulting tool_handler.py succeeds (no
codebase code executed — pure static parse, same technique the ratchet test
itself is not required to but the repo's own tooling relies on for git ls-files + AST checks elsewhere).

Test-compatibility check (static, not executed — per policy, I do not run
code from this codebase): grepped every test file that calls or
monkey-patches these six methods
(test_tool_handler_forbidden_work.py, test_tool_handler_config_protection.py,
test_tool_handler_fact_forcing.py, test_tool_handler_pre_action_verifier.py,
test_tool_handler_repetition_guard.py, test_governed_execution.py,
session_role_test.py, plus four files that stub these via
mixin._enforce_x = lambda *a, **k: None). All of them call through
mixin._enforce_x(...), which still resolves to the same-named method on
ToolHandlerMixin — now a delegate, but same signature, same return value.
None of them patch() an import path inside chat_workflow.tool_handler
for the enforcers' own dependencies (orchestration.agent_registry,
autobot_shared.config_guard, autobot_shared.fact_forcing_guard,
autobot_shared.pre_action_verifier_guard, autobot_shared.repetition_guard)
— every patch in those tests targets the origin module directly (e.g.
patch("autobot_shared.pre_action_verifier_guard.PreActionVerifier.verify")),
which resolves identically whether the deferred import executes inside
tool_handler.py or tool_dispatch_guards.py.

Conflict check against PR #14492 (in flight, same file, closes #14469 /
threads RBAC through _process_single_command, _handle_browser_tool,
_handle_web_search_tool): ran a real local 3-way merge of this branch
with origin/issue-14469 in a scratch worktree (not #14492's own branch —
read-only against the remote ref, discarded after). Result: clean,
zero-conflict automatic merge — git merge --no-commit --no-ff origin/issue-14469 reported "Automatic merge went well". The two PRs edit
disjoint regions of the file (this PR: the six _enforce_* bodies + one
import block; #14492: _process_single_command, _handle_browser_tool,
_handle_web_search_tool, _builtin_route, _dispatch_execute_command, and
a different two-line addition to the same import block). Merged result
parses clean at 3922 lines (3863 + #14492's own net +59).

Sequencing note for #14492 (not a blocker, but worth doing in order):
#14492 cannot merge before this PR regardless of order chosen, because the
base is already red for every open PR including #14492 — that's the bug
this PR fixes. Once this PR merges, #14492 will need to rebase onto the new
tip; at 3863 (this PR's ceiling) + #14492's own net +59 lines = ~3922, its
rebased branch will re-trip audit-ceilings (3922 > 3863) purely from its
own diff — the ratchet is working as designed: any PR that grows a
grandfathered file has to account for it. The fix for #14492 is a one-line
addition to their own diff: set KNOWN_LARGE["autobot-backend/chat_workflow/ tool_handler.py"] to their post-rebase actual line count once rebased (e.g.
~3922) — that is still comfortably below the frozen ratchet baseline of
4063 in repo_tests/python_file_size_ratchet_test.py::RATCHET_BASELINE, so
test_no_ceiling_may_be_raised does not object; only
test_recorded_ceilings_match_the_files_today requires it (ceiling must
equal the file's actual measured size, exactly, at all times). Recommended
order: merge this PR first, then rebase #14492 and have it adjust the
ceiling to its own new count as part of that rebase.

Model Used

Claude Opus 5 (1M context)

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!