fix: align finding field prose with v2 schema; harden webhook misconfiguration handling#35
Conversation
- docs/headless-contract.md: clarify that `line` and `recommendation` on findings are required (present in every finding) but may be null, matching the v2 JSON schema which lists them in `required` with nullable types. - deploy/coven-github/coven_github_adapter.py: catch RuntimeError from verify_webhook_signature() in route_signed_delivery() and return a structured 500 response instead of propagating an unhandled exception when the webhook secret is not configured.
There was a problem hiding this comment.
Pull request overview
This PR updates the headless contract documentation to match the v2 schema’s “required but nullable” finding fields and hardens the Python hosted adapter’s webhook routing so a missing webhook secret produces a structured 500 response instead of an unhandled exception.
Changes:
- Update
docs/headless-contract.mdto v2 contract wording, including “required, may be null” semantics for findinglineandrecommendation. - Add/expand the hosted Python adapter implementation, including structured review result validation and safer webhook signature routing.
- Add Python unit tests covering mention boundary behavior and stale PR head evidence rejection.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| docs/headless-contract.md | Updates contract version and clarifies v2 structured review/result requirements, including required-but-nullable finding fields. |
| deploy/coven-github/coven_github_adapter.py | Implements/updates hosted adapter logic (webhook routing hardening, result contract validation, review-context capture). |
| deploy/coven-github/test_coven_github_adapter.py | Adds unit tests for mention parsing and PR head evidence mismatch handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if action == "labeled" and not labels_include_trigger(issue.get("labels"), policy): | ||
| return ignored(base, "issue_label_not_enabled") | ||
| if action == "labeled" and not trigger_enabled(policy, "issue_label"): | ||
| return ignored(base, "issue_label_not_enabled") |
| def route_signed_delivery(headers, body, debug, webhook_secret=None): | ||
| raw_body = body.encode("utf-8") if isinstance(body, str) else body | ||
| if raw_body is None: | ||
| raw_body = b"" | ||
|
|
| def load_adapter(): | ||
| path = Path(__file__).with_name("coven_github_adapter.py") | ||
| spec = importlib.util.spec_from_file_location("coven_github_adapter", path) | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
| return module |
| try: | ||
| verified = verify_webhook_signature(webhook_secret or WEBHOOK_SECRET, raw_body, signature) | ||
| except RuntimeError: | ||
| return {"ok": False, "status": 500, "error": "webhook secret not configured"} | ||
| if not verified: |
|
Closing as superseded by #31 (merged as #35 and #31 were parallel implementations of the same contract-v2 work. Comparing this PR's adapter against the now-merged version on This PR also commits |
Two contract consistency/safety issues from the Copilot review of PR #31.
Changes
docs/headless-contract.md—lineandrecommendationwere described as optional on findings, but the v2 JSON schema lists both inrequiredwith nullable types. Updated prose to "required, may be null" to prevent consumers from treating absent fields as valid.deploy/coven-github/coven_github_adapter.py—verify_webhook_signature()raisesRuntimeErrorwhen the webhook secret is missing, butroute_signed_delivery()didn't catch it. Wraps the call so a misconfigured deployment returns a structured 500 instead of an unhandled exception: