Skip to content

Pin CLI_CONTRACT.md's argument and env-var tables to Cli::command() with a freshness test #949

Description

[agent] Filed by the scheduled architecture audit routine (CLI and core). Register: register comment.

Kind: refactor (guard test). Source: review 8.5 F; register row C33. Child 1 of tracking #948.

Problem

The contract's argument and env-var tables are maintained entirely by hand:

Nothing compares them with the parser:

  • The cli_parse_* tests, which the contract cites as its guard, call Cli::try_parse_from and never read the document.
  • GLOBAL_ARG_ENV_VARS / LOCAL_ARG_ENV_VARS drive the empty-var scrub and the clean-env harness, but they aren't compared with the contract or with the parser.
  • The only tests that read CLI_CONTRACT.md check codes: contract_gradle_codes.rs for Gradle/JVM and scripts/tests/test_vlt_coverage.py for vlt.

On 9c43dfc the tables are complete apart from the two small gaps in items 4 and 6 below. A debug build's --help for all nine visible subcommands lists no --long flag and no [env: SOCKET_*] that the tables miss. The tables' only flags absent from --help are the hidden deprecated spellings (--apply, --vendor, --no-apply) and the removed --redirect/--detached (exit 2). Two runs gave the same result. The test needs only those two fixes to land green, and then it stops the next flag from shipping undocumented.

Symptoms

None open. The same class of drift exists for codes (#930, #931) and for core-read env vars (#678). Impact: low risk, small size; it is mostly preventive.

Proposed change

Add crates/socket-patch-cli/tests/contract_cli_tables.rs, modelled on contract_gradle_codes.rs. It walks socket_patch_cli::Cli::command(), including hidden arguments, and checks:

  1. Every non-hidden subcommand appears in the Subcommands table, with its visible aliases.

  2. Every global argument (those on GlobalArgs) has a Global-arguments row naming its --long, its -short when it has one, and its env when it has one.

  3. Every local argument of each subcommand appears in backticks in a Per-subcommand row whose first cell names that subcommand. Hidden deprecated spellings must appear too, so their mapping stays documented.

  4. Every clap env binding appears by its full name in the Environment-variables section. Today four don't. The SOCKET_VEX row abbreviates SOCKET_VEX_PRODUCT, SOCKET_VEX_NO_VERIFY, SOCKET_VEX_DOC_ID and SOCKET_VEX_COMPACT as "the SOCKET_VEX_* knobs (_PRODUCT, …)", and they are spelled out only in the per-subcommand table. Give each one a row.

  5. Reverse direction: every backticked --flag in those tables either exists in Cli::command() or is on a short explicit list of removed spellings that the test asserts clap rejects (today --redirect and --detached).

  6. GLOBAL_ARG_ENV_VARS ∪ LOCAL_ARG_ENV_VARS equals the set of clap env bindings (hidden subcommands included), so the scrub list can't fall behind the parser. This fails today:

    • scan's clap-bound SOCKET_NO_SOCKET_YML is in neither list.
    • SOCKET_MIN_SEVERITY is read by scan itself, not by clap, although its help text carries a hand-written [env: SOCKET_MIN_SEVERITY]. It is in neither list either.
    • So the with_env_cleared harness clears neither one, and an ambient SOCKET_NO_SOCKET_YML=1 can leak into the in-process scan tests that rely on that harness.
    • An exported-but-empty value is harmless, because both parsers treat empty as unset (checked twice on a debug build).

    Add SOCKET_NO_SOCKET_YML to LOCAL_ARG_ENV_VARS. Have the test also require the help-text-only [env: …] names to be on the harness list, so SOCKET_MIN_SEVERITY joins LOCAL_ARG_ENV_VARS too.

Also replace the "How the contract is enforced" bullet that says the parser snapshots lock flag names with one that names this test. Nothing is deleted. Generating the tables is a possible later step; this child only pins them.

Size and scope

One new test file (~200 lines), a line or two in args.rs and a few lines in CLI_CONTRACT.md. Out of scope: core-read env vars (#678), error codes (#930) and exit codes (a later child of #948). It changes no flag, env var or default.

Acceptance criteria

  • cargo test -p socket-patch-cli --test contract_cli_tables passes on main.
  • Removing any row from one of the three tables, or adding a #[arg(long)] field without documenting it, makes the test fail with a message naming the flag or variable (check locally once each way).
  • The existing cli_parse_*, cli_global_args and help_text_hygiene tests stay green.
  • The only production change is the added LOCAL_ARG_ENV_VARS entries.

Dependencies

None; it can start now. It doesn't block other work, but #678 can extend the same test to the core-read registry.


Backlog review — 2026-10-08

Consolidated into #948. The retained tracker(s) preserve this issue’s implementation scope and acceptance criteria. Closing this separate scheduling item as not planned, not as completed.

Explicit CLI-contract freshness-test child; track it inside the parent documentation-contract work.

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions