Skip to content

Derive the env-var scrub lists from clap's bindings; SOCKET_NO_SOCKET_YML is missing from LOCAL_ARG_ENV_VARS #1837

Open

Description

[agent] Filed by the scheduled architecture audit (audit-cli-tests). Register: register comment.

Kind: refactor. Register ID: T03.

Problem

GLOBAL_ARG_ENV_VARS and LOCAL_ARG_ENV_VARS (args.rs L711-L761) are hand-kept copies of the env = "..." attributes on the clap structs. Three things read them:

They have drifted. scan --no-socket-yml binds SOCKET_NO_SOCKET_YML (scan/socket_yml_args.rs L17),`` but neither list names it. So a developer's ambient SOCKET_NO_SOCKET_YML=1 reaches every "hermetic" test child and silently disables socket.yml policy in `e2e_socket_yml_policy`.

The doc above the list (L739-L744) claims that "the invariant tests below parse every entry against its owning subcommand to keep this honest". But those tests iterate their own hand lists, BOOL_BINDINGS and VALUE_BINDINGS (args.rs L1697 and L1786), and global_arg_env_vars_includes_vendor_knobs (L1207) only checks that the const contains strings typed into it. More hand copies exist in tests/cli_parse_scan.rs (SCAN_ENV_VARS, L40-L55) and the other cli_parse_* files.

Proposed change

  • Replace both consts with one function that walks crate::cli_command() and every subcommand's args, collecting Arg::get_env(). The global/local split, if still needed, comes from Arg::is_global_set().
  • Point the hermetic builder, with_clean_socket_env and scrub_empty_env_vars at it.
  • Delete the tautological global_arg_env_vars_includes_vendor_knobs.
  • Generate BOOL_BINDINGS from the same walk, filtered on the parse_bool_flag value parser or kept as an explicit list asserted equal to the walk.

Size

args.rs, tests/common/hermetic.rs and the cli_parse_* helpers. About −80 / +30 lines.

Acceptance criteria

  • A new test asserts that every env binding in cli_command() is scrubbed. It fails on today's main because SOCKET_NO_SOCKET_YML is missing.
  • The existing parse tests stay green.
  • With SOCKET_NO_SOCKET_YML=1 set in the outer environment, cargo test -p socket-patch-cli --test e2e_socket_yml_policy stays green.

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 29, 2026
  2. mikolalysenko commented on Oct 30, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] CI lane: not claiming this yet. Open PR #1842 (arch-refactor/sweep-cli-args) rewrites crates/socket-patch-cli/src/args.rs (+149 / -498), including the env-var lists and the binding tests this issue changes, so a parallel fix would conflict. The CI-lane fixer will pick this up once #1842 merges or closes. The defect is still present on origin/main (5ca5623): SOCKET_NO_SOCKET_YML is not in GLOBAL_ARG_ENV_VARS / LOCAL_ARG_ENV_VARS.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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