Skip to content

register_docx_template removes the live tool before it knows the rebuild will work #192

Description

@dvejsada

Found while reviewing #191. Reproduced, not assumed.

register_docx_template() (docx_tools/dynamic_docx_tools.py:595) removes the existing tool first, then rebuilds it:

name = spec.get("name") if isinstance(spec, dict) else None
if name:
    safe_remove_tool(mcp, name)
    with _REG_LOCK:
        _REGISTERED_DOCX.pop(name, None)
return _register_single_template(mcp, spec, global_style_mapping)

_register_single_template() returns False without re-registering for a missing docx_path, a path with directory components, or — the reachable one — a source file that no longer resolves. The tool is then gone, replaced by nothing.

Reproduced

A registered template whose .docx was deleted on the volume since startup, then any re-registration:

BEFORE: ['letter']
AFTER : []

The loop in register_docx_template_tools_from_yaml() catches, logs and moves on, so nothing above it knows.

Why it matters

The re-registration paths are all admin actions about something else:

Action What the admin was doing
Save a template editing that template
Save the global style mapping (#161) editing a style
Adopt / clone / enable none of them about this template's file

So a tool that was working disappears as a side effect of an unrelated save. The only record is a log line, and docs/development/admin-ui.md is explicit that the admin never reads the server log.

Mitigation already in place

#191 compares the live names before and after and names what went missing, as a warning rather than a success (AdminContext.resync_docx_style_map). That is reporting the damage, not preventing it, and it covers the global-styles route only — an ordinary template save goes through AdminContext.register() and still loses the tool silently.

Suggested fix

Build first, swap second: have _register_single_template() do its validation and model construction before anything is removed, and only then remove and register. The remove exists so an edited template replaces its predecessor, which a build-then-swap still does — it just stops a failed build from counting as a delete.

Worth checking the email and pptx paths for the same shape while in there.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions