diff --git a/.github/workflows/codex-code-review.yml b/.github/workflows/codex-code-review.yml index 4203ea4..d3718a5 100644 --- a/.github/workflows/codex-code-review.yml +++ b/.github/workflows/codex-code-review.yml @@ -89,16 +89,24 @@ jobs: 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'}: + if state not in WAIT_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(): raise ReviewError('Missing durable OCR task ID.') - expected = {'success', 'failure'} if state == 'complete' else {'pending'} + 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. @@ -139,7 +147,7 @@ jobs: def summary(identity, reply): text = (f"## OCR dispatch accepted\n\nRepository: `{identity['repository']}`; PR #{identity['pull_number']}\n\n" f"Head: `{identity['head_sha']}`\n\n" - 'Task persisted and commit status published. Review continues in the backend.\n' + 'Task persisted. The backend owns dispatch, recovery and result publication.\n' 'The separate ocr-review status reports the review verdict; enforcement is configured in Rulesets.\n') if os.environ.get('GITHUB_STEP_SUMMARY'): with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as output: @@ -156,6 +164,9 @@ jobs: payload = {**receipt, 'source': {'run_id': os.environ.get('GITHUB_RUN_ID'), 'run_attempt': os.environ.get('GITHUB_RUN_ATTEMPT'), 'workflow_ref': os.environ.get('GITHUB_WORKFLOW_REF')}} + run_id, run_attempt = os.environ.get('GITHUB_RUN_ID', ''), os.environ.get('GITHUB_RUN_ATTEMPT', '') + if run_id.isdecimal() and run_attempt.isdecimal() and int(run_attempt) > 1: + payload['retry_key'] = run_id + ':' + run_attempt deadline = time.monotonic() + TIMEOUT reply = post('ocr-ci-submit', payload, token, deadline) last_state = None diff --git a/README.md b/README.md index fc95932..8e01507 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ 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 decides whether to route the task through the Agent or the default OCR path. 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 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. ## Security and limitations @@ -31,3 +31,7 @@ Regenerate the YAML after editing `ocr/ci_client.py`. The tests verify that the ## Quick rollback An administrator can set the new `OCR public repositories` Ruleset to **Disabled** to remove this required-workflow constraint. Keep other rules and existing secret access unchanged. Do not restore the public repository's reference to the private policy workflow. Disabling the ruleset does not stop OCR tasks already accepted by the backend. + +## Backend contract v2 + +Durable acceptance confirms backend scheduling responsibility, independently of whether the review is waiting, completed, skipped, blocked or failed. Legacy successful skipped responses are also supported. The workflow does not enforce the review verdict; configure the separate `ocr-review` required status when desired. Explicit Actions reruns provide a stable run/attempt retry key so failed pre-dispatch receipts can recover without duplicate submissions. The backend does not depend on n8n polling heartbeats. diff --git a/ocr/ci_client.py b/ocr/ci_client.py index c80c187..fe73337 100644 --- a/ocr/ci_client.py +++ b/ocr/ci_client.py @@ -65,16 +65,24 @@ def check_identity(reply, identity): 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'}: + if state not in WAIT_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(): raise ReviewError('Missing durable OCR task ID.') - expected = {'success', 'failure'} if state == 'complete' else {'pending'} + 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. @@ -115,7 +123,7 @@ def post(path, payload, token, deadline): def summary(identity, reply): text = (f"## OCR dispatch accepted\n\nRepository: `{identity['repository']}`; PR #{identity['pull_number']}\n\n" f"Head: `{identity['head_sha']}`\n\n" - 'Task persisted and commit status published. Review continues in the backend.\n' + 'Task persisted. The backend owns dispatch, recovery and result publication.\n' 'The separate ocr-review status reports the review verdict; enforcement is configured in Rulesets.\n') if os.environ.get('GITHUB_STEP_SUMMARY'): with open(os.environ['GITHUB_STEP_SUMMARY'], 'a') as output: @@ -132,6 +140,9 @@ def main(): payload = {**receipt, 'source': {'run_id': os.environ.get('GITHUB_RUN_ID'), 'run_attempt': os.environ.get('GITHUB_RUN_ATTEMPT'), 'workflow_ref': os.environ.get('GITHUB_WORKFLOW_REF')}} + run_id, run_attempt = os.environ.get('GITHUB_RUN_ID', ''), os.environ.get('GITHUB_RUN_ATTEMPT', '') + if run_id.isdecimal() and run_attempt.isdecimal() and int(run_attempt) > 1: + payload['retry_key'] = run_id + ':' + run_attempt deadline = time.monotonic() + TIMEOUT reply = post('ocr-ci-submit', payload, token, deadline) last_state = None diff --git a/ocr/test_ci_client.py b/ocr/test_ci_client.py index 76e8e44..ed5992d 100644 --- a/ocr/test_ci_client.py +++ b/ocr/test_ci_client.py @@ -63,8 +63,17 @@ 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) + 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.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', 'skipped', 'unknown']: + 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')