From 231f45a74eba8f7bc7bb74072c5dc499d4aca1f8 Mon Sep 17 00:00:00 2001 From: luckyPipewrench Date: Sun, 16 Aug 2026 09:41:27 -0400 Subject: [PATCH 1/3] chore(ci): call the shared reviewer instead of a local copy This repository carried its own copy of the reviewer, frozen at whatever commit it was copied from, so every fix made upstream since then never reached it. It now calls the shared reusable workflow, pinned to an immutable Pipelock commit in both positions so the workflow and the reviewer source are the same version. This trades two commands for that. The local copy answered /review tests and /review docs as well as /review and /review deep; the shared reviewer carries only the latter two, so those two modes go away here. They are worth naming rather than discovering: anyone who typed them will get no response at all, because an unmatched comment body simply fails the trigger condition. The local copy is deleted rather than left beside the caller. A second implementation that still looks live is how a later fix lands in the copy nobody runs, and nothing else in this repository referenced it. --- .github/requirements-pr-review.txt | 146 ------------ .github/workflows/pr-review.yaml | 107 ++++----- scripts/pr-review.py | 361 ----------------------------- tests/test_pr_review_routing.py | 149 ------------ 4 files changed, 40 insertions(+), 723 deletions(-) delete mode 100644 .github/requirements-pr-review.txt delete mode 100644 scripts/pr-review.py delete mode 100644 tests/test_pr_review_routing.py diff --git a/.github/requirements-pr-review.txt b/.github/requirements-pr-review.txt deleted file mode 100644 index 0c5b24d..0000000 --- a/.github/requirements-pr-review.txt +++ /dev/null @@ -1,146 +0,0 @@ -# Pinned dependencies for pr-review.yaml workflow. -# All hashes from PyPI; includes py3-none-any and cp312 manylinux_x86_64 -# variants so the install works on GitHub Ubuntu runners. -# Regenerate: pip download --no-deps, then pip hash . -certifi==2026.4.22 \ - --hash=sha256:3cb2210c8f88ba2318d29b0388d1023c8492ff72ecdde4ebdaddbb13a31b1c4a \ - --hash=sha256:8d455352a37b71bf76a79caa83a3d6c25afee4a385d632127b6afb3963f1c580 -charset-normalizer==3.4.7 \ - --hash=sha256:007d05ec7321d12a40227aae9e2bc6dca73f3cb21058999a1df9e193555a9dcc \ - --hash=sha256:03853ed82eeebbce3c2abfdbc98c96dc205f32a79627688ac9a27370ea61a49c \ - --hash=sha256:07d9e39b01743c3717745f4c530a6349eadbfa043c7577eef86c502c15df2c67 \ - --hash=sha256:08e721811161356f97b4059a9ba7bafb23ea5ee2255402c42881c214e173c6b4 \ - --hash=sha256:0c96c3b819b5c3e9e165495db84d41914d6894d55181d2d108cc1a69bfc9cce0 \ - --hash=sha256:0ea948db76d31190bf08bd371623927ee1339d5f2a0b4b1b4a4439a65298703c \ - --hash=sha256:0f7eb884681e3938906ed0434f20c63046eacd0111c4ba96f27b76084cd679f5 \ - --hash=sha256:12a6fff75f6bc66711b73a2f0addfc4c8c15a20e805146a02d147a318962c444 \ - --hash=sha256:12d8baf840cc7889b37c7c770f478adea7adce3dcb3944d02ec87508e2dcf153 \ - --hash=sha256:14265bfe1f09498b9d8ec91e9ec9fa52775edf90fcbde092b25f4a33d444fea9 \ - --hash=sha256:16d971e29578a5e97d7117866d15889a4a07befe0e87e703ed63cd90cb348c01 \ - --hash=sha256:177a0ba5f0211d488e295aaf82707237e331c24788d8d76c96c5a41594723217 \ - --hash=sha256:1a87ca9d5df6fe460483d9a5bbf2b18f620cbed41b432e2bddb686228282d10b \ - --hash=sha256:1c2a768fdd44ee4a9339a9b0b130049139b8ce3c01d2ce09f67f5a68048d477c \ - --hash=sha256:1c2aed2e5e41f24ea8ef1590b8e848a79b56f3a5564a65ceec43c9d692dc7d8a \ - --hash=sha256:1dc8b0ea451d6e69735094606991f32867807881400f808a106ee1d963c46a83 \ - --hash=sha256:1efde3cae86c8c273f1eb3b287be7d8499420cf2fe7585c41d370d3e790054a5 \ - --hash=sha256:202389074300232baeb53ae2569a60901f7efadd4245cf3a3bf0617d60b439d7 \ - --hash=sha256:203104ed3e428044fd943bc4bf45fa73c0730391f9621e37fe39ecf477b128cb \ - --hash=sha256:2257141f39fe65a3fdf38aeccae4b953e5f3b3324f4ff0daf9f15b8518666a2c \ - --hash=sha256:298930cec56029e05497a76988377cbd7457ba864beeea92ad7e844fe74cd1f1 \ - --hash=sha256:2cd4a60d0e2fb04537162c62bbbb4182f53541fe0ede35cdf270a1c1e723cc42 \ - --hash=sha256:2d6eb928e13016cea4f1f21d1e10c1cebd5a421bc57ddf5b1142ae3f86824fab \ - --hash=sha256:2fe249cb4651fd12605b7288b24751d8bfd46d35f12a20b1ba33dea122e690df \ - --hash=sha256:30b8d1d8c52a48c2c5690e152c169b673487a2a58de1ec7393196753063fcd5e \ - --hash=sha256:320ade88cfb846b8cd6b4ddf5ee9e80ee0c1f52401f2456b84ae1ae6a1a5f207 \ - --hash=sha256:3534e7dcbdcf757da6b85a0bbf5b6868786d5982dd959b065e65481644817a18 \ - --hash=sha256:36836d6ff945a00b88ba1e4572d721e60b5b8c98c155d465f56ad19d68f23734 \ - --hash=sha256:38c0109396c4cfc574d502df99742a45c72c08eff0a36158b6f04000043dbf38 \ - --hash=sha256:3946fa46a0cf3e4c8cb1cc52f56bb536310d34f25f01ca9b6c16afa767dab110 \ - --hash=sha256:3bec022aec2c514d9cf199522a802bd007cd588ab17ab2525f20f9c34d067c18 \ - --hash=sha256:3c9a494bc5ec77d43cea229c4f6db1e4d8fe7e1bbffa8b6f0f0032430ff8ab44 \ - --hash=sha256:3dce51d0f5e7951f8bb4900c257dad282f49190fdbebecd4ba99bcc41fef404d \ - --hash=sha256:3dedcc22d73ec993f42055eff4fcfed9318d1eeb9a6606c55892a26964964e48 \ - --hash=sha256:4042d5c8f957e15221d423ba781e85d553722fc4113f523f2feb7b188cc34c5e \ - --hash=sha256:481551899c856c704d58119b5025793fa6730adda3571971af568f66d2424bb5 \ - --hash=sha256:4dc1e73c36828f982bfe79fadf5919923f8a6f4df2860804db9a98c48824ce8d \ - --hash=sha256:4e5163c14bffd570ef2affbfdd77bba66383890797df43dc8b4cc7d6f500bf53 \ - --hash=sha256:511ef87c8aec0783e08ac18565a16d435372bc1ac25a91e6ac7f5ef2b0bff790 \ - --hash=sha256:532bc9bf33a68613fd7d65e4b1c71a6a38d7d42604ecf239c77392e9b4e8998c \ - --hash=sha256:54523e136b8948060c0fa0bc7b1b50c32c186f2fceee897a495406bb6e311d2b \ - --hash=sha256:5649fd1c7bade02f320a462fdefd0b4bd3ce036065836d4f42e0de958038e116 \ - --hash=sha256:56be790f86bfb2c98fb742ce566dfb4816e5a83384616ab59c49e0604d49c51d \ - --hash=sha256:5b77459df20e08151cd6f8b9ef8ef1f961ef73d85c21a555c7eed5b79410ec10 \ - --hash=sha256:5ed6ab538499c8644b8a3e18debabcd7ce684f3fa91cf867521a7a0279cab2d6 \ - --hash=sha256:6178f72c5508bfc5fd446a5905e698c6212932f25bcdd4b47a757a50605a90e2 \ - --hash=sha256:6370e8686f662e6a3941ee48ed4742317cafbe5707e36406e9df792cdb535776 \ - --hash=sha256:64f02c6841d7d83f832cd97ccf8eb8a906d06eb95d5276069175c696b024b60a \ - --hash=sha256:65bcd23054beab4d166035cabbc868a09c1a49d1efe458fe8e4361215df40265 \ - --hash=sha256:66671f93accb62ed07da56613636f3641f1a12c13046ce91ffc923721f23c008 \ - --hash=sha256:6696b7688f54f5af4462118f0bfa7c1621eeb87154f77fa04b9295ce7a8f2943 \ - --hash=sha256:6785f414ae0f3c733c437e0f3929197934f526d19dfaa75e18fdb4f94c6fb374 \ - --hash=sha256:67f6279d125ca0046a7fd386d01b311c6363844deac3e5b069b514ba3e63c246 \ - --hash=sha256:6c114670c45346afedc0d947faf3c7f701051d2518b943679c8ff88befe14f8e \ - --hash=sha256:6e0d51f618228538a3e8f46bd246f87a6cd030565e015803691603f55e12afb5 \ - --hash=sha256:6ed74185b2db44f41ef35fd1617c5888e59792da9bbc9190d6c7300617182616 \ - --hash=sha256:708838739abf24b2ceb208d0e22403dd018faeef86ddac04319a62ae884c4f15 \ - --hash=sha256:715479b9a2802ecac752a3b0efa2b0b60285cf962ee38414211abdfccc233b41 \ - --hash=sha256:733784b6d6def852c814bce5f318d25da2ee65dd4839a0718641c696e09a2960 \ - --hash=sha256:750e02e074872a3fad7f233b47734166440af3cdea0add3e95163110816d6752 \ - --hash=sha256:752a45dc4a6934060b3b0dab47e04edc3326575f82be64bc4fc293914566503e \ - --hash=sha256:7579e913a5339fb8fa133f6bbcfd8e6749696206cf05acdbdca71a1b436d8e72 \ - --hash=sha256:7641bb8895e77f921102f72833904dcd9901df5d6d72a2ab8f31d04b7e51e4e7 \ - --hash=sha256:7804338df6fcc08105c7745f1502ba68d900f45fd770d5bdd5288ddccb8a42d8 \ - --hash=sha256:80d04837f55fc81da168b98de4f4b797ef007fc8a79ab71c6ec9bc4dd662b15b \ - --hash=sha256:813c0e0132266c08eb87469a642cb30aaff57c5f426255419572aaeceeaa7bf4 \ - --hash=sha256:82b271f5137d07749f7bf32f70b17ab6eaabedd297e75dce75081a24f76eb545 \ - --hash=sha256:84c018e49c3bf790f9c2771c45e9313a08c2c2a6342b162cd650258b57817706 \ - --hash=sha256:8751d2787c9131302398b11e6c8068053dcb55d5a8964e114b6e196cf16cb366 \ - --hash=sha256:8778f0c7a52e56f75d12dae53ae320fae900a8b9b4164b981b9c5ce059cd1fcb \ - --hash=sha256:87fad7d9ba98c86bcb41b2dc8dbb326619be2562af1f8ff50776a39e55721c5a \ - --hash=sha256:8d828b6667a32a728a1ad1d93957cdf37489c57b97ae6c4de2860fa749b8fc1e \ - --hash=sha256:8e385e4267ab76874ae30db04c627faaaf0b509e1ccc11a95b3fc3e83f855c00 \ - --hash=sha256:92a0a01ead5e668468e952e4238cccd7c537364eb7d851ab144ab6627dbbe12f \ - --hash=sha256:94e1885b270625a9a828c9793b4d52a64445299baa1fea5a173bf1d3dd9a1a5a \ - --hash=sha256:a180c5e59792af262bf263b21a3c49353f25945d8d9f70628e73de370d55e1e1 \ - --hash=sha256:a277ab8928b9f299723bc1a2dabb1265911b1a76341f90a510368ca44ad9ab66 \ - --hash=sha256:a5fe03b42827c13cdccd08e6c0247b6a6d4b5e3cdc53fd1749f5896adcdc2356 \ - --hash=sha256:a6c5863edfbe888d9eff9c8b8087354e27618d9da76425c119293f11712a6319 \ - --hash=sha256:a89c23ef8d2c6b27fd200a42aa4ac72786e7c60d40efdc76e6011260b6e949c4 \ - --hash=sha256:adb2597b428735679446b46c8badf467b4ca5f5056aae4d51a19f9570301b1ad \ - --hash=sha256:ae196f021b5e7c78e918242d217db021ed2a6ace2bc6ae94c0fc596221c7f58d \ - --hash=sha256:ae89db9e5f98a11a4bf50407d4363e7b09b31e55bc117b4f7d80aab97ba009e5 \ - --hash=sha256:aed52fea0513bac0ccde438c188c8a471c4e0f457c2dd20cdbf6ea7a450046c7 \ - --hash=sha256:aef65cd602a6d0e0ff6f9930fcb1c8fec60dd2cfcb6facaf4bdb0e5873042db0 \ - --hash=sha256:af21eb4409a119e365397b2adbaca4c9ccab56543a65d5dbd9f920d6ac29f686 \ - --hash=sha256:b14b2d9dac08e28bb8046a1a0434b1750eb221c8f5b87a68f4fa11a6f97b5e34 \ - --hash=sha256:bb6d88045545b26da47aa879dd4a89a71d1dce0f0e549b1abcb31dfe4a8eac49 \ - --hash=sha256:bb8cc7534f51d9a017b93e3e85b260924f909601c3df002bcdb58ddb4dc41a5c \ - --hash=sha256:bc17a677b21b3502a21f66a8cc64f5bfad4df8a0b8434d661666f8ce90ac3af1 \ - --hash=sha256:bd6c2a1c7573c64738d716488d2cdd3c00e340e4835707d8fdb8dc1a66ef164e \ - --hash=sha256:bd9b23791fe793e4968dba0c447e12f78e425c59fc0e3b97f6450f4781f3ee60 \ - --hash=sha256:c03a41a8784091e67a39648f70c5f97b5b6a37f216896d44d2cdcb82615339a0 \ - --hash=sha256:c0f081d69a6e58272819b70288d3221a6ee64b98df852631c80f293514d3b274 \ - --hash=sha256:c35abb8bfff0185efac5878da64c45dafd2b37fb0383add1be155a763c1f083d \ - --hash=sha256:c36c333c39be2dbca264d7803333c896ab8fa7d4d6f0ab7edb7dfd7aea6e98c0 \ - --hash=sha256:c45e9440fb78f8ddabcf714b68f936737a121355bf59f3907f4e17721b9d1aae \ - --hash=sha256:c593052c465475e64bbfe5dbd81680f64a67fdc752c56d7a0ae205dc8aeefe0f \ - --hash=sha256:cdd68a1fb318e290a2077696b7eb7a21a49163c455979c639bf5a5dcdc46617d \ - --hash=sha256:ce3412fbe1e31eb81ea42f4169ed94861c56e643189e1e75f0041f3fe7020abe \ - --hash=sha256:cf1493cd8607bec4d8a7b9b004e699fcf8f9103a9284cc94962cb73d20f9d4a3 \ - --hash=sha256:cf29836da5119f3c8a8a70667b0ef5fdca3bb12f80fd06487cfa575b3909b393 \ - --hash=sha256:d4a48e5b3c2a489fae013b7589308a40146ee081f6f509e047e0e096084ceca1 \ - --hash=sha256:d560742f3c0d62afaccf9f41fe485ed69bd7661a241f86a3ef0f0fb8b1a397af \ - --hash=sha256:d6038d37043bced98a66e68d3aa2b6a35505dc01328cd65217cefe82f25def44 \ - --hash=sha256:d61f00a0869d77422d9b2aba989e2d24afa6ffd552af442e0e58de4f35ea6d00 \ - --hash=sha256:d635aab80466bc95771bb78d5370e74d36d1fe31467b6b29b8b57b2a3cd7d22c \ - --hash=sha256:dca4bbc466a95ba9c0234ef56d7dd9509f63da22274589ebd4ed7f1f4d4c54e3 \ - --hash=sha256:dd915403e231e6b1809fe9b6d9fc55cf8fb5e02765ac625d9cd623342a7905d7 \ - --hash=sha256:e044c39e41b92c845bc815e5ae4230804e8e7bc29e399b0437d64222d92809dd \ - --hash=sha256:e060d01aec0a910bdccb8be71faf34e7799ce36950f8294c8bf612cba65a2c9e \ - --hash=sha256:e1421b502d83040e6d7fb2fb18dff63957f720da3d77b2fbd3187ceb63755d7b \ - --hash=sha256:e17b8d5d6a8c47c85e68ca8379def1303fd360c3e22093a807cd34a71cd082b8 \ - --hash=sha256:e5f4d355f0a2b1a31bc3edec6795b46324349c9cb25eed068049e4f472fb4259 \ - --hash=sha256:e712b419df8ba5e42b226c510472b37bd57b38e897d3eca5e8cfd410a29fa859 \ - --hash=sha256:e74327fb75de8986940def6e8dee4f127cc9752bee7355bb323cc5b2659b6d46 \ - --hash=sha256:e80c8378d8f3d83cd3164da1ad2df9e37a666cdde7b1cb2298ed0b558064be30 \ - --hash=sha256:e8ac484bf18ce6975760921bb6148041faa8fef0547200386ea0b52b5d27bf7b \ - --hash=sha256:eca9705049ad3c7345d574e3510665cb2cf844c2f2dcfe675332677f081cbd46 \ - --hash=sha256:ed065083d0898c9d5b4bbec7b026fd755ff7454e6e8b73a67f8c744b13986e24 \ - --hash=sha256:edac0f1ab77644605be2cbba52e6b7f630731fc42b34cb0f634be1a6eface56a \ - --hash=sha256:effc3f449787117233702311a1b7d8f59cba9ced946ba727bdc329ec69028e24 \ - --hash=sha256:f22dec1690b584cea26fade98b2435c132c1b5f68e39f5a0b7627cd7ae31f1dc \ - --hash=sha256:f495a1652cf3fbab2eb0639776dad966c2fb874d79d87ca07f9d5f059b8bd215 \ - --hash=sha256:f496c9c3cc02230093d8330875c4c3cdfc3b73612a5fd921c65d39cbcef08063 \ - --hash=sha256:f59099f9b66f0d7145115e6f80dd8b1d847176df89b234a5a6b3f00437aa0832 \ - --hash=sha256:f59ad4c0e8f6bba240a9bb85504faa1ab438237199d4cce5f622761507b8f6a6 \ - --hash=sha256:fbccdc05410c9ee21bbf16a35f4c1d16123dcdeb8a1d38f33654fa21d0234f79 \ - --hash=sha256:fea24543955a6a729c45a73fe90e08c743f0b3334bbf3201e6c4bc1b0c7fa464 -idna==3.15 \ - --hash=sha256:048adeaf8c2d788c40fee287673ccaa74c24ffd8dcf09ffa555a2fbb59f10ac8 \ - --hash=sha256:ca962446ea538f7092a95e057da437618e886f4d349216d2b1e294abfdb65fdc -requests==2.34.0 \ - --hash=sha256:7d62fe92f50eb82c529b0916bb445afa1531a566fc8f35ffdc64446e771b856a \ - --hash=sha256:917520a21b767485ce7c588f4ebb917c436b24a31231b44228715eaeb5a52c60 -urllib3==2.7.0 \ - --hash=sha256:231e0ec3b63ceb14667c67be60f2f2c40a518cb38b03af60abc813da26505f4c \ - --hash=sha256:9fb4c81ebbb1ce9531cce37674bbc6f1360472bc18ca9a553ede278ef7276897 diff --git a/.github/workflows/pr-review.yaml b/.github/workflows/pr-review.yaml index 2a53314..d2fdca5 100644 --- a/.github/workflows/pr-review.yaml +++ b/.github/workflows/pr-review.yaml @@ -3,78 +3,51 @@ name: AI PR Review on: issue_comment: types: [created] + workflow_dispatch: + inputs: + pr_number: + description: Pull request to review with the workflow on this branch. + required: true + type: string + review_mode: + description: default or deep. + required: true + default: default + type: choice + options: [default, deep] permissions: contents: read + issues: write pull-requests: write -concurrency: - group: pr-review-${{ github.repository }}-${{ github.event.issue.number }} - cancel-in-progress: true - jobs: review: - # Restrict to the repository owner account. The review runner is checked - # out from main and never executes code from the PR, but the job holds an - # API key, so the trigger keeps a permission gate beyond the body check. if: >- - github.event.comment.user.login == 'luckyPipewrench' && - github.event.comment.author_association == 'OWNER' && - github.event.issue.pull_request && - (github.event.comment.body == '/review' || - github.event.comment.body == '/review deep' || - github.event.comment.body == '/review tests' || - github.event.comment.body == '/review docs') - runs-on: ubuntu-latest - timeout-minutes: 10 - steps: - - name: Check out trusted review runner - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 - with: - fetch-depth: 0 - # The runner must come from the default branch, never the pull - # request, since this job holds an API key. Bound to the repository's - # own default branch rather than a hard-coded name so a rename does - # not silently break the workflow. Still repository-controlled, not - # pull-request-controlled, so the trust property is unchanged. - ref: ${{ github.event.repository.default_branch }} - persist-credentials: false - - - name: Set up Python - uses: actions/setup-python@a309ff8b426b58ec0e2a45f0f869d46889d02405 # v6.2.0 - with: - python-version: '3.12' - - - name: Install dependencies - run: pip install --require-hashes -r .github/requirements-pr-review.txt - - - name: Test trusted review runner - run: python -m unittest tests/test_pr_review_routing.py - - - name: Determine review mode - id: mode - env: - COMMENT_BODY: ${{ github.event.comment.body }} - run: | - case "$COMMENT_BODY" in - "/review deep") echo "mode=deep" >> "$GITHUB_OUTPUT" ;; - "/review tests") echo "mode=tests" >> "$GITHUB_OUTPUT" ;; - "/review docs") echo "mode=docs" >> "$GITHUB_OUTPUT" ;; - *) echo "mode=default" >> "$GITHUB_OUTPUT" ;; - esac - - - name: Run PR review - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }} - LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }} - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - REVIEW_MODE: ${{ steps.mode.outputs.mode }} - PR_NUMBER: ${{ github.event.issue.number }} - REPO: ${{ github.repository }} - # Empty/unset repository variables deliberately fall back to the - # Python defaults in scripts/pr-review.py. Keep defaults there so - # manual and workflow runs cannot drift. - PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }} - PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }} - run: python scripts/pr-review.py + github.actor == 'luckyPipewrench' && + github.triggering_actor == 'luckyPipewrench' && + ((github.event_name == 'issue_comment' && + github.event.comment.user.login == 'luckyPipewrench' && + github.event.comment.author_association == 'OWNER' && + github.event.issue.pull_request && + (github.event.comment.body == '/review' || + github.event.comment.body == '/review deep')) || + github.event_name == 'workflow_dispatch') + # Pinned to an immutable Pipelock commit in both positions. A branch or tag + # here would run reviewer code that can change under the pin. + uses: luckyPipewrench/pipelock/.github/workflows/pr-review-reusable.yaml@74b3b3f1099d8d6d8ffeb67407ba7e99d1bd3119 + with: + pr_number: >- + ${{ github.event_name == 'issue_comment' && + github.event.issue.number || inputs.pr_number }} + review_mode: >- + ${{ github.event_name == 'issue_comment' && + (github.event.comment.body == '/review deep' && 'deep' || + 'default') || + inputs.review_mode }} + reviewer_sha: 74b3b3f1099d8d6d8ffeb67407ba7e99d1bd3119 + # Personal-account repositories cannot use secrets: inherit with a reusable + # workflow, so every secret the reviewer needs is mapped by name. + secrets: + review_token: ${{ secrets.GITHUB_TOKEN }} + openai_api_key: ${{ secrets.OPENAI_API_KEY }} diff --git a/scripts/pr-review.py b/scripts/pr-review.py deleted file mode 100644 index 55fba73..0000000 --- a/scripts/pr-review.py +++ /dev/null @@ -1,361 +0,0 @@ -#!/usr/bin/env python3 -# Copyright 2026 Pipelock contributors -# SPDX-License-Identifier: Apache-2.0 - -"""AI-powered PR review for the pipelock-verify Python package. - -Triggered by /review comments on PRs. Supports multiple review modes: - /review - Security and correctness review (default model) - /review deep - Deeper review (higher-capacity model) - /review tests - Test coverage and boundary analysis - /review docs - Documentation accuracy check - -Requires environment variables: - GITHUB_TOKEN - GitHub token (provided by Actions) - REPO - owner/repo - PR_NUMBER - PR number - REVIEW_MODE - "default", "deep", "tests", or "docs" - -LLM configuration (one of): - LITELLM_BASE_URL + LITELLM_API_KEY - LiteLLM proxy - OPENAI_API_KEY - Direct OpenAI API - -Model selection: - PR_REVIEW_MODEL_FAST - Optional model override for default/tests/docs - (default: gpt-5.6-luna) - PR_REVIEW_MODEL_DEEP - Optional model override for /review deep - (default: gpt-5.6-terra) - -The PR_REVIEW_MODEL_FAST env var keeps its name for backwards compatibility -with existing repository-variable overrides; the user-facing /review fast -alias was dropped 2026-04-23 because the default mode is sufficient. -""" - -import json -import os -import sys - -import requests - -# --- Constants --- - -MAX_DIFF_CHARS = 100_000 -# These are the sole default model definitions. GitHub Actions may supply -# optional repository-variable overrides, but an unset or empty override must -# fall back here so workflow configuration cannot drift from local behavior. -DEFAULT_MODEL_FAST = "gpt-5.6-luna" -DEFAULT_MODEL_DEEP = "gpt-5.6-terra" -DEFAULT_TEMPERATURE = 0.2 -DEFAULT_MAX_COMPLETION_TOKENS = 8192 -# max_completion_tokens is shared by reasoning and visible output. At xhigh -# effort, 25000 was consumed by reasoning alone and produced an empty review. -DEEP_MAX_COMPLETION_TOKENS = 64000 -DEFAULT_LLM_TIMEOUT_SECONDS = 120 -DEEP_LLM_TIMEOUT_SECONDS = 300 -FAST_REASONING_EFFORT = "low" -DEEP_REASONING_EFFORT = "xhigh" - - -class LLMReviewError(RuntimeError): - """Raised when the LLM call completed but did not produce a usable review.""" - - -PROMPT_SECURITY = """You are reviewing a pull request for pipelock-verify, the Python verifier for Pipelock action receipts: Ed25519-signed, hash-chained records that an independent party uses to check what an AI agent actually did. - -This library's defining property is BYTE-EXACT PARITY with the Go emitter. The signing input is the SHA-256 of a canonical JSON projection of the action record, reproduced to match Go's encoding/json byte for byte. Any divergence in field set, field order, omitempty semantics, escaping, or number spelling changes the hash and breaks verification. There is no slack. - -Two failure directions both matter, and they are not symmetric: -- FAIL-OPEN: accepting something Go rejects. A receipt that verifies here but not in Go means this tool blesses evidence the emitter considers invalid. Treat as high severity. -- FAIL-CLOSED: rejecting something Go accepts. This breaks honest operators and makes the tool untrustworthy in the other direction. Still a real defect, usually medium. - -Flag: -- any change to the canonical projection: added, removed, reordered, or re-typed fields, and whether omitempty matches Go -- signature or chain-hash construction changes, including what is and is not covered by each -- validation that accepts a shape Go's strict Unmarshal would reject, or rejects one it accepts -- parser-differential surfaces: duplicate keys, trailing tokens, unicode normalization, HTML escaping, integer bounds, float spelling, empty versus absent -- anywhere parsed-and-re-serialized data is used where the producer's original bytes are what actually get hashed -- guards that cannot be reached, or that no test would catch the removal of -- error paths that surface as tracebacks rather than a clean invalid result on attacker-supplied input - -Do not waste time on style nits. -For each finding, include: -1. severity: high, medium, or low -2. file and function -3. why it matters, and which direction it fails -4. a concrete fix - -If there are no material issues, say exactly: No material security or correctness issues found in this diff.""" - -PROMPT_TESTS = """You are reviewing the TEST COVERAGE of a pull request for pipelock-verify, a Python verifier for Ed25519-signed, hash-chained Pipelock receipts. - -The single most important question: WOULD THIS TEST FAIL IF THE THING IT GUARDS WERE DELETED? A test that passes whether or not the guard exists is worse than no test, because it reports safety it does not provide. Say so explicitly wherever you suspect it. - -For each code change, check: - -1. **Vacuity**: for every new guard or validation, is there a test that fails when that guard is removed? If the guard is only reachable through one entry point, does a test actually use that entry point? -2. **Parity, not just validity**: chain tests that assert only "valid" do not catch a divergence in what the chain commits to. Is the root hash or canonical output pinned to an independently computed value? -3. **Real emitter output**: is parity proven against a captured real chain, or only against hand-written data that can drift alongside the code it checks? -4. **Both directions**: for a new rejection, is there a positive test proving legitimate input still passes? Over-tightening is a real defect. -5. **Boundaries**: integer limits including int64 and uint64 edges, empty versus absent, null, empty object and array, non-sorted keys, unicode and HTML-escapable characters, very large values. -6. **Error paths**: are new error returns exercised? - -For each gap: -1. severity: high (untested guard or unproven parity), medium (untested boundary), low (nice-to-have) -2. file and line -3. the specific missing case -4. a concrete test to add, including input and expected result - -If coverage is adequate, say exactly: Test coverage is adequate for this diff.""" - -PROMPT_DOCS = """You are reviewing a pull request for pipelock-verify for DOCUMENTATION ACCURACY. - -Check every claim in the diff against the code: - -1. **Contract claims**: statements about which fields are signed, what the chain hash covers, or how canonicalization works must match the actual implementation and the Go source it mirrors. -2. **Source-of-truth pointers**: comments naming a Go file or struct as the contract must name the file that actually defines it. A pointer to the wrong file is how drift goes unnoticed. -3. **Capability claims**: if docs say something is verified, enforced, or guaranteed, confirm the code does it. Flag anything that overstates what an offline verifier can prove. -4. **Stated limitations**: known gaps should be documented plainly rather than implied or omitted. -5. **Examples**: sample code and CLI invocations must actually run as written. -6. **Stale references**: removed functions, renamed parameters, old behavior. - -For each issue: -1. severity: high (wrong claim about what is verified), medium (stale or misleading), low (unclear) -2. file and line -3. what it says versus what the code shows -4. the correct statement - -If documentation is accurate, say exactly: Documentation accurately reflects the codebase in this diff.""" - - -def get_pr_diff(repo: str, pr_number: str, token: str) -> str: - """Fetch the PR diff from GitHub.""" - url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3.diff", - } - resp = requests.get(url, headers=headers, timeout=30) - resp.raise_for_status() - return resp.text - - -def truncate_diff(diff: str, max_chars: int = MAX_DIFF_CHARS) -> str: - """Truncate diff to stay within token limits.""" - if len(diff) <= max_chars: - return diff - truncated = diff[:max_chars] - return truncated + f"\n\n... (diff truncated at {max_chars} chars, {len(diff)} total)" - - -def model_supports_custom_temperature(model: str) -> bool: - """Return whether chat completions should send a non-default temperature.""" - normalized = model.strip().lower() - model_name = normalized.rsplit("/", 1)[-1] - return not model_name.startswith(("gpt-5", "o1", "o3", "o4")) - - -def model_supports_reasoning_effort(model: str) -> bool: - """Return whether chat completions should pin reasoning effort.""" - normalized = model.strip().lower() - model_name = normalized.rsplit("/", 1)[-1] - return model_name.startswith(("gpt-5", "o1", "o3", "o4")) - - -def build_llm_payload( - model: str, - system_prompt: str, - diff: str, - *, - max_completion_tokens: int = DEFAULT_MAX_COMPLETION_TOKENS, - reasoning_effort: str = FAST_REASONING_EFFORT, -) -> dict: - """Build the chat completions payload for the selected review model.""" - payload = { - "model": model, - "messages": [ - {"role": "system", "content": system_prompt}, - { - "role": "user", - "content": f"Review this pull request diff:\n\n```diff\n{diff}\n```", - }, - ], - "max_completion_tokens": max_completion_tokens, - } - if model_supports_custom_temperature(model): - payload["temperature"] = DEFAULT_TEMPERATURE - if model_supports_reasoning_effort(model): - payload["reasoning_effort"] = reasoning_effort - return payload - - -def model_for_mode(mode: str) -> str: - """Return the configured model for a review mode, with Python defaults.""" - if mode == "deep": - return os.environ.get("PR_REVIEW_MODEL_DEEP") or DEFAULT_MODEL_DEEP - return os.environ.get("PR_REVIEW_MODEL_FAST") or DEFAULT_MODEL_FAST - - -def summarize_usage(data: dict) -> str: - """Return compact token usage details for operator-visible errors.""" - usage = data.get("usage") - if not isinstance(usage, dict): - return "usage unavailable" - details = usage.get("completion_tokens_details") or {} - parts = [ - f"prompt={usage.get('prompt_tokens', 'unknown')}", - f"completion={usage.get('completion_tokens', 'unknown')}", - f"total={usage.get('total_tokens', 'unknown')}", - ] - if isinstance(details, dict) and "reasoning_tokens" in details: - parts.append(f"reasoning={details['reasoning_tokens']}") - return ", ".join(parts) - - -def extract_chat_content(data: dict) -> str: - """Extract visible text from a chat-completions response.""" - choices = data.get("choices", []) - if not isinstance(choices, list) or not choices: - raise LLMReviewError("LLM returned no choices.") - - choice = choices[0] if isinstance(choices[0], dict) else {} - message = choice.get("message") if isinstance(choice.get("message"), dict) else {} - content = message.get("content", "") - if isinstance(content, list): - content = "".join(part.get("text", "") for part in content if isinstance(part, dict)) - if isinstance(content, str) and content.strip(): - if choice.get("finish_reason") == "length": - content += ( - "\n\n> **Warning:** Review output was truncated by the model " - f"completion limit ({summarize_usage(data)}). Treat this as an " - "incomplete review and rerun with a narrower diff if needed." - ) - return content - - finish_reason = choice.get("finish_reason", "unknown") - raise LLMReviewError( - f"LLM returned empty content (finish_reason={finish_reason}; {summarize_usage(data)})." - ) - - -def call_llm(diff: str, mode: str, system_prompt: str) -> str: - """Send the diff to the LLM and return the review.""" - litellm_url = os.environ.get("LITELLM_BASE_URL", "") - litellm_key = os.environ.get("LITELLM_API_KEY", "") - openai_key = os.environ.get("OPENAI_API_KEY", "") - - model = model_for_mode(mode) - - if litellm_url and litellm_key: - api_url = litellm_url.rstrip("/") + "/chat/completions" - bearer = litellm_key - elif openai_key: - api_url = "https://api.openai.com/v1/chat/completions" - bearer = openai_key - else: - raise LLMReviewError( - "No LLM API configured. Set LITELLM_BASE_URL + LITELLM_API_KEY " - "or OPENAI_API_KEY in repo secrets." - ) - - headers = { - "Authorization": f"Bearer {bearer}", - "Content-Type": "application/json", - } - is_deep = mode == "deep" - max_completion_tokens = DEEP_MAX_COMPLETION_TOKENS if is_deep else DEFAULT_MAX_COMPLETION_TOKENS - reasoning_effort = DEEP_REASONING_EFFORT if is_deep else FAST_REASONING_EFFORT - payload = build_llm_payload( - model, - system_prompt, - diff, - max_completion_tokens=max_completion_tokens, - reasoning_effort=reasoning_effort, - ) - - timeout = DEEP_LLM_TIMEOUT_SECONDS if is_deep else DEFAULT_LLM_TIMEOUT_SECONDS - resp = requests.post(api_url, headers=headers, json=payload, timeout=timeout) - if resp.status_code != 200: - raise LLMReviewError(f"LLM API returned {resp.status_code} for model `{model}`.") - try: - data = resp.json() - except (json.JSONDecodeError, ValueError) as error: - raise LLMReviewError("LLM returned invalid JSON.") from error - if not isinstance(data, dict): - raise LLMReviewError("LLM returned a non-object JSON response.") - return extract_chat_content(data) - - -def post_comment(repo: str, pr_number: str, token: str, body: str) -> None: - """Post a comment on the PR.""" - url = f"https://api.github.com/repos/{repo}/issues/{pr_number}/comments" - headers = { - "Authorization": f"Bearer {token}", - "Accept": "application/vnd.github.v3+json", - } - resp = requests.post(url, headers=headers, json={"body": body}, timeout=30) - resp.raise_for_status() - - -def main() -> None: - gh_token = os.environ.get("GITHUB_TOKEN", "") - repo = os.environ.get("REPO", "") - pr_number = os.environ.get("PR_NUMBER", "") - mode = os.environ.get("REVIEW_MODE", "default") - - if not all([gh_token, repo, pr_number]): - print("Missing required environment variables", file=sys.stderr) - sys.exit(1) - - print(f"Reviewing PR #{pr_number} in {repo} (mode: {mode})") - - # All other modes need the diff. - try: - diff = get_pr_diff(repo, pr_number, gh_token) - except requests.RequestException as e: - post_comment( - repo, pr_number, gh_token, f"**AI Review Error:** Failed to fetch PR diff: {e}" - ) - sys.exit(1) - - if not diff.strip(): - post_comment(repo, pr_number, gh_token, "**AI Review:** No diff found for this PR.") - return - - diff = truncate_diff(diff) - print(f"Diff size: {len(diff)} chars") - - # Select prompt. - prompts = { - "default": PROMPT_SECURITY, - "deep": PROMPT_SECURITY, - "tests": PROMPT_TESTS, - "docs": PROMPT_DOCS, - } - system_prompt = prompts.get(mode, PROMPT_SECURITY) - - try: - review = call_llm(diff, mode, system_prompt) - except (requests.RequestException, LLMReviewError) as e: - post_comment(repo, pr_number, gh_token, f"**AI Review Error:** {e}") - sys.exit(1) - - model_name = model_for_mode(mode) - - mode_labels = { - "default": "security", - "deep": "security deep", - "tests": "test coverage", - "docs": "docs accuracy", - } - label = mode_labels.get(mode, mode) - # The default mode is invoked as bare `/review`, not `/review default`, - # so the header omits the suffix in that case to match what the user - # actually typed. - cmd = "/review" if mode == "default" else f"/review {mode}" - header = f"## AI Review: {label} (`{cmd}`)\n\n**Model:** `{model_name}`\n\n---\n\n" - post_comment(repo, pr_number, gh_token, header + review) - print("Review posted.") - - -if __name__ == "__main__": - main() diff --git a/tests/test_pr_review_routing.py b/tests/test_pr_review_routing.py deleted file mode 100644 index a8f534b..0000000 --- a/tests/test_pr_review_routing.py +++ /dev/null @@ -1,149 +0,0 @@ -"""Regression tests for the trusted GitHub PR-review runner's model routing.""" - -from __future__ import annotations - -import importlib.util -import re -import unittest -from pathlib import Path -from types import ModuleType -from unittest.mock import Mock, patch - -ROOT = Path(__file__).resolve().parents[1] -SCRIPT_PATH = ROOT / "scripts" / "pr-review.py" -WORKFLOW_PATH = ROOT / ".github" / "workflows" / "pr-review.yaml" - - -def load_pr_review_module() -> ModuleType: - """Load the workflow script without executing its CLI entry point.""" - spec = importlib.util.spec_from_file_location("pr_review_routing", SCRIPT_PATH) - assert spec is not None - assert spec.loader is not None - module = importlib.util.module_from_spec(spec) - with patch.dict("sys.modules", {"requests": Mock()}): - spec.loader.exec_module(module) - return module - - -class PRReviewRoutingTests(unittest.TestCase): - def test_model_defaults_and_mode_routing(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, - clear=False, - ): - self.assertEqual(module.DEFAULT_MODEL_FAST, "gpt-5.6-luna") - self.assertEqual(module.DEFAULT_MODEL_DEEP, "gpt-5.6-terra") - self.assertEqual(module.model_for_mode("default"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("tests"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("docs"), "gpt-5.6-luna") - self.assertEqual(module.model_for_mode("deep"), "gpt-5.6-terra") - self.assertEqual(module.FAST_REASONING_EFFORT, "low") - self.assertEqual(module.DEEP_REASONING_EFFORT, "xhigh") - - def test_gpt_5_6_payloads_pin_the_requested_reasoning_effort(self): - module = load_pr_review_module() - - ordinary_payload = module.build_llm_payload( - module.DEFAULT_MODEL_FAST, - "system prompt", - "diff", - reasoning_effort=module.FAST_REASONING_EFFORT, - ) - deep_payload = module.build_llm_payload( - module.DEFAULT_MODEL_DEEP, - "system prompt", - "diff", - reasoning_effort=module.DEEP_REASONING_EFFORT, - max_completion_tokens=module.DEEP_MAX_COMPLETION_TOKENS, - ) - - self.assertEqual(ordinary_payload["model"], "gpt-5.6-luna") - self.assertEqual(ordinary_payload["reasoning_effort"], "low") - self.assertEqual(ordinary_payload["max_completion_tokens"], 8192) - self.assertEqual(deep_payload["model"], "gpt-5.6-terra") - self.assertEqual(deep_payload["reasoning_effort"], "xhigh") - self.assertEqual(deep_payload["max_completion_tokens"], 64000) - - def test_model_repository_variable_overrides(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - { - "PR_REVIEW_MODEL_FAST": "provider/ordinary-override", - "PR_REVIEW_MODEL_DEEP": "provider/deep-override", - }, - clear=False, - ): - self.assertEqual(module.model_for_mode("default"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("tests"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("docs"), "provider/ordinary-override") - self.assertEqual(module.model_for_mode("deep"), "provider/deep-override") - - def test_empty_repository_variable_overrides_use_python_defaults(self): - module = load_pr_review_module() - with patch.dict( - module.os.environ, - {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, - clear=False, - ): - self.assertEqual(module.model_for_mode("default"), module.DEFAULT_MODEL_FAST) - self.assertEqual(module.model_for_mode("deep"), module.DEFAULT_MODEL_DEEP) - - def test_workflow_delegates_model_defaults_to_python(self): - workflow = WORKFLOW_PATH.read_text(encoding="utf-8") - - self.assertIn("PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }}", workflow) - self.assertIn("PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }}", workflow) - self.assertIsNone(re.search(r"PR_REVIEW_MODEL_(?:FAST|DEEP): gpt-", workflow)) - - def test_workflow_keeps_trusted_runner_and_owner_gate(self): - workflow = WORKFLOW_PATH.read_text(encoding="utf-8") - - self.assertIn("github.event.comment.user.login == 'luckyPipewrench'", workflow) - self.assertIn("github.event.comment.author_association == 'OWNER'", workflow) - self.assertIn("ref: ${{ github.event.repository.default_branch }}", workflow) - self.assertIn("persist-credentials: false", workflow) - self.assertIn("LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }}", workflow) - self.assertIn("LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}", workflow) - self.assertIn("OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}", workflow) - self.assertIn( - "group: pr-review-${{ github.repository }}-${{ github.event.issue.number }}", workflow - ) - self.assertIn("cancel-in-progress: true", workflow) - self.assertIn("python -m unittest tests/test_pr_review_routing.py", workflow) - - runner = SCRIPT_PATH.read_text(encoding="utf-8") - self.assertNotIn("resp.text[:500]", runner) - - def test_response_shape_errors_are_generic_and_fail_closed(self): - module = load_pr_review_module() - - with self.assertRaises(module.LLMReviewError) as ctx: - module.extract_chat_content({"choices": [], "private": "provider detail"}) - self.assertIn("no choices", str(ctx.exception)) - self.assertNotIn("provider detail", str(ctx.exception)) - - with self.assertRaisesRegex(module.LLMReviewError, "empty content"): - module.extract_chat_content({"choices": [None]}) - - def test_call_rejects_invalid_or_non_object_json(self): - module = load_pr_review_module() - invalid = Mock(status_code=200) - invalid.json.side_effect = ValueError("invalid") - with ( - patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), - patch.object(module.requests, "post", return_value=invalid), - self.assertRaisesRegex(module.LLMReviewError, "invalid JSON"), - ): - module.call_llm("diff", "default", "system") - - non_object = Mock(status_code=200) - non_object.json.return_value = [] - with ( - patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), - patch.object(module.requests, "post", return_value=non_object), - self.assertRaisesRegex(module.LLMReviewError, "non-object JSON"), - ): - module.call_llm("diff", "default", "system") From 8cac02c81218b36920a6f3418efa29bf291b9677 Mon Sep 17 00:00:00 2001 From: luckyPipewrench Date: Sun, 16 Aug 2026 13:08:48 -0400 Subject: [PATCH 2/3] chore(ci): keep the superseded copy until the scanner can read its removal The local reviewer copy is dead once the caller above points at the shared workflow, but removing it in this pull request cannot pass the repository's own security scan. pipelock git scan-diff fails to parse a whole-file deletion hunk, and the action reports that parse failure as "Secrets detected in PR diff", so the build fails on a scanner error rather than on anything in the diff. The fix for that is already on Pipelock main and is in no release yet, so this change keeps the files and carries only the caller swap. Deleting them is a follow-up once a release carries the fix. --- .github/requirements-pr-review.txt | 146 ++++++++++++ scripts/pr-review.py | 361 +++++++++++++++++++++++++++++ tests/test_pr_review_routing.py | 149 ++++++++++++ 3 files changed, 656 insertions(+) create mode 100644 .github/requirements-pr-review.txt create mode 100644 scripts/pr-review.py create mode 100644 tests/test_pr_review_routing.py diff --git a/.github/requirements-pr-review.txt b/.github/requirements-pr-review.txt new file mode 100644 index 0000000..0c5b24d --- /dev/null +++ b/.github/requirements-pr-review.txt @@ -0,0 +1,146 @@ +# Pinned dependencies for pr-review.yaml workflow. +# All hashes from PyPI; includes py3-none-any and cp312 manylinux_x86_64 +# variants so the install works on GitHub Ubuntu runners. +# Regenerate: pip download --no-deps, then pip hash . +certifi==2026.4.22 \ + --hash=sha256:3cb2210c8f88ba2318d29b0388d1023c8492ff72ecdde4ebdaddbb13a31b1c4a \ + --hash=sha256:8d455352a37b71bf76a79caa83a3d6c25afee4a385d632127b6afb3963f1c580 +charset-normalizer==3.4.7 \ + --hash=sha256:007d05ec7321d12a40227aae9e2bc6dca73f3cb21058999a1df9e193555a9dcc \ + --hash=sha256:03853ed82eeebbce3c2abfdbc98c96dc205f32a79627688ac9a27370ea61a49c \ + --hash=sha256:07d9e39b01743c3717745f4c530a6349eadbfa043c7577eef86c502c15df2c67 \ + --hash=sha256:08e721811161356f97b4059a9ba7bafb23ea5ee2255402c42881c214e173c6b4 \ + --hash=sha256:0c96c3b819b5c3e9e165495db84d41914d6894d55181d2d108cc1a69bfc9cce0 \ + --hash=sha256:0ea948db76d31190bf08bd371623927ee1339d5f2a0b4b1b4a4439a65298703c \ + --hash=sha256:0f7eb884681e3938906ed0434f20c63046eacd0111c4ba96f27b76084cd679f5 \ + --hash=sha256:12a6fff75f6bc66711b73a2f0addfc4c8c15a20e805146a02d147a318962c444 \ + --hash=sha256:12d8baf840cc7889b37c7c770f478adea7adce3dcb3944d02ec87508e2dcf153 \ + --hash=sha256:14265bfe1f09498b9d8ec91e9ec9fa52775edf90fcbde092b25f4a33d444fea9 \ + --hash=sha256:16d971e29578a5e97d7117866d15889a4a07befe0e87e703ed63cd90cb348c01 \ + --hash=sha256:177a0ba5f0211d488e295aaf82707237e331c24788d8d76c96c5a41594723217 \ + --hash=sha256:1a87ca9d5df6fe460483d9a5bbf2b18f620cbed41b432e2bddb686228282d10b \ + --hash=sha256:1c2a768fdd44ee4a9339a9b0b130049139b8ce3c01d2ce09f67f5a68048d477c \ + --hash=sha256:1c2aed2e5e41f24ea8ef1590b8e848a79b56f3a5564a65ceec43c9d692dc7d8a \ + --hash=sha256:1dc8b0ea451d6e69735094606991f32867807881400f808a106ee1d963c46a83 \ + --hash=sha256:1efde3cae86c8c273f1eb3b287be7d8499420cf2fe7585c41d370d3e790054a5 \ + --hash=sha256:202389074300232baeb53ae2569a60901f7efadd4245cf3a3bf0617d60b439d7 \ + --hash=sha256:203104ed3e428044fd943bc4bf45fa73c0730391f9621e37fe39ecf477b128cb \ + --hash=sha256:2257141f39fe65a3fdf38aeccae4b953e5f3b3324f4ff0daf9f15b8518666a2c \ + --hash=sha256:298930cec56029e05497a76988377cbd7457ba864beeea92ad7e844fe74cd1f1 \ + --hash=sha256:2cd4a60d0e2fb04537162c62bbbb4182f53541fe0ede35cdf270a1c1e723cc42 \ + --hash=sha256:2d6eb928e13016cea4f1f21d1e10c1cebd5a421bc57ddf5b1142ae3f86824fab \ + --hash=sha256:2fe249cb4651fd12605b7288b24751d8bfd46d35f12a20b1ba33dea122e690df \ + --hash=sha256:30b8d1d8c52a48c2c5690e152c169b673487a2a58de1ec7393196753063fcd5e \ + --hash=sha256:320ade88cfb846b8cd6b4ddf5ee9e80ee0c1f52401f2456b84ae1ae6a1a5f207 \ + --hash=sha256:3534e7dcbdcf757da6b85a0bbf5b6868786d5982dd959b065e65481644817a18 \ + --hash=sha256:36836d6ff945a00b88ba1e4572d721e60b5b8c98c155d465f56ad19d68f23734 \ + --hash=sha256:38c0109396c4cfc574d502df99742a45c72c08eff0a36158b6f04000043dbf38 \ + --hash=sha256:3946fa46a0cf3e4c8cb1cc52f56bb536310d34f25f01ca9b6c16afa767dab110 \ + --hash=sha256:3bec022aec2c514d9cf199522a802bd007cd588ab17ab2525f20f9c34d067c18 \ + --hash=sha256:3c9a494bc5ec77d43cea229c4f6db1e4d8fe7e1bbffa8b6f0f0032430ff8ab44 \ + --hash=sha256:3dce51d0f5e7951f8bb4900c257dad282f49190fdbebecd4ba99bcc41fef404d \ + --hash=sha256:3dedcc22d73ec993f42055eff4fcfed9318d1eeb9a6606c55892a26964964e48 \ + --hash=sha256:4042d5c8f957e15221d423ba781e85d553722fc4113f523f2feb7b188cc34c5e \ + --hash=sha256:481551899c856c704d58119b5025793fa6730adda3571971af568f66d2424bb5 \ + --hash=sha256:4dc1e73c36828f982bfe79fadf5919923f8a6f4df2860804db9a98c48824ce8d \ + --hash=sha256:4e5163c14bffd570ef2affbfdd77bba66383890797df43dc8b4cc7d6f500bf53 \ + --hash=sha256:511ef87c8aec0783e08ac18565a16d435372bc1ac25a91e6ac7f5ef2b0bff790 \ + --hash=sha256:532bc9bf33a68613fd7d65e4b1c71a6a38d7d42604ecf239c77392e9b4e8998c \ + --hash=sha256:54523e136b8948060c0fa0bc7b1b50c32c186f2fceee897a495406bb6e311d2b \ + --hash=sha256:5649fd1c7bade02f320a462fdefd0b4bd3ce036065836d4f42e0de958038e116 \ + --hash=sha256:56be790f86bfb2c98fb742ce566dfb4816e5a83384616ab59c49e0604d49c51d \ + --hash=sha256:5b77459df20e08151cd6f8b9ef8ef1f961ef73d85c21a555c7eed5b79410ec10 \ + --hash=sha256:5ed6ab538499c8644b8a3e18debabcd7ce684f3fa91cf867521a7a0279cab2d6 \ + --hash=sha256:6178f72c5508bfc5fd446a5905e698c6212932f25bcdd4b47a757a50605a90e2 \ + --hash=sha256:6370e8686f662e6a3941ee48ed4742317cafbe5707e36406e9df792cdb535776 \ + --hash=sha256:64f02c6841d7d83f832cd97ccf8eb8a906d06eb95d5276069175c696b024b60a \ + --hash=sha256:65bcd23054beab4d166035cabbc868a09c1a49d1efe458fe8e4361215df40265 \ + --hash=sha256:66671f93accb62ed07da56613636f3641f1a12c13046ce91ffc923721f23c008 \ + --hash=sha256:6696b7688f54f5af4462118f0bfa7c1621eeb87154f77fa04b9295ce7a8f2943 \ + --hash=sha256:6785f414ae0f3c733c437e0f3929197934f526d19dfaa75e18fdb4f94c6fb374 \ + --hash=sha256:67f6279d125ca0046a7fd386d01b311c6363844deac3e5b069b514ba3e63c246 \ + --hash=sha256:6c114670c45346afedc0d947faf3c7f701051d2518b943679c8ff88befe14f8e \ + --hash=sha256:6e0d51f618228538a3e8f46bd246f87a6cd030565e015803691603f55e12afb5 \ + --hash=sha256:6ed74185b2db44f41ef35fd1617c5888e59792da9bbc9190d6c7300617182616 \ + --hash=sha256:708838739abf24b2ceb208d0e22403dd018faeef86ddac04319a62ae884c4f15 \ + --hash=sha256:715479b9a2802ecac752a3b0efa2b0b60285cf962ee38414211abdfccc233b41 \ + --hash=sha256:733784b6d6def852c814bce5f318d25da2ee65dd4839a0718641c696e09a2960 \ + --hash=sha256:750e02e074872a3fad7f233b47734166440af3cdea0add3e95163110816d6752 \ + --hash=sha256:752a45dc4a6934060b3b0dab47e04edc3326575f82be64bc4fc293914566503e \ + --hash=sha256:7579e913a5339fb8fa133f6bbcfd8e6749696206cf05acdbdca71a1b436d8e72 \ + --hash=sha256:7641bb8895e77f921102f72833904dcd9901df5d6d72a2ab8f31d04b7e51e4e7 \ + --hash=sha256:7804338df6fcc08105c7745f1502ba68d900f45fd770d5bdd5288ddccb8a42d8 \ + --hash=sha256:80d04837f55fc81da168b98de4f4b797ef007fc8a79ab71c6ec9bc4dd662b15b \ + --hash=sha256:813c0e0132266c08eb87469a642cb30aaff57c5f426255419572aaeceeaa7bf4 \ + --hash=sha256:82b271f5137d07749f7bf32f70b17ab6eaabedd297e75dce75081a24f76eb545 \ + --hash=sha256:84c018e49c3bf790f9c2771c45e9313a08c2c2a6342b162cd650258b57817706 \ + --hash=sha256:8751d2787c9131302398b11e6c8068053dcb55d5a8964e114b6e196cf16cb366 \ + --hash=sha256:8778f0c7a52e56f75d12dae53ae320fae900a8b9b4164b981b9c5ce059cd1fcb \ + --hash=sha256:87fad7d9ba98c86bcb41b2dc8dbb326619be2562af1f8ff50776a39e55721c5a \ + --hash=sha256:8d828b6667a32a728a1ad1d93957cdf37489c57b97ae6c4de2860fa749b8fc1e \ + --hash=sha256:8e385e4267ab76874ae30db04c627faaaf0b509e1ccc11a95b3fc3e83f855c00 \ + --hash=sha256:92a0a01ead5e668468e952e4238cccd7c537364eb7d851ab144ab6627dbbe12f \ + --hash=sha256:94e1885b270625a9a828c9793b4d52a64445299baa1fea5a173bf1d3dd9a1a5a \ + --hash=sha256:a180c5e59792af262bf263b21a3c49353f25945d8d9f70628e73de370d55e1e1 \ + --hash=sha256:a277ab8928b9f299723bc1a2dabb1265911b1a76341f90a510368ca44ad9ab66 \ + --hash=sha256:a5fe03b42827c13cdccd08e6c0247b6a6d4b5e3cdc53fd1749f5896adcdc2356 \ + --hash=sha256:a6c5863edfbe888d9eff9c8b8087354e27618d9da76425c119293f11712a6319 \ + --hash=sha256:a89c23ef8d2c6b27fd200a42aa4ac72786e7c60d40efdc76e6011260b6e949c4 \ + --hash=sha256:adb2597b428735679446b46c8badf467b4ca5f5056aae4d51a19f9570301b1ad \ + --hash=sha256:ae196f021b5e7c78e918242d217db021ed2a6ace2bc6ae94c0fc596221c7f58d \ + --hash=sha256:ae89db9e5f98a11a4bf50407d4363e7b09b31e55bc117b4f7d80aab97ba009e5 \ + --hash=sha256:aed52fea0513bac0ccde438c188c8a471c4e0f457c2dd20cdbf6ea7a450046c7 \ + --hash=sha256:aef65cd602a6d0e0ff6f9930fcb1c8fec60dd2cfcb6facaf4bdb0e5873042db0 \ + --hash=sha256:af21eb4409a119e365397b2adbaca4c9ccab56543a65d5dbd9f920d6ac29f686 \ + --hash=sha256:b14b2d9dac08e28bb8046a1a0434b1750eb221c8f5b87a68f4fa11a6f97b5e34 \ + --hash=sha256:bb6d88045545b26da47aa879dd4a89a71d1dce0f0e549b1abcb31dfe4a8eac49 \ + --hash=sha256:bb8cc7534f51d9a017b93e3e85b260924f909601c3df002bcdb58ddb4dc41a5c \ + --hash=sha256:bc17a677b21b3502a21f66a8cc64f5bfad4df8a0b8434d661666f8ce90ac3af1 \ + --hash=sha256:bd6c2a1c7573c64738d716488d2cdd3c00e340e4835707d8fdb8dc1a66ef164e \ + --hash=sha256:bd9b23791fe793e4968dba0c447e12f78e425c59fc0e3b97f6450f4781f3ee60 \ + --hash=sha256:c03a41a8784091e67a39648f70c5f97b5b6a37f216896d44d2cdcb82615339a0 \ + --hash=sha256:c0f081d69a6e58272819b70288d3221a6ee64b98df852631c80f293514d3b274 \ + --hash=sha256:c35abb8bfff0185efac5878da64c45dafd2b37fb0383add1be155a763c1f083d \ + --hash=sha256:c36c333c39be2dbca264d7803333c896ab8fa7d4d6f0ab7edb7dfd7aea6e98c0 \ + --hash=sha256:c45e9440fb78f8ddabcf714b68f936737a121355bf59f3907f4e17721b9d1aae \ + --hash=sha256:c593052c465475e64bbfe5dbd81680f64a67fdc752c56d7a0ae205dc8aeefe0f \ + --hash=sha256:cdd68a1fb318e290a2077696b7eb7a21a49163c455979c639bf5a5dcdc46617d \ + --hash=sha256:ce3412fbe1e31eb81ea42f4169ed94861c56e643189e1e75f0041f3fe7020abe \ + --hash=sha256:cf1493cd8607bec4d8a7b9b004e699fcf8f9103a9284cc94962cb73d20f9d4a3 \ + --hash=sha256:cf29836da5119f3c8a8a70667b0ef5fdca3bb12f80fd06487cfa575b3909b393 \ + --hash=sha256:d4a48e5b3c2a489fae013b7589308a40146ee081f6f509e047e0e096084ceca1 \ + --hash=sha256:d560742f3c0d62afaccf9f41fe485ed69bd7661a241f86a3ef0f0fb8b1a397af \ + --hash=sha256:d6038d37043bced98a66e68d3aa2b6a35505dc01328cd65217cefe82f25def44 \ + --hash=sha256:d61f00a0869d77422d9b2aba989e2d24afa6ffd552af442e0e58de4f35ea6d00 \ + --hash=sha256:d635aab80466bc95771bb78d5370e74d36d1fe31467b6b29b8b57b2a3cd7d22c \ + --hash=sha256:dca4bbc466a95ba9c0234ef56d7dd9509f63da22274589ebd4ed7f1f4d4c54e3 \ + --hash=sha256:dd915403e231e6b1809fe9b6d9fc55cf8fb5e02765ac625d9cd623342a7905d7 \ + --hash=sha256:e044c39e41b92c845bc815e5ae4230804e8e7bc29e399b0437d64222d92809dd \ + --hash=sha256:e060d01aec0a910bdccb8be71faf34e7799ce36950f8294c8bf612cba65a2c9e \ + --hash=sha256:e1421b502d83040e6d7fb2fb18dff63957f720da3d77b2fbd3187ceb63755d7b \ + --hash=sha256:e17b8d5d6a8c47c85e68ca8379def1303fd360c3e22093a807cd34a71cd082b8 \ + --hash=sha256:e5f4d355f0a2b1a31bc3edec6795b46324349c9cb25eed068049e4f472fb4259 \ + --hash=sha256:e712b419df8ba5e42b226c510472b37bd57b38e897d3eca5e8cfd410a29fa859 \ + --hash=sha256:e74327fb75de8986940def6e8dee4f127cc9752bee7355bb323cc5b2659b6d46 \ + --hash=sha256:e80c8378d8f3d83cd3164da1ad2df9e37a666cdde7b1cb2298ed0b558064be30 \ + --hash=sha256:e8ac484bf18ce6975760921bb6148041faa8fef0547200386ea0b52b5d27bf7b \ + --hash=sha256:eca9705049ad3c7345d574e3510665cb2cf844c2f2dcfe675332677f081cbd46 \ + --hash=sha256:ed065083d0898c9d5b4bbec7b026fd755ff7454e6e8b73a67f8c744b13986e24 \ + --hash=sha256:edac0f1ab77644605be2cbba52e6b7f630731fc42b34cb0f634be1a6eface56a \ + --hash=sha256:effc3f449787117233702311a1b7d8f59cba9ced946ba727bdc329ec69028e24 \ + --hash=sha256:f22dec1690b584cea26fade98b2435c132c1b5f68e39f5a0b7627cd7ae31f1dc \ + --hash=sha256:f495a1652cf3fbab2eb0639776dad966c2fb874d79d87ca07f9d5f059b8bd215 \ + --hash=sha256:f496c9c3cc02230093d8330875c4c3cdfc3b73612a5fd921c65d39cbcef08063 \ + --hash=sha256:f59099f9b66f0d7145115e6f80dd8b1d847176df89b234a5a6b3f00437aa0832 \ + --hash=sha256:f59ad4c0e8f6bba240a9bb85504faa1ab438237199d4cce5f622761507b8f6a6 \ + --hash=sha256:fbccdc05410c9ee21bbf16a35f4c1d16123dcdeb8a1d38f33654fa21d0234f79 \ + --hash=sha256:fea24543955a6a729c45a73fe90e08c743f0b3334bbf3201e6c4bc1b0c7fa464 +idna==3.15 \ + --hash=sha256:048adeaf8c2d788c40fee287673ccaa74c24ffd8dcf09ffa555a2fbb59f10ac8 \ + --hash=sha256:ca962446ea538f7092a95e057da437618e886f4d349216d2b1e294abfdb65fdc +requests==2.34.0 \ + --hash=sha256:7d62fe92f50eb82c529b0916bb445afa1531a566fc8f35ffdc64446e771b856a \ + --hash=sha256:917520a21b767485ce7c588f4ebb917c436b24a31231b44228715eaeb5a52c60 +urllib3==2.7.0 \ + --hash=sha256:231e0ec3b63ceb14667c67be60f2f2c40a518cb38b03af60abc813da26505f4c \ + --hash=sha256:9fb4c81ebbb1ce9531cce37674bbc6f1360472bc18ca9a553ede278ef7276897 diff --git a/scripts/pr-review.py b/scripts/pr-review.py new file mode 100644 index 0000000..55fba73 --- /dev/null +++ b/scripts/pr-review.py @@ -0,0 +1,361 @@ +#!/usr/bin/env python3 +# Copyright 2026 Pipelock contributors +# SPDX-License-Identifier: Apache-2.0 + +"""AI-powered PR review for the pipelock-verify Python package. + +Triggered by /review comments on PRs. Supports multiple review modes: + /review - Security and correctness review (default model) + /review deep - Deeper review (higher-capacity model) + /review tests - Test coverage and boundary analysis + /review docs - Documentation accuracy check + +Requires environment variables: + GITHUB_TOKEN - GitHub token (provided by Actions) + REPO - owner/repo + PR_NUMBER - PR number + REVIEW_MODE - "default", "deep", "tests", or "docs" + +LLM configuration (one of): + LITELLM_BASE_URL + LITELLM_API_KEY - LiteLLM proxy + OPENAI_API_KEY - Direct OpenAI API + +Model selection: + PR_REVIEW_MODEL_FAST - Optional model override for default/tests/docs + (default: gpt-5.6-luna) + PR_REVIEW_MODEL_DEEP - Optional model override for /review deep + (default: gpt-5.6-terra) + +The PR_REVIEW_MODEL_FAST env var keeps its name for backwards compatibility +with existing repository-variable overrides; the user-facing /review fast +alias was dropped 2026-04-23 because the default mode is sufficient. +""" + +import json +import os +import sys + +import requests + +# --- Constants --- + +MAX_DIFF_CHARS = 100_000 +# These are the sole default model definitions. GitHub Actions may supply +# optional repository-variable overrides, but an unset or empty override must +# fall back here so workflow configuration cannot drift from local behavior. +DEFAULT_MODEL_FAST = "gpt-5.6-luna" +DEFAULT_MODEL_DEEP = "gpt-5.6-terra" +DEFAULT_TEMPERATURE = 0.2 +DEFAULT_MAX_COMPLETION_TOKENS = 8192 +# max_completion_tokens is shared by reasoning and visible output. At xhigh +# effort, 25000 was consumed by reasoning alone and produced an empty review. +DEEP_MAX_COMPLETION_TOKENS = 64000 +DEFAULT_LLM_TIMEOUT_SECONDS = 120 +DEEP_LLM_TIMEOUT_SECONDS = 300 +FAST_REASONING_EFFORT = "low" +DEEP_REASONING_EFFORT = "xhigh" + + +class LLMReviewError(RuntimeError): + """Raised when the LLM call completed but did not produce a usable review.""" + + +PROMPT_SECURITY = """You are reviewing a pull request for pipelock-verify, the Python verifier for Pipelock action receipts: Ed25519-signed, hash-chained records that an independent party uses to check what an AI agent actually did. + +This library's defining property is BYTE-EXACT PARITY with the Go emitter. The signing input is the SHA-256 of a canonical JSON projection of the action record, reproduced to match Go's encoding/json byte for byte. Any divergence in field set, field order, omitempty semantics, escaping, or number spelling changes the hash and breaks verification. There is no slack. + +Two failure directions both matter, and they are not symmetric: +- FAIL-OPEN: accepting something Go rejects. A receipt that verifies here but not in Go means this tool blesses evidence the emitter considers invalid. Treat as high severity. +- FAIL-CLOSED: rejecting something Go accepts. This breaks honest operators and makes the tool untrustworthy in the other direction. Still a real defect, usually medium. + +Flag: +- any change to the canonical projection: added, removed, reordered, or re-typed fields, and whether omitempty matches Go +- signature or chain-hash construction changes, including what is and is not covered by each +- validation that accepts a shape Go's strict Unmarshal would reject, or rejects one it accepts +- parser-differential surfaces: duplicate keys, trailing tokens, unicode normalization, HTML escaping, integer bounds, float spelling, empty versus absent +- anywhere parsed-and-re-serialized data is used where the producer's original bytes are what actually get hashed +- guards that cannot be reached, or that no test would catch the removal of +- error paths that surface as tracebacks rather than a clean invalid result on attacker-supplied input + +Do not waste time on style nits. +For each finding, include: +1. severity: high, medium, or low +2. file and function +3. why it matters, and which direction it fails +4. a concrete fix + +If there are no material issues, say exactly: No material security or correctness issues found in this diff.""" + +PROMPT_TESTS = """You are reviewing the TEST COVERAGE of a pull request for pipelock-verify, a Python verifier for Ed25519-signed, hash-chained Pipelock receipts. + +The single most important question: WOULD THIS TEST FAIL IF THE THING IT GUARDS WERE DELETED? A test that passes whether or not the guard exists is worse than no test, because it reports safety it does not provide. Say so explicitly wherever you suspect it. + +For each code change, check: + +1. **Vacuity**: for every new guard or validation, is there a test that fails when that guard is removed? If the guard is only reachable through one entry point, does a test actually use that entry point? +2. **Parity, not just validity**: chain tests that assert only "valid" do not catch a divergence in what the chain commits to. Is the root hash or canonical output pinned to an independently computed value? +3. **Real emitter output**: is parity proven against a captured real chain, or only against hand-written data that can drift alongside the code it checks? +4. **Both directions**: for a new rejection, is there a positive test proving legitimate input still passes? Over-tightening is a real defect. +5. **Boundaries**: integer limits including int64 and uint64 edges, empty versus absent, null, empty object and array, non-sorted keys, unicode and HTML-escapable characters, very large values. +6. **Error paths**: are new error returns exercised? + +For each gap: +1. severity: high (untested guard or unproven parity), medium (untested boundary), low (nice-to-have) +2. file and line +3. the specific missing case +4. a concrete test to add, including input and expected result + +If coverage is adequate, say exactly: Test coverage is adequate for this diff.""" + +PROMPT_DOCS = """You are reviewing a pull request for pipelock-verify for DOCUMENTATION ACCURACY. + +Check every claim in the diff against the code: + +1. **Contract claims**: statements about which fields are signed, what the chain hash covers, or how canonicalization works must match the actual implementation and the Go source it mirrors. +2. **Source-of-truth pointers**: comments naming a Go file or struct as the contract must name the file that actually defines it. A pointer to the wrong file is how drift goes unnoticed. +3. **Capability claims**: if docs say something is verified, enforced, or guaranteed, confirm the code does it. Flag anything that overstates what an offline verifier can prove. +4. **Stated limitations**: known gaps should be documented plainly rather than implied or omitted. +5. **Examples**: sample code and CLI invocations must actually run as written. +6. **Stale references**: removed functions, renamed parameters, old behavior. + +For each issue: +1. severity: high (wrong claim about what is verified), medium (stale or misleading), low (unclear) +2. file and line +3. what it says versus what the code shows +4. the correct statement + +If documentation is accurate, say exactly: Documentation accurately reflects the codebase in this diff.""" + + +def get_pr_diff(repo: str, pr_number: str, token: str) -> str: + """Fetch the PR diff from GitHub.""" + url = f"https://api.github.com/repos/{repo}/pulls/{pr_number}" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github.v3.diff", + } + resp = requests.get(url, headers=headers, timeout=30) + resp.raise_for_status() + return resp.text + + +def truncate_diff(diff: str, max_chars: int = MAX_DIFF_CHARS) -> str: + """Truncate diff to stay within token limits.""" + if len(diff) <= max_chars: + return diff + truncated = diff[:max_chars] + return truncated + f"\n\n... (diff truncated at {max_chars} chars, {len(diff)} total)" + + +def model_supports_custom_temperature(model: str) -> bool: + """Return whether chat completions should send a non-default temperature.""" + normalized = model.strip().lower() + model_name = normalized.rsplit("/", 1)[-1] + return not model_name.startswith(("gpt-5", "o1", "o3", "o4")) + + +def model_supports_reasoning_effort(model: str) -> bool: + """Return whether chat completions should pin reasoning effort.""" + normalized = model.strip().lower() + model_name = normalized.rsplit("/", 1)[-1] + return model_name.startswith(("gpt-5", "o1", "o3", "o4")) + + +def build_llm_payload( + model: str, + system_prompt: str, + diff: str, + *, + max_completion_tokens: int = DEFAULT_MAX_COMPLETION_TOKENS, + reasoning_effort: str = FAST_REASONING_EFFORT, +) -> dict: + """Build the chat completions payload for the selected review model.""" + payload = { + "model": model, + "messages": [ + {"role": "system", "content": system_prompt}, + { + "role": "user", + "content": f"Review this pull request diff:\n\n```diff\n{diff}\n```", + }, + ], + "max_completion_tokens": max_completion_tokens, + } + if model_supports_custom_temperature(model): + payload["temperature"] = DEFAULT_TEMPERATURE + if model_supports_reasoning_effort(model): + payload["reasoning_effort"] = reasoning_effort + return payload + + +def model_for_mode(mode: str) -> str: + """Return the configured model for a review mode, with Python defaults.""" + if mode == "deep": + return os.environ.get("PR_REVIEW_MODEL_DEEP") or DEFAULT_MODEL_DEEP + return os.environ.get("PR_REVIEW_MODEL_FAST") or DEFAULT_MODEL_FAST + + +def summarize_usage(data: dict) -> str: + """Return compact token usage details for operator-visible errors.""" + usage = data.get("usage") + if not isinstance(usage, dict): + return "usage unavailable" + details = usage.get("completion_tokens_details") or {} + parts = [ + f"prompt={usage.get('prompt_tokens', 'unknown')}", + f"completion={usage.get('completion_tokens', 'unknown')}", + f"total={usage.get('total_tokens', 'unknown')}", + ] + if isinstance(details, dict) and "reasoning_tokens" in details: + parts.append(f"reasoning={details['reasoning_tokens']}") + return ", ".join(parts) + + +def extract_chat_content(data: dict) -> str: + """Extract visible text from a chat-completions response.""" + choices = data.get("choices", []) + if not isinstance(choices, list) or not choices: + raise LLMReviewError("LLM returned no choices.") + + choice = choices[0] if isinstance(choices[0], dict) else {} + message = choice.get("message") if isinstance(choice.get("message"), dict) else {} + content = message.get("content", "") + if isinstance(content, list): + content = "".join(part.get("text", "") for part in content if isinstance(part, dict)) + if isinstance(content, str) and content.strip(): + if choice.get("finish_reason") == "length": + content += ( + "\n\n> **Warning:** Review output was truncated by the model " + f"completion limit ({summarize_usage(data)}). Treat this as an " + "incomplete review and rerun with a narrower diff if needed." + ) + return content + + finish_reason = choice.get("finish_reason", "unknown") + raise LLMReviewError( + f"LLM returned empty content (finish_reason={finish_reason}; {summarize_usage(data)})." + ) + + +def call_llm(diff: str, mode: str, system_prompt: str) -> str: + """Send the diff to the LLM and return the review.""" + litellm_url = os.environ.get("LITELLM_BASE_URL", "") + litellm_key = os.environ.get("LITELLM_API_KEY", "") + openai_key = os.environ.get("OPENAI_API_KEY", "") + + model = model_for_mode(mode) + + if litellm_url and litellm_key: + api_url = litellm_url.rstrip("/") + "/chat/completions" + bearer = litellm_key + elif openai_key: + api_url = "https://api.openai.com/v1/chat/completions" + bearer = openai_key + else: + raise LLMReviewError( + "No LLM API configured. Set LITELLM_BASE_URL + LITELLM_API_KEY " + "or OPENAI_API_KEY in repo secrets." + ) + + headers = { + "Authorization": f"Bearer {bearer}", + "Content-Type": "application/json", + } + is_deep = mode == "deep" + max_completion_tokens = DEEP_MAX_COMPLETION_TOKENS if is_deep else DEFAULT_MAX_COMPLETION_TOKENS + reasoning_effort = DEEP_REASONING_EFFORT if is_deep else FAST_REASONING_EFFORT + payload = build_llm_payload( + model, + system_prompt, + diff, + max_completion_tokens=max_completion_tokens, + reasoning_effort=reasoning_effort, + ) + + timeout = DEEP_LLM_TIMEOUT_SECONDS if is_deep else DEFAULT_LLM_TIMEOUT_SECONDS + resp = requests.post(api_url, headers=headers, json=payload, timeout=timeout) + if resp.status_code != 200: + raise LLMReviewError(f"LLM API returned {resp.status_code} for model `{model}`.") + try: + data = resp.json() + except (json.JSONDecodeError, ValueError) as error: + raise LLMReviewError("LLM returned invalid JSON.") from error + if not isinstance(data, dict): + raise LLMReviewError("LLM returned a non-object JSON response.") + return extract_chat_content(data) + + +def post_comment(repo: str, pr_number: str, token: str, body: str) -> None: + """Post a comment on the PR.""" + url = f"https://api.github.com/repos/{repo}/issues/{pr_number}/comments" + headers = { + "Authorization": f"Bearer {token}", + "Accept": "application/vnd.github.v3+json", + } + resp = requests.post(url, headers=headers, json={"body": body}, timeout=30) + resp.raise_for_status() + + +def main() -> None: + gh_token = os.environ.get("GITHUB_TOKEN", "") + repo = os.environ.get("REPO", "") + pr_number = os.environ.get("PR_NUMBER", "") + mode = os.environ.get("REVIEW_MODE", "default") + + if not all([gh_token, repo, pr_number]): + print("Missing required environment variables", file=sys.stderr) + sys.exit(1) + + print(f"Reviewing PR #{pr_number} in {repo} (mode: {mode})") + + # All other modes need the diff. + try: + diff = get_pr_diff(repo, pr_number, gh_token) + except requests.RequestException as e: + post_comment( + repo, pr_number, gh_token, f"**AI Review Error:** Failed to fetch PR diff: {e}" + ) + sys.exit(1) + + if not diff.strip(): + post_comment(repo, pr_number, gh_token, "**AI Review:** No diff found for this PR.") + return + + diff = truncate_diff(diff) + print(f"Diff size: {len(diff)} chars") + + # Select prompt. + prompts = { + "default": PROMPT_SECURITY, + "deep": PROMPT_SECURITY, + "tests": PROMPT_TESTS, + "docs": PROMPT_DOCS, + } + system_prompt = prompts.get(mode, PROMPT_SECURITY) + + try: + review = call_llm(diff, mode, system_prompt) + except (requests.RequestException, LLMReviewError) as e: + post_comment(repo, pr_number, gh_token, f"**AI Review Error:** {e}") + sys.exit(1) + + model_name = model_for_mode(mode) + + mode_labels = { + "default": "security", + "deep": "security deep", + "tests": "test coverage", + "docs": "docs accuracy", + } + label = mode_labels.get(mode, mode) + # The default mode is invoked as bare `/review`, not `/review default`, + # so the header omits the suffix in that case to match what the user + # actually typed. + cmd = "/review" if mode == "default" else f"/review {mode}" + header = f"## AI Review: {label} (`{cmd}`)\n\n**Model:** `{model_name}`\n\n---\n\n" + post_comment(repo, pr_number, gh_token, header + review) + print("Review posted.") + + +if __name__ == "__main__": + main() diff --git a/tests/test_pr_review_routing.py b/tests/test_pr_review_routing.py new file mode 100644 index 0000000..a8f534b --- /dev/null +++ b/tests/test_pr_review_routing.py @@ -0,0 +1,149 @@ +"""Regression tests for the trusted GitHub PR-review runner's model routing.""" + +from __future__ import annotations + +import importlib.util +import re +import unittest +from pathlib import Path +from types import ModuleType +from unittest.mock import Mock, patch + +ROOT = Path(__file__).resolve().parents[1] +SCRIPT_PATH = ROOT / "scripts" / "pr-review.py" +WORKFLOW_PATH = ROOT / ".github" / "workflows" / "pr-review.yaml" + + +def load_pr_review_module() -> ModuleType: + """Load the workflow script without executing its CLI entry point.""" + spec = importlib.util.spec_from_file_location("pr_review_routing", SCRIPT_PATH) + assert spec is not None + assert spec.loader is not None + module = importlib.util.module_from_spec(spec) + with patch.dict("sys.modules", {"requests": Mock()}): + spec.loader.exec_module(module) + return module + + +class PRReviewRoutingTests(unittest.TestCase): + def test_model_defaults_and_mode_routing(self): + module = load_pr_review_module() + with patch.dict( + module.os.environ, + {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, + clear=False, + ): + self.assertEqual(module.DEFAULT_MODEL_FAST, "gpt-5.6-luna") + self.assertEqual(module.DEFAULT_MODEL_DEEP, "gpt-5.6-terra") + self.assertEqual(module.model_for_mode("default"), "gpt-5.6-luna") + self.assertEqual(module.model_for_mode("tests"), "gpt-5.6-luna") + self.assertEqual(module.model_for_mode("docs"), "gpt-5.6-luna") + self.assertEqual(module.model_for_mode("deep"), "gpt-5.6-terra") + self.assertEqual(module.FAST_REASONING_EFFORT, "low") + self.assertEqual(module.DEEP_REASONING_EFFORT, "xhigh") + + def test_gpt_5_6_payloads_pin_the_requested_reasoning_effort(self): + module = load_pr_review_module() + + ordinary_payload = module.build_llm_payload( + module.DEFAULT_MODEL_FAST, + "system prompt", + "diff", + reasoning_effort=module.FAST_REASONING_EFFORT, + ) + deep_payload = module.build_llm_payload( + module.DEFAULT_MODEL_DEEP, + "system prompt", + "diff", + reasoning_effort=module.DEEP_REASONING_EFFORT, + max_completion_tokens=module.DEEP_MAX_COMPLETION_TOKENS, + ) + + self.assertEqual(ordinary_payload["model"], "gpt-5.6-luna") + self.assertEqual(ordinary_payload["reasoning_effort"], "low") + self.assertEqual(ordinary_payload["max_completion_tokens"], 8192) + self.assertEqual(deep_payload["model"], "gpt-5.6-terra") + self.assertEqual(deep_payload["reasoning_effort"], "xhigh") + self.assertEqual(deep_payload["max_completion_tokens"], 64000) + + def test_model_repository_variable_overrides(self): + module = load_pr_review_module() + with patch.dict( + module.os.environ, + { + "PR_REVIEW_MODEL_FAST": "provider/ordinary-override", + "PR_REVIEW_MODEL_DEEP": "provider/deep-override", + }, + clear=False, + ): + self.assertEqual(module.model_for_mode("default"), "provider/ordinary-override") + self.assertEqual(module.model_for_mode("tests"), "provider/ordinary-override") + self.assertEqual(module.model_for_mode("docs"), "provider/ordinary-override") + self.assertEqual(module.model_for_mode("deep"), "provider/deep-override") + + def test_empty_repository_variable_overrides_use_python_defaults(self): + module = load_pr_review_module() + with patch.dict( + module.os.environ, + {"PR_REVIEW_MODEL_FAST": "", "PR_REVIEW_MODEL_DEEP": ""}, + clear=False, + ): + self.assertEqual(module.model_for_mode("default"), module.DEFAULT_MODEL_FAST) + self.assertEqual(module.model_for_mode("deep"), module.DEFAULT_MODEL_DEEP) + + def test_workflow_delegates_model_defaults_to_python(self): + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + + self.assertIn("PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }}", workflow) + self.assertIn("PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }}", workflow) + self.assertIsNone(re.search(r"PR_REVIEW_MODEL_(?:FAST|DEEP): gpt-", workflow)) + + def test_workflow_keeps_trusted_runner_and_owner_gate(self): + workflow = WORKFLOW_PATH.read_text(encoding="utf-8") + + self.assertIn("github.event.comment.user.login == 'luckyPipewrench'", workflow) + self.assertIn("github.event.comment.author_association == 'OWNER'", workflow) + self.assertIn("ref: ${{ github.event.repository.default_branch }}", workflow) + self.assertIn("persist-credentials: false", workflow) + self.assertIn("LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }}", workflow) + self.assertIn("LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}", workflow) + self.assertIn("OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}", workflow) + self.assertIn( + "group: pr-review-${{ github.repository }}-${{ github.event.issue.number }}", workflow + ) + self.assertIn("cancel-in-progress: true", workflow) + self.assertIn("python -m unittest tests/test_pr_review_routing.py", workflow) + + runner = SCRIPT_PATH.read_text(encoding="utf-8") + self.assertNotIn("resp.text[:500]", runner) + + def test_response_shape_errors_are_generic_and_fail_closed(self): + module = load_pr_review_module() + + with self.assertRaises(module.LLMReviewError) as ctx: + module.extract_chat_content({"choices": [], "private": "provider detail"}) + self.assertIn("no choices", str(ctx.exception)) + self.assertNotIn("provider detail", str(ctx.exception)) + + with self.assertRaisesRegex(module.LLMReviewError, "empty content"): + module.extract_chat_content({"choices": [None]}) + + def test_call_rejects_invalid_or_non_object_json(self): + module = load_pr_review_module() + invalid = Mock(status_code=200) + invalid.json.side_effect = ValueError("invalid") + with ( + patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), + patch.object(module.requests, "post", return_value=invalid), + self.assertRaisesRegex(module.LLMReviewError, "invalid JSON"), + ): + module.call_llm("diff", "default", "system") + + non_object = Mock(status_code=200) + non_object.json.return_value = [] + with ( + patch.dict(module.os.environ, {"OPENAI_API_KEY": "test"}, clear=True), + patch.object(module.requests, "post", return_value=non_object), + self.assertRaisesRegex(module.LLMReviewError, "non-object JSON"), + ): + module.call_llm("diff", "default", "system") From cb46df0b453aaa5dda00060725bebf52aa2fcfde Mon Sep 17 00:00:00 2001 From: luckyPipewrench Date: Sun, 16 Aug 2026 13:12:51 -0400 Subject: [PATCH 3/3] test(ci): guard the caller's shape instead of the retired inline runner The two workflow-shape assertions described the inline reviewer this change replaces, so they failed against the caller that supersedes it. They are rewritten rather than deleted, because what they were protecting still matters and only the shape they check has moved. They now assert the properties that can actually go wrong in a pinned caller. The pin appears twice and selects two separate things, which workflow runs and which reviewer source it runs, so the test requires the two to be equal rather than merely present; a mismatch runs one version's workflow against another version's code and reports nothing wrong. Neither position may carry a branch or tag, since either can move the reviewer under the pin. The secrets map must be exactly what the reusable workflow declares, because passing an undeclared secret fails at workflow load rather than at review time. Model names must not appear at all, since the shared reviewer owns that choice now and a name here would mean this repository had started diverging again. Verified by breaking it: mismatching the two pins fails the new test, and restoring it passes. --- tests/test_pr_review_routing.py | 46 ++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/tests/test_pr_review_routing.py b/tests/test_pr_review_routing.py index a8f534b..6d466a8 100644 --- a/tests/test_pr_review_routing.py +++ b/tests/test_pr_review_routing.py @@ -91,28 +91,44 @@ def test_empty_repository_variable_overrides_use_python_defaults(self): self.assertEqual(module.model_for_mode("default"), module.DEFAULT_MODEL_FAST) self.assertEqual(module.model_for_mode("deep"), module.DEFAULT_MODEL_DEEP) - def test_workflow_delegates_model_defaults_to_python(self): + def test_workflow_leaves_model_selection_to_the_shared_reviewer(self): + # The caller used to choose models through repository variables. The + # shared reviewer owns that decision now, so a model name appearing + # here would mean this repository had started diverging from the + # reviewer it delegates to, which is the drift this change removes. workflow = WORKFLOW_PATH.read_text(encoding="utf-8") - self.assertIn("PR_REVIEW_MODEL_FAST: ${{ vars.PR_REVIEW_MODEL_FAST }}", workflow) - self.assertIn("PR_REVIEW_MODEL_DEEP: ${{ vars.PR_REVIEW_MODEL_DEEP }}", workflow) - self.assertIsNone(re.search(r"PR_REVIEW_MODEL_(?:FAST|DEEP): gpt-", workflow)) + self.assertIsNone(re.search(r"\bgpt-[0-9]", workflow)) + self.assertNotIn("PR_REVIEW_MODEL_FAST", workflow) + self.assertNotIn("PR_REVIEW_MODEL_DEEP", workflow) - def test_workflow_keeps_trusted_runner_and_owner_gate(self): + def test_workflow_pins_one_immutable_reviewer_and_gates_on_owner(self): workflow = WORKFLOW_PATH.read_text(encoding="utf-8") self.assertIn("github.event.comment.user.login == 'luckyPipewrench'", workflow) self.assertIn("github.event.comment.author_association == 'OWNER'", workflow) - self.assertIn("ref: ${{ github.event.repository.default_branch }}", workflow) - self.assertIn("persist-credentials: false", workflow) - self.assertIn("LITELLM_BASE_URL: ${{ secrets.LITELLM_BASE_URL }}", workflow) - self.assertIn("LITELLM_API_KEY: ${{ secrets.LITELLM_API_KEY }}", workflow) - self.assertIn("OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}", workflow) - self.assertIn( - "group: pr-review-${{ github.repository }}-${{ github.event.issue.number }}", workflow - ) - self.assertIn("cancel-in-progress: true", workflow) - self.assertIn("python -m unittest tests/test_pr_review_routing.py", workflow) + + # The pin appears twice and selects two different things: which + # workflow runs, and which reviewer source it runs. A mismatch runs one + # version's workflow against another version's code and reports nothing + # wrong, so equality is the property worth asserting, not presence. + used = re.search(r"pr-review-reusable\.yaml@([0-9a-f]{40})\b", workflow) + declared = re.search(r"reviewer_sha:\s*([0-9a-f]{40})\b", workflow) + self.assertIsNotNone(used, "the reusable workflow must be pinned to a full commit sha") + self.assertIsNotNone(declared, "reviewer_sha must be a full commit sha") + self.assertEqual(used.group(1), declared.group(1)) + + # A branch or tag can move the reviewer code under the pin, so neither + # position may carry one. + self.assertIsNone(re.search(r"pr-review-reusable\.yaml@(?![0-9a-f]{40}\b)\S+", workflow)) + + # A caller may only pass secrets the reusable workflow declares. Passing + # an undeclared one fails at workflow load rather than at review time, + # which presents as the review simply never running. + secrets_block = workflow.split(" secrets:", 1) + self.assertEqual(len(secrets_block), 2, "the caller must map secrets explicitly") + mapped = set(re.findall(r"^ ([a-z_]+):", secrets_block[1], re.MULTILINE)) + self.assertEqual(mapped, {"review_token", "openai_api_key"}) runner = SCRIPT_PATH.read_text(encoding="utf-8") self.assertNotIn("resp.text[:500]", runner)