[review-only] Python port of Smithers + recursive meta-workflow

[lluisinthedesert]

Summary

Draft PR for cloud-reviewer feedback (Codex / Greptile / Devin). Do not merge — port/resume is the long-lived Python-port branch on this fork; this PR exists to give reviewers a surface to comment against.

34 commits ahead of main. Three threads of work:

1. smithers_py — Python port of upstream Smithers (hand-coded)

Brings the Python port from ~30% to ~60% of upstream's component surface. Added subsystems:

• smithers_py.memory (1,304 LoC, 18 tests) — cross-run memory, three layers (working / messages / semantic recall), four namespaces, pluggable embedding adapter (OpenAI default + Null for tests), three processors (TtlGC, TokenLimiter, Summarizer).
• smithers_py.tools (1,467 LoC, 35 tests) — read/write/edit/grep/bash sandboxed tools + define_tool factory + persisted ts_tool_calls log. Path containment, network policy block-list, output truncation.
• smithers_py.scorers (1,380 LoC, 27 tests) — schema_adherence / latency / relevancy / toxicity / faithfulness + llm_judge + create_scorer factory. Sampling (all/ratio/none). Persists to ts_scores.
• smithers_py.cache (562 LoC, 18 tests) — cache.by + version + schema-signature key derivation. Three scopes (run/workflow/global). TTL + lazy sweep.

Existing 724 tests still pass. New total: 822 passing.

See PARITY_PLAN.md for the per-subsystem scope.

2. examples/smithers-port-py/ — recursive meta-workflow

A Smithers workflow that uses Smithers to port itself to Python. Two variants:

• workflows/port-subsystem.tsx — API mode, parallel fan-out per file via AnthropicAgent. Cheap (~$0.20/subsystem) but suffers cross-file naming drift.
• workflows/port-subsystem-cli.tsx — CLI mode, single Task with ClaudeCodeAgent + file tools. Agent reads what it just wrote before the next file, runs pytest against the generated tests, iterates. Pattern matches Cory's bun-port-smithers.

Plus the ongoing-sync meta-workflow (workflow.tsx) that watches upstream PRs and classifies them as port / skip / port-with-replacement, with parity verification via the existing examples/wire_compat/ cross-runtime test.

3. smithers_py_meta/ — recursive port proof

Generated by port-subsystem-cli.tsx from markdown specs. 4 of 5 subsystems shipped this way:

Subsystem	Path	LoC	Tests	Workflow run
serve	smithers_py/serve/	756	12/12	port-serve-cli-v2
memory	smithers_py_meta/memory/	1004	16/16	port-mem-cli-v3
tools	smithers_py_meta/tools/	450	35/35	port-tools-cli
cache	smithers_py_meta/cache/	669	21/21	port-cache-cli

Scorers run was discarded — the agent overwrote the hand-coded version despite the explicit pythonTargetDir directive. See META_WORKFLOW_PROOF.md for the failure-mode writeup and the spec-text fix.

Per-subsystem cost: ~$0.30-0.80 on Sonnet 4.5 (≤$1/subsystem acceptance). Tests pass independently for both hand-coded and meta-generated versions.

What I want reviewed

In order of priority:

1. Idiom + correctness of the hand-coded subsystems — memory / tools / scorers / cache. Anything Pythonic-but-wrong, missing edge cases, or schemas that drift from upstream?
2. The meta-workflow design — is port-subsystem-cli.tsx the right shape for porting whole subsystems? Anything obvious to harden in the prompt or workflow scaffold?
3. Comparison artifact — smithers_py_meta/<subsystem> vs smithers_py/<subsystem> for memory / tools / cache. What did the agent miss that's load-bearing?
4. The recursive-port narrative — does META_WORKFLOW_PROOF.md hold up as the canonical write-up?

Test plan

• cd smithers_py && .venv/bin/python -m pytest -q — 822 passing baseline
• examples/smithers-port-py/scripts/verify-subsystem.sh serve — 12/12 meta-workflow output
• examples/smithers-port-py/scripts/verify-subsystem.sh memory smithers_py_meta — 16/16
• examples/smithers-port-py/scripts/verify-subsystem.sh tools smithers_py_meta — 35/35
• examples/smithers-port-py/scripts/verify-subsystem.sh cache smithers_py_meta — 21/21
• Cross-runtime parity: examples/wire_compat/ 5/5 row diffs match TS↔Python

