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: 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); + }); +});