ci: wire Claude + Gemini AI review (ai-review-prompts callers) - #701
Conversation
Adds the three thin caller workflows from the HarperFast/ai-review-prompts reusable set, pinned to main 1255f2e (#72, which added the repo-type/native-addon layer written for this repo): - claude-review.yml — layers: universal + repo-type/native-addon, plus repo-specific checks (pnpm/oxlint/oxfmt toolchain, no build tolerance, vendored-RocksDB bump handling, three-runtime-deps bar, harper-core consumer coupling). - gemini-review.yml — mirrors the Claude caller (opt-in via label unless GEMINI_ALWAYS_ON). - validate-caller-workflows.yml — SHA-pin + shadow-job guard; make the `validate` job a required check on main. Reviews post on the PR and log to HarperFast/ai-review-log (label repo:rocksdb-js, already created) once AI_REVIEW_LOG_TOKEN is set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
📊 Benchmark Resultsget-sync.bench.tsgetSync() > random keys - small key size (100 records)
getSync() > sequential keys - small key size (100 records)
ranges.bench.tsgetRange() > small range (100 records, 50 range)
realistic-load.bench.tsRealistic write load with workers > write variable records with transaction log
transaction-log.bench.tsTransaction log > read 100 iterators while write log with 100 byte records
Transaction log > read one entry from random position from log with 1000 100 byte records
worker-put-sync.bench.tsputSync() > random keys - small key size (100 records, 10 workers)
worker-transaction-log.bench.tsTransaction log with workers > write log with 100 byte records
Results from commit dabae25 |
kriszyp
left a comment
There was a problem hiding this comment.
LGTM — clean add-only AI-review CI wiring. Correct pull_request trigger (not pull_request_target), SHA-pinned action refs, and a fail-closed prompt validator, so no fork-PR token-exposure path. Reviewed via review-queue.
— Claude (Sonnet 5)
Mirror gemini-review.yml: grant contents:read + pull-requests:write + id-token:write at the calling-job level (the union of what the reusable's authorize and review jobs declare). Without the block the grants come from the repo's default-workflow-permissions setting — GitHub silently intersects the reusable's requests with that ceiling, so pull-requests:write survives only while the repo default is "write". Explicit grants make the caller independent of that setting. Addresses Barber AI review feedback on #701. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Follow-ups from the review feedback here, for the paper trail:
|
kriszyp
left a comment
There was a problem hiding this comment.
Thin AI-review callers look safe — pull_request (not pull_request_target), explicit per-secret passing (no blanket inherit), job-level least-privilege perms, and dual 40-char SHA pins. One thing to file as a follow-up (not blocking): the shadow-job/mutable-ref validator globs only claude-*.yml, so the gemini-review.yml caller with the same write + id-token grants isn't covered — an upstream ai-review-prompts limitation. Approving.
🤖 Reviewed with KrAIs (Claude Opus 4.8)
What
Wires @harperfast/rocksdb-js into the AI review pipeline — the Phase-1 expansion target from the 07-02 retro (52 PRs/30d, highest risk-per-line of the candidate repos). Three thin callers of the ai-review-prompts reusables, pinned to main
1255f2e(#72 — therepo-type/native-addonlayer written for this repo):universal+repo-type/native-addon; repo-specific checks for the pnpm/oxlint/oxfmt toolchain, vendored-RocksDB bumps, the three-runtime-deps bar, and the harper-core consumer coupling (MaybePromise / key-encoding changes need a core reference).gemini-reviewlabel unlessGEMINI_ALWAYS_ON=true.Already done outside this PR:
repo:rocksdb-jslabel in ai-review-log;claude-review/gemini-reviewlabels here.Setup needed on merge (repo settings — can't be done from a PR)
ANTHROPIC_API_KEY(required);GEMINI_API_KEY+AI_REVIEW_LOG_TOKEN(optional — missing keys skip cleanly; the log token reuses the existing fine-grained PAT). Add this repo to the visibility list of the org-levelHARPERFAST_AI_CLIENT_ID/HARPERFAST_AI_APP_PRIVATE_KEYsecrets.CLAUDE_ALWAYS_ON=trueto auto-review trusted-author PRs (or leave unset for label-only opt-in while calibrating).validate / validatea required status check onmain.Note: the Claude review check on this PR will fail with the expected "workflow file must match default branch" guard (it always does on PRs that introduce the caller workflows — claude-code-action's anti-tamper). Ignore it; it clears after merge.
🤖 Generated with Claude Code