diff --git a/docs/CONFIGURATION-APPROVAL.md b/docs/CONFIGURATION-APPROVAL.md index f0631611..2b2ab973 100644 --- a/docs/CONFIGURATION-APPROVAL.md +++ b/docs/CONFIGURATION-APPROVAL.md @@ -89,12 +89,30 @@ reject older formal-only engines before mutation or rollback restoration. Verify compatible checkpoints and last-known-good recovery before activating the opt-in. Remove the field before a separately authorized downgrade to a formal-only engine. -This opt-in accepts the bot's unedited issue comment beginning exactly +This opt-in accepts the bot's unedited issue comment with the opening sentence `Codex Review: Didn't find any major issues.` and containing the standard -`Reviewed commit` field. The current optional follow-up sentence and integration -help footer are recognized. The footer must contain the known integration-help -text; arbitrary details, warnings, and added findings reject acceptance. Other -wording fails closed and requires verifier support before it can count. +`Reviewed commit` field. The comment may begin with the exact header +`` followed by exactly two newline characters. +Unversioned comments remain supported. Leading text, other markers and unknown +versions reject acceptance. The opening sentence may have no suffix or exactly +one of these suffixes, separated by one space: + +- `What shall we delve into next?` +- `Keep it up!` +- `Nice work!` +- `:tada:` +- `:rocket:` +- `Another round soon, please!` +- `Chef's kiss.` +- `Breezy!` +- `Already looking forward to the next diff.` +- `Delightful!` +- `You're on a roll.` +- `Can't wait for the next one!` + +The optional footer must contain the known integration-help text. Arbitrary +suffixes, details, warnings, and added findings reject acceptance. Other wording +fails closed and requires verifier support before it can count. The GitHub author must be a bot with the pinned numeric ID and must differ from the PR author. A copied summary from another account cannot grant approval. @@ -116,11 +134,16 @@ resolved commit at or after the clean response. Running, failed, stale, and unrecognized dashboard updates block acceptance. The complete known dashboard text, footer, trigger, and matching relative-time text and datetime are required; additional prose or a changed relative-time label rejects the update. +The recognized triggers are exactly `Manual request`, `PR opened`, and +`Draft marked ready`. The dashboard cannot approve without a separate clean response. When machine-readable activity state is present, including an indented marker, it must also report completed code review -on the full head with no findings and no security-review activity. Unknown state -versions and unknown top-level or nested state fields fail closed. Both state +on the full head. Its `status` must be exactly `completed`, and both finding flags +must be exactly `false`; `null` flags reject acceptance even with a completed +status. Every security-review value must be `null`. The visible row must agree +with the state. Unknown state versions and unknown top-level or nested state +fields fail closed. Both state objects must contain exactly `commit_sha`, `status`, `has_any_findings`, and `has_unresolved_findings` with the documented clean or absent values. Same-second conflicts reject acceptance. Edited responses cannot approve, and diff --git a/scripts/test_config_approval.py b/scripts/test_config_approval.py index 471c20e1..b42590d4 100644 --- a/scripts/test_config_approval.py +++ b/scripts/test_config_approval.py @@ -389,7 +389,8 @@ def setUp(self): self.reviews.clear() self.comment = { "databaseId": 1000, "author": {"__typename": "Bot", "databaseId": 202}, - "body": "Codex Review: Didn't find any major issues. What shall we delve into next?\n\n" + "body": "\n\n" + "Codex Review: Didn't find any major issues. What shall we delve into next?\n\n" f"**Reviewed commit:** `{HEAD[:10]}`\n\n{CODEX_HELP_FIXTURE}", "createdAt": "2026-01-01T00:01:00Z", "lastEditedAt": None, } @@ -411,10 +412,43 @@ def test_clean_review_requires_operator_opt_in_and_successful_exact_applied_chec self.check() def test_known_clean_response_follow_up_sentences(self): - body = self.comment["body"] - for sentence in ("", " Keep it up!", " Nice work!", " :tada:"): - self.comment["body"] = body.replace(" What shall we delve into next?", sentence) + body = self.comment["body"].removeprefix("\n\n") + for prefix in ("", "\n\n"): + for sentence in ("", " What shall we delve into next?", " Keep it up!", " Nice work!", " :tada:", + " :rocket:", " Another round soon, please!", " Chef's kiss.", " Breezy!", + " Already looking forward to the next diff.", " Delightful!", " You're on a roll.", + " Can't wait for the next one!"): + with self.subTest(prefix=prefix, sentence=sentence): + self.comment["body"] = prefix + body.replace(" What shall we delve into next?", sentence) + self.check() + match = verify_config_approval.CODEX_CLEAN.fullmatch(self.comment["body"]) + self.assertEqual(match[1], HEAD[:10]) + self.assertEqual(match[2], CODEX_HELP_FIXTURE) + self.comment["body"] = prefix + body.split('\n\n
')[0] + '\n' self.check() + self.assertEqual(verify_config_approval.CODEX_CLEAN.fullmatch(self.comment["body"]).groups(), + (HEAD[:10], None)) + + def test_receipt_marker_and_follow_up_are_exact(self): + body = self.comment["body"].removeprefix("\n\n") + for prefix in ("\n\n", "\n\n", + "\n\n", "\n", + "\n\n\n", " \n\n", + "\r\n\r\n", "Leading text\n", + "\n\n" * 2): + with self.subTest(prefix=prefix), self.assertRaises(Rejected): + self.comment["body"] = prefix + body + self.check() + for sentence in (" Arbitrary text.", " :rocket: Warning: unresolved finding.", + " Another round soon, please! Warning: unresolved finding.", + " Chef's kiss. Warning: unresolved finding.", " Breezy! Warning: unresolved finding.", + " Already looking forward to the next diff. Warning: unresolved finding.", + " Delightful! Warning: unresolved finding.", " You're on a roll. Warning: unresolved finding.", + " Can't wait for the next one! Warning: unresolved finding."): + with self.subTest(sentence=sentence), self.assertRaises(Rejected): + self.comment["body"] = "\n\n" + body.replace( + " What shall we delve into next?", sentence) + self.check() def test_completed_activity_refresh_does_not_replace_clean_decision(self): summary = dict(self.comment, databaseId=999, createdAt="2026-01-01T00:00:00Z", @@ -423,7 +457,7 @@ def test_completed_activity_refresh_does_not_replace_clean_decision(self): 'This comment shows the latest Codex review activity on this pull request.\n\n' '| Review | Status | Commit | Review trigger |\n| --- | --- | --- | --- |\n' '| 📝 **Code Review** | ✅ **Completed** ' - f'2026-01-01T00:01:01Z | `{HEAD[:7]}` | Manual request |\n\n' + f'2026-01-01T00:01:01Z | `{HEAD[:7]}` | Draft marked ready |\n\n' + CODEX_SUMMARY_HELP_FIXTURE)) self.pr["comments"]["nodes"].append(summary) self.data[f"{PREFIX}/commits/{HEAD[:7]}"] = {"sha": HEAD} @@ -436,13 +470,31 @@ def test_completed_activity_refresh_does_not_replace_clean_decision(self): "has_unresolved_findings": None}} summary["body"] = body + '\n' self.check() + summary['body'] = body.replace('', + '\n') + self.check() + for trigger in ('Manual request', 'PR opened', 'Draft marked ready'): + for metadata in ('', '\n'): + with self.subTest(trigger=trigger, metadata=metadata): + summary['body'] = body.replace('Draft marked ready', trigger) + metadata + self.check() for changed in (dict(state, unexpected=True), dict(state, security_review={}), dict(state, code_review=dict(state['code_review'], blocking_findings=True)), - dict(state, code_review=dict(state['code_review'], has_any_findings=0)), + dict(state, code_review=dict(state['code_review'], commit_sha='4' * 40)), + dict(state, code_review=dict(state['code_review'], status='running')), + dict(state, code_review=dict(state['code_review'], has_any_findings=None, + has_unresolved_findings=None)), dict(state, security_review=dict(state['security_review'], has_any_findings=False))): with self.subTest(state=changed), self.assertRaises(Rejected): summary['body'] = body + '\n' self.check() + for field in ('has_any_findings', 'has_unresolved_findings'): + for value in (None, True, 0, 1, 'false'): + changed = dict(state, code_review=dict(state['code_review'], **{field: value})) + with self.subTest(field=field, value=value), self.assertRaises(Rejected): + summary['body'] = body + '\n' + self.check() state["code_review"]["has_unresolved_findings"] = True for prefix in ('', ' ', '\t', 'Text before '): with self.subTest(prefix=prefix), self.assertRaises(Rejected): @@ -453,7 +505,7 @@ def test_completed_activity_refresh_does_not_replace_clean_decision(self): body + '\n', body + '\nCRITICAL: credential leak found', body.replace('2026-01-01T00:01:01Z', 'CRITICAL finding'), - body.replace('Manual request', 'Unrecognized trigger'), + body.replace('Draft marked ready', 'Unrecognized trigger'), body.replace('once all reviews finish with no findings.', 'despite unresolved findings.'), body + '\n| Security review | Running | unknown | Manual request |'): with self.subTest(body=changed), self.assertRaises(Rejected): diff --git a/scripts/verify_config_approval.py b/scripts/verify_config_approval.py index 3ceb9e2e..9346a5d1 100644 --- a/scripts/verify_config_approval.py +++ b/scripts/verify_config_approval.py @@ -28,7 +28,11 @@ LOCAL_POLICY_KEYS = {"directory", "public_key", "validator_revision", "suite", "required_checks", "max_age_seconds"} LOCAL_EVIDENCE_MAX_BYTES = 1024 * 1024 CODEX_CLEAN = re.compile( - r"\ACodex Review: Didn't find any major issues\.(?: What shall we delve into next\?| Keep it up!| Nice work!| :tada:)?\n\n" + r"\A(?:\n\n)?" + r"Codex Review: Didn't find any major issues\." + r"(?: What shall we delve into next\?| Keep it up!| Nice work!| :tada:| :rocket:| Another round soon, please!" + r"| Chef's kiss\.| Breezy!| Already looking forward to the next diff\.| Delightful!" + r"| You're on a roll\.| Can't wait for the next one!)?\n\n" r"\*\*Reviewed commit:\*\* `([0-9a-f]{10,40})`\n" r"(?:\n(
[\s\S]*
\s*))?\Z" ) @@ -61,7 +65,7 @@
''' CODEX_COMPLETED_ROW = re.compile( r'\| 📝 \*\*Code Review\*\* \| ✅ \*\*Completed\*\* ' - r'(?P=time) \| `(?P[0-9a-f]{7,40})` \| (?:Manual request|PR opened) \|' + r'(?P=time) \| `(?P[0-9a-f]{7,40})` \| (?:Manual request|PR opened|Draft marked ready) \|' )