Skip to content

Dev container port-forwarding spec compliance - #887

Draft
skevetter wants to merge 7 commits into
mainfrom
devcontainer-port-forwarding-2
Draft

Dev container port-forwarding spec compliance#887
skevetter wants to merge 7 commits into
mainfrom
devcontainer-port-forwarding-2

Conversation

@skevetter

Copy link
Copy Markdown
Contributor

Summary

  • Supports the full onAutoForward enum, including openBrowser, openBrowserOnce, and openPreview, and treats non-numeric portsAttributes keys as regexes per the dev container spec.
  • Opens the browser on client-side port forward for ports whose resolved attribute uses openBrowser, openBrowserOnce, or openPreview.
  • Documents why the in-container forwarder does not duplicate this browser-open handling.
  • Fixes a substring-match bug where a bare numeric portsAttributes key could incorrectly match an unrelated port.
  • Fixes a non-deterministic map iteration in ResolvePortAttribute so repeated calls with multiple matching range or regex keys return a stable result.
  • Fixes a goroutine that left a port stuck in portMap after a failed SSH port forward, blocking retries.
  • Cleans up test gaps found in an independent CodeRabbit review and trims comments that only restated the code.

- ResolvePortAttribute: sort portsAttributes keys before the range/regex
  fallback passes so repeated calls with multiple matching keys return a
  deterministic result (Go map iteration order is randomized per run).
- forwarder.Forward: on devssh.PortForward failure, cancel the port's
  context and remove it from portMap so the port becomes retryable.
- browseronopen_test.go: TestMaybeOpenBrowser_NilOpenedOnceMap_DoesNotPanic
  now uses AutoForwardOpenBrowserOnce so it actually exercises the nil
  f.openedOnce init path it's named for.
- forwarder_test.go: register StopForward cleanup for the two Forward
  tests so their goroutines and listeners don't leak past the test.
- Trim comments in config.go/browseronopen.go that only restated the code
  below them.
Rewrote comments in config.go, browseronopen_test.go, and
credentials_server.go to use plain sentences instead of em-dash
clauses, and trimmed remaining restating text.
@netlify

netlify Bot commented Aug 5, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 5507a5a
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a737adaa1d4360008196526

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cfe0769a-1e39-4524-b7c8-0296a659a767

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

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

@netlify

netlify Bot commented Aug 5, 2026

Copy link
Copy Markdown

Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 5507a5a
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6a737ada7f796000080f63fc

@github-actions github-actions Bot added the size/l label Aug 5, 2026
Reorders openedRecorder methods so unexported record() follows the
exported Len(), extracts repeated test literals into constants, and
rewraps lines golines flagged as too long.
@github-actions github-actions Bot added size/xl and removed size/l labels Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant