Fix bounded OCR dispatch retries for dropped connections and incomplete ACKs

.github/workflows/codex-code-review.yml

@@ -25,10 +25,13 @@ jobs:
           # SPDX-License-Identifier: Apache-2.0
           """Trusted workflow client; never check out or execute pull request code."""
           import hashlib
+          import http.client
           import json
           import os
+          import random
           import re
           import signal
+          import ssl
           import sys
           import time
           import urllib.error
@@ -39,6 +42,8 @@ jobs:
           WAIT_STATES = {'accepted', 'dispatching', 'queued', 'running', 'retrying'}
           FAIL_STATES = {'failed', 'cancelled', 'superseded'}
           TIMEOUT = 90
+          MAX_ATTEMPTS = 4
+          MAX_RESPONSE_BYTES = 65536
 
 
           class ReviewError(Exception):
@@ -117,30 +122,68 @@ jobs:
               raise ReviewError('OCR acceptance exceeded 90 seconds; re-run to reuse the task.')
 
 
+          def log_request(path, payload, attempt, started, status, error=None, retry_delay=None):
+              record = {'event': 'ocr_gateway_request', 'operation': path,
+                        'request_id': payload.get('request_id'), 'attempt': attempt,
+                        'elapsed_ms': round((time.monotonic() - started) * 1000),
+                        'http_status': status, 'error': error, 'retry_delay_seconds': retry_delay}
+              print(json.dumps(record, sort_keys=True), flush=True)
+
+
           def post(path, payload, token, deadline):
               data = json.dumps(payload).encode()
-              for attempt in range(4):
-                  remaining = deadline - time.monotonic()
+              for attempt in range(MAX_ATTEMPTS):
+                  started = time.monotonic()
+                  remaining = deadline - started
                   if remaining <= 0:
                       raise ReviewError('Timed out waiting for OCR. Re-run this workflow to reuse the task.')
+                  status = None
                   req = urllib.request.Request(BASE_URL + path, data=data, headers={
                       'Authorization': 'Bearer ' + token, 'Content-Type': 'application/json'})
                   try:
                       with urllib.request.build_opener(NoRedirect).open(req, timeout=min(15, remaining)) as response:
-                          raw = response.read(65537)
-                          if len(raw) > 65536:
+                          status = response.status
+                          length = response.headers.get('Content-Length')
+                          if length is not None:
+                              length = length.strip()
+                          if length is not None and (not length.isascii() or not length.isdecimal()):
+                              raise ReviewError('OCR returned an invalid response length.')
+                          expected = int(length) if length is not None else None
+                          if expected is not None and expected > MAX_RESPONSE_BYTES:
+                              raise ReviewError('OCR response exceeds the allowed size.')
+                          raw = response.read(MAX_RESPONSE_BYTES + 1)
+                          if len(raw) > MAX_RESPONSE_BYTES:
                               raise ReviewError('OCR response exceeds the allowed size.')
-                          try:
-                              return json.loads(raw)
-                          except (ValueError, UnicodeError):
-                              raise ReviewError('OCR returned an invalid JSON response.') from None
+                          # A bounded read may return a short body without raising IncompleteRead.
+                          if expected is not None and len(raw) < expected:
+                              raise http.client.IncompleteRead(raw, expected - len(raw))
+                          reply = json.loads(raw)
                   except urllib.error.HTTPError as error:
-                      if error.code not in [408, 429, 500, 502, 503, 504]:
-                          raise ReviewError(f'OCR gateway HTTP {error.code}; check configuration and credentials.') from None
-                  except (urllib.error.URLError, TimeoutError):
-                      pass
-                  if attempt < 3:
-                      time.sleep(min(2 ** attempt, max(0, deadline - time.monotonic())))
+                      status = error.code
+                      error.close()
+                      if status not in [408, 429, 500, 502, 503, 504]:
+                          log_request(path, payload, attempt + 1, started, status, 'HTTPError')
+                          raise ReviewError(f'OCR gateway HTTP {status}; check configuration and credentials.') from None
+                      failure = 'HTTPError'
+                  except (urllib.error.URLError, ConnectionError, TimeoutError,
+                          ssl.SSLEOFError, http.client.IncompleteRead, http.client.BadStatusLine,
+                          json.JSONDecodeError, UnicodeError) as error:
+                      failure = type(error).__name__
+                  except ReviewError:
+                      log_request(path, payload, attempt + 1, started, status, 'ReviewError')
+                      raise
+                  else:
+                      log_request(path, payload, attempt + 1, started, status)
+                      return reply
+                  remaining = max(0, deadline - time.monotonic())
+                  delay = None
+                  if attempt + 1 < MAX_ATTEMPTS and remaining > 0:
+                      backoff = 2 ** attempt
+                      delay = min(random.uniform(backoff / 2, backoff), remaining)
+                  log_request(path, payload, attempt + 1, started, status, failure, delay)
+                  if delay is None:
+                      break
+                  time.sleep(delay)
               raise ReviewError('OCR gateway remains unavailable after bounded retries; re-run this workflow.')
 
 

README.md

@@ -11,7 +11,9 @@ The OCR dispatch entry point for selected public repositories in the organizatio
 5. Exclude this public repository from the previous OCR Ruleset that references the private `github-policy` workflow. Leave other protection rules unchanged.
 6. After completing the configuration, set the new ruleset to **Active** and verify it with a new PR or commit.
 
-The workflow exits once the backend confirms that the task has been durably accepted. Submission and polling take no more than 90 seconds in total, excluding runner queue and startup time. The job has a hard timeout of 3 minutes. n8n forwards the request. The backend owns Agent routing, default fallback, dispatch and recovery after durable acceptance. The final `ocr-review` status is reported separately. It is currently advisory and is not configured as required. To require a successful review before merging, add a separate required status check.
+The workflow exits once the backend confirms that the task has been durably accepted. Submission and network recovery take no more than 90 seconds in total, excluding runner queue and startup time. The job has a hard timeout of 3 minutes. n8n forwards the request. The backend owns Agent routing, default fallback, dispatch and recovery after durable acceptance. The final `ocr-review` status is reported separately. It is currently advisory and is not configured as required. To require a successful review before merging, add a separate required status check.
+
+Network failures, incomplete responses and transient HTTP errors retry the same submission up to four times, with exponential backoff and jitter. All attempts reuse the PR snapshot request ID and explicit workflow-rerun key. Each request logs its attempt number, elapsed milliseconds, HTTP status, error type and next retry delay. Credentials and response bodies are excluded from these logs. A v2 durable acceptance response ends the workflow immediately, including when status publication is still pending. Legacy acceptance responses can query the status endpoint within the same 90-second deadline.
 
 ## Security and limitations
 

ocr/ci_client.py

@@ -1,10 +1,13 @@
 # SPDX-License-Identifier: Apache-2.0
 """Trusted workflow client; never check out or execute pull request code."""
 import hashlib
+import http.client
 import json
 import os
+import random
 import re
 import signal
+import ssl
 import sys
 import time
 import urllib.error
@@ -15,6 +18,8 @@
 WAIT_STATES = {'accepted', 'dispatching', 'queued', 'running', 'retrying'}
 FAIL_STATES = {'failed', 'cancelled', 'superseded'}
 TIMEOUT = 90
+MAX_ATTEMPTS = 4
+MAX_RESPONSE_BYTES = 65536
 
 
 class ReviewError(Exception):
@@ -93,30 +98,68 @@ def timeout_handler(signum, frame):
     raise ReviewError('OCR acceptance exceeded 90 seconds; re-run to reuse the task.')
 
 
+def log_request(path, payload, attempt, started, status, error=None, retry_delay=None):
+    record = {'event': 'ocr_gateway_request', 'operation': path,
+              'request_id': payload.get('request_id'), 'attempt': attempt,
+              'elapsed_ms': round((time.monotonic() - started) * 1000),
+              'http_status': status, 'error': error, 'retry_delay_seconds': retry_delay}
+    print(json.dumps(record, sort_keys=True), flush=True)
+
+
 def post(path, payload, token, deadline):
     data = json.dumps(payload).encode()
-    for attempt in range(4):
-        remaining = deadline - time.monotonic()
+    for attempt in range(MAX_ATTEMPTS):
+        started = time.monotonic()
+        remaining = deadline - started
         if remaining <= 0:
             raise ReviewError('Timed out waiting for OCR. Re-run this workflow to reuse the task.')
+        status = None
         req = urllib.request.Request(BASE_URL + path, data=data, headers={
             'Authorization': 'Bearer ' + token, 'Content-Type': 'application/json'})
         try:
             with urllib.request.build_opener(NoRedirect).open(req, timeout=min(15, remaining)) as response:
-                raw = response.read(65537)
-                if len(raw) > 65536:
+                status = response.status
+                length = response.headers.get('Content-Length')
+                if length is not None:
+                    length = length.strip()
+                if length is not None and (not length.isascii() or not length.isdecimal()):
+                    raise ReviewError('OCR returned an invalid response length.')
+                expected = int(length) if length is not None else None
+                if expected is not None and expected > MAX_RESPONSE_BYTES:
+                    raise ReviewError('OCR response exceeds the allowed size.')
+                raw = response.read(MAX_RESPONSE_BYTES + 1)
+                if len(raw) > MAX_RESPONSE_BYTES:
                     raise ReviewError('OCR response exceeds the allowed size.')
