From ef647baf5d033a5954c02d720dc44ea290846bea Mon Sep 17 00:00:00 2001 From: Matt Norris Date: Fri, 31 Jul 2026 15:13:35 -0400 Subject: [PATCH 1/2] feat(skills): add pr-review skill, sync skills/ess from source Add skills/ess/pr-review (SKILL.md, 6 reference docs, 9 scripts) and bring the four existing skills back to parity with their source tree: - pr-address-comments now points at the pr-review skill rather than a stale /review-pr command, a reference that resolves now that pr-review ships here - summarize-change-log: correct the example to `langsmith-client deploy docker` - summarize-change-log: _REPO_ROOT was parents[5], carried over from an earlier .cursor/skills/mn/ path one level deeper. At skills/ess//scripts/ the repo root is parents[4], so the old value resolved above the repo. Latent rather than breaking, since the tests never exercise that lookup. Also replace the 13-line appendix stub in LICENSE, at the root and in every skill, with the full Apache-2.0 text that section 4(a) requires when redistributing, and move copyright attribution to a root NOTICE per 4(d). --- LICENSE | 209 ++++++++- NOTICE | 6 + skills/ess/mcp-hide-secrets/LICENSE | 209 ++++++++- skills/ess/pr-address-comments-all/LICENSE | 209 ++++++++- skills/ess/pr-address-comments/LICENSE | 209 ++++++++- skills/ess/pr-address-comments/SKILL.md | 2 +- skills/ess/pr-review/LICENSE | 202 +++++++++ skills/ess/pr-review/SKILL.md | 242 ++++++++++ .../pr-review/references/agent-conventions.md | 43 ++ .../pr-review/references/batch-mechanics.md | 128 ++++++ .../references/deterministic-checks.md | 60 +++ skills/ess/pr-review/references/github-cli.md | 80 ++++ .../ess/pr-review/references/output-format.md | 86 ++++ .../pr-review/references/pylint-disables.md | 54 +++ .../scripts/create_review_worktree.sh | 103 +++++ .../pr-review/scripts/find_review_requests.sh | 85 ++++ skills/ess/pr-review/scripts/lib.sh | 94 ++++ skills/ess/pr-review/scripts/lint_python.sh | 95 ++++ skills/ess/pr-review/scripts/lint_ts.sh | 134 ++++++ skills/ess/pr-review/scripts/report.py | 421 ++++++++++++++++++ skills/ess/pr-review/scripts/scan-pr.sh | 199 +++++++++ skills/ess/pr-review/scripts/scan_disables.sh | 57 +++ skills/ess/pr-review/scripts/test_report.py | 245 ++++++++++ skills/ess/summarize-change-log/LICENSE | 209 ++++++++- .../ess/summarize-change-log/examples-good.md | 2 +- .../scripts/test_validate_summary.py | 2 +- 26 files changed, 3332 insertions(+), 53 deletions(-) create mode 100644 NOTICE create mode 100644 skills/ess/pr-review/LICENSE create mode 100644 skills/ess/pr-review/SKILL.md create mode 100644 skills/ess/pr-review/references/agent-conventions.md create mode 100644 skills/ess/pr-review/references/batch-mechanics.md create mode 100644 skills/ess/pr-review/references/deterministic-checks.md create mode 100644 skills/ess/pr-review/references/github-cli.md create mode 100644 skills/ess/pr-review/references/output-format.md create mode 100644 skills/ess/pr-review/references/pylint-disables.md create mode 100755 skills/ess/pr-review/scripts/create_review_worktree.sh create mode 100755 skills/ess/pr-review/scripts/find_review_requests.sh create mode 100644 skills/ess/pr-review/scripts/lib.sh create mode 100755 skills/ess/pr-review/scripts/lint_python.sh create mode 100755 skills/ess/pr-review/scripts/lint_ts.sh create mode 100644 skills/ess/pr-review/scripts/report.py create mode 100755 skills/ess/pr-review/scripts/scan-pr.sh create mode 100755 skills/ess/pr-review/scripts/scan_disables.sh create mode 100644 skills/ess/pr-review/scripts/test_report.py diff --git a/LICENSE b/LICENSE index 9c7afe3..d645695 100644 --- a/LICENSE +++ b/LICENSE @@ -1,13 +1,202 @@ -Copyright 2025 Cisco Systems, Inc. or its Affiliates -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ - http://www.apache.org/licenses/LICENSE-2.0 + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/NOTICE b/NOTICE new file mode 100644 index 0000000..98ad8c1 --- /dev/null +++ b/NOTICE @@ -0,0 +1,6 @@ +Copyright 2021-2026 Cisco Systems, Inc. or its Affiliates + +This product includes software developed at Cisco Systems, Inc. + +Licensed under the Apache License, Version 2.0. See the LICENSE file +in this repository for the full license text. diff --git a/skills/ess/mcp-hide-secrets/LICENSE b/skills/ess/mcp-hide-secrets/LICENSE index 9c7afe3..d645695 100644 --- a/skills/ess/mcp-hide-secrets/LICENSE +++ b/skills/ess/mcp-hide-secrets/LICENSE @@ -1,13 +1,202 @@ -Copyright 2025 Cisco Systems, Inc. or its Affiliates -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ - http://www.apache.org/licenses/LICENSE-2.0 + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/skills/ess/pr-address-comments-all/LICENSE b/skills/ess/pr-address-comments-all/LICENSE index 9c7afe3..d645695 100644 --- a/skills/ess/pr-address-comments-all/LICENSE +++ b/skills/ess/pr-address-comments-all/LICENSE @@ -1,13 +1,202 @@ -Copyright 2025 Cisco Systems, Inc. or its Affiliates -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ - http://www.apache.org/licenses/LICENSE-2.0 + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/skills/ess/pr-address-comments/LICENSE b/skills/ess/pr-address-comments/LICENSE index 9c7afe3..d645695 100644 --- a/skills/ess/pr-address-comments/LICENSE +++ b/skills/ess/pr-address-comments/LICENSE @@ -1,13 +1,202 @@ -Copyright 2025 Cisco Systems, Inc. or its Affiliates -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ - http://www.apache.org/licenses/LICENSE-2.0 + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/skills/ess/pr-address-comments/SKILL.md b/skills/ess/pr-address-comments/SKILL.md index 39dc005..0e07db5 100644 --- a/skills/ess/pr-address-comments/SKILL.md +++ b/skills/ess/pr-address-comments/SKILL.md @@ -264,6 +264,6 @@ Only resolve threads whose comments were addressed in code. Skip threads that: ## Related Commands - `/fix-pr-comments` — Interactive mode (asks before each fix) -- `/review-pr` — Review someone else's PR +- `pr-review` skill — Review someone else's PR - `/create-pr` — Create a new PR - `pr-address-comments-all` — Parallel batch with worktrees and approval gate diff --git a/skills/ess/pr-review/LICENSE b/skills/ess/pr-review/LICENSE new file mode 100644 index 0000000..d645695 --- /dev/null +++ b/skills/ess/pr-review/LICENSE @@ -0,0 +1,202 @@ + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/skills/ess/pr-review/SKILL.md b/skills/ess/pr-review/SKILL.md new file mode 100644 index 0000000..f5f7987 --- /dev/null +++ b/skills/ess/pr-review/SKILL.md @@ -0,0 +1,242 @@ +--- +name: pr-review +description: >- + AI-assisted review of one or more GitHub Pull Requests. Offloads deterministic + checks (lint, complexity, duplication, security, perf) to repo linters via + self-contained scripts, then applies LLM judgment to what linters cannot catch + and posts high-signal findings after a single human approval. Pass one or many + PRs (numbers or URLs); with none, it discovers PRs where your review is + requested. Use for /pr-review, "review PR ", "review my requested + PRs", "code review these pull requests". Requires gh and a git checkout. +metadata: + version: "2.0" +--- + +# Review PR + +Review **one or more** GitHub Pull Requests. **Linters find, the LLM judges**: a +scan script runs the repo's own linters over each PR diff and emits a compact +report, so you spend attention on correctness, cross-file logic, and +disable-comment justification — not on re-deriving lint findings token by token. + +This skill is **variadic**: it always operates on a set of PRs of size N ≥ 1. A +single PR is just N = 1 and uses a fast in-place path; multiple PRs each get an +isolated read-only worktree. Nothing is posted to any PR until you approve at one +gate. + +**Phase**: Review + +## Quick start + +``` +/pr-review https://github.com///pull/456 # one PR +/pr-review 456 # one PR, bare number +/pr-review 456 461 470 # several PRs +/pr-review # discover: PRs awaiting my review +``` + +--- + +## Prerequisites + +- **GitHub CLI** (`gh`) — authenticated, with access to the repo. Primary integration. +- The current directory must be a git repo (`.git/` present). +- Linters are optional: the scan runs whatever is installed (`ruff`, `pylint`, + `bandit`, `eslint`) and records the rest as "not run". In this repo Python + linters run via `uv`. +- TypeScript/eslint needs the workspace deps hoisted to the repo-root + `node_modules` — run `npm install` once at the repo root. Until then the scan + records eslint as "not run" with that hint and continues. +- For N > 1, `openssl` and (ideally) `jq` are used to create and provision + isolated worktrees. GitHub MCP is an optional alternative for posting (see + [references/github-cli.md](references/github-cli.md)); `gh` is the default. + +--- + +## Workflow + +### 1. Resolve the set of PRs + +Build the list of PR numbers to review, then let N = its length. + +- **Refs given** (one or many): each arg is a full URL + (`https://github.com///pull/`) or a bare number. Extract + `owner/repo` from URLs; for bare numbers use + `gh repo view --json nameWithOwner --jq .nameWithOwner`. +- **No refs**: discover PRs awaiting your review: + + ```bash + scripts/find_review_requests.sh # user-requested PRs (default) + scripts/find_review_requests.sh --include-team # also team-requested + ``` + + It prints matching numbers to stdout and a human table to stderr. Show the + table and **confirm the set with the user** before reviewing. If empty, say so + and stop. + +If N = 1, continue with the fast path below. If N > 1, follow +[references/batch-mechanics.md](references/batch-mechanics.md) to scan each PR in +its own worktree (via a read-only subagent per PR), then converge on the same +single GATE in Step 7. + +### 2. Pre-check — skip unnecessary reviews (per PR) + +Fetch metadata, then **drop the PR from the set and record a skip** if any hold: + +```bash +gh pr view --json state,isDraft,title,files +``` + +| Condition | Check | Action | +| --- | --- | --- | +| Closed/merged | `state != "OPEN"` | Skip | +| Draft | `isDraft == true` | Skip | +| AI already reviewed | prior AI review in `gh api .../pulls//reviews` | Skip (no dupes) | +| Trivial only | changes limited to `CHANGELOG.md`, `uv.lock`, lockfiles | Skip | +| Dependency bump | title matches "bump", "update deps" | Skip | + +``` +> **Skipping review** for PR # +> **Reason**: +``` + +Still review AI-authored PRs — do **not** skip just because the author is an AI. + +### 3. Get the PR code (N = 1 fast path) + +**Optional.** `scan-pr.sh ` (Step 4) fetches and pins the PR's head commit +itself, so it scans the real PR diff regardless of what is checked out — you no +longer need to check out the branch just to scan. Check out only if you want the +PR code in your working tree for local inspection: + +```bash +BRANCH="$(gh pr view --json headRefName --jq .headRefName)" +git fetch origin "$BRANCH" +git checkout "$BRANCH" +git reset --hard "origin/$BRANCH" # if the local branch is behind +``` + +For N > 1, each PR is already isolated in its own detached read-only worktree +(no branch checkout in the main tree) — see the batch reference. + +### 4. Run the deterministic scan (per PR) + +```bash +scripts/scan-pr.sh # a PR ref (URL or number) +scripts/scan-pr.sh --base origin/main --head HEAD # or an explicit range +scripts/scan-pr.sh --base origin/main --head --number --repo / +``` + +Given a PR ref, `scan-pr.sh` fetches and pins the PR's head SHA (printed in its +output) and **aborts** if that commit can't be resolved — so it is safe to run +from any checkout and never silently scans the local tree. + +The output dir is unique per PR: a PR ref uses `...-`, and an explicit +`--base/--head` range with no `--number` falls back to a short head hash (so +concurrent range scans never share a dir). Batch reviews (N > 1) pass +`--number ` so each PR writes to its own `/tmp/pr-review---` +and the report is stamped `owner/repo#N` — see the batch reference. + +The orchestrator resolves base/head, computes changed files, runs every +available linter scoped to the diff, scans for suppression comments, and writes +`report.json` + `report.md` under `/tmp/pr-review---/`. **Read +`report.md`** — it replaces manual lint eyeballing and the simplification / +common-issue tables from the old command. + +What the scan covers (do not re-derive these by hand — see +[references/deterministic-checks.md](references/deterministic-checks.md)): + +- **Python**: `ruff` (`E,F,B,PERF,C,I,N,PL` — complexity `C901`, too-many-* + `PLR09xx`, magic values `PLR2004`, perf `PERF`, naming `N`, unused `F401`), + `bandit` (security), `pylint` duplicate-code `R0801` + perflint. +- **TypeScript**: `eslint` + `eslint-plugin-sonarjs` (duplication, cognitive complexity). +- **Suppressions**: every `pylint: disable` / `noqa` / `type: ignore` added by the diff. + +### 5. LLM judgment pass — the actual review (per PR) + +The report handles the mechanical checks. Spend your effort **only** on what +linters cannot decide: + +- **Correctness vs PR intent** — does the code do what the PR claims? +- **Cross-file / logic conflicts** — new code contradicting other changed files; + prompt instructions not matching code behavior. +- **Resource cleanup semantics** — files/connections/temp files freed on every path. +- **Suppression justifications** — for each disable in the report, judge whether + it is acceptable. `too-many-*` / `line-too-long` are never OK; `import-error` / + `no-member` on dynamic attributes often are. Full matrix: + [references/pylint-disables.md](references/pylint-disables.md). +- **AGENTS.md rules** not covered by ruff — locate root and directory-scoped + `AGENTS.md`, and quote the exact rule when flagging a violation. +- **Version-scoped deprecations** — check `pyproject.toml` / `.nvmrc` first; do + not flag a deprecation the project's minimum version is unaffected by. + +For LangChain/LangGraph files (`*/tools.py`, `*_skill.py`, `*/prompts/*.py`), +apply the framework checklist in +[references/agent-conventions.md](references/agent-conventions.md). + +### 6. High-signal filtering (required, per PR) + +**Only flag issues you are confident are real.** False positives erode trust. + +Flag: compile/parse failures, definite wrong results, security vulns, resource +leaks, quoted AGENTS.md violations. Do **not** flag: pre-existing issues, pure +style a linter already owns, speculative input-dependent bugs, subjective +nitpicks, or anything already silenced by a comment. If you are not certain an +issue is real, drop it. Validate each finding (is it truly undefined? is the +AGENTS.md rule scoped to this file? could it be a false positive?) before the GATE. + +Also dedup against existing reviews: **skip anything another reviewer already +flagged** (`gh api repos///pulls//reviews` and `.../comments`). + +### 7. GATE — one approval for the whole set + +Aggregate the proposed findings across **all** PRs into a single view and stop. +Nothing has been posted yet. Present, per PR: + +- the review summary and each proposed finding (severity, `path:line`, fix), per + [references/output-format.md](references/output-format.md); +- any PRs skipped in Step 2 and why. + +Let the user edit or drop individual findings and choose a post mode **per PR**: +inline comments / summary only / request changes / approve / don't post. Do not +post until they approve. + +### 8. Post approved findings (per PR) + +- Format findings per [references/output-format.md](references/output-format.md) + (severity table, inline `suggestion` blocks for <6-line fixes, + `{owner}/{repo}` code links). +- Post with `gh` — commands in [references/github-cli.md](references/github-cli.md). + Post **one comment per unique issue**, in each PR's chosen mode. + +### 9. Cleanup (N > 1 only) + +Remove the per-PR review worktrees created in Step 1: + +```bash +git worktree remove "$WORKTREE_PATH" # add --force if it complains +git worktree prune +``` + +The layout matches `/worktree`, so `/delete-worktree ` also works. +N = 1 creates no worktree, so there is nothing to clean up. + +--- + +## Scope & limitations + +- DRY/duplication is detected across **changed files only** — the scan will not + compare a changed file against untouched files. +- Same-origin PRs only (fork PRs are out of scope). +- Scripts depend only on `git`, `gh`, `openssl`/`jq` (N > 1), and the repo's + auto-discovered linter config; they run standalone when the skill is copied + into another repo. + +## References + +- [references/batch-mechanics.md](references/batch-mechanics.md) — N > 1 only: worktree-per-PR layout, per-PR read-only review subagent, GATE aggregation, cleanup. +- [references/deterministic-checks.md](references/deterministic-checks.md) — full check → linter/rule map, and the LLM boundary. +- [references/pylint-disables.md](references/pylint-disables.md) — disable evaluation matrix. +- [references/output-format.md](references/output-format.md) — review summary, inline comment, code-link format. +- [references/github-cli.md](references/github-cli.md) — gh commands to fetch and post; MCP note. +- [references/agent-conventions.md](references/agent-conventions.md) — LangChain/LangGraph checklist + file-type focus. diff --git a/skills/ess/pr-review/references/agent-conventions.md b/skills/ess/pr-review/references/agent-conventions.md new file mode 100644 index 0000000..329abbe --- /dev/null +++ b/skills/ess/pr-review/references/agent-conventions.md @@ -0,0 +1,43 @@ +# LangChain / LangGraph conventions (framework-specific) + +Apply this only when the PR touches LangChain/LangGraph agent code. These are +conventions the linters do not enforce, so they belong to the LLM judgment pass. + +## `@tool` checklist + +For each changed `*/tools.py` (or tool definition): + +- [ ] `@tool` decorator used correctly. +- [ ] `@handle_tool_error` decorator present. +- [ ] `RunnableConfig` parameter included. +- [ ] `should_fetch` parameter for cached data (where applicable). +- [ ] ROUTING hints in the docstring (so the LLM can select the tool). +- [ ] Docstring under 1024 chars (validation-enforced). +- [ ] Citation constant defined. +- [ ] Error messages are user-friendly. + +## AGENTS.md rules worth quoting + +When the diff violates one of these, quote the exact rule in the finding: + +| Rule | Check | +| --- | --- | +| Max 5 parameters | "Max 5 parameters per function — group related params into dataclasses/config objects" | +| Tool docstring format | "Docstrings must include ROUTING hints for LLM tool selection" | +| Tool docstring length | "Max 1024 chars for @tool docstrings (validation enforced)" | +| Type hints | "Python 3.11+ with type hints on all functions" | +| DRY | "DRY principle — consolidate duplicated logic into reusable functions" | +| Early returns | "Early returns — use guard clauses to reduce nesting depth" | +| No nested ternaries | "No nested ternaries — use if/else or match/case" | +| Descriptive names | "Descriptive names — use customer_data not d" | +| Dependency workflow | "Follow the uv workflow; don't hand-edit uv.lock" | + +Note: several of these (max parameters, nesting, complexity, naming) are also +enforced mechanically by ruff and will appear in `report.md` — quote the AGENTS.md +rule only when adding context the linter finding lacks, and don't double-post. + +## Scope a rule before flagging + +- Root `AGENTS.md` applies to all files. +- A directory-specific `AGENTS.md` applies only to files in that directory (or + its children). Don't apply an unrelated directory's rules to a file outside it. diff --git a/skills/ess/pr-review/references/batch-mechanics.md b/skills/ess/pr-review/references/batch-mechanics.md new file mode 100644 index 0000000..9af4328 --- /dev/null +++ b/skills/ess/pr-review/references/batch-mechanics.md @@ -0,0 +1,128 @@ +# Batch mechanics (N > 1) + +How `pr-review` reviews **multiple** PRs at once. Read this only when the set has +more than one PR — a single-PR review (N = 1) uses the in-place fast path in +`SKILL.md` and never touches any of this. + +The core idea: give each PR its own **read-only** worktree, run the scan + +judgment for it in a **read-only subagent**, collect each subagent's proposed +findings, then converge on the single GATE in `SKILL.md` Step 7 before anything +is posted. + +## Why worktrees + subagents (and why read-only) + +- **Isolation**: N PRs on different branches cannot share one working tree; each + needs its own checkout so linters see the right code. +- **Parallelism**: one subagent per PR runs the scan and judgment concurrently. +- **Read-only**: review posts comments, it never edits code. So worktrees are + created **detached at the PR head SHA** (`git worktree add --detach`), and the + subagents are told to edit nothing. This is the key contrast with + `pr-address-comments-all`, whose worktrees use `-B ` because it writes + fixes back to the PR. + +## Per-PR worktree layout + +[`scripts/create_review_worktree.sh`](../scripts/create_review_worktree.sh) +creates each worktree and provisions it (runs `.cursor/worktrees.json` +`setup-worktree`, or a best-effort `.env` + `uv sync` default): + +``` +~/.cursor/worktrees// +``` + +- `WORKTREE_ID = pr--<8 hex>` — unique per PR, so PRs never collide. +- `REPO_KEY = -` — matches the `/worktree` + command, so `/delete-worktree` recognizes it. + +Run it once per PR from the repo root: + +```bash +scripts/create_review_worktree.sh --pr --repo / +# prints WORKTREE_ID=..., WORKTREE_PATH=..., HEAD_SHA=... +``` + +For cross-repo sets (full URLs in different repos), run it from **each PR's own +repo root** and always pass `--repo owner/repo`. + +## Per-PR review subagent (read-only) + +Launch one subagent per PR. Use an `explore` subagent with `readonly: true` and +`run_in_background: true` so they run in parallel. Each must post nothing and +edit nothing — it only returns proposed findings for the GATE. + +First resolve `` = the absolute path to the directory holding the +`pr-review` skill you are running (the one that contains this `SKILL.md`, +`scripts/`, and `references/`). Pass it into each subagent so the scan script and +reference docs are reachable **by absolute path** — the worktree is a detached +checkout of the PR head and may not contain the skill at all (it is only on your +branch), or may contain a stale copy. + +Fill in `<...>` from `create_review_worktree.sh` output and the PR metadata: + +``` +Repo worktree (READ-ONLY, detached at the PR head): +PR # in /. pr-review skill dir: + +cd into that worktree. Do NOT edit, commit, push, or post anything. + +1. Run the deterministic scan by ABSOLUTE path (the worktree does not contain + the skill), passing --number so this PR gets its own output dir: + bash "/scripts/scan-pr.sh" \ + --base origin/ --head \ + --number --repo / + Read the generated report.md (its path is printed on the last line; + it is /tmp/pr-review---). +2. Do the LLM judgment pass and high-signal filtering exactly as + /SKILL.md Steps 5–6 describe (correctness vs intent, + cross-file logic, resource cleanup, suppression justifications, AGENTS.md + rules, version-scoped deprecations; /references/agent-conventions.md + for LangChain files). +3. Dedup against existing reviews/comments on the PR. + +Return ONLY a proposed-findings table for this PR (post nothing): + # | severity | path:line | issue | proposed fix | post mode suggestion +Also note: files scanned, any linters that did not run, and whether the PR +should be skipped (draft / already reviewed / trivial) with the reason. +``` + +The scan script and reference docs come from `` (your checkout), +not the worktree — do not assume a workspace-relative +`skills/ess/pr-review/...` path resolves inside the worktree. The worktree only +supplies the code under review: it is the scan's working directory, and because +worktrees share the repo object store, `origin/` is already available. +Reports are written under `/tmp`, so the worktree stays read-only, and the +`--number ` gives each PR its own `/tmp/pr-review---` dir so +concurrent scans never overwrite each other. + +## GATE aggregation + +Collect every subagent's table and present them together as one approval point +(`SKILL.md` Step 7): group by PR, list skips and their reasons, and show +per-finding severity / `path:line` / fix. The user edits or drops findings and +picks a post mode per PR. Only after approval do you post (Step 8), one comment +per unique issue, in each PR's chosen mode. + +For a single PR the orchestrator runs the scan + judgment inline instead of +spawning a subagent — there is no parallelism to gain. + +## Cleanup + +After posting, remove each worktree: + +```bash +git worktree remove "$WORKTREE_PATH" # add --force if it complains about state +git worktree prune +``` + +Or, since the layout matches `/worktree`, `/delete-worktree ` per +PR. Nothing was committed or pushed (detached, read-only), so removal is safe. + +## Edge cases + +- **PR head moves mid-run**: worktrees are pinned to the `HEAD_SHA` captured at + creation, so the review is consistent even if the author pushes more commits. + Note in the GATE if a PR advanced past what you reviewed. +- **Fork PRs**: out of scope (same-origin only). +- **A subagent fails to scan** (e.g. missing linter): it still returns judgment + findings and flags which linters did not run; surface that at the GATE rather + than silently dropping the PR. diff --git a/skills/ess/pr-review/references/deterministic-checks.md b/skills/ess/pr-review/references/deterministic-checks.md new file mode 100644 index 0000000..d7fb956 --- /dev/null +++ b/skills/ess/pr-review/references/deterministic-checks.md @@ -0,0 +1,60 @@ +# Deterministic checks: what the scan owns, what you own + +The scan (`scripts/scan-pr.sh` → `report.md`) runs the repo's own linters over +the PR diff so you don't re-derive mechanical findings by hand. Read the report; +then spend your judgment on the rows in the **LLM-only** table below. + +## Check → linter/rule mapping (scan handles these) + +| Category | Tool / rule | Notes | +| --- | --- | --- | +| Too many arguments | ruff `PLR0913` | Group into a `@dataclass` / config object | +| Too many locals | ruff `PLR0914` | Extract helpers | +| Too many branches / statements | ruff `PLR0912` / `PLR0915` | Extract condition handlers | +| Cyclomatic complexity | ruff `C901` | Split the function | +| Deep nesting | ruff `PLR1702` | Guard clauses / early returns | +| Magic values | ruff `PLR2004` | Named constants | +| Unused imports / vars | ruff `F401` / `F841` | Remove | +| Undefined names | ruff `F821` (and `F`\*) | Real bug — HIGH | +| Naming | ruff `N` | PEP 8 | +| Performance | ruff `PERF`, pylint `perflint` W8xxx | N+1, list/dict literals, loops | +| Security | `bandit -ll` | SQL injection `B608`, subprocess, etc. (medium+) | +| Secrets | TruffleHog (pre-commit/CI) | Not re-run here; note if relevant | +| Duplication (Python) | pylint `R0801` (re-enabled) | Root config disables it; the script turns it back on | +| Duplication / cognitive complexity (TS) | eslint `eslint-plugin-sonarjs` | `no-identical-functions`, `no-duplicate-string` | +| Suppressions added | `scripts/scan_disables.sh` | `pylint: disable`, `noqa`, `type: ignore`, `@ts-ignore` | + +Do **not** hand-flag anything in this table — if it's real, it's already in +`report.md`. Re-flagging it just adds noise. + +## LLM-only (not mechanizable — this is the actual review) + +| Category | What to check | Severity | +| --- | --- | --- | +| Correctness vs intent | Does the code do what the PR description claims? | High | +| Logic / cross-file conflicts | New code contradicting other changed files; detection logic vs documented behavior | High | +| Prompt ↔ code mismatch | Prompt instructions not matching the code that consumes them | High | +| Resource cleanup | Files/connections/temp files freed on every path (incl. error paths) | High | +| Breaking changes | Changed signature/return without updating callers | Medium | +| Suppression justification | Is each disable in the report actually justified? (see `pylint-disables.md`) | Varies | +| AGENTS.md rules | Rules not covered by ruff; quote the exact rule when flagging | Varies | +| Version-scoped deprecations | Check `pyproject.toml` / `.nvmrc` first; don't flag a deprecation the minimum version is unaffected by | Low | + +## Review focus by file type + +| File pattern | Extra scrutiny | +| --- | --- | +| `*/tools.py` | `@tool`, `@handle_tool_error`, ROUTING docstring, citation (see `agent-conventions.md`) | +| `*/queries.py` | SQL injection, parameterized queries, N+1 | +| `*_skill.py` | `routing_examples`, skill model pattern | +| `*/prompts/*.py` | Prompt-injection risk, instructions matching code | + +## Verify project config before flagging version issues + +```bash +grep -E "python_requires|python-version|requires-python|python =" pyproject.toml .python-version 2>/dev/null +cat .nvmrc package.json 2>/dev/null | grep -E "node|engines" +``` + +`datetime.utcnow()` is deprecated in 3.12+, but if the project supports 3.11 +don't flag it. Always confirm the minimum version first. diff --git a/skills/ess/pr-review/references/github-cli.md b/skills/ess/pr-review/references/github-cli.md new file mode 100644 index 0000000..e2027dd --- /dev/null +++ b/skills/ess/pr-review/references/github-cli.md @@ -0,0 +1,80 @@ +# GitHub CLI (gh) — fetch and post + +`gh` is the primary integration for this skill. GitHub MCP is an optional +alternative (see the bottom note). + +## Resolve the repo and PR + +```bash +gh repo view --json nameWithOwner --jq .nameWithOwner # {owner}/{repo} +gh pr view --json number,title,author,state,isDraft,headRefName,baseRefName,files,additions,deletions +``` + +## Fetch existing reviews and inline comments (dedup) + +```bash +gh api repos///pulls//reviews # prior reviews +gh api repos///pulls//comments # inline comments +``` + +Skip any issue another reviewer already raised. Note it as +`⏭️ Skipped (already flagged by @reviewer)` in the summary. + +## Checkout the branch + +```bash +BRANCH="$(gh pr view --json headRefName --jq .headRefName)" +git fetch origin "$BRANCH" +git checkout "$BRANCH" +git reset --hard "origin/$BRANCH" # only if the local branch is behind +``` + +## Post the review + +Ask the user first: inline comments / summary only / request changes / approve / +don't post. + +### Review with inline comments (preferred) + +Build a JSON payload and submit one review with all comments at once. `event` is +one of `COMMENT`, `REQUEST_CHANGES`, `APPROVE`. + +```bash +cat > /tmp/pr-review-body.json <<'JSON' +{ + "event": "COMMENT", + "body": "", + "comments": [ + { "path": "path/to/file.py", "line": 42, "side": "RIGHT", + "body": "Missing error handling for the customer-not-found case." }, + { "path": "path/to/tools.py", "line": 88, "side": "RIGHT", + "body": "Extract this to a named constant." } + ] +} +JSON +gh api repos///pulls//reviews \ + --method POST --input /tmp/pr-review-body.json +``` + +### Summary-only comment + +```bash +gh pr comment --body-file /tmp/review-summary.md +``` + +## Confirm + +``` +## Review Posted ✅ + +**PR**: # | **Action**: REQUEST_CHANGES / COMMENT / APPROVE +**Comments**: X added, Y skipped (already flagged) +``` + +## Optional: GitHub MCP + +If GitHub MCP is configured, the equivalents are `get_pull_request`, +`get_pull_request_files`, `get_pull_request_reviews`, `get_pull_request_comments` +(fetch) and `create_pull_request_review` / `create_issue_comment` (post). Prefer +`gh` unless the user explicitly asks for MCP — it needs no extra setup and works +in every checkout. diff --git a/skills/ess/pr-review/references/output-format.md b/skills/ess/pr-review/references/output-format.md new file mode 100644 index 0000000..e0fbbe6 --- /dev/null +++ b/skills/ess/pr-review/references/output-format.md @@ -0,0 +1,86 @@ +# Review output format + +## Review summary (post as the review body) + +``` +## AI Code Review: PR # + +**Title**: +**Author**: @<author> +**Files Changed**: X files (+Y/-Z lines) + +--- + +### Summary + +| Severity | Count | Note | +| --- | --- | --- | +| 🔴 High | X | | +| 🟡 Medium | Y | | +| 🟢 Low | Z | | +| ⏭️ Skipped | N | Already flagged by other reviewers | + +**Recommendation**: APPROVE / REQUEST_CHANGES / COMMENT + +--- + +### Issues Found + +#### 🔴 HIGH: <short title> + +**File**: `path/to/file.py` (line 42-45) + +**Problem**: <what is wrong and why it matters> + +**Suggested Fix**: <corrected approach or code> + +--- + +### Already Flagged (Skipping) + +- ⏭️ `path/to/file.py:290-377` — <issue> (flagged by @reviewer) + +### What Looks Good ✅ + +- <genuinely good things worth calling out> +``` + +## Inline comment format + +**Small fixes (< 6 lines): include a `suggestion` block** so the author can +one-click apply it. + +````markdown +Missing error handling for the customer-not-found case. + +**AGENTS.md rule**: "Early returns — use guard clauses to reduce nesting depth" + +```suggestion +if not customer: + return {"error": "Customer not found", "data": None} +``` +```` + +**Larger / structural fixes (6+ lines): description only.** Describe the issue +and the approach; do not attach a `suggestion` block — structural changes are +better discussed than auto-applied. + +## Code link format + +When linking to code, use the full commit SHA (not `HEAD` or a branch): + +``` +https://github.com/{owner}/{repo}/blob/{full_sha}/path/to/file.py#L10-L15 +``` + +- Derive `{owner}/{repo}` from `gh repo view --json nameWithOwner --jq .nameWithOwner` + or the PR metadata. +- Get the SHA with `git rev-parse HEAD`. +- Include `#L<start>-L<end>` and give at least one line of context on each side. + +## Rules + +- **One comment per unique issue.** Never post duplicates. +- Post **only high-signal** findings (see `SKILL.md` step 6). If you are not + confident an issue is real, drop it. +- Quote the exact AGENTS.md rule when flagging a compliance violation. diff --git a/skills/ess/pr-review/references/pylint-disables.md b/skills/ess/pr-review/references/pylint-disables.md new file mode 100644 index 0000000..96be392 --- /dev/null +++ b/skills/ess/pr-review/references/pylint-disables.md @@ -0,0 +1,54 @@ +# Evaluating suppression comments + +`scripts/scan_disables.sh` lists every `pylint: disable`, `noqa`, `type: ignore`, +and `@ts-ignore` the PR **adds** (with file and new-file line number). A +suppression is not automatically acceptable just because it carries a comment — +judge each one. The disable exists to hide a linter finding; decide whether +hiding it is the right call or whether the underlying issue should be fixed. + +## Disable → verdict matrix + +| Disable | Verdict | Acceptable when | Flag when | +| --- | --- | --- | --- | +| `import-error` | ✅ Often OK | Runtime import pylint can't resolve (monorepo, plugins, sub-venvs) | Import is genuinely broken | +| `no-member` | ✅ Often OK | Dynamic attributes (SQLAlchemy, Pydantic, etc.) | Typo / missing attribute | +| `unused-argument` | ⚠️ Sometimes | Interface requires the signature (`_`-prefix preferred) | Just lazy; the arg should be used | +| `protected-access` | ⚠️ Sometimes | Testing or framework API limitation | Production code reaching into private members | +| `broad-exception-caught` | ⚠️ Sometimes | Top-level handler / graceful degradation, with a comment explaining why | No comment; hides specific exceptions | +| `too-many-arguments` | 🔴 Never OK | — | Always — refactor to a dataclass/config object | +| `too-many-locals` | 🔴 Never OK | — | Always — extract helper functions | +| `too-many-branches` | 🔴 Never OK | — | Always — extract condition handlers | +| `line-too-long` | 🔴 Never OK | — | Always — just break the line | + +## How to evaluate one + +1. **Is there a comment, and does it explain _why_?** "This function is complex" + restates the problem — it is not a justification. +2. **Code smell or false positive?** `too-many-*` is always a smell; + `import-error` is often a false positive. +3. **Can it be refactored away?** Extract helpers, use a dataclass, catch + specific exceptions. +4. **Is the justification just avoidance?** "Would require refactoring" → flag it + for refactoring. + +### Common invalid justifications (flag these) + +- "This function is complex" → extract helpers. +- "Need many parameters" → dataclass / config object. +- "Many branches for edge cases" → extract condition handlers. +- "External library raises a generic Exception" → acceptable **only** if no + narrower exception exists. + +## Severity when flagging + +- 🔴 **High**: `too-many-*`, `line-too-long`, unjustified disables, "lazy" justifications. +- 🟡 **Medium**: valid use case that could still be improved by refactoring. +- 🟢 **Low**: `import-error`, `no-member` on valid dynamic attributes, + `protected-access` for a known API limitation. Usually no action needed. + +## Repo note + +The root `pyproject.toml` disables pylint's `C`/`R`/`W` categories, so +`scripts/lint_python.sh` re-enables `duplicate-code` (R0801) and the perflint +checks explicitly. The `too-many-*` rules are enforced by **ruff** (`PLR09xx`, +`C901`, `PLR1702`) and appear in the report from there. diff --git a/skills/ess/pr-review/scripts/create_review_worktree.sh b/skills/ess/pr-review/scripts/create_review_worktree.sh new file mode 100755 index 0000000..1bc94fa --- /dev/null +++ b/skills/ess/pr-review/scripts/create_review_worktree.sh @@ -0,0 +1,103 @@ +#!/usr/bin/env bash +# +# create_review_worktree.sh +# Create a READ-ONLY git worktree checked out at a PR's head commit, so each PR +# in an N>1 review can be scanned in isolation without touching the main tree. +# +# Review is read-only: this uses `git worktree add --detach <headSha>` (NOT +# `-B <branch>`), so there is no local branch to accidentally commit or push to. +# Contrast with pr-address-comments-all/scripts/create_pr_worktree.sh, which +# needs a real branch because it writes back to the PR. +# +# The on-disk layout matches the /worktree command +# (~/.cursor/worktrees/<WORKTREE_ID>/<repo-key>) so /delete-worktree still works. +# +# Usage: create_review_worktree.sh --pr N --repo owner/repo +# --pr N - PR number (used to name the worktree) +# --repo owner/repo - Repo the PR lives in +# +# Run from inside the target repo. Prints WORKTREE_ID, WORKTREE_PATH, HEAD_SHA. + +set -euo pipefail + +_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${_LIB_DIR}/lib.sh" + +PR_NUMBER="" +OWNER_REPO="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --pr) PR_NUMBER="${2:-}"; shift 2 ;; + --repo) OWNER_REPO="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,19p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +[[ -n "$PR_NUMBER" ]] || error "Missing --pr N" +[[ -n "$OWNER_REPO" ]] || error "Missing --repo owner/repo" +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" +command -v openssl >/dev/null 2>&1 || error "openssl not found on PATH" + +require_git_repo +REPO_ROOT="$(repo_root)" + +# Resolve the PR's head branch and exact head commit. We check out the SHA +# (detached) rather than the branch tip so the review is pinned to what the PR +# is at now, even if it moves mid-run. +read -r BRANCH HEAD_SHA < <(gh pr view "$PR_NUMBER" --repo "$OWNER_REPO" \ + --json headRefName,headRefOid \ + --jq '[.headRefName, .headRefOid] | @tsv' | tr '\t' ' ') \ + || error "Could not look up head for ${OWNER_REPO}#${PR_NUMBER}" +[[ -n "$HEAD_SHA" ]] || error "Empty head SHA for ${OWNER_REPO}#${PR_NUMBER}" + +# /worktree-compatible repo key: <basename>-<sha256(repo_root)[:12]>. +REPO_BASENAME="$(basename "$REPO_ROOT")" +REPO_HASH="$(sha256_short "$REPO_ROOT")" +REPO_KEY="${REPO_BASENAME}-${REPO_HASH}" + +WORKTREE_ID="pr-${PR_NUMBER}-$(openssl rand -hex 4)" +WORKTREE_DIR="${HOME}/.cursor/worktrees/${WORKTREE_ID}/${REPO_KEY}" + +[[ -d "$WORKTREE_DIR" ]] && error "Worktree directory already exists: ${WORKTREE_DIR}" +mkdir -p "$(dirname "$WORKTREE_DIR")" + +info "Fetching origin/${BRANCH} (${HEAD_SHA:0:8})" +# `--` so a branch name starting with `-` is treated as a refspec, not an option. +git fetch origin -- "$BRANCH" + +info "Creating read-only detached worktree at ${HEAD_SHA:0:8}" +# --detach: no local branch is created, so nothing can be committed/pushed by +# mistake. This is the key difference from the write-capable -all skill. +git worktree add --detach "$WORKTREE_DIR" "$HEAD_SHA" + +# Run the repo's worktree setup so linters resolve (venv, .env). Mirrors +# create_pr_worktree.sh; kept identical so both skills behave the same. +WORKTREES_JSON="${REPO_ROOT}/.cursor/worktrees.json" +export ROOT_WORKTREE_PATH="$REPO_ROOT" +( + cd "$WORKTREE_DIR" + if [[ -f "$WORKTREES_JSON" ]] && command -v jq >/dev/null 2>&1; then + info "Running setup-worktree from .cursor/worktrees.json" + jq -r 'if (."setup-worktree"|type) == "array" then ."setup-worktree"[] else (."setup-worktree" // empty) end' \ + "$WORKTREES_JSON" | while IFS= read -r cmd; do + [[ -z "$cmd" || "$cmd" == "null" ]] && continue + info " \$ ${cmd}" + bash -c "$cmd" || error "setup step failed: ${cmd}" + done + else + warn "jq or .cursor/worktrees.json unavailable; running best-effort default setup" + rsync -am --exclude='node_modules' --exclude='.next' --exclude='.git' \ + --include='*/' --include='.env' --exclude='*' "${ROOT_WORKTREE_PATH}/" . \ + || warn "rsync of .env files into the worktree failed (continuing)" + if command -v uv >/dev/null 2>&1 && [[ -f pyproject.toml || -f uv.lock ]]; then + uv sync --all-packages || warn "uv sync --all-packages failed (continuing)" + fi + fi +) + +echo "WORKTREE_ID=${WORKTREE_ID}" +echo "WORKTREE_PATH=${WORKTREE_DIR}" +echo "HEAD_SHA=${HEAD_SHA}" diff --git a/skills/ess/pr-review/scripts/find_review_requests.sh b/skills/ess/pr-review/scripts/find_review_requests.sh new file mode 100755 index 0000000..edc1a3e --- /dev/null +++ b/skills/ess/pr-review/scripts/find_review_requests.sh @@ -0,0 +1,85 @@ +#!/usr/bin/env bash +# +# find_review_requests.sh +# Discover the current repo's open PRs where a review is requested of you +# (review-requested:@me). Use when /pr-review is invoked with no PR refs. +# +# Usage: find_review_requests.sh [--repo owner/repo] [--limit N] [--include-team] +# --repo owner/repo - Repo to query (default: current remote) +# --limit N - Max PRs to return (default: 50) +# --include-team - Also include PRs requested from a team you belong to +# (default only counts direct, user-level requests) +# +# Output: +# stdout - matching PR numbers, one per line (for the skill to loop over) +# stderr - a human-readable table (number, branch, author, title) +# +# Drafts are skipped. Exits 0 with empty stdout when nothing matches. + +set -euo pipefail + +_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${_LIB_DIR}/lib.sh" + +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" + +OWNER_REPO="" +LIMIT="50" +INCLUDE_TEAM="false" + +while [[ $# -gt 0 ]]; do + case "$1" in + --repo) OWNER_REPO="${2:-}"; shift 2 ;; + --limit) LIMIT="${2:-}"; shift 2 ;; + --include-team) INCLUDE_TEAM="true"; shift ;; + -h|--help) sed -n '2,17p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +if [[ -z "$OWNER_REPO" ]]; then + OWNER_REPO="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" \ + || error "Could not determine repo from current remote; pass --repo owner/repo" +fi + +# GitHub search distinguishes: +# user-review-requested:@me - only direct, user-level review requests +# review-requested:@me - direct requests PLUS requests to a team you are on +# Default to the narrower user-level filter; --include-team broadens it. +if [[ "$INCLUDE_TEAM" == "true" ]]; then + SEARCH="review-requested:@me" +else + SEARCH="user-review-requested:@me" +fi +info "Scanning open PRs in ${OWNER_REPO} where review is requested of you (limit ${LIMIT})" + +# Numbers + metadata as TSV (number<TAB>branch<TAB>author<TAB>title), drafts out. +PR_ROWS="$(gh pr list --repo "$OWNER_REPO" --state open --limit "$LIMIT" \ + --search "$SEARCH" \ + --json number,headRefName,author,title,isDraft \ + --jq '.[] | select(.isDraft == false) + | [.number, .headRefName, (.author.login // ""), .title] | @tsv')" \ + || error "gh pr list failed for ${OWNER_REPO}" + +if [[ -z "$PR_ROWS" ]]; then + info "No open PRs currently request your review in ${OWNER_REPO}" + exit 0 +fi + +printf 'PR\tBRANCH\tAUTHOR\tTITLE\n' >&2 + +MATCHES=() +while IFS=$'\t' read -r NUMBER BRANCH AUTHOR TITLE; do + [[ -z "$NUMBER" ]] && continue + MATCHES+=("$NUMBER") + printf '%s\t%s\t%s\t%s\n' "$NUMBER" "$BRANCH" "${AUTHOR:-unknown}" "$TITLE" >&2 +done <<< "$PR_ROWS" + +if [[ ${#MATCHES[@]} -eq 0 ]]; then + info "No open, non-draft PRs request your review in ${OWNER_REPO}" + exit 0 +fi + +info "Matched ${#MATCHES[@]} PR(s)" +printf '%s\n' "${MATCHES[@]}" diff --git a/skills/ess/pr-review/scripts/lib.sh b/skills/ess/pr-review/scripts/lib.sh new file mode 100644 index 0000000..c563167 --- /dev/null +++ b/skills/ess/pr-review/scripts/lib.sh @@ -0,0 +1,94 @@ +#!/usr/bin/env bash +# +# lib.sh - shared helpers for the pr-review skill scripts. +# +# Sourced (not executed) by the other scripts in this directory. It sets no +# shell options and owns no `set -e/-u`: each script keeps its own so that the +# lint scripts (which intentionally continue past tool failures) and the strict +# scripts (which exit on error) both behave correctly. Portable to bash 3.2 +# (macOS): no associative arrays / mapfile. +# +# Source it with, right after the `set ...` line: +# _LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# # shellcheck source=lib.sh +# . "${_LIB_DIR}/lib.sh" + +# --- logging (stderr, colorized) --------------------------------------------- +RED='\033[0;31m'; GREEN='\033[0;32m'; YELLOW='\033[1;33m'; NC='\033[0m' +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 1; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } +warn() { echo -e "${YELLOW}Warning:${NC} $1" >&2; } + +# --- git --------------------------------------------------------------------- +# Toplevel of the repo containing the CURRENT WORKING DIRECTORY -- i.e. the tree +# being scanned/reviewed, NOT the tree that holds this skill. Callers must cd +# into the target repo first. Falls back to $PWD when outside a git repo. +repo_root() { git rev-parse --show-toplevel 2>/dev/null || pwd; } + +# Exit with an error unless the cwd is inside a git work tree. Use before +# repo_root() when the pwd fallback would be wrong (e.g. worktree creation). +require_git_repo() { + git rev-parse --git-dir >/dev/null 2>&1 || error "not inside a git repository" +} + +# --- tool runners ------------------------------------------------------------ +# Prefer the repo venv via uv when available, else bare python3. +py_runner() { + if command -v uv >/dev/null 2>&1 && uv run python --version >/dev/null 2>&1; then + echo "uv run python" + elif command -v python3 >/dev/null 2>&1; then + echo "python3" + fi +} + +# Echo a command prefix that can run the given tool via the repo venv (uv) when +# available, else the bare executable, else empty when unavailable. +resolve_tool() { + local tool="$1" + if command -v uv >/dev/null 2>&1 && uv run "$tool" --version >/dev/null 2>&1; then + echo "uv run $tool" + elif command -v "$tool" >/dev/null 2>&1; then + echo "$tool" + fi +} + +# --- json / status helpers --------------------------------------------------- +# True when <file> exists, is non-empty, and starts with '[' (a JSON array). +is_json_array() { [[ -s "$1" ]] && [[ "$(head -c1 "$1")" == "[" ]]; } + +# Append "tool<TAB>status" to $STATUS_FILE. The caller sets STATUS_FILE (usually +# "${OUT_DIR}/tools.tsv") before invoking this. +status() { printf '%s\t%s\n' "$1" "$2" >> "$STATUS_FILE"; } + +# Read non-empty lines of <file> into the global FILES array (bash 3.2 safe). +read_file_list() { + FILES=() + local line + while IFS= read -r line; do + [[ -n "$line" ]] && FILES+=("$line") + done < "$1" +} + +# --- hashing ----------------------------------------------------------------- +# Short hex hash of a string (sha1, 8 chars), tolerant of hosts lacking shasum. +hash_str() { + if command -v shasum >/dev/null 2>&1; then + printf '%s' "$1" | shasum | cut -c1-8 + elif command -v sha1sum >/dev/null 2>&1; then + printf '%s' "$1" | sha1sum | cut -c1-8 + else + printf '%s' "$1" | cksum | tr -d ' ' | cut -c1-8 + fi +} + +# Longer hex hash of a string (sha256, 12 chars) for /worktree-compatible repo +# keys. Falls back to openssl on hosts shipping neither shasum nor sha256sum. +sha256_short() { + if command -v shasum >/dev/null 2>&1; then + printf '%s' "$1" | shasum -a 256 | cut -c1-12 + elif command -v sha256sum >/dev/null 2>&1; then + printf '%s' "$1" | sha256sum | cut -c1-12 + else + printf '%s' "$1" | openssl dgst -sha256 | awk '{print $NF}' | cut -c1-12 + fi +} diff --git a/skills/ess/pr-review/scripts/lint_python.sh b/skills/ess/pr-review/scripts/lint_python.sh new file mode 100755 index 0000000..9b0cf6a --- /dev/null +++ b/skills/ess/pr-review/scripts/lint_python.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# +# lint_python.sh <out_dir> +# +# Runs ruff, bandit, and a duplicate-code + perflint pylint pass over the changed +# Python files listed in <out_dir>/py_files.txt, writing machine-readable output: +# ruff.json - ruff check --output-format=json +# bandit.json - bandit -f json (security) +# pylint.json - pylint duplicate-code (R0801, re-enabled) + perflint +# +# Per-tool run status is appended to <out_dir>/tools.tsv as "tool<TAB>status". +# Each linter's config is auto-discovered from the repo. In a uv project the +# tools run via `uv run`; otherwise the bare executables are used. Portable to +# bash 3.2 (macOS): no mapfile / associative arrays. + +set -uo pipefail + +_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${_LIB_DIR}/lib.sh" + +OUT_DIR="${1:?usage: lint_python.sh <out_dir>}" +STATUS_FILE="${OUT_DIR}/tools.tsv" + +# Read the changed-file list into FILES (bash 3.2 compatible). +read_file_list "${OUT_DIR}/py_files.txt" + +if [[ "${#FILES[@]}" -eq 0 ]]; then + status ruff "not run (no python files)" + status bandit "not run (no python files)" + status pylint "not run (no python files)" + exit 0 +fi + +# --- ruff --------------------------------------------------------------------- +RUFF="$(resolve_tool ruff)" +if [[ -n "$RUFF" ]]; then + # shellcheck disable=SC2086 + $RUFF check --output-format=json --force-exclude -- "${FILES[@]}" > "${OUT_DIR}/ruff.json" 2>/dev/null || true + if is_json_array "${OUT_DIR}/ruff.json"; then + status ruff ok + else + echo "[]" > "${OUT_DIR}/ruff.json"; status ruff "ran (no parseable output)" + fi +else + status ruff "not run (tool unavailable)" +fi + +# --- bandit ------------------------------------------------------------------- +# Honor the repo's [tool.bandit] config (skips, test excludes) when present -- +# bandit does not auto-discover pyproject.toml, so it must be passed explicitly. +BANDIT_CFG=() +REPO_ROOT="$(repo_root)" +if [[ -n "$REPO_ROOT" && -f "${REPO_ROOT}/pyproject.toml" ]] \ + && grep -q '^\[tool.bandit\]' "${REPO_ROOT}/pyproject.toml" 2>/dev/null; then + BANDIT_CFG=(-c "${REPO_ROOT}/pyproject.toml") +fi +BANDIT="$(resolve_tool bandit)" +if [[ -n "$BANDIT" ]]; then + # -ll: report medium+ severity only (drops B101 assert-in-test noise, keeps + # real issues like B608 SQL injection). Explicit files bypass config excludes. + # shellcheck disable=SC2086 + $BANDIT ${BANDIT_CFG[@]+"${BANDIT_CFG[@]}"} -ll -q -f json "${FILES[@]}" > "${OUT_DIR}/bandit.json" 2>/dev/null || true + if [[ -s "${OUT_DIR}/bandit.json" ]] && [[ "$(head -c1 "${OUT_DIR}/bandit.json")" == "{" ]]; then + status bandit ok + else + echo '{"results": []}' > "${OUT_DIR}/bandit.json"; status bandit "ran (no parseable output)" + fi +else + status bandit "not run (tool unavailable)" +fi + +# --- pylint: duplicate-code (R0801) + perflint -------------------------------- +# Root pyproject disables the C/R/W categories, so duplicate-code and the +# perflint W8xxx checks must be re-enabled explicitly here. +PYLINT="$(resolve_tool pylint)" +if [[ -n "$PYLINT" ]]; then + PERF_ENABLE="duplicate-code,use-list-literal,use-dict-literal,use-tuple-over-list,use-set-for-membership,dotted-import-in-loop,use-fstring-for-concatenation,incorrect-dictionary-iterator,unnecessary-list-index-lookup" + # shellcheck disable=SC2086 + $PYLINT --disable=all --enable="$PERF_ENABLE" --load-plugins=perflint \ + --output-format=json "${FILES[@]}" > "${OUT_DIR}/pylint.json" 2>/dev/null || true + if ! is_json_array "${OUT_DIR}/pylint.json"; then + # perflint plugin or a symbol was unavailable; fall back to duplicate-code only. + # shellcheck disable=SC2086 + $PYLINT --disable=all --enable=duplicate-code \ + --output-format=json "${FILES[@]}" > "${OUT_DIR}/pylint.json" 2>/dev/null || true + fi + if is_json_array "${OUT_DIR}/pylint.json"; then + status pylint ok + else + echo "[]" > "${OUT_DIR}/pylint.json"; status pylint "ran (no parseable output)" + fi +else + status pylint "not run (tool unavailable)" +fi diff --git a/skills/ess/pr-review/scripts/lint_ts.sh b/skills/ess/pr-review/scripts/lint_ts.sh new file mode 100755 index 0000000..476520b --- /dev/null +++ b/skills/ess/pr-review/scripts/lint_ts.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# +# lint_ts.sh <out_dir> +# +# Best-effort ESLint pass (with eslint-plugin-sonarjs when the repo config +# enables it) over the changed TypeScript files listed in <out_dir>/ts_files.txt. +# +# ESLint flat config is resolved from the working directory, so each app must be +# linted from the directory that holds its `eslint.config.*`. Changed files are +# therefore grouped by their nearest ancestor *config* dir, and eslint is run +# once per group from that dir. The eslint *binary* is resolved separately: +# in an npm-workspaces monorepo apps often have no local eslint, but the shared +# config's plugins (e.g. eslint-plugin-sonarjs) plus the eslint binary are +# hoisted to the *root* node_modules -- so the root binary is used, invoked from +# the app dir, which is exactly what lets the app's config resolve its plugins. +# +# A one-time `npm install` at the repo root is required to populate that hoisted +# node_modules. Output is one <out_dir>/eslint-<n>.json part per group (report.py +# merges the parts). If eslint cannot be resolved or run, the status records an +# actionable hint and the scan continues. Portable to bash 3.2 (macOS): no +# mapfile / associative arrays. + +set -uo pipefail + +_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${_LIB_DIR}/lib.sh" + +OUT_DIR="${1:?usage: lint_ts.sh <out_dir>}" +STATUS_FILE="${OUT_DIR}/tools.tsv" + +ROOT="$(repo_root)" + +read_file_list "${OUT_DIR}/ts_files.txt" + +if [[ "${#FILES[@]}" -eq 0 ]]; then + echo "[]" > "${OUT_DIR}/eslint.json" + status eslint "not run (no typescript files)" + exit 0 +fi + +# Nearest ancestor dir (of the given file) that holds an eslint config; the run +# dir for flat config. Echoes a repo-relative dir, or returns non-zero if none. +find_config_dir() { + local dir; dir="$(dirname "$1")" + while :; do + for cfg in eslint.config.mjs eslint.config.js eslint.config.cjs \ + eslint.config.ts .eslintrc.js .eslintrc.cjs .eslintrc.json \ + .eslintrc.yml .eslintrc.yaml; do + [[ -f "${ROOT}/${dir}/${cfg}" ]] && { echo "$dir"; return 0; } + done + [[ "$dir" == "." || "$dir" == "/" ]] && break + dir="$(dirname "$dir")" + done + return 1 +} + +# Resolve an eslint binary runnable from the config dir: the dir's own or any +# ancestor's (incl. the hoisted root) node_modules/.bin/eslint (absolute path), +# else "npx". Returns non-zero when nothing is available. +resolve_eslint_bin() { + local dir; dir="${ROOT}/${1}" + while [[ "$dir" != "/" ]]; do + [[ -x "$dir/node_modules/.bin/eslint" ]] && { echo "$dir/node_modules/.bin/eslint"; return 0; } + dir="$(dirname "$dir")" + done + # Only fall back to npx if eslint actually resolves there without a network + # install; probe from the config dir so it matches the real run below. Blind + # npx yields no JSON and a misleading "config error" when the fix is `npm + # install` at the repo root -- returning non-zero routes to that message. + if command -v npx >/dev/null 2>&1 \ + && ( cd "${ROOT}/${1}" && npx --no-install eslint --version >/dev/null 2>&1 ); then + echo "npx"; return 0 + fi + return 1 +} + +# Map each file to its config dir (or NONE) in a flat TSV -- no assoc arrays. +# NB: do not name this var GROUPS -- that is a read-only special bash variable +# (the user's group IDs); assignments to it are silently ignored. +GROUP_MAP="${OUT_DIR}/eslint-groups.tsv" +: > "$GROUP_MAP" +for f in "${FILES[@]}"; do + if dir="$(find_config_dir "$f")"; then + printf '%s\t%s\n' "$dir" "$f" >> "$GROUP_MAP" + else + printf 'NONE\t%s\n' "$f" >> "$GROUP_MAP" + fi +done + +RAN=0 +ATTEMPTED=0 +FAILED=0 +INDEX=0 +MISSING_BIN=0 + +# One eslint run per config dir, from that dir, with file paths relative to it. +for dir in $(cut -f1 "$GROUP_MAP" | sort -u); do + [[ "$dir" == "NONE" ]] && continue + BIN="$(resolve_eslint_bin "$dir")" || { MISSING_BIN=1; continue; } + RELS=() + while IFS=$'\t' read -r gdir gfile; do + [[ "$gdir" == "$dir" ]] || continue + RELS+=("${gfile#"$dir"/}") + done < "$GROUP_MAP" + INDEX=$((INDEX + 1)) + ATTEMPTED=$((ATTEMPTED + 1)) + PART="${OUT_DIR}/eslint-${INDEX}.json" + if [[ "$BIN" == "npx" ]]; then + ( cd "${ROOT}/${dir}" && npx --no-install eslint -f json "${RELS[@]}" ) > "$PART" 2>/dev/null || true + else + ( cd "${ROOT}/${dir}" && "$BIN" -f json "${RELS[@]}" ) > "$PART" 2>/dev/null || true + fi + # A run that produced no JSON array (crash, config error) is a failure, not a + # skip: count it so the status below never mislabels it as "not run". + if is_json_array "$PART"; then RAN=1; else echo "[]" > "$PART"; FAILED=$((FAILED + 1)); fi +done + +# Ensure report.py finds at least one (empty) part even when nothing ran. +[[ "$INDEX" -gt 0 ]] || [[ -f "${OUT_DIR}/eslint.json" ]] || echo "[]" > "${OUT_DIR}/eslint.json" + +if [[ "$ATTEMPTED" -eq 0 ]]; then + if [[ "$MISSING_BIN" -eq 1 ]]; then + status eslint "not run (eslint deps missing — run 'npm install' at repo root)" + else + status eslint "not run (no eslint config found for changed files)" + fi +elif [[ "$FAILED" -eq 0 ]]; then + status eslint ok +elif [[ "$RAN" -eq 1 ]]; then + status eslint "partial (${FAILED} of ${ATTEMPTED} eslint runs produced no JSON — likely a config error)" +else + status eslint "error (eslint ran but produced no JSON for ${ATTEMPTED} group(s) — check eslint config)" +fi diff --git a/skills/ess/pr-review/scripts/report.py b/skills/ess/pr-review/scripts/report.py new file mode 100644 index 0000000..ef3b332 --- /dev/null +++ b/skills/ess/pr-review/scripts/report.py @@ -0,0 +1,421 @@ +#!/usr/bin/env python3 +"""Merge and rank linter output for the pr-review skill. + +Reads the raw linter JSON written by the scan scripts into an input directory and +emits two artifacts next to them: + +* ``report.json`` -- normalized, severity-ranked findings plus suppression list. +* ``report.md`` -- the human/LLM-readable review scan the skill actually reads. + +Stdlib only; no third-party dependencies, so it runs under bare ``python3`` when +the skill is lifted out of this repo. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from dataclasses import asdict, dataclass +from pathlib import Path + +SEVERITY_ORDER = {"HIGH": 0, "MEDIUM": 1, "LOW": 2} +SEVERITY_EMOJI = {"HIGH": "🔴", "MEDIUM": "🟡", "LOW": "🟢"} + +# ruff codes for complexity / structure smells (never silently "just style"). +_RUFF_COMPLEXITY = { + "C901", + "PLR0904", + "PLR0911", + "PLR0912", + "PLR0913", + "PLR0914", + "PLR0915", + "PLR0916", + "PLR1702", +} +_RUFF_UNUSED = {"F401", "F811", "F841", "F842"} +# "E9" (E902 IO, E999 syntax/parse) are "can't run" errors -- rank HIGH so they +# are not buried by the generic "E" -> LOW rule below. Checked before LOW. +_RUFF_HIGH_PREFIXES = ("PLE", "S", "E9") +_RUFF_LOW_PREFIXES = ("E", "W", "I", "N", "D", "Q", "COM") +_ESLINT_ERROR_LEVEL = 2 +_SHA_LEN = 40 +_SHORT_SHA_LEN = 8 + + +@dataclass(frozen=True) +class Finding: + """A single normalized linter finding.""" + + tool: str + rule: str + severity: str + path: str + line: int + message: str + + +def ruff_severity(code: str) -> str: + """Map a ruff rule code to HIGH/MEDIUM/LOW.""" + if code in _RUFF_COMPLEXITY: + return "MEDIUM" + if code == "PLR2004": + return "LOW" + if code.startswith("F"): + return "LOW" if code in _RUFF_UNUSED else "HIGH" + if code.startswith(_RUFF_HIGH_PREFIXES): + return "HIGH" + if code.startswith(_RUFF_LOW_PREFIXES): + return "LOW" + # B (bugbear), PERF, PLC/PLW/PLR and anything else -> a reviewable middle. + return "MEDIUM" + + +def pylint_severity(message_id: str, symbol: str) -> str: + """Map a pylint message to HIGH/MEDIUM/LOW.""" + if message_id.startswith("E") or message_id.startswith("F"): + return "HIGH" + if symbol == "duplicate-code" or message_id.startswith("W8"): + return "MEDIUM" + return "MEDIUM" + + +def eslint_severity(rule_id: str, eslint_level: int) -> str: + """Map an eslint message to HIGH/MEDIUM/LOW.""" + smell_markers = ( + "no-identical-functions", + "no-duplicate-string", + "cognitive-complexity", + ) + if rule_id and any(marker in rule_id for marker in smell_markers): + return "MEDIUM" + return "MEDIUM" if eslint_level == _ESLINT_ERROR_LEVEL else "LOW" + + +def normalize_path(raw: str) -> str: + """Present a repo-relative path: drop a leading ``./`` and relativize + absolute paths under the current working directory (eslint emits absolute).""" + path = raw.strip() + if path.startswith("./"): + path = path[2:] + if path.startswith("/"): + try: + path = str(Path(path).resolve().relative_to(Path.cwd())) + except ValueError: + pass + return path + + +def _load_json(path: Path) -> object | None: + if not path.is_file(): + return None + try: + return json.loads(path.read_text(encoding="utf-8")) + except (json.JSONDecodeError, UnicodeDecodeError): + return None + + +def load_ruff(path: Path) -> list[Finding]: + """Parse ruff --output-format=json into findings.""" + data = _load_json(path) + findings: list[Finding] = [] + if not isinstance(data, list): + return findings + for item in data: + code = str(item.get("code") or "?") + location = item.get("location") or {} + findings.append( + Finding( + tool="ruff", + rule=code, + severity=ruff_severity(code), + path=normalize_path(str(item.get("filename") or "?")), + line=int(location.get("row") or 0), + message=str(item.get("message") or "").strip(), + ) + ) + return findings + + +def load_bandit(path: Path) -> list[Finding]: + """Parse bandit -f json into findings (bandit's own severity is used).""" + data = _load_json(path) + findings: list[Finding] = [] + if not isinstance(data, dict): + return findings + for item in data.get("results", []) or []: + severity = str(item.get("issue_severity") or "MEDIUM").upper() + if severity not in SEVERITY_ORDER: + severity = "MEDIUM" + findings.append( + Finding( + tool="bandit", + rule=str(item.get("test_id") or "?"), + severity=severity, + path=normalize_path(str(item.get("filename") or "?")), + line=int(item.get("line_number") or 0), + message=str(item.get("issue_text") or "").strip(), + ) + ) + return findings + + +def load_pylint(path: Path) -> list[Finding]: + """Parse pylint --output-format=json into findings.""" + data = _load_json(path) + findings: list[Finding] = [] + if not isinstance(data, list): + return findings + for item in data: + message_id = str(item.get("message-id") or item.get("messageId") or "?") + symbol = str(item.get("symbol") or "") + findings.append( + Finding( + tool="pylint", + rule=symbol or message_id, + severity=pylint_severity(message_id, symbol), + path=normalize_path(str(item.get("path") or "?")), + line=int(item.get("line") or 0), + message=str(item.get("message") or "").strip(), + ) + ) + return findings + + +def load_eslint(input_dir: Path) -> list[Finding]: + """Parse every ``eslint*.json`` part in ``input_dir`` into findings. + + ESLint is often installed per-app in a monorepo, so ``lint_ts.sh`` may run it + once per app and emit one part file per group; they are merged here. + """ + findings: list[Finding] = [] + for part in sorted(input_dir.glob("eslint*.json")): + data = _load_json(part) + if not isinstance(data, list): + continue + for file_result in data: + file_path = normalize_path(str(file_result.get("filePath") or "?")) + for message in file_result.get("messages", []) or []: + rule_id = str(message.get("ruleId") or "?") + level = int(message.get("severity") or 1) + findings.append( + Finding( + tool="eslint", + rule=rule_id, + severity=eslint_severity(rule_id, level), + path=file_path, + line=int(message.get("line") or 0), + message=str(message.get("message") or "").strip(), + ) + ) + return findings + + +def load_disables(path: Path) -> list[dict]: + """Parse the suppression-comment JSON array.""" + data = _load_json(path) + if not isinstance(data, list): + return [] + return [item for item in data if isinstance(item, dict)] + + +def load_tools(path: Path) -> dict: + """Parse the tool<TAB>status run-status file into an ordered dict.""" + tools: dict[str, str] = {} + if not path.is_file(): + return tools + for line in path.read_text(encoding="utf-8").splitlines(): + if "\t" in line: + name, state = line.split("\t", 1) + tools[name.strip()] = state.strip() + return tools + + +def rank(findings: list[Finding]) -> list[Finding]: + """Sort findings by severity, then path, then line.""" + return sorted( + findings, + key=lambda finding: ( + SEVERITY_ORDER.get(finding.severity, 1), + finding.path, + finding.line, + ), + ) + + +def build_report( + findings: list[Finding], + disables: list[dict], + meta: dict, +) -> tuple[dict, str]: + """Build the report.json payload and the report.md text.""" + ordered = rank(findings) + counts = {"HIGH": 0, "MEDIUM": 0, "LOW": 0} + for finding in ordered: + counts[finding.severity] = counts.get(finding.severity, 0) + 1 + + tools = meta.get("tools", {}) if isinstance(meta, dict) else {} + payload = { + "slug": _slug(meta), + "base": meta.get("base", ""), + "head": meta.get("head", ""), + "python_files": meta.get("python_files", 0), + "typescript_files": meta.get("typescript_files", 0), + "summary": {k.lower(): v for k, v in counts.items()}, + "tools": tools, + "findings": [asdict(finding) for finding in ordered], + "disables": disables, + } + return payload, _render_markdown(ordered, counts, disables, meta, tools) + + +def _slug(meta: dict) -> str: + owner = meta.get("owner") or "local" + repo = meta.get("repo") or "repo" + number = meta.get("number") or "range" + return ( + f"{owner}/{repo}#{number}" if number != "range" else f"{owner}/{repo} (range)" + ) + + +def _short_ref(ref: str) -> str: + """Abbreviate a full 40-char git SHA to 8 chars; leave named refs untouched.""" + if len(ref) == _SHA_LEN and all(c in "0123456789abcdef" for c in ref): + return ref[:_SHORT_SHA_LEN] + return ref + + +def _render_markdown( + ordered: list[Finding], + counts: dict, + disables: list[dict], + meta: dict, + tools: dict, +) -> str: + lines: list[str] = [] + lines.append(f"# PR review scan — {_slug(meta)}") + lines.append("") + lines.append( + f"Range `{meta.get('base', '?')}...{_short_ref(meta.get('head', '?'))}` · " + f"{meta.get('python_files', 0)} python / " + f"{meta.get('typescript_files', 0)} typescript files changed." + ) + lines.append("") + lines.append("## Summary") + lines.append("") + lines.append("| Severity | Count |") + lines.append("| --- | --- |") + lines.extend( + f"| {SEVERITY_EMOJI[sev]} {sev.title()} | {counts.get(sev, 0)} |" + for sev in ("HIGH", "MEDIUM", "LOW") + ) + lines.append(f"| Suppressions added | {len(disables)} |") + lines.append("") + lines.append( + "> These are deterministic linter findings. Judge them, don't just " + "repost them: skip pre-existing noise, keep the high-signal issues, and " + "spend your effort on correctness, cross-file logic, and the suppression " + "justifications below (see the skill's references)." + ) + lines.append("") + + for sev in ("HIGH", "MEDIUM", "LOW"): + bucket = [finding for finding in ordered if finding.severity == sev] + if not bucket: + continue + lines.append(f"## {SEVERITY_EMOJI[sev]} {sev.title()} ({len(bucket)})") + lines.append("") + lines.extend( + f"- `{finding.path}:{finding.line}` — **{finding.rule}** " + f"({finding.tool}) {finding.message}" + for finding in bucket + ) + lines.append("") + + lines.append("## Suppressions added by this PR") + lines.append("") + if disables: + lines.append( + "Evaluate each: `too-many-*` / `line-too-long` are never OK; " + "`import-error` / dynamic `no-member` often are." + ) + lines.append("") + for item in disables: + path = item.get("path", "?") + line = item.get("line", 0) + text = str(item.get("text", "")).strip() + lines.append(f"- `{path}:{line}` — `{text}`") + else: + lines.append("None.") + lines.append("") + + lines.append("## Tools") + lines.append("") + if tools: + for name, state in tools.items(): + lines.append(f"- {name}: {state}") + else: + lines.append("- (no tool status recorded)") + lines.append("") + return "\n".join(lines) + + +def load_all(input_dir: Path) -> tuple[list[Finding], list[dict], dict]: + """Load every linter artifact from ``input_dir``.""" + findings: list[Finding] = [] + findings += load_ruff(input_dir / "ruff.json") + findings += load_bandit(input_dir / "bandit.json") + findings += load_pylint(input_dir / "pylint.json") + findings += load_eslint(input_dir) + disables = load_disables(input_dir / "disables.json") + meta = _load_json(input_dir / "scan-meta.json") + if not isinstance(meta, dict): + meta = {} + meta.setdefault("tools", {}) + tools = load_tools(input_dir / "tools.tsv") + if tools: + meta["tools"] = tools + return findings, disables, meta + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Merge pr-review linter output.") + parser.add_argument( + "--input-dir", + required=True, + help="Directory containing the raw linter JSON (scan output).", + ) + parser.add_argument( + "--output-dir", + default=None, + help="Where to write report.json/report.md (default: input dir).", + ) + args = parser.parse_args(argv) + + input_dir = Path(args.input_dir).resolve() + if not input_dir.is_dir(): + print(f"report: input dir not found: {input_dir}", file=sys.stderr) + return 1 + output_dir = Path(args.output_dir).resolve() if args.output_dir else input_dir + output_dir.mkdir(parents=True, exist_ok=True) + + findings, disables, meta = load_all(input_dir) + payload, markdown = build_report(findings, disables, meta) + + (output_dir / "report.json").write_text( + json.dumps(payload, indent=2, ensure_ascii=False) + "\n", encoding="utf-8" + ) + (output_dir / "report.md").write_text(markdown + "\n", encoding="utf-8") + + print( + f"report: {payload['summary']['high']} high / " + f"{payload['summary']['medium']} medium / " + f"{payload['summary']['low']} low, " + f"{len(disables)} suppression(s) -> {output_dir / 'report.md'}" + ) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/skills/ess/pr-review/scripts/scan-pr.sh b/skills/ess/pr-review/scripts/scan-pr.sh new file mode 100755 index 0000000..8a54ab6 --- /dev/null +++ b/skills/ess/pr-review/scripts/scan-pr.sh @@ -0,0 +1,199 @@ +#!/usr/bin/env bash +# +# scan-pr.sh +# Run the repo's own linters over a PR diff and emit a compact, ranked report so +# the reviewer's attention goes to judgment, not to re-deriving lint findings. +# +# Usage: +# scan-pr.sh <pr-ref> [--repo owner/repo] [--output DIR] +# scan-pr.sh --base <ref> --head <ref> [--number N] [--repo owner/repo] [--output DIR] +# +# <pr-ref> - Full PR URL (https://github.com/<o>/<r>/pull/<N>) or a bare number. +# Fetches and pins the PR's actual head commit, so the current +# checkout does not matter and the local tree is never scanned. +# --base / --head - Explicit git refs to diff (default range: origin/main...HEAD) +# --number N - PR number for an explicit --base/--head range: names the +# output dir (...-<N>) and stamps the report with owner/repo#N. +# Batch reviews pass this so concurrent PRs never collide. +# --repo owner/repo - Repo identity (a bare number, or an explicit range's slug) +# --output DIR - Output directory (default: /tmp/pr-review-<owner>-<repo>-<N>; +# an explicit range with no --number uses a short head hash) +# +# Writes into DIR (whatever the environment supports; missing tools => "not run"): +# scan-meta.json - base/head + changed-file counts + repo identity +# tools.tsv - per-tool run status (tool<TAB>status) +# py_files.txt - changed *.py paths (one per line) +# ts_files.txt - changed *.ts/*.tsx paths +# ruff.json / pylint.json / bandit.json / eslint.json - raw linter output +# disables.json - suppression comments added by the diff +# report.json / report.md - merged, ranked findings (read report.md) +# +# Portable to bash 3.2 (macOS): no associative arrays or mapfile. Depends only on +# git/gh plus whatever linters are installed, and on the repo's own +# auto-discovered linter config -- nothing outside this skill directory. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${SCRIPT_DIR}/lib.sh" + +# --- parse args --------------------------------------------------------------- +PR_REF=""; BASE=""; HEAD=""; REPO_OVERRIDE=""; OUTPUT_DIR=""; NUMBER_OVERRIDE="" +while [[ $# -gt 0 ]]; do + case "$1" in + --base) BASE="${2:-}"; shift 2 ;; + --head) HEAD="${2:-}"; shift 2 ;; + --number) NUMBER_OVERRIDE="${2:-}"; shift 2 ;; + --repo) REPO_OVERRIDE="${2:-}"; shift 2 ;; + --output) OUTPUT_DIR="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,33p' "$0"; exit 0 ;; + -*) error "Unknown option: $1" ;; + *) if [[ -z "$PR_REF" ]]; then PR_REF="$1"; else error "Unexpected argument: $1"; fi; shift ;; + esac +done + +command -v git >/dev/null 2>&1 || error "git not found on PATH" +require_git_repo + +OWNER=""; REPO=""; NUMBER="" + +# --- resolve base/head + repo identity --------------------------------------- +if [[ -n "$PR_REF" ]]; then + command -v gh >/dev/null 2>&1 || error "gh CLI required to resolve a PR ref" + if [[ "$PR_REF" =~ ^https?://[^/]+/([^/]+)/([^/]+)/pull/([0-9]+) ]]; then + OWNER="${BASH_REMATCH[1]}"; REPO="${BASH_REMATCH[2]}"; NUMBER="${BASH_REMATCH[3]}" + elif [[ "$PR_REF" =~ ^[0-9]+$ ]]; then + NUMBER="$PR_REF" + if [[ -n "$REPO_OVERRIDE" ]]; then + OWNER="${REPO_OVERRIDE%%/*}"; REPO="${REPO_OVERRIDE#*/}" + else + NWO="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" \ + || error "could not determine repo from remote; pass --repo owner/repo" + OWNER="${NWO%%/*}"; REPO="${NWO#*/}" + fi + else + error "could not parse PR reference: ${PR_REF} (expected a URL or a bare number)" + fi + PR_META="$(gh pr view "$NUMBER" --repo "${OWNER}/${REPO}" \ + --json baseRefName,headRefName,headRefOid --jq '[.baseRefName,.headRefName,.headRefOid]|@tsv')" \ + || error "gh pr view failed for ${OWNER}/${REPO}#${NUMBER}" + BASE_REF="$(printf '%s' "$PR_META" | cut -f1)" + HEAD_REF="$(printf '%s' "$PR_META" | cut -f2)" + HEAD_OID="$(printf '%s' "$PR_META" | cut -f3)" + info "fetching base ${BASE_REF}" + # ``--`` separates the refspec from options so a branch name beginning with + # ``-`` is never mistaken for a git flag. + git fetch --quiet origin -- "$BASE_REF" || warn "could not fetch origin/${BASE_REF}" + BASE="origin/${BASE_REF}" + # Pin to the PR's actual head commit -- never the local working tree. This is + # what stops a scan run from the wrong checkout from silently diffing the + # local branch instead of the real PR. ``pull/<N>/head`` resolves the head of + # the same-origin PRs this skill supports; fall back to the head branch name + # if the pull ref is missing. + info "fetching PR #${NUMBER} head ${HEAD_OID:0:8} (${HEAD_REF})" + git fetch --quiet origin -- "pull/${NUMBER}/head" \ + || git fetch --quiet origin -- "$HEAD_REF" \ + || warn "could not fetch PR head for #${NUMBER}" + # Verify guard: the exact PR head commit must be present locally after the + # fetch, or we refuse to scan rather than fall back to the local tree. + git cat-file -e "${HEAD_OID}^{commit}" 2>/dev/null \ + || error "PR #${NUMBER} head ${HEAD_OID} not available after fetch; refusing to scan the local tree" + HEAD="$HEAD_OID" +else + # Explicit --base/--head range. --repo makes the output slug and report + # identity deterministic (batch reviews pass it); otherwise fall back to the + # current remote. --number labels this range as a specific PR. + if [[ -n "$REPO_OVERRIDE" ]]; then + OWNER="${REPO_OVERRIDE%%/*}"; REPO="${REPO_OVERRIDE#*/}" + elif [[ -z "$OWNER" ]] && command -v gh >/dev/null 2>&1; then + NWO="$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null || true)" + if [[ -n "$NWO" ]]; then OWNER="${NWO%%/*}"; REPO="${NWO#*/}"; fi + fi + NUMBER="$NUMBER_OVERRIDE" + BASE="${BASE:-origin/main}" + HEAD="${HEAD:-HEAD}" +fi + +# --- output dir --------------------------------------------------------------- +# The suffix must be unique per PR so concurrent/batch scans never share (and +# thus clobber) a directory. A PR number is best; for a bare --base/--head range +# fall back to the head SHA (or a hash of the range) instead of a literal that +# every range would collide on. +if [[ -z "$OUTPUT_DIR" ]]; then + SAFE_OWNER="${OWNER//\//-}"; SAFE_REPO="${REPO//\//-}" + if [[ -n "$NUMBER" ]]; then + SUFFIX="$NUMBER" + elif [[ "$HEAD" =~ ^[0-9a-f]{7,40}$ ]]; then + SUFFIX="${HEAD:0:8}" + else + SUFFIX="$(hash_str "${BASE}...${HEAD}")" + fi + OUTPUT_DIR="/tmp/pr-review-${SAFE_OWNER:-local}-${SAFE_REPO:-repo}-${SUFFIX}" +fi +# Guard against a mistyped --output (e.g. /, ., .., $HOME) wiping unintended +# files before the rm -rf below. +case "$OUTPUT_DIR" in + ""|/|.|..|"$HOME") error "refusing to remove unsafe output dir: '${OUTPUT_DIR}'" ;; + */) error "output dir must not end with '/': '${OUTPUT_DIR}'" ;; +esac +rm -rf "$OUTPUT_DIR"; mkdir -p "$OUTPUT_DIR" +: > "${OUTPUT_DIR}/tools.tsv" +info "diff range: ${BASE}...${HEAD}" +info "output dir: ${OUTPUT_DIR}" + +# --- changed files ------------------------------------------------------------ +git diff --name-only --diff-filter=ACMR "${BASE}...${HEAD}" 2>/dev/null > "${OUTPUT_DIR}/changed.txt" \ + || error "git diff failed for range ${BASE}...${HEAD}" +grep -E '\.py$' "${OUTPUT_DIR}/changed.txt" > "${OUTPUT_DIR}/py_files.txt" || true +grep -E '\.(ts|tsx)$' "${OUTPUT_DIR}/changed.txt" > "${OUTPUT_DIR}/ts_files.txt" || true +PY_COUNT="$(wc -l < "${OUTPUT_DIR}/py_files.txt" | tr -d ' ')" +TS_COUNT="$(wc -l < "${OUTPUT_DIR}/ts_files.txt" | tr -d ' ')" +info "changed files: ${PY_COUNT} python, ${TS_COUNT} typescript" + +# --- run the linters (each sub-script appends its own status to tools.tsv) ----- +if [[ "$PY_COUNT" -gt 0 ]]; then + bash "${SCRIPT_DIR}/lint_python.sh" "$OUTPUT_DIR" || warn "python lint step failed" +else + printf 'ruff\tnot run (no python files)\n' >> "${OUTPUT_DIR}/tools.tsv" + printf 'pylint\tnot run (no python files)\n' >> "${OUTPUT_DIR}/tools.tsv" + printf 'bandit\tnot run (no python files)\n' >> "${OUTPUT_DIR}/tools.tsv" +fi + +if [[ "$TS_COUNT" -gt 0 ]]; then + bash "${SCRIPT_DIR}/lint_ts.sh" "$OUTPUT_DIR" || warn "typescript lint step failed" +else + printf 'eslint\tnot run (no typescript files)\n' >> "${OUTPUT_DIR}/tools.tsv" +fi + +# --- suppression comments added by the diff ----------------------------------- +bash "${SCRIPT_DIR}/scan_disables.sh" "$OUTPUT_DIR" "$BASE" "$HEAD" || warn "disable scan failed" + +# --- scan-meta.json ----------------------------------------------------------- +{ + printf '{\n' + printf ' "owner": "%s",\n' "$OWNER" + printf ' "repo": "%s",\n' "$REPO" + printf ' "number": "%s",\n' "$NUMBER" + printf ' "base": "%s",\n' "$BASE" + printf ' "head": "%s",\n' "$HEAD" + printf ' "python_files": %s,\n' "$PY_COUNT" + printf ' "typescript_files": %s\n' "$TS_COUNT" + printf '}\n' +} > "${OUTPUT_DIR}/scan-meta.json" + +# --- merge + rank ------------------------------------------------------------- +PYRUN="$(py_runner)" +[[ -n "$PYRUN" ]] || error "no python interpreter found to build the report" +# report.py relativizes eslint's absolute paths against its cwd, so run it from +# the SCANNED repo's root -- otherwise invoking scan-pr.sh from a subdirectory +# leaks absolute paths into the report. repo_root() resolves the cwd's repo (the +# tree being scanned), not the skill's. Pass an absolute OUTPUT_DIR since we cd. +REPO_ROOT="$(repo_root)" +OUTPUT_DIR_ABS="$(cd "$OUTPUT_DIR" && pwd)" +# shellcheck disable=SC2086 +( cd "$REPO_ROOT" && $PYRUN "${SCRIPT_DIR}/report.py" --input-dir "$OUTPUT_DIR_ABS" ) \ + || error "report.py failed" + +info "report ready: ${OUTPUT_DIR}/report.md" +echo "${OUTPUT_DIR}/report.md" diff --git a/skills/ess/pr-review/scripts/scan_disables.sh b/skills/ess/pr-review/scripts/scan_disables.sh new file mode 100755 index 0000000..47428b5 --- /dev/null +++ b/skills/ess/pr-review/scripts/scan_disables.sh @@ -0,0 +1,57 @@ +#!/usr/bin/env bash +# +# scan_disables.sh <out_dir> <base> <head> +# +# Scans the ADDED lines of the diff for suppression comments the PR introduces: +# Python: # pylint: disable=... # noqa # type: ignore +# TypeScript: // @ts-ignore // @ts-expect-error +# +# Writes <out_dir>/disables.json as a JSON array of {path, line, text}, where +# line is the new-file line number. The LLM evaluates whether each suppression +# is justified (see references/pylint-disables.md) -- this script only locates +# them. Uses only git + awk (no jq / python dependency). + +set -uo pipefail + +_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +. "${_LIB_DIR}/lib.sh" + +OUT_DIR="${1:?usage: scan_disables.sh <out_dir> <base> <head>}" +BASE="${2:?missing base ref}" +HEAD="${3:?missing head ref}" + +# Capture the diff first so a git failure (bad refs, missing commits) is caught +# and surfaced to the caller, rather than feeding awk an empty stream and writing +# a silently incomplete disables list. +if ! DIFF="$(git diff --no-color "${BASE}...${HEAD}" -- '*.py' '*.pyi' '*.ts' '*.tsx' 2>/dev/null)"; then + error "scan_disables.sh: git diff failed for ${BASE}...${HEAD}" +fi + +printf '%s\n' "$DIFF" | awk ' +function esc(s) { + gsub(/\\/, "\\\\", s); gsub(/"/, "\\\"", s); gsub(/\t/, " ", s); sub(/\r$/, "", s) + return s +} +BEGIN { printf "["; first = 1 } +/^\+\+\+ / { f = $2; sub(/^b\//, "", f); next } +/^--- / { next } +/^@@ / { h = $3; sub(/^\+/, "", h); split(h, a, ","); ln = a[1] + 0; next } +{ + c = substr($0, 1, 1); rest = substr($0, 2) + if (c == "+") { + if (rest ~ /pylint:[[:space:]]*disable/ \ + || rest ~ /(#|\/\/)[[:space:]]*noqa/ \ + || rest ~ /type:[[:space:]]*ignore/ \ + || rest ~ /@ts-(ignore|expect-error)/) { + body = rest; sub(/^[[:space:]]+/, "", body) + if (first) { first = 0 } else { printf "," } + printf "\n {\"path\": \"%s\", \"line\": %d, \"text\": \"%s\"}", esc(f), ln, esc(body) + } + ln++ + } else if (c == " ") { + ln++ + } +} +END { if (first) printf "]\n"; else printf "\n]\n" } +' > "${OUT_DIR}/disables.json" diff --git a/skills/ess/pr-review/scripts/test_report.py b/skills/ess/pr-review/scripts/test_report.py new file mode 100644 index 0000000..4243797 --- /dev/null +++ b/skills/ess/pr-review/scripts/test_report.py @@ -0,0 +1,245 @@ +"""Tests for the pr-review linter merge/rank logic.""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +EXPECTED_BANDIT_LINE = 42 + +from report import ( # noqa: E402 + Finding, + build_report, + eslint_severity, + load_all, + load_bandit, + load_eslint, + load_pylint, + load_ruff, + load_tools, + normalize_path, + pylint_severity, + rank, + ruff_severity, +) + + +def test_normalize_path_strips_dot_slash() -> None: + assert normalize_path("./pkg/mod.py") == "pkg/mod.py" + assert normalize_path("pkg/mod.py") == "pkg/mod.py" + # An absolute path outside cwd is left unchanged (no crash). + assert normalize_path("/elsewhere/x.ts") == "/elsewhere/x.ts" + + +def test_ruff_severity_buckets() -> None: + assert ruff_severity("PLR0913") == "MEDIUM" # too-many-arguments -> smell + assert ruff_severity("C901") == "MEDIUM" # complexity + assert ruff_severity("PLR2004") == "LOW" # magic value + assert ruff_severity("F821") == "HIGH" # undefined name + assert ruff_severity("F401") == "LOW" # unused import + assert ruff_severity("PERF401") == "MEDIUM" + assert ruff_severity("N802") == "LOW" + assert ruff_severity("E999") == "HIGH" # syntax/parse error -> can't run + assert ruff_severity("E902") == "HIGH" # IO error -> can't run + assert ruff_severity("E501") == "LOW" # line too long -> style + + +def test_pylint_severity_buckets() -> None: + assert pylint_severity("R0801", "duplicate-code") == "MEDIUM" + assert pylint_severity("W8101", "use-list-literal") == "MEDIUM" + assert pylint_severity("E0602", "undefined-variable") == "HIGH" + + +def test_eslint_severity_buckets() -> None: + assert eslint_severity("sonarjs/no-identical-functions", 1) == "MEDIUM" + assert eslint_severity("sonarjs/cognitive-complexity", 2) == "MEDIUM" + assert eslint_severity("@typescript-eslint/no-explicit-any", 2) == "MEDIUM" + assert eslint_severity("prefer-const", 1) == "LOW" + + +def test_load_ruff_parses_findings(tmp_path: Path) -> None: + (tmp_path / "ruff.json").write_text( + json.dumps( + [ + { + "code": "F401", + "message": "`os` imported but unused", + "filename": "pkg/mod.py", + "location": {"row": 3, "column": 1}, + } + ] + ), + encoding="utf-8", + ) + findings = load_ruff(tmp_path / "ruff.json") + assert findings == [ + Finding( + tool="ruff", + rule="F401", + severity="LOW", + path="pkg/mod.py", + line=3, + message="`os` imported but unused", + ) + ] + + +def test_load_bandit_uses_own_severity(tmp_path: Path) -> None: + (tmp_path / "bandit.json").write_text( + json.dumps( + { + "results": [ + { + "test_id": "B608", + "issue_severity": "HIGH", + "issue_text": "Possible SQL injection", + "filename": "pkg/db.py", + "line_number": 42, + } + ] + } + ), + encoding="utf-8", + ) + findings = load_bandit(tmp_path / "bandit.json") + assert len(findings) == 1 + assert findings[0].severity == "HIGH" + assert findings[0].tool == "bandit" + assert findings[0].line == EXPECTED_BANDIT_LINE + + +def test_load_pylint_duplicate_code(tmp_path: Path) -> None: + (tmp_path / "pylint.json").write_text( + json.dumps( + [ + { + "message-id": "R0801", + "symbol": "duplicate-code", + "message": "Similar lines in 2 files", + "path": "pkg/a.py", + "line": 10, + } + ] + ), + encoding="utf-8", + ) + findings = load_pylint(tmp_path / "pylint.json") + assert findings[0].rule == "duplicate-code" + assert findings[0].severity == "MEDIUM" + + +def test_load_eslint_flattens_messages(tmp_path: Path) -> None: + (tmp_path / "eslint.json").write_text( + json.dumps( + [ + { + "filePath": "/repo/app/x.ts", + "messages": [ + { + "ruleId": "sonarjs/no-duplicate-string", + "severity": 1, + "message": "Define a constant", + "line": 7, + } + ], + } + ] + ), + encoding="utf-8", + ) + findings = load_eslint(tmp_path) + assert findings[0].tool == "eslint" + assert findings[0].severity == "MEDIUM" + assert findings[0].path == "/repo/app/x.ts" + + +def test_load_eslint_merges_multiple_parts(tmp_path: Path) -> None: + (tmp_path / "eslint-1.json").write_text( + '[{"filePath": "/r/a.ts", "messages": [{"ruleId": "no-x", ' + '"severity": 2, "message": "a", "line": 1}]}]', + encoding="utf-8", + ) + (tmp_path / "eslint-2.json").write_text( + '[{"filePath": "/r/b.ts", "messages": [{"ruleId": "no-y", ' + '"severity": 1, "message": "b", "line": 2}]}]', + encoding="utf-8", + ) + findings = load_eslint(tmp_path) + assert {f.path for f in findings} == {"/r/a.ts", "/r/b.ts"} + + +def test_load_missing_file_is_empty(tmp_path: Path) -> None: + assert load_ruff(tmp_path / "nope.json") == [] + + +def test_rank_orders_by_severity_then_location() -> None: + findings = [ + Finding("ruff", "N802", "LOW", "b.py", 1, ""), + Finding("bandit", "B608", "HIGH", "z.py", 9, ""), + Finding("ruff", "C901", "MEDIUM", "a.py", 5, ""), + Finding("ruff", "F821", "HIGH", "a.py", 2, ""), + ] + ordered = rank(findings) + assert [f.severity for f in ordered] == ["HIGH", "HIGH", "MEDIUM", "LOW"] + # Within HIGH, a.py:2 sorts before z.py:9. + assert ordered[0].path == "a.py" + assert ordered[1].path == "z.py" + + +def test_build_report_counts_and_disables() -> None: + findings = [ + Finding("bandit", "B608", "HIGH", "db.py", 1, "sqli"), + Finding("ruff", "C901", "MEDIUM", "a.py", 5, "too complex"), + Finding("ruff", "F401", "LOW", "a.py", 1, "unused"), + ] + disables = [ + {"path": "a.py", "line": 5, "text": "# pylint: disable=too-many-branches"} + ] + meta = { + "owner": "o", + "repo": "r", + "number": "42", + "base": "origin/main", + "head": "HEAD", + "python_files": 2, + "typescript_files": 0, + "tools": {"ruff": "ok", "eslint": "not run (no typescript files)"}, + } + payload, markdown = build_report(findings, disables, meta) + assert payload["summary"] == {"high": 1, "medium": 1, "low": 1} + assert payload["slug"] == "o/r#42" + assert payload["disables"] == disables + assert "🔴 High (1)" in markdown + assert "too-many-branches" in markdown + assert "not run (no typescript files)" in markdown + + +def test_load_tools_parses_tsv(tmp_path: Path) -> None: + (tmp_path / "tools.tsv").write_text( + "ruff\tok\npylint\tnot run (tool unavailable)\n", encoding="utf-8" + ) + tools = load_tools(tmp_path / "tools.tsv") + assert tools == {"ruff": "ok", "pylint": "not run (tool unavailable)"} + + +def test_load_all_reads_directory(tmp_path: Path) -> None: + (tmp_path / "ruff.json").write_text( + '[{"code": "F401", "message": "x", ' + '"filename": "m.py", "location": {"row": 1}}]', + encoding="utf-8", + ) + (tmp_path / "disables.json").write_text( + '[{"path": "m.py", "line": 2, "text": "# noqa"}]', encoding="utf-8" + ) + (tmp_path / "scan-meta.json").write_text( + '{"owner": "o", "repo": "r", "number": "1"}', encoding="utf-8" + ) + (tmp_path / "tools.tsv").write_text("ruff\tok\n", encoding="utf-8") + findings, disables, meta = load_all(tmp_path) + assert len(findings) == 1 + assert len(disables) == 1 + assert meta["owner"] == "o" + assert meta["tools"] == {"ruff": "ok"} diff --git a/skills/ess/summarize-change-log/LICENSE b/skills/ess/summarize-change-log/LICENSE index 9c7afe3..d645695 100644 --- a/skills/ess/summarize-change-log/LICENSE +++ b/skills/ess/summarize-change-log/LICENSE @@ -1,13 +1,202 @@ -Copyright 2025 Cisco Systems, Inc. or its Affiliates -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ - http://www.apache.org/licenses/LICENSE-2.0 + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/skills/ess/summarize-change-log/examples-good.md b/skills/ess/summarize-change-log/examples-good.md index 5dc6f78..abb6622 100644 --- a/skills/ess/summarize-change-log/examples-good.md +++ b/skills/ess/summarize-change-log/examples-good.md @@ -1,4 +1,4 @@ -- **fix(langsmith-client): centralize deploy naming** — Delegate `<service>-<env>` resolution to `langsmith-deploy-docker`, add `--deployment` for explicit base names, and resolve live deployments for idempotent delete. +- **fix(langsmith-client): centralize deploy naming** — Delegate `<service>-<env>` resolution to `langsmith-client deploy docker`, add `--deployment` for explicit base names, and resolve live deployments for idempotent delete. - **fix(azure-ai): align prod NAT gateway IPs** — Prod Azure OpenAI firewall rules were allowlisting dev EKS egress; prod and aoh-prod stacks now use the aoh-langsmith-hybrid-prod NAT IP. - **fix(langsmith-client): defer delete confirm** — Fix `deploy_docker` docstring, defer delete confirmation until a target exists, and remove erroneous `.env` gates from deploy scripts. - **docs(langsmith-client): document --deployment** — Clarify `DEPLOYMENT_NAME` in `push_secrets.py` and document `--deployment` in `docker-deployment.md`. diff --git a/skills/ess/summarize-change-log/scripts/test_validate_summary.py b/skills/ess/summarize-change-log/scripts/test_validate_summary.py index f9a46a2..50f519e 100644 --- a/skills/ess/summarize-change-log/scripts/test_validate_summary.py +++ b/skills/ess/summarize-change-log/scripts/test_validate_summary.py @@ -17,7 +17,7 @@ validate_summary_text, ) -_REPO_ROOT = Path(__file__).resolve().parents[5] +_REPO_ROOT = Path(__file__).resolve().parents[4] def test_parse_document_extracts_subjects() -> None: From cf7c7620a67ef275b8d8c0ef93aa593c6128eaac Mon Sep 17 00:00:00 2001 From: Matt Norris <matt@mattnorris.dev> Date: Fri, 31 Jul 2026 15:13:35 -0400 Subject: [PATCH 2/2] perf(essentials-sync): copy verbatim when the source scans clean Phase B handed every sync to the primary agent, including sources the scanners had already cleared. With no jargon to generalize the agent paraphrases working prose instead: on a 16-file skill directory it rewrote all 16 files and 187 lines, replacing a live sibling-skill reference and a named tool with vague descriptions, and swapping one table row for unrelated content. Skip the agent when --no-extract is set and the source pre-scan is empty, and copy the tree instead. The copy still faces the deterministic scanners and the adversarial reviewer, and escalates to the agent if either raises anything critical, so the safety bar is unchanged. The reviewer was always a detector; only the primary agent rewrote, and it did so unconditionally. Same run before and after: 11min with 187 changed lines, versus 27s with zero drift. The copy honors .gitignore and the scanners' skip list, which is a safety property rather than an optimization: the scanners never read gitignored paths, so copying them would ship content nothing vouched for, .env being the obvious hazard. Files copied but unreadable by the text scanners, binaries and anything past the size cap, are reported at the end of the run. Target-only files such as LICENSE and NOTICE are left alone so a re-sync cannot clobber them. Add --no-fast-copy to force the agent path. --- tools/typescript/essentials-sync/README.md | 13 +- tools/typescript/essentials-sync/src/agent.ts | 153 ++++++++++++++---- tools/typescript/essentials-sync/src/cli.ts | 3 + tools/typescript/essentials-sync/src/copy.ts | 73 +++++++++ .../src/scanners/text-files.ts | 14 +- tools/typescript/essentials-sync/src/types.ts | 1 + .../essentials-sync/tests/copy.test.ts | 125 ++++++++++++++ .../tests/fast-copy-gate.test.ts | 66 ++++++++ 8 files changed, 419 insertions(+), 29 deletions(-) create mode 100644 tools/typescript/essentials-sync/src/copy.ts create mode 100644 tools/typescript/essentials-sync/tests/copy.test.ts create mode 100644 tools/typescript/essentials-sync/tests/fast-copy-gate.test.ts diff --git a/tools/typescript/essentials-sync/README.md b/tools/typescript/essentials-sync/README.md index 270bc5f..34cbfc9 100644 --- a/tools/typescript/essentials-sync/README.md +++ b/tools/typescript/essentials-sync/README.md @@ -32,6 +32,16 @@ Critical findings block; non-blocking reviewer findings are surfaced as warnings For sources that are already generic and don't need an extract step, pass `--no-extract` and the tool runs Phase B only against `--source`. +### Fast-path copy + +When `--no-extract` is set and the source pre-scan comes back with **zero** findings, Phase B skips the primary agent and copies the tree verbatim instead. The copy is still held to the same bar afterwards: deterministic scanners run against the target, then the adversarial reviewer. If either surfaces a critical finding, the run escalates to the primary agent and continues as normal. + +This exists because a clean source gives the primary agent nothing useful to do, and an agent asked to generalize an already-generic tree will paraphrase working prose and drop valid cross-references instead. On a 16-file skill directory the fast path finished in 27s with zero content drift, against 11min and 187 rewritten lines for the agent path. + +The copy honors `.gitignore` and skips the same cache and build directories the scanners skip. That is a safety property, not an optimization: the scanners never read gitignored paths, so copying them would ship content nothing vouched for (`.env`, local credentials). Anything copied that the text scanners could not read -- binaries, files over the 5 MB cap -- is listed explicitly at the end of the run. Files that exist only in the target (`LICENSE`, `NOTICE`) are left untouched, so re-syncing never clobbers them. + +Pass `--no-fast-copy` to force the agent path regardless. + ## Installation ```bash @@ -104,6 +114,7 @@ essentials-sync [options] | `--source-repo <path>` | no | Source repo root. Default: the git root discovered by walking up from `--source`. | | `--package-name <name>` | no | Override for the extracted `ess-*` package name. Must start with `ess-` and be kebab-case. Default: `ess-<basename of --source>`. | | `--no-extract` | no | Skip Phase A. Assume `--source` is already a generic package and only run the sync to essentials. | +| `--no-fast-copy` | no | Always let the primary agent author the sync, even when the source scan is completely clean. See [Fast-path copy](#fast-path-copy). | | `--model <spec>` | no | Primary model. Accepts a concrete ID, a family sentinel (`opus`, `codex`, `claude`, `gpt`, `gemini`, `composer`), or `auto`. Default: `opus`. | | `--review-model <spec>` | no | Adversarial reviewer model. Same shape as `--model`. Default: `codex`. | | `--max-revisions <n>` | no | Maximum scan-and-revise iterations *per phase*. Default: `3`. | @@ -133,7 +144,7 @@ Files whose basename starts with `tmp-` or `tmp.` are skipped by the determinist npm test ``` -Tests use [Vitest](https://vitest.dev) and exercise the jargon and PII scanners against the `clean-package` and `dirty-package` fixtures under `tests/fixtures/`, plus the extract-plan name/path derivation logic. +Tests use [Vitest](https://vitest.dev) and cover the jargon and PII scanners against the `clean-package` and `dirty-package` fixtures under `tests/fixtures/`, the extract-plan name/path derivation logic, the verbatim copy (including that it refuses to copy gitignored files and preserves executable bits), and the fast-path eligibility gate. ## License diff --git a/tools/typescript/essentials-sync/src/agent.ts b/tools/typescript/essentials-sync/src/agent.ts index 48b8854..6097ed3 100644 --- a/tools/typescript/essentials-sync/src/agent.ts +++ b/tools/typescript/essentials-sync/src/agent.ts @@ -12,6 +12,7 @@ import { } from "./prompts.js"; import { runScanners, hasCriticalFindings, formatFindings } from "./scanners/index.js"; import { runReviewer } from "./reviewer.js"; +import { copyTreeVerbatim } from "./copy.js"; export type SessionPhase = "extract" | "sync"; @@ -24,6 +25,7 @@ export interface RunSyncSessionInputs { maxRevisions: number; adversarialReview: boolean; extraJargonTerms?: string[]; + fastCopy: boolean; } export interface RunSyncSessionResult { @@ -102,8 +104,109 @@ async function runExtractPhase(inputs: RunSyncSessionInputs): Promise<PhaseResul } } +// A source that scanned completely clean needs no rewriting, so the primary +// agent has nothing useful to do: pointing it at the tree anyway makes it +// paraphrase already-generic prose and drop valid cross-references. Copy +// instead, then hold the copy to the same scan-and-review bar. Returns null when +// something does surface, so the caller can escalate to the agent. +async function tryFastCopyPhase( + inputs: RunSyncSessionInputs, +): Promise<PhaseResult | null> { + const { apiKey, reviewModel, plan, adversarialReview, extraJargonTerms } = inputs; + + const copied = await copyTreeVerbatim(plan.sourceAbs, plan.targetAbs); + console.log( + chalk.green( + `[fast-copy] copied ${copied.filesCopied.length} file(s) verbatim ` + + `(source scan was clean, so there is nothing to generalize)`, + ), + ); + if (copied.unscannedFiles.length > 0) { + console.log( + chalk.yellow( + `[fast-copy] ${copied.unscannedFiles.length} file(s) copied without text scanning ` + + `(binary or over the size cap) -- review manually:`, + ), + ); + for (const relPath of copied.unscannedFiles) { + console.log(chalk.yellow(` - ${relPath}`)); + } + } + if (copied.symlinksSkipped.length > 0) { + console.log( + chalk.yellow( + `[fast-copy] skipped ${copied.symlinksSkipped.length} symlink(s): ` + + copied.symlinksSkipped.join(", "), + ), + ); + } + + const deterministic = await runScanners({ + rootPath: plan.targetAbs, + jargonSeverity: "critical", + extraJargonTerms, + }); + logScanReport("fast-copy deterministic", deterministic); + if (deterministic.findings.some((finding) => finding.severity === "critical")) { + console.log( + chalk.yellow( + "[fast-copy] target scan surfaced critical finding(s) the source scan did not; " + + "escalating to the primary agent.", + ), + ); + return null; + } + + if (adversarialReview) { + console.log(chalk.dim(`[reviewer] running on model ${reviewModel}`)); + const reviewerFindings = await runReviewer({ + apiKey, + reviewModel, + plan, + reviewRootAbs: plan.targetAbs, + cwd: plan.targetRepoAbs, + }); + const criticals = reviewerFindings.filter((finding) => finding.severity === "critical"); + reportReviewerWarnings(reviewerFindings); + if (criticals.length > 0) { + console.log( + chalk.yellow( + `[fast-copy] reviewer raised ${criticals.length} critical finding(s); ` + + "escalating to the primary agent.", + ), + ); + return null; + } + } + + return { status: "clean", remainingFindings: [], iterations: 0 }; +} + +// The fast path is only sound when a scan actually vouched for the source. +// Extract mode is excluded because Phase A's output is authored by the agent, so +// there is no pre-existing generic tree to copy. +export function canFastCopy(inputs: RunSyncSessionInputs): boolean { + if (!inputs.fastCopy) return false; + if (inputs.plan.phaseAEnabled) return false; + if (!inputs.sourceScan) return false; + return inputs.sourceScan.findings.length === 0; +} + async function runSyncPhase(inputs: RunSyncSessionInputs): Promise<PhaseResult> { const { apiKey, primaryModel, reviewModel, plan, sourceScan, maxRevisions, adversarialReview, extraJargonTerms } = inputs; + + if (canFastCopy(inputs)) { + const fastResult = await tryFastCopyPhase(inputs); + if (fastResult) { + return fastResult; + } + } else if (inputs.fastCopy && !inputs.plan.phaseAEnabled) { + const reason = inputs.sourceScan + ? `source scan found ${inputs.sourceScan.findings.length} finding(s)` + : "source scan was skipped"; + console.log(chalk.dim(`[fast-copy] not eligible (${reason}); using the primary agent.`)); + } + const agent = await Agent.create({ apiKey, model: { id: primaryModel }, @@ -214,35 +317,10 @@ async function scanAndReviseLoop({ reviewRootAbs, cwd: reviewerCwd, }); - const reviewerCritical = reviewerFindings.filter( + combinedFindings = reviewerFindings.filter( (finding) => finding.severity === "critical", ); - const reviewerWarnings = reviewerFindings.filter( - (finding) => finding.severity === "warning", - ); - combinedFindings = reviewerCritical; - if (reviewerFindings.length > 0) { - console.log( - chalk.dim( - `[reviewer] returned ${reviewerFindings.length} finding(s) ` - + `(critical=${reviewerCritical.length}, warning=${reviewerWarnings.length})`, - ), - ); - } - if (reviewerWarnings.length > 0) { - console.log( - chalk.yellow( - `[reviewer] ${reviewerWarnings.length} non-blocking finding(s) (review manually):`, - ), - ); - for (const finding of reviewerWarnings) { - console.log( - chalk.yellow( - ` - ${finding.file}:${finding.line} [${finding.type}] ${finding.message}`, - ), - ); - } - } + reportReviewerWarnings(reviewerFindings); } if (combinedFindings.length === 0) { @@ -342,6 +420,27 @@ function extractAssistantText(event: unknown): string | null { return parts.length > 0 ? parts.join("") : null; } +function reportReviewerWarnings(reviewerFindings: Finding[]): void { + if (reviewerFindings.length === 0) return; + const criticals = reviewerFindings.filter((finding) => finding.severity === "critical"); + const warnings = reviewerFindings.filter((finding) => finding.severity === "warning"); + console.log( + chalk.dim( + `[reviewer] returned ${reviewerFindings.length} finding(s) ` + + `(critical=${criticals.length}, warning=${warnings.length})`, + ), + ); + if (warnings.length === 0) return; + console.log( + chalk.yellow(`[reviewer] ${warnings.length} non-blocking finding(s) (review manually):`), + ); + for (const finding of warnings) { + console.log( + chalk.yellow(` - ${finding.file}:${finding.line} [${finding.type}] ${finding.message}`), + ); + } +} + function logScanReport(label: string, report: ScanReport): void { if (!hasCriticalFindings(report) && report.findings.length === 0) { console.log(chalk.green(`[scan] ${label}: clean (${report.durationMs} ms)`)); diff --git a/tools/typescript/essentials-sync/src/cli.ts b/tools/typescript/essentials-sync/src/cli.ts index 37f8cb6..c035557 100644 --- a/tools/typescript/essentials-sync/src/cli.ts +++ b/tools/typescript/essentials-sync/src/cli.ts @@ -39,6 +39,7 @@ const program = new Command() .option("--no-source-scan", "Skip the informational source pre-scan") .option("--no-adversarial-review", "Skip the LLM reviewer; deterministic scanners only") .option("--no-extract", "Skip the in-source extract phase; assume --source is already a generic package and only run the sync to essentials") + .option("--no-fast-copy", "Always let the primary agent author the sync, even when the source scan is completely clean") .option("--list-models", "Print the model catalog for the current API key and exit", false) .parse(process.argv); @@ -75,6 +76,7 @@ async function main(cmd: Command): Promise<void> { const noSourceScan = opts.sourceScan === false; const noAdversarial = opts.adversarialReview === false; const extractEnabled = opts.extract !== false; + const fastCopy = opts.fastCopy !== false; let extractHalf = null; if (extractEnabled) { @@ -206,6 +208,7 @@ async function main(cmd: Command): Promise<void> { maxRevisions, adversarialReview: !noAdversarial, extraJargonTerms, + fastCopy, }); if (result.status === "clean") { diff --git a/tools/typescript/essentials-sync/src/copy.ts b/tools/typescript/essentials-sync/src/copy.ts new file mode 100644 index 0000000..9587697 --- /dev/null +++ b/tools/typescript/essentials-sync/src/copy.ts @@ -0,0 +1,73 @@ +import { promises as fs } from "node:fs"; +import path from "node:path"; +import { globby } from "globby"; +import { HARD_SKIP_DIRECTORIES, isUnscannableTextFile } from "./scanners/text-files.js"; + +export interface CopyTreeResult { + filesCopied: string[]; + // Copied files the text scanners never read (binary extension or over the + // size cap). They are still copied so the package stays complete, but the + // caller surfaces them so a human knows what went out unscanned. + unscannedFiles: string[]; + symlinksSkipped: string[]; +} + +// Copies sourceAbs into targetAbs without deleting anything already in the +// target, so target-only files (LICENSE, NOTICE) survive a re-sync. +// +// Traversal deliberately mirrors the scanners: .gitignore is honored and the +// same cache/build directories are skipped. That invariant is what makes the +// fast path safe -- every file this copies is a file the scanners inspected, so +// a clean scan really does cover the whole payload. Copying gitignored files +// would break it, since those are exactly the paths (.env, local credentials) +// the scanners never see. +export async function copyTreeVerbatim( + sourceAbs: string, + targetAbs: string, +): Promise<CopyTreeResult> { + // onlyFiles is off so symlinks come back as entries we can classify and + // report; with it on, globby drops them silently and the copy would be + // quietly incomplete. + const relPaths = await globby("**/*", { + cwd: sourceAbs, + dot: true, + gitignore: true, + onlyFiles: false, + followSymbolicLinks: false, + ignore: HARD_SKIP_DIRECTORIES.map((dir) => `**/${dir}/**`), + }); + relPaths.sort(); + + const filesCopied: string[] = []; + const unscannedFiles: string[] = []; + const symlinksSkipped: string[] = []; + + for (const relPath of relPaths) { + const from = path.join(sourceAbs, relPath); + const to = path.join(targetAbs, relPath); + + const stat = await fs.lstat(from); + if (stat.isSymbolicLink()) { + symlinksSkipped.push(relPath); + continue; + } + if (stat.isDirectory()) { + continue; + } + if (!stat.isFile()) { + continue; + } + + await fs.mkdir(path.dirname(to), { recursive: true }); + // copyFile carries the source mode across, which keeps the executable bit + // on shipped scripts. + await fs.copyFile(from, to); + filesCopied.push(relPath); + + if (await isUnscannableTextFile(from)) { + unscannedFiles.push(relPath); + } + } + + return { filesCopied, unscannedFiles, symlinksSkipped }; +} diff --git a/tools/typescript/essentials-sync/src/scanners/text-files.ts b/tools/typescript/essentials-sync/src/scanners/text-files.ts index f533320..ab1d789 100644 --- a/tools/typescript/essentials-sync/src/scanners/text-files.ts +++ b/tools/typescript/essentials-sync/src/scanners/text-files.ts @@ -1,11 +1,12 @@ import { promises as fs } from "node:fs"; +import path from "node:path"; import { globbyStream } from "globby"; import type { Finding } from "../types.js"; // Always skipped, even when the scanned tree has no .gitignore. These are // caches, build output, and VCS/dependency directories that never carry // meaningful findings. -const HARD_SKIP_DIRECTORIES = [ +export const HARD_SKIP_DIRECTORIES = [ "node_modules", ".git", ".venv", "venv", "__pycache__", ".pytest_cache", ".ruff_cache", ".mypy_cache", "dist", "build", ".pulumi", ]; @@ -18,6 +19,17 @@ const BINARY_EXTENSIONS = [ const MAX_FILE_BYTES = 5 * 1024 * 1024; +// True when walkTextFiles would have passed over this file: a binary extension +// or past the size cap. Callers that copy files rather than scan them use this +// to report what left the source repo without being read. +export async function isUnscannableTextFile(filePath: string): Promise<boolean> { + if (BINARY_EXTENSIONS.includes(path.extname(filePath).toLowerCase())) { + return true; + } + const stat = await fs.stat(filePath); + return stat.size > MAX_FILE_BYTES; +} + // Yields absolute paths of candidate text files under rootPath. Traversal, // .gitignore handling, and directory/extension skipping are delegated to // globby; the only thing globby cannot express is the per-file size cap, so diff --git a/tools/typescript/essentials-sync/src/types.ts b/tools/typescript/essentials-sync/src/types.ts index fc55655..c950a40 100644 --- a/tools/typescript/essentials-sync/src/types.ts +++ b/tools/typescript/essentials-sync/src/types.ts @@ -73,4 +73,5 @@ export interface CliOptions { noSourceScan: boolean; noAdversarialReview: boolean; noExtract: boolean; + noFastCopy: boolean; } diff --git a/tools/typescript/essentials-sync/tests/copy.test.ts b/tools/typescript/essentials-sync/tests/copy.test.ts new file mode 100644 index 0000000..d13b318 --- /dev/null +++ b/tools/typescript/essentials-sync/tests/copy.test.ts @@ -0,0 +1,125 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { promises as fs } from "node:fs"; +import path from "node:path"; +import os from "node:os"; +import { copyTreeVerbatim } from "../src/copy.js"; + +const tempRoots: string[] = []; + +async function makeTree( + files: Record<string, string>, +): Promise<string> { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "copy-test-")); + tempRoots.push(root); + for (const [relPath, contents] of Object.entries(files)) { + const abs = path.join(root, relPath); + await fs.mkdir(path.dirname(abs), { recursive: true }); + await fs.writeFile(abs, contents, "utf8"); + } + return root; +} + +afterEach(async () => { + while (tempRoots.length > 0) { + const root = tempRoots.pop(); + if (root) await fs.rm(root, { recursive: true, force: true }); + } +}); + +describe("copyTreeVerbatim", () => { + it("copies files byte-for-byte", async () => { + const source = await makeTree({ + "SKILL.md": "# Skill\nbody\n", + "references/guide.md": "guidance\n", + }); + const target = await makeTree({}); + + const result = await copyTreeVerbatim(source, target); + + expect(result.filesCopied.sort()).toEqual(["SKILL.md", "references/guide.md"]); + expect(await fs.readFile(path.join(target, "SKILL.md"), "utf8")).toBe("# Skill\nbody\n"); + expect(await fs.readFile(path.join(target, "references/guide.md"), "utf8")).toBe("guidance\n"); + }); + + it("preserves the executable bit on scripts", async () => { + const source = await makeTree({ "scripts/run.sh": "#!/usr/bin/env bash\n" }); + await fs.chmod(path.join(source, "scripts/run.sh"), 0o755); + const target = await makeTree({}); + + await copyTreeVerbatim(source, target); + + const stat = await fs.stat(path.join(target, "scripts/run.sh")); + expect(stat.mode & 0o111).not.toBe(0); + }); + + // The safety invariant behind the fast path: the scanners skip gitignored + // paths, so copying them would ship unscanned content (.env, local creds). + it("never copies gitignored files", async () => { + const source = await makeTree({ + ".gitignore": ".env\nsecrets/\n", + "SKILL.md": "# Skill\n", + ".env": "API_KEY=sk_live_do_not_ship\n", + "secrets/token.txt": "token\n", + }); + const target = await makeTree({}); + + const result = await copyTreeVerbatim(source, target); + + expect(result.filesCopied).toContain("SKILL.md"); + expect(result.filesCopied).not.toContain(".env"); + expect(result.filesCopied).not.toContain("secrets/token.txt"); + await expect(fs.stat(path.join(target, ".env"))).rejects.toThrow(); + await expect(fs.stat(path.join(target, "secrets/token.txt"))).rejects.toThrow(); + }); + + it("skips cache and build directories", async () => { + const source = await makeTree({ + "scripts/report.py": "print('hi')\n", + "scripts/__pycache__/report.cpython-312.pyc": "bytecode\n", + "node_modules/dep/index.js": "module.exports = 1\n", + }); + const target = await makeTree({}); + + const result = await copyTreeVerbatim(source, target); + + expect(result.filesCopied).toEqual(["scripts/report.py"]); + }); + + // Re-syncing must not clobber files that exist only in the open-source repo. + it("leaves target-only files in place", async () => { + const source = await makeTree({ "SKILL.md": "updated\n" }); + const target = await makeTree({ + "SKILL.md": "stale\n", + LICENSE: "Apache-2.0 full text\n", + }); + + await copyTreeVerbatim(source, target); + + expect(await fs.readFile(path.join(target, "SKILL.md"), "utf8")).toBe("updated\n"); + expect(await fs.readFile(path.join(target, "LICENSE"), "utf8")).toBe("Apache-2.0 full text\n"); + }); + + it("reports copied files the text scanners could not read", async () => { + const source = await makeTree({ + "SKILL.md": "# Skill\n", + "docs/diagram.png": "not really a png\n", + }); + const target = await makeTree({}); + + const result = await copyTreeVerbatim(source, target); + + expect(result.filesCopied).toContain("docs/diagram.png"); + expect(result.unscannedFiles).toEqual(["docs/diagram.png"]); + }); + + it("skips symlinks instead of dereferencing them", async () => { + const source = await makeTree({ "SKILL.md": "# Skill\n" }); + await fs.symlink(path.join(source, "SKILL.md"), path.join(source, "alias.md")); + const target = await makeTree({}); + + const result = await copyTreeVerbatim(source, target); + + expect(result.filesCopied).toEqual(["SKILL.md"]); + expect(result.symlinksSkipped).toEqual(["alias.md"]); + }); +}); diff --git a/tools/typescript/essentials-sync/tests/fast-copy-gate.test.ts b/tools/typescript/essentials-sync/tests/fast-copy-gate.test.ts new file mode 100644 index 0000000..a362d6c --- /dev/null +++ b/tools/typescript/essentials-sync/tests/fast-copy-gate.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { canFastCopy } from "../src/agent.js"; +import type { RunSyncSessionInputs } from "../src/agent.js"; +import type { Finding, ScanReport, SyncPlan } from "../src/types.js"; + +const plan: SyncPlan = { + mode: "COPY", + sourceAbs: "/src/skills/ess/demo", + targetRepoAbs: "/target", + targetPathRel: "skills/ess/demo", + targetAbs: "/target/skills/ess/demo", + phaseAEnabled: false, +}; + +function scan(findings: Finding[]): ScanReport { + return { findings, scannedPaths: ["/src/skills/ess/demo"], durationMs: 1 }; +} + +const jargonFinding: Finding = { + file: "SKILL.md", + line: 3, + type: "jargon", + severity: "warning", + source: "jargon", + message: "company term", + term: "cisco", +}; + +function inputs(overrides: Partial<RunSyncSessionInputs> = {}): RunSyncSessionInputs { + return { + apiKey: "test", + primaryModel: "primary", + reviewModel: "review", + plan, + sourceScan: scan([]), + maxRevisions: 3, + adversarialReview: true, + fastCopy: true, + ...overrides, + }; +} + +describe("canFastCopy", () => { + it("allows the copy when the source scan is completely clean", () => { + expect(canFastCopy(inputs())).toBe(true); + }); + + it("refuses when the source scan found anything, even a warning", () => { + expect(canFastCopy(inputs({ sourceScan: scan([jargonFinding]) }))).toBe(false); + }); + + // Without a scan there is no evidence the source is generic, so the agent has + // to do the work. + it("refuses when the source scan was skipped", () => { + expect(canFastCopy(inputs({ sourceScan: null }))).toBe(false); + }); + + // Phase A output is authored by the agent; there is no pre-existing tree. + it("refuses in extract mode", () => { + expect(canFastCopy(inputs({ plan: { ...plan, phaseAEnabled: true } }))).toBe(false); + }); + + it("refuses when the operator passed --no-fast-copy", () => { + expect(canFastCopy(inputs({ fastCopy: false }))).toBe(false); + }); +});