Skip to content

Remove legacy OCR receipt polling #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zehongye-gaga merged 1 commit into from
Nov 15, 2026
Merged

Remove legacy OCR receipt polling #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 20 additions & 45 deletions .github/workflows/codex-code-review.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,30 +128,15 @@ jobs:
raise ReviewError('OCR receipt does not match the submitted request.')


def verdict(reply):
state = reply.get('state')
if reply.get('contract_version') == 2:
if state not in WAIT_STATES | FAIL_STATES | {'complete', 'skipped'}:
raise ReviewError('Unknown OCR task state.')
if reply.get('acceptance') != 'accepted' or reply.get('scheduler') != 'backend' or reply.get('durable') is not True:
raise ReviewError('Backend did not accept responsibility for this task.')
if not isinstance(reply.get('task_id'), str) or not reply['task_id'].strip():
raise ReviewError('Missing durable OCR task ID.')
return 0
if state in FAIL_STATES:
raise ReviewError('OCR task failed: ' + state)
if state not in WAIT_STATES | {'complete', 'skipped'}:
def validate_acceptance(reply):
if reply.get('contract_version') != 2:
raise ReviewError('Backend must return the durable acceptance v2 contract.')
if reply.get('state') not in WAIT_STATES | FAIL_STATES | {'complete', 'skipped'}:
raise ReviewError('Unknown OCR task state.')
if reply.get('durable') is not True or reply.get('status_published') is not True:
return None
task_id = reply.get('task_id')
if not isinstance(task_id, str) or not task_id.strip():
if reply.get('acceptance') != 'accepted' or reply.get('scheduler') != 'backend' or reply.get('durable') is not True:
raise ReviewError('Backend did not accept responsibility for this task.')
if not isinstance(reply.get('task_id'), str) or not reply['task_id'].strip():
raise ReviewError('Missing durable OCR task ID.')
expected = {'success', 'failure'} if state in {'complete', 'skipped'} else {'pending'}
if reply.get('commit_status') not in expected:
raise ReviewError('Missing authoritative OCR commit status.')
# This gate confirms dispatch only. Rulesets may separately require ocr-review to enforce the verdict.
return 0


def timeout_handler(signum, frame):
Expand Down Expand Up @@ -263,29 +248,19 @@ jobs:
payload['retry_key'] = run_id + ':' + run_attempt
deadline = time.monotonic() + TIMEOUT
reply = post('ocr-ci-submit', payload, token, deadline)
last_state = None
while True:
check_identity(reply, identity)
result = verdict(reply)
state = reply['state']
if state != last_state:
print('OCR dispatch state: ' + state, flush=True)
last_state = state
if result is not None:
skipped = state == 'skipped' and reply.get('reason') == 'repository_not_enabled'
if _telemetry:
_telemetry.finish('skipped' if skipped else 'accepted', 'repository_not_enabled' if skipped else None)
summary(identity, reply)
if state == 'skipped' and reply.get('reason') == 'repository_not_enabled':
print('OCR dispatch skipped: repository_not_enabled.')
else:
print('OCR dispatch accepted; the separate ocr-review status reports the review verdict.')
return result
remaining = deadline - time.monotonic()
if remaining <= 0:
raise ReviewError('OCR acceptance exceeded 90 seconds. Re-run to reuse the task.', 'ack_deadline_exceeded')
time.sleep(min(5, remaining))
reply = post('ocr-ci-status', receipt, token, deadline)
check_identity(reply, identity)
validate_acceptance(reply)
state = reply['state']
print('OCR dispatch state: ' + state, flush=True)
skipped = state == 'skipped' and reply.get('reason') == 'repository_not_enabled'
if _telemetry:
_telemetry.finish('skipped' if skipped else 'accepted', 'repository_not_enabled' if skipped else None)
summary(identity, reply)
if skipped:
print('OCR dispatch skipped: repository_not_enabled.')
else:
print('OCR dispatch accepted; the separate ocr-review status reports the review verdict.')
return 0


def main():
Expand Down
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ The workflow is displayed as `OCR Dispatch`. An authenticated request for an une

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.
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 fail validation. The client never polls a status endpoint.

## Security and limitations

Expand Down
65 changes: 20 additions & 45 deletions ocr/ci_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,30 +104,15 @@ def check_identity(reply, identity):
raise ReviewError('OCR receipt does not match the submitted request.')