🤖 Generated with Claude Code

Greptile Summary

This PR ports the Smithers TypeScript runtime to Python (smithers_py), adding four new subsystems — memory, tools, scorers, and cache — totalling ~4,200 lines of hand-coded Python, plus a parallel smithers_py_meta set generated by a recursive meta-workflow using ClaudeCodeAgent.

• smithers_py/tools: sandboxed read/write/edit/grep/bash built-ins with path-containment and network-block-list enforcement; two correctness bugs found (rg flag injection via un-guarded pattern, CRLF mismatch in the pure-Python unified-diff applier).
• smithers_py/cache: SQLite-backed output cache with TTL and scope-prefix purge; purge_scope uses an unescaped LIKE wildcard that can delete entries from unintended scopes when a scope_id contains % or _.
• smithers_py/serve/auth.py: bearer-token comparison uses Python != instead of hmac.compare_digest, making it vulnerable to timing-based token enumeration.

Confidence Score: 3/5

Several concrete bugs in the core tool and cache paths need fixing before the hand-coded subsystems are production-ready.

The edit tool silently fails on CRLF files, the grep tool misfires on patterns starting with -, cache purge can delete unrelated entries on scope_ids with %, and the auth handler leaks timing information. These are real defects on actively-used code paths.

smithers_py/tools/builtins.py (two bugs), smithers_py/cache/init.py (LIKE wildcard escaping), and smithers_py/serve/auth.py (timing-safe comparison)

Security Review

• Timing-sensitive token comparison (smithers_py/serve/auth.py line 31): token != auth_token short-circuits on the first differing byte; an attacker with enough requests can use response-time differences to recover the token. Fix: hmac.compare_digest.
• rg flag injection (smithers_py/tools/builtins.py): a grep pattern starting with - is passed to rg without a -- end-of-options marker, causing it to be interpreted as a flag rather than a search pattern.

Important Files Changed

Filename	Overview
smithers_py/tools/builtins.py	Implements read/write/edit/grep/bash built-in tools. Two bugs: rg receives pattern without a -- sentinel (flag injection), and _apply_unified_diff uses .rstrip("\n") which silently mismatches CRLF files.
smithers_py/tools/sandbox.py	Path containment and network-command block-list helpers. Symlink-walk defense is solid. Block list mirrors upstream and intentionally leaves some vectors (nc, ssh) unblocked.
smithers_py/cache/init.py	SQLite-backed task output cache with TTL and scope-prefix purge. purge_scope builds a LIKE pattern without escaping %/_ wildcards, so a scope_id containing those characters silently deletes entries from other scopes.
smithers_py/memory/store.py	Three-layer SQLite memory store (working/messages/semantic recall). Correct for single-process asyncio use. save_message SELECT MAX + INSERT is not transactionally isolated for multi-process writers.
smithers_py/memory/processors.py	TtlGarbageCollector, TokenLimiter, and Summarizer. Correct behavior; processors reach into store._connect() directly which is a tight coupling concern.
smithers_py/serve/auth.py	FastAPI bearer-token dependency. Token comparison uses Python != (timing-sensitive); should use hmac.compare_digest for a production-facing service.
smithers_py/scorers/builtins.py	Five built-in scorers plus llm_judge and create_scorer factories. Logic is clean and well-matched to upstream surface.
smithers_py/scorers/runner.py	Concurrent scorer execution and persistence to ts_scores. Contains dead code: _one inner function is defined but never called; actual execution goes through _runWithCapture.
smithers_py/tools/define.py	define_tool factory and ToolCallLog persistence. Well-structured; idempotency-key warning at construction time is a nice touch.
smithers_py/memory/embeddings.py	OpenAI and Null embedding adapters plus pack/unpack helpers. Model-mismatch guard in recall() correctly skips vectors from a different model.

_{Reviews (1): Last reviewed commit: "META_WORKFLOW_PROOF.md: fold in the appr..." | Re-trigger Greptile}

Greptile also left 6 inline comments on this PR.

[greptile-apps]

The grep tool passes pattern directly as the third positional argument to rg without a -- end-of-options sentinel. Any pattern that starts with - (e.g., --files, -l, -g *.py) is silently interpreted as an rg flag rather than a regex pattern, producing wrong output or unintended directory listings instead of raising a ToolError.

