fix(lint): resolve all cyclop findings

[skevetter]

Summary

Like funcorder, cyclop is already fully active once enabled (max-complexity: 8 is already set in .golangci.yaml) -- no separate opt-in setting exists, so all 10 pre-existing findings are fixed here in one PR.

Each fix extracts a cohesive sub-step out of the over-complex function into a well-named helper:

• cmd/internal/agentcontainer/setup.go: syncMounts -> restoreSnapshotMounts
• cmd/provider/configure_shared.go: initProvider -> providerConfigEntry
• cmd/provider/list.go: runAvailable -> availableProviderNames
• cmd/provider/set_source.go: Run -> activateProvider
• cmd/workspace/import.go: importWorkspace -> extractWorkspaceData, applySnapshotSource
• cmd/workspace/up/up.go: Run -> setUpTask
• cmd/workspace/up/up_client.go: prepareClient -> checkProviderUpdate
• pkg/agent/delivery/factory.go: NewAgentDelivery split into namedDriverDelivery (exact-match dispatch) plus appleDelivery/microsandboxDelivery helpers -- verified behavior-identical against factory_test.go, which exercises every branch of the driver dispatch
• pkg/provider/parse.go: validateAgentDriver's chained driver-name comparison replaced with a validAgentDrivers set lookup (same boolean result, De Morgan-equivalent)
• pkg/ssh/forward.go: portForwarding -> watchTransportClosed

No behavior changes. A few pre-existing argument-limit findings remain on unrelated, untouched lines in cmd/internal/agentcontainer/setup.go and pkg/ssh/forward.go -- out of scope for this PR, will be picked up when that rule is worked.

Summary by CodeRabbit

• Bug Fixes

  • Improved snapshot workspace restoration while preserving existing reset, skip, logging, and error behavior.
  • Improved provider activation and update validation error handling.
  • Improved agent delivery across Docker, Apple, Kubernetes, and Microsandbox environments.
  • Improved SSH port-forwarding cleanup and transport-closure handling.

• Refactor

  • Streamlined workspace imports, task setup, provider configuration, and available-provider detection.
  • Centralized supported agent-driver validation.

[netlify]

✅ Deploy Preview for devsydev canceled.

Name	Link
🔨 Latest commit	0828b88
🔍 Latest deploy log	https://app.netlify.com/projects/devsydev/deploys/6a70f6937265740007886fcc

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request extracts inline logic into focused helpers across workspace lifecycle, provider commands, agent delivery, driver validation, and SSH transport monitoring. Existing restoration, validation, dispatch, error, and cancellation behavior remains in place.

Changes

Workspace lifecycle helpers

Layer / File(s)	Summary
Snapshot restoration and workspace import cmd/internal/agentcontainer/setup.go, cmd/workspace/import.go	Snapshot restoration, archive extraction, and snapshot-source application now use dedicated helpers.
Workspace task and provider setup cmd/workspace/up/up.go, cmd/workspace/up/up_client.go	Workspace startup delegates task initialization and provider-update checks to helper methods.

Provider command helpers

Layer / File(s)	Summary
Provider configuration and activation cmd/provider/configure_shared.go, cmd/provider/set_source.go	Provider entry creation and automatic activation now use dedicated methods.
Available provider extraction cmd/provider/list.go	Provider names are filtered and normalized by availableProviderNames.

Agent delivery and driver validation

Layer / File(s)	Summary
Driver validation and delivery dispatch pkg/provider/parse.go, pkg/agent/delivery/factory.go	Driver validation uses a lookup map. Delivery dispatch separates named, Docker, Apple, Microsandbox, and fallback paths.

SSH transport monitoring

Layer / File(s)	Summary
Transport-close monitoring pkg/ssh/forward.go	SSH transport-close handling now uses watchTransportClosed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• devsy-org/devsy#20: Both changes modify SSH transport monitoring in pkg/ssh/forward.go.
• devsy-org/devsy#265: Both changes modify AgentDelivery factory dispatch.
• devsy-org/devsy#763: Both changes modify SSH transport-closure handling in portForwarding.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: resolving all reported cyclop lint findings.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[netlify]

