Skip to content

Cross-platform hardening and test enablement#3

Merged
itsdevcoffee merged 5 commits intomainfrom
fix/hardening-followup
Dec 16, 2025
Merged

Cross-platform hardening and test enablement#3
itsdevcoffee merged 5 commits intomainfrom
fix/hardening-followup

Conversation

@itsdevcoffee
Copy link
Owner

Focused follow-up pass to close remaining edge cases after PRs #1 and #2.

Changes

1. Windows-safe atomic file replacement

  • Add atomicRename() helper with remove+retry fallback
  • Fixes os.Rename failures when destination exists on Windows
  • Applied to both cache.go and registry.go
  • Preserves atomic behavior on POSIX systems

2. Tightened retry policy

  • Only retry transient failures (5xx, 429, network/timeout errors)
  • Skip retries for permanent failures (4xx except 429)
  • Add httpStatusError type for structured error handling
  • Add isRetryableError() helper with proper error type checking

3. HTTP tests made real

  • Convert GitHubRawBase from const to package variable
  • Remove all t.Skip() scaffolding
  • Add asserting tests for:
    • Body size limit enforcement
    • Retry behavior on transient failures
    • Non-retry behavior on permanent failures (404)
    • Invalid JSON handling
    • Timeout handling (skipped in short mode)

4. CI polish

  • Switch from hardcoded go-version to go-version-file: go.mod
  • Improves maintainability and version consistency

Files Changed

  • internal/marketplace/cache.go (+28 lines)
  • internal/marketplace/registry.go (+2 lines)
  • internal/marketplace/github.go (+66 lines)
  • internal/marketplace/github_test.go (+118 lines, removed 4 skips)
  • .github/workflows/ci.yml (4 changes)

Validation

All tests pass with race detector:

go test -v -race ./...
# PASS (98.174s)

No breaking changes. Backward-compatible.

Focused follow-up pass to close remaining edge cases after PRs #1 and #2:

1. Windows-safe atomic file replacement:
   - Add atomicRename helper with remove+retry fallback
   - Fixes os.Rename failures when destination exists on Windows
   - Applied to both cache.go and registry.go

2. Tightened retry policy:
   - Only retry transient failures (5xx, 429, network/timeout errors)
   - Skip retries for permanent failures (4xx except 429)
   - Add httpStatusError type for structured error handling
   - Add isRetryableError() helper with proper error type checking

3. HTTP tests made real:
   - Convert GitHubRawBase from const to package variable
   - Remove all t.Skip() scaffolding
   - Add asserting tests for:
     * Body size limit enforcement
     * Retry behavior on transient failures
     * Non-retry behavior on permanent failures (404)
     * Invalid JSON handling
     * Timeout handling (skipped in short mode)

4. CI polish:
   - Switch from hardcoded go-version to go-version-file: go.mod
   - Improves maintainability and version consistency

All tests passing with race detector enabled.
@itsdevcoffee itsdevcoffee force-pushed the fix/hardening-followup branch from 652badf to 169e697 Compare December 16, 2025 07:37
- Fix errcheck: use _, _ = w.Write(...) in test handlers
- Fix errcheck: use defer func() { _ = os.Remove(...) } for cleanup
- Fix errcheck: use _ = tmpFile.Close() for best-effort cleanup
- Fix gosec G304: add #nosec with justification for trusted paths
- Update CI: golangci-lint output format to colored-line-number
Replace golangci-lint-action with manual installation to ensure
golangci-lint is built with the same Go version (1.24) as the project.

This fixes the version mismatch error:
'golangci-lint built with go1.23 but repo targets go1.24'

Changes:
- Install golangci-lint via go install @v1.64.8
- Run golangci-lint with same args as before
- errcheck: defer func() { _ = resp.Body.Close() } in github.go and registry.go
- gosec G304: add #nosec suppressions in config.go (lines 54, 77, 99)
- unused: remove refreshCache() from model.go
- unused: remove min() from view.go
@itsdevcoffee itsdevcoffee merged commit fa57580 into main Dec 16, 2025
4 checks passed
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