-                try:
-                    return json.loads(raw)
-                except (ValueError, UnicodeError):
-                    raise ReviewError('OCR returned an invalid JSON response.') from None
+                # A bounded read may return a short body without raising IncompleteRead.
+                if expected is not None and len(raw) < expected:
+                    raise http.client.IncompleteRead(raw, expected - len(raw))
+                reply = json.loads(raw)
         except urllib.error.HTTPError as error:
-            if error.code not in [408, 429, 500, 502, 503, 504]:
-                raise ReviewError(f'OCR gateway HTTP {error.code}; check configuration and credentials.') from None
-        except (urllib.error.URLError, TimeoutError):
-            pass
-        if attempt < 3:
-            time.sleep(min(2 ** attempt, max(0, deadline - time.monotonic())))
+            status = error.code
+            error.close()
+            if status not in [408, 429, 500, 502, 503, 504]:
+                log_request(path, payload, attempt + 1, started, status, 'HTTPError')
+                raise ReviewError(f'OCR gateway HTTP {status}; check configuration and credentials.') from None
+            failure = 'HTTPError'
+        except (urllib.error.URLError, ConnectionError, TimeoutError,
+                ssl.SSLEOFError, http.client.IncompleteRead, http.client.BadStatusLine,
+                json.JSONDecodeError, UnicodeError) as error:
+            failure = type(error).__name__
+        except ReviewError:
+            log_request(path, payload, attempt + 1, started, status, 'ReviewError')
+            raise
+        else:
+            log_request(path, payload, attempt + 1, started, status)
+            return reply
+        remaining = max(0, deadline - time.monotonic())
+        delay = None
+        if attempt + 1 < MAX_ATTEMPTS and remaining > 0:
+            backoff = 2 ** attempt
+            delay = min(random.uniform(backoff / 2, backoff), remaining)
+        log_request(path, payload, attempt + 1, started, status, failure, delay)
+        if delay is None:
+            break
+        time.sleep(delay)
     raise ReviewError('OCR gateway remains unavailable after bounded retries; re-run this workflow.')
 
 

ocr/test_ci_client.py

@@ -109,23 +109,27 @@ def test_complete_submit_and_poll_flow_without_route(self):
                 self.assertIn('separate ocr-review', (Path(directory) / 'summary.md').read_text())
 
     def test_gateway_401_is_not_retried(self):
-        error = urllib.error.HTTPError('https://example.invalid', 401, 'No', {}, None)
+        error = urllib.error.HTTPError('https://example.invalid', 401, 'No', {}, io.BytesIO())
         with patch.object(client.urllib.request, 'build_opener') as opener:
             opener.return_value.open.side_effect = error
             with self.assertRaises(client.ReviewError): client.post('ocr-ci-submit', {}, 'test', client.time.monotonic() + 60)
             self.assertEqual(opener.return_value.open.call_count, 1)
 
     def test_gateway_503_has_bounded_retries(self):
-        error = urllib.error.HTTPError('https://example.invalid', 503, 'Unavailable', {}, None)
+        error = urllib.error.HTTPError('https://example.invalid', 503, 'Unavailable', {}, io.BytesIO())
         with patch.object(client.urllib.request, 'build_opener') as opener, patch.object(client.time, 'sleep'):
             opener.return_value.open.side_effect = error
             with self.assertRaises(client.ReviewError): client.post('ocr-ci-submit', {}, 'test', client.time.monotonic() + 60)
             self.assertEqual(opener.return_value.open.call_count, 4)
 
     def test_bad_json_is_not_success(self):
-        with patch.object(client.urllib.request, 'build_opener') as opener:
-            opener.return_value.open.return_value.__enter__.return_value.read.return_value = b'<html>proxy error</html>'
+        with patch.object(client.urllib.request, 'build_opener') as opener, patch.object(client.time, 'sleep'):
+            response = opener.return_value.open.return_value.__enter__.return_value
+            response.status = 200
+            response.headers = {}
+            response.read.return_value = b'<html>proxy error</html>'
             with self.assertRaises(client.ReviewError): client.post('ocr-ci-status', {}, 'test', client.time.monotonic() + 60)
+            self.assertEqual(opener.return_value.open.call_count, client.MAX_ATTEMPTS)
 
 
 if __name__ == '__main__': unittest.main()