Skip to content

Combined dependabot go#3231

Merged
zerbitx merged 2 commits intomainfrom
combined-dependabot-go
Apr 24, 2026
Merged

Combined dependabot go#3231
zerbitx merged 2 commits intomainfrom
combined-dependabot-go

Conversation

@zerbitx
Copy link
Copy Markdown
Collaborator

@zerbitx zerbitx commented Apr 23, 2026

What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves #

Please provide a short message that should be published in the DevSpace release notes
Fixed an issue where DevSpace ...

What else do we need to know?

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 23, 2026

Deploy Preview for devspace-docs canceled.

Name Link
🔨 Latest commit 188d9b5
🔍 Latest deploy log https://app.netlify.com/projects/devspace-docs/deploys/69ead9e06b8f3d0008e78c35

@zerbitx zerbitx force-pushed the combined-dependabot-go branch 2 times, most recently from f0e7c99 to 5315256 Compare April 24, 2026 00:03
Signed-off-by: Ryan Swanson <ryan.swanson@loft.sh>
@zerbitx zerbitx force-pushed the combined-dependabot-go branch from 5315256 to b9a900c Compare April 24, 2026 00:49
@zerbitx
Copy link
Copy Markdown
Collaborator Author

zerbitx commented Apr 24, 2026

Claude review

Critical (1)

C1 — Nil dereference panic in termSizeQueueAdapter (exec.go:72)

t.MonitorSize() returns nil when stdout isn't a real terminal. The old code assigned this directly to a
remotecommand.TerminalSizeQueue interface, producing a nil interface (safe). The new code wraps it:
&termSizeQueueAdapter{q: nil} — a non-nil interface holding a struct with a nil inner queue. When the
executor calls sizeQueue.Next() → a.q.Next() → panic. Rare in production since t.Raw == true usually
implies a real TTY, but triggerable in tests or with piped stdin.

Fix: if q := t.MonitorSize(t.GetSize()); q != nil { sizeQueue = &termSizeQueueAdapter{q: q} }


High (3)

  • H1 — overrideOnce (sync.Once) is never resettable, making the runtime error handler non-testable across
    test runs
  • H2 — No test for newRuntimeErrorHandler or OverrideRuntimeErrorHandler end-to-end; only the format
    helper is tested
  • H3 — gorilla/websocket is pinned to a pre-release pseudo-version (v1.5.4-0.20250319132907-e064f32e3674)
    — no CVE tracking, no stable release guarantees

Medium (5)

  • M1 — CustomNavLink active detection silently changed from explicit startsWith to React Router v6's
    built-in matcher; end=false default undocumented for leaf routes
  • M2 — Dead isDevelopment comment in webpack dev config misleads future editors
  • M3 — Both webpack configs use the deprecated postcss-loader v3 plugins() API (will break on v5 upgrade)
  • M4 — terser compress.warnings: false is silently ignored in Terser v5
  • M5 — Post-wait pods.Items[0] access in inject.go is only safe because ExpectNoError aborts; implicit
    dependency

Low (4)

  • L1 — No var _ remotecommand.TerminalSizeQueue = (*termSizeQueueAdapter)(nil) compile-time assertion
  • L2 — _ = upgradeRoundTripper.Close() discards error silently with no comment
  • L3 — Missing formatRuntimeError(nil, "") test case (switch default branch uncovered)
  • L4 — silenceDeprecations: ['legacy-js-api'] masks the Dart Sass migration rather than fixing it

Full details saved to claude-review.md. C1 must be fixed before merge — the nil dereference is a real
crash path.

@zerbitx zerbitx force-pushed the combined-dependabot-go branch from 5a48ae8 to 58fea83 Compare April 24, 2026 02:20
@zerbitx zerbitx marked this pull request as ready for review April 24, 2026 02:20
@zerbitx zerbitx force-pushed the combined-dependabot-go branch from 58fea83 to f40c6c7 Compare April 24, 2026 02:29
- guard kubectl exec terminal sizing against nil queues and add adapter tests
- make runtime error handler overrides resettable and add handler/formatter coverage
- make devspacehelper e2e pod checks explicit after the wait loop
- document CustomNavLink leaf-route matching and clean up webpack/postcss/sass config
- update assets

Signed-off-by: Ryan Swanson <ryan.swanson@loft.sh>
@zerbitx
Copy link
Copy Markdown
Collaborator Author

zerbitx commented Apr 24, 2026

Lint is expected to fail due to the number of files in the commit, its finding old linting errors, despite the config for only new.

@zerbitx zerbitx force-pushed the combined-dependabot-go branch from f40c6c7 to 188d9b5 Compare April 24, 2026 02:47
@zerbitx zerbitx merged commit 7f9a798 into main Apr 24, 2026
19 of 23 checks passed
@zerbitx zerbitx deleted the combined-dependabot-go branch April 24, 2026 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant