Skip to content

fix(sdks/go): require confirmed success for interpreter readiness probes - #2480

Draft
yangjj-iso wants to merge 1 commit into
opensandbox-group:mainfrom
yangjj-iso:fix/go-interpreter-probe-completion-20261030
Draft

yangjj-iso wants to merge 1 commit into
opensandbox-group:mainfrom
yangjj-iso:fix/go-interpreter-probe-completion-20261030

Conversation

@yangjj-iso

Copy link
Copy Markdown
Contributor

Summary

  • Fix false-positive Go CodeInterpreter.IsHealthy and waitRuntimeReady results when the runtime-check SSE response ends after only init or stdout. These executions have no error event, but also no confirmed completion; the existing checks incorrectly treated them as healthy/ready.
  • Require a non-nil execution, no execution error, and a confirmed zero exit code. A completion reporting exit code 1 without a separate error event must also fail the probe.
  • Preserve compatibility with older servers: RunCommand already infers exit code 0 from a successful execution_complete event that omits exit_code.
  • Add HTTP regression coverage for incomplete probes, nonzero completion, and a second successful legacy-format probe. Empty responses remain rejected as before and are included as a control, not claimed as a newly fixed defect.

Testing

  • New tests fail on unchanged production code: init-only, stdout-only, and nonzero completion wrongly pass health/readiness; polling stops after the first unfinished probe instead of waiting for the second confirmed probe.
  • Go 1.20.14 final patch: CodeInterpreter tests passed 20 runs; full go test ./... -count=1 passed (12.272s).
  • Final go test -race ./... -count=1 passed (13.425s); go vet ./... and git diff --check passed.
  • An earlier full race-suite run hit the independent timeout-message diagnostic flake addressed by test(sdks/go): remove timeout-message race from connection diagnostic #2479. It was an assertion failure, not a race-detector report. That separate change is not included here; final runs above are against this PR's own two-file patch.
  • Live sandbox/Jupyter and Redis integration tests not run. Tests use local HTTP/SSE responses and the real SDK parsing/readiness path.

Breaking Changes

  • None: restores strict runtime-readiness semantics; successful old-server completion events still pass.

Checklist

  • Clearly described motivation and reproduction
  • Regression tests added
  • Security impact considered: no auth, permission, transport, or retry changes
  • Backward compatibility considered
  • Documentation unchanged: a runtime probe without confirmed success should not report healthy.

@github-actions github-actions Bot added sdk/go size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Oct 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sdk/go size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant