fix(cli): clean usage errors for missing/invalid --count/--goal values

[protostatis]

Summary

Found during review of #48. The new search/open CLI commands crashed with raw Python tracebacks on malformed flag input:

• unbrowser search foo --count → IndexError (args[i + 1] out of bounds)
• unbrowser search foo --count abc → unhandled ValueError from int()
• unbrowser open --goal → same IndexError

Every other bad-input path in the wrapper prints a usage hint to stderr and exits 2; these two were the exception.

Fix

Add a small _pop_value(args, flag) helper that pops flag <value> from the arg list and exits 2 with the house-style stderr message when the value is missing; _cmd_search catches the non-integer case for --count explicitly.

Verification

$ unbrowser search foo --count
unbrowser: --count requires a value.
Run `unbrowser --help` to see what's available.
exit=2

$ unbrowser search foo --count abc
unbrowser: --count must be an integer.
exit=2

$ unbrowser open --goal
unbrowser: --goal requires a value.
exit=2

Happy-path popping unit-checked (value consumed, remaining args intact, absent flag returns None). No lint/typecheck config in repo; compileall passes.

[protostatis]

Sky's Code Review

Clean, well-scoped fix for unhandled tracebacks on malformed --count/--goal input. A new _pop_value() helper centralizes flag-value popping and emits the house-style stderr usage message with exit 2 when a value is missing, while _cmd_search now explicitly catches ValueError from int() for non-integer --count values. The change removes duplicated manual index/del logic and the diff includes clear before/after verification outputs plus a unit-checked happy-path note. No security, config, or reliability concerns; this is a correct and idiomatic small CLI improvement.

Verdict: Approve

Comments

• The name _pop_value is slightly misleading for the absent-flag case (returns None without popping), but the docstring makes this behavior explicit. No action required.
• Edge case: if a flag's value is itself a flag-like token (e.g. --goal --count), args.index(flag) can match the value rather than the flag. This is pre-existing behavior and extremely unlikely in practice, so it's not worth blocking on.

---

Reviewed by Sky — Unchained Sky engineering agent

Inline Comments (could not attach to lines)

python/unbrowser/_cli.py:147 — Optional: int() will also silently accept negative values (e.g. --count -3) and non-decimal forms like int(" 5"). If a positive count is expected, consider validating raw_count > 0 and emitting a similar stderr message. Minor, non-blocking.

[protostatis]

Verified locally on the branch: all three repro cases now exit 2 with house-style stderr (missing value x2, non-integer --count), happy paths intact (--count pops correctly, flag-looking tokens still filtered from query), release_check --strict-skill passes, cargo test 128/128, py fixtures green.

Nits for follow-up (none blocking, all pre-existing patterns):

1. Duplicate flags: 'search foo --count 2 --count 9' pops only the first pair; the second value leaks into the query via the not-startswith('-') filter.
2. Non-positive --count: 0 returns [], -5 falls through to a default-ish result set — inconsistent; MCP schema declares minimum 1, CLI could mirror it.
3. PR mentions happy-path popping unit checks but no test file is included — worth appending a _pop_value fixture to tests/test_mcp_minimal.py so the contract survives refactors.