docs(adapters): fix add-a-harness recipe - #1056
Conversation
Signed-off-by: Polly Labs <pollychen.lab@gmail.com>
potiuk
left a comment
There was a problem hiding this comment.
LGTM — all three corrections check out, and one of them fixes a command that could never have worked.
The broken invocation was genuinely broken
python3 -m symlink_lint fails outright:
No module named symlink_lint.__main__; 'symlink_lint' is a package and cannot be directly executed
tools/symlink-lint/src/symlink_lint/ contains only __init__.py — no __main__.py — so -m has nothing to execute. The shipped console script is declared ([project.scripts] symlink-lint = "symlink_lint:main") and runs clean. Anyone following the recipe literally hit a traceback at the first validation step; both occurrences are now fixed.
Worth noting why this was easy to miss: the sibling agent_guard package does have a __main__.py, so python3 -m agent_guard works. The two tools look interchangeable and are not.
The cli() description matches the code
Verified against tools/agent-guard/src/agent_guard/__init__.py:
554 def opencode_main() -> int
584 def kiro_main() -> int
729 def cli(argv: list[str] | None = None) -> int
So "one thin *_main() per hook-shaped harness, selected by a cli() router near the end of the file" is exactly right, and the new step 4 — add a --<runtime> branch in cli() — is the step the old text omitted entirely. That is the part that actually helps someone adding a harness.
On dropping __main__.py from the tree
I flagged this to myself first, because the file does still exist, and then changed my mind on the evidence. Recording the reasoning since the deletion looks wrong at a glance:
The old row described __main__.py as "CLI entry point (stdin JSON → stdout JSON)", and that is not how the guard actually runs. tools/agent-guard/README.md says the hook is invoked as python3 <path>/agent_guard/__init__.py, never via uv run, shipped as "a single self-contained file" — and __init__.py duly ends with if __name__ == "__main__": raise SystemExit(cli()). So __init__.py is the real entry point; __main__.py is a three-line python -m shim that the deployed path never touches. Removing the row deletes a claim that was actively misleading, in a document whose job is to tell a contributor where things live.
Only nit, and genuinely optional: __main__.py is still on disk, so someone listing the directory sees a file the tree does not mention. A single row — ← python -m shim → cli() — would cost nothing and pre-empt the "what is this for?" question. Silence is defensible for a recipe rather than an inventory, so take it or leave it.
This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.More on how Apache Magpie handles maintainer review:
CONTRIBUTING.md § Opening a pull request.
|
@potiuk thanks |
Summary
python3 -m symlink_lintexamples with the shippedsymlink-lintconsole script.tools/agent-guard/src/agent_guard/__init__.pylayout.*_main()adapter plus acli()routing branch, matching the existing OpenCode and Kiro paths.Fixes #997
Validation
uv run --project tools/symlink-lint symlink-lintuv run --directory tools/agent-guard --project . --group dev pytestuv run --project tools/skill-and-tool-validator --group dev skill-and-tool-validatebash -n tools/spec-loop/loop.sh && bash -n tools/spec-loop/lib.shuv run prek run --all-files