test(parser_app): guard the dev Makefile target against argument drift

[prasanna-anchorage]

Why

Nothing in CI runs the dev make targets. The ubuntu job is make -C src generated, lint, test, narrow-build-check — so an argument parser_app's CLI stops accepting leaves make -C src parser_app broken with every check green.

That is not hypothetical. The target passed --usock for some time after 751118bd replaced it with --host-ip/--host-port, and panicked before reaching any flag the existing cli.rs tests cover:

thread 'main' panicked at parser/app/src/cli.rs:31:33:
Parser: invalid CLI args: found --usock, which was not an expected argument

It took someone running the target by hand to find (#454), and then a rebase reintroduced it on another branch (#450). Twice, silently, because no automated check looks.

What

One test, parser_app's cli.rs. It extracts the parser_app target's recipe arguments from src/Makefile, substitutes $(VAR) references, and feeds them to the real OptionsParser::<ParserParser> — so it catches drift from either side: a flag removed from the CLI, or a stale flag left in the recipe.

It then resolves the ABI trust posture too. Parsing alone would still pass a target that had lost --accept-unsigned-abis, which Cli::execute refuses to start without.

Two deliberate choices worth flagging for review:

• include_str! rather than a runtime read, so the Makefile is a compile-time dependency. Moving or renaming it is a build failure, not a guard that quietly stops checking anything.
• The structural assertions pin only "some arguments, at least one a flag" — not which flags. They exist because an unreadable recipe yields an empty argument list, which parses cleanly and proves nothing. Pinning specific flag names instead would make a legitimate rename fail with a misleading "the extractor broke" message before reaching the parse. I wrote it that way first and it misdiagnosed the --usock case, so the ordering here is deliberate: parse first, and let the failure name the offending argument.

Scope

parser_app only, deliberately. Of the five dev run targets, grpc-server and parser_gateway pass no arguments, parser_enclave passes positionals, and parser_host is qos_core's binary whose CLI is not ours to test. parser_app is the only one with flags this repo owns, and the only one that has actually rotted.

Verification

Mutated the recipe locally to confirm the guard fails for the right reason in each direction, rather than only that it passes as written:

Recipe state	Result
as written	passes
--usock restored	fails: UnexpectedInput("--usock"), naming the argument
--accept-unsigned-abis dropped	fails on the posture check
target renamed out from under it	fails structurally, not vacuously

make -C src fmt, lint (including -p parser_app --no-default-features, so narrow-build-check is unaffected) and test (39 suites, 1674 tests, 0 failures) all green.

Base

Stacked on #454 because that PR carries the --usock fix; on main this guard would correctly fail, since main's target still has the broken flag. GitHub will retarget this to main when #454 merges. #454's own approval is untouched — this is a child branch, not a push to prs-556-parser-app.

[shahan-khatchadourian-anchorage]

Nice, focused addition — single file, a new test plus two small private helpers that guard make parser_app's dev target against argument drift. I traced the Makefile-parsing logic against the actual parser_app target in src/Makefile line-by-line and it correctly extracts the argv; clippy is clean and the test passes. Good use of a real regression case (the --usock/--host-ip/--host-port flag rename) to justify why this guard is worth having, and it fills a real gap — nothing else in CI exercises this target's actual argv.

One non-blocking note for follow-up, not this PR:

The Makefile-recipe parsing (makefile_target_args/substitute_make_vars) is a bespoke text scraper — it doesn't handle conditionals, $(shell ...), or more than one -- separator. If the parser_app recipe ever grows any of those, this helper would likely mis-extract tokens silently rather than fail loudly, which undermines the drift-detection this test is meant to provide. Not asking for a rewrite here, just flagging it in case the recipe gets more complex later — a shared PARSER_APP_ARGS-style variable referenced by both the recipe and the test would make this airtight.

[Copilot]

🟡 Changes recommended

Two moderate issues remain in argument parsing and host validation coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a regression test ensuring the parser_app Makefile target remains compatible with its CLI and ABI trust requirements.

Changes:

• Extracts and substitutes Makefile recipe arguments.
• Validates them with the real CLI parser.
• Adds structural and ABI posture checks.

File summaries

File	Description
src/parser/app/src/cli.rs	Adds Makefile target extraction and CLI compatibility testing.

Review details

Suppressed comments (1)

src/parser/app/src/cli.rs:328

• When an unbalanced $(...) reference occurs after a prefix, out already contains the prefix, but this break leaves rest unchanged and line 337 appends the entire token again (foo$(VAR becomes foofoo$(VAR). That contradicts the helper's stated 'leave it verbatim' behavior and can make the parser validate different arguments from the recipe; append only &rest[start..] and return, or otherwise advance rest before breaking.

            let Some(end) = after.find(')') else {
                // Unbalanced `$(`: leave it verbatim rather than guessing, so the
                // caller's parse fails loudly instead of on a silently mangled arg.
                break;

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[prasanna-anchorage]

Addressed Copilot's finding in a352b462: the guard now calls host_addr() after abi_trust(), so a malformed --host-ip/--host-port in the recipe is caught the same way a missing posture flag already was.

Verified by mutation, not just that it compiles:

• As-is: passes.
• Mutated the recipe's --host-ip $(PARSER_HOST) to a literal --host-ip localhost: now fails with could not parse ip to IP v4: AddrParseError(Ipv4) at cli.rs:39, confirming the new call actually catches it. Reverted after.

cargo test -p parser_app --lib 42/42, cargo clippy -p parser_app --all-targets -- -D warnings clean, cargo fmt -p parser_app -- --check clean.

No new test needed — this extends the existing guard's own assertion rather than adding a parallel one, since the thing being verified (the target's actual startup path) is the same.

[prasanna-anchorage]

@shahan-khatchadourian-anchorage — your approval got dismissed by a352b462 (the Copilot-thread fix above). Re-approval needed to unblock; the only change since your last review is that one-line host_addr() addition, mutation-verified in the comment above. Everything else you already reviewed is untouched.

[shahan-khatchadourian-anchorage]

LGTM. Thanks @prasanna-anchorage