fix(cli): validate follower and inbox options before querying

[devYRPauli]

Problem

Four options accept any text and turn it into NaN. typeof NaN is "number", so NaN passes every typeof x === "number" guard and reaches the query layer. The command then returns nothing and exits 0, which is indistinguishable from "no matches".

command	options
search dms <query>	--min-followers, --max-followers
dms list	--min-followers, --max-followers
inbox	--min-score, --limit

The handlers do options.minFollowers ? Number(options.minFollowers) : undefined. A non-empty string is truthy, so --min-followers 10k yields NaN.

For the DM commands, listDmConversations does typeof minFollowers === "number" (true for NaN), then Math.max(NaN, 0) is NaN, then binds it:

where += " and p.followers_count >= ?";
params.push(effectiveMinFollowers);

SQLite returns zero rows for that comparison.

For inbox, listInboxItems declares minScore = 0 and limit = 20 as default parameters. Default parameters do not fire for NaN, only for undefined. So NaN flows into item.score >= lowSignalFloor, which is false for every item, and into .slice(0, limit), which returns []. src/lib/inbox.ts has no Number.isFinite guard anywhere. A triage command silently reports an empty inbox while unreplied mentions and DMs are waiting.

A realistic typo is enough: --min-followers 10k, --min-score high.

Change

Each of the four options is parsed with parseNonNegativeIntegerOption, the helper already defined in src/cli/command-context.ts and already used for other options in these same files. The handler returns early when the value is rejected, before any read model runs. Follower counts, the inbox rank and the inbox limit are all non-negative integers.

Scope: the influence-score options are NOT changed, on purpose

--min-influence-score and --max-influence-score look like the same defect and are not. getMinFollowersForInfluenceScore and getMaxFollowersForInfluenceScore (src/lib/dm-read-model.ts) both begin with:

if (!Number.isFinite(score)) return undefined;

so NaN is already rejected there and never reaches SQL through that path. I checked before writing the patch and left them alone rather than claim a fix that is not one.

Real behavior proof

Real CLI, built from this branch and from origin/main, run against an empty local archive.
Exit codes are the process exit codes.

BEFORE (origin/main)
$ birdclaw search dms query --min-followers 10k
[]
exit=0
$ birdclaw dms list --max-followers abc
[]
exit=0
$ birdclaw inbox --min-score high
{ items: [], stats: { total: 0, openai: 0, heuristic: 0 } }
exit=0
$ birdclaw inbox --limit 3.5
{ items: [], stats: { total: 0, openai: 0, heuristic: 0 } }
exit=0

AFTER (this branch)
$ birdclaw search dms query --min-followers 10k
{"error":"--min-followers must be a non-negative integer"}
exit=1
$ birdclaw dms list --max-followers abc
{"error":"--max-followers must be a non-negative integer"}
exit=1
$ birdclaw inbox --min-score high
{"error":"--min-score must be a non-negative integer"}
exit=1
$ birdclaw inbox --limit 3.5
{"error":"--limit must be a non-negative integer"}
exit=1

$ birdclaw inbox --limit 5
{ items: [], stats: { total: 0, openai: 0, heuristic: 0 } }
exit=0

Before, every bad value returned an empty result with exit 0, which a caller cannot tell apart from "no matches". After, each one is rejected with a structured error and a nonzero exit, and a valid value still runs.

Note on the build: bun run cli does not start here. The repo pins a Bun 1.4 canary and my local Bun is 1.3.14, which has no node:sqlite. I bundled src/cli.ts with the repository's own esbuild and ran it on Node 24, which does have node:sqlite. No source file was changed to produce this.

Proof

node node_modules/vitest/vitest.mjs run src/cli.test.ts
Tests  78 passed (78)

Source changes reverted, tests kept:

FAIL  rejects invalid search dms --min-followers values before reading
FAIL  rejects invalid search dms --max-followers values before reading
FAIL  rejects invalid dms list --min-followers values before reading
FAIL  rejects invalid dms list --max-followers values before reading
FAIL  rejects invalid inbox --min-score values before reading
FAIL  rejects invalid inbox --limit values before reading
Tests  6 failed | 72 passed (78)

The tests drive the real registered Commander actions through runCli with mocked read models, so a passing test also shows the rejected value never reaches a query. Each option is covered twice: an invalid value is rejected before the read model is called, and a valid value still reaches it unchanged.

oxlint with the repository's own flags: exit 0.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 10:39 PM ET / August 31, 2026, 02:39 UTC.

ClawSweeper review

What this changes

The PR validates DM follower filters and inbox score/limit arguments as non-negative integers before querying Birdclaw’s local archive, with regression cases for rejected and accepted values.

Regression provenance

Possible regression — probable (reviewed change; failure trace). No predecessor PR is attributed.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

