[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Summary

This PR sweeps crates/socket-patch-cli/src/args.rs, the first audited and unswept row in the coverage manifest. It is part of the architecture refactor routine, and the run is logged in discussion #1830. The work follows the audit-cli-tests notes on the file: inline single-caller functions, flatten validate_paths, trim history comments, and delete duplicated or can't-fail inline tests.

Changes

Inlined

• unsupported_ecosystem_message is now inside parse_supported_ecosystem, which also returns early instead of using if/else.
• telemetry_credentials is now inside telemetry_auth, which still goes through api_client_overrides so it uses the same values as the API client.
• refuse_unloaded_deno_prefix had one caller and is now inside Commands::validate_paths in lib.rs, as a plain guard. The error text is byte-identical.

Flattened

• validate_paths now checks --cwd and --global-prefix with one require_dir closure instead of two copies of the same block.

Comments

• About 110 doc and comment lines removed: history narration, bare #444 / #745 / #1123 references, and text that restated the code.
• The why comments stay, including the scrub warning, the manifest_unreadable / --json reason, the Deno refusal reason and the secret-leak note.
• --help text (the field doc comments) is untouched.

Tests deleted as duplicates of tests/cli_global_args.rs

• empty_bool_env_var_parses_as_false_not_crash
• truthy_bool_env_var_parses_as_true
• bare_cli_flag_sets_true
• bools_default_false_when_unset
• cli_arg_overrides_env_var
• env_var_used_then_default_applies
• api_client_overrides_default_is_all_none
• vendor_url_and_patch_server_url_flow_from_cli_and_env
• parse_supported_ecosystem_rejects_unknown_names (a .contains check, now folded into the exact-message test below)

Tests deleted because they can't fail

• vendor_service_config_default_source_permits_service and vendor_service_config_honors_service_and_build_sources assert VendorSource predicates that are constant true (C23 / Delete the vendored and hosted-vlt helpers left without a production caller by the v5 consolidation #782).
• global_arg_env_vars_includes_vendor_knobs checks a hand-written list against itself. Derive the env-var scrub lists from clap's bindings; SOCKET_NO_SOCKET_YML is missing from LOCAL_ARG_ENV_VARS #1837 tracks deriving that list from clap.

Tests fixed

• ecosystem_rejection_messages now goes through parse_supported_ecosystem and compares the exact error strings against the literal ecosystem list. Before, it called the deleted helper with a made-up list. Red→green: changing s.trim().is_empty() to s.is_empty() fails it.
• telemetry_auth_prefers_explicit_values_and_treats_empty_as_unset replaces the telemetry_credentials test. It asserts the org URL and bearer for explicit values, and the proxy with no bearer for empty values. It pins SOCKET_NO_CONFIG and is now #[serial]. Red→green: passing None for the token fails it.
• vendor_service_config_threads_overrides_through now also asserts that use_public_proxy threads through. Red→green: hardcoding use_public_proxy: false fails it.
• vendor_source_flag_parses_normalizes_and_defaults is renamed vendor_source_flag_normalizes_and_rejects_removed_sources. Its comments claimed an auto default and that the env var was "honored", while the test actually asserts a rejection. It now also covers the auto alias.

Not done

• The coverage note "drop api_client_overrides empty filter" is wrong: build_proxy_fallback_client (core api/client.rs) uses overrides.proxy_url without filtering empties, so the filter is load-bearing. The filter and its test stay, and the doc now says why.
• The dead --vendor-source / VendorSource plumbing spans core and is user-visible. It stays with C23 / Delete the vendored and hosted-vlt helpers left without a production caller by the v5 consolidation #782.

Size

git diff --stat: 3 files, +180 / −506.

• Production: args.rs 839 → 722 lines; lib.rs +23. Net −94.
• Tests: args.rs test module 971 → 739. Net −232.

Behavior change

None. Messages, exit codes and JSON are unchanged.

Verification

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-cli --lib: 928 passed.
• --test cli_global_args (14), --test cli_path_flags_validated (29), --test global_scope_project_state (47) and --test cli -- list_render (4) all pass.
• /code-review high ran. Every finding is addressed except the deleted tautology test, which is covered by Derive the env-var scrub lists from clap's bindings; SOCKET_NO_SOCKET_YML is missing from LOCAL_ARG_ENV_VARS #1837.

Risk

Low. The production change is limited to moving code that already existed. No cfg(windows) / cfg(unix) code is touched.

🤖 Generated with Claude Code

https://claude.ai/code/session_011f9aeSbHUhUaxmpmd61LMP

---

Note

Low Risk
Refactor-only: logic is relocated or deduplicated with no intended message, exit-code, or JSON changes; risk is limited to accidental wiring during the move.

Overview
This PR sweeps crates/socket-patch-cli/src/args.rs as part of the CLI audit routine: structure and tests, not user-visible behavior.

Production code

• parse_supported_ecosystem inlines the old unsupported_ecosystem_message helper (same empty vs unsupported wording).
• validate_paths deduplicates --cwd / --global-prefix checks via a shared require_dir closure.
• telemetry_credentials is folded into telemetry_auth, still using api_client_overrides and resolve_ambient_credentials.
• refuse_unloaded_deno_prefix moves from GlobalArgs into Commands::validate_paths in lib.rs (same error text; restore commands still skip the check).

Comments

• Long historical doc comments on GlobalArgs helpers are shortened; operational why notes (env mirror, scrub, manifest vs --json, secret mirroring) stay. --help field docs are unchanged.

Tests

• Removes duplicate or tautological unit tests (bool/env precedence, vendor config predicates, hand-written env-var list checks) that tests/cli_global_args.rs or other suites already cover.
• Tightens remaining tests: exact ecosystem error strings, telemetry_auth URL/bearer vs proxy, use_public_proxy threading, vendor-source auto / removed build.

Integration

• list_render.rs comments now reference telemetry_auth instead of telemetry_credentials.

Net: ~500 lines removed, mostly comments and redundant tests; args.rs shrinks while path validation, telemetry credential routing, and Deno --global-prefix refusal stay the same.

^{Reviewed by Cursor Bugbot for commit d221369. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit d221369. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: labeled Ready for review at d221369f.

• CI: 32/32 run checks green (50 skipped by path filters); mergeable, no conflicts.
• Bugbot: reviewed d221369, no findings.
• For the reviewer: refactor-only sweep of crates/socket-patch-cli/src/args.rs (inlining, deduped tests, trimmed docs) plus small touch-ups in lib.rs and tests/cli/list_render.rs; no intended behaviour change.
• Slack announcement not sent this run (Slack send unavailable); the next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (d221369)

What it does. This is a cleanup of crates/socket-patch-cli/src/args.rs from the architecture refactor routine. It inlines three single-caller helpers, folds the duplicated --cwd/--global-prefix checks into one closure, and trims about 110 history comment lines. It also deletes 12 inline tests that were duplicates of tests/cli_global_args.rs or couldn't fail, and strengthens 4 others. Behavior should not change: error text and --help are byte-identical.

Risk: low. It is a pure refactor (net −326 lines, most of them tests and comments). The one move to check is the Deno --global-prefix refusal, which went from GlobalArgs into Commands::validate_paths.

Look here

• lib.rs:170: the inlined Deno refusal. Rollback, remove, repair and vendor --revert are still exempt, and self-update and hosted-bundle still return before it. This matches the old two-step match.
• args.rs:369: the require_dir closure for --cwd and --global-prefix keeps the same messages and order.
• args.rs:18: parse_supported_ecosystem with the error-message helper inlined.
• args.rs:533: telemetry_auth now calls resolve_ambient_credentials directly, still through api_client_overrides.
• args.rs:1071: the rewritten telemetry test, which now asserts the actual URL and bearer and is #[serial].

Verified

• Read the full diff. I checked the Deno exemption list and early returns against main, and checked that the deleted env and bool tests have counterparts in tests/cli_global_args.rs (empty_bool_env_var_resolves_to_false_not_crash, cli_arg_overrides_env_var, env_vars_populate_global_args, …).
• cargo test -p socket-patch-cli --lib args::: 31/31 pass.
• CI on d221369: 82 check runs, 0 failing (32 success, 50 skipped).
• Cursor Bugbot reviewed it and found no issues; there are no review threads.
• CHANGELOG.md is untouched.

Changes I made: none.

Open questions: none blocking. One note: on a developer machine that exports an ambient token alias outside the flag-env list, the "empty" half of the new telemetry test could resolve a token. CI's environment is clean, so this only affects local runs.

Auto-merge is armed, so approving sends this straight to the merge queue.

---

Generated by Claude Code