DEV Community

Enjoy Kumawat
Enjoy Kumawat

Posted on

My Commit-Message Script Has an Empty-Diff Guard. My MCP Tool Version Doesn't — and It Doesn't Fail Loud.

Three days ago I wrote about git_commit.py and generate_commit_message — the standalone script and the MCP tool in server.py that do the exact same job, calling the exact same claude -p subprocess with the exact same attribution filter. The conclusion back then was that they're provably identical code wearing two different interfaces. That conclusion was wrong in one specific place, and a commenter is the reason I went back to check.

On that same article, someone pointed out that -> str as a return type doesn't tell you what happens on an empty diff. I filed it away as "probably fine" and moved on. I shouldn't have. -> str is exactly the kind of type signature that hides a missing guard clause, because Python will happily let you return any string at all, including one that reads like a valid answer but isn't.

Here's git_commit.py's guard, which I'd forgotten was doing real work:

diff = subprocess.check_output(["git", "diff", "--staged"], text=True)
if not diff.strip():
    print("Nothing staged. Run `git add` first.")
    raise SystemExit(1)
Enter fullscreen mode Exit fullscreen mode

Nothing subtle about it. Empty diff, clear message, non-zero exit, done. Anything downstream — a shell script, a git hook, a human — gets an unambiguous signal that nothing happened.

Here's the MCP tool version, as it existed until today:

@mcp.tool()
def generate_commit_message(diff: str) -> str:
    """Generate a Conventional Commits message from a git diff string."""
    return _claude(
        diff,
        system=(
            "You are a git commit message generator. "
            "Output ONLY the commit message — no explanation, no markdown, no quotes. "
            "Follow Conventional Commits: type(scope): subject. "
            "Types: feat, fix, docs, style, refactor, test, chore. "
            "Subject: imperative, lowercase, max 72 chars."
        ),
    )
Enter fullscreen mode Exit fullscreen mode

No check. Whatever diff is, it goes straight into _claude(), which shells out to claude -p and returns whatever comes back, filtered only for attribution lines. I wanted to know what "whatever comes back" actually looks like for an empty string, instead of guessing, so I called it directly:

>>> _claude("", system=SYSTEM)
"There are no staged or unstaged changes in the repository, so there's nothing to generate a commit message for."
Enter fullscreen mode Exit fullscreen mode

Not a crash. Not a fabricated commit message either — my first guess was that the model would hallucinate something plausible-looking from nothing, so I tested that too, feeding it a diff for a file that doesn't exist anywhere on disk (totally_fake_file_xyz.py, a wombat_teleporter function I made up on the spot):

>>> _claude(FAKE_DIFF, system=SYSTEM)
'feat: add wombat teleporter stub function'
Enter fullscreen mode Exit fullscreen mode

That second test mattered more than I expected. It rules out the theory that claude -p is quietly running git status itself and answering from the real repo state instead of the string you handed it — if it were, the fake-file diff would have produced the same "nothing to see here" answer as the empty one. It didn't. The tool trusts the diff argument completely, for better and worse: pass it garbage that looks like a diff, get a commit message about the garbage; pass it nothing, get a sentence about there being nothing.

That sentence is the actual bug. It's not an error. It's not formatted like a Conventional Commit. It's also, if you're not looking closely, indistinguishable from a valid tool response, because the function's contract is -> str and this is very much a str. Nothing about the return value tells a caller "this failed." A workflow that pipes this tool's output straight into git commit -m — which is the entire point of the tool existing — would commit a full English sentence explaining that there was nothing to commit, as the actual commit message. That's a worse outcome than the script's hard stop, not a better one, and it would look, in a git log, exactly like something a person wrote on purpose.

The fix is the same three lines the script already has, just returned instead of printed:

@mcp.tool()
def generate_commit_message(diff: str) -> str:
    """Generate a Conventional Commits message from a git diff string."""
    if not diff.strip():
        return "ERROR: empty diff — nothing to generate a commit message from."
    return _claude(diff, system=SYSTEM)
Enter fullscreen mode Exit fullscreen mode

An ERROR: prefix isn't elegant, but it's checkable — a caller that cares can .startswith("ERROR:") before doing anything destructive with the string, and one that doesn't care at least gets a return value that no longer masquerades as a real commit subject line.

The part I keep turning over is that this was never a mystery. The gap was named out loud, in a comment, on the article that should have caught it, three days before I actually checked. "Same code" was true for the half of the function that calls the model. It was never true for the half that decides whether to call the model at all — and that's the half a type signature like -> str will never tell you about. If a guard clause only exists in one of two call paths that are supposed to be equivalent, the type checker won't flag it, the tests won't flag it (there weren't any), and the only thing that did was someone reading closely enough to ask a question I answered too fast.

Top comments (0)