Suggested change

	proc = await asyncio.create_subprocess_exec(
	"rg",
	"-n",
	"--no-heading",
	pattern,
	resolved,
	proc = await asyncio.create_subprocess_exec(
	"rg",
	"-n",
	"--no-heading",
	"--",
	pattern,
	resolved,

[greptile-apps]

Context and deletion lines are matched with .rstrip(" "), but original.splitlines(keepends=True) preserves endings on CRLF files. For a CRLF line "foo ", .rstrip(" ") yields "foo " which never equals the patch's expected "foo", so every hunk returns None — a silent patch failure. The same issue affects the empty-line check on line 212. Fix: use .rstrip(" ") at all three sites.

Suggested change

	if body_line.startswith(" "):
	# Context line — must match.
	expected = body_line[1:]
	if cursor >= len(lines) or lines[cursor].rstrip("\n") != expected:
	return None
	result.append(lines[cursor])
	cursor += 1
	elif body_line.startswith("-"):
	# Deletion — must match.
	expected = body_line[1:]
	if cursor >= len(lines) or lines[cursor].rstrip("\n") != expected:
	return None
	cursor += 1
	elif body_line.startswith("+"):
	# Addition.
	added = body_line[1:]
	result.append(added + "\n")
	else:
	# Empty line in the patch body. Treat as context for
	# tolerance with patches that omit the leading space.
	if cursor < len(lines) and lines[cursor].rstrip("\n") == "":
	if body_line.startswith(" "):
	# Context line — must match.
	expected = body_line[1:]
	if cursor >= len(lines) or lines[cursor].rstrip("\r\n") != expected:
	return None
	result.append(lines[cursor])
	cursor += 1
	elif body_line.startswith("-"):
	# Deletion — must match.
	expected = body_line[1:]
	if cursor >= len(lines) or lines[cursor].rstrip("\r\n") != expected:
	return None
	cursor += 1
	elif body_line.startswith("+"):
	# Addition.
	added = body_line[1:]
	result.append(added + "\n")
	else:
	# Empty line in the patch body. Treat as context for
	# tolerance with patches that omit the leading space.
	if cursor < len(lines) and lines[cursor].rstrip("\r\n") == "":

[greptile-apps]

purge_scope builds the LIKE pattern as {scope}:{scope_id}:% without escaping SQLite LIKE wildcards. If scope_id contains a % or _ character (or a workflow name does), the DELETE will match unintended rows in adjacent scopes. The LIKE clause needs ESCAPE '\\' and the prefix must have % and _ escaped.

Suggested change

	prefix = f"{scope}:{scope_id}:"
	with self._connect() as db:
	cur = db.execute(
	"DELETE FROM ts_cache WHERE key LIKE ?",
	(prefix + "%",),
	)
	escaped_prefix = (
	f"{scope}:{scope_id}:"
	.replace("\\", "\\\\")
	.replace("%", "\\%")
	.replace("_", "\\_")
	)
	with self._connect() as db:
	cur = db.execute(
	"DELETE FROM ts_cache WHERE key LIKE ? ESCAPE '\\'",
	(escaped_prefix + "%",),
	)

[greptile-apps]

The bearer-token comparison uses != (Python string inequality), which short-circuits on the first differing byte. An attacker with enough requests can use response-time differences to infer the token character by character. hmac.compare_digest runs in constant time regardless of where the strings diverge.

Suggested change

	if token != auth_token:
	import hmac
	if not hmac.compare_digest(token or "", auth_token):

[greptile-apps]

The inner function _one is defined but never called — _runWithCapture performs the actual concurrent execution. The dead code is mildly confusing because _one references the outer input variable from the enclosing scope, which makes it look like it might be wired up somewhere.

Suggested change

	async def _one(key: str, binding: ScorerBinding) -> tuple[str, ScoreResult]:
	result = await binding.scorer.score(input)
	return key, result
	results: dict[str, ScoreResult] = {}
	results: dict[str, ScoreResult] = {}

[greptile-apps]

Both TokenLimiter.process and Summarizer.process reach into store._connect() to execute the delete + re-insert batch atomically. This works today but breaks if MemoryStore ever switches its storage backend or refactors _connect. A cleaner contract would be a package-internal _replace_thread_messages(thread_id, messages) method on MemoryStore that these processors call, keeping connection management inside the store.