chore: refactor remaining too_many_arguments functions to config structs #2003

Problem Statement

PR #1997 (issue #1408) refactored run::sandbox_create from a 21-parameter positional function into a 4-parameter function using a SandboxCreateConfig struct with Default. This eliminated the clippy::too_many_arguments suppression and cut test call sites from ~23 lines to ~6 lines.

The codebase still has 28 remaining #[allow(clippy::too_many_arguments)] suppressions across 11 files. Several of these functions have the same symptoms as sandbox_create: walls of None, false, &[], &HashMap::new() at call sites, test readability problems, and fragility when parameters are added or reordered.

This issue tracks the follow-up cleanup, prioritized by call-site count and readability impact.

Technical Context

Two config-struct precedents now exist in the CLI crate:

• ProviderRefreshConfigInput<'a> (PR feat(providers): add credential refresh foundation #1349): 6 all-required fields, no Default, consumed via fn(server, input, tls)
• SandboxCreateConfig<'a> (PR refactor(cli): replace sandbox_create positional args with SandboxCreateConfig struct #1997): 18 fields with Default impl, consumed via fn(server, gateway_name, config, tls)

Both keep infrastructure params (server, tls) positional and bundle domain-specific options into a struct. The refactoring pattern is mechanical: define struct → add Default if most fields have sensible defaults → destructure at function entry → update call sites.

Affected Components

Technical Investigation

Complete Inventory — too_many_arguments Suppressions

Tier 1 — High Priority (many call sites, clear struct boundary)

Tier 2 — Medium Priority (fewer call sites, straightforward)

Tier 3 — Medium Priority (clusters sharing overlapping params)

Supervisor SSH cluster (5 functions in ssh.rs):

• handle_connection (9 params), SshHandler::new (7), apply_child_env (8), spawn_pty_shell (11), spawn_pipe_exec (10)
• Share: policy, workdir, netns_fd, proxy_url, ca_file_paths, provider_env, user_environment
• One SshSessionContext struct replaces 5 suppressions

Supervisor network cluster (4 functions across proxy.rs and run.rs):

• run_networking (12 params), start_with_bind_addr (11), handle_tcp_connection (12), handle_forward_proxy (14)
• Share: opa_engine, entrypoint_pid, tls_state, inference_ctx, denial_tx, activity_tx
• One ProxyContext struct serves all four

Server compute:

• ComputeRuntime::from_driver (12 params, 4 call sites) — gateway shared state is the same across all 4 callers

Tier 4 — Low Priority (skip or defer)

Cross-Cutting Observations

• The server: &str + tls: &TlsOptions pair appears in nearly every function in run.rs. A GatewayConnection struct could eliminate this repetition, but that's a broader refactoring suitable for a separate issue.
• ComputeRuntime::from_driver has a parameter _allows_loopback_endpoints: bool prefixed with _ — appears unused (dead code). Confirm and remove before or during the struct refactoring.

Proposed Approach

Tackle Tier 1 items first as individual PRs (one per function or function cluster), following the same mechanical pattern established by PR #1997. Each PR defines a config struct with Default, updates the function signature, destructures at entry, and migrates call sites. Tier 2 items follow. Tier 3 clusters (proxy context, SSH session) require a brief design decision on struct scope before implementation. Tier 4 items are deferred unless someone is already touching those areas.

Scope Assessment

• Complexity: Low for Tier 1-2 (mechanical, identical to refactor(cli): replace sandbox_create positional args with SandboxCreateConfig struct #1997). Medium for Tier 3 (struct design needed for shared contexts).
• Confidence: High — clear path, excellent test coverage, two established precedents.
• Estimated files to change: ~3 per Tier 1 item; ~2 per Tier 2 item; ~2 per Tier 3 cluster.
• Issue type: chore

Risks & Open Questions

• Proxy context struct design: Should ProxyContext hold Arcs directly or references? The tokio::spawn in the accept loop likely requires Arcs — confirm ownership model before implementing.
• server + tls extraction: Each individual struct PR should keep server/tls positional (matching existing precedent) so a broader extraction can happen independently later.
• Sequencing: Tier 1 items are independent of each other and can be tackled in parallel by different contributors.
• _allows_loopback_endpoints: Confirm this is dead code before removing.

Recommended Execution Order

1. gateway_add → GatewayAddConfig (9 params, 11 call sites)
2. provider_create / provider_create_with_options → ProviderCreateConfig (8-9 params, ~30 call sites)
3. sandbox_policy_update + build_policy_update_plan → PolicyUpdateConfig (13 params)
4. sandbox_exec_grpc / sandbox_list / sandbox_logs → individual small structs (8 params each)
5. ComputeRuntime::from_driver → ComputeRuntimeDeps (12 params, 4 call sites)
6. SSH spawn cluster → SshSessionContext (5 functions, overlapping params)
7. Proxy context cluster → ProxyContext (4 functions, overlapping params)

Test Considerations

• No new tests needed for any item. Existing tests validate behavior preservation.
• Tier 1 items produce the biggest test readability wins: provider_commands_integration.rs has ~30 calls that will each shrink from ~12 lines to ~4 lines.
• All config structs should have Default where sensible, with server/tls as mandatory function params.
• Run cargo test -p <crate> and mise run pre-commit for each PR.

Created by spike investigation. Predecessor: #1408 / PR #1997. Use build-from-issue to plan and implement individual items.