✅ Deploy Preview for images-devsy-sh canceled.

Name	Link
🔨 Latest commit	0828b88
🔍 Latest deploy log	https://app.netlify.com/projects/images-devsy-sh/deploys/6a70f6932b15b30008acd552

[coderabbitai]

🧹 Nitpick comments (1)

pkg/ssh/forward.go (1)

189-201: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Avoid leaking client.Wait() when forwarding exits with an open SSH client.

watchTransportClosed starts a goroutine that waits on client.Wait() until transport close, while only the second goroutine listens for done. portForwarding returns on context cancellation and can leak this waiter if callers leave the ssh.Client open. Cancel client.Wait() directly on the done/context path, or centralize transport monitoring in a shared watcher owned by the client/session.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/ssh/forward.go` around lines 189 - 201, Update watchTransportClosed so
the goroutine waiting in client.Wait() is unblocked when done is signaled or
forwarding exits through context cancellation. Coordinate the done and
transportClosed paths so an open SSH client cannot leave the Wait watcher
running, while preserving onClosed() behavior for transport-initiated closure.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@pkg/ssh/forward.go`:
- Around line 189-201: Update watchTransportClosed so the goroutine waiting in
client.Wait() is unblocked when done is signaled or forwarding exits through
context cancellation. Coordinate the done and transportClosed paths so an open
SSH client cannot leave the Wait watcher running, while preserving onClosed()
behavior for transport-initiated closure.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 053095fb-9597-43e3-af63-f368c27a56b2

📥 Commits

Reviewing files that changed from the base of the PR and between 01e2a98 and 0828b88.

📒 Files selected for processing (10)

• cmd/internal/agentcontainer/setup.go
• cmd/provider/configure_shared.go
• cmd/provider/list.go
• cmd/provider/set_source.go
• cmd/workspace/import.go
• cmd/workspace/up/up.go
• cmd/workspace/up/up_client.go
• pkg/agent/delivery/factory.go
• pkg/provider/parse.go
• pkg/ssh/forward.go

[skevetter]

Investigated the CodeRabbit nitpick on watchTransportClosed (pkg/ssh/forward.go) as a fast-follow.

Verified: pre-existing behavior, not introduced by this PR. The client.Wait() goroutine's lifetime was identical before this refactor — I only extracted it into a named helper (watchTransportClosed), the blocking behavior and its bound didn't change. Confirmed against 01e2a9877 (pre-PR875 main).

Verified: ssh.Client.Wait() has no cancellable variant. It's backed by golang.org/x/crypto/ssh's mux.Wait(), which blocks on a sync.Cond with no context/cancellation parameter (checked the vendored source at v0.54.0). The only thing that unblocks it is the connection actually closing — there's nothing local to portForwarding that can bound it without closing the client itself.

Why closing the client here would introduce a real bug: cmd/workspace/port_forward.go's runPortForwards shares one *ssh.Client across every -L/-R mapping, each running its own portForwarding goroutine concurrently. If portForwarding closed the client when its own done/context fired (the fix CodeRabbit's prompt suggests), it would tear down the client out from under every other concurrent forward still using it — turning a benign bounded-lifetime goroutine into a cross-goroutine correctness bug.

Actual bound: the goroutine lives exactly as long as the *ssh.Client does. The client is owned and closed exactly once by the top-level caller (cmd/workspace/ssh.go:242, defer sshClient.Close()), which unblocks every outstanding Wait() call across all concurrent forwards at once. This is standard for a shared-resource watcher and isn't a leak in the unbounded sense — it's scoped to the client's lifetime, same as before this PR.

Conclusion: no code change. A real fix would require restructuring client ownership (e.g., one client per forward, or a caller-side context that ties into Close()) — an architectural change out of scope for this lint-cleanup PR, and not something I'd want to do as a "fast-follow" without a deliberate look at the concurrent-forward design.