diff --git a/scripts/post_update.py b/scripts/post_update.py index 4b70a0c0a9..ec3d1f4c66 100644 --- a/scripts/post_update.py +++ b/scripts/post_update.py @@ -5,7 +5,7 @@ from pathlib import Path import subprocess import sys -from research_coordination import messages, validate_message +from research_coordination import messages, parse_message, validate_message REPO = 'CrocSwap/integer-mult-bounds' @@ -31,7 +31,7 @@ def prepare_body(path, metadata=None): if present: raise ValueError('body already contains coordination metadata') validate_message(metadata) - body = body.rstrip() + '\n\n```json\n' + json.dumps(metadata, indent=2) + '\n```\n' + body = body.rstrip() + '\n\n```json\n' + json.dumps(metadata, indent=2, allow_nan=False) + '\n```\n' return body @@ -78,7 +78,7 @@ def main(): try: if args.discussion <= 0: raise ValueError('discussion number must be positive') - metadata = json.loads(args.metadata_file.read_text(encoding='utf-8')) if args.metadata_file else None + metadata = parse_message(args.metadata_file.read_text(encoding='utf-8')) if args.metadata_file else None body = prepare_body(args.body_file, metadata) result = ({'repository': REPO, 'discussion': args.discussion, 'reply_to': args.reply_to, 'body': body, 'posted': False} if args.dry_run else post(args.discussion, args.reply_to, body)) diff --git a/scripts/research_coordination.py b/scripts/research_coordination.py index d129df77a0..f06887cafc 100644 --- a/scripts/research_coordination.py +++ b/scripts/research_coordination.py @@ -39,6 +39,22 @@ def validate_message(value): return value +def parse_message(raw): + """Read one unambiguous JSON object before validating its fields.""" + def unique(pairs): + result = {} + for key, value in pairs: + if key in result: + raise ValueError('duplicate coordination JSON key: ' + key) + result[key] = value + return result + + def finite(token): + raise ValueError('nonfinite coordination JSON number: ' + token) + + return validate_message(json.loads(raw, object_pairs_hook=unique, parse_constant=finite)) + + def messages(body): result, warnings = [], [] for block in re.findall(r'^```json\s*\n(.*?)^```\s*$', body, re.M | re.S): @@ -47,7 +63,7 @@ def messages(body): try: if len(block) > 16384: raise ValueError('coordination block exceeds 16 KiB') - result.append(validate_message(json.loads(block))) + result.append(parse_message(block)) except (ValueError, TypeError, AttributeError) as exc: warnings.append(str(exc)) if len(result) > 1: diff --git a/tests/test_coordination_json.py b/tests/test_coordination_json.py new file mode 100644 index 0000000000..8ae56bb51e --- /dev/null +++ b/tests/test_coordination_json.py @@ -0,0 +1,63 @@ +"""Ambiguous coordination JSON must fail before any network mutation.""" +import contextlib +import io +import json +from pathlib import Path +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1] / 'scripts')) +import research_coordination as board +import post_update as post +sys.path.pop(0) + + +def value(): + return {'schema': board.SCHEMA, 'kind': 'HANDOFF', 'task': 'fixture', + 'scope': 'one missing check', 'base': 'a' * 40, 'artifact': None} + + +def fenced(raw): + return 'A scoped handoff.\n\n```json\n' + raw + '\n```\n' + + +class CoordinationJSONTests(unittest.TestCase): + def test_duplicate_fields_and_nonfinite_extensions_are_rejected(self): + raw = json.dumps(value()) + for extra in ('"kind":"DONE"', '"base":"' + 'b' * 40 + '"', + '"extra":{"x":1,"x":2}', '"extra":NaN', '"extra":Infinity'): + with self.subTest(extra=extra): + rows, warnings = board.messages(fenced(raw[:-1] + ',' + extra + '}')) + self.assertEqual(rows, []) + self.assertEqual(len(warnings), 1) + + def test_prepared_body_rejects_duplicate_status(self): + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / 'body.md' + path.write_text(fenced(json.dumps(value())[:-1] + ',"kind":"DONE"}')) + with self.assertRaisesRegex(ValueError, 'duplicate.*kind'): + post.prepare_body(path) + + def test_metadata_file_rejects_before_post(self): + with tempfile.TemporaryDirectory() as tmp: + body, metadata = Path(tmp) / 'body.md', Path(tmp) / 'meta.json' + body.write_text('A handoff.\n') + metadata.write_text(json.dumps(value())[:-1] + ',"kind":"DONE"}') + args = ['post_update.py', '--discussion', '1166', '--body-file', str(body), '--metadata-file', str(metadata)] + error = io.StringIO() + with patch.object(sys, 'argv', args), patch.object(post, 'post') as send, contextlib.redirect_stderr(error): + self.assertEqual(post.main(), 1) + send.assert_not_called() + self.assertIn('duplicate coordination JSON key: kind', error.getvalue()) + + def test_valid_unicode_metadata_and_prose_round_trip(self): + data = dict(value(), scope='A κ check', extension={'count': 3}) + raw = json.dumps(data, ensure_ascii=False) + self.assertEqual(board.parse_message(raw), data) + self.assertEqual(board.messages(fenced(raw)), ([data], [])) + + +if __name__ == '__main__': + unittest.main()