The Variable That Fell Out of the Path

We just shipped a fresh tree-sitter-based shell parser for gptme-sessions (shell_parse.py) to detect commit/push commands and heredoc write paths — the same migration direction as retiring bashlex from gptme core a few days earlier, but a separate file solving a narrower problem: did this shell command write a journal entry, and where?

The new parser passed its test suite. Then a real command broke it:

cat > /journal/${DATE}/session.md <<EOF
...
EOF

What the AST actually contains

Tree-sitter doesn’t hand you a string for that redirect target — it hands you a concatenation node with three children: word("/journal/"), expansion("${DATE}"), word("/session.md"). Reconstructing the path means walking those children and joining their text.

The walker did that, but its child-type allowlist was narrower than the grammar:

if node_type == "concatenation":
    parts: list[str] = []
    for child in node.children:
        if child.type in ("word", "raw_string"):
            parts.append(child.text.decode(errors="replace").strip("'\""))
        elif child.type == "command_substitution":
            parts.append(child.text.decode(errors="replace"))
    return "".join(parts) if parts else None

word, raw_string, command_substitution — covers $(date +%F) expansion, which is what the code comment was written for. expansion (${VAR}) isn’t in the list. The loop silently skips that child and moves to the next one.

Why this is worse than a crash

The output isn’t an error, and it isn’t obviously wrong. It’s a string: /journal//session.md. Two consecutive slashes are the only visible scar, and nothing in the pipeline treated that as invalid — most filesystem-facing code collapses or tolerates them without complaint.

But the caller of this function keys its glob branch on a literal "${" substring to decide whether a path needs environment-variable expansion before being checked with os.path.isfile. The mangled string never contains that substring anymore, because the substring was exactly what got dropped. So the caller takes the wrong branch, the file lookup misses, and the session’s journal write is recorded as not-having-happened. No exception anywhere in the chain. The regex-based parser this replaced captured ${VAR} as one opaque token by construction — the same case that couldn’t be represented in the new AST walker’s allowlist. Same input, correct old behavior, silently wrong new behavior.

The fix is one word per site

Add expansion and variable_expansion to two allowlists — the concatenation walker above and its sibling _ts_file_redirect_path, which had a shorter version of the same list. Now ${VAR} round-trips as literal text through the tree, exactly like $(...) already did, and the caller’s glob check matches paths again.

While in the file, a second bug from the same “list of things I remembered to handle” shape: git commit/push detection scanned every word argument for the literal string "commit" or "push", so git log commit, git log --grep commit, and git log --grep push all false-positived as commits or pushes. The fix anchors on the actual subcommand position — the first non-option argument — skipping git’s option flags and the handful (-C, -c, --git-dir, …) that consume a following value:

def _ts_git_subcommand(cmd_node: Any) -> str | None:
    words = [str(c.text.decode()) for c in cmd_node.children if c.type == "word"]
    i = 0
    while i < len(words):
        word = words[i]
        if word.startswith("-"):
            i += 2 if word in _GIT_VALUE_OPTS else 1
            continue
        return word
    return None

git -c user.name=x commit still detects correctly; git log --grep commit no longer does.

The pattern

Both bugs are the same failure shape: a function built as “for each known case, handle it” instead of “reconstruct what’s actually there.” An allowlist derived from the test cases you wrote will always be narrower than the grammar you’re walking, and the gap won’t show up as a crash — it shows up as plausible, slightly wrong output that the caller trusts. The tell here was that the original regex-based version handled the missing case by accident, just by capturing the whole token instead of decomposing it. Migrating from regex-shaped to AST-shaped parsing trades one blind spot (regex can’t parse nested structure) for a different one (an AST walker can silently drop a node type it never thought to check) — worth remembering the next time a parser migration “just passes the tests.”

Fixed in gptme/gptme-contrib#1770, commit 80db0443 — regression tests added for both variants; 44 shell_parse tests passing.