The branch still fixes a current-main CLI defect: malformed numeric filters can become NaN and silently produce empty archive results. The focused patch, targeted tests, successful checks, and supplied before/after terminal transcript support ordinary maintainer merge review.

Priority: P2
Reviewed head: d43c5b98aca249942f266944b659921bcc0ea424

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, correct CLI validation repair with direct terminal behavior proof and targeted regression coverage.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The changed production owners are the three CLI command handlers; the supplied real-CLI before/after transcript shows malformed follower, score, and limit flags changing from empty successful output on main to structured errors and exit 1 on this branch, with a valid limit still succeeding. The test run supplements this with no-read and valid-forwarding coverage.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owners are the three CLI command handlers; the supplied real-CLI before/after transcript shows malformed follower, score, and limit flags changing from empty successful output on main to structured errors and exit 1 on this branch, with a valid limit still succeeding. The test run supplements this with no-read and valid-forwarding coverage.
Evidence reviewed	5 items	Current-main behavior: The fetched current main still converts inbox numeric options directly with Number(), and both DM command handlers still directly convert follower strings rather than rejecting malformed input. Read-model failure path: DM filtering accepts number-typed values and binds effective follower values into SQL, while inbox filtering compares scores and slices with supplied values; NaN therefore bypasses defaults and can yield no results. Established parser and regression coverage: The existing shared parser rejects non-digits and unsafe integers with structured errors; six introduced parameterized cases cover invalid rejection-before-read and valid forwarding across all affected command-option combinations.
Findings	None	None.
Security	None	None.

How this fits together

Birdclaw’s CLI converts user-supplied archive filters into parameters for local DM and inbox read models. Each command either rejects invalid input or passes typed filters to query and ranking code that formats the operator’s results.

flowchart LR
  User[CLI user] --> Flags[Numeric filter flags]
  Flags --> Parse[Option validation]
  Parse --> Decision{Valid input?}
  Decision -->|No| Error[Structured error and exit 1]
  Decision -->|Yes| Read[DM or inbox read model]
  Read --> Output[Results output]

Loading

Before merge

• Complete next step (P2) - The open PR already contains the bounded fix, passing checks, and sufficient real behavior proof; it needs ordinary maintainer merge review rather than a separate repair lane.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Focused validation coverage	production +38/-16; tests +98; 4 files affected	The change is confined to three CLI handlers and a focused regression suite.

Technical review

Best possible solution:

Merge the CLI-boundary validation so malformed filters consistently fail with a structured error before Birdclaw queries the local archive.

Do we have a high-confidence way to reproduce the issue?

Yes—current main directly converts the affected strings with Number(), and the read models accept the resulting numeric NaN path; the supplied terminal transcript also exercises the changed CLI behavior.

Is this the best way to solve the issue?

Yes—the patch reuses Birdclaw’s existing non-negative-integer parser at the command boundary, preventing invalid values from reaching otherwise valid read-model code.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against c184cf5f1b5e.

Labels

Label justifications:

• P2: Malformed archive filter values currently create a silent empty-result failure, while valid local CLI workflows remain available.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owners are the three CLI command handlers; the supplied real-CLI before/after transcript shows malformed follower, score, and limit flags changing from empty successful output on main to structured errors and exit 1 on this branch, with a valid limit still succeeding. The test run supplements this with no-read and valid-forwarding coverage.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owners are the three CLI command handlers; the supplied real-CLI before/after transcript shows malformed follower, score, and limit flags changing from empty successful output on main to structured errors and exit 1 on this branch, with a valid limit still succeeding. The test run supplements this with no-read and valid-forwarding coverage.

Evidence

What I checked:

• Current-main behavior: The fetched current main still converts inbox numeric options directly with Number(), and both DM command handlers still directly convert follower strings rather than rejecting malformed input. (src/cli/register-inbox.ts:27, c184cf5f1b5e)
• Read-model failure path: DM filtering accepts number-typed values and binds effective follower values into SQL, while inbox filtering compares scores and slices with supplied values; NaN therefore bypasses defaults and can yield no results. (src/lib/dm-read-model.ts:31, d43c5b98aca2)
• Established parser and regression coverage: The existing shared parser rejects non-digits and unsafe integers with structured errors; six introduced parameterized cases cover invalid rejection-before-read and valid forwarding across all affected command-option combinations. (src/cli/command-context.ts:94, d43c5b98aca2)
• Feature-history routing: The current command-registration layout, including the inbox registration file, was introduced by the split-registration refactor; this is a reasonable routing surface for the fix. (src/cli/register-inbox.ts:16, d73a56a5c093)
• Real behavior proof: The PR body supplies a before/after terminal transcript from a real bundled CLI: malformed follower, score, and limit flags change from empty successful output to structured errors with exit 1, while a valid limit succeeds. All five supplied CI checks are successful. (src/cli/test.ts:2782, d43c5b98aca2)

