opencode-provider-switch/.trellis/tasks/04-17-deep-code-review-ocswitch/session-content.md

1.8 KiB

  • Added .trellis/tasks/04-17-deep-code-review-ocswitch/review.md to capture the full deep review output and concrete remediation guidance.
  • Recorded 2 high-risk items:
    • concurrent config/OpenCode save paths are unsafe under concurrent writers
    • proxy request body has no read timeout after headers
  • Recorded 4 medium-risk items:
    • streaming responses have no idle timeout
    • retryable upstream failures are collapsed into a generic 502
    • opencode sync --set-model / --set-small-model can silently write invalid defaults
    • fixed default API key remains unsafe when binding to non-loopback addresses
  • Recorded 4 low-risk or improvement items:
    • alias lifecycle lacks enable/disable recovery path
    • provider import docs and implementation disagree on empty apiKey
    • opencode sync side effects on JSONC comments and $schema need clearer docs
    • header forwarding should better handle dynamic hop-by-hop headers and narrower forwarding rules
  • Implemented direct fixes for the clearly scoped items:
    • added advisory file locking and atomic writes for both local config and OpenCode sync writes
    • validated non-loopback server binds cannot keep the default local API key
    • added routable alias validation for opencode sync --set-model and --set-small-model
    • added request body read timeout handling and stable timeout error mapping
    • added stream idle timeout and last retryable upstream error passthrough for 429/5xx
    • narrowed forwarded request headers conservatively, including dynamic Connection-declared hop-by-hop removal
    • aligned provider import behavior/docs to allow empty apiKey
    • documented that OpenCode sync rewrites JSONC as normalized JSON
  • Verification completed after implementation:
    • go test ./...
    • go test -race ./...