def verdict(reply):
state = reply.get('state')
if reply.get('contract_version') == 2:
if state not in WAIT_STATES | FAIL_STATES | {'complete', 'skipped'}:
raise ReviewError('Unknown OCR task state.')
if reply.get('acceptance') != 'accepted' or reply.get('scheduler') != 'backend' or reply.get('durable') is not True:
raise ReviewError('Backend did not accept responsibility for this task.')
if not isinstance(reply.get('task_id'), str) or not reply['task_id'].strip():
raise ReviewError('Missing durable OCR task ID.')
return 0
if state in FAIL_STATES:
raise ReviewError('OCR task failed: ' + state)
if state not in WAIT_STATES | {'complete', 'skipped'}:
def validate_acceptance(reply):
if reply.get('contract_version') != 2:
raise ReviewError('Backend must return the durable acceptance v2 contract.')
if reply.get('state') not in WAIT_STATES | FAIL_STATES | {'complete', 'skipped'}:
raise ReviewError('Unknown OCR task state.')
if reply.get('durable') is not True or reply.get('status_published') is not True:
return None
task_id = reply.get('task_id')
if not isinstance(task_id, str) or not task_id.strip():
if reply.get('acceptance') != 'accepted' or reply.get('scheduler') != 'backend' or reply.get('durable') is not True:
raise ReviewError('Backend did not accept responsibility for this task.')
if not isinstance(reply.get('task_id'), str) or not reply['task_id'].strip():
raise ReviewError('Missing durable OCR task ID.')
expected = {'success', 'failure'} if state in {'complete', 'skipped'} else {'pending'}
if reply.get('commit_status') not in expected:
raise ReviewError('Missing authoritative OCR commit status.')
# This gate confirms dispatch only. Rulesets may separately require ocr-review to enforce the verdict.
return 0


def timeout_handler(signum, frame):
Expand Down Expand Up @@ -239,29 +224,19 @@ def submit():
payload['retry_key'] = run_id + ':' + run_attempt
deadline = time.monotonic() + TIMEOUT
reply = post('ocr-ci-submit', payload, token, deadline)
last_state = None
while True:
check_identity(reply, identity)
result = verdict(reply)
state = reply['state']
if state != last_state:
print('OCR dispatch state: ' + state, flush=True)
last_state = state
if result is not None:
skipped = state == 'skipped' and reply.get('reason') == 'repository_not_enabled'
if _telemetry:
_telemetry.finish('skipped' if skipped else 'accepted', 'repository_not_enabled' if skipped else None)
summary(identity, reply)
if state == 'skipped' and reply.get('reason') == 'repository_not_enabled':
print('OCR dispatch skipped: repository_not_enabled.')
else:
print('OCR dispatch accepted; the separate ocr-review status reports the review verdict.')
return result
remaining = deadline - time.monotonic()
if remaining <= 0:
raise ReviewError('OCR acceptance exceeded 90 seconds. Re-run to reuse the task.', 'ack_deadline_exceeded')
time.sleep(min(5, remaining))
reply = post('ocr-ci-status', receipt, token, deadline)
check_identity(reply, identity)
validate_acceptance(reply)
state = reply['state']
print('OCR dispatch state: ' + state, flush=True)
skipped = state == 'skipped' and reply.get('reason') == 'repository_not_enabled'
if _telemetry:
_telemetry.finish('skipped' if skipped else 'accepted', 'repository_not_enabled' if skipped else None)
summary(identity, reply)
if skipped:
print('OCR dispatch skipped: repository_not_enabled.')
else:
print('OCR dispatch accepted; the separate ocr-review status reports the review verdict.')
return 0


def main():
Expand Down
2 changes: 2 additions & 0 deletions ocr/telemetry.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ Cancellation, runner termination or lost output can prevent a final line. Collec
The independent ReviewHub collector reads completed job logs through the GitHub Actions API, validates the snapshot and run attempt, then writes PostgreSQL. Its App needs Actions read permission on target repositories; this is separate from this workflow's `permissions: {}`. Existing historical logs do not acquire structured results retroactively. Client and collector rollout need a real target-repository run to verify the path.

Run `python3 ocr/render_workflow.py` and `python3 -m unittest discover -s ocr -p 'test_*.py'` after editing the client. The generated YAML must stay in sync.

Current clients emit gateway requests only for `ocr-ci-submit`. Readers retain support for historical `ocr-ci-status` telemetry; removal of polling does not rewrite past audit records.
74 changes: 41 additions & 33 deletions ocr/test_ci_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,37 +54,33 @@ def test_receipt_fences_stale_and_cross_repo_results(self):
with self.assertRaises(client.ReviewError): client.check_identity(reply(**patch_data), IDENTITY)
client.check_identity(reply(), IDENTITY)

def test_receipt_requires_persistence_and_published_status(self):
for state in client.WAIT_STATES:
self.assertIsNone(client.verdict(reply(state)))
self.assertEqual(client.verdict(reply(state, durable=True, status_published=True,
task_id='task-1', commit_status='pending')), 0)

def test_failed_review_can_ack_but_never_pretends_review_success(self):
self.assertEqual(client.verdict(reply('complete', durable=True, status_published=True,
task_id='task-1', commit_status='failure')), 0)

