DEV Community

Felixwang007
Felixwang007

Posted on

My Code Reviewer Scored a Nonexistent Directory 100/100 and Exited 0

Every skill package I ship — for Claude Code, Cursor, Codex, and a couple of agent marketplaces — has to clear one gate before publishing: its own selftest must exit 0.

So this week I ran an audit over the 34 packages sitting in my skills folder to find out how many actually have that gate. The audit broke twice, in two different ways. Both breakages are the same bug wearing different clothes, and it's the bug that makes agent tooling untrustworthy the moment you put it in CI:

a success signal that isn't attached to work actually done.

Here's the whole thing, including the raw output.


Pass one: the audit script

The script was not clever. Walk each package, find a script mentioning selftest, run python scripts/<script>.py selftest, read the exit code.

Result:

  • 12 packages passed
  • 5 packages "failed"
  • 17 packages had no self-test at all

The five failures looked like this (output is Chinese, I'll translate):

FAKE(2) license-compliance-checker :: 错误: not a directory: ...\license-compliance-checker\selftest
FAKE(2) meeting-minutes-assistant   :: 找不到文件:selftest
Enter fullscreen mode Exit fullscreen mode
error: not a directory: ...\selftest
file not found: selftest
Enter fullscreen mode Exit fullscreen mode

Those five tools were fine. My harness was wrong. They implement the self-test as a --selftest flag, not as a positional subcommand. Five working self-tests (30/30, 54/54, 16/16, 40/40, 77/77 assertions) were reported as failures because the invocation contract was never part of the contract.

That's a false negative. Annoying, but it fails safe — the gate stays closed.

The bug that matters: 100/100 for a directory that doesn't exist

One of those five is a code-review tool. Called with a positional argument, it does not complain. It treats the argument as a path:

$ python scripts/code_review.py selftest
跳过不存在的路径: selftest
# 代码审查报告
- 审查范围: 0 个文件,0 行
- 发现问题: 0 个(致命 0 / 高 0 / 中 0 / 低 0)
- 健康分: 100/100 — ✅ 可以合并(按低优先级项择机清理)
EXIT=0
Enter fullscreen mode Exit fullscreen mode
skipped nonexistent path: selftest
# Code review report
- scope: 0 files, 0 lines
- findings: 0 (critical 0 / high 0 / medium 0 / low 0)
- health score: 100/100 — ✅ safe to merge
EXIT=0
Enter fullscreen mode Exit fullscreen mode

Read that again. The tool skipped a path that doesn't exist, scanned zero files, handed out a perfect score and the string "safe to merge", and returned success.

Its actual --selftest works fine — 54 assertions, 38 rules, 23 of them firing on dirty samples. The bug only shows up in scan mode, and only when the input is empty, missing or malformed. Which is exactly the state a path variable is in when a shell glob expands to nothing, a CI job runs on a docs-only diff, or an agent passes the wrong parameter name.

Now imagine the two ways this gets wired up:

  • code_review.py --selftest && deploy — fine.
  • code_review.py $CHANGED_FILES — with $CHANGED_FILES empty, you get "100/100, safe to merge" on a diff nobody looked at, green pipeline, exit 0.

And if there's an LLM agent on top reading that output, it will faithfully report "review passed." Not because the model is sloppy — because the tool lied first.

"Nothing wrong" and "nothing examined" are not the same result, and a tool that returns the same status for both cannot be part of a gate.


What the audit measured, precisely

Across 34 packages:

self-test contract packages
--selftest flag 5
selftest positional subcommand 12
no self-test 17

Two conventions in one folder. Any harness that guesses will produce false negatives (safe) and — the dangerous half — false positives, since a wrong invocation is silently treated as data rather than as an error. That's not a hypothetical: it's the output above.

Assertion counts in the 17 that do have a self-test: 129, 85, 77, 71, 54, 44, 44, 40, 30, 26, 21, 19, 18, 16, 15, 12, plus one boolean PASS. About 700 assertions total, and the useful ones are all two-sided:

  • positive cases — the check fires on a known-bad sample
  • negative cases — the check stays silent on a known-good sample

Without the second half, a linter that flags everything looks perfect. Without the first half, a linter that flags nothing looks perfect. Exit-code-only gates cannot tell those apart.

Concrete, from the 129-assertion SQL inspector:

  • DROP TABLE is a finding; DROP TABLE IF EXISTS is not.
  • 'no where clause here' inside a string literal is not a missing WHERE clause.
  • SELECT * inside a comment is not a finding.
  • os.environ["DB_PASSWORD"] is not a hardcoded credential; a literal password is.
  • BEGIN ... END in a PL/pgSQL function body is not an unclosed transaction.

Every one of those negative cases exists because the naive version of the rule got it wrong first.


Five rules for a gate that can't lie

  1. Assert on work done, not on exit status. Zero files scanned, zero rules run, zero tokens in — all need a distinct non-success status. "No input" ≠ "input clean."
  2. One documented self-test command per package. Write the exact command in the package's SKILL.md, and have the harness read it from metadata instead of pattern-matching the source. Guessing is how you get the false negative; silent acceptance is how you get the false positive.
  3. Break an assertion on purpose. A self-test nobody has seen fail is a print statement. Delete one rule, run the self-test, confirm it exits non-zero. Do this once per release, not once per project.
  4. Every check gets a positive and a negative case. One dirty sample, one clean sample, both asserted. This is the difference between a test suite and a smoke test.
  5. The publish/deploy step runs the gate itself and hard-fails. My publish scripts run the self-test before zipping and refuse on a non-zero exit; the numbers and the exact commands are recorded in an evaluation_cases.md next to the source. A gate that trusts a previous run's report is not a gate.

And one more, because it's the failure mode I keep hitting: state what the tool does not do. The review tool prints "static rules can only disprove, not prove — still verify permissions, concurrency and money precision by hand." That sentence is the most honest thing in the package. A gate that doesn't declare its blind spots gets trusted for them.


The remaining 17

Adding self-tests to them is mechanical now — the templates exist, and the ones already done took about an hour of real assertions each, not real research. I'm starting with the packages that get wired into automation, because a missing self-test in a package nobody runs is a backlog item, while a zero-input success path in a pre-commit hook is an incident waiting for a quiet Friday.

If you maintain skills, MCP servers, or CLI wrappers that agents call: the highest-value 30 minutes you can spend today is running your tool with an empty input and reading what it says. If it says "safe," "passed," or "0 issues," you've found the bug.


Where this stuff lives

If you've hit a zero-input success path in a tool you rely on, I'd like the exact output — that's the most useful kind of bug report.

Top comments (1)

Collapse
 
mythex profile image
Mythex •

The tools that get this right mostly do it with a dedicated exit code for "nothing examined", which might be the cleanest fix for the review tool:

  • pytest exits 5 when no tests were collected, not 0.
  • ESLint fails with "No files matching the pattern" unless you opt out with --no-error-on-unmatched-pattern.
  • Jest fails when it finds no tests unless you pass --passWithNoTests.

The nice property is that the caller decides. A docs-only CI job can treat "nothing to review" as fine on purpose, while a pre-commit hook treats it as an error, and neither has to parse the report.

The other cheap guard, especially with an LLM reading the output: print the scope before the verdict. "Reviewed 0 files" as the first line is much harder for an agent (or a tired human) to summarise as "review passed" than a score at the top with the scope buried in the middle.