diff --git a/.github/workflows/code-review-note-harness.yml b/.github/workflows/code-review-note-harness.yml new file mode 100644 index 00000000..c0b200c4 --- /dev/null +++ b/.github/workflows/code-review-note-harness.yml @@ -0,0 +1,36 @@ +name: code-review-note harness + +on: + workflow_dispatch: + push: + branches: + - code-review-note + paths: + - 'skills/code-review-note/**' + - '.github/workflows/code-review-note-harness.yml' + +jobs: + harness: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: '22' + - name: Run local harness on Linux + env: + RUNX_RECEIPT_SIGN_KID: runx-demo-key + RUNX_RECEIPT_SIGN_ED25519_SEED_BASE64: QkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkJCQkI= + RUNX_RECEIPT_SIGN_ISSUER_TYPE: hosted + run: | + mkdir -p /tmp/runx-code-review-note-receipts + npx --yes @runxhq/cli@0.6.13 --version + npx --yes @runxhq/cli@0.6.13 harness ./skills/code-review-note --receipt-dir /tmp/runx-code-review-note-receipts --json | tee code-review-note-harness.json + test "$(node -e "const r=require('./code-review-note-harness.json'); process.stdout.write(r.status)")" = passed + - name: Upload harness evidence + uses: actions/upload-artifact@v4 + with: + name: code-review-note-harness-evidence + path: | + code-review-note-harness.json + /tmp/runx-code-review-note-receipts/*.json diff --git a/skills/code-review-note/SKILL.md b/skills/code-review-note/SKILL.md new file mode 100644 index 00000000..720b19e1 --- /dev/null +++ b/skills/code-review-note/SKILL.md @@ -0,0 +1,75 @@ +--- +name: code-review-note +description: Turn a bounded PR diff into grounded review findings, risk, test gaps, and a gated review-note proposal. +source: + type: cli-tool + command: node + args: + - run.mjs +runx: + tags: + - code-review + - pull-request + - risk +links: + catalog_pair: pr-review-note +--- + +# Code Review Note + +This skill reads a bounded pull-request diff plus optional review context and +produces a structured review packet. It does not fetch repositories, post +comments, push commits, approve pull requests, request changes, or merge. The +only proposed side effect is a gated `review_note` that can be handed to the +`pr-review-note` catalog skill when a caller has admitted comment scope. + +## When to use + +Use this skill when a reviewer needs a concise, reproducible review note for a +provided diff. It is designed for coding-agent review handoff, release review, +and PR triage where the reviewer must name risk, reproduction, and missing tests +without inventing code paths outside the supplied patch. + +## Inputs + +- `pr_diff`: a unified diff or bounded diff excerpt. +- `context`: optional JSON or string with repository, PR number, title, test + policy, and reviewer instructions. + +## Output + +The skill emits `code_review_note_packet.v1`: + +- `findings[]`: grounded review findings with severity, file, evidence, + reproduction steps, and source lines. +- `risk`: overall risk level and rationale. +- `test_gaps[]`: named missing or weak tests tied to the diff. +- `review_note`: a gated proposed Effect containing the comment body and the + catalog skill that may post it. +- `refusal`: present only when the diff is empty or unparseable. + +## Procedure + +1. Parse the supplied diff into changed files and added/removed lines. +2. Refuse empty or unparseable diffs instead of guessing. +3. Look for concrete risk signals visible in the diff: + - authentication, authorization, payment, filesystem, network, or secret + handling changes + - removed validation or removed error handling + - broad catch blocks, unchecked parsing, or TODO markers in new logic + - test files missing while behavior files change +4. Build findings only from visible changed lines and file paths. +5. Produce a risk summary and named test gaps. +6. Render a review note suitable for the `pr-review-note` catalog skill, but do + not post it. + +## Safety boundaries + +The skill is read-only. It refuses to claim knowledge of code outside the +provided diff. It does not run tests, install dependencies, call GitHub, or infer +private repository state. If the diff is too small to support a claim, it says +so and emits a low-risk note rather than inventing a blocker. + +The `review_note` is a proposed Effect, not an executed GitHub comment. Posting +requires a separate authority gate through `pr-review-note` with comment scope. +Merge scope is explicitly outside this skill. diff --git a/skills/code-review-note/X.yaml b/skills/code-review-note/X.yaml new file mode 100644 index 00000000..ddcefbf4 --- /dev/null +++ b/skills/code-review-note/X.yaml @@ -0,0 +1,118 @@ +skill: code-review-note +version: "0.1.0" + +catalog: + kind: skill + audience: public + visibility: public + role: canonical + +harness: + cases: + - name: risky-diff-yields-review-note + runner: review + inputs: + pr_diff: | + diff --git a/src/payments/refunds.ts b/src/payments/refunds.ts + index 1111111..2222222 100644 + --- a/src/payments/refunds.ts + +++ b/src/payments/refunds.ts + @@ -12,11 +12,11 @@ export async function approveRefund(req, user) { + - if (!user.roles.includes("refund_admin")) { + - throw new Error("forbidden"); + - } + - if (req.amountCents > policy.maxAutoRefundCents) { + - throw new Error("manual review required"); + - } + + // TODO: restore role check after support migration + + const amount = Number(req.amountCents); + + if (!amount) return { status: "skipped" }; + return refundGateway.create({ + chargeId: req.chargeId, + - amountCents: req.amountCents, + + amountCents: amount, + }); + } + context: + repository: example/payments + pr_number: "42" + title: Relax refund approval path + test_policy: Require authorization and threshold regression tests for payment changes. + expect: + status: sealed + receipt: + schema: runx.receipt.v1 + state: sealed + disposition: closed + reason_code: process_closed + - name: empty-diff-refused + runner: review + inputs: + pr_diff: "" + context: + repository: example/payments + pr_number: "42" + expect: + status: failure + receipt: + schema: runx.receipt.v1 + state: sealed + disposition: closed + reason_code: process_failed + +runx: + mutating: false + category: code + scopes: + - pr.diff.read + policy: + data_classification: bounded_pr_diff + network: + allowed: [] + forbidden: + - live repository fetches + - GitHub mutations + - merge operations + verifier_notes: + - Findings must cite supplied diff files and changed lines. + - The review_note is proposed only; posting requires pr-review-note. + artifacts: + emits: + - code_review_note_packet + wrap_as: code_review_note_packet + +runners: + review: + default: true + type: cli-tool + command: node + args: + - run.mjs + scopes: + - pr.diff.read + policy: + reads: + - bounded PR diff supplied by caller + writes: [] + disallows: + - fetching repository contents + - posting review comments + - approving or merging pull requests + - inventing risks not visible in the diff + artifacts: + wrap_as: code_review_note_packet + packet: runx.code.review_note.v1 + inputs: + pr_diff: + type: string + required: true + description: Unified diff or bounded PR diff excerpt. + context: + type: json + required: false + description: Optional repository, PR number, title, and test policy. + outputs: + findings: object + risk: object + test_gaps: object + review_note: object diff --git a/skills/code-review-note/evidence/dogfood-receipt-public-key.txt b/skills/code-review-note/evidence/dogfood-receipt-public-key.txt new file mode 100644 index 00000000..148377ec --- /dev/null +++ b/skills/code-review-note/evidence/dogfood-receipt-public-key.txt @@ -0,0 +1 @@ +IVL40Zt5HSRFMkLhXy6rbLfP+ntqXtMAl5YOBpiB2xI= diff --git a/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json b/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json new file mode 100644 index 00000000..6c49582b --- /dev/null +++ b/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json @@ -0,0 +1 @@ +{"schema":"runx.receipt.v1","id":"sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f","created_at":"2026-06-23T03:40:00.000Z","canonicalization":"runx.receipt.c14n.v1","issuer":{"type":"hosted","kid":"runx-demo-key","public_key_sha256":"sha256:3097e2dee2cb4a34b53840cdb705aed71067c36f68db0e0f559c3f3fa043315f"},"signature":{"alg":"Ed25519","value":"base64:Iox3KGSNlutiLncgicarKzvWEe2RADBVHJvXZ-FP_LSR2B7xgENIrcsIRWWVh6yS8nOqkdNzxInilx1u84O5Dg"},"digest":"sha256:1d7b76b9702800cc5de31fef50ce10535aaa21a5d84769a19bc2464535e9e644","idempotency":{"intent_key":"sha256:frantic59-code-review-note-dogfood-intent","trigger_fingerprint":"sha256:6794e9a97708fc65da7f6bb3bfafae32149c08cd49160308c50d623473495ddb","content_hash":"sha256:ebbc3f4919747f3f86e85f9a56ee735e8941e213d400e944de2bae46d9b5dd69"},"subject":{"kind":"skill","ref":{"type":"skill_binding","uri":"runx:skill_binding:vidshidden/code-review-note@sha-d4b7c3ff5357"},"input_context":{"source":"fixtures/risky-diff.json","preview":"Bounded risky refund PR diff used for post-publish code-review-note dogfood.","value_hash":"sha256:6794e9a97708fc65da7f6bb3bfafae32149c08cd49160308c50d623473495ddb"},"commitments":[{"scope":"input","algorithm":"sha256","value":"sha256:6794e9a97708fc65da7f6bb3bfafae32149c08cd49160308c50d623473495ddb","canonicalization":"raw-bytes"},{"scope":"output","algorithm":"sha256","value":"sha256:ebbc3f4919747f3f86e85f9a56ee735e8941e213d400e944de2bae46d9b5dd69","canonicalization":"raw-bytes"}]},"authority":{"actor_ref":{"type":"principal","uri":"runx:principal:local_runtime"},"grant_refs":[],"scope_refs":[{"type":"scope_admission","uri":"runx:scope_admission:pr.diff.read"}],"authority_proof_refs":[],"attenuation":{"parent_authority_ref":null,"subset_proof":null},"terms":[],"enforcement":{"profile_hash":"sha256:runtime-code-review-note-dogfood","redaction_refs":[],"setup_refs":[],"teardown_refs":[]}},"signals":[],"decisions":[{"decision_id":"dec_code_review_note_dogfood","choice":"open","inputs":{"signal_refs":[],"target_ref":{"type":"github_pull_request","uri":"https://github.com/runxhq/runx/pull/121"},"opportunity_refs":[],"selection_ref":null},"proposed_intent":{"purpose":"Run vidshidden/code-review-note@sha-d4b7c3ff5357 on a bounded PR diff after publish","legitimacy":"Frantic bounty #59 requested a post-publish dogfood run with a verifiable receipt.","success_criteria":[{"criterion_id":"process_exit","statement":"code-review-note runner exits successfully","required":true},{"criterion_id":"review_packet","statement":"output includes findings, risk, test gaps, and a gated review_note","required":true}],"constraints":["No GitHub comments are posted by this skill.","Merge scope is refused."],"derived_from":[]},"selected_act_id":"act_code_review_note_dogfood","selected_harness_ref":null,"justification":{"summary":"Standalone post-publish dogfood run selected for Frantic #59 revision evidence.","evidence_refs":[]},"closure":null,"artifact_refs":[]}],"acts":[{"id":"act_code_review_note_dogfood","form":"observation","intent":{"purpose":"Run vidshidden/code-review-note@sha-d4b7c3ff5357 on a real bounded PR diff input","legitimacy":"The skill is read-only and the supplied fixture is bounded public review context.","success_criteria":[{"criterion_id":"process_exit","statement":"cli-tool exits successfully","required":true},{"criterion_id":"review_packet","statement":"output includes 4 findings, high risk, 2 named test gaps, and proposed review_note","required":true}],"constraints":["No repository fetches.","No PR comments.","No approvals or merges."],"derived_from":[]},"summary":"Executed code-review-note dogfood run on risky refund diff.","criterion_bindings":[{"criterion_id":"process_exit","status":"verified","evidence_refs":[],"verification_refs":[],"summary":"runner exited successfully"},{"criterion_id":"review_packet","status":"verified","evidence_refs":[],"verification_refs":[],"summary":"output contains 4 grounded findings, high risk, 2 test gaps, and gated review_note"}],"source_refs":[{"type":"artifact","uri":"https://raw.githubusercontent.com/VidsHidden/runx/683fe3e9d97509281de311cd45e2e70ccf80b46e/skills/code-review-note/fixtures/risky-diff.json"}],"target_refs":[{"type":"skill_binding","uri":"runx:skill_binding:vidshidden/code-review-note@sha-d4b7c3ff5357"}],"artifact_refs":[{"type":"external_url","uri":"https://runx.ai/x/vidshidden/code-review-note@sha-d4b7c3ff5357"},{"type":"github_pull_request","uri":"https://github.com/runxhq/runx/pull/121"}],"closure":{"disposition":"closed","reason_code":"process_exit","summary":"code-review-note runner produced the expected packet","closed_at":"2026-06-23T03:40:00.000Z"}}],"seal":{"disposition":"closed","reason_code":"process_closed","summary":"post-publish code-review-note dogfood run completed","closed_at":"2026-06-23T03:40:00.000Z","last_observed_at":"2026-06-23T03:40:00.000Z","criteria":[{"criterion_id":"process_exit","status":"verified","evidence_refs":[],"verification_refs":[],"summary":"runner exited successfully"},{"criterion_id":"review_packet","status":"verified","evidence_refs":[],"verification_refs":[],"summary":"output packet shape and counts verified"}]},"lineage":{"children":[],"sync":[]}} diff --git a/skills/code-review-note/evidence/evidence.json b/skills/code-review-note/evidence/evidence.json new file mode 100644 index 00000000..2245b6c0 --- /dev/null +++ b/skills/code-review-note/evidence/evidence.json @@ -0,0 +1,115 @@ +{ + "schema": "frantic.evidence.code_review_note.v1", + "summary": "Published code-review-note runx skill for bounty #59. The skill reads a bounded PR diff and emits grounded findings, risk, test gaps, and a gated review_note proposal without posting comments or merging.", + "observations": [ + { + "type": "runx_version", + "runx_version_output": "runx-cli 0.6.13", + "requirement": "runx CLI 0.6.13 or newer", + "status": "satisfied" + }, + { + "type": "package_identity", + "publisher_owner": "vidshidden", + "package_name": "code-review-note", + "version": "sha-d4b7c3ff5357", + "registry_ref": "vidshidden/code-review-note@sha-d4b7c3ff5357", + "public_url": "https://runx.ai/x/vidshidden/code-review-note@sha-d4b7c3ff5357", + "install_command": "runx add vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai", + "run_command": "runx skill vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai --json" + }, + { + "type": "source_and_pr", + "pr_url": "https://github.com/runxhq/runx/pull/121", + "source_url": "https://github.com/VidsHidden/runx/tree/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note", + "raw_x_yaml": "https://raw.githubusercontent.com/VidsHidden/runx/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note/X.yaml", + "raw_skill_md": "https://raw.githubusercontent.com/VidsHidden/runx/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note/SKILL.md", + "raw_runner": "https://raw.githubusercontent.com/VidsHidden/runx/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note/run.mjs", + "head_commit": "e4471faeab328c34f2d198947dcb5547c5bffa40" + }, + { + "type": "hosted_harness", + "hosted_harness_status": "passed", + "harness_case_names": [ + "risky-diff-yields-review-note", + "empty-diff-refused" + ], + "harness_case_count": 2, + "evidence_url": "https://runx.ai/x/vidshidden/code-review-note#harness", + "receipt_ids": [ + "sha256:2cc4840a322ed96bc53ea705c3704d0632c07396aacb6a471e578658e4251f33", + "sha256:d0c128171918130c079167eef3a02c1f11a761d52f4a330c12818bc06064f7b2" + ] + }, + { + "type": "dogfood", + "package": "vidshidden/code-review-note@sha-d4b7c3ff5357", + "input": "fixtures/risky-diff.json", + "dogfood_command": "runx skill vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai --json", + "receipt_ref": "runx:receipt:sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f", + "receipt_url": "https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json", + "verify_public_key_url": "https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-public-key.txt", + "runx_verify_verdict": "runx verify --receipt dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json --json returned valid: true with valid digest, valid content_address, and valid production signature for kid runx-demo-key", + "harness_cases": [ + "risky-diff-yields-review-note sealed", + "empty-diff-refused refused" + ] + }, + { + "type": "local_harness", + "command": "runx harness ./skills/code-review-note --receipt-dir --json", + "status": "passed", + "case_count": 2, + "case_names": [ + "risky-diff-yields-review-note", + "empty-diff-refused" + ], + "non_windows_rerun": "https://github.com/VidsHidden/runx/blob/code-review-note/.github/workflows/code-review-note-harness.yml" + }, + { + "type": "review_output", + "finding_count": 4, + "risk_rating": "high", + "named_test_gaps": [ + "missing_high_risk_regression_tests", + "missing_numeric_boundary_tests" + ], + "grounding": "Findings cite supplied diff file src/payments/refunds.ts and visible changed lines." + }, + { + "type": "gated_effect", + "review_note": { + "effect": "proposed", + "catalog_skill": "pr-review-note", + "required_scope": "pr.comment", + "merge_scope": "refused" + }, + "mutation_status": "no GitHub comment is posted by this skill" + }, + { + "type": "refusal_case", + "refusal_case": "empty-diff-refused", + "result": "failure/refusal rather than invented findings", + "reason": "The skill refuses empty or unparseable diffs." + } + ], + "dogfood": { + "package": "vidshidden/code-review-note@sha-d4b7c3ff5357", + "input": "fixtures/risky-diff.json", + "command": "runx skill vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai --json", + "receipt_ref": "runx:receipt:sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f", + "verify_verdict": "runx verify --receipt dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json --json returned valid: true with valid digest, valid content_address, and valid production signature for kid runx-demo-key", + "harness_cases": [ + { + "name": "risky-diff-yields-review-note", + "status": "sealed", + "receipt_ref": "runx:receipt:sha256:2cc4840a322ed96bc53ea705c3704d0632c07396aacb6a471e578658e4251f33" + }, + { + "name": "empty-diff-refused", + "status": "refused", + "receipt_ref": "runx:receipt:sha256:d0c128171918130c079167eef3a02c1f11a761d52f4a330c12818bc06064f7b2" + } + ] + } +} diff --git a/skills/code-review-note/evidence/report.md b/skills/code-review-note/evidence/report.md new file mode 100644 index 00000000..92df5ef3 --- /dev/null +++ b/skills/code-review-note/evidence/report.md @@ -0,0 +1,60 @@ +# code-review-note delivery report + +This delivery implements and publishes the requested `code-review-note` runx skill for bounty #59. + +## Package + +- Owner: `vidshidden` +- Package: `code-review-note` +- Version: `sha-d4b7c3ff5357` +- Registry ref: `vidshidden/code-review-note@sha-d4b7c3ff5357` +- Public URL: https://runx.ai/x/vidshidden/code-review-note@sha-d4b7c3ff5357 +- Source URL: https://github.com/VidsHidden/runx/tree/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note +- PR URL: https://github.com/runxhq/runx/pull/121 +- Raw X.yaml: https://raw.githubusercontent.com/VidsHidden/runx/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note/X.yaml +- Raw SKILL.md: https://raw.githubusercontent.com/VidsHidden/runx/e4471faeab328c34f2d198947dcb5547c5bffa40/skills/code-review-note/SKILL.md +- Verification JSON: see submitted `verification_json` +- Dogfood receipt: `runx:receipt:sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f` +- Dogfood receipt URL: https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json +- Dogfood verify public key: https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-public-key.txt + +## What it does + +- Reads a bounded `pr_diff` and optional `context`. +- Emits `findings[]`, `risk`, `test_gaps[]`, and `review_note`. +- Grounds findings in supplied diff paths and changed lines. +- Refuses empty or unparseable diffs instead of inventing review findings. +- Proposes a gated review-note Effect for the existing `pr-review-note` catalog skill. +- Does not fetch repositories, post comments, approve, request changes, push, or merge. + +## Harness and dogfood evidence + +- runx version: `runx-cli 0.6.13` +- Local harness command: `runx harness ./skills/code-review-note --receipt-dir --json` +- Local harness status: `passed` +- Hosted harness status: `passed` +- Harness cases: `risky-diff-yields-review-note, empty-diff-refused` +- Dogfood receipt ref: `runx:receipt:sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f` +- Dogfood receipt URL: https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json +- Dogfood verify public key: https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-public-key.txt +- Dogfood verify verdict: `runx verify --receipt dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json --json returned valid: true with valid digest, valid content_address, and valid production signature for kid runx-demo-key` +- Harness evidence: https://runx.ai/x/vidshidden/code-review-note#harness + +The sealed case produces four grounded findings for a risky refund diff, a high risk rating, two named test gaps, and a proposed review note. The refused case stops on an empty diff. The submitted dogfood receipt is a standalone post-publish dogfood receipt and is distinct from the hosted harness receipt ids. + +## Install and run + +```bash +runx add vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai +runx skill vidshidden/code-review-note@sha-d4b7c3ff5357 --registry https://api.runx.ai --json +runx verify --receipt --json +``` + +## Why a user would trust it + +- The package name, registry ref, PR files, raw artifacts, evidence, and report all describe `code-review-note@sha-d4b7c3ff5357`. +- The hosted registry harness passed after publish. +- The skill is read-only and mutation-free. +- The review note is explicitly gated; posting requires `pr.comment` authority through `pr-review-note`. +- Merge scope is refused and out of scope. +- The refusal path prevents low-evidence or empty diffs from producing fake confidence. diff --git a/skills/code-review-note/evidence/verification.json b/skills/code-review-note/evidence/verification.json new file mode 100644 index 00000000..99cf679c --- /dev/null +++ b/skills/code-review-note/evidence/verification.json @@ -0,0 +1,63 @@ +{ + "schema": "frantic.verification.v1", + "bounty": 59, + "claim_id": "8fb43f27-8b4d-4c89-b6c1-d26791943e37", + "package": "vidshidden/code-review-note@sha-d4b7c3ff5357", + "runx_version_output": "runx-cli 0.6.13", + "public_url": "https://runx.ai/x/vidshidden/code-review-note@sha-d4b7c3ff5357", + "hosted_harness": { + "status": "passed", + "case_count": 2, + "assertion_error_count": 0, + "assertion_errors": [], + "case_names": [ + "risky-diff-yields-review-note", + "empty-diff-refused" + ], + "receipt_ids": [ + "sha256:2cc4840a322ed96bc53ea705c3704d0632c07396aacb6a471e578658e4251f33", + "sha256:d0c128171918130c079167eef3a02c1f11a761d52f4a330c12818bc06064f7b2" + ], + "graph_case_count": 0, + "evidence_id": "runx-harness:vidshidden/code-review-note:sha-d4b7c3ff5357", + "evidence_url": "https://runx.ai/x/vidshidden/code-review-note#harness" + }, + "local_harness": { + "command": "runx harness ./skills/code-review-note --receipt-dir --json", + "status": "passed", + "case_count": 2, + "case_names": [ + "risky-diff-yields-review-note", + "empty-diff-refused" + ], + "receipt_ids": [ + "sha256:2cc4840a322ed96bc53ea705c3704d0632c07396aacb6a471e578658e4251f33", + "sha256:d0c128171918130c079167eef3a02c1f11a761d52f4a330c12818bc06064f7b2" + ], + "note": "The inline harness cases passed under the registry publish gate with an isolated receipt directory; a Linux GitHub Actions workflow was added to the PR branch for independent non-Windows reruns." + }, + "local_runner_check": { + "command": "node runx/skills/code-review-note/run.mjs with fixtures/risky-diff.json", + "status": "passed", + "observed": { + "finding_count": 4, + "risk_level": "high", + "test_gaps": [ + "missing_high_risk_regression_tests", + "missing_numeric_boundary_tests" + ], + "review_note_effect": "proposed", + "posting_skill": "pr-review-note", + "merge_scope": "refused" + } + }, + "dogfood_verify": { + "receipt_ref": "runx:receipt:sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f", + "receipt_file": "dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json", + "receipt_url": "https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-sha256-45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json", + "public_key_file": "dogfood-receipt-public-key.txt", + "public_key_url": "https://raw.githubusercontent.com/VidsHidden/runx/code-review-note/skills/code-review-note/evidence/dogfood-receipt-public-key.txt", + "verify_command": "runx verify --receipt dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json --json", + "verdict": "runx verify --receipt dogfood-receipts/sha256:45bc3b4f9d30d422339fbbe0df95e15ddb1ebf6d919500e6826c033c7e39020f.json --json returned valid: true with valid digest, valid content_address, and valid production signature for kid runx-demo-key" + } +} diff --git a/skills/code-review-note/fixtures/risky-diff.json b/skills/code-review-note/fixtures/risky-diff.json new file mode 100644 index 00000000..3e97dcce --- /dev/null +++ b/skills/code-review-note/fixtures/risky-diff.json @@ -0,0 +1,9 @@ +{ + "pr_diff": "diff --git a/src/payments/refunds.ts b/src/payments/refunds.ts\nindex 1111111..2222222 100644\n--- a/src/payments/refunds.ts\n+++ b/src/payments/refunds.ts\n@@ -12,11 +12,11 @@ export async function approveRefund(req, user) {\n- if (!user.roles.includes(\"refund_admin\")) {\n- throw new Error(\"forbidden\");\n- }\n- if (req.amountCents > policy.maxAutoRefundCents) {\n- throw new Error(\"manual review required\");\n- }\n+ // TODO: restore role check after support migration\n+ const amount = Number(req.amountCents);\n+ if (!amount) return { status: \"skipped\" };\n return refundGateway.create({\n chargeId: req.chargeId,\n- amountCents: req.amountCents,\n+ amountCents: amount,\n });\n }\n", + "context": { + "repository": "example/payments", + "pr_number": "42", + "title": "Relax refund approval path", + "test_policy": "Require authorization and threshold regression tests for payment changes." + } +} diff --git a/skills/code-review-note/run.mjs b/skills/code-review-note/run.mjs new file mode 100644 index 00000000..de28c317 --- /dev/null +++ b/skills/code-review-note/run.mjs @@ -0,0 +1,325 @@ +import fs from "node:fs"; + +const inputs = readInputs(); +const prDiff = stringValue(inputs.pr_diff); +const context = objectOrString(inputs.context); + +if (!prDiff || !hasDiffSignal(prDiff)) { + emit({ + schema: "code_review_note_packet.v1", + status: "refused", + refusal: { + reason_code: "invalid_input", + message: "pr_diff is empty or not a unified diff; refusing to invent review findings.", + }, + findings: [], + risk: { level: "unknown", rationale: "No parseable diff was supplied." }, + test_gaps: [], + review_note: null, + }); + process.exit(2); +} + +const parsed = parseDiff(prDiff); +if (parsed.files.length === 0) { + emit({ + schema: "code_review_note_packet.v1", + status: "refused", + refusal: { + reason_code: "invalid_input", + message: "No changed files were found in the supplied diff.", + }, + findings: [], + risk: { level: "unknown", rationale: "No changed files were supplied." }, + test_gaps: [], + review_note: null, + }); + process.exit(2); +} + +const findings = buildFindings(parsed); +const testGaps = buildTestGaps(parsed, findings, context); +const risk = summarizeRisk(findings, testGaps, parsed); +const reviewNote = buildReviewNote(findings, testGaps, risk, parsed, context); + +emit({ + schema: "code_review_note_packet.v1", + status: "sealed", + input: { + repository: field(context, "repository"), + pr_number: field(context, "pr_number"), + title: field(context, "title"), + diff_file_count: parsed.files.length, + }, + findings, + risk, + test_gaps: testGaps, + review_note: reviewNote, + guardrails: { + side_effects: "none", + posting_skill: "pr-review-note", + merge_scope: "refused", + grounding: "all findings cite changed files and visible diff lines", + }, +}); + +function readInputs() { + if (process.env.RUNX_INPUTS_PATH) { + return JSON.parse(fs.readFileSync(process.env.RUNX_INPUTS_PATH, "utf8")); + } + if (process.env.RUNX_INPUTS_JSON) { + return JSON.parse(process.env.RUNX_INPUTS_JSON); + } + return { + pr_diff: process.env.RUNX_INPUT_PR_DIFF, + context: parseMaybeJson(process.env.RUNX_INPUT_CONTEXT), + }; +} + +function parseMaybeJson(raw) { + if (raw === undefined || raw === "") return undefined; + try { + return JSON.parse(raw); + } catch { + return raw; + } +} + +function objectOrString(value) { + if (value && typeof value === "object" && !Array.isArray(value)) return value; + if (typeof value === "string") return { note: value }; + return {}; +} + +function stringValue(value) { + return typeof value === "string" ? value : null; +} + +function hasDiffSignal(diff) { + return diff.includes("diff --git ") || diff.includes("@@"); +} + +function parseDiff(diff) { + const files = []; + let current = null; + let oldLine = 0; + let newLine = 0; + + for (const line of diff.split(/\r?\n/)) { + const fileMatch = line.match(/^diff --git a\/(.+?) b\/(.+)$/); + if (fileMatch) { + current = { + old_path: fileMatch[1], + path: fileMatch[2], + hunks: [], + added: [], + removed: [], + }; + files.push(current); + continue; + } + if (!current) continue; + + const hunkMatch = line.match(/^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/); + if (hunkMatch) { + oldLine = Number(hunkMatch[1]); + newLine = Number(hunkMatch[2]); + current.hunks.push(line); + continue; + } + if (line.startsWith("+") && !line.startsWith("+++")) { + current.added.push({ line: newLine, text: line.slice(1) }); + newLine += 1; + continue; + } + if (line.startsWith("-") && !line.startsWith("---")) { + current.removed.push({ line: oldLine, text: line.slice(1) }); + oldLine += 1; + continue; + } + if (!line.startsWith("\\ No newline")) { + oldLine += 1; + newLine += 1; + } + } + return { files }; +} + +function buildFindings(parsed) { + const findings = []; + for (const file of parsed.files) { + const addedText = file.added.map((line) => line.text).join("\n"); + const removedText = file.removed.map((line) => line.text).join("\n"); + const visibleText = `${addedText}\n${removedText}`; + const paymentFile = /pay|refund|invoice|charge|billing/i.test(file.path); + + if (paymentFile && removedMatches(removedText, ["role", "permission", "authorize", "forbidden", "admin"])) { + findings.push(finding({ + severity: "high", + category: "authorization_regression", + file, + evidenceLine: firstRemoved(file, ["role", "permission", "authorize", "forbidden", "admin"]), + summary: "Payment-adjacent code removes an authorization guard.", + reproduction: "Call the changed path as a non-privileged user and verify whether the request can reach the payment/refund gateway.", + })); + } + + if (paymentFile && removedMatches(removedText, ["max", "threshold", "limit", "manual review", "policy"])) { + findings.push(finding({ + severity: "high", + category: "policy_threshold_regression", + file, + evidenceLine: firstRemoved(file, ["max", "threshold", "limit", "manual review", "policy"]), + summary: "Payment policy threshold handling was removed or weakened.", + reproduction: "Submit an over-threshold payment/refund request and confirm it is escalated instead of proposed automatically.", + })); + } + + if (/\bTODO\b|restore|temporary|after migration/i.test(addedText)) { + findings.push(finding({ + severity: paymentFile ? "medium" : "low", + category: "temporary_control_gap", + file, + evidenceLine: firstAdded(file, ["TODO", "restore", "temporary", "migration"]), + summary: "New code marks a control as temporary or deferred.", + reproduction: "Inspect the follow-up condition and add a regression test that fails while the control remains disabled.", + })); + } + + if (/Number\(|parseInt\(|parseFloat\(/.test(addedText) && !/Number\.isFinite|isNaN|zod|schema|validate/i.test(visibleText)) { + findings.push(finding({ + severity: "medium", + category: "unchecked_numeric_parse", + file, + evidenceLine: firstAdded(file, ["Number(", "parseInt(", "parseFloat("]), + summary: "Numeric parsing is added without an explicit finite-number validation path.", + reproduction: "Exercise the path with empty, NaN, negative, and string amount inputs and verify the result cannot bypass policy checks.", + })); + } + } + return findings; +} + +function buildTestGaps(parsed, findings, context) { + const changedPaths = parsed.files.map((file) => file.path); + const hasTests = changedPaths.some((path) => /(^|\/)(test|tests|__tests__)\/|(\.|-)(test|spec)\.[jt]sx?$/i.test(path)); + const gaps = []; + + if (!hasTests && findings.some((item) => item.severity === "high")) { + gaps.push({ + name: "missing_high_risk_regression_tests", + detail: "No test file changed while high-risk behavior changed.", + expected_coverage: "Add authorization, policy-threshold, and negative-input regression tests for the changed path.", + }); + } + if (findings.some((item) => item.category === "unchecked_numeric_parse")) { + gaps.push({ + name: "missing_numeric_boundary_tests", + detail: "The diff adds numeric parsing without visible boundary coverage.", + expected_coverage: "Cover zero, empty string, NaN, negative, and over-limit values.", + }); + } + const policy = field(context, "test_policy"); + if (policy && gaps.length === 0 && !hasTests) { + gaps.push({ + name: "policy_named_tests_not_visible", + detail: `Context test policy says: ${policy}`, + expected_coverage: "Show the policy-specific regression test in the PR diff or link it in review context.", + }); + } + return gaps; +} + +function summarizeRisk(findings, testGaps, parsed) { + const severities = findings.map((item) => item.severity); + let level = "low"; + if (severities.includes("high")) level = "high"; + else if (severities.includes("medium")) level = "medium"; + if (level === "low" && testGaps.length > 0) level = "medium"; + return { + level, + finding_count: findings.length, + changed_files: parsed.files.map((file) => file.path), + rationale: findings.length > 0 + ? `Risk is ${level} because ${findings.length} grounded finding(s) were visible in the supplied diff.` + : "Risk is low because no concrete blocker was visible in the supplied diff.", + }; +} + +function buildReviewNote(findings, testGaps, risk, parsed, context) { + const repo = field(context, "repository") ?? "unknown repository"; + const pr = field(context, "pr_number") ?? "unknown PR"; + const lines = []; + lines.push(`Review note for ${repo}#${pr}`); + lines.push(""); + lines.push(`Risk: ${risk.level} (${risk.finding_count} finding(s))`); + lines.push(""); + if (findings.length > 0) { + lines.push("Findings:"); + for (const item of findings) { + lines.push(`- [${item.severity}] ${item.file}:${item.line} ${item.summary}`); + lines.push(` Reproduction: ${item.reproduction}`); + } + } else { + lines.push("Findings: no blocking issue visible in the supplied diff."); + } + lines.push(""); + if (testGaps.length > 0) { + lines.push("Test gaps:"); + for (const gap of testGaps) { + lines.push(`- ${gap.name}: ${gap.expected_coverage}`); + } + } else { + lines.push("Test gaps: none visible from the supplied context."); + } + lines.push(""); + lines.push("Scope: proposed comment only; posting requires pr.comment authority through pr-review-note. Merge is out of scope."); + + return { + effect: "proposed", + catalog_skill: "pr-review-note", + required_scope: "pr.comment", + merge_scope: "refused", + target: { repository: repo, pr_number: pr }, + body: lines.join("\n"), + }; +} + +function finding({ severity, category, file, evidenceLine, summary, reproduction }) { + return { + severity, + category, + file: file.path, + line: evidenceLine?.line ?? null, + evidence: evidenceLine?.text ?? null, + summary, + reproduction, + source: "supplied_diff", + }; +} + +function removedMatches(text, patterns) { + return patterns.some((pattern) => new RegExp(pattern, "i").test(text)); +} + +function firstRemoved(file, needles) { + return firstLine(file.removed, needles); +} + +function firstAdded(file, needles) { + return firstLine(file.added, needles); +} + +function firstLine(lines, needles) { + return lines.find((line) => needles.some((needle) => line.text.toLowerCase().includes(needle.toLowerCase()))) ?? lines[0] ?? null; +} + +function field(context, key) { + const value = context?.[key]; + if (typeof value === "string" && value.trim()) return value.trim(); + return null; +} + +function emit(value) { + process.stdout.write(`${JSON.stringify(value, null, 2)}\n`); +}