Likely related people:

• Peter Steinberger: Raw commit d73a56a adds src/cli/register-inbox.ts:15 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: medium; commits: d73a56a5c093; files: src/cli/register-inbox.ts)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (27 earlier review cycles; latest 8 shown)

• reviewed 2026-08-29T21:34:29.077Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T00:01:20.928Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T04:50:25.899Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T08:00:00.275Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T11:51:10.928Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T15:41:42.896Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T19:52:35.312Z sha d43c5b9 :: needs maintainer review before merge. :: none
• reviewed 2026-08-30T23:36:02.352Z sha d43c5b9 :: needs maintainer review before merge. :: none

[devYRPauli]

Added the real CLI transcript ClawSweeper asked for, in the description.

bun run cli does not start on this machine: the repo pins a Bun 1.4 canary and my local Bun is 1.3.14, which has no node:sqlite. I bundled src/cli.ts with the repository's own esbuild and ran it on Node 24 instead. No source file was changed to do that.

Before, all four bad values returned an empty result with exit 0. After, each is rejected with a structured error and exit 1, and a valid --limit 5 still runs and exits 0.

[steipete]

Maintainer triage: LAND recommended with the prepared compatibility fix, not the current head unchanged. Commit 9b1ebf4 on triage/133-numeric-filter-proof follows the original contributor commits and includes regression coverage, docs, and a changelog entry. The original fork branch has not been rewritten; nothing has been merged.

The reported bug reproduces against a populated demo database: malformed follower filters and inbox scores become NaN, yielding empty successful output. The PR rejects those values, but its integer-only parser also rejects existing valid inputs: inbox --min-score 3.5, inbox --limit 1e3, and dms list --min-followers 1e3 all fail on d43c5b9 despite working on main.

The maintainer follow-up rejects non-finite thresholds while retaining their existing finite-number semantics, including fractions and signed values. Inbox result limits remain non-negative safe integers, with the existing numeric spellings accepted. The limit helper matches the one in #132 so the eventual combined tree should retain one implementation. Both PRs also add tests at the same insertion point; preserve both sets when combining them.

Real built CLI proof, using the checksum-pinned Bun runtime and an isolated bundled demo database:

$ ./scripts/bun-canary.sh scripts/build-cli.mjs
Done
$ export BIRDCLAW_HOME="$(mktemp -d)"
$ BIRDCLAW_BACKUP_AUTO_SYNC=0 ./scripts/bun-canary.sh bin/birdclaw.mjs init --demo --json
# demo seeded: 2 accounts, 6 profiles, 6 tweets, 4 conversations, 8 messages
$ BIRDCLAW_BACKUP_AUTO_SYNC=0 ./scripts/bun-canary.sh bin/birdclaw.mjs inbox --min-score abc --json
{"error":"--min-score must be a finite number"}
# exit 1; main returned an empty inbox, exit 0
$ BIRDCLAW_BACKUP_AUTO_SYNC=0 ./scripts/bun-canary.sh bin/birdclaw.mjs inbox --min-score 3.5 --json | jq '.stats.total'
3
$ BIRDCLAW_BACKUP_AUTO_SYNC=0 ./scripts/bun-canary.sh bin/birdclaw.mjs inbox --limit 1e3 --json | jq '.stats.total'
3
$ BIRDCLAW_BACKUP_AUTO_SYNC=0 ./scripts/bun-canary.sh bin/birdclaw.mjs dms list --min-followers 1e3 --json | jq length
4

The 56-invocation before/after matrix showed only the six intended malformed-option rejections and fractional inbox-limit rejection changing; the tested valid thresholds and numeric forms matched main.

Validation:

$ ./scripts/bun-canary.sh run --bun check
# format, lint, and typecheck passed
$ TMPDIR=/Volumes/BirdclawTriage0831/fixtures BIRDCLAW_BACKUP_AUTO_SYNC=0 BIRDCLAW_DISABLE_LIVE_WRITES=1 ./scripts/bun-canary.sh ./scripts/run-vitest.mjs run src/cli.test.ts
Test Files  1 passed (1)
     Tests  81 passed (81)
  Duration  4.62s

The temporary RAM-backed fixture directory resolved local I/O timeouts that also reproduced on unchanged main; no assertions or timeouts were changed. Codex autoreview returned scoped-clean at the skill's default P0 threshold. The original head's hosted checks are green; the orchestrator should apply this follow-up to the original PR and run CI on the resulting head before landing.

Thanks @devYRPauli for the original fix.

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(cli): validate follower and inbox options before querying This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.