chore: refactor remaining too_many_arguments functions to config structs

[lunarwhite]

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

Component	Key Files	Role
CLI run module	crates/openshell-cli/src/run.rs	6 remaining suppressed functions
CLI policy update	crates/openshell-cli/src/policy_update.rs	build_policy_update_plan (7 params)
CLI dispatch	crates/openshell-cli/src/main.rs	Call sites for all CLI functions
CLI integration tests	crates/openshell-cli/tests/	30+ provider test calls, 11+ gateway test calls
Server compute	crates/openshell-server/src/compute/mod.rs	ComputeRuntime::from_driver (12 params)
Supervisor network	crates/openshell-supervisor-network/src/proxy.rs, run.rs	4 functions sharing proxy context params
Supervisor process	crates/openshell-supervisor-process/src/ssh.rs	5 functions sharing SSH session params

Technical Investigation

Complete Inventory — too_many_arguments Suppressions

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

Function	File	Params	Call Sites	Natural Struct Grouping
gateway_add	run.rs:851	9	11 (1 main + 3 mTLS tests + 7 inline tests)	OIDC fields (issuer, client_id, audience, scopes) + connection params
provider_create / provider_create_with_options	run.rs:4515/4540	8-9	~30 combined (1 main + ~29 in provider_commands_integration.rs)	Credential source params (from_existing, credentials, from_gcloud_adc, runtime_credentials)
sandbox_policy_update	run.rs:6691	13	2 (1 main)	Policy modification params + execution options (dry_run, wait, timeout_secs)

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

Function	File	Params	Call Sites	Notes
sandbox_exec_grpc	run.rs:2654	8	2	Also has implicit_hasher — both suppressions vanish with one struct
sandbox_list	run.rs:3265	8	2	Filter/output options struct
sandbox_logs	run.rs:7301	8	2	Log query options struct
build_policy_update_plan	policy_update.rs:21	7	2 + 22 indirect test calls	Couples with sandbox_policy_update

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)

Function	Reason
BaseEvent::new (OCSF, 8 params)	Internal to builders; callers use builder API
draw_text_field (TUI, 9 params)	UI helper, 5 calls in same module
build_vertex_route (inference, 8 params)	Pure data construction, 3 calls
stream_exec_over_relay / stream_interactive_exec_over_relay (gRPC, 8-9 params)	1 call site each
VM driver / K8s driver helpers	Internal methods, 1-2 call sites each
EntrypointProcess::spawn / spawn_impl	Dispatcher pattern
ServerState::new / build_compute_runtime	Startup constructors, 1-3 calls

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.

[lunarwhite]

Ref: #1997 (comment)

Experimental spike. Please feel free to modify or close.