def test_legacy_skip_and_v2_terminal_admission(self):
self.assertEqual(client.verdict(reply('skipped', durable=True, status_published=True,
task_id='task-1', commit_status='success')), 0)
def test_v2_acceptance_does_not_wait_for_execution_or_publication(self):
for state in client.WAIT_STATES | client.FAIL_STATES | {'complete', 'skipped'}:
self.assertEqual(client.verdict(reply(state, durable=True, contract_version=2,
acceptance='accepted', scheduler='backend', task_id='task-1')), 0)
with self.subTest(state=state):
client.validate_acceptance(reply(state, durable=True, contract_version=2,
acceptance='accepted', scheduler='backend',
task_id='task-1', status_published=False))

def test_legacy_and_invalid_acknowledgements_fail(self):
good = dict(durable=True, contract_version=2, acceptance='accepted',
scheduler='backend', task_id='task-1')
for patch_data in [{'contract_version': None}, {'contract_version': 1},
{'contract_version': 3}, {'durable': False}, {'durable': 1},
{'scheduler': 'n8n'}, {'acceptance': None},
{'task_id': ''}, {'task_id': ' '}, {'state': 'unknown'}]:
with self.subTest(patch_data=patch_data):
data = reply('queued', **good)
data.update(patch_data)
with self.assertRaises(client.ReviewError):
client.validate_acceptance(data)
with self.assertRaises(client.ReviewError):
client.verdict(reply('accepted', durable=True, contract_version=2, acceptance='accepted', scheduler='n8n', task_id='task-1'))

def test_invalid_receipts_fail(self):
for state in ['failed', 'cancelled', 'superseded', 'unknown']:
with self.assertRaises(client.ReviewError): client.verdict(reply(state))
for extra in [{'task_id': ''}, {'commit_status': 'success'}]:
data = dict(durable=True, status_published=True, task_id='task-1', commit_status='pending')
data.update(extra)
with self.assertRaises(client.ReviewError): client.verdict(reply('queued', **data))
client.validate_acceptance(reply('queued', durable=True, status_published=True,
task_id='task-1', commit_status='pending'))

def test_deadline_expires_before_another_request(self):
with patch.object(client.urllib.request, 'build_opener') as opener:
with self.assertRaises(client.ReviewError):
client.post('ocr-ci-status', {}, 'test', client.time.monotonic() - 1)
client.post('ocr-ci-submit', {}, 'test', client.time.monotonic() - 1)
opener.assert_not_called()
with self.assertRaises(client.ReviewError): client.timeout_handler(None, None)
self.assertEqual(client.TIMEOUT, 90)
Expand All @@ -97,17 +93,29 @@ def test_missing_secret_fails_before_submission(self):
with self.assertRaises(client.ReviewError): client.main()
post.assert_not_called()

def test_complete_submit_and_poll_flow_without_route(self):
def test_submission_never_polls_even_after_a_legacy_ack(self):
acknowledgements = [
(reply('accepted', durable=True, contract_version=2, acceptance='accepted',
scheduler='backend', task_id='task-1', status_published=False), True),
(reply('queued', durable=True, status_published=True, task_id='task-1',
commit_status='pending'), False),
(reply('accepted'), False),
]
with tempfile.TemporaryDirectory() as directory:
event = Path(directory) / 'event.json'; event.write_text(json.dumps(EVENT))
env = {**ENV, 'GITHUB_EVENT_PATH': str(event), 'OCR_CI_TOKEN': 'test-value',
'GITHUB_STEP_SUMMARY': str(Path(directory) / 'summary.md')}
with patch.dict(os.environ, env, clear=True), patch.object(client, 'post', side_effect=[reply('accepted'), reply('queued', durable=True, status_published=True, task_id='task-1', commit_status='pending')]) as post, patch.object(client.time, 'sleep'):
self.assertEqual(client.main(), 0)
self.assertEqual([x.args[0] for x in post.call_args_list], ['ocr-ci-submit', 'ocr-ci-status'])
for call in post.call_args_list:
self.assertNotIn('route', call.args[1])
self.assertIn('separate ocr-review', (Path(directory) / 'summary.md').read_text())
for ack, accepted in acknowledgements:
with self.subTest(accepted=accepted, ack=ack), patch.dict(os.environ, env, clear=True), patch.object(client, 'post', return_value=ack) as post, patch.object(client.time, 'sleep') as sleep:
if accepted:
self.assertEqual(client.main(), 0)
else:
with self.assertRaises(client.ReviewError): client.main()
self.assertEqual(post.call_count, 1)
self.assertEqual(post.call_args.args[0], 'ocr-ci-submit')
self.assertNotIn('route', post.call_args.args[1])
sleep.assert_not_called()
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', {}, io.BytesIO())
Expand Down Expand Up @@ -146,7 +154,7 @@ def test_bad_json_is_not_success(self):
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)
with self.assertRaises(client.ReviewError): client.post('ocr-ci-submit', {}, 'test', client.time.monotonic() + 60)
self.assertEqual(opener.return_value.open.call_count, client.MAX_ATTEMPTS)


Expand Down
Loading