fix(#7059): preserve provider cancellation errors

[shairevivo]

Summary

Return the parent context error when cancellation kills an in-flight provider
creation command, instead of leaking the wrapped signal: killed process error.
Make the regression test wait for provider creation before cancelling.

Related Issue

Fixes #7059

Changes

• prioritize ctx.Err() after a failed provider creation attempt
• replace the timing-dependent timeout test with condition-based cancellation

Testing

• make lint passes (stage changes first, then run)
• go test -race ./internal/sandbox/... -count=1
• ensureProviderArgs coverage: 84.6% (threshold: 80%)
• Tests added/updated for new or modified logic

Checklist

• PR title follows Conventional Commits
• Commits are signed off (DCO)
• I wrote this contribution myself and can explain all changes in it

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[qodo-code-review]

PR Summary by Qodo

Preserve provider cancellation errors during creation

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Return parent context cancellation instead of subprocess kill errors during provider creation.
• Synchronize regression cancellation with provider startup to eliminate timing-dependent test
 behavior.

Diagram

sequenceDiagram
  actor Caller
  participant Provider as EnsureProvider
  participant Context as Parent Context
  participant CLI as OpenShell CLI
  Caller->>Provider: Create provider
  Provider->>CLI: Start command
  Context-->>CLI: Cancel command
  CLI-->>Provider: Process killed
  Provider->>Context: Check error
  Context-->>Provider: context.Canceled
  Provider-->>Caller: Return cancellation

Loading

High-Level Assessment

The current approach is appropriate: checking ctx.Err() in the shared creation retry loop covers both provider entry points and preserves ordinary command errors when cancellation did not occur. Moving this translation into lower-level command execution was considered but would broaden behavior beyond provider creation unnecessarily.

Files changed (2) +27 / -13

Bug fix (1) +3 / -0

sandbox.goPrioritize context errors after failed provider creation +3/-0

Prioritize context errors after failed provider creation

• Checks the parent context immediately after a provider creation attempt fails. If cancellation caused the subprocess failure, the function now returns the context error before transient-error classification or retry handling.

internal/sandbox/sandbox.go

Tests (1) +24 / -13

sandbox_test.goMake provider cancellation regression test deterministic +24/-13

Make provider cancellation regression test deterministic

• Reworks the cancellation test to run a blocking fake OpenShell command, wait until provider creation starts, and then cancel explicitly. It verifies 'context.Canceled' is returned promptly and no retry occurs.

internal/sandbox/sandbox_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Tip of the day

💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!